From cbba225edc792dfc69dfe14cecc43316a7c0e89d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 8 May 2026 11:30:39 -0500 Subject: [PATCH 01/13] Add config stub --- crates/config/src/sections/experimental.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/crates/config/src/sections/experimental.rs b/crates/config/src/sections/experimental.rs index 6806ec760..4a8d61a86 100644 --- a/crates/config/src/sections/experimental.rs +++ b/crates/config/src/sections/experimental.rs @@ -162,6 +162,17 @@ pub struct SessionLimitConfig { /// /// [`dangerous_hard_limit_eviction`]: Self::dangerous_hard_limit_eviction pub hard_limit: NonZeroU64, + /// When set, only accounts with <= `max_session_threshold` sessions have the + /// session limits applied. + /// + /// This is most applicable in scenarios where your homeserver has many legacy + /// bots/scripts that login over and over (which ideally should be using [personal + /// access + /// tokens](https://github.com/element-hq/matrix-authentication-service/issues/4492)) + /// and you want to avoid breaking their operation while maintaining some level of + /// sanity with the number of devices that people can have. This will prevent anyone + /// else from crossing the limit. + pub max_session_threshold: Option, /// Whether we should automatically choose the least recently used devices /// to remove when the [`Self::hard_limit`] is reached; in order to /// allow the new login to continue. @@ -185,8 +196,14 @@ pub struct SessionLimitConfig { /// and you want to avoid breaking their operation while maintaining some /// level of sanity with the number of devices that people can have. /// + /// Removing devices is a non-trivial tasks for some homeservers to tackle and can + /// cause lots of device list changes, `/sync`, federation, and replication traffic. + /// Consider using [`max_session_threshold`] to limit the size of accounts that are + /// acted upon. + /// /// [`hard_limit`]: Self::hard_limit /// [`dangerous_hard_limit_eviction`]: Self::dangerous_hard_limit_eviction + /// [`max_session_threshold`]: Self::max_session_threshold #[serde(default = "default_false")] pub dangerous_hard_limit_eviction: bool, } From ce3cfe73d9b0f055fd66e95be894b97f9f9d43b7 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 8 May 2026 13:45:08 -0500 Subject: [PATCH 02/13] Pass through `max_session_threshold` --- crates/cli/src/util.rs | 2 ++ crates/config/src/sections/experimental.rs | 2 +- crates/data-model/src/site_config.rs | 1 + crates/handlers/src/compat/login.rs | 4 ++++ 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/cli/src/util.rs b/crates/cli/src/util.rs index c3a8412f9..56b6a98f5 100644 --- a/crates/cli/src/util.rs +++ b/crates/cli/src/util.rs @@ -156,6 +156,7 @@ pub async fn policy_factory_from_config( .map(|c| SessionLimitConfig { soft_limit: c.soft_limit, hard_limit: c.hard_limit, + max_session_threshold: c.max_session_threshold, dangerous_hard_limit_eviction: c.dangerous_hard_limit_eviction, }); @@ -246,6 +247,7 @@ pub fn site_config_from_config( .map(|c| SessionLimitConfig { soft_limit: c.soft_limit, hard_limit: c.hard_limit, + max_session_threshold: c.max_session_threshold, dangerous_hard_limit_eviction: c.dangerous_hard_limit_eviction, }), }) diff --git a/crates/config/src/sections/experimental.rs b/crates/config/src/sections/experimental.rs index 4a8d61a86..d006c1f79 100644 --- a/crates/config/src/sections/experimental.rs +++ b/crates/config/src/sections/experimental.rs @@ -196,7 +196,7 @@ pub struct SessionLimitConfig { /// and you want to avoid breaking their operation while maintaining some /// level of sanity with the number of devices that people can have. /// - /// Removing devices is a non-trivial tasks for some homeservers to tackle and can + /// Removing devices is a non-trivial task for some homeservers to tackle and can /// cause lots of device list changes, `/sync`, federation, and replication traffic. /// Consider using [`max_session_threshold`] to limit the size of accounts that are /// acted upon. diff --git a/crates/data-model/src/site_config.rs b/crates/data-model/src/site_config.rs index 9d164c639..bd8336daa 100644 --- a/crates/data-model/src/site_config.rs +++ b/crates/data-model/src/site_config.rs @@ -44,6 +44,7 @@ pub struct SessionExpirationConfig { pub struct SessionLimitConfig { pub soft_limit: NonZeroU64, pub hard_limit: NonZeroU64, + pub max_session_threshold: Option, pub dangerous_hard_limit_eviction: bool, } diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index e36e57a5b..24335fb4e 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -1759,6 +1759,7 @@ mod tests { soft_limit: NonZeroU64::new(1).unwrap(), // Some arbitrary high value (more than we login) hard_limit: NonZeroU64::new(5).unwrap(), + max_session_threshold: None, dangerous_hard_limit_eviction: false, }), ..test_site_config() @@ -1809,6 +1810,7 @@ mod tests { soft_limit: NonZeroU64::new(1).unwrap(), // Lowest non-zero value so we don't have to login a bunch hard_limit: NonZeroU64::new(1).unwrap(), + max_session_threshold: None, dangerous_hard_limit_eviction: false, }), ..test_site_config() @@ -1873,6 +1875,7 @@ mod tests { soft_limit: NonZeroU64::new(1).unwrap(), // Must be at least 2 when `dangerous_hard_limit_eviction` hard_limit: NonZeroU64::new(2).unwrap(), + max_session_threshold: None, // Option under test dangerous_hard_limit_eviction: true, }), @@ -2040,6 +2043,7 @@ mod tests { soft_limit: NonZeroU64::new(1).unwrap(), // Must be at least 2 when `dangerous_hard_limit_eviction` hard_limit: NonZeroU64::new(2).unwrap(), + max_session_threshold: None, // Option under test dangerous_hard_limit_eviction: true, }), From 984f836fdeb99b14a5fa32318ee91c74cc4baa90 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 8 May 2026 13:45:37 -0500 Subject: [PATCH 03/13] Update policies --- .../authorization_grant.rego | 17 ++++++++++++++++ policies/compat_login/compat_login.rego | 20 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/policies/authorization_grant/authorization_grant.rego b/policies/authorization_grant/authorization_grant.rego index 34334d36b..6b8a8bfe0 100644 --- a/policies/authorization_grant/authorization_grant.rego +++ b/policies/authorization_grant/authorization_grant.rego @@ -167,9 +167,26 @@ violation contains { # Only apply if it's a user logging in (who therefore has countable sessions) input.session_counts != null + # Only apply limits to accounts under the threshold (if configured) + passes_session_threshold + # For OAuth 2 login, a violation occurs when the soft limit has already been # reached or exceeded. # We use the soft limit because the user will be able to interactively remove # sessions to return under the limit. data.session_limit.soft_limit <= input.session_counts.total } + +# The session limits only apply to accounts within the `max_session_threshold`. +# +# True if the `max_session_threshold` isn't configured or <= `max_session_threshold`. +passes_session_threshold if { + # If no `session_limit` configured, automatically passes + not data.session_limit +} else if { + # If no `max_session_threshold` configured, automatically passes + not data.session_limit.max_session_threshold +} else if { + # Otherwise, check whether the total number of sessions is under the threshold + input.session_counts.total <= data.session_limit.max_session_threshold +} diff --git a/policies/compat_login/compat_login.rego b/policies/compat_login/compat_login.rego index 5f1825268..127f5ef14 100644 --- a/policies/compat_login/compat_login.rego +++ b/policies/compat_login/compat_login.rego @@ -42,6 +42,9 @@ violation contains { # (As then this login is not actually increasing the number of devices) not input.session_replaced + # Only apply limits to accounts under the threshold (if configured) + passes_session_threshold + # For web-based 'compat SSO' login, a violation occurs when the soft limit has already been # reached or exceeded. # We use the soft limit because the user will be able to interactively remove @@ -66,6 +69,9 @@ violation contains { # (As then this login is not actually increasing the number of devices) not input.session_replaced + # Only apply limits to accounts under the threshold (if configured) + passes_session_threshold + # For `m.login.password` login, a violation occurs when the hard limit has already been # reached or exceeded. # We don't use the soft limit because the user won't be able to interactively remove @@ -78,3 +84,17 @@ is_interactive if { # `m.login.password` and `m.login.token` (including the finalisation of an SSO login) are not input.login.type == "m.login.sso" } + +# The session limits only apply to accounts within the `max_session_threshold`. +# +# True if the `max_session_threshold` isn't configured or <= `max_session_threshold`. +passes_session_threshold if { + # If no `session_limit` configured, automatically passes + not data.session_limit +} else if { + # If no `max_session_threshold` configured, automatically passes + not data.session_limit.max_session_threshold +} else if { + # Otherwise, check whether the total number of sessions is under the threshold + input.session_counts.total <= data.session_limit.max_session_threshold +} From 31653438b573a5b826098857e96276b34736a9ef Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 8 May 2026 14:00:35 -0500 Subject: [PATCH 04/13] Add policy tests --- .../authorization_grant_test.rego | 34 +++++++++++++++++ policies/compat_login/compat_login_test.rego | 38 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/policies/authorization_grant/authorization_grant_test.rego b/policies/authorization_grant/authorization_grant_test.rego index 4bf6442f5..cb3acd39a 100644 --- a/policies/authorization_grant/authorization_grant_test.rego +++ b/policies/authorization_grant/authorization_grant_test.rego @@ -302,3 +302,37 @@ test_session_limiting_no_limit if { result.allow result.need_to_remove_sessions == 0 } + +test_session_limiting_under_max_session_threshold if { + result := { + "allow": authorization_grant.allow, + "need_to_remove_sessions": need_to_remove_sessions(authorization_grant.violation), + } with input.user as user + # Some value above `soft_limit` but below `max_session_threshold` + with input.session_counts as {"total": 42} + with data.session_limit as { + "soft_limit": 32, + "hard_limit": 64, + # The main thing we're trying to test + "max_session_threshold": 64 + } + not result.allow + result.need_to_remove_sessions == 11 +} + +test_no_session_limiting_past_max_session_threshold if { + result := { + "allow": authorization_grant.allow, + "need_to_remove_sessions": need_to_remove_sessions(authorization_grant.violation), + } with input.user as user + # Some value above the limits and `max_session_threshold` + with input.session_counts as {"total": 128} + with data.session_limit as { + "soft_limit": 32, + "hard_limit": 64, + # The main thing we're trying to test + "max_session_threshold": 64 + } + result.allow + result.need_to_remove_sessions == 0 +} diff --git a/policies/compat_login/compat_login_test.rego b/policies/compat_login/compat_login_test.rego index 339b1b684..7d4a7f5b0 100644 --- a/policies/compat_login/compat_login_test.rego +++ b/policies/compat_login/compat_login_test.rego @@ -198,3 +198,41 @@ test_no_session_limiting_password_upon_replacement if { result.allow result.need_to_remove_sessions == 0 } + +test_session_limiting_under_max_session_threshold if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user + # Some value above `soft_limit` but below `max_session_threshold` + with input.session_counts as {"total": 42} + with input.login as {"type": "m.login.sso"} + with input.session_replaced as false + with data.session_limit as { + "soft_limit": 32, + "hard_limit": 64, + # The main thing we're trying to test + "max_session_threshold": 64 + } + not result.allow + result.need_to_remove_sessions == 11 +} + +test_no_session_limiting_past_max_session_threshold if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user + # Some value above the limits and `max_session_threshold` + with input.session_counts as {"total": 128} + with input.login as {"type": "m.login.sso"} + with input.session_replaced as false + with data.session_limit as { + "soft_limit": 32, + "hard_limit": 64, + # The main thing we're trying to test + "max_session_threshold": 64 + } + result.allow + result.need_to_remove_sessions == 0 +} From f31c7417247230fe5c0ba2feebbefec6dc7ca8e8 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 8 May 2026 14:41:26 -0500 Subject: [PATCH 05/13] Format policies --- .../authorization_grant/authorization_grant_test.rego | 10 ++++------ policies/compat_login/compat_login_test.rego | 10 ++++------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/policies/authorization_grant/authorization_grant_test.rego b/policies/authorization_grant/authorization_grant_test.rego index cb3acd39a..5186e6c3e 100644 --- a/policies/authorization_grant/authorization_grant_test.rego +++ b/policies/authorization_grant/authorization_grant_test.rego @@ -308,13 +308,12 @@ test_session_limiting_under_max_session_threshold if { "allow": authorization_grant.allow, "need_to_remove_sessions": need_to_remove_sessions(authorization_grant.violation), } with input.user as user - # Some value above `soft_limit` but below `max_session_threshold` - with input.session_counts as {"total": 42} + with input.session_counts as {"total": 42} # Some value above `soft_limit` but below `max_session_threshold` with data.session_limit as { "soft_limit": 32, "hard_limit": 64, # The main thing we're trying to test - "max_session_threshold": 64 + "max_session_threshold": 64, } not result.allow result.need_to_remove_sessions == 11 @@ -325,13 +324,12 @@ test_no_session_limiting_past_max_session_threshold if { "allow": authorization_grant.allow, "need_to_remove_sessions": need_to_remove_sessions(authorization_grant.violation), } with input.user as user - # Some value above the limits and `max_session_threshold` - with input.session_counts as {"total": 128} + with input.session_counts as {"total": 128} # Some value above the limits and `max_session_threshold` with data.session_limit as { "soft_limit": 32, "hard_limit": 64, # The main thing we're trying to test - "max_session_threshold": 64 + "max_session_threshold": 64, } result.allow result.need_to_remove_sessions == 0 diff --git a/policies/compat_login/compat_login_test.rego b/policies/compat_login/compat_login_test.rego index 7d4a7f5b0..ca6d7290a 100644 --- a/policies/compat_login/compat_login_test.rego +++ b/policies/compat_login/compat_login_test.rego @@ -204,15 +204,14 @@ test_session_limiting_under_max_session_threshold if { "allow": compat_login.allow, "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), } with input.user as user - # Some value above `soft_limit` but below `max_session_threshold` - with input.session_counts as {"total": 42} + with input.session_counts as {"total": 42} # Some value above `soft_limit` but below `max_session_threshold` with input.login as {"type": "m.login.sso"} with input.session_replaced as false with data.session_limit as { "soft_limit": 32, "hard_limit": 64, # The main thing we're trying to test - "max_session_threshold": 64 + "max_session_threshold": 64, } not result.allow result.need_to_remove_sessions == 11 @@ -223,15 +222,14 @@ test_no_session_limiting_past_max_session_threshold if { "allow": compat_login.allow, "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), } with input.user as user - # Some value above the limits and `max_session_threshold` - with input.session_counts as {"total": 128} + with input.session_counts as {"total": 128} # Some value above the limits and `max_session_threshold` with input.login as {"type": "m.login.sso"} with input.session_replaced as false with data.session_limit as { "soft_limit": 32, "hard_limit": 64, # The main thing we're trying to test - "max_session_threshold": 64 + "max_session_threshold": 64, } result.allow result.need_to_remove_sessions == 0 From 600841bdf279bbb6edd8eacc570efdb907276cf1 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 8 May 2026 14:44:08 -0500 Subject: [PATCH 06/13] Add Rust tests --- crates/handlers/src/compat/login.rs | 103 +++++++++++++++++++++++++--- 1 file changed, 94 insertions(+), 9 deletions(-) diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index 24335fb4e..d6c28cdd7 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -1742,13 +1742,21 @@ mod tests { token } - /// Test that the `soft_limit` is not enforced for compat login. - /// - /// `soft_limit` is for when we allow the user to remove devices in - /// interactive contexts. With the compatibility login API, there is no - /// opportunity for us to present a web UI. + /// Test that `soft_limit` works against interative login (like the `m.login.sso` + /// compat Matrix login flow) #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_soft_limit_does_not_affect_compat_login(pool: PgPool) { + async fn test_session_soft_limit_interactive_login(pool: PgPool) { + // TODO + } + + /// Test that the `soft_limit` is not enforced for non-interactive login + /// (like the `m.login.password` compat Matrix login flow). + /// + /// `soft_limit` is for when we allow the user to remove devices in interactive + /// contexts. If someone uses the `m.login.password` compatibility login API, there + /// is no opportunity for us to present a web UI. + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_session_soft_limit_does_not_affect_non_interactive_login(pool: PgPool) { setup(); let state = TestState::from_pool_with_site_config( pool, @@ -1800,7 +1808,7 @@ mod tests { /// Test that the `hard_limit` prevents more sessions #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_hard_limit_compat_login(pool: PgPool) { + async fn test_session_hard_limit_compat_login(pool: PgPool) { setup(); let state = TestState::from_pool_with_site_config( pool, @@ -1862,10 +1870,87 @@ mod tests { ); } + /// Test that session limits are enforced for anyone who is *under* the + /// `max_session_threshold` + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_session_limit_under_max_session_threshold(pool: PgPool) { + // TODO + } + + /// Test that session limits are not enforced (logins are allowed) for anyone who is + /// already *past* the `max_session_threshold` + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_session_limit_past_max_session_threshold(pool: PgPool) { + setup(); + let state = TestState::from_pool_with_site_config( + pool.clone(), + SiteConfig { + // Setup an account with a few sessions before we add a `session_limit` + session_limit: None, + ..test_site_config() + }, + ) + .await + .unwrap(); + + // Lowest non-zero value so we don't have to login a bunch + let upcoming_hard_limit = 1; + + // Keep logging in to add more sessions. We want to be past the + // `hard_limit`/`max_session_threshold` + let _user = user_with_password(&state, "alice", "password", false).await; + let password_login_json = serde_json::json!({ + "type": "m.login.password", + "identifier": { + "type": "m.id.user", + "user": "alice", + }, + "password": "password", + }); + #[allow(clippy::range_plus_one)] + for _ in 0..(upcoming_hard_limit + 1) { + let request = Request::post("/_matrix/client/v3/login") + .json(serde_json::json!(password_login_json.clone())); + let response = state.request(request.clone()).await; + response.assert_status(StatusCode::OK); + } + + // Update the app state to configure a `max_session_threshold` + // FIXME: This creates an entirely new state + let state = TestState::from_pool_with_site_config( + pool, + SiteConfig { + session_limit: Some(SessionLimitConfig { + soft_limit: NonZeroU64::new(upcoming_hard_limit).unwrap(), + hard_limit: NonZeroU64::new(upcoming_hard_limit).unwrap(), + // The main thing we're trying to test + max_session_threshold: Some(NonZeroU64::new(upcoming_hard_limit).unwrap()), + dangerous_hard_limit_eviction: false, + }), + ..test_site_config() + }, + ) + .await + .unwrap(); + + let _session_limit_config = state + .site_config + .session_limit + .as_ref() + .expect("Expected `session_limit` to be configured at this point in the test"); + + // Since we're already above `max_session_threshold`, the `session_limit` won't + // stop us from adding another session. + let request = Request::post("/_matrix/client/v3/login") + .json(serde_json::json!(password_login_json.clone())); + let response = state.request(request.clone()).await; + response.assert_status(StatusCode::OK); + } + /// Test that the `dangerous_hard_limit_eviction` will automatically drop /// old sessions when we go over the limit #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_dangerous_hard_limit_eviction_old_compat_login(pool: PgPool) { + async fn test_session_dangerous_hard_limit_eviction_old_compat_login(pool: PgPool) { setup(); let state = TestState::from_pool_with_site_config( pool, @@ -2033,7 +2118,7 @@ mod tests { /// the oldest sessions when we go over the limit even if all of the /// sessions are recent. #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_dangerous_hard_limit_eviction_recent_compat_login(pool: PgPool) { + async fn test_session_dangerous_hard_limit_eviction_recent_compat_login(pool: PgPool) { setup(); let state = TestState::from_pool_with_site_config( pool, From 1ef9a8f87b8b446fa977070eb1b7c811dfb7718d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 12 May 2026 17:24:59 -0500 Subject: [PATCH 07/13] Adjust tests --- crates/handlers/src/compat/login.rs | 122 +++++++++++++++++++--------- 1 file changed, 84 insertions(+), 38 deletions(-) diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index d6c28cdd7..01e05a836 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -1834,31 +1834,25 @@ mod tests { .expect("Expected `session_limit` configured for this test"); let _user = user_with_password(&state, "alice", "password", false).await; - - // Keep logging in to add more sessions, up to the `hard_limit` - #[allow(clippy::range_plus_one)] - for _ in 0..session_limit_config.hard_limit.get() { - 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::OK); - } - - // One more login will tip us over the `hard_limit` - let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({ + let password_login_json = serde_json::json!({ "type": "m.login.password", "identifier": { "type": "m.id.user", "user": "alice", }, "password": "password", - })); + }); + + // Keep logging in to add more sessions, up to the `hard_limit` + for _ in 0..session_limit_config.hard_limit.get() { + let request = + Request::post("/_matrix/client/v3/login").json(password_login_json.clone()); + let response = state.request(request.clone()).await; + response.assert_status(StatusCode::OK); + } + + // One more login will tip us over the `hard_limit` + let request = Request::post("/_matrix/client/v3/login").json(password_login_json); let response = state.request(request.clone()).await; response.assert_status(StatusCode::FORBIDDEN); let body: serde_json::Value = response.json(); @@ -1874,7 +1868,63 @@ mod tests { /// `max_session_threshold` #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] async fn test_session_limit_under_max_session_threshold(pool: PgPool) { - // TODO + setup(); + let state = TestState::from_pool_with_site_config( + pool, + SiteConfig { + session_limit: Some(SessionLimitConfig { + // Lowest non-zero value so we don't have to login a bunch + soft_limit: NonZeroU64::new(1).unwrap(), + hard_limit: NonZeroU64::new(1).unwrap(), + // The main thing we're trying to test + max_session_threshold: Some(NonZeroU64::new(1).unwrap()), + dangerous_hard_limit_eviction: false, + }), + ..test_site_config() + }, + ) + .await + .unwrap(); + + let session_limit_config = state + .site_config + .session_limit + .as_ref() + .expect("Expected `session_limit` configured for this test"); + // Make sure this is configured as its the main differentiator we're trying to test + session_limit_config.max_session_threshold.as_ref().expect("Expected `session_limit.max_session_threshold` to be configured at this point in the test"); + + let _user = user_with_password(&state, "alice", "password", false).await; + let password_login_json = serde_json::json!({ + "type": "m.login.password", + "identifier": { + "type": "m.id.user", + "user": "alice", + }, + "password": "password", + }); + + // Keep logging in to add more sessions, up to the `hard_limit` + for _ in 0..session_limit_config.hard_limit.get() { + let request = + Request::post("/_matrix/client/v3/login").json(password_login_json.clone()); + let response = state.request(request.clone()).await; + response.assert_status(StatusCode::OK); + } + + // One more login will tip us over the `hard_limit` + // + // Session limits are enforced because we're <= `max_session_threshold` + let request = Request::post("/_matrix/client/v3/login").json(password_login_json); + let response = state.request(request.clone()).await; + response.assert_status(StatusCode::FORBIDDEN); + let body: serde_json::Value = response.json(); + assert_eq!( + body.get("errcode") + .expect("Expected errror response to include an `errcode`"), + "M_FORBIDDEN", + "Expected `errcode` to be `M_FORBIDDEN`" + ); } /// Test that session limits are not enforced (logins are allowed) for anyone who is @@ -1882,7 +1932,7 @@ mod tests { #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] async fn test_session_limit_past_max_session_threshold(pool: PgPool) { setup(); - let state = TestState::from_pool_with_site_config( + let mut state = TestState::from_pool_with_site_config( pool.clone(), SiteConfig { // Setup an account with a few sessions before we add a `session_limit` @@ -1896,6 +1946,8 @@ mod tests { // Lowest non-zero value so we don't have to login a bunch let upcoming_hard_limit = 1; + // Setup a user that has more sessions than the `max_session_threshold`. + // // Keep logging in to add more sessions. We want to be past the // `hard_limit`/`max_session_threshold` let _user = user_with_password(&state, "alice", "password", false).await; @@ -1916,28 +1968,22 @@ mod tests { } // Update the app state to configure a `max_session_threshold` - // FIXME: This creates an entirely new state - let state = TestState::from_pool_with_site_config( - pool, - SiteConfig { - session_limit: Some(SessionLimitConfig { - soft_limit: NonZeroU64::new(upcoming_hard_limit).unwrap(), - hard_limit: NonZeroU64::new(upcoming_hard_limit).unwrap(), - // The main thing we're trying to test - max_session_threshold: Some(NonZeroU64::new(upcoming_hard_limit).unwrap()), - dangerous_hard_limit_eviction: false, - }), - ..test_site_config() - }, - ) - .await - .unwrap(); + state.site_config.session_limit = Some(SessionLimitConfig { + soft_limit: NonZeroU64::new(upcoming_hard_limit).unwrap(), + hard_limit: NonZeroU64::new(upcoming_hard_limit).unwrap(), + // The main thing we're trying to test + max_session_threshold: Some(NonZeroU64::new(upcoming_hard_limit).unwrap()), + dangerous_hard_limit_eviction: false, + }); + let state = state.reset().await; - let _session_limit_config = state + let session_limit_config = state .site_config .session_limit .as_ref() .expect("Expected `session_limit` to be configured at this point in the test"); + // Make sure this is configured as its the main differentiator we're trying to test + session_limit_config.max_session_threshold.as_ref().expect("Expected `session_limit.max_session_threshold` to be configured at this point in the test"); // Since we're already above `max_session_threshold`, the `session_limit` won't // stop us from adding another session. From e28b917290ec5288cc0bf1f29f2301a2591bce24 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 12 May 2026 18:39:26 -0500 Subject: [PATCH 08/13] Remove test for now --- crates/handlers/src/compat/login.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index 01e05a836..b1b455be8 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -1742,13 +1742,6 @@ mod tests { token } - /// Test that `soft_limit` works against interative login (like the `m.login.sso` - /// compat Matrix login flow) - #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_session_soft_limit_interactive_login(pool: PgPool) { - // TODO - } - /// Test that the `soft_limit` is not enforced for non-interactive login /// (like the `m.login.password` compat Matrix login flow). /// @@ -1791,6 +1784,8 @@ mod tests { let _user = user_with_password(&state, "alice", "password", false).await; // Keep logging in to add more sessions, more than the `soft_limit` + // + // We're using `m.login.password` login flow which is non-interactive #[allow(clippy::range_plus_one)] for _ in 0..(session_limit_config.soft_limit.get() + 1) { let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({ From a7ebe900cc869296bfcc119b184b803b7858b772 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 13 May 2026 15:40:49 -0500 Subject: [PATCH 09/13] Fix `reset` to retain mock homeserver im-memory state --- crates/handlers/src/compat/login.rs | 6 +++--- crates/handlers/src/test_utils.rs | 17 +++++++++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index b1b455be8..a08b59b86 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -1269,8 +1269,8 @@ mod tests { } "###); - // Reset the state, to reset rate limits - let state = state.reset().await; + // Restart, to reset rate limits + let state = state.restart().await; // Try to login with a wrong password. let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({ @@ -1970,7 +1970,7 @@ mod tests { max_session_threshold: Some(NonZeroU64::new(upcoming_hard_limit).unwrap()), dangerous_hard_limit_eviction: false, }); - let state = state.reset().await; + let state = state.restart().await; let session_limit_config = state .site_config diff --git a/crates/handlers/src/test_utils.rs b/crates/handlers/src/test_utils.rs index 619d995a7..83a588d06 100644 --- a/crates/handlers/src/test_utils.rs +++ b/crates/handlers/src/test_utils.rs @@ -299,12 +299,18 @@ impl TestState { queue.process_all_jobs_in_tests().await.unwrap(); } - /// Reset the test utils to a fresh state, with the same configuration. - pub async fn reset(self) -> Self { + /// Restart the app with the same configuration. + /// + /// To change the config, mutate `TestState.site_config` before calling this + /// function. + pub async fn restart(self) -> Self { let site_config = self.site_config.clone(); let pool = self.repository_factory.pool(); let task_tracker = self.task_tracker.clone(); + // Retain the homeserver connection so we keep the in-memory state + let homeserver_connection = self.homeserver_connection.clone(); + // This should trigger the cancellation drop guard drop(self); @@ -312,9 +318,12 @@ impl TestState { task_tracker.close(); task_tracker.wait().await; - Self::from_pool_with_site_config(pool, site_config) + let mut state = Self::from_pool_with_site_config(pool, site_config) .await - .unwrap() + .unwrap(); + // Restore the homeserver connection so we keep the in-memory state + state.homeserver_connection = homeserver_connection; + state } pub async fn request(&self, request: Request) -> Response From b635465cfab5532d76e26be18d5b8dbc1e73f8e0 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 13 May 2026 19:06:20 -0500 Subject: [PATCH 10/13] Fix null vs undefined difference causing weird results in policies --- .../authorization_grant.rego | 10 ++- policies/compat_login/compat_login.rego | 10 ++- policies/compat_login/compat_login_test.rego | 66 +++++++++++++++++++ 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/policies/authorization_grant/authorization_grant.rego b/policies/authorization_grant/authorization_grant.rego index 6b8a8bfe0..f02fdaac4 100644 --- a/policies/authorization_grant/authorization_grant.rego +++ b/policies/authorization_grant/authorization_grant.rego @@ -181,11 +181,17 @@ violation contains { # # True if the `max_session_threshold` isn't configured or <= `max_session_threshold`. passes_session_threshold if { - # If no `session_limit` configured, automatically passes + # If no `session_limit` configured, automatically passes (undefined) not data.session_limit } else if { - # If no `max_session_threshold` configured, automatically passes + # If no `session_limit` configured, automatically passes (null) + data.session_limit == null +} else if { + # If no `max_session_threshold` configured, automatically passes (undefined) not data.session_limit.max_session_threshold +} else if { + # If no `max_session_threshold` configured, automatically passes (null) + data.session_limit.max_session_threshold == null } else if { # Otherwise, check whether the total number of sessions is under the threshold input.session_counts.total <= data.session_limit.max_session_threshold diff --git a/policies/compat_login/compat_login.rego b/policies/compat_login/compat_login.rego index 5c50a16bf..5ba3510b1 100644 --- a/policies/compat_login/compat_login.rego +++ b/policies/compat_login/compat_login.rego @@ -89,11 +89,17 @@ is_interactive if { # # True if the `max_session_threshold` isn't configured or <= `max_session_threshold`. passes_session_threshold if { - # If no `session_limit` configured, automatically passes + # If no `session_limit` configured, automatically passes (undefined) not data.session_limit } else if { - # If no `max_session_threshold` configured, automatically passes + # If no `session_limit` configured, automatically passes (null) + data.session_limit == null +} else if { + # If no `max_session_threshold` configured, automatically passes (undefined) not data.session_limit.max_session_threshold +} else if { + # If no `max_session_threshold` configured, automatically passes (null) + data.session_limit.max_session_threshold == null } else if { # Otherwise, check whether the total number of sessions is under the threshold input.session_counts.total <= data.session_limit.max_session_threshold diff --git a/policies/compat_login/compat_login_test.rego b/policies/compat_login/compat_login_test.rego index ca6d7290a..335e1171d 100644 --- a/policies/compat_login/compat_login_test.rego +++ b/policies/compat_login/compat_login_test.rego @@ -21,6 +21,34 @@ need_to_remove_sessions(violations) := need if { # Tests session limiting when using (the interactive part of) `m.login.sso` # (interactive, therefore `soft_limit` applies) # ========================================================================= +test_no_session_limiting_undefined if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user + with input.session_counts as {"total": 1} + with input.login as {"type": "m.login.sso"} + with input.session_replaced as false + # The main thing we're trying to test (undefined) + # with data.session_limit as null + result.allow + result.need_to_remove_sessions == 0 +} + +test_no_session_limiting_null if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user + with input.session_counts as {"total": 1} + with input.login as {"type": "m.login.sso"} + with input.session_replaced as false + # The main thing we're trying to test (null) + with data.session_limit as null + result.allow + result.need_to_remove_sessions == 0 +} + test_session_limiting_sso_under_limit if { result := { "allow": compat_login.allow, @@ -199,6 +227,44 @@ test_no_session_limiting_password_upon_replacement if { result.need_to_remove_sessions == 0 } +# Testing null vs undefined +test_session_limiting_null_max_session_threshold_under_limit if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user + with input.session_counts as {"total": 3} # Some value below `soft_limit` + with input.login as {"type": "m.login.sso"} + with input.session_replaced as false + with data.session_limit as { + "soft_limit": 32, + "hard_limit": 64, + # The main thing we're trying to test + "max_session_threshold": null, + } + result.allow + result.need_to_remove_sessions == 0 +} + +# Testing null vs undefined +test_session_limiting_null_max_session_threshold_over_limit if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user + with input.session_counts as {"total": 42} # Some value above `soft_limit` + with input.login as {"type": "m.login.sso"} + with input.session_replaced as false + with data.session_limit as { + "soft_limit": 32, + "hard_limit": 64, + # The main thing we're trying to test + "max_session_threshold": null, + } + not result.allow + result.need_to_remove_sessions == 11 +} + test_session_limiting_under_max_session_threshold if { result := { "allow": compat_login.allow, From 0cd6bc72780c2df62b8b55b24a0b734f53ca4220 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 14 May 2026 15:16:24 -0500 Subject: [PATCH 11/13] Fix lints --- crates/config/src/sections/experimental.rs | 24 ++++++++++---------- crates/handlers/src/compat/login.rs | 17 ++++++++------ policies/compat_login/compat_login_test.rego | 6 ++--- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/crates/config/src/sections/experimental.rs b/crates/config/src/sections/experimental.rs index 00c9d6c0f..841357e14 100644 --- a/crates/config/src/sections/experimental.rs +++ b/crates/config/src/sections/experimental.rs @@ -163,16 +163,16 @@ pub struct SessionLimitConfig { /// /// [`dangerous_hard_limit_eviction`]: Self::dangerous_hard_limit_eviction pub hard_limit: NonZeroU64, - /// When set, only accounts with <= `max_session_threshold` sessions have the - /// session limits applied. + /// When set, only accounts with <= `max_session_threshold` sessions have + /// the session limits applied. /// - /// This is most applicable in scenarios where your homeserver has many legacy - /// bots/scripts that login over and over (which ideally should be using [personal - /// access + /// This is most applicable in scenarios where your homeserver has many + /// legacy bots/scripts that login over and over (which ideally should + /// be using [personal access /// tokens](https://github.com/element-hq/matrix-authentication-service/issues/4492)) - /// and you want to avoid breaking their operation while maintaining some level of - /// sanity with the number of devices that people can have. This will prevent anyone - /// else from crossing the limit. + /// and you want to avoid breaking their operation while maintaining some + /// level of sanity with the number of devices that people can have. + /// This will prevent anyone else from crossing the limit. pub max_session_threshold: Option, /// Whether we should automatically choose the least recently used devices /// to remove when the [`Self::hard_limit`] is reached; in order to @@ -197,10 +197,10 @@ pub struct SessionLimitConfig { /// and you want to avoid breaking their operation while maintaining some /// level of sanity with the number of devices that people can have. /// - /// Removing devices is a non-trivial task for some homeservers to tackle and can - /// cause lots of device list changes, `/sync`, federation, and replication traffic. - /// Consider using [`max_session_threshold`] to limit the size of accounts that are - /// acted upon. + /// Removing devices is a non-trivial task for some homeservers to tackle + /// and can cause lots of device list changes, `/sync`, federation, and + /// replication traffic. Consider using [`max_session_threshold`] to + /// limit the size of accounts that are acted upon. /// /// [`hard_limit`]: Self::hard_limit /// [`dangerous_hard_limit_eviction`]: Self::dangerous_hard_limit_eviction diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index a08b59b86..f9c9535fa 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -1745,9 +1745,10 @@ mod tests { /// Test that the `soft_limit` is not enforced for non-interactive login /// (like the `m.login.password` compat Matrix login flow). /// - /// `soft_limit` is for when we allow the user to remove devices in interactive - /// contexts. If someone uses the `m.login.password` compatibility login API, there - /// is no opportunity for us to present a web UI. + /// `soft_limit` is for when we allow the user to remove devices in + /// interactive contexts. If someone uses the `m.login.password` + /// compatibility login API, there is no opportunity for us to present a + /// web UI. #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] async fn test_session_soft_limit_does_not_affect_non_interactive_login(pool: PgPool) { setup(); @@ -1886,7 +1887,8 @@ mod tests { .session_limit .as_ref() .expect("Expected `session_limit` configured for this test"); - // Make sure this is configured as its the main differentiator we're trying to test + // Make sure this is configured as its the main differentiator we're trying to + // test session_limit_config.max_session_threshold.as_ref().expect("Expected `session_limit.max_session_threshold` to be configured at this point in the test"); let _user = user_with_password(&state, "alice", "password", false).await; @@ -1922,8 +1924,8 @@ mod tests { ); } - /// Test that session limits are not enforced (logins are allowed) for anyone who is - /// already *past* the `max_session_threshold` + /// Test that session limits are not enforced (logins are allowed) for + /// anyone who is already *past* the `max_session_threshold` #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] async fn test_session_limit_past_max_session_threshold(pool: PgPool) { setup(); @@ -1977,7 +1979,8 @@ mod tests { .session_limit .as_ref() .expect("Expected `session_limit` to be configured at this point in the test"); - // Make sure this is configured as its the main differentiator we're trying to test + // Make sure this is configured as its the main differentiator we're trying to + // test session_limit_config.max_session_threshold.as_ref().expect("Expected `session_limit.max_session_threshold` to be configured at this point in the test"); // Since we're already above `max_session_threshold`, the `session_limit` won't diff --git a/policies/compat_login/compat_login_test.rego b/policies/compat_login/compat_login_test.rego index 335e1171d..3f8197b07 100644 --- a/policies/compat_login/compat_login_test.rego +++ b/policies/compat_login/compat_login_test.rego @@ -29,8 +29,7 @@ test_no_session_limiting_undefined if { with input.session_counts as {"total": 1} with input.login as {"type": "m.login.sso"} with input.session_replaced as false - # The main thing we're trying to test (undefined) - # with data.session_limit as null + with input._noop as null # The main thing we're trying to test (`data.session_limit` as undefined) result.allow result.need_to_remove_sessions == 0 } @@ -43,8 +42,7 @@ test_no_session_limiting_null if { with input.session_counts as {"total": 1} with input.login as {"type": "m.login.sso"} with input.session_replaced as false - # The main thing we're trying to test (null) - with data.session_limit as null + with data.session_limit as null # The main thing we're trying to test (null) result.allow result.need_to_remove_sessions == 0 } From e61653cd2da20bbb63dc25e1e4ee1d0780d980ef Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 14 May 2026 15:27:36 -0500 Subject: [PATCH 12/13] Automatic updates --- docs/config.schema.json | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/config.schema.json b/docs/config.schema.json index 3af8b5ace..6b1f57d5b 100644 --- a/docs/config.schema.json +++ b/docs/config.schema.json @@ -2927,8 +2927,17 @@ "format": "uint64", "minimum": 1 }, + "max_session_threshold": { + "description": "When set, only accounts with <= `max_session_threshold` sessions have\n the session limits applied.\n\n This is most applicable in scenarios where your homeserver has many\n legacy bots/scripts that login over and over (which ideally should\n be using [personal access\n tokens](https://github.com/element-hq/matrix-authentication-service/issues/4492))\n and you want to avoid breaking their operation while maintaining some\n level of sanity with the number of devices that people can have.\n This will prevent anyone else from crossing the limit.", + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 1 + }, "dangerous_hard_limit_eviction": { - "description": "Whether we should automatically choose the least recently used devices\n to remove when the [`Self::hard_limit`] is reached; in order to\n allow the new login to continue.\n\n Disabled by default\n\n WARNING: Removing sessions is a potentially damaging operation. Any\n end-to-end encrypted history on the device will be lost and can only\n be recovered if you have another verified active device or have a\n recovery key setup.\n\n When using [`dangerous_hard_limit_eviction`], the [`hard_limit`] must be\n at least 2 to avoid catastrophically losing encrypted history and\n digital identity in pathological cases. Keep in mind this is a bare\n minimum restriction and you can still run into trouble.\n\n This is most applicable in scenarios where your homeserver has many\n legacy bots/scripts that login over and over (which ideally should\n be using [personal access\n tokens](https://github.com/element-hq/matrix-authentication-service/issues/4492))\n and you want to avoid breaking their operation while maintaining some\n level of sanity with the number of devices that people can have.\n\n [`hard_limit`]: Self::hard_limit\n [`dangerous_hard_limit_eviction`]: Self::dangerous_hard_limit_eviction", + "description": "Whether we should automatically choose the least recently used devices\n to remove when the [`Self::hard_limit`] is reached; in order to\n allow the new login to continue.\n\n Disabled by default\n\n WARNING: Removing sessions is a potentially damaging operation. Any\n end-to-end encrypted history on the device will be lost and can only\n be recovered if you have another verified active device or have a\n recovery key setup.\n\n When using [`dangerous_hard_limit_eviction`], the [`hard_limit`] must be\n at least 2 to avoid catastrophically losing encrypted history and\n digital identity in pathological cases. Keep in mind this is a bare\n minimum restriction and you can still run into trouble.\n\n This is most applicable in scenarios where your homeserver has many\n legacy bots/scripts that login over and over (which ideally should\n be using [personal access\n tokens](https://github.com/element-hq/matrix-authentication-service/issues/4492))\n and you want to avoid breaking their operation while maintaining some\n level of sanity with the number of devices that people can have.\n\n Removing devices is a non-trivial task for some homeservers to tackle\n and can cause lots of device list changes, `/sync`, federation, and\n replication traffic. Consider using [`max_session_threshold`] to\n limit the size of accounts that are acted upon.\n\n [`hard_limit`]: Self::hard_limit\n [`dangerous_hard_limit_eviction`]: Self::dangerous_hard_limit_eviction\n [`max_session_threshold`]: Self::max_session_threshold", "type": "boolean", "default": false } From d38d0cd366b92bee8bafd5b03e4e033ae2529b4c Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 20 May 2026 16:36:39 -0500 Subject: [PATCH 13/13] Add interactive tests --- crates/handlers/src/compat/login.rs | 150 ++++++++++++++++++++++++++-- 1 file changed, 144 insertions(+), 6 deletions(-) diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index d402148b1..bbfd906a8 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -2180,16 +2180,77 @@ mod tests { let body: serde_json::Value = response.json(); assert_eq!( body.get("errcode") - .expect("Expected errror response to include an `errcode`"), + .expect("Expected error response to include an `errcode`"), "M_FORBIDDEN", "Expected `errcode` to be `M_FORBIDDEN`" ); } /// Test that session limits are enforced for anyone who is *under* the - /// `max_session_threshold` + /// `max_session_threshold` (when using interactive login methods) #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_session_limit_under_max_session_threshold(pool: PgPool) { + async fn test_session_limit_under_max_session_threshold_interactive(pool: PgPool) { + setup(); + let state = TestState::from_pool_with_site_config( + pool, + SiteConfig { + session_limit: Some(SessionLimitConfig { + // Lowest non-zero value so we don't have to login a bunch + soft_limit: NonZeroU64::new(1).unwrap(), + hard_limit: NonZeroU64::new(1).unwrap(), + // The main thing we're trying to test + max_session_threshold: Some(NonZeroU64::new(1).unwrap()), + dangerous_hard_limit_eviction: false, + }), + ..test_site_config() + }, + ) + .await + .unwrap(); + + let session_limit_config = state + .site_config + .session_limit + .as_ref() + .expect("Expected `session_limit` configured for this test"); + // Make sure this is configured as its the main differentiator we're trying to + // test + session_limit_config.max_session_threshold.as_ref().expect("Expected `session_limit.max_session_threshold` to be configured at this point in the test"); + + let _user = user_with_password(&state, "alice", "password", false).await; + + // Keep logging in to add more sessions, up to the `hard_limit` + for _ in 0..session_limit_config.hard_limit.get() { + let login_token = matrix_compat_sso_login(&state, "alice", "password") + .await + .expect("Should be able to login without issue when under the session limit"); + + // Exchange the `loginToken` for an access token. This is what actually + // creates the session. + let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({ + "type": "m.login.token", + "token": login_token, + })); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + } + + // One more login will tip us over the `hard_limit` + // + // Session limits are enforced because we're <= `max_session_threshold` + assert_matches!( + matrix_compat_sso_login(&state, "alice", "password").await, + Err(MatrixCompatSsoLoginError::ConsentForbidden { + page_type: ConsentForbiddenPageType::DeviceLimitReached, + .. + }) + ); + } + + /// Test that session limits are enforced for anyone who is *under* the + /// `max_session_threshold` (when using non-interactive login methods) + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_session_limit_under_max_session_threshold_non_interactive(pool: PgPool) { setup(); let state = TestState::from_pool_with_site_config( pool, @@ -2244,16 +2305,93 @@ mod tests { let body: serde_json::Value = response.json(); assert_eq!( body.get("errcode") - .expect("Expected errror response to include an `errcode`"), + .expect("Expected error response to include an `errcode`"), "M_FORBIDDEN", "Expected `errcode` to be `M_FORBIDDEN`" ); } /// Test that session limits are not enforced (logins are allowed) for - /// anyone who is already *past* the `max_session_threshold` + /// anyone who is already *past* the `max_session_threshold` (when using + /// interactive login methods) #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_session_limit_past_max_session_threshold(pool: PgPool) { + async fn test_session_limit_past_max_session_threshold_interactive(pool: PgPool) { + setup(); + let mut state = TestState::from_pool_with_site_config( + pool.clone(), + SiteConfig { + // Setup an account with a few sessions before we add a `session_limit` + session_limit: None, + ..test_site_config() + }, + ) + .await + .unwrap(); + + // Lowest non-zero value so we don't have to login a bunch + let upcoming_hard_limit = 1; + + // Setup a user that has more sessions than the `max_session_threshold`. + // + // Keep logging in to add more sessions. We want to be past the + // `hard_limit`/`max_session_threshold` + let _user = user_with_password(&state, "alice", "password", false).await; + #[allow(clippy::range_plus_one)] + for _ in 0..(upcoming_hard_limit + 1) { + let login_token = matrix_compat_sso_login(&state, "alice", "password") + .await + .expect("Should be able to login without issue when under the session limit"); + + // Exchange the `loginToken` for an access token. This is what actually + // creates the session. + let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({ + "type": "m.login.token", + "token": login_token, + })); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + } + + // Update the app state to configure a `max_session_threshold` + state.site_config.session_limit = Some(SessionLimitConfig { + soft_limit: NonZeroU64::new(upcoming_hard_limit).unwrap(), + hard_limit: NonZeroU64::new(upcoming_hard_limit).unwrap(), + // The main thing we're trying to test + max_session_threshold: Some(NonZeroU64::new(upcoming_hard_limit).unwrap()), + dangerous_hard_limit_eviction: false, + }); + let state = state.restart().await; + + let session_limit_config = state + .site_config + .session_limit + .as_ref() + .expect("Expected `session_limit` to be configured at this point in the test"); + // Make sure this is configured as its the main differentiator we're trying to + // test + session_limit_config.max_session_threshold.as_ref().expect("Expected `session_limit.max_session_threshold` to be configured at this point in the test"); + + // Since we're already above `max_session_threshold`, the `session_limit` won't + // stop us from adding another session. + let login_token = matrix_compat_sso_login(&state, "alice", "password") + .await + .expect("Should be able to login since we're past the `max_session_threshold`"); + + // Exchange the `loginToken` for an access token. This is what actually + // creates the session. + let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({ + "type": "m.login.token", + "token": login_token, + })); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + } + + /// Test that session limits are not enforced (logins are allowed) for + /// anyone who is already *past* the `max_session_threshold` (when using + /// non-interactive login methods) + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_session_limit_past_max_session_threshold_non_interactive(pool: PgPool) { setup(); let mut state = TestState::from_pool_with_site_config( pool.clone(),