Skip to content

Commit 7241d6b

Browse files
feanilclaude
andcommitted
refactor: move third_party_auth settings from app startup to static config
This refactoring moves all third-party authentication settings from runtime configuration during AppConfig.ready() to static definitions in lms/envs/common.py. ## Problem The third_party_auth app used an `apply_settings()` function called during Django's app initialization to modify Django settings. This pattern caused issues: 1. Settings were modified after Django initialization when they should be finalized, making debugging difficult 2. Operators couldn't override these settings in their YAML config files because apply_settings() would overwrite their values ## Why the ENABLE_THIRD_PARTY_AUTH conditional was removed The settings are now defined unconditionally because: 1. These settings are inert when social auth backends aren't configured in AUTHENTICATION_BACKENDS - they have no effect 2. ENABLE_THIRD_PARTY_AUTH still controls what matters: registration of authentication backends and exposure of auth URLs 3. This follows standard Django patterns where middleware and settings exist but are no-ops when their feature isn't active ## Enterprise pipeline integration The enterprise pipeline step (handle_enterprise_logistration) is included statically in SOCIAL_AUTH_PIPELINE rather than being inserted dynamically. This step handles its own runtime checks and returns early if enterprise is not configured, making it safe to include always. This avoids complexity with Derived() functions that would need to import Django models at settings load time (before apps are ready). ## Operator impact Operators can now properly override any of these settings in their YAML configuration files, including SOCIAL_AUTH_PIPELINE for custom flows. Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 6cb2ea3 commit 7241d6b

File tree

4 files changed

+169
-184
lines changed

4 files changed

+169
-184
lines changed
Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# lint-amnesty, pylint: disable=missing-module-docstring
22

33
from django.apps import AppConfig
4-
from django.conf import settings
54

65

76
class ThirdPartyAuthConfig(AppConfig): # lint-amnesty, pylint: disable=missing-class-docstring
@@ -11,16 +10,4 @@ class ThirdPartyAuthConfig(AppConfig): # lint-amnesty, pylint: disable=missing-
1110
def ready(self):
1211
# Import signal handlers to register them
1312
from .signals import handlers # noqa: F401 pylint: disable=unused-import
14-
15-
# To override the settings before loading social_django.
16-
if settings.FEATURES.get('ENABLE_THIRD_PARTY_AUTH', False):
17-
self._enable_third_party_auth()
18-
19-
def _enable_third_party_auth(self):
20-
"""
21-
Enable the use of third_party_auth, which allows users to sign in to edX
22-
using other identity providers. For configuration details, see
23-
common/djangoapps/third_party_auth/settings.py.
24-
"""
25-
from common.djangoapps.third_party_auth import settings as auth_settings
26-
auth_settings.apply_settings(settings)
13+
# Note: Third-party auth settings are now defined statically in lms/envs/common.py

common/djangoapps/third_party_auth/settings.py

Lines changed: 0 additions & 107 deletions
This file was deleted.
Lines changed: 98 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,67 +1,102 @@
1-
"""Unit tests for settings.py."""
1+
"""Unit tests for third-party auth settings in lms/envs/common.py."""
22

