Skip to content

Commit 5b449d8

Browse files
poljarpoljar
committed
fix(qr-login): Support the new variant of the m.login.protocols message
1 parent 6fc3247 commit 5b449d8

4 files changed

Lines changed: 142 additions & 23 deletions

File tree

‎crates/matrix-sdk/src/authentication/oauth/qrcode/grant.rs‎

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ use crate::{
3737
Client,
3838
authentication::oauth::qrcode::{
3939
CheckCodeSender, GeneratedQrProgress, LoginFailureReason, QRCodeGrantLoginError,
40-
QrProgress, SecureChannelError,
40+
QrProgress, SecureChannelError, messages::LoginProtocolsMessage,
4141
},
4242
};
4343

@@ -280,9 +280,16 @@ impl<'a> IntoFuture for GrantLoginWithScannedQrCode<'a> {
280280
// Inform the other device about the available login protocols and the
281281
// homeserver to use.
282282
// -- MSC4108 OAuth 2.0 login step 1
283-
let message = QrAuthMessage::LoginProtocols {
284-
protocols: vec![LoginProtocolType::DeviceAuthorizationGrant],
285-
homeserver: self.client.homeserver(),
283+
let message = if channel.is_using_msc_4388() {
284+
QrAuthMessage::LoginProtocols(LoginProtocolsMessage::Msc4388 {
285+
protocols: vec![LoginProtocolType::DeviceAuthorizationGrant],
286+
base_url: self.client.homeserver(),
287+
})
288+
} else {
289+
QrAuthMessage::LoginProtocols(LoginProtocolsMessage::Msc4108 {
290+
protocols: vec![LoginProtocolType::DeviceAuthorizationGrant],
291+
homeserver: self.client.homeserver(),
292+
})
286293
};
287294
channel.send_json(message).await?;
288295

@@ -424,7 +431,7 @@ mod test {
424431

425432
use assert_matches2::{assert_let, assert_matches};
426433
use futures_util::StreamExt;
427-
use matrix_sdk_base::crypto::types::SecretsBundle;
434+
use matrix_sdk_base::crypto::types::{SecretsBundle, qr_login::QrCodeIntentData};
428435
use matrix_sdk_common::executor::spawn;
429436
use matrix_sdk_test::async_test;
430437
use oauth2::{EndUserVerificationUrl, VerificationUriComplete};
@@ -636,6 +643,11 @@ mod test {
636643
device_authorization_grant: Option<AuthorizationGrant>,
637644
secrets_bundle: Option<SecretsBundle>,
638645
) {
646+
let is_using_msc_4388 = match channel.qr_code_data().intent_data() {
647+
QrCodeIntentData::Msc4108 { .. } => false,
648+
QrCodeIntentData::Msc4388 { .. } => true,
649+
};
650+
639651
// Wait for Alice to scan the qr code and connect the secure channel.
640652
let channel =
641653
channel.connect().await.expect("Bob should be able to connect the secure channel");
@@ -651,9 +663,27 @@ mod test {
651663
.receive_json()
652664
.await
653665
.expect("Bob should receive the LoginProtocolAccepted message from Alice");
654-
assert_let!(
655-
QrAuthMessage::LoginProtocols { protocols, homeserver: alice_homeserver } = message
656-
);
666+
667+
let (protocols, alice_homeserver) = if is_using_msc_4388 {
668+
assert_let!(
669+
QrAuthMessage::LoginProtocols(LoginProtocolsMessage::Msc4388 {
670+
protocols,
671+
base_url,
672+
}) = message
673+
);
674+
675+
(protocols, base_url)
676+
} else {
677+
assert_let!(
678+
QrAuthMessage::LoginProtocols(LoginProtocolsMessage::Msc4108 {
679+
protocols,
680+
homeserver,
681+
}) = message
682+
);
683+
684+
(protocols, homeserver)
685+
};
686+
657687
assert_eq!(protocols, vec![LoginProtocolType::DeviceAuthorizationGrant]);
658688
assert_eq!(alice_homeserver, homeserver);
659689

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

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,10 @@ use crate::{
4040
Client,
4141
authentication::oauth::{
4242
ClientRegistrationData, OAuth, OAuthError,
43-
qrcode::{CheckCodeSender, GeneratedQrProgress, LoginProtocolType, QrProgress},
43+
qrcode::{
44+
CheckCodeSender, GeneratedQrProgress, LoginProtocolType, QrProgress,
45+
messages::LoginProtocolsMessage,
46+
},
4447
},
4548
};
4649

@@ -401,7 +404,10 @@ impl<'a> IntoFuture for LoginWithGeneratedQrCode<'a> {
401404
// Verify that the device authorization grant is supported and extract
402405
// the homeserver URL.
403406
let homeserver = match message {
404-
QrAuthMessage::LoginProtocols { protocols, homeserver } => {
407+
QrAuthMessage::LoginProtocols(LoginProtocolsMessage::Msc4108 {
408+
protocols,
409+
homeserver,
410+
}) if !channel.is_using_msc_4388() => {
405411
if !protocols.contains(&LoginProtocolType::DeviceAuthorizationGrant) {
406412
channel
407413
.send_json(QrAuthMessage::LoginFailure {
@@ -418,6 +424,27 @@ impl<'a> IntoFuture for LoginWithGeneratedQrCode<'a> {
418424

419425
homeserver
420426
}
427+
QrAuthMessage::LoginProtocols(LoginProtocolsMessage::Msc4388 {
428+
protocols,
429+
base_url,
430+
}) if channel.is_using_msc_4388() => {
431+
if !protocols.contains(&LoginProtocolType::DeviceAuthorizationGrant) {
432+
channel
433+
.send_json(QrAuthMessage::LoginFailure {
434+
reason: LoginFailureReason::UnsupportedProtocol,
435+
homeserver: None,
436+
})
437+
.await?;
438+
439+
return Err(QRCodeLoginError::LoginFailure {
440+
reason: LoginFailureReason::UnsupportedProtocol,
441+
homeserver: None,
442+
});
443+
}
444+
445+
base_url
446+
}
447+
421448
_ => {
422449
send_unexpected_message_error(&mut channel).await?;
423450

@@ -520,7 +547,7 @@ mod test {
520547
use super::*;
521548
use crate::{
522549
authentication::oauth::qrcode::{
523-
messages::LoginProtocolType,
550+
messages::{LoginProtocolType, LoginProtocolsMessage},
524551
secure_channel::{SecureChannel, test::MockedRendezvousServer},
525552
},
526553
config::RequestConfig,
@@ -740,9 +767,16 @@ mod test {
740767
.expect("Alice should be able to send the check code to Bob");
741768

742769
// Alice sends m.login.protocols message
743-
let message = QrAuthMessage::LoginProtocols {
744-
protocols: vec![LoginProtocolType::DeviceAuthorizationGrant],
745-
homeserver: alice.homeserver(),
770+
let message = if channel.is_using_msc_4388() {
771+
QrAuthMessage::LoginProtocols(LoginProtocolsMessage::Msc4388 {
772+
protocols: vec![LoginProtocolType::DeviceAuthorizationGrant],
773+
base_url: alice.homeserver(),
774+
})
775+
} else {
776+
QrAuthMessage::LoginProtocols(LoginProtocolsMessage::Msc4108 {
777+
protocols: vec![LoginProtocolType::DeviceAuthorizationGrant],
778+
homeserver: alice.homeserver(),
779+
})
746780
};
747781
channel
748782
.send_json(message)

‎crates/matrix-sdk/src/authentication/oauth/qrcode/messages.rs‎

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,7 @@ pub enum QrAuthMessage {
3333
/// Message declaring the available protocols for sign in. Sent by the
3434
/// existing device.
3535
#[serde(rename = "m.login.protocols")]
36-
LoginProtocols {
37-
/// The login protocols the existing device supports.
38-
protocols: Vec<LoginProtocolType>,
39-
/// The homeserver we're going to log in to.
40-
homeserver: Url,
41-
},
36+
LoginProtocols(LoginProtocolsMessage),
4237

4338
/// Message declaring which protocols from the previous `m.login.protocols`
4439
/// message the new device has picked. Sent by the new device.
@@ -88,6 +83,28 @@ pub enum QrAuthMessage {
8883
LoginSecrets(SecretsBundle),
8984
}
9085

86+
/// Message declaring the available protocols for sign in. Sent by the
87+
/// existing device.
88+
///
89+
/// Supports both the MSC4108 variant of the m.login.protocols message as well
90+
/// as the MSC4388 variant.
91+
#[derive(Debug, Serialize, Deserialize)]
92+
#[serde(untagged)]
93+
pub enum LoginProtocolsMessage {
94+
Msc4388 {
95+
/// The login protocols the existing device supports.
96+
protocols: Vec<LoginProtocolType>,
97+
/// The homeserver we're going to log in to.
98+
base_url: Url,
99+
},
100+
Msc4108 {
101+
/// The login protocols the existing device supports.
102+
protocols: Vec<LoginProtocolType>,
103+
/// The homeserver we're going to log in to.
104+
homeserver: Url,
105+
},
106+
}
107+
91108
impl QrAuthMessage {
92109
/// Create a new [`QrAuthMessage::LoginProtocol`] message with the
93110
/// [`LoginProtocolType::DeviceAuthorizationGrant`] protocol type.
@@ -170,17 +187,48 @@ mod test {
170187
use super::*;
171188

172189
#[test]
173-
fn test_protocols_serialization() {
190+
fn test_protocols_serialization_msc_4108() {
191+
const HOMESERVER: &str = "https://matrix-client.matrix.org/";
192+
174193
let json = json!({
175194
"type": "m.login.protocols",
176195
"protocols": ["device_authorization_grant"],
177-
"homeserver": "https://matrix-client.matrix.org/"
196+
"homeserver": HOMESERVER,
178197

179198
});
180199

181200
let message: QrAuthMessage = serde_json::from_value(json.clone()).unwrap();
182-
assert_let!(QrAuthMessage::LoginProtocols { protocols, .. } = &message);
201+
assert_let!(
202+
QrAuthMessage::LoginProtocols(LoginProtocolsMessage::Msc4108 {
203+
protocols,
204+
homeserver
205+
}) = &message
206+
);
207+
assert!(protocols.contains(&LoginProtocolType::DeviceAuthorizationGrant));
208+
assert_eq!(homeserver.as_str(), HOMESERVER);
209+
210+
let serialized = serde_json::to_value(&message).unwrap();
211+
assert_eq!(json, serialized);
212+
}
213+
214+
#[test]
215+
fn test_protocols_serialization_msc_4388() {
216+
const HOMESERVER: &str = "https://matrix-client.matrix.org/";
217+
218+
let json = json!({
219+
"type": "m.login.protocols",
220+
"protocols": ["device_authorization_grant"],
221+
"base_url": HOMESERVER,
222+
223+
});
224+
225+
let message: QrAuthMessage = serde_json::from_value(json.clone()).unwrap();
226+
assert_let!(
227+
QrAuthMessage::LoginProtocols(LoginProtocolsMessage::Msc4388 { protocols, base_url }) =
228+
&message
229+
);
183230
assert!(protocols.contains(&LoginProtocolType::DeviceAuthorizationGrant));
231+
assert_eq!(base_url.as_str(), HOMESERVER);
184232

185233
let serialized = serde_json::to_value(&message).unwrap();
186234
assert_eq!(json, serialized);

‎crates/matrix-sdk/src/authentication/oauth/qrcode/secure_channel/mod.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,13 @@ impl EstablishedSecureChannel {
319319
}
320320
}
321321

322+
pub(super) fn is_using_msc_4388(&self) -> bool {
323+
match &self.channel {
324+
RendezvousChannel::Msc4108(_) => false,
325+
RendezvousChannel::Msc4388(_) => true,
326+
}
327+
}
328+
322329
/// Get the [`CheckCode`] which can be used to, out of band, verify that
323330
/// both sides of the channel are indeed communicating with each other and
324331
/// not with a 3rd party.

0 commit comments

Comments
 (0)