Skip to content

fix: skip hinted login if pipeline already running#38140

Closed
jono-booth wants to merge 200 commits intoopenedx:masterfrom
edx:jb/saml-redirect-loop
Closed

fix: skip hinted login if pipeline already running#38140
jono-booth wants to merge 200 commits intoopenedx:masterfrom
edx:jb/saml-redirect-loop

Conversation

@jono-booth
Copy link
Copy Markdown

@jono-booth jono-booth commented Mar 10, 2026

login_and_registration_form checks tpa_hint before checking saml_provider:

# Lines 167-183: tpa_hint + skip_hinted_login_dialog → RETURN to Auth0 (no pipeline check!)
if '?' in redirect_to:
  if tpa_hint_provider.skip_hinted_login_dialog:
      return redirect(pipeline.get_login_url(...))  #  loops back to Auth0

# Lines 196-208: saml_provider check (never reached if above fires)
running_pipeline = pipeline.get(request)
if running_pipeline:
  saml_provider, __ = is_saml_provider(...)
if should_redirect_to_authn_microfrontend() and not saml_provider:
  return redirect(AUTHN_MFE_URL)

Once redirect_to has tpa_hint from THIRD_PARTY_AUTH_HINT, the function redirects to Auth0 without ever checking if a SAML pipeline is already running. The pipeline check at line 197 is never reached.

Two changes, working together:

  1. running_pipeline is computed first (lines 163-169) — before any tpa_hint check. Previously it was computed at line 197, unreachable if the skip_hinted_login_dialog redirect fired.
  2. and not running_pipeline added to the redirect condition (line 182) — if a TPA pipeline is already in flight, the skip_hinted_login_dialog redirect is suppressed. The user falls through to initial_mode = "hinted_login" and sees the login/register form with the provider pre-selected, which is the correct behavior for a new SAML user who needs to complete registration.

dianakhuang and others added 30 commits October 9, 2025 09:23
feat: add an endpoint to create a customer admin user

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`
feat: future proof artifact uploads (backport)
- when copying a component that has image in it, and we try to paste it. Image URL appends `static_None`. Result in crash or image not found error.
  - In this commit we have fixed this scenario, copy paste is working for components containing images.
…_edx

fix: pasting a component with image isn't working
…or_edx

fix: copy paste component from one course to another
openedx#37539)

* feat: allows a reversion of the  retirement partner report reset toggle

This allows you to set retirement partner report statuses to True as well as to False. One sample use case: if an overly large number of retirement partner reports have their status reset to false, the partner report queue can struggle to deal with the large queue.

FIXES: APER-4177
…rtner-report-reset_toggle-from-upstream

feat: allows a reversion of the  retirement partner report reset toggle
One retirement partner status report admin toggle has  being renamed,
and another has been added. This PR displays them on the appropriate
django admin page.
…rtner-report-reset_toggle-fix_from_upstream

feat: display the reset toggles for a report (openedx#37556)
…penedx#37377) (#18)

- Removes custom attributes for report. Uses report output only.
- Adds a count for disabled SAML configs.
- Displays disabled status of provider.
- Slug mismatch now informational only (rather than warning)
* Cleans up unit tests.
This seems to reduce instances of audio garbling when switching levels during HLS video streaming.
These changes fix a bug in how LTI-based exam due dates are computed and written to the exams service. Prior to this change, an LTI exam due date was computed irrespective of the course pacing type. In certain cases, this caused incorrect due dates to be written to the exams service for LTI-based exams.

For example, if a course team initially develops a course as an instructor-paced course and sets a due date on an exam subsection, that subsection due date is written to the modulestore. If the course team subsequently changes that course pacing type to self-paced, then that due date remains in the modulestore to allow course teams to switch pacing types without erasing due dates. The impact of this is that, when the course is published, the exam subsection due date is written to the exams service as the due date, even though there are no static due dates in a self-paced course. Frequently, these due dates are in the past (e.g. for course reruns), so learners automatically cannot access exams. Even if the due date is manually corrected in the exams service, every course publish reverts the due date to the incorrect due date.

This change computes the due date of LTI-based exams as...
* the exam subsection due date if the course is instructor-paced, if the subsection has a due date; else None
* the course end date if the course is self-paced, if the course has an end date; else None

In order to correct any incorrect due dates, course teams should republish their courses.
…tes-self-paced-cherry-pick

fix: incorrect LTI exam due dates for self-paced courses
* fix: move defaultAudioCodec config earlier in init

This causes it to get picked up in the places that it is actually
needed to handle issues in audio quality switching.
…scripts_for_edx

fix: accessibility issue on video transcripts
fix: add text alternative for external link icon in LTI components
Co-authored-by: Sameen Fatima <sameen.fatima@A006-01036.local>
Commit generated by workflow `edx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/release-ulmo`
…ted-channels-ac02b33

feat: Upgrade Python dependency enterprise-integrated-channels
fix: do not autogenerate username if coming through SSO (openedx#37522)
pwnage101 and others added 19 commits February 24, 2026 20:48
* chore: bump edx-enterprise-integrated-channels to 0.1.42

ENT-11542

* chore: run make compile-requirements

This seems to cause the pip constraint to be removed, but I'm not going
to upgrade pip yet.

ENT-11542
Commit generated by workflow `openedx/openedx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* feat(xblock): support safe returnTo redirect for XBlock edit view

