Skip to content

Commit

Permalink
refactor(oidc): allow passing a Prompt to get an OIDC url
Browse files Browse the repository at this point in the history
Changelog: `Client::url_for_oidc_login` is now `Client::url_for_oidc` with an additional `OidcPrompt` parameter. `abort_oidc_login` has been renamed to `abort_oidc_auth`.

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.
  • Loading branch information
jmartinesp authored and bnjbvr committed Oct 16, 2024
1 parent 3d04234 commit 79798a9
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 11 deletions.
58 changes: 53 additions & 5 deletions bindings/matrix-sdk-ffi/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use matrix_sdk::{
registration::{
ClientMetadata, ClientMetadataVerificationError, VerifiedClientMetadata,
},
requests::Prompt as SdkOidcPrompt,
},
OidcAuthorizationData, OidcSession,
},
Expand Down Expand Up @@ -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_auth` to inform the client of this.
pub async fn url_for_oidc(
&self,
oidc_configuration: &OidcConfiguration,
prompt: OidcPrompt,
) -> Result<Arc<OidcAuthorizationData>, OidcError> {
let oidc_metadata: VerifiedClientMetadata = oidc_configuration.try_into()?;
let registrations_file = Path::new(&oidc_configuration.dynamic_registrations_file);
Expand All @@ -387,14 +389,15 @@ 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))
}

/// Aborts an existing OIDC login operation that might have been cancelled,
/// failed etc.
pub async fn abort_oidc_login(&self, authorization_data: Arc<OidcAuthorizationData>) {
pub async fn abort_oidc_auth(&self, authorization_data: Arc<OidcAuthorizationData>) {
self.inner.oidc().abort_authorization(&authorization_data.state).await;
}

Expand Down Expand Up @@ -1731,3 +1734,48 @@ impl TryFrom<SlidingSyncVersion> 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 { value: String },
}

impl From<OidcPrompt> 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),
}
}
}
7 changes: 4 additions & 3 deletions crates/matrix-sdk/src/oidc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<OidcAuthorizationData, OidcError> {
let issuer = match self.fetch_authentication_issuer().await {
Ok(issuer) => issuer,
Expand All @@ -470,15 +471,15 @@ 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)
}

/// 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,
Expand Down
7 changes: 4 additions & 3 deletions crates/matrix-sdk/src/oidc/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use mas_oidc_client::{
errors::ClientErrorCode,
iana::oauth::OAuthClientAuthenticationMethod,
registration::{ClientMetadata, VerifiedClientMetadata},
requests::Prompt,
},
};
use matrix_sdk_base::SessionMeta;
Expand Down Expand Up @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand Down

0 comments on commit 79798a9

Please sign in to comment.