Skip to content

Commit fccafd8

Browse files
Johennespoljar
authored andcommitted
feat(oauth): expose session expiration errors when requesting login with a QR code
Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
1 parent 1e30d5f commit fccafd8

File tree

5 files changed

+108
-13
lines changed

5 files changed

+108
-13
lines changed

bindings/matrix-sdk-ffi/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file.
88

99
### Breaking changes
1010

11+
- Add `HumanQrLoginError::NotFound` for non-existing / expired rendezvous sessions
12+
([#5898](https://github.com/matrix-org/matrix-rust-sdk/pull/5898))
1113
- Add `HumanQrGrantLoginError::NotFound` for non-existing / expired rendezvous sessions
1214
([#5898](https://github.com/matrix-org/matrix-rust-sdk/pull/5898))
1315
- The `LatestEventValue::Local` type gains 2 new fields: `sender` and `profile`.

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,8 @@ pub enum HumanQrLoginError {
284284
CheckCodeAlreadySent,
285285
#[error("The check code could not be sent.")]
286286
CheckCodeCannotBeSent,
287+
#[error("The rendezvous session was not found and might have expired")]
288+
NotFound,
287289
}
288290

289291
impl From<qrcode::QRCodeLoginError> for HumanQrLoginError {
@@ -331,6 +333,8 @@ impl From<qrcode::QRCodeLoginError> for HumanQrLoginError {
331333
| QRCodeLoginError::UserIdDiscovery(_)
332334
| QRCodeLoginError::SecretImport(_)
333335
| QRCodeLoginError::ServerReset(_) => HumanQrLoginError::Unknown,
336+
337+
QRCodeLoginError::NotFound => HumanQrLoginError::NotFound,
334338
}
335339
}
336340
}

crates/matrix-sdk/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file.
88

99
### Features
1010

11+
- Add `QRCodeLoginError::NotFound` for non-existing / expired rendezvous sessions
12+
([#5898](https://github.com/matrix-org/matrix-rust-sdk/pull/5898))
1113
- Add `QRCodeGrantLoginError::NotFound` for non-existing / expired rendezvous sessions
1214
([#5898](https://github.com/matrix-org/matrix-rust-sdk/pull/5898))
1315
- Improve logging around key history bundles when joining a room.

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

Lines changed: 78 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,7 @@ mod test {
511511
UnexpectedMessage,
512512
UnexpectedMessageInsteadOfSecrets,
513513
RefuseSecrets,
514+
LetSessionExpire,
514515
}
515516

516517
/// The possible token responses.
@@ -953,17 +954,37 @@ mod test {
953954
alice_behavior: AliceBehaviour,
954955
) -> Result<(), QRCodeLoginError> {
955956
let server = MatrixMockServer::new().await;
957+
let expiration = match alice_behavior {
958+
AliceBehaviour::LetSessionExpire => Duration::from_secs(2),
959+
_ => Duration::MAX,
960+
};
956961
let rendezvous_server =
957-
MockedRendezvousServer::new(server.server(), "abcdEFG12345", Duration::MAX).await;
962+
MockedRendezvousServer::new(server.server(), "abcdEFG12345", expiration).await;
958963
let (sender, receiver) = tokio::sync::oneshot::channel();
959964

960965
let oauth_server = server.oauth();
961-
oauth_server.mock_server_metadata().ok().expect(1).named("server_metadata").mount().await;
962-
oauth_server.mock_registration().ok().expect(1).named("registration").mount().await;
966+
let expected_calls = match alice_behavior {
967+
AliceBehaviour::LetSessionExpire => 0,
968+
_ => 1,
969+
};
970+
oauth_server
971+
.mock_server_metadata()
972+
.ok()
973+
.expect(expected_calls)
974+
.named("server_metadata")
975+
.mount()
976+
.await;
977+
oauth_server
978+
.mock_registration()
979+
.ok()
980+
.expect(expected_calls)
981+
.named("registration")
982+
.mount()
983+
.await;
963984
oauth_server
964985
.mock_device_authorization()
965986
.ok()
966-
.expect(1)
987+
.expect(expected_calls)
967988
.named("device_authorization")
968989
.mount()
969990
.await;
@@ -1017,7 +1038,12 @@ mod test {
10171038
}
10181039
}
10191040
});
1020-
let _alice_task = spawn(async move { grant_login(alice, receiver, alice_behavior).await });
1041+
1042+
if !matches!(alice_behavior, AliceBehaviour::LetSessionExpire) {
1043+
let _alice_task =
1044+
spawn(async move { grant_login(alice, receiver, alice_behavior).await });
1045+
}
1046+
10211047
login_bob.await
10221048
}
10231049

@@ -1026,18 +1052,39 @@ mod test {
10261052
alice_behavior: AliceBehaviour,
10271053
) -> Result<(), QRCodeLoginError> {
10281054
let server = MatrixMockServer::new().await;
1055+
let expiration = match alice_behavior {
1056+
AliceBehaviour::LetSessionExpire => Duration::from_secs(2),
1057+
_ => Duration::MAX,
1058+
};
10291059
let rendezvous_server =
1030-
MockedRendezvousServer::new(server.server(), "abcdEFG12345", Duration::MAX).await;
1060+
MockedRendezvousServer::new(server.server(), "abcdEFG12345", expiration).await;
1061+
10311062
let (qr_sender, qr_receiver) = tokio::sync::oneshot::channel();
10321063
let (cctx_sender, cctx_receiver) = tokio::sync::oneshot::channel();
10331064

10341065
let oauth_server = server.oauth();
1035-
oauth_server.mock_server_metadata().ok().expect(1).named("server_metadata").mount().await;
1036-
oauth_server.mock_registration().ok().expect(1).named("registration").mount().await;
1066+
let expected_calls = match alice_behavior {
1067+
AliceBehaviour::LetSessionExpire => 0,
1068+
_ => 1,
1069+
};
1070+
oauth_server
1071+
.mock_server_metadata()
1072+
.ok()
1073+
.expect(expected_calls)
1074+
.named("server_metadata")
1075+
.mount()
1076+
.await;
1077+
oauth_server
1078+
.mock_registration()
1079+
.ok()
1080+
.expect(expected_calls)
1081+
.named("registration")
1082+
.mount()
1083+
.await;
10371084
oauth_server
10381085
.mock_device_authorization()
10391086
.ok()
1040-
.expect(1)
1087+
.expect(expected_calls)
10411088
.named("device_authorization")
10421089
.mount()
10431090
.await;
@@ -1107,9 +1154,13 @@ mod test {
11071154
}
11081155
});
11091156

1110-
let _alice_task = spawn(async move {
1111-
grant_login_with_generated_qr(&alice, qr_receiver, cctx_receiver, alice_behavior).await
1112-
});
1157+
if !matches!(alice_behavior, AliceBehaviour::LetSessionExpire) {
1158+
let _alice_task = spawn(async move {
1159+
grant_login_with_generated_qr(&alice, qr_receiver, cctx_receiver, alice_behavior)
1160+
.await
1161+
});
1162+
}
1163+
11131164
bob_login.await
11141165
}
11151166

@@ -1243,6 +1294,21 @@ mod test {
12431294
assert_eq!(reason, LoginFailureReason::DeviceNotFound);
12441295
}
12451296

1297+
#[async_test]
1298+
async fn test_qr_login_session_expired() {
1299+
let result = test_failure(TokenResponse::Ok, AliceBehaviour::LetSessionExpire).await;
1300+
1301+
assert_matches!(result, Err(QRCodeLoginError::NotFound));
1302+
}
1303+
1304+
#[async_test]
1305+
async fn test_generated_qr_login_session_expired() {
1306+
let result =
1307+
test_generated_failure(TokenResponse::Ok, AliceBehaviour::LetSessionExpire).await;
1308+
1309+
assert_matches!(result, Err(QRCodeLoginError::NotFound));
1310+
}
1311+
12461312
#[async_test]
12471313
async fn test_device_authorization_endpoint_missing() {
12481314
let server = MatrixMockServer::new().await;

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,11 @@ pub enum QRCodeLoginError {
8686

8787
/// An error happened while exchanging messages with the other device.
8888
#[error(transparent)]
89-
SecureChannel(#[from] SecureChannelError),
89+
SecureChannel(SecureChannelError),
90+
91+
/// The rendezvous session was not found and might have expired.
92+
#[error("The rendezvous session was not found and might have expired")]
93+
NotFound,
9094

9195
/// The cross-process refresh lock failed to be initialized.
9296
#[error(transparent)]
@@ -118,6 +122,23 @@ pub enum QRCodeLoginError {
118122
ServerReset(crate::Error),
119123
}
120124

125+
impl From<SecureChannelError> for QRCodeLoginError {
126+
fn from(e: SecureChannelError) -> Self {
127+
match e {
128+
SecureChannelError::RendezvousChannel(HttpError::Api(ref boxed)) => {
129+
if let FromHttpResponseError::Server(api_error) = boxed.as_ref()
130+
&& let Some(ErrorKind::NotFound) =
131+
api_error.as_client_api_error().and_then(|e| e.error_kind())
132+
{
133+
return Self::NotFound;
134+
}
135+
Self::SecureChannel(e)
136+
}
137+
e => Self::SecureChannel(e),
138+
}
139+
}
140+
}
141+
121142
/// The error type for failures while trying to grant log in to a new device
122143
/// using a QR code.
123144
#[derive(Debug, Error)]

0 commit comments

Comments
 (0)