diff --git a/bindings/matrix-sdk-ffi/src/authentication_service.rs b/bindings/matrix-sdk-ffi/src/authentication_service.rs index 3e726187db7..9cd727e4d70 100644 --- a/bindings/matrix-sdk-ffi/src/authentication_service.rs +++ b/bindings/matrix-sdk-ffi/src/authentication_service.rs @@ -1,5 +1,6 @@ use std::{ collections::HashMap, + path::Path, sync::{Arc, RwLock as StdRwLock}, }; @@ -75,8 +76,6 @@ pub enum AuthenticationError { #[error("Login was successful but is missing a valid Session to configure the file store.")] SessionMissing, - #[error("Failed to use the supplied base path.")] - InvalidBasePath, #[error( "The homeserver doesn't provide an authentication issuer in its well-known configuration." @@ -86,6 +85,8 @@ pub enum AuthenticationError { OidcMetadataMissing, #[error("Unable to use OIDC as the supplied client metadata is invalid.")] OidcMetadataInvalid, + #[error("Failed to use the supplied registrations file path.")] + OidcRegistrationsPathInvalid, #[error("The supplied callback URL used to complete OIDC is invalid.")] OidcCallbackUrlInvalid, #[error("The OIDC login was cancelled by the user.")] @@ -130,7 +131,9 @@ impl From for AuthenticationError { impl From for AuthenticationError { fn from(e: OidcRegistrationsError) -> AuthenticationError { match e { - OidcRegistrationsError::InvalidBasePath => AuthenticationError::InvalidBasePath, + OidcRegistrationsError::InvalidFilePath => { + AuthenticationError::OidcRegistrationsPathInvalid + } _ => AuthenticationError::OidcError { message: e.to_string() }, } } @@ -164,6 +167,11 @@ pub struct OidcConfiguration { /// Pre-configured registrations for use with issuers that don't support /// dynamic client registration. pub static_registrations: HashMap, + + /// A file path where any dynamic registrations should be stored. + /// + /// Suggested value: `{base_path}/oidc/registrations.json` + pub dynamic_registrations_file: String, } /// The data required to authenticate against an OIDC server. @@ -455,8 +463,14 @@ impl AuthenticationService { }; let oidc_metadata: VerifiedClientMetadata = configuration.try_into()?; + let registrations_file = Path::new(&configuration.dynamic_registrations_file); - if self.load_client_registration(oidc, issuer.clone(), oidc_metadata.clone()) { + if self.load_client_registration( + oidc, + issuer.clone(), + oidc_metadata.clone(), + registrations_file, + ) { tracing::info!("OIDC configuration loaded from disk."); return Ok(()); } @@ -472,14 +486,18 @@ impl AuthenticationService { oidc.restore_registered_client(issuer, oidc_metadata, credentials); tracing::info!("Persisting OIDC registration data."); - self.store_client_registration(oidc)?; + self.store_client_registration(oidc, registrations_file)?; Ok(()) } /// Stores the current OIDC dynamic client registration so it can be re-used /// if we ever log in via the same issuer again. - fn store_client_registration(&self, oidc: &Oidc) -> Result<(), AuthenticationError> { + fn store_client_registration( + &self, + oidc: &Oidc, + registrations_file: &Path, + ) -> Result<(), AuthenticationError> { let issuer = Url::parse(oidc.issuer().ok_or(AuthenticationError::OidcNotSupported)?) .map_err(|_| AuthenticationError::OidcError { message: String::from("Failed to parse issuer URL."), @@ -497,7 +515,7 @@ impl AuthenticationService { })?; let registrations = OidcRegistrations::new( - &self.base_path, + registrations_file, metadata.clone(), self.oidc_static_registrations(), )?; @@ -513,13 +531,14 @@ impl AuthenticationService { oidc: &Oidc, issuer: String, oidc_metadata: VerifiedClientMetadata, + registrations_file: &Path, ) -> bool { let Ok(issuer_url) = Url::parse(&issuer) else { tracing::error!("Failed to parse {issuer:?}"); return false; }; let Some(registrations) = OidcRegistrations::new( - &self.base_path, + registrations_file, oidc_metadata.clone(), self.oidc_static_registrations(), ) diff --git a/crates/matrix-sdk/src/oidc/registrations.rs b/crates/matrix-sdk/src/oidc/registrations.rs index 57491611e0c..5d3bb81def6 100644 --- a/crates/matrix-sdk/src/oidc/registrations.rs +++ b/crates/matrix-sdk/src/oidc/registrations.rs @@ -25,7 +25,7 @@ use std::{ fs, fs::File, io::{BufReader, BufWriter}, - path::PathBuf, + path::{Path, PathBuf}, }; use mas_oidc_client::types::registration::{ @@ -37,9 +37,9 @@ use url::Url; /// Errors related to persisting OIDC registrations. #[derive(Debug, thiserror::Error)] pub enum OidcRegistrationsError { - /// The supplied base path is invalid. - #[error("Failed to use the supplied base path.")] - InvalidBasePath, + /// The supplied registrations file path is invalid. + #[error("Failed to use the supplied registrations file path.")] + InvalidFilePath, /// An error occurred whilst saving the registration data. #[error("Failed to save the registration data {0}.")] SaveFailure(#[source] Box), @@ -107,9 +107,9 @@ impl OidcRegistrations { /// /// # Arguments /// - /// * `base_path` - A directory where the registrations file can be stored. - /// It will be nested inside of a directory called `oidc` as - /// `registrations.json`. + /// * `registrations_file` - A file path where the registrations will be + /// stored. This previously took a directory and stored the registrations + /// with the path `supplied_directory/oidc/registrations.json`. /// /// * `metadata` - The metadata used to register the client. If this /// changes, any stored registrations will be lost so the client can @@ -118,15 +118,15 @@ impl OidcRegistrations { /// * `static_registrations` - Pre-configured registrations for use with /// issuers that don't support dynamic client registration. pub fn new( - base_path: &str, + registrations_file: &Path, metadata: VerifiedClientMetadata, static_registrations: HashMap, ) -> Result { - let oidc_directory = PathBuf::from(base_path).join("oidc"); - fs::create_dir_all(&oidc_directory).map_err(|_| OidcRegistrationsError::InvalidBasePath)?; + let parent = registrations_file.parent().ok_or(OidcRegistrationsError::InvalidFilePath)?; + fs::create_dir_all(parent).map_err(|_| OidcRegistrationsError::InvalidFilePath)?; Ok(OidcRegistrations { - file_path: oidc_directory.join("registrations.json"), + file_path: registrations_file.to_owned(), verified_metadata: metadata, static_registrations, }) @@ -211,7 +211,7 @@ mod tests { fn test_oidc_registrations() { // Given a fresh registration store with a single static registration. let dir = tempdir().unwrap(); - let base_path = dir.path().to_str().unwrap(); + let registrations_file = dir.path().join("oidc").join("registrations.json"); let static_url = Url::parse("https://example.com").unwrap(); let static_id = ClientId("static_client_id".to_owned()); @@ -224,7 +224,8 @@ mod tests { let oidc_metadata = mock_metadata("Example".to_owned()); let registrations = - OidcRegistrations::new(base_path, oidc_metadata, static_registrations).unwrap(); + OidcRegistrations::new(®istrations_file, oidc_metadata, static_registrations) + .unwrap(); assert_eq!(registrations.client_id(&static_url), Some(static_id.clone())); assert_eq!(registrations.client_id(&dynamic_url), None); @@ -242,7 +243,7 @@ mod tests { fn test_change_of_metadata() { // Given a single registration with an example app name. let dir = tempdir().unwrap(); - let base_path = dir.path().to_str().unwrap(); + let registrations_file = dir.path().join("oidc").join("registrations.json"); let static_url = Url::parse("https://example.com").unwrap(); let static_id = ClientId("static_client_id".to_owned()); @@ -254,8 +255,12 @@ mod tests { let mut static_registrations = HashMap::new(); static_registrations.insert(static_url.clone(), static_id.clone()); - let registrations = - OidcRegistrations::new(base_path, oidc_metadata, static_registrations.clone()).unwrap(); + let registrations = OidcRegistrations::new( + ®istrations_file, + oidc_metadata, + static_registrations.clone(), + ) + .unwrap(); registrations.set_and_write_client_id(dynamic_id.clone(), dynamic_url.clone()).unwrap(); assert_eq!(registrations.client_id(&static_url), Some(static_id.clone())); @@ -265,7 +270,8 @@ mod tests { let new_oidc_metadata = mock_metadata("New App".to_owned()); let registrations = - OidcRegistrations::new(base_path, new_oidc_metadata, static_registrations).unwrap(); + OidcRegistrations::new(®istrations_file, new_oidc_metadata, static_registrations) + .unwrap(); // Then the dynamic registrations are cleared. assert_eq!(registrations.client_id(&dynamic_url), None);