-
Notifications
You must be signed in to change notification settings - Fork 302
Migrate logout endpoint to v1/logout
#15613
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
Conversation
|
This depends on rancher/rancher#51284 to merge |
aalves08
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.
@rak-phillip something is wrong here:
Amazon Cognito OIDC:
https://github.com/user-attachments/assets/56cc7a2c-0308-40e2-b9d6-80cce75f2fed
Okta SAML:
https://github.com/user-attachments/assets/c477d81c-d0c5-47b5-8fec-ff5316f4938d
While trying both "full" logout's the URL the network request looks good logout&all but on both I was expecting to have to enter the credentials when logging back in, which didn't happen. I did a quick look-around of the UI's code and couldn't find anything suspicious/missing in relation to this PR. I am really thinking this might be a backend problem 🤔
I would say the best person to also cross-check this on Collie's side is Kevin McDermott since we worked together on the OIDC single-logout and he did a bit of refactoring to the logic on the backend side since we now support the feature in both SAML and OIDC providers 🙏
|
@pmatseykanets @rak-phillip today I gave this a test without @rak-phillip change and the single-logout feature is currently broken 🙏 |
|
Please all bear in mind that this is currently a 2.14 issue and pr |
|
@rak-phillip @pmatseykanets I can confirm that with @pmatseykanets changes rancher/rancher#52417, Normal user logout + login with user+pass also working with this change. I tested |
|
Linking both backend fixes: |
aalves08
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.
LGTM
Signed-off-by: Phillip Rak <[email protected]>
af56af3 to
b8381c9
Compare
aalves08
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.
LGTM
Summary
This updates the logout endpoint to
v1/logout.Fixes # 15326Contributes to #15326
Occurred changes and/or fixed issues
v3/tokenstov1/logoutTechnical notes summary
This appears to behave mostly the same as before - the biggest change is that there is no
logoutActionfor the base case.Areas or cases that should be tested
Local & Auth Providers logout actions, including SLO.
Areas which could experience regressions
Local & Auth Providers logout actions, including SLO.
Screenshot/Video
Checklist