diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index fb2a4d8b63e..79bec892039 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -800,10 +800,8 @@ impl Client { Ok(Arc::new(session_verification_controller)) } - /// Log out the current user. This method returns an optional URL that - /// should be presented to the user to complete logout (in the case of - /// Session having been authenticated using OIDC). - pub async fn logout(&self) -> Result, ClientError> { + /// Log the current user out. + pub async fn logout(&self) -> Result<(), ClientError> { let Some(auth_api) = self.inner.auth_api() else { return Err(anyhow!("Missing authentication API").into()); }; @@ -812,19 +810,13 @@ impl Client { AuthApi::Matrix(a) => { tracing::info!("Logging out via the homeserver."); a.logout().await?; - Ok(None) + Ok(()) } AuthApi::Oidc(api) => { tracing::info!("Logging out via OIDC."); - let end_session_builder = api.logout().await?; - - if let Some(builder) = end_session_builder { - let url = builder.build()?.url; - return Ok(Some(url.to_string())); - } - - Ok(None) + api.logout().await?; + Ok(()) } _ => Err(anyhow!("Unknown authentication API").into()), } diff --git a/crates/matrix-sdk/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index 125fba4d613..9d57ca685fe 100644 --- a/crates/matrix-sdk/CHANGELOG.md +++ b/crates/matrix-sdk/CHANGELOG.md @@ -72,6 +72,14 @@ simpler methods: - `OidcError::MissingAuthenticationIssuer` was removed. - [**breaking**]: The `authentication::qrcode` module was moved inside `authentication::oidc`, because it is only available through the `Oidc` API. +- [**breaking**]: The behavior of `Oidc::logout()` is now aligned with + [MSC4254](https://github.com/matrix-org/matrix-spec-proposals/pull/4254) + ([#4674](https://github.com/matrix-org/matrix-rust-sdk/pull/4674)) + - Support for [RP-Initiated Logout](https://openid.net/specs/openid-connect-rpinitiated-1_0.html) + was removed, so it doesn't return an `OidcEndSessionUrlBuilder` anymore. + - Only one request is made to revoke the access token, since the server is + supposed to revoke both the access token and the associated refresh token + when the request is made. ## [0.10.0] - 2025-02-04 diff --git a/crates/matrix-sdk/src/authentication/oidc/cross_process.rs b/crates/matrix-sdk/src/authentication/oidc/cross_process.rs index 8eb7f067b10..1e0453c5f1d 100644 --- a/crates/matrix-sdk/src/authentication/oidc/cross_process.rs +++ b/crates/matrix-sdk/src/authentication/oidc/cross_process.rs @@ -609,20 +609,13 @@ mod tests { // Restore the session. oidc.restore_session(tests::mock_session(tokens.clone())).await?; - let end_session_builder = oidc.logout().await?; + oidc.logout().await?; - // No end session builder because our test impl doesn't provide an end session - // endpoint. - assert!(end_session_builder.is_none()); - - // Both the access token and the refresh tokens have been invalidated. + // The access token has been invalidated. { let revoked = backend.revoked_tokens.lock().unwrap(); - assert_eq!(revoked.len(), 2); - assert_eq!( - *revoked, - vec![tokens.access_token.clone(), tokens.refresh_token.clone().unwrap(),] - ); + assert_eq!(revoked.len(), 1); + assert_eq!(*revoked, &[tokens.access_token]); } { diff --git a/crates/matrix-sdk/src/authentication/oidc/end_session_builder.rs b/crates/matrix-sdk/src/authentication/oidc/end_session_builder.rs deleted file mode 100644 index 748d6d6de9f..00000000000 --- a/crates/matrix-sdk/src/authentication/oidc/end_session_builder.rs +++ /dev/null @@ -1,112 +0,0 @@ -// Copyright 2023 Kévin Commaille -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -use language_tags::LanguageTag; -use mas_oidc_client::{ - error::TokenRevokeError, - requests::rp_initiated_logout::{build_end_session_url, LogoutData}, -}; -use tracing::instrument; -use url::Url; - -use super::{Oidc, OidcError}; -use crate::Result; - -/// Builder type used to configure optional settings for constructing an -/// [RP-Initiated Logout] URL with an OpenID Connect provider. -/// -/// Created with [`Oidc::logout()`]. Finalized with [`Self::build()`]. -/// -/// [RP-Initiated Logout]: https://openid.net/specs/openid-connect-rpinitiated-1_0.html -#[allow(missing_debug_implementations)] -pub struct OidcEndSessionUrlBuilder { - oidc: Oidc, - end_session_endpoint: Url, - client_id: String, - post_logout_redirect_uri: Option, - ui_locales: Option>, -} - -impl OidcEndSessionUrlBuilder { - pub(super) fn new(oidc: Oidc, end_session_endpoint: Url, client_id: String) -> Self { - Self { - oidc, - end_session_endpoint, - client_id, - post_logout_redirect_uri: None, - ui_locales: None, - } - } - - /// Set the URI where the user will be redirected after logging out. - /// - /// Must be one of the `post_logout_redirect_uris` registered in the client - /// metadata. - pub fn post_logout_redirect_uri(mut self, redirect_uri: Url) -> Self { - self.post_logout_redirect_uri = Some(redirect_uri); - self - } - - /// Set the preferred locales of the user. - /// - /// Must be ordered from the preferred locale to the least preferred locale. - pub fn ui_locales(mut self, ui_locales: Vec) -> Self { - self.ui_locales = Some(ui_locales); - self - } - - /// Get the URL that should be presented to log out from the OIDC provider's - /// interface. - /// - /// If a `post_logout_redirect_uri` was provided, the user will be - /// redirected to it after logging out with a `state` query parameter that - /// is the same as the one in the `OidcEndSessionData`. - #[instrument(target = "matrix_sdk::client", skip_all)] - pub fn build(self) -> Result { - let Self { oidc, end_session_endpoint, client_id, post_logout_redirect_uri, ui_locales } = - self; - - // We only need one of those. - let (id_token_hint, logout_hint) = if let Some(id_token) = oidc.latest_id_token() { - (Some(id_token.into_string()), None) - } else { - let logout_hint = oidc.client.user_id().map(|user_id| format!("mxid:{user_id}")); - (None, logout_hint) - }; - - let logout_data = LogoutData { - id_token_hint, - logout_hint, - client_id: Some(client_id), - post_logout_redirect_uri, - ui_locales, - }; - - let (url, state) = - build_end_session_url(end_session_endpoint, logout_data, &mut super::rng()?) - .map_err(TokenRevokeError::from)?; - - Ok(OidcEndSessionData { url, state }) - } -} - -/// Data for the user to log out from their account in the issuer's interface. -#[derive(Debug, Clone)] -pub struct OidcEndSessionData { - /// The URL that should be presented. - pub url: Url, - /// A unique identifier for the request, if the user is to be redirected to - /// the client after logging out. - pub state: Option, -} diff --git a/crates/matrix-sdk/src/authentication/oidc/mod.rs b/crates/matrix-sdk/src/authentication/oidc/mod.rs index 06276bbf6bf..9b2a1d6b193 100644 --- a/crates/matrix-sdk/src/authentication/oidc/mod.rs +++ b/crates/matrix-sdk/src/authentication/oidc/mod.rs @@ -183,7 +183,6 @@ mod auth_code_builder; mod backend; mod cross_process; mod data_serde; -mod end_session_builder; #[cfg(all(feature = "e2e-encryption", not(target_arch = "wasm32")))] pub mod qrcode; pub mod registrations; @@ -193,7 +192,6 @@ mod tests; pub use self::{ auth_code_builder::{OidcAuthCodeUrlBuilder, OidcAuthorizationData}, cross_process::CrossProcessRefreshLockError, - end_session_builder::{OidcEndSessionData, OidcEndSessionUrlBuilder}, }; use self::{ backend::{server::OidcServer, OidcBackend}, @@ -1469,13 +1467,7 @@ impl Oidc { } /// Log out from the currently authenticated session. - /// - /// On success, if the provider supports [RP-Initiated Logout], an - /// [`OidcEndSessionUrlBuilder`] will be provided to build the URL allowing - /// the user to log out from their account in the provider's interface. - /// - /// [RP-Initiated Logout]: https://openid.net/specs/openid-connect-rpinitiated-1_0.html - pub async fn logout(&self) -> Result, OidcError> { + pub async fn logout(&self) -> Result<(), OidcError> { let provider_metadata = self.provider_metadata().await?; let client_credentials = self.data().ok_or(OidcError::NotAuthenticated)?.credentials(); @@ -1484,42 +1476,21 @@ impl Oidc { let tokens = self.session_tokens().ok_or(OidcError::NotAuthenticated)?; - // Revoke the access token. + // Revoke the access token, it should revoke both tokens. self.backend .revoke_token( - client_credentials.clone(), + client_credentials, revocation_endpoint, tokens.access_token, Some(OAuthTokenTypeHint::AccessToken), ) .await?; - // Revoke the refresh token, if any. - if let Some(refresh_token) = tokens.refresh_token { - self.backend - .revoke_token( - client_credentials.clone(), - revocation_endpoint, - refresh_token, - Some(OAuthTokenTypeHint::RefreshToken), - ) - .await?; - } - - let end_session_builder = - provider_metadata.end_session_endpoint.clone().map(|end_session_endpoint| { - OidcEndSessionUrlBuilder::new( - self.clone(), - end_session_endpoint, - client_credentials.client_id().to_owned(), - ) - }); - if let Some(manager) = self.ctx().cross_process_token_refresh_manager.get() { manager.on_logout().await?; } - Ok(end_session_builder) + Ok(()) } } diff --git a/examples/oidc_cli/src/main.rs b/examples/oidc_cli/src/main.rs index e55f5121a7c..ea19d7f734e 100644 --- a/examples/oidc_cli/src/main.rs +++ b/examples/oidc_cli/src/main.rs @@ -610,22 +610,13 @@ impl OidcCli { /// Log out from this session. async fn logout(&self) -> anyhow::Result<()> { // Log out via OIDC. - let url_builder = self.client.oidc().logout().await?; + self.client.oidc().logout().await?; // Delete the stored session and database. let data_dir = self.session_file.parent().expect("The file has a parent directory"); fs::remove_dir_all(data_dir).await?; println!("\nLogged out successfully"); - - if let Some(url_builder) = url_builder { - let data = url_builder.build()?; - println!( - "\nTo log out from your account in the provider's interface, visit: {}", - data.url - ); - } - println!("\nExiting…"); Ok(())