From b4b0f3a20394161b217dd47bb18fed5e265b573b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= <76261501+zecakeh@users.noreply.github.com> Date: Fri, 27 Jun 2025 19:24:30 +0200 Subject: [PATCH] refactor(sdk): Remove fallback support for the `/auth_issuer` endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `/auth_metadata` endpoint has been supported by Synapse for 6 months now so there shouldn't be any homeserver that still depend exclusively on it. This endpoint is also part of Matrix 1.15. Support for this endpoint has been removed from Ruma so this is necessary before an upgrade of the dependency. Signed-off-by: Kévin Commaille --- crates/matrix-sdk/CHANGELOG.md | 9 ++- .../src/authentication/oauth/mod.rs | 61 ++++-------------- .../authentication/oauth/oidc_discovery.rs | 64 ------------------- .../src/authentication/oauth/tests.rs | 26 +------- 4 files changed, 20 insertions(+), 140 deletions(-) delete mode 100644 crates/matrix-sdk/src/authentication/oauth/oidc_discovery.rs diff --git a/crates/matrix-sdk/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index 3520408ef..587e3298e 100644 --- a/crates/matrix-sdk/CHANGELOG.md +++ b/crates/matrix-sdk/CHANGELOG.md @@ -37,7 +37,7 @@ All notable changes to this project will be documented in this file. ### Bug fixes -- `m.room.avatar` has been added as required state for sliding sync until [the existing backend issue](https://github.com/element-hq/synapse/issues/18598) +- `m.room.avatar` has been added as required state for sliding sync until [the existing backend issue](https://github.com/element-hq/synapse/issues/18598) causing deleted room avatars to not be flagged is fixed. ([#5293](https://github.com/matrix-org/matrix-rust-sdk/pull/5293)) ## [0.12.0] - 2025-06-10 @@ -78,7 +78,7 @@ causing deleted room avatars to not be flagged is fixed. ([#5293](https://github - `Room::send_single_receipt()` and `Room::send_multiple_receipts()` now also unset the unread flag of the room if an unthreaded read receipt is sent. ([#5055](https://github.com/matrix-org/matrix-rust-sdk/pull/5055)) -- `Client::is_user_ignored(&UserId)` can be used to check if a user is currently ignored. +- `Client::is_user_ignored(&UserId)` can be used to check if a user is currently ignored. ([#5081](https://github.com/matrix-org/matrix-rust-sdk/pull/5081)) - `RoomSendQueue::send_gallery` has been added to allow sending MSC4274-style media galleries via the send queue under the `unstable-msc4274` feature. @@ -89,12 +89,15 @@ causing deleted room avatars to not be flagged is fixed. ([#5293](https://github - A invited DM room joined with `Client::join_room_by_id()` or `Client::join_room_by_id_or_alias()` will now be correctly marked as a DM. ([#5043](https://github.com/matrix-org/matrix-rust-sdk/pull/5043)) -- API responses with an HTTP status code `520` won't be retried anymore, as this is used by some proxies +- API responses with an HTTP status code `520` won't be retried anymore, as this is used by some proxies (including Cloudflare) to warn that an unknown error has happened in the actual server. ([#5105](https://github.com/matrix-org/matrix-rust-sdk/pull/5105)) ### Refactor +- Support for the deprecated `GET /auth_issuer` endpoint was removed in the `OAuth` API. Only the + `GET /auth_metadata` endpoint is used now. + ([#5302](https://github.com/matrix-org/matrix-rust-sdk/pull/5302)) - `Room::push_context()` has been renamed into `Room::push_condition_room_ctx()`. The newer `Room::push_context` now returns a `matrix_sdk::Room::PushContext`, which can be used to compute the push actions for any event. diff --git a/crates/matrix-sdk/src/authentication/oauth/mod.rs b/crates/matrix-sdk/src/authentication/oauth/mod.rs index ff5323030..6a64dbd4a 100644 --- a/crates/matrix-sdk/src/authentication/oauth/mod.rs +++ b/crates/matrix-sdk/src/authentication/oauth/mod.rs @@ -185,12 +185,9 @@ use oauth2::{ }; pub use oauth2::{ClientId, CsrfToken}; use ruma::{ - api::client::discovery::{ - get_authentication_issuer, - get_authorization_server_metadata::{ - self, - msc2965::{AccountManagementAction, AuthorizationServerMetadata}, - }, + api::client::discovery::get_authorization_server_metadata::{ + self, + msc2965::{AccountManagementAction, AuthorizationServerMetadata}, }, serde::Raw, DeviceId, OwnedDeviceId, @@ -207,7 +204,6 @@ mod auth_code_builder; mod cross_process; pub mod error; mod http_client; -mod oidc_discovery; #[cfg(feature = "e2e-encryption")] pub mod qrcode; pub mod registration; @@ -225,7 +221,6 @@ pub use self::{ }; use self::{ http_client::OAuthHttpClient, - oidc_discovery::discover, registration::{register_client, ClientMetadata, ClientRegistrationResponse}, }; use super::{AuthData, SessionTokens}; @@ -569,33 +564,6 @@ impl OAuth { Ok(metadata.account_management_uri.map(AccountManagementUrlBuilder::new)) } - /// Discover the authentication issuer and retrieve the - /// [`AuthorizationServerMetadata`] using the GET `/auth_issuer` endpoint - /// previously defined in [MSC2965]. - /// - /// **Note**: This endpoint is deprecated. - /// - /// MSC2956: https://github.com/matrix-org/matrix-spec-proposals/pull/2965 - async fn fallback_discover( - &self, - ) -> Result, OAuthDiscoveryError> { - #[allow(deprecated)] - let issuer = - match self.client.send(get_authentication_issuer::msc2965::Request::new()).await { - Ok(response) => response.issuer, - Err(error) - if error - .as_client_api_error() - .is_some_and(|err| err.status_code == http::StatusCode::NOT_FOUND) => - { - return Err(OAuthDiscoveryError::NotSupported); - } - Err(error) => return Err(error.into()), - }; - - discover(self.http_client(), &issuer).await - } - /// Fetch the OAuth 2.0 authorization server metadata of the homeserver. /// /// Returns an error if a problem occurred when fetching or validating the @@ -609,23 +577,20 @@ impl OAuth { .is_some_and(|err| err.status_code == http::StatusCode::NOT_FOUND) }; - let raw_metadata = match self + let response = self .client .send(get_authorization_server_metadata::msc2965::Request::new()) .await - { - Ok(response) => response.metadata, - // If the endpoint returns a 404, i.e. the server doesn't support the endpoint, attempt - // to use the equivalent, but deprecated, endpoint. - Err(error) if is_endpoint_unsupported(&error) => { - // TODO: remove this fallback behavior when the metadata endpoint has wider - // support. - self.fallback_discover().await? - } - Err(error) => return Err(error.into()), - }; + .map_err(|error| { + // If the endpoint returns a 404, i.e. the server doesn't support the endpoint. + if is_endpoint_unsupported(&error) { + OAuthDiscoveryError::NotSupported + } else { + error.into() + } + })?; - let metadata = raw_metadata.deserialize()?; + let metadata = response.metadata.deserialize()?; if self.ctx().insecure_discover { metadata.insecure_validate_urls()?; diff --git a/crates/matrix-sdk/src/authentication/oauth/oidc_discovery.rs b/crates/matrix-sdk/src/authentication/oauth/oidc_discovery.rs deleted file mode 100644 index 6fed1ca85..000000000 --- a/crates/matrix-sdk/src/authentication/oauth/oidc_discovery.rs +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2025 Kévin Commaille -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! Requests for [OpenID Connect Provider Discovery]. -//! -//! [OpenID Connect Provider Discovery]: https://openid.net/specs/openid-connect-discovery-1_0.html - -use oauth2::{AsyncHttpClient, HttpClientError, RequestTokenError}; -use ruma::{ - api::client::discovery::get_authorization_server_metadata::msc2965::AuthorizationServerMetadata, - serde::Raw, -}; -use url::Url; - -use super::{ - error::OAuthDiscoveryError, - http_client::{check_http_response_json_content_type, check_http_response_status_code}, - OAuthHttpClient, -}; - -/// Fetch the OpenID Connect provider metadata. -pub(super) async fn discover( - http_client: &OAuthHttpClient, - issuer: &str, -) -> Result, OAuthDiscoveryError> { - tracing::debug!("Fetching OpenID Connect provider metadata..."); - - let mut url = Url::parse(issuer)?; - - // If the path doesn't end with a slash, the last segment is removed when - // using `join`. - if !url.path().ends_with('/') { - let mut path = url.path().to_owned(); - path.push('/'); - url.set_path(&path); - } - - let config_url = url.join(".well-known/openid-configuration")?; - - let request = http::Request::get(config_url.as_str()) - .body(Vec::new()) - .map_err(|err| RequestTokenError::Request(HttpClientError::Http(err)))?; - let response = http_client.call(request).await.map_err(RequestTokenError::Request)?; - - check_http_response_status_code(&response)?; - check_http_response_json_content_type(&response)?; - - let metadata = serde_json::from_slice(&response.into_body())?; - - tracing::debug!(?metadata); - - Ok(metadata) -} diff --git a/crates/matrix-sdk/src/authentication/oauth/tests.rs b/crates/matrix-sdk/src/authentication/oauth/tests.rs index 74c52b5f5..4e425daa7 100644 --- a/crates/matrix-sdk/src/authentication/oauth/tests.rs +++ b/crates/matrix-sdk/src/authentication/oauth/tests.rs @@ -7,13 +7,8 @@ use ruma::{ api::client::discovery::get_authorization_server_metadata::msc2965::Prompt, device_id, owned_device_id, user_id, DeviceId, ServerName, }; -use serde_json::json; use tokio::sync::broadcast::error::TryRecvError; use url::Url; -use wiremock::{ - matchers::{method, path}, - Mock, ResponseTemplate, -}; use super::{ AuthorizationCode, AuthorizationError, AuthorizationResponse, OAuth, OAuthAuthorizationData, @@ -30,7 +25,7 @@ use crate::{ oauth::{mock_client_id, mock_client_metadata, mock_redirect_uri, mock_session}, MockClientBuilder, }, - mocks::{oauth::MockServerMetadataBuilder, MatrixMockServer}, + mocks::MatrixMockServer, }, Client, Error, SessionChange, }; @@ -648,30 +643,11 @@ async fn test_server_metadata() { let server = MatrixMockServer::new().await; let client = server.client_builder().unlogged().build().await; let oauth = client.oauth(); - let issuer = server.server().uri(); // The endpoint is not mocked so it is not supported. let error = oauth.server_metadata().await.unwrap_err(); assert!(error.is_not_supported()); - // Mock the `GET /auth_issuer` fallback endpoint. - Mock::given(method("GET")) - .and(path("/_matrix/client/unstable/org.matrix.msc2965/auth_issuer")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({"issuer": issuer}))) - .expect(1) - .named("auth_issuer") - .mount(server.server()) - .await; - let metadata = MockServerMetadataBuilder::new(&issuer).build(); - Mock::given(method("GET")) - .and(path("/.well-known/openid-configuration")) - .respond_with(ResponseTemplate::new(200).set_body_json(metadata)) - .expect(1) - .named("openid-configuration") - .mount(server.server()) - .await; - oauth.server_metadata().await.unwrap(); - // Mock the `GET /auth_metadata` endpoint. let oauth_server = server.oauth(); oauth_server.mock_server_metadata().ok().expect(1).named("auth_metadata").mount().await;