-
Notifications
You must be signed in to change notification settings - Fork 68
Expose end_session endpoint to perform rp_initiated_logout #5135
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,211 @@ | ||||||||||||
| // Copyright 2024, 2025 New Vector Ltd. | ||||||||||||
| // Copyright 2023, 2024 The Matrix.org Foundation C.I.C. | ||||||||||||
| // | ||||||||||||
| // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial | ||||||||||||
| // Please see LICENSE files in the repository root for full details. | ||||||||||||
| use axum::{ | ||||||||||||
| Json, | ||||||||||||
| extract::State, | ||||||||||||
| response::{IntoResponse, Redirect, Response}, | ||||||||||||
| }; | ||||||||||||
| use axum_extra::extract::Query; | ||||||||||||
| use hyper::StatusCode; | ||||||||||||
| use mas_axum_utils::{SessionInfoExt, cookies::CookieJar, record_error}; | ||||||||||||
| use mas_data_model::{BoxClock, BoxRng, Clock}; | ||||||||||||
| use mas_keystore::Keystore; | ||||||||||||
| use mas_oidc_client::{ | ||||||||||||
| error::IdTokenError, | ||||||||||||
| requests::jose::{JwtVerificationData, verify_id_token}, | ||||||||||||
| }; | ||||||||||||
| use mas_router::UrlBuilder; | ||||||||||||
| use mas_storage::{ | ||||||||||||
| BoxRepository, RepositoryAccess, | ||||||||||||
| queue::{QueueJobRepositoryExt as _, SyncDevicesJob}, | ||||||||||||
| user::BrowserSessionRepository, | ||||||||||||
| }; | ||||||||||||
| use oauth2_types::errors::{ClientError, ClientErrorCode}; | ||||||||||||
| use serde::{Deserialize, Serialize}; | ||||||||||||
| use thiserror::Error; | ||||||||||||
|
|
||||||||||||
| use crate::{BoundActivityTracker, impl_from_error_for_route}; | ||||||||||||
|
|
||||||||||||
| #[derive(Debug, Deserialize, Serialize)] | ||||||||||||
| pub(crate) struct EndSessionParam { | ||||||||||||
| id_token_hint: String, | ||||||||||||
| post_logout_redirect_uri: String, | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| #[derive(Debug, Error)] | ||||||||||||
| pub(crate) enum RouteError { | ||||||||||||
| #[error(transparent)] | ||||||||||||
| Internal(Box<dyn std::error::Error + Send + Sync + 'static>), | ||||||||||||
|
|
||||||||||||
| #[error("bad request")] | ||||||||||||
| BadRequest, | ||||||||||||
|
|
||||||||||||
| #[error("client not found")] | ||||||||||||
| ClientNotFound, | ||||||||||||
|
|
||||||||||||
| #[error("client is unauthorized")] | ||||||||||||
| UnauthorizedClient, | ||||||||||||
|
|
||||||||||||
| // #[error("unsupported token type")] | ||||||||||||
| // UnsupportedTokenType, | ||||||||||||
|
||||||||||||
| // #[error("unsupported token type")] | |
| // UnsupportedTokenType, |
Outdated
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.
| // Self::ClientNotAllowed | |
Outdated
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.
| // Self::UnsupportedTokenType => ( | |
| // StatusCode::BAD_REQUEST, | |
| // Json(ClientError::from(ClientErrorCode::UnsupportedTokenType)), | |
| // ) | |
| // .into_response(), |
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.
We should have a nice view in some of those cases, in the end that is going to be a redirect, potentially where it doesn't go back to the client
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.
You can have many OAuth session started from a single browser session, so this way is not the right approach here.
Instead, best is to decode the JWT (but not necessarily verify it, maybe verify_id_token should be changed a little bit?) to get the claims we need and go from there? I'm unclear what we should be checking exactly on that JWT though, I don't know whether there is something that would let us link it to the current browser session or not 🤔
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.
Not sure about that semantic. I don't know whether we should finish the oauth session at this point or let the client still call the revocation endpoint.
Either way, we need a better way to find the right OAuth session, and I think this should be done by injecting the OAuth session ID in the id_token sid claim when we give out the id_token
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.
The spec says we really should be validating the post_logout_redirect_uri, it should be registered by the client first
The value MUST have been previously registered with the OP, either using the post_logout_redirect_uris Registration parameter or via another mechanism.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.