refactor(oidc): Don't take the client metadata as an argument of url_for_oidc

The OidcRegistrations already hold the metadata. We can just clone it lazily when we need it.

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

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

@@ -425,17 +425,10 @@ impl Client {
                 Some((issuer, ClientId::new(client_id.clone())))
             })
             .collect::<HashMap<_, _>>();
-        let registrations = OidcRegistrations::new(
-            registrations_file,
-            oidc_metadata.clone(),
-            static_registrations,
-        )?;
+        let registrations =
+            OidcRegistrations::new(registrations_file, oidc_metadata, static_registrations)?;
 
-        let data = self
-            .inner
-            .oidc()
-            .url_for_oidc(oidc_metadata, registrations, prompt.map(Into::into))
-            .await?;
+        let data = self.inner.oidc().url_for_oidc(registrations, prompt.map(Into::into)).await?;
 
         Ok(Arc::new(data))
     }

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

@@ -441,9 +441,6 @@ impl Oidc {
     ///
     /// # Arguments
     ///
-    /// * `client_metadata` - The [`VerifiedClientMetadata`] to register, if
-    ///   needed.
-    ///
     /// * `registrations` - The storage where the registered client ID will be
     ///   loaded from, if the client is already registered, or stored into, if
     ///   the client is not registered yet.
@@ -454,18 +451,20 @@ impl Oidc {
     ///   account.
     pub async fn url_for_oidc(
         &self,
-        client_metadata: VerifiedClientMetadata,
         registrations: OidcRegistrations,
         prompt: Option<Prompt>,
     ) -> Result<OidcAuthorizationData, OidcError> {
         let metadata = self.provider_metadata().await?;
 
-        let redirect_uris =
-            client_metadata.redirect_uris.clone().ok_or(OidcError::MissingRedirectUri)?;
+        let redirect_uris = registrations
+            .verified_metadata
+            .redirect_uris
+            .clone()
+            .ok_or(OidcError::MissingRedirectUri)?;
 
         let redirect_url = redirect_uris.first().ok_or(OidcError::MissingRedirectUri)?;
 
-        self.configure(metadata.issuer().to_owned(), client_metadata, registrations).await?;
+        self.configure(metadata.issuer().to_owned(), registrations).await?;
 
         let mut data_builder = self.login(redirect_url.clone(), None)?;
 
@@ -515,7 +514,6 @@ impl Oidc {
     async fn configure(
         &self,
         issuer: String,
-        client_metadata: VerifiedClientMetadata,
         registrations: OidcRegistrations,
     ) -> std::result::Result<(), OidcError> {
         if self.client_id().is_some() {
@@ -529,7 +527,7 @@ impl Oidc {
         }
 
         tracing::info!("Registering this client for OIDC.");
-        self.register_client(client_metadata, None).await?;
+        self.register_client(registrations.verified_metadata.clone(), None).await?;
 
         tracing::info!("Persisting OIDC registration data.");
         self.store_client_registration(&registrations)

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

@@ -53,7 +53,7 @@ pub struct OidcRegistrations {
     file_path: PathBuf,
     /// The hash for the metadata used to register the client.
     /// This is used to check if the client needs to be re-registered.
-    verified_metadata: VerifiedClientMetadata,
+    pub(super) verified_metadata: VerifiedClientMetadata,
     /// Pre-configured registrations for use with issuers that don't support
     /// dynamic client registration.
     static_registrations: HashMap<Url, ClientId>,

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

@@ -162,8 +162,7 @@ async fn test_high_level_login() -> anyhow::Result<()> {
     assert!(oidc.client_id().is_none());
 
     // When getting the OIDC login URL.
-    let authorization_data =
-        oidc.url_for_oidc(metadata.clone(), registrations, Some(Prompt::Create)).await.unwrap();
+    let authorization_data = oidc.url_for_oidc(registrations, Some(Prompt::Create)).await.unwrap();
 
     // Then the client should be configured correctly.
     assert_let!(Some(issuer) = oidc.issuer());
@@ -185,8 +184,7 @@ async fn test_high_level_login() -> anyhow::Result<()> {
 async fn test_high_level_login_cancellation() -> anyhow::Result<()> {
     // Given a client ready to complete login.
     let (oidc, _server, metadata, registrations) = mock_environment().await.unwrap();
-    let authorization_data =
-        oidc.url_for_oidc(metadata.clone(), registrations, None).await.unwrap();
+    let authorization_data = oidc.url_for_oidc(registrations, None).await.unwrap();
 
     assert_let!(Some(issuer) = oidc.issuer());
     assert!(oidc.client_id().is_some());
@@ -215,8 +213,7 @@ async fn test_high_level_login_cancellation() -> anyhow::Result<()> {
 async fn test_high_level_login_invalid_state() -> anyhow::Result<()> {
     // Given a client ready to complete login.
     let (oidc, _server, metadata, registrations) = mock_environment().await.unwrap();
-    let authorization_data =
-        oidc.url_for_oidc(metadata.clone(), registrations, None).await.unwrap();
+    let authorization_data = oidc.url_for_oidc(registrations, None).await.unwrap();
 
     assert_let!(Some(issuer) = oidc.issuer());
     assert!(oidc.client_id().is_some());