refactor(oauth): Introduce AccountManagementUrlBuilder

It allows to reuse the URL for different actions more easily than having
to call `OAuth::account_management_url` every time for a different
action.

It also adds a method with fallback if we want to ignore action
serialization errors, to always present a URL.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>

bindings/matrix-sdk-ffi/src/client.rs

@@ -8,7 +8,7 @@ use anyhow::{anyhow, Context as _};
 use async_compat::get_runtime_handle;
 use matrix_sdk::{
     authentication::oauth::{
-        AccountManagementActionFull, ClientId, OAuthAuthorizationData, OAuthSession,
+        AccountManagementActionFull, ClientId, OAuthAuthorizationData, OAuthError, OAuthSession,
     },
     event_cache::EventCacheError,
     media::{
@@ -606,11 +606,24 @@ impl Client {
             return Ok(None);
         }
 
-        match self.inner.oauth().account_management_url(action.map(Into::into)).await {
-            Ok(url) => Ok(url.map(|u| u.to_string())),
+        let mut url_builder = match self.inner.oauth().account_management_url().await {
+            Ok(Some(url_builder)) => url_builder,
+            Ok(None) => return Ok(None),
             Err(e) => {
                 tracing::error!("Failed retrieving account management URL: {e}");
-                Err(e.into())
+                return Err(e.into());
+            }
+        };
+
+        if let Some(action) = action {
+            url_builder = url_builder.action(action.into());
+        }
+
+        match url_builder.build() {
+            Ok(url) => Ok(Some(url.to_string())),
+            Err(e) => {
+                tracing::error!("Failed to build account management URL: {e}");
+                Err(OAuthError::AccountManagementUrl(e).into())
             }
         }
     }

crates/matrix-sdk/CHANGELOG.md

@@ -224,6 +224,12 @@ simpler methods:
     call to `OAuth::finish_login()`.
   - `AuthorizationResponse`, `AuthorizationCode` and `AuthorizationError` are
     now private.
+- [**breaking**] - `OAuth::account_management_url()` and
+  `OAuth::fetch_account_management_url()` don't take an action anymore but
+  return an `AccountManagementUrlBuilder`. The final URL can be obtained with
+  `AccountManagementUrlBuilder::build()` or
+  `AccountManagementUrlBuilder::build_or_ignore_action()`.
+  ([#4831](https://github.com/matrix-org/matrix-rust-sdk/pull/4831))
 
 ## [0.10.0] - 2025-02-04
 

crates/matrix-sdk/src/authentication/oauth/account_management_url.rs

@@ -73,43 +73,82 @@ pub enum AccountManagementActionFull {
     CrossSigningReset,
 }
 
-/// Build the URL for accessing the account management capabilities, as defined
-/// in [MSC].
+/// Builder for the URL for accessing the account management capabilities, as
+/// defined in [MSC4191].
 ///
-/// # Arguments
+/// This type can be instantiated with [`OAuth::account_management_url()`] and
+/// [`OAuth::fetch_account_management_url()`].
 ///
-/// * `account_management_uri` - The URL to access the issuer's account
-///   management capabilities.
-///
-/// * `action` - The action that the user wishes to take.
-///
-/// # Returns
-///
-/// A URL to be opened in a web browser where the end-user will be able to
-/// access the account management capabilities of the issuer.
-///
-/// # Errors
-///
-/// Returns an error if serializing the action fails.
+/// [`AccountManagementUrlBuilder::build()`] and
+/// [`AccountManagementUrlBuilder::build_or_ignore_action()`] return a URL to be
+/// opened in a web browser where the end-user will be able to access the
+/// account management capabilities of the issuer.
 ///
 /// [MSC4191]: https://github.com/matrix-org/matrix-spec-proposals/pull/4191
-pub(crate) fn build_account_management_url(
-    mut account_management_uri: Url,
-    action: AccountManagementActionFull,
-) -> Result<Url, serde_html_form::ser::Error> {
-    let extra_query = serde_html_form::to_string(action)?;
+/// [`OAuth::account_management_url()`]: super::OAuth::account_management_url
+/// [`OAuth::fetch_account_management_url()`]: super::OAuth::fetch_account_management_url
+#[derive(Debug, Clone)]
+pub struct AccountManagementUrlBuilder {
+    account_management_uri: Url,
+    action: Option<AccountManagementActionFull>,
+}
 
-    // Add our parameters to the query, because the URL might already have one.
-    let mut full_query = account_management_uri.query().map(ToOwned::to_owned).unwrap_or_default();
-
-    if !full_query.is_empty() {
-        full_query.push('&');
+impl AccountManagementUrlBuilder {
+    /// Construct an [`AccountManagementUrlBuilder`] for the given URL.
+    pub(super) fn new(account_management_uri: Url) -> Self {
+        Self { account_management_uri, action: None }
     }
-    full_query.push_str(&extra_query);
 
-    account_management_uri.set_query(Some(&full_query));
+    /// Set the action that the user wishes to take.
+    pub fn action(mut self, action: AccountManagementActionFull) -> Self {
+        self.action = Some(action);
+        self
+    }
 
-    Ok(account_management_uri)
+    /// Get the serialized action, if any.
+    fn serialized_action(&self) -> Result<Option<String>, serde_html_form::ser::Error> {
+        self.action.as_ref().map(serde_html_form::to_string).transpose()
+    }
+
+    /// Build the URL with the given serialized action.
+    fn build_inner(self, serialized_action: Option<String>) -> Url {
+        let Some(serialized_action) = serialized_action else {
+            return self.account_management_uri;
+        };
+
+        // Add our parameters to the query, because the URL might already have one.
+        let mut account_management_uri = self.account_management_uri;
+        let mut full_query =
+            account_management_uri.query().map(ToOwned::to_owned).unwrap_or_default();
+
+        if !full_query.is_empty() {
+            full_query.push('&');
+        }
+        full_query.push_str(&serialized_action);
+
+        account_management_uri.set_query(Some(&full_query));
+
+        account_management_uri
+    }
+
+    /// Build the URL to present to the end user.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if serializing the action fails.
+    pub fn build(self) -> Result<Url, serde_html_form::ser::Error> {
+        let serialized_action = self.serialized_action()?;
+        Ok(self.build_inner(serialized_action))
+    }
+
+    /// Build the URL to present to the end user and ignore errors while
+    /// serializing the action.
+    ///
+    /// Returns the URL without an action if the action fails to serialize.
+    pub fn build_or_ignore_action(self) -> Url {
+        let serialized_action = self.serialized_action().ok().flatten();
+        self.build_inner(serialized_action)
+    }
 }
 
 #[cfg(test)]
@@ -117,55 +156,56 @@ mod tests {
     use ruma::owned_device_id;
     use url::Url;
 
-    use super::{build_account_management_url, AccountManagementActionFull};
+    use super::{AccountManagementActionFull, AccountManagementUrlBuilder};
 
     #[test]
     fn test_build_account_management_url_actions() {
         let base_url = Url::parse("https://example.org").unwrap();
         let device_id = owned_device_id!("ABCDEFG");
 
-        let url =
-            build_account_management_url(base_url.clone(), AccountManagementActionFull::Profile)
-                .unwrap();
+        let url = AccountManagementUrlBuilder::new(base_url.clone()).build().unwrap();
+        assert_eq!(url, base_url);
+
+        let url = AccountManagementUrlBuilder::new(base_url.clone())
+            .action(AccountManagementActionFull::Profile)
+            .build()
+            .unwrap();
         assert_eq!(url.as_str(), "https://example.org/?action=org.matrix.profile");
 
-        let url = build_account_management_url(
-            base_url.clone(),
-            AccountManagementActionFull::SessionsList,
-        )
-        .unwrap();
+        let url = AccountManagementUrlBuilder::new(base_url.clone())
+            .action(AccountManagementActionFull::SessionsList)
+            .build()
+            .unwrap();
         assert_eq!(url.as_str(), "https://example.org/?action=org.matrix.sessions_list");
 
-        let url = build_account_management_url(
-            base_url.clone(),
-            AccountManagementActionFull::SessionView { device_id: device_id.clone() },
-        )
-        .unwrap();
+        let url = AccountManagementUrlBuilder::new(base_url.clone())
+            .action(AccountManagementActionFull::SessionView { device_id: device_id.clone() })
+            .build()
+            .unwrap();
         assert_eq!(
             url.as_str(),
             "https://example.org/?action=org.matrix.session_view&device_id=ABCDEFG"
         );
 
-        let url = build_account_management_url(
-            base_url.clone(),
-            AccountManagementActionFull::SessionEnd { device_id },
-        )
-        .unwrap();
+        let url = AccountManagementUrlBuilder::new(base_url.clone())
+            .action(AccountManagementActionFull::SessionEnd { device_id })
+            .build()
+            .unwrap();
         assert_eq!(
             url.as_str(),
             "https://example.org/?action=org.matrix.session_end&device_id=ABCDEFG"
         );
 
-        let url = build_account_management_url(
-            base_url.clone(),
-            AccountManagementActionFull::AccountDeactivate,
-        )
-        .unwrap();
+        let url = AccountManagementUrlBuilder::new(base_url.clone())
+            .action(AccountManagementActionFull::AccountDeactivate)
+            .build()
+            .unwrap();
         assert_eq!(url.as_str(), "https://example.org/?action=org.matrix.account_deactivate");
 
-        let url =
-            build_account_management_url(base_url, AccountManagementActionFull::CrossSigningReset)
-                .unwrap();
+        let url = AccountManagementUrlBuilder::new(base_url)
+            .action(AccountManagementActionFull::CrossSigningReset)
+            .build()
+            .unwrap();
         assert_eq!(url.as_str(), "https://example.org/?action=org.matrix.cross_signing_reset");
     }
 
@@ -173,19 +213,34 @@ mod tests {
     fn test_build_account_management_url_with_query() {
         let base_url = Url::parse("https://example.org/?sid=123456").unwrap();
 
-        let url =
-            build_account_management_url(base_url.clone(), AccountManagementActionFull::Profile)
-                .unwrap();
+        let url = AccountManagementUrlBuilder::new(base_url.clone())
+            .action(AccountManagementActionFull::Profile)
+            .build()
+            .unwrap();
         assert_eq!(url.as_str(), "https://example.org/?sid=123456&action=org.matrix.profile");
 
-        let url = build_account_management_url(
-            base_url,
-            AccountManagementActionFull::SessionView { device_id: owned_device_id!("ABCDEFG") },
-        )
-        .unwrap();
+        let url = AccountManagementUrlBuilder::new(base_url)
+            .action(AccountManagementActionFull::SessionView {
+                device_id: owned_device_id!("ABCDEFG"),
+            })
+            .build()
+            .unwrap();
         assert_eq!(
             url.as_str(),
             "https://example.org/?sid=123456&action=org.matrix.session_view&device_id=ABCDEFG"
         );
     }
+
+    #[test]
+    fn test_build_account_management_url_or_ignore_action() {
+        let base_url = Url::parse("https://example.org").unwrap();
+
+        let url = AccountManagementUrlBuilder::new(base_url.clone()).build_or_ignore_action();
+        assert_eq!(url, base_url);
+
+        let url = AccountManagementUrlBuilder::new(base_url)
+            .action(AccountManagementActionFull::Profile)
+            .build_or_ignore_action();
+        assert_eq!(url.as_str(), "https://example.org/?action=org.matrix.profile");
+    }
 }

crates/matrix-sdk/src/authentication/oauth/mod.rs

@@ -187,17 +187,16 @@ use self::cross_process::{CrossProcessRefreshLockGuard, CrossProcessRefreshManag
 use self::qrcode::LoginWithQrCode;
 #[cfg(not(target_arch = "wasm32"))]
 pub use self::registration_store::OAuthRegistrationStore;
+pub use self::{
+    account_management_url::{AccountManagementActionFull, AccountManagementUrlBuilder},
+    auth_code_builder::{OAuthAuthCodeUrlBuilder, OAuthAuthorizationData},
+    error::OAuthError,
+};
 use self::{
-    account_management_url::build_account_management_url,
     http_client::OAuthHttpClient,
     oidc_discovery::discover,
     registration::{register_client, ClientMetadata, ClientRegistrationResponse},
 };
-pub use self::{
-    account_management_url::AccountManagementActionFull,
-    auth_code_builder::{OAuthAuthCodeUrlBuilder, OAuthAuthorizationData},
-    error::OAuthError,
-};
 use super::{AuthData, SessionTokens};
 use crate::{client::SessionChange, executor::spawn, Client, HttpError, RefreshTokenError, Result};
 
@@ -516,63 +515,46 @@ impl OAuth {
 
     /// Build the URL where the user can manage their account.
     ///
-    /// # Arguments
+    /// This will always request the latest server metadata to get the account
+    /// management URL.
     ///
-    /// * `action` - An optional action that wants to be performed by the user
-    ///   when they open the URL. The list of supported actions by the account
-    ///   management URL can be found in the [`AuthorizationServerMetadata`], or
-    ///   directly with [`OAuth::account_management_actions_supported()`].
+    /// To avoid making a request each time, you can use
+    /// [`OAuth::account_management_url()`].
     ///
-    /// Returns `Ok(None)` if the URL was not found. Returns an error if the
-    /// request to get the server metadata fails or the URL could not be parsed.
+    /// Returns an [`AccountManagementUrlBuilder`] if the URL was found. An
+    /// optional action to perform can be added with `.action()`, and the final
+    /// URL is obtained with `.build()` or `.build_or_ignore_action()`.
+    ///
+    /// Returns `Ok(None)` if the URL was not found.
+    ///
+    /// Returns an error if the request to get the server metadata fails or the
+    /// URL could not be parsed.
     pub async fn fetch_account_management_url(
         &self,
-        action: Option<AccountManagementActionFull>,
-    ) -> Result<Option<Url>, OAuthError> {
+    ) -> Result<Option<AccountManagementUrlBuilder>, OAuthError> {
         let server_metadata = self.server_metadata().await?;
-        self.management_url_from_server_metadata(server_metadata, action)
-    }
-
-    fn management_url_from_server_metadata(
-        &self,
-        server_metadata: AuthorizationServerMetadata,
-        action: Option<AccountManagementActionFull>,
-    ) -> Result<Option<Url>, OAuthError> {
-        let Some(base_url) = server_metadata.account_management_uri else {
-            return Ok(None);
-        };
-
-        let url = if let Some(action) = action {
-            build_account_management_url(base_url, action)
-                .map_err(OAuthError::AccountManagementUrl)?
-        } else {
-            base_url
-        };
-
-        Ok(Some(url))
+        Ok(server_metadata.account_management_uri.map(AccountManagementUrlBuilder::new))
     }
 
     /// Get the account management URL where the user can manage their
     /// identity-related settings.
     ///
-    /// # Arguments
-    ///
-    /// * `action` - An optional action that wants to be performed by the user
-    ///   when they open the URL. The list of supported actions by the account
-    ///   management URL can be found in the [`AuthorizationServerMetadata`], or
-    ///   directly with [`OAuth::account_management_actions_supported()`].
-    ///
-    /// Returns `Ok(None)` if the URL was not found. Returns an error if the
-    /// request to get the server metadata fails or the URL could not be parsed.
-    ///
     /// This method will cache the URL for a while, if the cache is not
-    /// populated it will internally call
-    /// [`OAuth::fetch_account_management_url()`] and cache the resulting URL
+    /// populated it will request the server metadata, like a call to
+    /// [`OAuth::fetch_account_management_url()`], and cache the resulting URL
     /// before returning it.
+    ///
+    /// Returns an [`AccountManagementUrlBuilder`] if the URL was found. An
+    /// optional action to perform can be added with `.action()`, and the final
+    /// URL is obtained with `.build()` or `.build_or_ignore_action()`.
+    ///
+    /// Returns `Ok(None)` if the URL was not found.
+    ///
+    /// Returns an error if the request to get the server metadata fails or the
+    /// URL could not be parsed.
     pub async fn account_management_url(
         &self,
-        action: Option<AccountManagementActionFull>,
-    ) -> Result<Option<Url>, OAuthError> {
+    ) -> Result<Option<AccountManagementUrlBuilder>, OAuthError> {
         const CACHE_KEY: &str = "SERVER_METADATA";
 
         let mut cache = self.client.inner.caches.server_metadata.lock().await;
@@ -585,7 +567,7 @@ impl OAuth {
             server_metadata
         };
 
-        self.management_url_from_server_metadata(metadata, action)
+        Ok(metadata.account_management_uri.map(AccountManagementUrlBuilder::new))
     }
 
     /// Discover the authentication issuer and retrieve the

crates/matrix-sdk/src/authentication/oauth/tests.rs

@@ -24,8 +24,7 @@ use super::{
 use crate::{
     authentication::oauth::{
         error::{AuthorizationCodeErrorResponseType, OAuthClientRegistrationError},
-        AccountManagementActionFull, AuthorizationValidationData, ClientRegistrationMethod,
-        OAuthAuthorizationCodeError,
+        AuthorizationValidationData, ClientRegistrationMethod, OAuthAuthorizationCodeError,
     },
     test_utils::{
         client::{
@@ -659,7 +658,7 @@ async fn test_management_url_cache() {
     assert!(!client.inner.caches.server_metadata.lock().await.contains("SERVER_METADATA"));
 
     let management_url = oauth
-        .account_management_url(Some(AccountManagementActionFull::Profile))
+        .account_management_url()
         .await
         .expect("We should be able to fetch the account management url");
 
@@ -668,9 +667,9 @@ async fn test_management_url_cache() {
     // Check that the server metadata has been inserted into the cache.
     assert!(client.inner.caches.server_metadata.lock().await.contains("SERVER_METADATA"));
 
-    // Another parameter doesn't make another request for the metadata.
+    // Another call doesn't make another request for the metadata.
     let management_url = oauth
-        .account_management_url(Some(AccountManagementActionFull::SessionsList))
+        .account_management_url()
         .await
         .expect("We should be able to fetch the account management url");
 

examples/oauth_cli/src/main.rs

@@ -389,14 +389,23 @@ impl OAuthCli {
 
     /// Get the account management URL.
     async fn account(&self, action: Option<AccountManagementActionFull>) {
-        match self.client.oauth().fetch_account_management_url(action).await {
-            Ok(Some(url)) => {
-                println!("\nTo manage your account, visit: {url}");
-            }
+        let mut url_builder = match self.client.oauth().fetch_account_management_url().await {
+            Ok(Some(url_builder)) => url_builder,
             _ => {
-                println!("\nThis homeserver does not provide the URL to manage your account")
+                println!("\nThis homeserver does not provide the URL to manage your account");
+                return;
             }
+        };
+
+        if let Some(action) = action {
+            url_builder = url_builder.action(action);
         }
+
+        // We ignore errors while serializing the action, that way we can always direct
+        // the end user to the URL and they should be able to do what they want there.
+        let url = url_builder.build_or_ignore_action();
+
+        println!("\nTo manage your account, visit: {url}");
     }
 
     /// Watch incoming messages.