Skip to content

Commit 01dbd3d

Browse files
authored
feat: replace enterprise_support import with AccountSettingsReadOnlyFields filter
1 parent 1f596eb commit 01dbd3d

9 files changed

Lines changed: 76 additions & 88 deletions

File tree

lms/envs/common.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3705,6 +3705,18 @@ def _should_send_certificate_events(settings):
37053705
# The project ID should be obtained from the Google Cloud Console when creating a reCAPTCHA
37063706
RECAPTCHA_PROJECT_ID = None
37073707

3708+
# .. setting_name: OPEN_EDX_FILTERS_CONFIG
3709+
# .. setting_default: {"org.openedx.learning.account.settings.read_only_fields.requested.v1": {"fail_silently": true, "pipeline": ["enterprise.filters.accounts.AccountSettingsReadOnlyFieldsStep"]}}
3710+
# .. setting_description: Configuration dict for openedx-filters pipeline steps.
3711+
# Keys are filter type strings; values are dicts with 'fail_silently' (bool) and
3712+
# 'pipeline' (list of dotted-path strings to PipelineStep subclasses).
3713+
OPEN_EDX_FILTERS_CONFIG = {
3714+
"org.openedx.learning.account.settings.read_only_fields.requested.v1": {
3715+
"fail_silently": True,
3716+
"pipeline": ["enterprise.filters.accounts.AccountSettingsReadOnlyFieldsStep"],
3717+
},
3718+
}
3719+
37083720
############################## Miscellaneous ###############################
37093721

37103722
# To limit the number of courses displayed on learner dashboard

lms/envs/production.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ def get_env_setting(setting):
8484
'EVENT_BUS_PRODUCER_CONFIG',
8585
'DEFAULT_FILE_STORAGE',
8686
'STATICFILES_STORAGE',
87+
'OPEN_EDX_FILTERS_CONFIG',
8788
]
8889
})
8990

@@ -281,6 +282,19 @@ def get_env_setting(setting):
281282
EVENT_TRACKING_SEGMENTIO_EMIT_WHITELIST
282283
)
283284

