Make policy depend on whether the login is interactive or not

This commit is contained in:
Olivier 'reivilibre
2025-11-25 18:20:14 +00:00
parent 6fdb63b361
commit f450d0449c
6 changed files with 37 additions and 8 deletions

View File

@@ -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,

View File

@@ -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?

View File

@@ -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,

View File

@@ -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)

View File

@@ -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}

View File

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