refactor(oauth): Remove OAuthRegistrationStore and issuer

bindings/matrix-sdk-ffi/CHANGELOG.md

@@ -70,6 +70,9 @@ Breaking changes:
   event ID (and thread reply behaviour) inside a `ReplyParameters` struct.
   ([#4880](https://github.com/matrix-org/matrix-rust-sdk/pull/4880/))
 
+- The `dynamic_registrations_file` field of `OidcConfiguration` was removed.
+  Clients are supposed to re-register with the homeserver for every login.
+
 Additions:
 
 - Add `Encryption::get_user_identity` which returns `UserIdentity`

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

@@ -1,15 +1,14 @@
 use std::{
     collections::HashMap,
     fmt::{self, Debug},
-    path::PathBuf,
     sync::Arc,
 };
 
 use matrix_sdk::{
     authentication::oauth::{
-        error::{OAuthAuthorizationCodeError, OAuthRegistrationStoreError},
+        error::OAuthAuthorizationCodeError,
         registration::{ApplicationType, ClientMetadata, Localized, OAuthGrantType},
-        ClientId, OAuthError as SdkOAuthError, OAuthRegistrationStore,
+        ClientId, ClientRegistrationData, OAuthError as SdkOAuthError,
     },
     Error,
 };
@@ -123,14 +122,12 @@ pub struct OidcConfiguration {
     /// An array of e-mail addresses of people responsible for this client.
     pub contacts: Option<Vec<String>>,
 
-    /// Pre-configured registrations for use with issuers that don't support
+    /// Pre-configured registrations for use with homeservers 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 keys of the map should be the URLs of the homeservers, but keys
+    /// using `issuer` URLs are also supported.
+    pub static_registrations: HashMap<String, String>,
 }
 
 impl OidcConfiguration {
@@ -165,12 +162,10 @@ impl OidcConfiguration {
         Raw::new(&metadata).map_err(|_| OidcError::MetadataInvalid)
     }
 
-    pub async fn registrations(&self) -> Result<OAuthRegistrationStore, OidcError> {
+    pub(crate) fn registration_data(&self) -> Result<ClientRegistrationData, OidcError> {
         let client_metadata = self.client_metadata()?;
 
-        let registrations_file = PathBuf::from(&self.dynamic_registrations_file);
-        let mut registrations =
-            OAuthRegistrationStore::new(registrations_file, client_metadata).await?;
+        let mut registration_data = ClientRegistrationData::new(client_metadata);
 
         if !self.static_registrations.is_empty() {
             let static_registrations = self
@@ -185,10 +180,10 @@ impl OidcConfiguration {
                 })
                 .collect();
 
-            registrations = registrations.with_static_registrations(static_registrations);
+            registration_data.static_registrations = Some(static_registrations);
         }
 
-        Ok(registrations)
+        Ok(registration_data)
     }
 }
 
@@ -201,8 +196,6 @@ pub enum OidcError {
     NotSupported,
     #[error("Unable to use OIDC as the supplied client metadata is invalid.")]
     MetadataInvalid,
-    #[error("Failed to use the supplied registrations file path.")]
-    RegistrationsPathInvalid,
     #[error("The supplied callback URL used to complete OIDC is invalid.")]
     CallbackUrlInvalid,
     #[error("The OIDC login was cancelled by the user.")]
@@ -228,17 +221,6 @@ impl From<SdkOAuthError> for OidcError {
     }
 }
 
-impl From<OAuthRegistrationStoreError> for OidcError {
-    fn from(e: OAuthRegistrationStoreError) -> OidcError {
-        match e {
-            OAuthRegistrationStoreError::NotAFilePath | OAuthRegistrationStoreError::File(_) => {
-                OidcError::RegistrationsPathInvalid
-            }
-            _ => OidcError::Generic { message: e.to_string() },
-        }
-    }
-}
-
 impl From<Error> for OidcError {
     fn from(e: Error) -> OidcError {
         match e {

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

@@ -43,10 +43,7 @@ use matrix_sdk_ui::notification_client::{
 };
 use mime::Mime;
 use ruma::{
-    api::client::{
-        alias::get_alias, discovery::discover_homeserver::AuthenticationServerInfo,
-        error::ErrorKind, uiaa::UserIdentifier,
-    },
+    api::client::{alias::get_alias, error::ErrorKind, uiaa::UserIdentifier},
     events::{
         ignored_user_list::IgnoredUserListEventContent,
         key::verification::request::ToDeviceKeyVerificationRequestEvent,
@@ -408,10 +405,10 @@ impl Client {
         oidc_configuration: &OidcConfiguration,
         prompt: Option<OidcPrompt>,
     ) -> Result<Arc<OAuthAuthorizationData>, OidcError> {
-        let registrations = oidc_configuration.registrations().await?;
+        let registration_data = oidc_configuration.registration_data()?;
         let redirect_uri = oidc_configuration.redirect_uri()?;
 
-        let mut url_builder = self.inner.oauth().login(registrations.into(), redirect_uri, None);
+        let mut url_builder = self.inner.oauth().login(redirect_uri, None, Some(registration_data));
 
         if let Some(prompt) = prompt {
             url_builder = url_builder.prompt(vec![prompt.into()]);
@@ -1659,10 +1656,9 @@ impl Session {
                 let matrix_sdk::authentication::oauth::UserSession {
                     meta: matrix_sdk::SessionMeta { user_id, device_id },
                     tokens: matrix_sdk::SessionTokens { access_token, refresh_token },
-                    issuer,
                 } = api.user_session().context("Missing session")?;
                 let client_id = api.client_id().context("OIDC client ID is missing.")?.clone();
-                let oidc_data = OidcSessionData { client_id, issuer };
+                let oidc_data = OidcSessionData { client_id };
 
                 let oidc_data = serde_json::to_string(&oidc_data).ok();
                 Ok(Session {
@@ -1707,7 +1703,6 @@ impl TryFrom<Session> for AuthSession {
                     device_id: device_id.into(),
                 },
                 tokens: matrix_sdk::SessionTokens { access_token, refresh_token },
-                issuer: oidc_data.issuer,
             };
 
             let session = OAuthSession { client_id: oidc_data.client_id, user: user_session };
@@ -1731,31 +1726,8 @@ impl TryFrom<Session> for AuthSession {
 /// Represents a client registration against an OpenID Connect authentication
 /// issuer.
 #[derive(Serialize, Deserialize)]
-#[serde(try_from = "OidcSessionDataDeHelper")]
 pub(crate) struct OidcSessionData {
     client_id: ClientId,
-    issuer: Url,
-}
-
-#[derive(Deserialize)]
-struct OidcSessionDataDeHelper {
-    client_id: ClientId,
-    issuer_info: Option<AuthenticationServerInfo>,
-    issuer: Option<Url>,
-}
-
-impl TryFrom<OidcSessionDataDeHelper> for OidcSessionData {
-    type Error = String;
-
-    fn try_from(value: OidcSessionDataDeHelper) -> Result<Self, Self::Error> {
-        let OidcSessionDataDeHelper { client_id, issuer_info, issuer } = value;
-
-        let issuer = issuer
-            .or(issuer_info.and_then(|info| Url::parse(&info.issuer).ok()))
-            .ok_or_else(|| "missing field `issuer`".to_owned())?;
-
-        Ok(Self { client_id, issuer })
-    }
 }
 
 #[derive(uniffi::Enum)]

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

@@ -755,13 +755,12 @@ impl ClientBuilder {
             }
         })?;
 
-        let registrations = oidc_configuration
-            .registrations()
-            .await
+        let registration_data = oidc_configuration
+            .registration_data()
             .map_err(|_| HumanQrLoginError::OidcMetadataInvalid)?;
 
         let oauth = client.inner.oauth();
-        let login = oauth.login_with_qr_code(&qr_code_data.inner, registrations.into());
+        let login = oauth.login_with_qr_code(&qr_code_data.inner, Some(&registration_data));
 
         let mut progress = login.subscribe_to_progress();
 

crates/matrix-sdk/CHANGELOG.md

@@ -215,12 +215,12 @@ simpler methods:
 - [**breaking**] Allow to use any registration method with `OAuth::login()` and
   `OAuth::login_with_qr_code()`.
   ([#4827](https://github.com/matrix-org/matrix-rust-sdk/pull/4827))
-  - `OAuth::login` takes an `ClientRegistrationMethod` to be able to register
-    and login with a single function call.
+  - `OAuth::login` takes an optional `ClientRegistrationData` to be able to
+    register and login with a single function call.
   - `OAuth::url_for_oidc()` was removed, it can be replaced by a call to
     `OAuth::login()`.
-  - `OAuth::login_with_qr_code()` takes a `ClientRegistrationMethod` instead of
-    the client metadata.
+  - `OAuth::login_with_qr_code()` takes an optional `ClientRegistrationData`
+    instead of the client metadata.
   - `OAuth::finish_login` takes a `UrlOrQuery` instead of an
     `AuthorizationCode`. The deserialization of the query string will occur
     inside the method and eventual errors will be handled.
@@ -238,6 +238,14 @@ simpler methods:
 - [**breaking**] The parameters `event_id` and `enforce_thread` on [`Room::make_reply_event()`]
   have been wrapped in a `reply` struct parameter.
   ([#4880](https://github.com/matrix-org/matrix-rust-sdk/pull/4880/))
+- [**breaking**] `OidcRegistrations` was removed. Clients are supposed to
+  re-register with the homeserver for every login.
+  ([#4879](https://github.com/matrix-org/matrix-rust-sdk/pull/4879))
+- [**breaking**] The `OAuth::restore_registered_client()` doesn't take an
+  `issuer` anymore.
+  ([#4879](https://github.com/matrix-org/matrix-rust-sdk/pull/4879))
+  - `OAuth::issuer()` was removed.
+  - The `issuer` field of `UserSession` was removed.
 
 ## [0.10.0] - 2025-02-04
 

crates/matrix-sdk/src/authentication/oauth/auth_code_builder.rs

@@ -24,7 +24,7 @@ use ruma::{
 use tracing::{info, instrument};
 use url::Url;
 
-use super::{ClientRegistrationMethod, OAuth, OAuthError};
+use super::{ClientRegistrationData, OAuth, OAuthError};
 use crate::{authentication::oauth::AuthorizationValidationData, Result};
 
 /// Builder type used to configure optional settings for authorization with an
@@ -34,7 +34,7 @@ use crate::{authentication::oauth::AuthorizationValidationData, Result};
 #[allow(missing_debug_implementations)]
 pub struct OAuthAuthCodeUrlBuilder {
     oauth: OAuth,
-    registration_method: ClientRegistrationMethod,
+    registration_data: Option<ClientRegistrationData>,
     scopes: Vec<Scope>,
     device_id: OwnedDeviceId,
     redirect_uri: Url,
@@ -45,14 +45,14 @@ pub struct OAuthAuthCodeUrlBuilder {
 impl OAuthAuthCodeUrlBuilder {
     pub(super) fn new(
         oauth: OAuth,
-        registration_method: ClientRegistrationMethod,
         scopes: Vec<Scope>,
         device_id: OwnedDeviceId,
         redirect_uri: Url,
+        registration_data: Option<ClientRegistrationData>,
     ) -> Self {
         Self {
             oauth,
-            registration_method,
+            registration_data,
             scopes,
             device_id,
             redirect_uri,
@@ -93,23 +93,16 @@ impl OAuthAuthCodeUrlBuilder {
     /// request fails.
     #[instrument(target = "matrix_sdk::client", skip_all)]
     pub async fn build(self) -> Result<OAuthAuthorizationData, OAuthError> {
-        let Self {
-            oauth,
-            registration_method,
-            scopes,
-            device_id,
-            redirect_uri,
-            prompt,
-            login_hint,
-        } = self;
+        let Self { oauth, registration_data, scopes, device_id, redirect_uri, prompt, login_hint } =
+            self;
 
         let server_metadata = oauth.server_metadata().await?;
 
-        oauth.use_registration_method(&server_metadata, &registration_method).await?;
+        oauth.use_registration_data(&server_metadata, registration_data.as_ref()).await?;
 
         let data = oauth.data().expect("OAuth 2.0 data should be set after registration");
         info!(
-            issuer = data.issuer.as_str(),
+            issuer = server_metadata.issuer.as_str(),
             ?scopes,
             "Authorizing scope via the OAuth 2.0 Authorization Code flow"
         );

crates/matrix-sdk/src/authentication/oauth/cross_process.rs

@@ -293,10 +293,7 @@ mod tests {
         let session_hash = compute_session_hash(&tokens);
         client
             .oauth()
-            .restore_session(
-                mock_session(tokens.clone(), "https://oauth.example.com/issuer"),
-                RoomLoadSettings::default(),
-            )
+            .restore_session(mock_session(tokens.clone()), RoomLoadSettings::default())
             .await?;
 
         assert_eq!(client.session_tokens().unwrap(), tokens);
@@ -324,12 +321,8 @@ mod tests {
         server.mock_who_am_i().ok().expect(1).named("whoami").mount().await;
 
         let tmp_dir = tempfile::tempdir()?;
-        let client = server
-            .client_builder()
-            .sqlite_store(&tmp_dir)
-            .registered_with_oauth(server.server().uri())
-            .build()
-            .await;
+        let client =
+            server.client_builder().sqlite_store(&tmp_dir).registered_with_oauth().build().await;
         let oauth = client.oauth();
 
         // Enable cross-process lock.
@@ -389,7 +382,7 @@ mod tests {
         // Restore the session.
         oauth
             .restore_session(
-                mock_session(mock_prev_session_tokens_with_refresh(), server.server().uri()),
+                mock_session(mock_prev_session_tokens_with_refresh()),
                 RoomLoadSettings::default(),
             )
             .await?;
@@ -417,7 +410,6 @@ mod tests {
     #[async_test]
     async fn test_cross_process_concurrent_refresh() -> anyhow::Result<()> {
         let server = MatrixMockServer::new().await;
-        let issuer = server.server().uri();
 
         let oauth_server = server.oauth();
         oauth_server.mock_server_metadata().ok().expect(1..).named("server_metadata").mount().await;
@@ -434,10 +426,7 @@ mod tests {
         oauth.enable_cross_process_refresh_lock("client1".to_owned()).await?;
 
         oauth
-            .restore_session(
-                mock_session(prev_tokens.clone(), issuer.clone()),
-                RoomLoadSettings::default(),
-            )
+            .restore_session(mock_session(prev_tokens.clone()), RoomLoadSettings::default())
             .await?;
 
         // Create a second client, without restoring it, to test that a token update
@@ -455,10 +444,7 @@ mod tests {
             let oauth3 = client3.oauth();
             oauth3.enable_cross_process_refresh_lock("client3".to_owned()).await?;
             oauth3
-                .restore_session(
-                    mock_session(prev_tokens.clone(), issuer.clone()),
-                    RoomLoadSettings::default(),
-                )
+                .restore_session(mock_session(prev_tokens.clone()), RoomLoadSettings::default())
                 .await?;
 
             // Run a refresh in the second client; this will invalidate the tokens from the
@@ -492,10 +478,7 @@ mod tests {
             )?;
 
             oauth
-                .restore_session(
-                    mock_session(prev_tokens.clone(), issuer),
-                    RoomLoadSettings::default(),
-                )
+                .restore_session(mock_session(prev_tokens.clone()), RoomLoadSettings::default())
                 .await?;
 
             // And this client is now aware of the latest tokens.
@@ -572,12 +555,7 @@ mod tests {
 
         // Restore the session.
         let tokens = mock_session_tokens_with_refresh();
-        oauth
-            .restore_session(
-                mock_session(tokens.clone(), server.server().uri()),
-                RoomLoadSettings::default(),
-            )
-            .await?;
+        oauth.restore_session(mock_session(tokens.clone()), RoomLoadSettings::default()).await?;
 
         oauth.logout().await.unwrap();