285+
# Merge OPEN_EDX_FILTERS_CONFIG from YAML into the default defined in common.py.
286+
# Pipeline steps from YAML are appended after steps defined in common.py.
287+
# The fail_silently value from YAML takes precedence over the one in common.py.
288+
for _filter_type, _filter_config in _YAML_TOKENS.get('OPEN_EDX_FILTERS_CONFIG', {}).items():
289+
if _filter_type in OPEN_EDX_FILTERS_CONFIG: # noqa: F405
290+
OPEN_EDX_FILTERS_CONFIG[_filter_type]['pipeline'].extend( # noqa: F405
291+
_filter_config.get('pipeline', [])
292+
)
293+
if 'fail_silently' in _filter_config:
294+
OPEN_EDX_FILTERS_CONFIG[_filter_type]['fail_silently'] = _filter_config['fail_silently'] # noqa: F405
295+
else:
296+
OPEN_EDX_FILTERS_CONFIG[_filter_type] = _filter_config # noqa: F405
297+
284298
if ENABLE_THIRD_PARTY_AUTH:
285299
AUTHENTICATION_BACKENDS = _YAML_TOKENS.get('THIRD_PARTY_AUTH_BACKENDS', [
286300
'social_core.backends.google.GoogleOAuth2',

openedx/core/djangoapps/user_api/accounts/api.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from django.utils.translation import gettext as _
1313
from django.utils.translation import override as override_language
1414
from eventtracking import tracker
15+
from openedx_filters.learning.filters import AccountSettingsReadOnlyFieldsRequested
1516
from pytz import UTC
1617

1718
from common.djangoapps.student import views as student_views
@@ -39,7 +40,6 @@
3940
from openedx.core.djangoapps.user_authn.views.registration_form import validate_name, validate_username
4041
from openedx.core.lib.api.view_utils import add_serializer_errors
4142
from common.djangoapps.third_party_auth.utils import get_saml_provider_for_user
42-
from openedx.features.enterprise_support.utils import get_enterprise_readonly_account_fields
4343
from openedx.features.name_affirmation_api.utils import is_name_affirmation_installed
4444

4545
from .serializers import AccountLegacyProfileSerializer, AccountUserSerializer, UserReadOnlySerializer, _visible_fields
@@ -194,11 +194,19 @@ def update_account_settings(requesting_user, update, username=None):
194194

195195
def _validate_read_only_fields(user, data, field_errors):
196196
# Check for fields that are not editable. Marking them read-only causes them to be ignored, but we wish to 400.
197+
# .. filter_implemented_name: AccountSettingsReadOnlyFieldsRequested
198+
# .. filter_type: org.openedx.learning.account.settings.read_only_fields.requested.v1
199+
plugin_readonly_fields, __ = AccountSettingsReadOnlyFieldsRequested.run_filter(
200+
readonly_fields=set(),
201+
user=user,
202+
)
203+
plugin_readonly_fields = plugin_readonly_fields or set()
204+
197205
read_only_fields = set(data.keys()).intersection(
198206
# Remove email since it is handled separately below when checking for changing_email.
199207
(set(AccountUserSerializer.get_read_only_fields()) - {"email"}) |
200208
set(AccountLegacyProfileSerializer.get_read_only_fields() or set()) |
201-
get_enterprise_readonly_account_fields(user)
209+
plugin_readonly_fields
202210
)
203211

204212
for read_only_field in read_only_fields:

openedx/core/djangoapps/user_api/accounts/tests/test_api.py

Lines changed: 35 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
"""
55

66
import datetime
7-
import itertools
87
import unicodedata
98
from unittest.mock import Mock, patch
109

@@ -14,11 +13,10 @@
1413
from django.contrib.auth.hashers import make_password
1514
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
1615
from django.http import HttpResponse
17-
from django.test import TestCase
16+
from django.test import TestCase, override_settings
1817
from django.test.client import RequestFactory
1918
from django.urls import reverse
2019
from pytz import UTC
21-
from social_django.models import UserSocialAuth
2220

2321
from common.djangoapps.student.models import (
2422
AccountRecovery,
@@ -51,6 +49,7 @@
5149
)
5250
from openedx.core.djangolib.testing.utils import skip_unless_lms
5351
from openedx.features.enterprise_support.tests.factories import EnterpriseCustomerUserFactory
52+
from openedx_filters import PipelineStep
5453

5554

5655
def mock_render_to_string(template_name, context):
@@ -67,6 +66,18 @@ def mock_render_to_response(template_name):
6766
return HttpResponse(template_name)
6867

6968

69+
class TestAccountSettingsReadOnlyFieldsPipelineStep(PipelineStep):
70+
"""
71+
Test pipeline step for AccountSettingsReadOnlyFieldsRequested.
72+
"""
73+
74+
def run_filter(self, user, readonly_fields, **kwargs): # pylint: disable=arguments-differ
75+
return {
76+
"readonly_fields": {"name"},
77+
"updated_account_settings": None,
78+
}
79+
80+
7081
class CreateAccountMixin: # lint-amnesty, pylint: disable=missing-class-docstring
7182
def create_account(self, username, password, email):
7283
# pylint: disable=missing-docstring
@@ -104,11 +115,6 @@ def setUp(self):
104115
self.staff_user = UserFactory(is_staff=True, password=self.password)
105116
self.reset_tracker()
106117

107-
enterprise_patcher = patch('openedx.features.enterprise_support.api.enterprise_customer_for_request')
108-
enterprise_learner_patcher = enterprise_patcher.start()
109-
enterprise_learner_patcher.return_value = {}
110-
self.addCleanup(enterprise_learner_patcher.stop)
111-
112118
def test_get_username_provided(self):
113119
"""Test the difference in behavior when a username is supplied to get_account_settings."""
114120
account_settings = get_account_settings(self.default_request)[0]
@@ -248,73 +254,26 @@ def test_update_success_for_enterprise(self):
248254
account_settings = get_account_settings(self.default_request)[0]
249255
assert level_of_education == account_settings['level_of_education']
250256

251-
@patch('openedx.features.enterprise_support.api.enterprise_customer_for_request')
252-
@patch('openedx.features.enterprise_support.utils.third_party_auth.provider.Registry.get')
253-
@ddt.data(
254-
*itertools.product(
255-
# field_name_value values
256-
(("email", "new_email@example.com"), ("name", "new name"), ("country", "IN")),
257-
# is_enterprise_user
258-
(True, False),
259-
# is_synch_learner_profile_data
260-
(True, False),
261-
# has `UserSocialAuth` record
262-
(True, False),
263-
)
264-
)
265-
@ddt.unpack
266-
def test_update_validation_error_for_enterprise(
267-
self,
268-
field_name_value,
269-
is_enterprise_user,
270-
is_synch_learner_profile_data,
271-
has_user_social_auth_record,
272-
mock_auth_provider,
273-
mock_customer,
274-
):
275-
idp_backend_name = 'tpa-saml'
276-
mock_customer.return_value = {}
277-
if is_enterprise_user:
278-
mock_customer.return_value.update({
279-
'uuid': 'real-ent-uuid',
280-
'name': 'Dummy Enterprise',
281-
'identity_provider': 'saml-ubc',
282-
'identity_providers': [
283-
{
284-
"provider_id": "saml-ubc",
285-
}
257+
@override_settings(
258+
OPEN_EDX_FILTERS_CONFIG={
259+
"org.openedx.learning.account.settings.read_only_fields.requested.v1": {
260+
"pipeline": [
261+
"openedx.core.djangoapps.user_api.accounts.tests.test_api.TestAccountSettingsReadOnlyFieldsPipelineStep",
286262
],
287-
})
288-
mock_auth_provider.return_value.sync_learner_profile_data = is_synch_learner_profile_data
289-
mock_auth_provider.return_value.backend_name = idp_backend_name
290-
291-
update_data = {field_name_value[0]: field_name_value[1]}
263+
"fail_silently": False,
264+
},
265+
},
266+
)
267+
def test_update_name_blocked_by_account_settings_read_only_fields_filter(self):
268+
"""
269+
Test that update_account_settings honors the configured read-only-fields filter.
270+
"""
271+
with pytest.raises(AccountValidationError) as context_manager:
272+
update_account_settings(self.user, {"name": "Blocked Name"})
292273

293-
user_fullname_editable = False
294-
if has_user_social_auth_record:
295-
UserSocialAuth.objects.create(
296-
provider=idp_backend_name,
297-
user=self.user
298-
)
299-
else:
300-
UserSocialAuth.objects.all().delete()
301-
# user's fullname is editable if no `UserSocialAuth` record exists
302-
user_fullname_editable = field_name_value[0] == 'name'
303-
304-
# prevent actual email change requests
305-
with patch('openedx.core.djangoapps.user_api.accounts.api.student_views.do_email_change_request'):
306-
# expect field un-editability only when all of the following conditions are met
307-
if is_enterprise_user and is_synch_learner_profile_data and not user_fullname_editable:
308-
with pytest.raises(AccountValidationError) as validation_error:
309-
update_account_settings(self.user, update_data)
310-
field_errors = validation_error.value.field_errors
311-
assert 'This field is not editable via this API' == \
312-
field_errors[field_name_value[0]]['developer_message']
313-
else:
314-
update_account_settings(self.user, update_data)
315-
account_settings = get_account_settings(self.default_request)[0]
316-
if field_name_value[0] != "email":
317-
assert field_name_value[1] == account_settings[field_name_value[0]]
274+
field_errors = context_manager.value.field_errors
275+
assert "name" in field_errors
276+
assert field_errors["name"]["developer_message"] == "This field is not editable via this API"
318277

319278
def test_update_error_validating(self):
320279
"""Test that AccountValidationError is thrown if incorrect values are supplied."""
@@ -472,17 +431,12 @@ def test_email_changes_disabled(self):
472431
assert 'Email address changes have been disabled' in context_manager.value.developer_message
473432

474433
@patch.dict(settings.FEATURES, dict(ALLOW_EMAIL_ADDRESS_CHANGE=True))
475-
def test_email_changes_blocked_by_saml_provider(self):
434+
@patch('openedx.core.djangoapps.user_api.accounts.api.get_saml_provider_for_user')
435+
def test_email_changes_blocked_by_saml_provider(self, mock_get_saml_provider):
476436
"""
477437
Test that email changes are rejected when the user's SAML provider has disable_email_editing=True.
478438
"""
479-
from common.djangoapps.third_party_auth.tests.factories import SAMLProviderConfigFactory
480-
saml_config = SAMLProviderConfigFactory(disable_email_editing=True)
481-
UserSocialAuth.objects.create(
482-
user=self.user,
483-
provider='tpa-saml',
484-
uid=f'{saml_config.slug}:remote-user-id',
485-
)
439+
mock_get_saml_provider.return_value = Mock(disable_email_editing=True)
486440

487441
with pytest.raises(AccountValidationError) as context_manager:
488442
update_account_settings(self.user, {"email": "new@example.com"})

requirements/edx/base.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,7 @@ openedx-events==10.5.0
831831
# edx-name-affirmation
832832
# event-tracking
833833
# ora2
834-
openedx-filters==2.1.0
834+
openedx-filters==3.3.0
835835
# via
836836
# -r requirements/edx/kernel.in
837837
# edx-enterprise

requirements/edx/development.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1380,7 +1380,7 @@ openedx-events==10.5.0
13801380
# edx-name-affirmation
13811381
# event-tracking
13821382
# ora2
1383-
openedx-filters==2.1.0
1383+
openedx-filters==3.3.0
13841384
# via
13851385
# -r requirements/edx/doc.txt
13861386
# -r requirements/edx/testing.txt

requirements/edx/doc.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,7 @@ openedx-events==10.5.0
10051005
# edx-name-affirmation
10061006
# event-tracking
10071007
# ora2
1008-
openedx-filters==2.1.0
1008+
openedx-filters==3.3.0
10091009
# via
10101010
# -r requirements/edx/base.txt
10111011
# edx-enterprise

requirements/edx/kernel.in

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ openedx-atlas # CLI tool to manage translations
118118
openedx-calc # Library supporting mathematical calculations for Open edX
119119
openedx-django-require
120120
openedx-events # Open edX Events from Hooks Extension Framework (OEP-50)
121-
openedx-filters # Open edX Filters from Hooks Extension Framework (OEP-50)
121+
openedx-filters>=3.1.0 # Open edX Filters from Hooks Extension Framework (OEP-50)
122122
openedx-forum # Open edX forum v2 application
123123
openedx-learning # Open edX Learning core (experimental)
124124
openedx-django-wiki

requirements/edx/testing.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1050,7 +1050,7 @@ openedx-events==10.5.0
10501050
# edx-name-affirmation
10511051
# event-tracking
10521052
# ora2
1053-
openedx-filters==2.1.0
1053+
openedx-filters==3.3.0
10541054
# via
10551055
# -r requirements/edx/base.txt
10561056
# edx-enterprise

0 commit comments

Comments
 (0)