From f450d0449c7cf69e31d2ca48abb151f2d2cfb1ad Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Tue, 25 Nov 2025 18:20:14 +0000 Subject: [PATCH] Make policy depend on whether the login is interactive or not --- crates/handlers/src/compat/login.rs | 2 ++ crates/handlers/src/compat/login_sso_complete.rs | 7 ++++--- crates/policy/src/model.rs | 11 +++++++++-- policies/compat_login/compat_login.rego | 5 ++--- policies/compat_login/compat_login_test.rego | 15 +++++++++++++++ policies/schema/compat_login_input.json | 5 +++++ 6 files changed, 37 insertions(+), 8 deletions(-) diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index 23a13d59e..f159ee791 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -586,6 +586,7 @@ async fn token_login( let res = policy .evaluate_compat_login(mas_policy::CompatLoginInput { user: &browser_session.user, + is_interactive: false, login_type: CompatLoginType::WebSso, session_replaced, session_counts, @@ -714,6 +715,7 @@ async fn user_password_login( let res = policy .evaluate_compat_login(mas_policy::CompatLoginInput { user: &user, + is_interactive: false, login_type: CompatLoginType::Password, session_replaced, session_counts, diff --git a/crates/handlers/src/compat/login_sso_complete.rs b/crates/handlers/src/compat/login_sso_complete.rs index 4c34ca7cd..229eb993c 100644 --- a/crates/handlers/src/compat/login_sso_complete.rs +++ b/crates/handlers/src/compat/login_sso_complete.rs @@ -20,12 +20,11 @@ use mas_axum_utils::{ csrf::{CsrfExt, ProtectedForm}, }; use mas_data_model::{BoxClock, BoxRng, Clock}; -use mas_policy::{Policy, ViolationCode, model::CompatLoginType}; +use mas_policy::{Policy, model::CompatLoginType}; use mas_router::{CompatLoginSsoAction, UrlBuilder}; use mas_storage::{BoxRepository, RepositoryAccess, compat::CompatSsoLoginRepository}; use mas_templates::{ - CompatLoginPolicyViolationContext, CompatSsoContext, EmptyContext, ErrorContext, - PolicyViolationContext, TemplateContext, Templates, + CompatLoginPolicyViolationContext, CompatSsoContext, ErrorContext, TemplateContext, Templates, }; use serde::{Deserialize, Serialize}; use ulid::Ulid; @@ -122,6 +121,7 @@ pub async fn get( let res = policy .evaluate_compat_login(mas_policy::CompatLoginInput { user: &session.user, + is_interactive: true, login_type: CompatLoginType::WebSso, // TODO should we predict a replacement? session_replaced: false, @@ -251,6 +251,7 @@ pub async fn post( let res = policy .evaluate_compat_login(mas_policy::CompatLoginInput { user: &session.user, + is_interactive: true, login_type: CompatLoginType::WebSso, session_counts, // TODO should we predict a replacement? diff --git a/crates/policy/src/model.rs b/crates/policy/src/model.rs index f8dc8e129..81609eb47 100644 --- a/crates/policy/src/model.rs +++ b/crates/policy/src/model.rs @@ -200,8 +200,15 @@ pub struct CompatLoginInput<'a> { /// 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 :() + /// Whether the user is currently in an interactive context. + /// For `m.login.password`: false + /// For `m.login.sso`: + /// - true when asking for consent, + /// - false when actually performing the login (at which point we create the + /// compat session, but it's too late to show a web page) + pub is_interactive: bool, + + // TODO I don't know if we should keep this anymore, not used by current policy pub login_type: CompatLoginType, pub requester: Requester, diff --git a/policies/compat_login/compat_login.rego b/policies/compat_login/compat_login.rego index d81fe6e6e..913ef38c4 100644 --- a/policies/compat_login/compat_login.rego +++ b/policies/compat_login/compat_login.rego @@ -33,8 +33,7 @@ violation contains { data.session_limit != null # This is a web-based interactive login - # TODO not strictly correct... - input.login_type == "m.login.sso" + input.is_interactive # Only apply if this login doesn't replace a session # (As then this login is not actually increasing the number of devices) @@ -55,7 +54,7 @@ violation contains { data.session_limit != null # This is not a web-based interactive login - input.login_type == "m.login.password" + not input.is_interactive # Only apply if this login doesn't replace a session # (As then this login is not actually increasing the number of devices) diff --git a/policies/compat_login/compat_login_test.rego b/policies/compat_login/compat_login_test.rego index 070e0b7e8..9a246a6a5 100644 --- a/policies/compat_login/compat_login_test.rego +++ b/policies/compat_login/compat_login_test.rego @@ -10,33 +10,39 @@ import rego.v1 user := {"username": "john"} +# Tests session limiting when using (the interactive part of) `m.login.sso` test_session_limiting_sso if { compat_login.allow with input.user as user with input.session_counts as {"total": 1} + with input.is_interactive as true 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.is_interactive as true 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.is_interactive as true 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.is_interactive as true 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.is_interactive as true 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} @@ -44,32 +50,38 @@ test_session_limiting_sso if { # No limit configured compat_login.allow with input.user as user with input.session_counts as {"total": 1} + with input.is_interactive as true with input.login_type as "m.login.sso" with input.session_replaced as false with data.session_limit as null } +# Test session limiting when using `m.login.password` test_session_limiting_password if { compat_login.allow with input.user as user with input.session_counts as {"total": 1} + with input.is_interactive as false 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.is_interactive as false 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.is_interactive as false 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.is_interactive as false 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} @@ -77,6 +89,7 @@ test_session_limiting_password if { # No limit configured compat_login.allow with input.user as user with input.session_counts as {"total": 1} + with input.is_interactive as false with input.login_type as "m.login.password" with input.session_replaced as false with data.session_limit as null @@ -85,12 +98,14 @@ test_session_limiting_password if { 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.is_interactive as false 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.is_interactive as true 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 a7814c059..99d33b7e5 100644 --- a/policies/schema/compat_login_input.json +++ b/policies/schema/compat_login_input.json @@ -4,6 +4,7 @@ "description": "Input for the compatibility login policy.", "type": "object", "required": [ + "is_interactive", "login_type", "requester", "session_counts", @@ -27,6 +28,10 @@ "description": "Whether a session will be replaced by this login", "type": "boolean" }, + "is_interactive": { + "description": "Whether the user is currently in an interactive context. For `m.login.password`: false For `m.login.sso`: - true when asking for consent, - false when actually performing the login (at which point we create the compat session, but it's too late to show a web page)", + "type": "boolean" + }, "login_type": { "$ref": "#/definitions/CompatLoginType" },