Skip to content

Commit

Permalink
feat: Add a memoized variant of Oidc::fetch_account_management_url
Browse files Browse the repository at this point in the history
  • Loading branch information
poljar committed Feb 13, 2025
1 parent aa9aef4 commit 861078a
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 5 deletions.
2 changes: 1 addition & 1 deletion bindings/matrix-sdk-ffi/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
Expand Down
7 changes: 7 additions & 0 deletions crates/matrix-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
12 changes: 12 additions & 0 deletions crates/matrix-sdk/src/authentication/oidc/backend/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,

Expand Down Expand Up @@ -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,
Expand All @@ -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]
Expand Down Expand Up @@ -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)
Expand Down
44 changes: 44 additions & 0 deletions crates/matrix-sdk/src/authentication/oidc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand All @@ -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.
Expand Down
43 changes: 41 additions & 2 deletions crates/matrix-sdk/src/authentication/oidc/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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";

Expand Down Expand Up @@ -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"));
}
8 changes: 7 additions & 1 deletion crates/matrix-sdk/src/client/caches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
}
8 changes: 7 additions & 1 deletion crates/matrix-sdk/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 861078a

Please sign in to comment.