Make policy depend on whether the login is interactive or not

Author: reivilibre

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,

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?

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,

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)

policies/compat_login/compat_login_test.rego

@@ -10,73 +10,86 @@ 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}
 
 	# 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}
 
 	# 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}

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"
     },