3-
from unittest.mock import patch
4-
from common.djangoapps.third_party_auth import provider, settings
5-
from common.djangoapps.third_party_auth.tests import testutil
3+
from django.conf import settings
4+
from django.test import TestCase, override_settings
5+
6+
from common.djangoapps.third_party_auth import provider
67
from common.djangoapps.third_party_auth.tests.utils import skip_unless_thirdpartyauth
7-
_ORIGINAL_AUTHENTICATION_BACKENDS = ['first_authentication_backend']
8-
_ORIGINAL_INSTALLED_APPS = ['first_installed_app']
9-
_ORIGINAL_MIDDLEWARE_CLASSES = ['first_middleware_class']
10-
_ORIGINAL_TEMPLATE_CONTEXT_PROCESSORS = ['first_template_context_preprocessor']
11-
_SETTINGS_MAP = {
12-
'AUTHENTICATION_BACKENDS': _ORIGINAL_AUTHENTICATION_BACKENDS,
13-
'INSTALLED_APPS': _ORIGINAL_INSTALLED_APPS,
14-
'MIDDLEWARE': _ORIGINAL_MIDDLEWARE_CLASSES,
15-
'TEMPLATES': [{
16-
'OPTIONS': {
17-
'context_processors': _ORIGINAL_TEMPLATE_CONTEXT_PROCESSORS
18-
}
19-
}],
20-
'FEATURES': {},
21-
}
22-
_SETTINGS_MAP['DEFAULT_TEMPLATE_ENGINE'] = _SETTINGS_MAP['TEMPLATES'][0]
23-
24-
25-
class SettingsUnitTest(testutil.TestCase):
26-
"""Unit tests for settings management code."""
27-
28-
# Suppress spurious no-member warning on fakes.
29-
# pylint: disable=no-member
30-
31-
def setUp(self):
32-
super().setUp()
33-
self.settings = testutil.FakeDjangoSettings(_SETTINGS_MAP)
34-
35-
def test_apply_settings_adds_exception_middleware(self):
36-
settings.apply_settings(self.settings)
37-
assert 'common.djangoapps.third_party_auth.middleware.ExceptionMiddleware' in self.settings.MIDDLEWARE
38-
39-
def test_apply_settings_adds_fields_stored_in_session(self):
40-
settings.apply_settings(self.settings)
41-
assert ['auth_entry', 'next'] == self.settings.FIELDS_STORED_IN_SESSION
8+
9+
10+
class SettingsUnitTest(TestCase):
11+
"""Unit tests for third-party auth settings defined in lms/envs/common.py."""
12+
13+
def test_exception_middleware_in_middleware_list(self):
14+
"""Verify ExceptionMiddleware is included in MIDDLEWARE."""
15+
assert 'common.djangoapps.third_party_auth.middleware.ExceptionMiddleware' in settings.MIDDLEWARE
16+
17+
def test_fields_stored_in_session_defined(self):
18+
"""Verify FIELDS_STORED_IN_SESSION is defined with expected values."""
19+
assert settings.FIELDS_STORED_IN_SESSION == ['auth_entry', 'next']
4220

