-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat: tpa automatic logout with a single redirect #37048
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
feat: tpa automatic logout with a single redirect #37048
Conversation
|
Thanks for the pull request, @Cup0fCoffee! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
|
@sarina pinging you here re this comment. The new PR is up to date and passing all the tests. |
3b28e13 to
7bf8c30
Compare
|
Hi @Cup0fCoffee I was only commenting on the previous PR because it was one of our top 10 oldest. I don't have any context here and as I mentioned in the previous PR, I don't know who does. Do you think you can find a reviewer? |
|
@sarina Ok, I'll see if we can get a Core Contributor for our team assigned to this. |
7bf8c30 to
99f99fb
Compare
|
@Cup0fCoffee, I'm trying to test this in the local Tutor environment, but I am not getting redirected anywhere from the logout page (http://local.openedx.io:8000/logout). Instead, I'm seeing the following error in the console: Uncaught TypeError: this.unbind is not a function
jQuery 13
<anonymous> http://local.openedx.io:8000/static/js/logout.js:19
jQuery 8It comes from this line. I tried running |
|
@Agrendalath Yep, I'm getting the same error. However, I was able to figure it out. This issue is not caused by the changes (master fails for me the same as the branch from this PR, if you keep the |
99f99fb to
a843cb0
Compare
Agrendalath
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
- I tested this: checked that the user is correctly redirected to the
logout_urlfrom Other/Advanced TPA settings - I read through the code
- I checked for accessibility issues: n/a
- Includes documentation: n/a
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
Description
Current Third Party Authentication logout requires a redirect from the SSO IDP to the LMS to handle IDAs logout, i.e. we need to visit the
<LMS>/logouttwice. This PR attempts to handle the TPA logout with only one visit to<LMS>/logout.Testing instructions
TPA_AUTOMATIC_LOGOUT_ENABLED = Truetoedx-platform/lms/envs/devstack.pyself.tpa_logout_url = tpa_pipeline.get_idp_logout_url_from_running_pipeline(request)inedx-platform/openedx/core/djangoapps/user_authn/views/logout.pywithself.tpa_logout_url = "https://example.com/logout"(to simplify testing)make dev.up.lms+cmshttps://example.com/logout(fake IDP logout URL)Supporting information
Private ref BB-8759
Deadline
"None" if there's no rush, or provide a specific date or event (and reason) if there is one.