From d4b92de8e4b4ebeaeff7388e36a476f9d5817583 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Mon, 17 Feb 2025 11:20:03 +0100 Subject: [PATCH] refactor(oidc): Remove support for OIDC RP-Initiated logout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- bindings/matrix-sdk-ffi/src/client.rs | 18 +-- .../src/authentication/oidc/cross_process.rs | 6 +- .../oidc/end_session_builder.rs | 112 ------------------ .../matrix-sdk/src/authentication/oidc/mod.rs | 21 +--- examples/oidc_cli/src/main.rs | 11 +- 5 files changed, 9 insertions(+), 159 deletions(-) delete mode 100644 crates/matrix-sdk/src/authentication/oidc/end_session_builder.rs diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index fb2a4d8b6..79bec8920 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/src/authentication/oidc/cross_process.rs b/crates/matrix-sdk/src/authentication/oidc/cross_process.rs index 8eb7f067b..5ba0344cb 100644 --- a/crates/matrix-sdk/src/authentication/oidc/cross_process.rs +++ b/crates/matrix-sdk/src/authentication/oidc/cross_process.rs @@ -609,11 +609,7 @@ mod tests { // Restore the session. oidc.restore_session(tests::mock_session(tokens.clone())).await?; - let end_session_builder = oidc.logout().await?; - - // No end session builder because our test impl doesn't provide an end session - // endpoint. - assert!(end_session_builder.is_none()); + oidc.logout().await?; // Both the access token and the refresh tokens have been invalidated. { 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 748d6d6de..000000000 --- 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 06276bbf6..de19fc1dc 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(); @@ -1506,20 +1498,11 @@ impl Oidc { .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 e55f5121a..ea19d7f73 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(())