chore: Remove waffle flag and logs for saml tpa redirect

[jono-booth]

Remove SAML site-fallback waffle flag

The third_party_auth.saml_provider_site_fallback behaviour has been tested and confirmed working, so this removes the waffle flag and makes the fallback unconditional. Also removes the verbose debug logging that was added during investigation, keeping only the two dispatch path log lines in ensure_user_information.

Fix Rate limiting on SAML registration

The fix correctly exempts the two registration endpoints from IP rate limiting when pipeline.running(request) is True — meaning there's a valid partial_pipeline_token in the server-side session, placed there when the SAML assertion was processed. This covers the primary failure path:
new SAML user → IdP → assertion processed → redirected to /register → rate limited before they can register.

What it doesn't fully solve

1. SAML users still increment the rate limit counter

block=False doesn't prevent the counter from advancing — django-ratelimit always increments it, it just stops blocking. So legitimate SAML registrations from a corporate IP still consume slots against the 60/7d / 30/7d buckets. A very high volume of SAML registrations could eventually exhaust the limit for non-SAML traffic from the same IP, though in practice that's unlikely to matter since non-SAML registration from a corporate IP would be unusual.

2. Session expiry during form fill

If the user's session expires between landing on /register and submitting the form (e.g. a very slow connection, a tab left open overnight), pipeline.running() returns False and they'll hit the rate limit. They'd get a tpa-session-expired error separately anyway — but the rate limit would fire first and return a less informative 403.

3. The block=True → block=False change has a subtle behavioural difference

Previously, once the limit was hit, django-ratelimit raised Ratelimited before the view code ran at all. Now the view runs for every request — including SAML ones — just to check the pipeline state. This is a small overhead but functionally correct.

4. The root infrastructure concern remains

The fix is applied at the Django view layer. If there's a separate nginx or WAF rate limit upstream (which could explain the 429 vs 403 discrepancy in the original report), those would still fire regardless of pipeline state. Those would need to be addressed at the infrastructure level.

[Copilot]

Pull request overview

This PR removes the now-unneeded waffle flag guarding the SAML provider “site fallback” behavior and makes the fallback unconditional, while also trimming investigation-era verbose logging in the third-party auth pipeline.

Changes:

• Removed the third_party_auth.saml_provider_site_fallback waffle flag and its helper.
• Made Registry.get_from_pipeline() always perform the SAML site-independent provider lookup for tpa-saml.
• Simplified get_complete_url() and removed verbose debug logging in ensure_user_information.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
common/djangoapps/third_party_auth/toggles.py	Removes the SAML site-fallback toggle definition and accessor.
common/djangoapps/third_party_auth/provider.py	Makes SAML provider lookup fallback unconditional for tpa-saml pipelines.
common/djangoapps/third_party_auth/pipeline.py	Updates get_complete_url() to always allow tpa-saml through and removes verbose debug logs in ensure_user_information.

Comments suppressed due to low confidence (1)

common/djangoapps/third_party_auth/provider.py:110

• Registry.get_from_pipeline now always performs the SAML site-independent fallback for backend == 'tpa-saml'. This is a behavioral change compared to the prior flag-gated version and should be covered with a focused test (e.g., when no providers are enabled / site-filtered lookup yields none but running_pipeline['kwargs']['response']['idp_name'] is present, the method returns the matching SAMLProviderConfig).

        # Fallback for SAML: SAMLAuthBackend.get_idp() uses SAMLProviderConfig.current()
        # which has no site check. If the provider's site_id doesn't match the current
        # site (or SAMLConfiguration isn't enabled for the current site), _enabled_providers()
        # won't yield it — but the SAML handshake already completed. Look up the provider
        # directly by idp_name so that pipeline steps like should_force_account_creation()
        # can still read provider flags.
        if running_pipeline.get('backend') == 'tpa-saml':
            try:
                idp_name = running_pipeline['kwargs']['response']['idp_name']
                return SAMLProviderConfig.current(idp_name)
            except (KeyError, SAMLProviderConfig.DoesNotExist):

---

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

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

common/djangoapps/third_party_auth/pipeline.py:1

• The function now explicitly allows backend_name == 'tpa-saml' to proceed even when no enabled provider exists, which contradicts the docstring’s stated ValueError behavior (“if no provider is enabled…”). Update the docstring to document this special-case so callers don’t rely on the older contract.

"""Auth pipeline definitions.

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

@jono-booth I've opened a new pull request, #169, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

---

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

[macdiesel]

@jono-booth Question, have you also been sending these changes to openedx/openedx-platform?

[jono-booth]

@jono-booth Question, have you also been sending these changes to openedx/openedx-platform?

No, but just to be clear, these are patches for changes the auth team made. So they are going where ever the auth team sends them.

[pwnage101]

IMO this would read better and be more self-documenting if refactored:

Suggested change

	pipeline_data = pipeline.get(request)
	pipeline_running = pipeline_data is not None
	pipeline_backend = pipeline_data.get('backend') if pipeline_data else None
	if should_be_rate_limited and (not pipeline_running or pipeline_backend != 'tpa-saml'):
	pipeline_data = pipeline.get(request)
	pipeline_running = pipeline_data is not None
	saml_pipeline_running = pipeline_running and pipeline_data.get('backend') == 'tpa-saml'
	# Bypass rate limit when a SAML pipeline is running.
	if should_be_rate_limited and not saml_pipeline_running:

[pwnage101]

Approved with 1 nit.

[pwnage101]

I have to ask, but have you considered openedx/openedx-platform yet? Do you have plans to backport these changes upstream? Remember anything you merge in this fork is ephemeral, and will revert back to upstream after we cut over to Verawood.