refactor: move third_party_auth settings from app startup to static config#37965
Open
refactor: move third_party_auth settings from app startup to static config#37965
Conversation
feanil
force-pushed
the
feanil/fix_third_part_auth_config
branch
from
a87844f to
7241d6b
Compare
…onfig 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 <noreply@anthropic.com>
feanil
force-pushed
the
feanil/fix_third_part_auth_config
branch
2 times, most recently
from
f602399 to
9ebd4f7
Compare
…ise pipeline The settings tests were failing when run with CMS settings because the third_party_auth settings (SOCIAL_AUTH_PIPELINE, ExceptionMiddleware, etc.) are now defined only in lms/envs/common.py. Investigation confirmed CMS does not need these settings: - CMS has no social auth URL endpoints (third_party_auth URLs only included in LMS when ENABLE_THIRD_PARTY_AUTH is true) - CMS uses EdxDjangoStrategy for OAuth2 SSO with LMS, not ConfigurationModelStrategy for third-party identity providers - CMS authentication backends are EdXOAuth2 (LMS SSO) and LtiAuthenticationBackend, not social auth backends - The third_party_auth app is only in cms/envs/test.py INSTALLED_APPS to avoid import errors from indirect dependencies (like enterprise) Changes: - Added @skip_unless_lms decorator to SettingsUnitTest class - Added hasattr guard for SOCIAL_AUTH_PIPELINE in apps.py to prevent AttributeError when running under CMS (which doesn't have this setting) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
feanil
force-pushed
the
feanil/fix_third_part_auth_config
branch
from
9ebd4f7 to
6058ade
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This refactoring moves all third-party authentication settings from
runtime configuration during AppConfig.ready() to static definitions
in lms/envs/common.py.
The problem was first reported here: https://discuss.openedx.org/t/third-party-auth-customization/16929/6
Problem
The third_party_auth app used an
apply_settings()function calledduring Django's app initialization to modify Django settings. This
pattern caused issues:
be finalized, making debugging difficult
files because apply_settings() would overwrite their values
Why the ENABLE_THIRD_PARTY_AUTH conditional was removed
The settings are now defined unconditionally because:
in AUTHENTICATION_BACKENDS - they have no effect
authentication backends and exposure of auth URLs
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 noreply@anthropic.com