4321
@skip_unless_thirdpartyauth()
44-
def test_apply_settings_enables_no_providers_by_default(self):
45-
# Providers are only enabled via ConfigurationModels in the database
46-
settings.apply_settings(self.settings)
47-
assert [] == provider.Registry.enabled()
48-
49-
def test_apply_settings_turns_off_raising_social_exceptions(self):
50-
# Guard against submitting a conf change that's convenient in dev but
51-
# bad in prod.
52-
settings.apply_settings(self.settings)
53-
assert not self.settings.SOCIAL_AUTH_RAISE_EXCEPTIONS
54-
55-
def test_apply_settings_turns_off_redirect_sanitization(self):
56-
settings.apply_settings(self.settings)
57-
assert not self.settings.SOCIAL_AUTH_SANITIZE_REDIRECTS
58-
59-
def test_apply_settings_avoids_default_username_check(self):
60-
# Avoid the default username check where non-ascii characters are not
61-
# allowed when unicode username is enabled
62-
settings.apply_settings(self.settings)
63-
assert self.settings.SOCIAL_AUTH_CLEAN_USERNAMES
64-
# verify default behavior
65-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_UNICODE_USERNAME': True}):
66-
settings.apply_settings(self.settings)
67-
assert not self.settings.SOCIAL_AUTH_CLEAN_USERNAMES
22+
def test_no_providers_enabled_by_default(self):
23+
"""Providers are only enabled via ConfigurationModels in the database."""
24+
assert provider.Registry.enabled() == []
25+
26+
def test_social_auth_raise_exceptions_is_false(self):
27+
"""Guard against submitting a conf change that's convenient in dev but bad in prod."""
28+
assert settings.SOCIAL_AUTH_RAISE_EXCEPTIONS is False
29+
30+
def test_social_auth_sanitize_redirects_is_false(self):
31+
"""Verify redirect sanitization is disabled (platform does its own)."""
32+
assert settings.SOCIAL_AUTH_SANITIZE_REDIRECTS is False
33+
34+
def test_social_auth_login_error_url(self):
35+
"""Verify SOCIAL_AUTH_LOGIN_ERROR_URL is set."""
36+
assert settings.SOCIAL_AUTH_LOGIN_ERROR_URL == '/'
37+
38+
def test_social_auth_login_redirect_url(self):
39+
"""Verify SOCIAL_AUTH_LOGIN_REDIRECT_URL is set."""
40+
assert settings.SOCIAL_AUTH_LOGIN_REDIRECT_URL == '/dashboard'
41+
42+
def test_social_auth_strategy(self):
43+
"""Verify SOCIAL_AUTH_STRATEGY is set to use ConfigurationModelStrategy."""
44+
assert settings.SOCIAL_AUTH_STRATEGY == 'common.djangoapps.third_party_auth.strategy.ConfigurationModelStrategy'
45+
46+
def test_social_auth_pipeline_defined(self):
47+
"""Verify SOCIAL_AUTH_PIPELINE is defined and includes expected steps."""
48+
pipeline = settings.SOCIAL_AUTH_PIPELINE
49+
assert isinstance(pipeline, list)
50+
assert len(pipeline) > 0
51+
# Verify some key pipeline steps are present
52+
assert 'common.djangoapps.third_party_auth.pipeline.parse_query_params' in pipeline
53+
assert 'social_core.pipeline.user.create_user' in pipeline
54+
assert 'common.djangoapps.third_party_auth.pipeline.ensure_redirect_url_is_safe' in pipeline
55+
56+
def test_social_auth_context_processors(self):
57+
"""Verify social_django context processors are included."""
58+
# CONTEXT_PROCESSORS is used to build TEMPLATES, so check there
59+
context_processors = settings.TEMPLATES[0]['OPTIONS']['context_processors']
60+
assert 'social_django.context_processors.backends' in context_processors
61+
assert 'social_django.context_processors.login_redirect' in context_processors
62+
63+
@override_settings(FEATURES={'ENABLE_UNICODE_USERNAME': False})
64+
def test_social_auth_clean_usernames_default(self):
65+
"""Verify SOCIAL_AUTH_CLEAN_USERNAMES is True when unicode usernames disabled."""
66+
# Note: SOCIAL_AUTH_CLEAN_USERNAMES is a Derived setting, computed at settings load time.
67+
# This test verifies the default behavior (unicode usernames disabled).
68+
assert settings.SOCIAL_AUTH_CLEAN_USERNAMES is True
69+
70+
def test_social_auth_clean_usernames_computation(self):
71+
"""
72+
Verify the SOCIAL_AUTH_CLEAN_USERNAMES computation logic.
73+
74+
SOCIAL_AUTH_CLEAN_USERNAMES is a Derived setting that is computed at settings load time,
75+
so we can't use @override_settings to test both cases. Instead, we test the computation
76+
logic directly to ensure it correctly inverts the ENABLE_UNICODE_USERNAME feature flag.
77+
"""
78+
# The logic in lms/envs/common.py is:
79+
# SOCIAL_AUTH_CLEAN_USERNAMES = Derived(
80+
# lambda settings: not settings.FEATURES.get('ENABLE_UNICODE_USERNAME', False)
81+
# )
82+
# We replicate and test that logic here.
83+
84+
class FakeSettings:
85+
"""Fake settings object for testing the Derived computation."""
86+
def __init__(self, features):
87+
self.FEATURES = features
88+
89+
# When ENABLE_UNICODE_USERNAME is False (default), SOCIAL_AUTH_CLEAN_USERNAMES should be True
90+
fake_settings = FakeSettings({'ENABLE_UNICODE_USERNAME': False})
91+
result = not fake_settings.FEATURES.get('ENABLE_UNICODE_USERNAME', False)
92+
assert result is True
93+
94+
# When ENABLE_UNICODE_USERNAME is True, SOCIAL_AUTH_CLEAN_USERNAMES should be False
95+
fake_settings = FakeSettings({'ENABLE_UNICODE_USERNAME': True})
96+
result = not fake_settings.FEATURES.get('ENABLE_UNICODE_USERNAME', False)
97+
assert result is False
98+
99+
# When ENABLE_UNICODE_USERNAME is not set, should default to False, so result is True
100+
fake_settings = FakeSettings({})
101+
result = not fake_settings.FEATURES.get('ENABLE_UNICODE_USERNAME', False)
102+
assert result is True

lms/envs/common.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,69 @@
300300
# .. toggle_creation_date: 2014-09-15
301301
ENABLE_THIRD_PARTY_AUTH = False
302302

