From 3294bd0fa7bb1dfec517a10600169d979ea527d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Wed, 16 Oct 2024 10:11:30 +0200 Subject: [PATCH] refactor(oidc): allow passing a `Prompt` to get an OIDC url This allows clients to directly open the web page they want: the login one, the registration one, consent, etc. It should improve the UX in the registration flow since we can now skip the login one. --- bindings/matrix-sdk-ffi/src/client.rs | 56 +++++++++++++++++++++++++-- crates/matrix-sdk/src/oidc/mod.rs | 7 ++-- crates/matrix-sdk/src/oidc/tests.rs | 7 ++-- 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 8e451b65b6e..3ba42f98bb7 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -19,6 +19,7 @@ use matrix_sdk::{ registration::{ ClientMetadata, ClientMetadataVerificationError, VerifiedClientMetadata, }, + requests::Prompt as SdkOidcPrompt, }, OidcAuthorizationData, OidcSession, }, @@ -360,13 +361,14 @@ impl Client { Ok(Arc::new(SsoHandler { client: Arc::clone(self), url })) } - /// Requests the URL needed for login in a web view using OIDC. Once the web + /// Requests the URL needed for opening a web view using OIDC. Once the web /// view has succeeded, call `login_with_oidc_callback` with the callback it /// returns. If a failure occurs and a callback isn't available, make sure - /// to call `abort_oidc_login` to inform the client of this. - pub async fn url_for_oidc_login( + /// to call `abort_oidc` to inform the client of this. + pub async fn url_for_oidc( &self, oidc_configuration: &OidcConfiguration, + prompt: OidcPrompt, ) -> Result, OidcError> { let oidc_metadata: VerifiedClientMetadata = oidc_configuration.try_into()?; let registrations_file = Path::new(&oidc_configuration.dynamic_registrations_file); @@ -387,7 +389,8 @@ impl Client { static_registrations, )?; - let data = self.inner.oidc().url_for_oidc_login(oidc_metadata, registrations).await?; + let data = + self.inner.oidc().url_for_oidc(oidc_metadata, registrations, prompt.into()).await?; Ok(Arc::new(data)) } @@ -1731,3 +1734,48 @@ impl TryFrom for SdkSlidingSyncVersion { }) } } + +#[derive(uniffi::Enum)] +pub enum OidcPrompt { + /// The Authorization Server must not display any authentication or consent + /// user interface pages. + None, + + /// The Authorization Server should prompt the End-User for + /// reauthentication. + Login, + + /// The Authorization Server should prompt the End-User for consent before + /// returning information to the Client. + Consent, + + /// The Authorization Server should prompt the End-User to select a user + /// account. + /// + /// This enables an End-User who has multiple accounts at the Authorization + /// Server to select amongst the multiple accounts that they might have + /// current sessions for. + SelectAccount, + + /// The Authorization Server should prompt the End-User to create a user + /// account. + /// + /// Defined in [Initiating User Registration via OpenID Connect](https://openid.net/specs/openid-connect-prompt-create-1_0.html). + Create, + + /// An unknown value. + Unknown(String), +} + +impl From for SdkOidcPrompt { + fn from(value: OidcPrompt) -> Self { + match value { + OidcPrompt::None => Self::None, + OidcPrompt::Login => Self::Login, + OidcPrompt::Consent => Self::Consent, + OidcPrompt::SelectAccount => Self::SelectAccount, + OidcPrompt::Create => Self::Create, + OidcPrompt::Unknown(value) => Self::Unknown(value), + } + } +} diff --git a/crates/matrix-sdk/src/oidc/mod.rs b/crates/matrix-sdk/src/oidc/mod.rs index 4f2e7fc2d0b..ab686b23a96 100644 --- a/crates/matrix-sdk/src/oidc/mod.rs +++ b/crates/matrix-sdk/src/oidc/mod.rs @@ -443,10 +443,11 @@ impl Oidc { /// webview for a user to login to their account. Call /// [`Oidc::login_with_oidc_callback`] to finish the process when the /// webview is complete. - pub async fn url_for_oidc_login( + pub async fn url_for_oidc( &self, client_metadata: VerifiedClientMetadata, registrations: OidcRegistrations, + prompt: Prompt, ) -> Result { let issuer = match self.fetch_authentication_issuer().await { Ok(issuer) => issuer, @@ -470,7 +471,7 @@ impl Oidc { self.configure(issuer, client_metadata, registrations).await?; let mut data_builder = self.login(redirect_url.clone(), None)?; - data_builder = data_builder.prompt(vec![Prompt::Consent]); + data_builder = data_builder.prompt(vec![prompt]); let data = data_builder.build().await?; Ok(data) @@ -478,7 +479,7 @@ impl Oidc { /// A higher level wrapper around the methods to complete a login after the /// user has logged in through a webview. This method should be used in - /// tandem with [`Oidc::url_for_oidc_login`]. + /// tandem with [`Oidc::url_for_oidc`]. pub async fn login_with_oidc_callback( &self, authorization_data: &OidcAuthorizationData, diff --git a/crates/matrix-sdk/src/oidc/tests.rs b/crates/matrix-sdk/src/oidc/tests.rs index 5c6a7bc8ba1..0c40cff89cf 100644 --- a/crates/matrix-sdk/src/oidc/tests.rs +++ b/crates/matrix-sdk/src/oidc/tests.rs @@ -9,6 +9,7 @@ use mas_oidc_client::{ errors::ClientErrorCode, iana::oauth::OAuthClientAuthenticationMethod, registration::{ClientMetadata, VerifiedClientMetadata}, + requests::Prompt, }, }; use matrix_sdk_base::SessionMeta; @@ -124,7 +125,7 @@ async fn test_high_level_login() -> anyhow::Result<()> { // When getting the OIDC login URL. let authorization_data = - oidc.url_for_oidc_login(metadata.clone(), registrations).await.unwrap(); + oidc.url_for_oidc(metadata.clone(), registrations, Prompt::Login).await.unwrap(); // Then the client should be configured correctly. assert!(oidc.issuer().is_some()); @@ -146,7 +147,7 @@ async fn test_high_level_login_cancellation() -> anyhow::Result<()> { // Given a client ready to complete login. let (oidc, _server, metadata, registrations) = mock_environment().await.unwrap(); let authorization_data = - oidc.url_for_oidc_login(metadata.clone(), registrations).await.unwrap(); + oidc.url_for_oidc(metadata.clone(), registrations, Prompt::Login).await.unwrap(); assert!(oidc.issuer().is_some()); assert!(oidc.client_metadata().is_some()); @@ -170,7 +171,7 @@ async fn test_high_level_login_invalid_state() -> anyhow::Result<()> { // Given a client ready to complete login. let (oidc, _server, metadata, registrations) = mock_environment().await.unwrap(); let authorization_data = - oidc.url_for_oidc_login(metadata.clone(), registrations).await.unwrap(); + oidc.url_for_oidc(metadata.clone(), registrations, Prompt::Login).await.unwrap(); assert!(oidc.issuer().is_some()); assert!(oidc.client_metadata().is_some());