Skip to content

feat: add pipeline steps for logistration filters - #2551

Merged
pwnage101 merged 1 commit into
masterfrom
pwnage101/ENT-11568
Aug 7, 2026
Merged

feat: add pipeline steps for logistration filters#2551
pwnage101 merged 1 commit into
masterfrom
pwnage101/ENT-11568

Conversation

@pwnage101

@pwnage101 pwnage101 commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Implement steps for five logistration filters replacing logic in the user_authn platform module, and register them in ENTERPRISE_FILTERS_CONFIG:

  • LogistrationContextEnricher (for LogistrationContextRequested)
  • LoginFormEnterpriseOverrides (for LoginFormTPAOverridesRequested)
  • RegistrationFormEnterpriseOverrides (for RegistrationFormTPAOverridesRequested)
  • LogistrationCookieSetter (for LogistrationResponseRendered)
  • PostLoginEnterpriseRedirect (for PostLoginRedirectURLRequested)

ENT-11568


Integration Testing in 2U Devstack

Setup:

  1. Launch the required containers:
    • make dev.up.lms+enterprise-catalog+frontend-app-account+frontend-app-authn
  2. Add this line to /etc/hosts:
    • 127.0.0.1 edx.devstack.keycloak
  3. Create the user_authn.enable_enterprise_redirect_to_authn waffle flag and enable it globally.

Control Test:

  1. Set up your devstack to run the following branches:
    • lms: release-ulmo
    • edx-enterprise: master
  2. from edx-enterprise, run two commands:
    • make dev.provision.keycloak
    • ./scripts/provision-integration-test-ENT-11568.sh
  3. Follow the script output to conduct entire integration test suite.

Integration Test:

  1. Reconfigure your devstack to switch to the new code:
    • lms: pwnage101/ENT-11568-edx
      • pip install -e /edx/src/edx-enterprise/
      • pip install -e /edx/src/openedx-filters/
    • edx-enterprise: pwnage101/ENT-11568
    • openedx-filters: pwnage101/ENT-11568
  2. Repeat steps 5-6 and confirm everything works exactly the same.

Related:

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.47059% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.02%. Comparing base (99c7e2c) to head (f4c6aaf).

Files with missing lines Patch % Lines
enterprise/filters/logistration.py 96.47% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2551      +/-   ##
==========================================
+ Coverage   86.97%   87.02%   +0.04%     
==========================================
  Files         261      262       +1     
  Lines       17143    17228      +85     
  Branches     1693     1705      +12     
==========================================
+ Hits        14910    14992      +82     
- Misses       1896     1897       +1     
- Partials      337      339       +2     
Flag Coverage Δ
unittests 87.02% <96.47%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pwnage101
pwnage101 force-pushed the pwnage101/ENT-11568 branch 4 times, most recently from 34a6dc4 to 4e1987d Compare July 14, 2026 04:49
@pwnage101 pwnage101 changed the title feat: add logistration context and post-login redirect pipeline steps feat: add pipeline steps for the logistration filters Jul 14, 2026
@pwnage101
pwnage101 force-pushed the pwnage101/ENT-11568 branch 2 times, most recently from 5b99d92 to 0ee8066 Compare July 22, 2026 14:42
Comment thread enterprise/filters/logistration.py
Comment thread enterprise/filters/logistration.py
Comment thread enterprise/filters/logistration.py Outdated
@pwnage101
pwnage101 force-pushed the pwnage101/ENT-11568 branch 2 times, most recently from 5cf65c3 to 38361f7 Compare July 29, 2026 02:54
@pwnage101 pwnage101 changed the title feat: add pipeline steps for the logistration filters feat: add pipeline steps for logistration filters Jul 29, 2026
@pwnage101
pwnage101 marked this pull request as ready for review July 29, 2026 03:17
@@ -1,17 +1,11 @@
#!/usr/bin/env bash

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind leaving references to the ticket number in docs, but this filename is a bit weird. Maybe logistration-pipeline-integration-test.sh?

@pwnage101 pwnage101 Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My goal is to polish and document as much official logic as possible in official locations:

  • enterprise/devstack_api.py
  • enterprise/roles_api.py
  • provision-tpa.py

But stuff in scripts/ are one-time use and not intended for long-term support. The other script, provision-integration-test-ENT-11544.sh for a previous ticket does not even work anymore because so much of devstack_api.py has changed, but there's no problem because it's not a supported feature and not "on our books" as a maintenance liability.

The ticket number in the script name reinforces the temporariness and unsupported nature of the script.

@pwnage101
pwnage101 force-pushed the pwnage101/ENT-11568 branch from 38361f7 to abd13d1 Compare July 29, 2026 21:43
Comment thread docs/saml_testing.rst
Comment thread enterprise/filters/logistration.py Outdated
@pwnage101
pwnage101 force-pushed the pwnage101/ENT-11568 branch 5 times, most recently from 42619a3 to c25f1fb Compare August 5, 2026 22:16
Comment thread enterprise/filters/logistration.py Outdated
Comment thread enterprise/filters/logistration.py Outdated
Comment thread enterprise/filters/logistration.py

