Skip to content

Return M_USER_LOCKED error when logging in with a locked account on the compatibility endpoints #4789

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 17, 2025
Merged

Return M_USER_LOCKED error when logging in with a locked account on the compatibility endpoints #4789

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8dedf6b
Support M_USER_LOCKED error for compat sessions
AndrewFerr Jul 14, 2025
8e19d98
Check for compat session locking earlier
AndrewFerr Jul 15, 2025
82e3ea6
Add test coverage
AndrewFerr Jul 15, 2025
d7037c5
Satisfy Clippy
AndrewFerr Jul 15, 2025
05827d1
Don't return locked error for deactivated users
AndrewFerr Jul 16, 2025
6183cae
Format
AndrewFerr Jul 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 172 additions & 17 deletions crates/handlers/src/compat/login.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,9 @@ pub enum RouteError {
#[error("invalid login token")]
InvalidLoginToken,

#[error("user is locked")]
UserLocked,

#[error("failed to provision device")]
ProvisionDeviceFailed(#[source] anyhow::Error),
}
Expand Down Expand Up @@ -263,6 +266,11 @@ impl IntoResponse for RouteError {
error: "Invalid login token",
status: StatusCode::FORBIDDEN,
},
Self::UserLocked => MatrixError {
errcode: "M_USER_LOCKED",
error: "User account has been locked",
status: StatusCode::UNAUTHORIZED,
},
};

(sentry_event_id, response).into_response()
Expand Down Expand Up @@ -506,7 +514,15 @@ async fn token_login(
browser_session.id = %browser_session_id,
"Attempt to exchange login token but browser session is not active"
);
return Err(RouteError::InvalidLoginToken);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be more readable to check if the user is locked earlier

if browser_session.user.locked_at.is_some() {
  return Err(RouteError::UserLocked);
}

if !browser_session.active() || !browser_session.user.is_valid() {
  // ...
}

It changes a little bit what error we show in what condition, but it's an edge-case anyway, as usually we would show a user comprehensible error earlier in the UI

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0d7fde0

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, I've reverted this with 05827d1 (the commit message explains why). The logic is more precise this time, though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, works for me!

return Err(
if browser_session.finished_at.is_some()
|| browser_session.user.deactivated_at.is_some()
{
RouteError::InvalidLoginToken
} else {
RouteError::UserLocked
},
);
}

