diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index f0be20ca9..23a13d59e 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -576,7 +576,8 @@ async fn token_login( Device::generate(rng) }; - repo.app_session() + let session_replaced = repo + .app_session() .finish_sessions_to_replace_device(clock, &browser_session.user, &device) .await?; @@ -586,6 +587,7 @@ async fn token_login( .evaluate_compat_login(mas_policy::CompatLoginInput { user: &browser_session.user, login_type: CompatLoginType::WebSso, + session_replaced, session_counts, requester, }) @@ -702,7 +704,8 @@ async fn user_password_login( Device::generate(&mut rng) }; - repo.app_session() + let session_replaced = repo + .app_session() .finish_sessions_to_replace_device(clock, &user, &device) .await?; @@ -712,6 +715,7 @@ async fn user_password_login( .evaluate_compat_login(mas_policy::CompatLoginInput { user: &user, login_type: CompatLoginType::Password, + session_replaced, session_counts, requester: policy_requester, }) diff --git a/crates/handlers/src/compat/login_sso_complete.rs b/crates/handlers/src/compat/login_sso_complete.rs index 3c33b5d46..4c34ca7cd 100644 --- a/crates/handlers/src/compat/login_sso_complete.rs +++ b/crates/handlers/src/compat/login_sso_complete.rs @@ -123,6 +123,8 @@ pub async fn get( .evaluate_compat_login(mas_policy::CompatLoginInput { user: &session.user, login_type: CompatLoginType::WebSso, + // TODO should we predict a replacement? + session_replaced: false, session_counts, requester: mas_policy::Requester { ip_address: activity_tracker.ip(), @@ -251,6 +253,8 @@ pub async fn post( user: &session.user, login_type: CompatLoginType::WebSso, session_counts, + // TODO should we predict a replacement? + session_replaced: false, requester: mas_policy::Requester { ip_address: activity_tracker.ip(), user_agent, diff --git a/crates/policy/src/model.rs b/crates/policy/src/model.rs index 81c37a6e5..f8dc8e129 100644 --- a/crates/policy/src/model.rs +++ b/crates/policy/src/model.rs @@ -197,6 +197,9 @@ pub struct CompatLoginInput<'a> { /// How many sessions the user has. pub session_counts: SessionCounts, + /// Whether a session will be replaced by this login + pub session_replaced: bool, + // TODO is this actually what we care about? Don't we care a bit more about whether we're in an // interactive context or a non-interactive context? (SSO type has both phases :() pub login_type: CompatLoginType, diff --git a/policies/compat_login/compat_login.rego b/policies/compat_login/compat_login.rego index 3aeb566cf..d81fe6e6e 100644 --- a/policies/compat_login/compat_login.rego +++ b/policies/compat_login/compat_login.rego @@ -36,6 +36,10 @@ violation contains { # TODO not strictly correct... input.login_type == "m.login.sso" + # Only apply if this login doesn't replace a session + # (As then this login is not actually increasing the number of devices) + not input.session_replaced + # 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 @@ -53,6 +57,10 @@ violation contains { # This is not a web-based interactive login input.login_type == "m.login.password" + # Only apply if this login doesn't replace a session + # (As then this login is not actually increasing the number of devices) + not input.session_replaced + # 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 diff --git a/policies/compat_login/compat_login_test.rego b/policies/compat_login/compat_login_test.rego index 46ec1b0a2..070e0b7e8 100644 --- a/policies/compat_login/compat_login_test.rego +++ b/policies/compat_login/compat_login_test.rego @@ -14,32 +14,38 @@ test_session_limiting_sso if { compat_login.allow with input.user as user with input.session_counts as {"total": 1} with input.login_type as "m.login.sso" + with input.session_replaced as false with data.session_limit as {"soft_limit": 32, "hard_limit": 64} compat_login.allow with input.user as user with input.session_counts as {"total": 31} with input.login_type as "m.login.sso" + with input.session_replaced as false with data.session_limit as {"soft_limit": 32, "hard_limit": 64} not compat_login.allow with input.user as user with input.session_counts as {"total": 32} with input.login_type as "m.login.sso" + with input.session_replaced as false with data.session_limit as {"soft_limit": 32, "hard_limit": 64} not compat_login.allow with input.user as user with input.session_counts as {"total": 42} with input.login_type as "m.login.sso" + with input.session_replaced as false with data.session_limit as {"soft_limit": 32, "hard_limit": 64} not compat_login.allow with input.user as user with input.session_counts as {"total": 65} with input.login_type as "m.login.sso" + with input.session_replaced as false with data.session_limit as {"soft_limit": 32, "hard_limit": 64} # No limit configured compat_login.allow with input.user as user with input.session_counts as {"total": 1} with input.login_type as "m.login.sso" + with input.session_replaced as false with data.session_limit as null } @@ -47,26 +53,45 @@ test_session_limiting_password if { compat_login.allow with input.user as user with input.session_counts as {"total": 1} with input.login_type as "m.login.password" + with input.session_replaced as false with data.session_limit as {"soft_limit": 32, "hard_limit": 64} compat_login.allow with input.user as user with input.session_counts as {"total": 63} with input.login_type as "m.login.password" + with input.session_replaced as false with data.session_limit as {"soft_limit": 32, "hard_limit": 64} not compat_login.allow with input.user as user with input.session_counts as {"total": 64} with input.login_type as "m.login.password" + with input.session_replaced as false with data.session_limit as {"soft_limit": 32, "hard_limit": 64} not compat_login.allow with input.user as user with input.session_counts as {"total": 65} with input.login_type as "m.login.password" + with input.session_replaced as false with data.session_limit as {"soft_limit": 32, "hard_limit": 64} # No limit configured compat_login.allow with input.user as user with input.session_counts as {"total": 1} with input.login_type as "m.login.password" + with input.session_replaced as false with data.session_limit as null } + +test_no_session_limiting_upon_replacement if { + not compat_login.allow with input.user as user + with input.session_counts as {"total": 65} + with input.login_type as "m.login.password" + with input.session_replaced as false + with data.session_limit as {"soft_limit": 32, "hard_limit": 64} + + not compat_login.allow with input.user as user + with input.session_counts as {"total": 65} + with input.login_type as "m.login.sso" + with input.session_replaced as false + with data.session_limit as {"soft_limit": 32, "hard_limit": 64} +} diff --git a/policies/schema/compat_login_input.json b/policies/schema/compat_login_input.json index c28c79388..a7814c059 100644 --- a/policies/schema/compat_login_input.json +++ b/policies/schema/compat_login_input.json @@ -7,6 +7,7 @@ "login_type", "requester", "session_counts", + "session_replaced", "user" ], "properties": { @@ -22,6 +23,10 @@ } ] }, + "session_replaced": { + "description": "Whether a session will be replaced by this login", + "type": "boolean" + }, "login_type": { "$ref": "#/definitions/CompatLoginType" },