Don't apply a session limit when genuinely replacing a session

This commit is contained in:
Olivier 'reivilibre
2025-11-25 18:20:14 +00:00
parent 3b04fd5621
commit 6fdb63b361
6 changed files with 51 additions and 2 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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