feat: replace enterprise logistration logic with openedx-filters hooks - #407
Open
pwnage101 wants to merge 1 commit into
Open
feat: replace enterprise logistration logic with openedx-filters hooks#407pwnage101 wants to merge 1 commit into
pwnage101 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes enterprise-specific logic from the user_authn logistration/login/registration flow and replaces it with Open edX Filters hooks so downstream plugins can customize behavior without importing enterprise_support.
Changes:
- Removed enterprise-specific redirect and context/cookie logic from legacy logistration and post-login redirect handling.
- Introduced/used six generic Open edX Filters hook points for logistration context/response, MFE redirect veto, post-login redirect, and TPA form overrides.
- Updated and expanded test coverage to validate the new filter hook behavior and removed enterprise-specific test cases.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| openedx/core/djangoapps/user_authn/views/utils.py | Removes enterprise URL regex constants that are no longer needed. |
| openedx/core/djangoapps/user_authn/views/login_form.py | Replaces enterprise logistration branching with filter hooks (context, response, MFE redirect veto, login-form TPA overrides). |
| openedx/core/djangoapps/user_authn/views/registration_form.py | Adds registration-form TPA override filter hook and returns the modified form_desc. |
| openedx/core/djangoapps/user_authn/views/login.py | Replaces enterprise multi-enterprise redirect logic with a post-login redirect filter hook. |
| openedx/core/djangoapps/user_authn/views/tests/test_logistration.py | Removes enterprise-specific logistration assertions and related SAML/enterprise tests. |
| openedx/core/djangoapps/user_authn/views/tests/test_login.py | Removes enterprise multi-enterprise redirect tests tied to the deleted logic. |
| openedx/core/djangoapps/user_authn/views/tests/test_filters.py | Adds new filter tests for logistration/login/registration/post-login redirect hooks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
1
to
3
| """ | ||
| Test that various filters are fired for the vies in the user_authn app. | ||
| """ |
Comment on lines
+16
to
+21
| from openedx_filters.learning.filters import ( | ||
| LoginFormTPAOverridesRequested, | ||
| LogistrationContextRequested, | ||
| LogistrationMFERedirectRequested, | ||
| LogistrationResponseRendered, | ||
| ) |
| root_url, enterprise_selection_page(request, possibly_authenticated_user, finish_auth_url or next_url) | ||
| # .. filter_implemented_name: PostLoginRedirectURLRequested | ||
| # .. filter_type: org.openedx.learning.auth.post_login.redirect_url.requested.v1 | ||
| post_login_redirect_url, __, __ = PostLoginRedirectURLRequested.run_filter( |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
openedx/core/djangoapps/user_authn/views/login_form.py:225
- The PR description lists a new LogistrationMFERedirectRequested hook, but the logistration view still redirects to the Authn MFE directly when should_redirect_to_authn_microfrontend() is true (no filter invocation or tests for that hook). Either wire in the LogistrationMFERedirectRequested filter at this decision point (so plugins can override/suppress the redirect), or update the PR description to reflect the actual set of filters implemented.
# Redirect to the authn MFE when it is enabled, unless auth is provided externally
# (SAML/TPA users must remain on the legacy login/registration page).
has_external_provider = bool(tpa_hint_provider or saml_provider)
if should_redirect_to_authn_microfrontend() and not has_external_provider:
query_params = request.GET.urlencode()
url_path = '/{}{}'.format(
initial_mode,
'?' + query_params if query_params else ''
)
return redirect(settings.AUTHN_MICROFRONTEND_URL + url_path)
openedx/core/djangoapps/user_authn/views/utils.py:25
- The PR description says this change “remov[es] all enterprise_support imports”, but this file still imports enterprise_support in get_mfe_context() (openedx/core/djangoapps/user_authn/views/utils.py:117-118). If the intent is only to remove enterprise logic from the legacy logistration views, the PR description should be narrowed; otherwise these remaining imports should be removed/refactored as well.
API_V1 = 'v1'
pwnage101
force-pushed
the
pwnage101/ENT-11568-edx
branch
from
July 29, 2026 03:01
cbc2739 to
3858d01
Compare
This was referenced Jul 29, 2026
iloveagent57
approved these changes
Jul 29, 2026
| """ | ||
|
|
||
| from edx_toggles.toggles import WaffleFlag, WaffleSwitch | ||
| from edx_toggles.toggles import WaffleSwitch |
Member
Author
There was a problem hiding this comment.
Indeed, perhaps you caught on, but this waffle flag has been enabled in both stage and prod for a while. It no longer serves any purpose, and upstream openedx-platform never received this waffle flag because the original PR was never upstreamed.
Replace the enterprise-specific logic in the user_authn logistration views with 5 generic filters, removing all enterprise_support imports: - LogistrationContextRequested - LoginFormTPAOverridesRequested - RegistrationFormTPAOverridesRequested - LogistrationResponseRendered - PostLoginRedirectURLRequested ENT-11568
pwnage101
force-pushed
the
pwnage101/ENT-11568-edx
branch
from
July 29, 2026 19:54
3858d01 to
c50f851
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
openedx/core/djangoapps/user_authn/views/tests/test_filters.py:3
- Docstring typo: “vies” should be “views”.
"""
Test that various filters are fired for the vies in the user_authn app.
"""
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
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.
Replace the enterprise-specific logic in the user_authn logistration views with 5 generic filters, removing all enterprise_support imports:
ENT-11568
Integration Testing
See openedx/edx-enterprise#2551 (comment)
Related: