From 7457ecb1a874894cd1265ab9cbbb2a3029e2345e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Fri, 21 Mar 2025 03:10:25 +0100 Subject: [PATCH] feat(oauth): Allow to use any registration method with login_with_qr_code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces the ClientRegistrationMethod type Signed-off-by: Kévin Commaille --- bindings/matrix-sdk-ffi/src/client_builder.rs | 7 +- .../src/authentication/oauth/mod.rs | 98 ++++++++++++++++--- .../src/authentication/oauth/qrcode/login.rs | 19 ++-- examples/qr-login/src/main.rs | 2 +- 4 files changed, 97 insertions(+), 29 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client_builder.rs b/bindings/matrix-sdk-ffi/src/client_builder.rs index 4fed5b14e..1c1133544 100644 --- a/bindings/matrix-sdk-ffi/src/client_builder.rs +++ b/bindings/matrix-sdk-ffi/src/client_builder.rs @@ -688,12 +688,13 @@ impl ClientBuilder { } })?; - let client_metadata = oidc_configuration - .client_metadata() + let registrations = oidc_configuration + .registrations() + .await .map_err(|_| HumanQrLoginError::OidcMetadataInvalid)?; let oauth = client.inner.oauth(); - let login = oauth.login_with_qr_code(&qr_code_data.inner, client_metadata); + let login = oauth.login_with_qr_code(&qr_code_data.inner, registrations.into()); let mut progress = login.subscribe_to_progress(); diff --git a/crates/matrix-sdk/src/authentication/oauth/mod.rs b/crates/matrix-sdk/src/authentication/oauth/mod.rs index edb6481a6..cde0087a3 100644 --- a/crates/matrix-sdk/src/authentication/oauth/mod.rs +++ b/crates/matrix-sdk/src/authentication/oauth/mod.rs @@ -379,7 +379,7 @@ impl OAuth { /// /// // Subscribing to the progress is necessary since we need to input the check /// // code on the existing device. - /// let login = oauth.login_with_qr_code(&qr_code_data, metadata); + /// let login = oauth.login_with_qr_code(&qr_code_data, metadata.into()); /// let mut progress = login.subscribe_to_progress(); /// /// // Create a task which will show us the progress and tell us the check @@ -411,9 +411,9 @@ impl OAuth { pub fn login_with_qr_code<'a>( &'a self, data: &'a QrCodeData, - client_metadata: Raw, + registration_method: ClientRegistrationMethod, ) -> LoginWithQrCode<'a> { - LoginWithQrCode::new(&self.client, client_metadata, data) + LoginWithQrCode::new(&self.client, registration_method, data) } /// A higher level wrapper around the configuration and login methods that @@ -443,9 +443,7 @@ impl OAuth { redirect_uri: Url, prompt: Option, ) -> Result { - let server_metadata = self.server_metadata().await?; - - self.restore_or_register_client(server_metadata.issuer, registrations).await?; + self.use_registration_method(®istrations.into()).await?; let mut data_builder = self.login(redirect_uri, None); @@ -489,26 +487,19 @@ impl OAuth { Ok(()) } - /// Restore or register the OAuth 2.0 client for the given issuer with the - /// given [`OAuthRegistrationStore`]. + /// Restore or register the OAuth 2.0 client for the server with the given + /// metadata, with the given [`OAuthRegistrationStore`]. /// /// If there is a client ID in the store, it is used to restore the client. /// Otherwise, the client is registered with the metadata in the store. /// - /// If we already have a client ID, this is a noop. - /// /// Returns an error if there is an error while accessing the store, or /// while registering the client. async fn restore_or_register_client( &self, issuer: Url, - registrations: OAuthRegistrationStore, + registrations: &OAuthRegistrationStore, ) -> std::result::Result<(), OAuthError> { - if self.client_id().is_some() { - tracing::info!("OAuth 2.0 is already configured."); - return Ok(()); - }; - if let Some(client_id) = registrations.client_id(&issuer).await.map_err(OAuthClientRegistrationError::from)? { @@ -530,6 +521,39 @@ impl OAuth { Ok(()) } + /// Restore or register the OAuth 2.0 client for the server with the given + /// metadata, with the given [`ClientRegistrationMethod`]. + /// + /// If we already have a client ID, this is a noop. + /// + /// Returns an error if there was a problem using the registration method. + async fn use_registration_method( + &self, + method: &ClientRegistrationMethod, + ) -> std::result::Result<(), OAuthError> { + if self.client_id().is_some() { + tracing::info!("OAuth 2.0 is already configured."); + return Ok(()); + }; + + match method { + ClientRegistrationMethod::None => return Err(OAuthError::NotRegistered), + ClientRegistrationMethod::ClientId(client_id) => { + let server_metadata = self.server_metadata().await?; + self.restore_registered_client(server_metadata.issuer, client_id.clone()); + } + ClientRegistrationMethod::Metadata(client_metadata) => { + self.register_client(client_metadata).await?; + } + ClientRegistrationMethod::Store(registrations) => { + let server_metadata = self.server_metadata().await?; + self.restore_or_register_client(server_metadata.issuer, registrations).await? + } + } + + Ok(()) + } + /// The OAuth 2.0 authorization server used for authorization. /// /// Returns `None` if the client was not registered or if the registration @@ -1557,3 +1581,45 @@ pub struct AuthorizationError { fn hash_str(x: &str) -> impl fmt::LowerHex { sha2::Sha256::new().chain_update(x).finalize() } + +/// The available methods to register or restore a client. +#[derive(Debug)] +pub enum ClientRegistrationMethod { + /// No registration will be done. + /// + /// This should only be set if [`OAuth::register_client()`] or + /// [`OAuth::restore_registered_client()`] was already called before. + None, + + /// The given client ID will be used. + /// + /// This will call [`OAuth::restore_registered_client()`] internally. + ClientId(ClientId), + + /// The client will register using dynamic client registration, with the + /// given metadata. + /// + /// This will call [`OAuth::register_client()`] internally. + Metadata(Raw), + + /// Use an [`OAuthRegistrationStore`] to handle registrations. + Store(OAuthRegistrationStore), +} + +impl From for ClientRegistrationMethod { + fn from(value: ClientId) -> Self { + Self::ClientId(value) + } +} + +impl From> for ClientRegistrationMethod { + fn from(value: Raw) -> Self { + Self::Metadata(value) + } +} + +impl From for ClientRegistrationMethod { + fn from(value: OAuthRegistrationStore) -> Self { + Self::Store(value) + } +} diff --git a/crates/matrix-sdk/src/authentication/oauth/qrcode/login.rs b/crates/matrix-sdk/src/authentication/oauth/qrcode/login.rs index cdea11ced..6bee65e49 100644 --- a/crates/matrix-sdk/src/authentication/oauth/qrcode/login.rs +++ b/crates/matrix-sdk/src/authentication/oauth/qrcode/login.rs @@ -22,7 +22,7 @@ use matrix_sdk_base::{ SessionMeta, }; use oauth2::{DeviceCodeErrorResponseType, StandardDeviceAuthorizationResponse}; -use ruma::{serde::Raw, OwnedDeviceId}; +use ruma::OwnedDeviceId; use tracing::trace; use vodozemac::{ecies::CheckCode, Curve25519PublicKey}; @@ -33,7 +33,7 @@ use super::{ }; #[cfg(doc)] use crate::authentication::oauth::OAuth; -use crate::{authentication::oauth::ClientMetadata, Client}; +use crate::{authentication::oauth::ClientRegistrationMethod, Client}; async fn send_unexpected_message_error( channel: &mut EstablishedSecureChannel, @@ -76,7 +76,7 @@ pub enum LoginProgress { #[derive(Debug)] pub struct LoginWithQrCode<'a> { client: &'a Client, - client_metadata: Raw, + registration_method: ClientRegistrationMethod, qr_code_data: &'a QrCodeData, state: SharedObservable, } @@ -260,10 +260,10 @@ impl<'a> IntoFuture for LoginWithQrCode<'a> { impl<'a> LoginWithQrCode<'a> { pub(crate) fn new( client: &'a Client, - client_metadata: Raw, + registration_method: ClientRegistrationMethod, qr_code_data: &'a QrCodeData, ) -> LoginWithQrCode<'a> { - LoginWithQrCode { client, client_metadata, qr_code_data, state: Default::default() } + LoginWithQrCode { client, registration_method, qr_code_data, state: Default::default() } } async fn establish_secure_channel( @@ -284,7 +284,8 @@ impl<'a> LoginWithQrCode<'a> { /// Register the client with the OAuth 2.0 authorization server. async fn register_client(&self) -> Result<(), DeviceAuthorizationOAuthError> { let oauth = self.client.oauth(); - oauth.register_client(&self.client_metadata).await?; + oauth.use_registration_method(&self.registration_method).await?; + Ok(()) } @@ -446,7 +447,7 @@ mod test { let qr_code = alice.qr_code_data().clone(); let oauth = bob.oauth(); - let login_bob = oauth.login_with_qr_code(&qr_code, mock_client_metadata()); + let login_bob = oauth.login_with_qr_code(&qr_code, mock_client_metadata().into()); let mut updates = login_bob.subscribe_to_progress(); let updates_task = tokio::spawn(async move { @@ -533,7 +534,7 @@ mod test { let qr_code = alice.qr_code_data().clone(); let oauth = bob.oauth(); - let login_bob = oauth.login_with_qr_code(&qr_code, mock_client_metadata()); + let login_bob = oauth.login_with_qr_code(&qr_code, mock_client_metadata().into()); let mut updates = login_bob.subscribe_to_progress(); let _updates_task = tokio::spawn(async move { @@ -656,7 +657,7 @@ mod test { let qr_code = alice.qr_code_data().clone(); let oauth = bob.oauth(); - let login_bob = oauth.login_with_qr_code(&qr_code, mock_client_metadata()); + let login_bob = oauth.login_with_qr_code(&qr_code, mock_client_metadata().into()); let mut updates = login_bob.subscribe_to_progress(); let _updates_task = tokio::spawn(async move { diff --git a/examples/qr-login/src/main.rs b/examples/qr-login/src/main.rs index 22744f5f1..650ae7021 100644 --- a/examples/qr-login/src/main.rs +++ b/examples/qr-login/src/main.rs @@ -120,7 +120,7 @@ async fn login(proxy: Option) -> Result<()> { let metadata = client_metadata(); let oauth = client.oauth(); - let login_client = oauth.login_with_qr_code(&data, metadata); + let login_client = oauth.login_with_qr_code(&data, metadata.into()); let mut subscriber = login_client.subscribe_to_progress(); let task = tokio::spawn(async move {