refactor(oauth): Remove the issuer from OAuthAuthData

It is actually unused, and now that we only need homeserver URLs for
static registrations, users don't need to access it easily.

Signed-off-by: Kévin Commaille <[email protected]>

Author: zecakeh

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,
@@ -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)]

crates/matrix-sdk/CHANGELOG.md

@@ -241,6 +241,11 @@ simpler methods:
 - [**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

@@ -102,7 +102,7 @@ impl OAuthAuthCodeUrlBuilder {
 
         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();
 

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

@@ -225,7 +225,6 @@ impl OAuthCtx {
 }
 
 pub(crate) struct OAuthAuthData {
-    pub(crate) issuer: Url,
     pub(crate) client_id: ClientId,
     /// The data necessary to validate authorization responses.
     authorization_data: Mutex<HashMap<CsrfToken, AuthorizationValidationData>>,
@@ -234,9 +233,7 @@ pub(crate) struct OAuthAuthData {
 #[cfg(not(tarpaulin_include))]
 impl fmt::Debug for OAuthAuthData {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_struct("OAuthAuthData")
-            .field("issuer", &self.issuer.as_str())
-            .finish_non_exhaustive()
+        f.debug_struct("OAuthAuthData").finish_non_exhaustive()
     }
 }
 
@@ -457,7 +454,7 @@ impl OAuth {
                 .or_else(|| static_registrations.get(&server_metadata.issuer));
 
             if let Some(client_id) = client_id {
-                self.restore_registered_client(server_metadata.issuer.clone(), client_id.clone());
+                self.restore_registered_client(client_id.clone());
                 return Ok(());
             }
         }
@@ -467,15 +464,6 @@ impl OAuth {
         Ok(())
     }
 
-    /// The OAuth 2.0 authorization server used for authorization.
-    ///
-    /// Returns `None` if the client was not registered or if the registration
-    /// was not restored with [`OAuth::restore_registered_client()`] or
-    /// [`OAuth::restore_session()`].
-    pub fn issuer(&self) -> Option<&Url> {
-        self.data().map(|data| &data.issuer)
-    }
-
     /// The account management actions supported by the authorization server's
     /// account management URL.
     ///
@@ -629,8 +617,7 @@ impl OAuth {
     pub fn user_session(&self) -> Option<UserSession> {
         let meta = self.client.session_meta()?.to_owned();
         let tokens = self.client.session_tokens()?;
-        let issuer = self.data()?.issuer.clone();
-        Some(UserSession { meta, tokens, issuer })
+        Some(UserSession { meta, tokens })
     }
 
     /// The full OAuth 2.0 session of this client.
@@ -655,11 +642,6 @@ impl OAuth {
     ///
     /// * `client_metadata` - The serialized client metadata to register.
     ///
-    /// The client ID in the response should be persisted for future use and
-    /// reused for the same authorization server, identified by the
-    /// [`OAuth::issuer()`], along with the client metadata sent to the server,
-    /// even for different sessions or user accounts.
-    ///
     /// # Panic
     ///
     /// Panics if the authentication data was already set.
@@ -672,7 +654,7 @@ impl OAuth {
     /// # use matrix_sdk::authentication::oauth::registration::ClientMetadata;
     /// # use ruma::serde::Raw;
     /// # let client_metadata = unimplemented!();
-    /// # fn persist_client_registration (_: &url::Url, _: &Raw<ClientMetadata>, _: &ClientId) {}
+    /// # fn persist_client_registration (_: url::Url, _: &ClientId) {}
     /// # _ = async {
     /// let server_name = ServerName::parse("myhomeserver.org")?;
     /// let client = Client::builder().server_name(&server_name).build().await?;
@@ -697,9 +679,8 @@ impl OAuth {
     ///
     /// // The API only supports clients without secrets.
     /// let client_id = response.client_id;
-    /// let issuer = oauth.issuer().expect("issuer should be set after registration");
     ///
-    /// persist_client_registration(issuer, &client_metadata, &client_id);
+    /// persist_client_registration(client.homeserver(), &client_id);
     /// # anyhow::Ok(()) };
     /// ```
     pub async fn register_client(
@@ -725,10 +706,7 @@ impl OAuth {
 
         // The format of the credentials changes according to the client metadata that
         // was sent. Public clients only get a client ID.
-        self.restore_registered_client(
-            server_metadata.issuer.clone(),
-            registration_response.client_id.clone(),
-        );
+        self.restore_registered_client(registration_response.client_id.clone());
 
         Ok(registration_response)
     }
@@ -744,17 +722,14 @@ impl OAuth {
     ///
     /// # Arguments
     ///
-    /// * `issuer` - The authorization server that was used to register the
-    ///   client.
-    ///
     /// * `client_id` - The unique identifier to authenticate the client with
     ///   the server, obtained after registration.
     ///
     /// # Panic
     ///
     /// Panics if authentication data was already set.
-    pub fn restore_registered_client(&self, issuer: Url, client_id: ClientId) {
-        let data = OAuthAuthData { issuer, client_id, authorization_data: Default::default() };
+    pub fn restore_registered_client(&self, client_id: ClientId) {
+        let data = OAuthAuthData { client_id, authorization_data: Default::default() };
 
         self.client
             .auth_ctx()
@@ -783,9 +758,9 @@ impl OAuth {
         session: OAuthSession,
         room_load_settings: RoomLoadSettings,
     ) -> Result<()> {
-        let OAuthSession { client_id, user: UserSession { meta, tokens, issuer } } = session;
+        let OAuthSession { client_id, user: UserSession { meta, tokens } } = session;
 
-        let data = OAuthAuthData { issuer, client_id, authorization_data: Default::default() };
+        let data = OAuthAuthData { client_id, authorization_data: Default::default() };
 
         self.client.auth_ctx().set_session_tokens(tokens.clone());
         self.client
@@ -1434,9 +1409,6 @@ pub struct UserSession {
     /// The tokens used for authentication.
     #[serde(flatten)]
     pub tokens: SessionTokens,
-
-    /// The OAuth 2.0 server used for this session.
-    pub issuer: Url,
 }
 
 /// The data necessary to validate a response from the Token endpoint in the