@kramakrushna kramakrushna left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks like a solid extraction of the enterprise logistration behavior into openedx-filters, and the new tests/docs are helpful. I have three main concerns before merge:

  1. LoginFormEnterpriseOverrides assumes running_pipeline -> kwargs -> details always exists; if that shape changes or is missing, this can crash the login flow.

  2. PostLoginEnterpriseRedirect relies on re.search(UUID4_REGEX, redirect_url).group(0) after a regex match; this is brittle and could raise if the URL format changes or is encoded differently.

  3. RegistrationFormEnterpriseOverrides intentionally changes how skip_registration_optional_checkboxes is resolved compared with the legacy platform logic. That seems reasonable, but it should be called out more explicitly since it changes behavior in an edge case.

@pwnage101
pwnage101 force-pushed the pwnage101/ENT-11568 branch 3 times, most recently from 1365f66 to 6d47625 Compare August 7, 2026 17:27
Implement steps for six logistration filters replacing enterprise logic in the
user_authn platform module, and register them in ENTERPRISE_FILTERS_CONFIG:

- LogistrationViewEnterpriseContextEnricher (for LogistrationViewContextGenerated)
- AuthnMFEEnterpriseContextEnricher         (for AuthnMFEContextGenerated)
- LogistrationViewEnterpriseCookieSetter    (for LogistrationViewRenderCompleted)
- LoginFormEnterpriseOverrides              (for LoginFormGenerated)
- RegistrationFormEnterpriseOverrides       (for RegistrationFormGenerated)
- PostLoginEnterpriseRedirect               (for LoginAltRedirectURLRequested)

ENT-11568

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds enterprise-owned openedx-filters pipeline steps to implement (and document/test) enterprise logistration behavior that previously lived in the platform’s user_authn module, moving that logic behind filter hooks and wiring it into the plugin’s settings.

Critical issues to address:

  • enterprise/settings/test.py defines COURSE_KEY_REGEX via an incorrect string replacement that produces an invalid regex (will raise re.error when used).
  • enterprise/filters/logistration.py can raise AttributeError if openedx.core.djangoapps.user_api.accounts isn’t importable (accounts is set to None but later dereferenced).

Changes:

  • Introduces 6 authentication/logistration pipeline steps in enterprise/filters/logistration.py and registers them in ENTERPRISE_FILTERS_CONFIG (merged into OPEN_EDX_FILTERS_CONFIG).
  • Adds comprehensive unit tests for the new pipeline steps.
  • Updates devstack integration-test provisioning script and docs; enables Sphinx Markdown + Mermaid support for ADRs.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tox.ini Ensures pii_check env installs Django 5.2.x.
enterprise/filters/logistration.py Adds new openedx-filters pipeline steps implementing enterprise logistration behavior.
enterprise/settings/common.py Registers the new authentication filter pipelines in ENTERPRISE_FILTERS_CONFIG and merges into OPEN_EDX_FILTERS_CONFIG.
enterprise/settings/test.py Adds COURSE_KEY_REGEX for standalone tests (currently incorrect).
tests/filters/test_logistration.py New unit tests covering the pipeline step behaviors.
scripts/provision-integration-test-ENT-11568.sh Updates provisioning + manual test plan; adds a preflight validation of auth filter mappings.
requirements/test.txt Bumps openedx-filters to 3.9.0.
requirements/test-master.txt Bumps openedx-filters to 3.9.0.
requirements/edx-platform-constraints.txt Bumps openedx-filters to 3.9.0 under platform constraints.
requirements/doc.in Adds sphinxcontrib-mermaid and myst-parser to doc inputs.
requirements/doc.txt Regenerated pins/comments to include MyST + Mermaid deps and openedx-filters==3.9.0.
requirements/dev.txt Regenerated pins/comments to include MyST + Mermaid deps and openedx-filters==3.9.0.
docs/conf.py Enables .md sources + MyST parser and Mermaid extension.
docs/decisions/0016-logistration-filters.md New ADR documenting the filter-based architecture and flows (Mermaid diagram).
docs/saml_testing.rst Updates SAML devstack prerequisites and streamlines provisioning/testing docs.
enterprise/__init__.py Version bump to 8.8.0.
CHANGELOG.rst Adds 8.8.0 entry dated 2026-08-07.

Comment thread enterprise/settings/test.py
Comment thread enterprise/filters/logistration.py
Comment thread enterprise/settings/common.py
Comment thread scripts/provision-integration-test-ENT-11568.sh
@pwnage101
pwnage101 merged commit 06d3340 into master Aug 7, 2026
12 checks passed
@pwnage101
pwnage101 deleted the pwnage101/ENT-11568 branch August 7, 2026 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants