feat: Add a memoized variant of Oidc::fetch_account_management_url

Author: poljar

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

@@ -626,7 +626,7 @@ impl Client {
             return Ok(None);
         }
 
-        match self.inner.oidc().fetch_account_management_url(action.map(Into::into)).await {
+        match self.inner.oidc().account_management_url(action.map(Into::into)).await {
             Ok(url) => Ok(url.map(|u| u.to_string())),
             Err(e) => {
                 tracing::error!("Failed retrieving account management URL: {e}");

crates/matrix-sdk/CHANGELOG.md

@@ -8,6 +8,13 @@ All notable changes to this project will be documented in this file.
 
 ### Features
 
+- [**breaking**]: The `Oidc::account_management_url` method now caches the
+  result of a call, subsequent calls to the method will not contact the OIDC
+  provider for a while, instead the cached URI will be returned. If caching of
+  this URI is not desirable, the `Oidc::fetch_account_management_url` method
+  can be used.
+  ([#4663](https://github.com/matrix-org/matrix-rust-sdk/pull/4663))
+
 - The `MediaRetentionPolicy` can now trigger regular cleanups with its new
   `cleanup_frequency` setting.
   ([#4603](https://github.com/matrix-org/matrix-rust-sdk/pull/4603))

crates/matrix-sdk/src/authentication/oidc/backend/mock.rs

@@ -67,6 +67,8 @@ pub(crate) struct MockImpl {
     /// Must be an HTTPS URL.
     registration_endpoint: Option<Url>,
 
+    account_management_uri: Option<String>,
+
     /// The next session tokens that will be returned by a login or refresh.
     next_session_tokens: Option<OidcSessionTokens>,
 
@@ -94,6 +96,7 @@ impl MockImpl {
             registration_endpoint: Some(Url::parse(REGISTRATION_URL).unwrap()),
             next_session_tokens: None,
             expected_refresh_token: None,
+            account_management_uri: None,
             num_refreshes: Default::default(),
             revoked_tokens: Default::default(),
             is_insecure: false,
@@ -119,6 +122,11 @@ impl MockImpl {
         self.registration_endpoint = registration_endpoint;
         self
     }
+
+    pub fn account_management_uri(mut self, uri: String) -> Self {
+        self.account_management_uri = Some(uri);
+        self
+    }
 }
 
 #[async_trait::async_trait]
@@ -147,6 +155,10 @@ impl OidcBackend for MockImpl {
             response_types_supported: Some(vec![]),
             subject_types_supported: Some(vec![]),
             id_token_signing_alg_values_supported: Some(vec![]),
+            account_management_uri: self
+                .account_management_uri
+                .as_ref()
+                .map(|uri| Url::parse(uri).unwrap()),
             ..Default::default()
         }
         .validate(issuer)

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

@@ -630,7 +630,14 @@ impl Oidc {
         action: Option<AccountManagementActionFull>,
     ) -> Result<Option<Url>, OidcError> {
         let provider_metadata = self.provider_metadata().await?;
+        self.management_url_from_provider_metadata(provider_metadata, action)
+    }
 
+    fn management_url_from_provider_metadata(
+        &self,
+        provider_metadata: VerifiedProviderMetadata,
+        action: Option<AccountManagementActionFull>,
+    ) -> Result<Option<Url>, OidcError> {
         let Some(base_url) = provider_metadata.account_management_uri.clone() else {
             return Ok(None);
         };
@@ -643,6 +650,43 @@ impl Oidc {
         Ok(Some(url))
     }
 
+    /// 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 [`VerifiedProviderMetadata`], or
+    ///   directly with [`Oidc::account_management_actions_supported()`].
+    ///
+    /// Returns `Ok(None)` if the URL was not found. Returns an error if the
+    /// request to get the provider 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
+    /// [`Oidc::fetch_account_management_url()`] and cache the resulting URL
+    /// before returning it.
+    pub async fn account_management_url(
+        &self,
+        action: Option<AccountManagementActionFull>,
+    ) -> Result<Option<Url>, OidcError> {
+        const CACHE_KEY: &str = "PROVIDER_METADATA";
+
+        let mut cache = self.client.inner.caches.provider_metadata.lock().await;
+
+        let metadata = if let Some(metadata) = cache.get("PROVIDER_METADATA") {
+            metadata
+        } else {
+            let provider_metadata = self.provider_metadata().await?;
+            cache.insert(CACHE_KEY.to_owned(), provider_metadata.clone());
+            provider_metadata
+        };
+
+        self.management_url_from_provider_metadata(metadata, action)
+    }
+
     /// Fetch the OpenID Connect metadata of the given issuer.
     ///
     /// Returns an error if fetching the metadata failed.

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

@@ -3,7 +3,10 @@ use std::{collections::HashMap, sync::Arc};
 use anyhow::Context as _;
 use assert_matches::assert_matches;
 use mas_oidc_client::{
-    requests::authorization_code::AuthorizationValidationData,
+    requests::{
+        account_management::AccountManagementActionFull,
+        authorization_code::AuthorizationValidationData,
+    },
     types::{
         errors::ClientErrorCode,
         iana::oauth::OAuthClientAuthenticationMethod,
@@ -29,7 +32,10 @@ use super::{
     AuthorizationCode, AuthorizationError, AuthorizationResponse, Oidc, OidcError, OidcSession,
     OidcSessionTokens, RedirectUriQueryParseError, UserSession,
 };
-use crate::{test_utils::test_client_builder, Client, Error};
+use crate::{
+    test_utils::{client::MockClientBuilder, test_client_builder},
+    Client, Error,
+};
 
 const REDIRECT_URI_STRING: &str = "http://matrix.example.com/oidc/callback";
 
@@ -474,3 +480,36 @@ async fn test_register_client() {
     assert_eq!(auth_data.client_id.0, response.client_id);
     assert_eq!(auth_data.metadata, client_metadata);
 }
+
+#[async_test]
+async fn test_management_url_cache() {
+    let client = MockClientBuilder::new("http://localhost".to_owned()).unlogged().build().await;
+    let backend = Arc::new(
+        MockImpl::new().mark_insecure().account_management_uri("http://localhost".to_owned()),
+    );
+    let oidc = Oidc { client: client.clone(), backend: backend.clone() };
+
+    let tokens = OidcSessionTokens {
+        access_token: "4cc3ss".to_owned(),
+        refresh_token: Some("r3fr3sh".to_owned()),
+        latest_id_token: None,
+    };
+
+    let session = mock_session(tokens.clone());
+    oidc.restore_session(session.clone())
+        .await
+        .expect("We should be able to restore an OIDC session");
+
+    // The cache should not contain the entry.
+    assert!(!client.inner.caches.provider_metadata.lock().await.contains("PROVIDER_METADATA"));
+
+    let management_url = oidc
+        .account_management_url(Some(AccountManagementActionFull::Profile))
+        .await
+        .expect("We should be able to fetch the account management url");
+
+    assert!(management_url.is_some());
+
+    // Check that the provider metadata has been inserted into the cache.
+    assert!(client.inner.caches.provider_metadata.lock().await.contains("PROVIDER_METADATA"));
+}

crates/matrix-sdk/src/client/caches.rs

@@ -12,14 +12,20 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#[cfg(feature = "experimental-oidc")]
+use mas_oidc_client::types::oidc::VerifiedProviderMetadata;
+#[cfg(feature = "experimental-oidc")]
+use matrix_sdk_base::ttl_cache::TtlCache;
 use tokio::sync::RwLock;
 
 use super::ClientServerCapabilities;
 
 /// A collection of in-memory data that the `Client` might want to cache to
 /// avoid hitting the homeserver every time users request the data.
-pub(super) struct ClientCaches {
+pub(crate) struct ClientCaches {
     /// Server capabilities, either prefilled during building or fetched from
     /// the server.
     pub(super) server_capabilities: RwLock<ClientServerCapabilities>,
+    #[cfg(feature = "experimental-oidc")]
+    pub(crate) provider_metadata: tokio::sync::Mutex<TtlCache<String, VerifiedProviderMetadata>>,
 }

crates/matrix-sdk/src/client/mod.rs

@@ -36,6 +36,8 @@ use matrix_sdk_base::{
     BaseClient, RoomInfoNotableUpdate, RoomState, RoomStateFilter, SendOutsideWasm, SessionMeta,
     StateStoreDataKey, StateStoreDataValue, SyncOutsideWasm,
 };
+#[cfg(feature = "experimental-oidc")]
+use matrix_sdk_common::ttl_cache::TtlCache;
 #[cfg(feature = "e2e-encryption")]
 use ruma::events::{room::encryption::RoomEncryptionEventContent, InitialStateEvent};
 use ruma::{
@@ -352,7 +354,11 @@ impl ClientInner {
         #[cfg(feature = "e2e-encryption")] encryption_settings: EncryptionSettings,
         cross_process_store_locks_holder_name: String,
     ) -> Arc<Self> {
-        let caches = ClientCaches { server_capabilities: server_capabilities.into() };
+        let caches = ClientCaches {
+            server_capabilities: server_capabilities.into(),
+            #[cfg(feature = "experimental-oidc")]
+            provider_metadata: Mutex::new(TtlCache::new()),
+        };
 
         let client = Self {
             server,