// We're about to create a device, let's explicitly acquire a lock, so that
Expand Down Expand Up @@ -565,9 +581,13 @@ async fn user_password_login(
.user()
.find_by_username(username)
.await?
.filter(mas_data_model::User::is_valid)
.filter(|user| user.deactivated_at.is_none())
.ok_or(RouteError::UserNotFound)?;

if user.locked_at.is_some() {
return Err(RouteError::UserLocked);
}

// Check the rate limit
limiter.check_password(requester, &user)?;

Expand Down Expand Up @@ -785,7 +805,12 @@ mod tests {
"###);
}

async fn user_with_password(state: &TestState, username: &str, password: &str) {
async fn user_with_password(
state: &TestState,
username: &str,
password: &str,
locked: bool,
) -> User {
let mut rng = state.rng();
let mut repo = state.repository().await.unwrap();

Expand All @@ -811,7 +836,14 @@ mod tests {
.await
.unwrap();

let user = if locked {
repo.user().lock(&state.clock, user).await.unwrap()
} else {
user
};

repo.save().await.unwrap();
user
}

/// Test that a user can login with a password using the Matrix
Expand All @@ -821,7 +853,7 @@ mod tests {
setup();
let state = TestState::from_pool(pool).await.unwrap();

user_with_password(&state, "alice", "password").await;
let user = user_with_password(&state, "alice", "password", true).await;

// Now let's try to login with the password, without asking for a refresh token.
let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({
Expand All @@ -833,14 +865,30 @@ mod tests {
"password": "password",
}));

// First try to login to a locked account
let response = state.request(request.clone()).await;
response.assert_status(StatusCode::UNAUTHORIZED);
let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r###"
{
"errcode": "M_USER_LOCKED",
"error": "User account has been locked"
}
"###);

// Now try again after unlocking the account
let mut repo = state.repository().await.unwrap();
let user = repo.user().unlock(user).await.unwrap();
repo.save().await.unwrap();

let response = state.request(request).await;
response.assert_status(StatusCode::OK);

let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r###"
{
"access_token": "mct_16tugBE5Ta9LIWoSJaAEHHq2g3fx8S_alcBB4",
"device_id": "ZGpSvYQqlq",
"access_token": "mct_cxG6gZXyvelQWW9XqfNbm5KAQovodf_XvJz43",
"device_id": "42oTpLoieH",
"user_id": "@alice:example.com"
}
"###);
Expand All @@ -862,10 +910,10 @@ mod tests {
let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r###"
{
"access_token": "mct_cxG6gZXyvelQWW9XqfNbm5KAQovodf_XvJz43",
"device_id": "42oTpLoieH",
"access_token": "mct_PGMLvvMXC4Ds1A3lCWc6Hx4l9DGzqG_lVEIV2",
"device_id": "Yp7FM44zJN",
"user_id": "@alice:example.com",
"refresh_token": "mcr_7IvDc44woP66fRQoS9MVcHXO9OeBmR_0jDGr1",
"refresh_token": "mcr_LoYqtrtBUBcWlE4RX6o47chBCGkadB_9gzpc1",
"expires_in_ms": 300000
}
"###);
Expand All @@ -883,8 +931,8 @@ mod tests {
let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r###"
{
"access_token": "mct_PGMLvvMXC4Ds1A3lCWc6Hx4l9DGzqG_lVEIV2",
"device_id": "Yp7FM44zJN",
"access_token": "mct_Xl3bbpfh9yNy9NzuRxyR3b3PLW0rqd_DiXAH2",
"device_id": "6cq7FqNSYo",
"user_id": "@alice:example.com"
}
"###);
Expand Down Expand Up @@ -930,6 +978,45 @@ mod tests {
// The response should be the same as the previous one, so that we don't leak if
// it's the user that is invalid or the password.
assert_eq!(body, old_body);

// Try to login to a deactivated account
let mut repo = state.repository().await.unwrap();
let user = repo.user().deactivate(&state.clock, user).await.unwrap();
repo.save().await.unwrap();

let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({
"type": "m.login.password",
"identifier": {
"type": "m.id.user",
"user": "alice",
},
"password": "password",
}));

let response = state.request(request.clone()).await;
response.assert_status(StatusCode::FORBIDDEN);
let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r###"
{
"errcode": "M_FORBIDDEN",
"error": "Invalid username/password"
}
"###);

// Should get the same error if the deactivated user is also locked
let mut repo = state.repository().await.unwrap();
let _user = repo.user().lock(&state.clock, user).await.unwrap();
repo.save().await.unwrap();

let response = state.request(request).await;
response.assert_status(StatusCode::FORBIDDEN);
let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r###"
{
"errcode": "M_FORBIDDEN",
"error": "Invalid username/password"
}
"###);
}

/// Test that we can send a login request without a Content-Type header
Expand All @@ -938,7 +1025,7 @@ mod tests {
setup();
let state = TestState::from_pool(pool).await.unwrap();

user_with_password(&state, "alice", "password").await;
user_with_password(&state, "alice", "password", false).await;
// Try without a Content-Type header
let mut request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({
"type": "m.login.password",
Expand Down Expand Up @@ -970,7 +1057,7 @@ mod tests {
setup();
let state = TestState::from_pool(pool).await.unwrap();

user_with_password(&state, "alice", "password").await;
let user = user_with_password(&state, "alice", "password", true).await;

// Login with a full MXID as identifier
let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({
Expand All @@ -982,13 +1069,29 @@ mod tests {
"password": "password",
}));

// First try to login to a locked account
let response = state.request(request.clone()).await;
response.assert_status(StatusCode::UNAUTHORIZED);
let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r###"
{
"errcode": "M_USER_LOCKED",
"error": "User account has been locked"
}
"###);

// Now try again after unlocking the account
let mut repo = state.repository().await.unwrap();
let _ = repo.user().unlock(user).await.unwrap();
repo.save().await.unwrap();

let response = state.request(request).await;
response.assert_status(StatusCode::OK);
let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r###"
{
"access_token": "mct_16tugBE5Ta9LIWoSJaAEHHq2g3fx8S_alcBB4",
"device_id": "ZGpSvYQqlq",
"access_token": "mct_cxG6gZXyvelQWW9XqfNbm5KAQovodf_XvJz43",
"device_id": "42oTpLoieH",
"user_id": "@alice:example.com"
}
"###);
Expand Down Expand Up @@ -1132,6 +1235,8 @@ mod tests {
.add(&mut state.rng(), &state.clock, "alice".to_owned())
.await
.unwrap();
// Start with a locked account
let user = repo.user().lock(&state.clock, user).await.unwrap();
repo.save().await.unwrap();

let mxid = state.homeserver_connection.mxid(&user.username);
Expand Down Expand Up @@ -1164,14 +1269,29 @@ mod tests {
"type": "m.login.token",
"token": token,
}));
let response = state.request(request.clone()).await;
response.assert_status(StatusCode::UNAUTHORIZED);
let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r###"
{
"errcode": "M_USER_LOCKED",
"error": "User account has been locked"
}
"###);

// Now try again after unlocking the account
let mut repo = state.repository().await.unwrap();
let user = repo.user().unlock(user).await.unwrap();
repo.save().await.unwrap();

let response = state.request(request).await;
response.assert_status(StatusCode::OK);

let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r#"
{
"access_token": "mct_bnkWh1tPmm1MZOpygPaXwygX8PfxEY_hE6do1",
"device_id": "O3Ju1MUh3Z",
"access_token": "mct_bUTa4XIh92RARTPTjqQrCZLAkq2ild_0VsYE6",
"device_id": "uihy4bk51g",
"user_id": "@alice:example.com"
}
"#);
Expand Down Expand Up @@ -1212,6 +1332,41 @@ mod tests {
"error": "Login token expired"
}
"###);

// Try to login to a deactivated account
let token = get_login_token(&state, &user).await;

let mut repo = state.repository().await.unwrap();
let user = repo.user().deactivate(&state.clock, user).await.unwrap();
repo.save().await.unwrap();
let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({
"type": "m.login.token",
"token": token,
}));
let response = state.request(request.clone()).await;
response.assert_status(StatusCode::FORBIDDEN);
let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r###"
{
"errcode": "M_FORBIDDEN",
"error": "Invalid login token"
}
"###);

// Should get the same error if the deactivated user is also locked
let mut repo = state.repository().await.unwrap();
let _user = repo.user().lock(&state.clock, user).await.unwrap();
repo.save().await.unwrap();

let response = state.request(request).await;
response.assert_status(StatusCode::FORBIDDEN);
let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r###"
{
"errcode": "M_FORBIDDEN",
"error": "Invalid login token"
}
"###);
}

/// Get a login token for a user.
Expand Down
Loading