Skip to content

Fix logout redirects #2736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Fix logout redirects #2736

wants to merge 10 commits into from

Conversation

rhysyngsun
Copy link
Collaborator

@rhysyngsun rhysyngsun commented Jun 16, 2025

What are the relevant tickets?

Fixes https://github.com/mitodl/hq/issues/7564

Description (What does it do?)

This re-adjusts the logout flow when the apisix middleware is enabled to ensure the user is logged out of openedx, keycloak, apisix, and mitxonline.

I refactored the urls.py files a bit too because in order to write tests for the new logout endpoint I needed to be able to load a specific set of urlpatterns under pytest.

How can this be tested?

  • Ensure in opendex that you have this or something similar set:
IDA_LOGOUT_URI_LIST.append("https:/mitxonline.odl.local/logout")
  • Login as a user and ensure you're also logged in on openedx
  • Click logout in mitxonline, you should land on the mitxonline home page and have no session there or in openedx or keycloak
  • Login as a user and ensure you're also logged in on openedx
  • Click logout in openedx, you should land on the mitxonline home page and have no session there or in openedx or keycloak

Then, turn off the API gateway middleware and retest all of the above, it should still work.

@rhysyngsun rhysyngsun marked this pull request as ready for review June 17, 2025 17:59
@jkachel jkachel self-assigned this Jun 17, 2025
@rhysyngsun rhysyngsun force-pushed the nl/logout-urls-shuffle branch from 64dd629 to d6e34d1 Compare June 17, 2025 21:07
Copy link
Contributor

@jkachel jkachel left a comment

Choose a reason for hiding this comment

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

I think this is fine. Running the flow runs things into edX for logout, which then hits the /logout/oidc route that APISIX consumes, and that kills all the sessions. So, LGTM 👍

The issues I was having testing this amounted to differences in hostnames. I think you have your Tutor setup configured to exist at openedx.odl.local and I was using the default local.openedx.io, and I had none of these services set up to use SSL. So, when the iframe in the edX logout interstitial starts pinging things to try to log you out, it does hit /logout/?no_redirect=1 as it should but the cookies (and thus the session) don't transmit in that transaction, because it's an iframe in a different origin. So, it was acting correctly; it actually didn't have a session.

With Tutor set up to use a .odl.local domain and the cookie domains set in apisix to be .odl.local, the cookies do come over in the iframe and things work properly. (The other option for fixing this is getting APISIX set up with a certificate; we then have to set the cookies to be SameSite=None and Secure so they will transfer over (Lax won't do it).) But, this is a problem for local deployments and shouldn't be an issue on RC I think, since the origins aren't totally different there.

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.

2 participants