-
Notifications
You must be signed in to change notification settings - Fork 269
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
refactor(oidc): Align logout behavior with MSC4254 #4674
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4674 +/- ##
==========================================
+ Coverage 85.89% 85.93% +0.04%
==========================================
Files 291 290 -1
Lines 33868 33839 -29
==========================================
- Hits 29091 29080 -11
+ Misses 4777 4759 -18 ☔ View full report in Codecov by Sentry. |
76e28df
to
7d47231
Compare
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.
Alright, sorry for the delay. I wanted to test this PR since we don't have integration tests with a real OIDC provider. Can confirm, it works with beta.matrix.org.
I left a small nit and could you please get rid of the merge conflict, then we can merge.
Token revocation was split out from MSC2964 to MSC4254, and RP-Initiated logout is now mentioned only as an alternative. Signed-off-by: Kévin Commaille <[email protected]>
The server is supposed to revoke any token associated with the token that we revoke. Signed-off-by: Kévin Commaille <[email protected]>
Signed-off-by: Kévin Commaille <[email protected]>
7d47231
to
d0cf1ae
Compare
I rebased on main to solve the conflicts |
Token revocation was split out from MSC2964 (commit matrix-org/matrix-spec-proposals@c859c0b, section called "Logout initiated from a Matrix client") to MSC4254.
According to MSC4254: