oidc: Supply a direct path to the registrations file instead of a bas…

…e path.

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<ClientBuildError> for AuthenticationError {
 impl From<OidcRegistrationsError> 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<String, String>,
+
+    /// 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(),
         )

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<dyn std::error::Error>),
@@ -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<Url, ClientId>,
     ) -> Result<Self, OidcRegistrationsError> {
-        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(&registrations_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(
+            &registrations_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(&registrations_file, new_oidc_metadata, static_registrations)
+                .unwrap();
 
         // Then the dynamic registrations are cleared.
         assert_eq!(registrations.client_id(&dynamic_url), None);