* test(xblock): add tests for _get_safe_return_to returnTo validation

* test(xblock): add JS tests for isSafeReturnTo client-side validation
…ignorable

We want to support a flow for SSO-enabled Enterprise customers who have
agreed off-platform that none of their learners will opt-in to marketing emails
or sharing research data. This change proposes to do so by
adding an optional field that, when enabled, disables the presence of
the two checkboxes on this registration form and sets their values to false.

ENT-11401
Removes `videos.enable_devstack_video_uploads` waffle flag. We may need to
reimplement with boto3 in mind, or maybe AWS environment variables in the
dev environment are sufficient.

This also removes some wrapper functions that I'm reasonably sure are
unused and that were exposing boto v2 code paths. For reference, they were
introduced in <openedx#32803>.

I've filed https://2u-internal.atlassian.net/browse/TNL2-545 to ensure that
functionality is either confirmed to still work or is reimplemented as needed.

BOMS-421, TNL-122
fix: lms config 400 errors when editing SAP etc

Commit generated by workflow `edx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/release-ulmo`
chore: upgrade enterprise-integrated-chanels to 0.1.48
Co-authored-by: robrap <robrap@users.noreply.github.com>
chore: bumped edx-enterprise vesrion from 6.6.5 to 6.6.7
@jono-booth jono-booth requested a review from a team as a code owner March 10, 2026 11:41
Copilot AI review requested due to automatic review settings March 10, 2026 11:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an infinite redirect loop that could occur for new SAML users when a provider has skip_hinted_login_dialog = True and the ?next= URL contains a tpa_hint parameter. Without the fix, the function would redirect the user back to the Auth0/SAML provider even when a TPA pipeline was already in flight (e.g., a new SAML user needing to complete registration), causing an infinite loop. The fix works by computing the running pipeline state early and using it to suppress the automatic redirect.

Changes:

  • The running_pipeline and saml_provider detection block is moved earlier in the function (before the tpa_hint processing) so it is available to guard the skip_hinted_login_dialog redirect.
  • A and not running_pipeline condition is added to the skip_hinted_login_dialog redirect guard, preventing re-dispatch to the provider when a pipeline is already in progress.
  • The duplicate original pipeline-detection block (which was now unreachable in the bug scenario) is removed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jono-booth jono-booth force-pushed the jb/saml-redirect-loop branch from b6037ba to b323f6c Compare March 10, 2026 11:55
@jono-booth jono-booth closed this Mar 10, 2026
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.