Skip to content

Commit 05827d1

Browse files
committed
Don't return locked error for deactivated users
When a user is both locked and deactivated, give precedence to deactivation errors over locked errors, as a locked error suggests that unlocking the user would make it available.
1 parent d7037c5 commit 05827d1

File tree

1 file changed

+82
-5
lines changed

1 file changed

+82
-5
lines changed

crates/handlers/src/compat/login.rs

Lines changed: 82 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -508,16 +508,19 @@ async fn token_login(
508508
);
509509
return Err(RouteError::InvalidLoginToken);
510510
};
511-
if browser_session.user.locked_at.is_some() {
512-
return Err(RouteError::UserLocked);
513-
}
514511
if !browser_session.active() || !browser_session.user.is_valid() {
515512
tracing::info!(
516513
compat_sso_login.id = %login.id,
517514
browser_session.id = %browser_session_id,
518515
"Attempt to exchange login token but browser session is not active"
519516
);
520-
return Err(RouteError::InvalidLoginToken);
517+
return Err(
518+
if browser_session.finished_at.is_some() || browser_session.user.deactivated_at.is_some() {
519+
RouteError::InvalidLoginToken
520+
} else {
521+
RouteError::UserLocked
522+
}
523+
);
521524
}
522525

523526
// We're about to create a device, let's explicitly acquire a lock, so that
@@ -873,7 +876,7 @@ mod tests {
873876

874877
// Now try again after unlocking the account
875878
let mut repo = state.repository().await.unwrap();
876-
let _ = repo.user().unlock(user).await.unwrap();
879+
let user = repo.user().unlock(user).await.unwrap();
877880
repo.save().await.unwrap();
878881

879882
let response = state.request(request).await;
@@ -973,6 +976,45 @@ mod tests {
973976
// The response should be the same as the previous one, so that we don't leak if
974977
// it's the user that is invalid or the password.
975978
assert_eq!(body, old_body);
979+
980+
// Try to login to a deactivated account
981+
let mut repo = state.repository().await.unwrap();
982+
let user = repo.user().deactivate(&state.clock, user).await.unwrap();
983+
repo.save().await.unwrap();
984+
985+
let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({
986+
"type": "m.login.password",
987+
"identifier": {
988+
"type": "m.id.user",
989+
"user": "alice",
990+
},
991+
"password": "password",
992+
}));
993+
994+
let response = state.request(request.clone()).await;
995+
response.assert_status(StatusCode::FORBIDDEN);
996+
let body: serde_json::Value = response.json();
997+
insta::assert_json_snapshot!(body, @r###"
998+
{
999+
"errcode": "M_FORBIDDEN",
1000+
"error": "Invalid username/password"
1001+
}
1002+
"###);
1003+
1004+
// Should get the same error if the deactivated user is also locked
1005+
let mut repo = state.repository().await.unwrap();
1006+
let _user = repo.user().lock(&state.clock, user).await.unwrap();
1007+
repo.save().await.unwrap();
1008+
1009+
let response = state.request(request).await;
1010+
response.assert_status(StatusCode::FORBIDDEN);
1011+
let body: serde_json::Value = response.json();
1012+
insta::assert_json_snapshot!(body, @r###"
1013+
{
1014+
"errcode": "M_FORBIDDEN",
1015+
"error": "Invalid username/password"
1016+
}
1017+
"###);
9761018
}
9771019

9781020
/// Test that we can send a login request without a Content-Type header
@@ -1288,6 +1330,41 @@ mod tests {
12881330
"error": "Login token expired"
12891331
}
12901332
"###);
1333+
1334+
// Try to login to a deactivated account
1335+
let token = get_login_token(&state, &user).await;
1336+
1337+
let mut repo = state.repository().await.unwrap();
1338+
let user = repo.user().deactivate(&state.clock, user).await.unwrap();
1339+
repo.save().await.unwrap();
1340+
let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({
1341+
"type": "m.login.token",
1342+
"token": token,
1343+
}));
1344+
let response = state.request(request.clone()).await;
1345+
response.assert_status(StatusCode::FORBIDDEN);
1346+
let body: serde_json::Value = response.json();
1347+
insta::assert_json_snapshot!(body, @r###"
1348+
{
1349+
"errcode": "M_FORBIDDEN",
1350+
"error": "Invalid login token"
1351+
}
1352+
"###);
1353+
1354+
// Should get the same error if the deactivated user is also locked
1355+
let mut repo = state.repository().await.unwrap();
1356+
let _user = repo.user().lock(&state.clock, user).await.unwrap();
1357+
repo.save().await.unwrap();
1358+
1359+
let response = state.request(request).await;
1360+
response.assert_status(StatusCode::FORBIDDEN);
1361+
let body: serde_json::Value = response.json();
1362+
insta::assert_json_snapshot!(body, @r###"
1363+
{
1364+
"errcode": "M_FORBIDDEN",
1365+
"error": "Invalid login token"
1366+
}
1367+
"###);
12911368
}
12921369

12931370
/// Get a login token for a user.

0 commit comments

Comments
 (0)