303+
# Third-party auth settings for python-social-auth
304+
# These are defined unconditionally; they only take effect when
305+
# AUTHENTICATION_BACKENDS includes social auth backends.
306+
307+
# Where to send the user if there's an error during social authentication
308+
SOCIAL_AUTH_LOGIN_ERROR_URL = '/'
309+
# Where to send the user once social authentication is successful
310+
SOCIAL_AUTH_LOGIN_REDIRECT_URL = '/dashboard'
311+
# Disable sanitizing of redirect urls in social-auth since the platform
312+
# already does its own sanitization via the LOGIN_REDIRECT_WHITELIST setting.
313+
SOCIAL_AUTH_SANITIZE_REDIRECTS = False
314+
# Adding extra key value pair in the url query string for microsoft as per request
315+
SOCIAL_AUTH_AZUREAD_OAUTH2_AUTH_EXTRA_ARGUMENTS = {'msafed': 0}
316+
# Required so that we can use unmodified PSA OAuth2 backends:
317+
SOCIAL_AUTH_STRATEGY = 'common.djangoapps.third_party_auth.strategy.ConfigurationModelStrategy'
318+
# We let the user specify their email address during signup.
319+
SOCIAL_AUTH_PROTECTED_USER_FIELDS = ['email']
320+
# Disable exceptions by default for prod so you get redirect behavior
321+
# instead of a Django error page.
322+
SOCIAL_AUTH_RAISE_EXCEPTIONS = False
323+
# Clean username to make sure username is compatible with our system requirements
324+
SOCIAL_AUTH_CLEAN_USERNAME_FUNCTION = 'common.djangoapps.third_party_auth.models.clean_username'
325+
# Allow users to login using social auth even if their account is not verified yet
326+
SOCIAL_AUTH_INACTIVE_USER_LOGIN = True
327+
SOCIAL_AUTH_INACTIVE_USER_URL = '/auth/inactive'
328+
SOCIAL_AUTH_UUID_LENGTH = 10
329+
# Whitelisted URL query parameters retained in the pipeline session.
330+
FIELDS_STORED_IN_SESSION = ['auth_entry', 'next']
331+
332+
# Computed setting: disable clean usernames check when unicode usernames are enabled
333+
SOCIAL_AUTH_CLEAN_USERNAMES = Derived(
334+
lambda settings: not settings.FEATURES.get('ENABLE_UNICODE_USERNAME', False)
335+
)
336+
337+
# Social auth pipeline for third-party authentication.
338+
# Operators can override SOCIAL_AUTH_PIPELINE directly in their settings
339+
# to customize the pipeline.
340+
# The enterprise step (handle_enterprise_logistration) handles its own checks at runtime
341+
# and is a no-op if enterprise is not enabled.
342+
SOCIAL_AUTH_PIPELINE = [
343+
'common.djangoapps.third_party_auth.pipeline.parse_query_params',
344+
'social_core.pipeline.social_auth.social_details',
345+
'social_core.pipeline.social_auth.social_uid',
346+
'social_core.pipeline.social_auth.auth_allowed',
347+
'social_core.pipeline.social_auth.social_user',
348+
'common.djangoapps.third_party_auth.pipeline.associate_by_email_if_login_api',
349+
'common.djangoapps.third_party_auth.pipeline.associate_by_email_if_saml',
350+
'common.djangoapps.third_party_auth.pipeline.associate_by_email_if_oauth',
351+
'common.djangoapps.third_party_auth.pipeline.get_username',
352+
'common.djangoapps.third_party_auth.pipeline.set_pipeline_timeout',
353+
'common.djangoapps.third_party_auth.pipeline.ensure_user_information',
354+
'social_core.pipeline.user.create_user',
355+
'social_core.pipeline.social_auth.associate_user',
356+
'enterprise.tpa_pipeline.handle_enterprise_logistration',
357+
'social_core.pipeline.social_auth.load_extra_data',
358+
'social_core.pipeline.user.user_details',
359+
'common.djangoapps.third_party_auth.pipeline.user_details_force_sync',
360+
'common.djangoapps.third_party_auth.pipeline.set_id_verification_status',
361+
'common.djangoapps.third_party_auth.pipeline.set_logged_in_cookies',
362+
'common.djangoapps.third_party_auth.pipeline.login_analytics',
363+
'common.djangoapps.third_party_auth.pipeline.ensure_redirect_url_is_safe',
364+
]
365+
303366
# Prevent concurrent logins per user
304367
PREVENT_CONCURRENT_LOGINS = True
305368

@@ -799,6 +862,10 @@
799862

800863
# Context processor necessary for the survey report message appear on the admin site
801864
'openedx.features.survey_report.context_processors.admin_extra_context',
865+
866+
# Third-party auth context processors for social_django
867+
'social_django.context_processors.backends',
868+
'social_django.context_processors.login_redirect',
802869
]
803870

804871
DEFAULT_TEMPLATE_ENGINE_DIRS = Derived(lambda settings: settings.TEMPLATES[0]['DIRS'][:])
@@ -1211,6 +1278,9 @@
12111278
# Handles automatically storing user ids in django-simple-history tables when possible.
12121279
'simple_history.middleware.HistoryRequestMiddleware',
12131280

1281+
# Third-party auth exception handling for social auth redirects
1282+
'common.djangoapps.third_party_auth.middleware.ExceptionMiddleware',
1283+
12141284
# This must be last
12151285
'openedx.core.djangoapps.site_configuration.middleware.SessionCookieDomainOverrideMiddleware',
12161286
]

0 commit comments

Comments
 (0)