Skip to content

Commit 51a1cd3

Browse files
authored
Merge pull request #4604 from zecakeh/qr-login-oauth2
refactor(auth-qrcode): Use oauth2 crate instead of openidconnect
2 parents 9c9944a + c6d2ab4 commit 51a1cd3

File tree

9 files changed

+219
-347
lines changed

9 files changed

+219
-347
lines changed

Cargo.lock

Lines changed: 1 addition & 60 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ impl From<qrcode::QRCodeLoginError> for HumanQrLoginError {
104104
_ => HumanQrLoginError::Unknown,
105105
},
106106

107-
QRCodeLoginError::Oidc(e) => {
107+
QRCodeLoginError::Oauth(e) => {
108108
if let Some(e) = e.as_request_token_error() {
109109
match e {
110110
DeviceCodeErrorResponseType::AccessDenied => HumanQrLoginError::Declined,
@@ -153,8 +153,8 @@ pub enum QrLoginProgress {
153153
/// first digit is a zero.
154154
check_code_string: String,
155155
},
156-
/// We are waiting for the login and for the OIDC provider to give us an
157-
/// access token.
156+
/// We are waiting for the login and for the OAuth 2.0 authorization server
157+
/// to give us an access token.
158158
WaitingForToken { user_code: String },
159159
/// The login has successfully finished.
160160
Done,
@@ -673,8 +673,8 @@ impl ClientBuilder {
673673
///
674674
/// This method will build the client and immediately attempt to log the
675675
/// client in using the provided [`QrCodeData`] using the login
676-
/// mechanism described in [MSC4108]. As such this methods requires OIDC
677-
/// support as well as sliding sync support.
676+
/// mechanism described in [MSC4108]. As such this methods requires OAuth
677+
/// 2.0 support as well as sliding sync support.
678678
///
679679
/// The usage of the progress_listener is required to transfer the
680680
/// [`CheckCode`] to the existing client.

crates/matrix-sdk/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ All notable changes to this project will be documented in this file.
3939
`ClientCredentials`
4040
- `Oidc::restore_registered_client()` must NOT be called after
4141
`Oidc::register_client()` anymore.
42+
- [**breaking**]: The `authentication::qrcode` module now reexports types from
43+
`oauth2` rather than `openidconnect`. Some type names might have changed.
44+
([#4604](https://github.com/matrix-org/matrix-rust-sdk/pull/4604))
4245

4346
## [0.10.0] - 2025-02-04
4447

crates/matrix-sdk/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ experimental-oidc = [
5353
"dep:rand",
5454
"dep:sha2",
5555
"dep:tower",
56-
"dep:openidconnect",
56+
"dep:oauth2",
5757
]
5858
experimental-widgets = ["dep:language-tags", "dep:uuid"]
5959

@@ -129,7 +129,7 @@ tokio = { workspace = true, features = ["macros"] }
129129

130130
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
131131
backoff = { version = "0.4.0", features = ["tokio"] }
132-
openidconnect = { version = "4.0.0", optional = true }
132+
oauth2 = { version = "5.0.0", default-features = false, features = ["reqwest"], optional = true }
133133
# only activate reqwest's stream feature on non-wasm, the wasm part seems to not
134134
# support *sending* streams, which makes it useless for us.
135135
reqwest = { workspace = true, features = ["stream", "gzip", "http2"] }

crates/matrix-sdk/src/authentication/qrcode/login.rs

Lines changed: 51 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ use matrix_sdk_base::{
2222
crypto::types::qr_login::{QrCodeData, QrCodeMode},
2323
SessionMeta,
2424
};
25-
use openidconnect::DeviceCodeErrorResponseType;
25+
use oauth2::DeviceCodeErrorResponseType;
2626
use ruma::OwnedDeviceId;
2727
use tracing::trace;
2828
use vodozemac::ecies::CheckCode;
2929

3030
use super::{
31-
messages::LoginFailureReason, oidc_client::OidcClient, DeviceAuhorizationOidcError,
31+
messages::LoginFailureReason, oauth_client::OauthClient, DeviceAuthorizationOauthError,
3232
SecureChannelError,
3333
};
3434
#[cfg(doc)]
@@ -64,11 +64,13 @@ pub enum LoginProgress {
6464
/// The check code we need to, out of band, send to the other device.
6565
check_code: CheckCode,
6666
},
67-
/// We're waiting for the OIDC provider to give us the access token. This
68-
/// will only happen if the other device allows the OIDC provider to so.
67+
/// We're waiting for the OAuth 2.0 authorization server to give us the
68+
/// access token. This will only happen if the other device allows the
69+
/// OAuth 2.0 authorization server to do so.
6970
WaitingForToken {
70-
/// The user code the OIDC provider has given us, the OIDC provider
71-
/// might ask the other device to enter this code.
71+
/// The user code the OAuth 2.0 authorization server has given us, the
72+
/// OAuth 2.0 authorization server might ask the other device to
73+
/// enter this code.
7274
user_code: String,
7375
},
7476
/// The login process has completed.
@@ -113,20 +115,20 @@ impl<'a> IntoFuture for LoginWithQrCode<'a> {
113115
let check_code = channel.check_code().to_owned();
114116
self.state.set(LoginProgress::EstablishingSecureChannel { check_code });
115117

116-
// Register the client with the OIDC provider.
117-
trace!("Registering the client with the OIDC provider.");
118-
let oidc_client = self.register_client().await?;
118+
// Register the client with the OAuth 2.0 authorization server.
119+
trace!("Registering the client with the OAuth 2.0 authorization server.");
120+
let oauth_client = self.register_client().await?;
119121

120122
// We want to use the Curve25519 public key for the device ID, so let's generate
121123
// a new vodozemac `Account` now.
122124
let account = vodozemac::olm::Account::new();
123125
let public_key = account.identity_keys().curve25519;
124126
let device_id = public_key;
125127

126-
// Let's tell the OIDC provider that we want to log in using the device
127-
// authorization grant described in [RFC8628](https://datatracker.ietf.org/doc/html/rfc8628).
128+
// Let's tell the OAuth 2.0 authorization server that we want to log in using
129+
// the device authorization grant described in [RFC8628](https://datatracker.ietf.org/doc/html/rfc8628).
128130
trace!("Requesting device authorization.");
129-
let auth_grant_response = oidc_client.request_device_authorization(device_id).await?;
131+
let auth_grant_response = oauth_client.request_device_authorization(device_id).await?;
130132

131133
// Now we need to inform the other device of the login protocols we picked and
132134
// the URL they should use to log us in.
@@ -153,17 +155,17 @@ impl<'a> IntoFuture for LoginWithQrCode<'a> {
153155
}
154156
}
155157

156-
// The OIDC provider may or may not show this user code to double check that
157-
// we're talking to the right OIDC provider. Let us display this, so
158+
// The OAuth 2.0 authorization server may or may not show this user code to
159+
// double check that we're talking to the right server. Let us display this, so
158160
// the other device can double check this as well.
159161
let user_code = auth_grant_response.user_code();
160162
self.state
161163
.set(LoginProgress::WaitingForToken { user_code: user_code.secret().to_owned() });
162164

163-
// Let's now wait for the access token to be provided to use by the OIDC
164-
// provider.
165-
trace!("Waiting for the OIDC provider to give us the access token.");
166-
let session_tokens = match oidc_client.wait_for_tokens(&auth_grant_response).await {
165+
// Let's now wait for the access token to be provided to use by the OAuth 2.0
166+
// authorization server.
167+
trace!("Waiting for the OAuth 2.0 authorization server to give us the access token.");
168+
let session_tokens = match oauth_client.wait_for_tokens(&auth_grant_response).await {
167169
Ok(t) => t,
168170
Err(e) => {
169171
// If we received an error, and it's one of the ones we should report to the
@@ -190,11 +192,11 @@ impl<'a> IntoFuture for LoginWithQrCode<'a> {
190192
};
191193
self.client.oidc().set_session_tokens(session_tokens);
192194

193-
// We only received an access token from the OIDC provider, we have no clue who
194-
// we are, so we need to figure out our user ID now.
195-
// TODO: This snippet is almost the same as the Oidc::finish_login_method(), why
196-
// is that method even a public method and not called as part of the set session
197-
// tokens method.
195+
// We only received an access token from the OAuth 2.0 authorization server, we
196+
// have no clue who we are, so we need to figure out our user ID
197+
// now. TODO: This snippet is almost the same as the
198+
// Oidc::finish_login_method(), why is that method even a public
199+
// method and not called as part of the set session tokens method.
198200
trace!("Discovering our own user id.");
199201
let whoami_response =
200202
self.client.whoami().await.map_err(QRCodeLoginError::UserIdDiscovery)?;
@@ -288,30 +290,26 @@ impl<'a> LoginWithQrCode<'a> {
288290
Ok(channel)
289291
}
290292

291-
async fn register_client(&self) -> Result<OidcClient, DeviceAuhorizationOidcError> {
292-
// Let's figure out the OIDC issuer, this fetches the info from the homeserver.
293-
let issuer = self
294-
.client
295-
.oidc()
293+
async fn register_client(&self) -> Result<OauthClient, DeviceAuthorizationOauthError> {
294+
let oidc = self.client.oidc();
295+
296+
// Let's figure out the OAuth 2.0 issuer, this fetches the info from the
297+
// homeserver.
298+
let issuer = oidc
296299
.fetch_authentication_issuer()
297300
.await
298-
.map_err(DeviceAuhorizationOidcError::AuthenticationIssuer)?;
301+
.map_err(DeviceAuthorizationOauthError::AuthenticationIssuer)?;
299302

300-
// Now we register the client with the OIDC provider.
303+
// Now we register the client with the OAuth 2.0 authorization server.
301304
let registration_response =
302-
self.client.oidc().register_client(&issuer, self.client_metadata.clone(), None).await?;
305+
oidc.register_client(&issuer, self.client_metadata.clone(), None).await?;
303306

304307
// We're now switching to the oauth2 crate, it has a bit of a strange API
305308
// where you need to provide the HTTP client in every call you make.
306309
let http_client = self.client.inner.http_client.clone();
310+
let server_metadata = oidc.provider_metadata().await?;
307311

308-
OidcClient::new(
309-
registration_response.client_id,
310-
issuer,
311-
http_client,
312-
registration_response.client_secret.as_deref(),
313-
)
314-
.await
312+
OauthClient::new(registration_response.client_id, &server_metadata, http_client)
315313
}
316314
}
317315

@@ -614,21 +612,26 @@ mod test {
614612
alice.send_json(message).await.unwrap();
615613
}
616614

617-
async fn mock_oidc_provider(server: &MockServer, token_response: ResponseTemplate) {
615+
async fn mock_oauth_authorization_server(
616+
server: &MockServer,
617+
token_response: ResponseTemplate,
618+
) {
618619
Mock::given(method("GET"))
619620
.and(path("/_matrix/client/unstable/org.matrix.msc2965/auth_issuer"))
620621
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
621622
"issuer": server.uri(),
622623

623624
})))
624625
.expect(1)
626+
.named("auth_issuer")
625627
.mount(server)
626628
.await;
627629

628630
Mock::given(method("GET"))
629631
.and(path("/.well-known/openid-configuration"))
630632
.respond_with(ResponseTemplate::new(200).set_body_json(open_id_configuration(server)))
631633
.expect(1..)
634+
.named("server_metadata")
632635
.mount(server)
633636
.await;
634637

@@ -639,26 +642,29 @@ mod test {
639642
"client_id_issued_at": 1716375696
640643
})))
641644
.expect(1)
645+
.named("registration_endpoint")
642646
.mount(server)
643647
.await;
644648

645649
Mock::given(method("GET"))
646650
.and(path("/oauth2/keys.json"))
647651
.respond_with(ResponseTemplate::new(200).set_body_json(keys_json()))
648-
.expect(1)
652+
.named("jwks")
649653
.mount(server)
650654
.await;
651655

652656
Mock::given(method("POST"))
653657
.and(path("/oauth2/device"))
654658
.respond_with(ResponseTemplate::new(200).set_body_json(device_code(server)))
655659
.expect(1)
660+
.named("device_authorization_endpoint")
656661
.mount(server)
657662
.await;
658663

659664
Mock::given(method("POST"))
660665
.and(path("/oauth2/token"))
661666
.respond_with(token_response)
667+
.named("token_endpoint")
662668
.mount(server)
663669
.await;
664670
}
@@ -669,7 +675,8 @@ mod test {
669675
let rendezvous_server = MockedRendezvousServer::new(&server, "abcdEFG12345").await;
670676
let (sender, receiver) = tokio::sync::oneshot::channel();
671677

672-
mock_oidc_provider(&server, ResponseTemplate::new(200).set_body_json(token())).await;
678+
mock_oauth_authorization_server(&server, ResponseTemplate::new(200).set_body_json(token()))
679+
.await;
673680

674681
Mock::given(method("GET"))
675682
.and(path("/_matrix/client/r0/account/whoami"))
@@ -764,7 +771,7 @@ mod test {
764771
let rendezvous_server = MockedRendezvousServer::new(&server, "abcdEFG12345").await;
765772
let (sender, receiver) = tokio::sync::oneshot::channel();
766773

767-
mock_oidc_provider(&server, token_response).await;
774+
mock_oauth_authorization_server(&server, token_response).await;
768775

769776
Mock::given(method("GET"))
770777
.and(path("/_matrix/client/r0/account/whoami"))
@@ -830,7 +837,7 @@ mod test {
830837
)
831838
.await;
832839

833-
assert_let!(Err(QRCodeLoginError::Oidc(e)) = result);
840+
assert_let!(Err(QRCodeLoginError::Oauth(e)) = result);
834841
assert_eq!(
835842
e.as_request_token_error(),
836843
Some(&DeviceCodeErrorResponseType::AccessDenied),
@@ -848,7 +855,7 @@ mod test {
848855
)
849856
.await;
850857

851-
assert_let!(Err(QRCodeLoginError::Oidc(e)) = result);
858+
assert_let!(Err(QRCodeLoginError::Oauth(e)) = result);
852859
assert_eq!(
853860
e.as_request_token_error(),
854861
Some(&DeviceCodeErrorResponseType::ExpiredToken),

0 commit comments

Comments
 (0)