diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index f159ee791..0fbcf5e0f 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -16,7 +16,7 @@ use mas_data_model::{ User, }; use mas_matrix::HomeserverConnection; -use mas_policy::{Policy, Requester, ViolationCode, model::CompatLoginType}; +use mas_policy::{Policy, Requester, ViolationCode, model::CompatLogin}; use mas_storage::{ BoxRepository, BoxRepositoryFactory, RepositoryAccess, compat::{ @@ -586,8 +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, + login: CompatLogin::Token, session_replaced, session_counts, requester, @@ -715,8 +714,7 @@ async fn user_password_login( let res = policy .evaluate_compat_login(mas_policy::CompatLoginInput { user: &user, - is_interactive: false, - login_type: CompatLoginType::Password, + login: CompatLogin::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 020ddd707..9ede9a923 100644 --- a/crates/handlers/src/compat/login_sso_complete.rs +++ b/crates/handlers/src/compat/login_sso_complete.rs @@ -20,7 +20,7 @@ use mas_axum_utils::{ csrf::{CsrfExt, ProtectedForm}, }; use mas_data_model::{BoxClock, BoxRng, Clock}; -use mas_policy::{Policy, model::CompatLoginType}; +use mas_policy::{Policy, model::CompatLogin}; use mas_router::{CompatLoginSsoAction, UrlBuilder}; use mas_storage::{BoxRepository, RepositoryAccess, compat::CompatSsoLoginRepository}; use mas_templates::{ @@ -121,8 +121,9 @@ pub async fn get( let res = policy .evaluate_compat_login(mas_policy::CompatLoginInput { user: &session.user, - is_interactive: true, - login_type: CompatLoginType::WebSso, + login: CompatLogin::Sso { + redirect_uri: login.redirect_uri.to_string(), + }, // We don't know if there's going to be a replacement until we received the device ID, // which happens too late. session_replaced: false, @@ -252,8 +253,9 @@ pub async fn post( let res = policy .evaluate_compat_login(mas_policy::CompatLoginInput { user: &session.user, - is_interactive: true, - login_type: CompatLoginType::WebSso, + login: CompatLogin::Sso { + redirect_uri: login.redirect_uri.to_string(), + }, session_counts, // We don't know if there's going to be a replacement until we received the device ID, // which happens too late. diff --git a/crates/policy/src/model.rs b/crates/policy/src/model.rs index 81609eb47..3f6a72d66 100644 --- a/crates/policy/src/model.rs +++ b/crates/policy/src/model.rs @@ -200,25 +200,25 @@ pub struct CompatLoginInput<'a> { /// Whether a session will be replaced by this login pub session_replaced: bool, - /// 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, + /// What type of login is being performed. + /// This also determines whether the login is interactive. + pub login: CompatLogin, pub requester: Requester, } #[derive(Serialize, Debug, JsonSchema)] -pub enum CompatLoginType { +#[serde(tag = "type")] +pub enum CompatLogin { + /// Used as the interactive part of SSO login. #[serde(rename = "m.login.sso")] - WebSso, + Sso { redirect_uri: String }, + /// Used as the final (non-interactive) stage of SSO login. + #[serde(rename = "m.login.token")] + Token, + + /// Non-interactive password-over-the-API login. #[serde(rename = "m.login.password")] Password, } diff --git a/policies/compat_login/compat_login.rego b/policies/compat_login/compat_login.rego index 913ef38c4..c856aaf05 100644 --- a/policies/compat_login/compat_login.rego +++ b/policies/compat_login/compat_login.rego @@ -14,6 +14,12 @@ import data.common default allow := false +is_interactive if { + # Only `m.login.sso` (the interactive web form) is interactive; + # `m.login.password` and `m.login.token` (including the finalisation of an SSO login) are not + input.login.type == "m.login.sso" +} + allow if { count(violation) == 0 } @@ -33,7 +39,7 @@ violation contains { data.session_limit != null # This is a web-based interactive login - input.is_interactive + is_interactive # Only apply if this login doesn't replace a session # (As then this login is not actually increasing the number of devices) @@ -54,7 +60,7 @@ violation contains { data.session_limit != null # This is not a web-based interactive login - not input.is_interactive + not 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 9a246a6a5..1b8049844 100644 --- a/policies/compat_login/compat_login_test.rego +++ b/policies/compat_login/compat_login_test.rego @@ -14,44 +14,38 @@ user := {"username": "john"} 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.login as {"type": "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.login as {"type": "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.login as {"type": "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.login as {"type": "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.login as {"type": "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.login as {"type": "m.login.sso"} with input.session_replaced as false with data.session_limit as null } @@ -60,37 +54,32 @@ test_session_limiting_sso if { 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.login as {"type": "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.login as {"type": "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.login as {"type": "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.login as {"type": "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.login as {"type": "m.login.password"} with input.session_replaced as false with data.session_limit as null } @@ -98,15 +87,13 @@ 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.login as {"type": "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.login as {"type": "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 eb036014d..ffb182de4 100644 --- a/policies/schema/compat_login_input.json +++ b/policies/schema/compat_login_input.json @@ -20,12 +20,13 @@ "description": "Whether a session will be replaced by this login", "type": "boolean" }, - "is_interactive": { - "description": "Whether the user is currently in an interactive context.\n For `m.login.password`: false\n For `m.login.sso`:\n - true when asking for consent,\n - false when actually performing the login (at which point we create the\n compat session, but it's too late to show a web page)", - "type": "boolean" - }, - "login_type": { - "$ref": "#/definitions/CompatLoginType" + "login": { + "description": "What type of login is being performed.\n This also determines whether the login is interactive.", + "allOf": [ + { + "$ref": "#/definitions/CompatLogin" + } + ] }, "requester": { "$ref": "#/definitions/Requester" @@ -35,8 +36,7 @@ "user", "session_counts", "session_replaced", - "is_interactive", - "login_type", + "login", "requester" ], "definitions": { @@ -72,11 +72,51 @@ "personal" ] }, - "CompatLoginType": { - "type": "string", - "enum": [ - "m.login.sso", - "m.login.password" + "CompatLogin": { + "oneOf": [ + { + "description": "Used as the interactive part of SSO login.", + "type": "object", + "properties": { + "type": { + "type": "string", + "const": "m.login.sso" + }, + "redirect_uri": { + "type": "string" + } + }, + "required": [ + "type", + "redirect_uri" + ] + }, + { + "description": "Used as the final (non-interactive) stage of SSO login.", + "type": "object", + "properties": { + "type": { + "type": "string", + "const": "m.login.token" + } + }, + "required": [ + "type" + ] + }, + { + "description": "Non-interactive password-over-the-API login.", + "type": "object", + "properties": { + "type": { + "type": "string", + "const": "m.login.password" + } + }, + "required": [ + "type" + ] + } ] }, "Requester": {