diff --git a/Cargo.lock b/Cargo.lock index be09dd458..ce72a1d02 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3259,6 +3259,7 @@ dependencies = [ "base64ct", "chrono", "crc", + "mas-config", "mas-iana", "mas-jose", "oauth2-types", diff --git a/Cargo.toml b/Cargo.toml index 0a8e26047..9262305d4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,9 +24,14 @@ unsafe_code = "deny" # We use groups as good defaults, but with a lower priority so that we can override them all = { level = "deny", priority = -1 } pedantic = { level = "warn", priority = -1 } - +# Allowed because it's nice to have temporary semantic names, see +# https://github.com/rust-lang/rust-clippy/issues/12512#issuecomment-3316736180 +let_and_return = "allow" str_to_string = "deny" too_many_lines = "allow" +# Sometimes variables are long and annoying to inline in the format string. +# And this lint also complains about aliases, https://github.com/rust-lang/rust-clippy/issues/10247 +uninlined_format_args = "allow" [workspace.lints.rustdoc] broken_intra_doc_links = "deny" diff --git a/crates/cli/src/util.rs b/crates/cli/src/util.rs index 454276150..c3a8412f9 100644 --- a/crates/cli/src/util.rs +++ b/crates/cli/src/util.rs @@ -156,10 +156,14 @@ pub async fn policy_factory_from_config( .map(|c| SessionLimitConfig { soft_limit: c.soft_limit, hard_limit: c.hard_limit, + dangerous_hard_limit_eviction: c.dangerous_hard_limit_eviction, }); - let data = mas_policy::Data::new(matrix_config.homeserver.clone(), session_limit_config) - .with_rest(config.data.clone()); + let data = mas_policy::Data::new(mas_policy::BaseData { + server_name: matrix_config.homeserver.clone(), + session_limit: session_limit_config, + }) + .with_rest(config.data.clone()); PolicyFactory::load(policy_file, data, entrypoints) .await @@ -242,6 +246,7 @@ pub fn site_config_from_config( .map(|c| SessionLimitConfig { soft_limit: c.soft_limit, hard_limit: c.hard_limit, + dangerous_hard_limit_eviction: c.dangerous_hard_limit_eviction, }), }) } diff --git a/crates/config/src/sections/experimental.rs b/crates/config/src/sections/experimental.rs index b8f3920b0..6806ec760 100644 --- a/crates/config/src/sections/experimental.rs +++ b/crates/config/src/sections/experimental.rs @@ -17,6 +17,10 @@ fn default_true() -> bool { true } +fn default_false() -> bool { + false +} + fn default_token_ttl() -> Duration { Duration::microseconds(5 * 60 * 1000 * 1000) } @@ -116,11 +120,87 @@ impl ExperimentalConfig { impl ConfigurationSection for ExperimentalConfig { const PATH: Option<&'static str> = Some("experimental"); + + fn validate( + &self, + figment: &figment::Figment, + ) -> Result<(), Box> { + if let Some(session_limit) = &self.session_limit { + session_limit.validate().map_err(|mut err| { + // Save the error location information in the error + err.metadata = figment.find_metadata(Self::PATH.unwrap()).cloned(); + err.profile = Some(figment::Profile::Default); + err.path.insert(0, Self::PATH.unwrap().to_owned()); + err.path.insert(1, "session_limit".to_owned()); + err + })?; + } + Ok(()) + } } /// Configuration options for the session limit feature #[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] pub struct SessionLimitConfig { + /// Upon login in interactive contexts (like OAuth 2.0 sessions), if the + /// soft limit is reached, it will display a policy violation screen + /// (web UI) to remove sessions before creating the new session. + /// + /// This is not enforced in non-interactive contexts (like the legacy + /// compability login API) as there is no opportunity for us to show + /// some UI for people remove some sessions. See [`hard_limit`] for + /// enforcement on that side. + /// + /// [`hard_limit`]: Self::hard_limit pub soft_limit: NonZeroU64, + /// Upon login, when `dangerous_hard_limit_eviction: false`, will refuse the + /// new login (policy violation error), otherwise, see + /// [`dangerous_hard_limit_eviction`]. + /// + /// The hard limit is enforced in all contexts + /// (interactive/non-interactive). + /// + /// [`dangerous_hard_limit_eviction`]: Self::dangerous_hard_limit_eviction pub hard_limit: NonZeroU64, + /// Whether we should automatically choose the least recently used devices + /// to remove when the [`Self::hard_limit`] is reached; in order to + /// allow the new login to continue. + /// + /// Disabled by default + /// + /// WARNING: Removing sessions is a potentially damaging operation. Any + /// end-to-end encrypted history on the device will be lost and can only + /// be recovered if you have another verified active device or have a + /// recovery key setup. + /// + /// When using [`dangerous_hard_limit_eviction`], the [`hard_limit`] must be + /// at least 2 to avoid catastrophically losing encrypted history and + /// digital identity in pathological cases. Keep in mind this is a bare + /// minimum restriction and you can still run into trouble. + /// + /// This is most applicable in scenarios where your homeserver has many + /// legacy bots/scripts that login over and over (which ideally should + /// be using [personal access + /// tokens](https://github.com/element-hq/matrix-authentication-service/issues/4492)) + /// and you want to avoid breaking their operation while maintaining some + /// level of sanity with the number of devices that people can have. + /// + /// [`hard_limit`]: Self::hard_limit + /// [`dangerous_hard_limit_eviction`]: Self::dangerous_hard_limit_eviction + #[serde(default = "default_false")] + pub dangerous_hard_limit_eviction: bool, +} + +impl SessionLimitConfig { + fn validate(&self) -> Result<(), Box> { + // See [`SessionLimitConfig::dangerous_hard_limit_eviction`] docstring + if self.dangerous_hard_limit_eviction && self.hard_limit.get() < 2 { + return Err(figment::error::Error::from( + "Session `hard_limit` must be at least 2 when automatic `dangerous_hard_limit_eviction` is set. \ + See configuration docs for more info.", + ).with_path("hard_limit").into()); + } + + Ok(()) + } } diff --git a/crates/config/src/sections/mod.rs b/crates/config/src/sections/mod.rs index eb4ff2a44..ae805c230 100644 --- a/crates/config/src/sections/mod.rs +++ b/crates/config/src/sections/mod.rs @@ -34,7 +34,7 @@ pub use self::{ clients::{ClientAuthMethodConfig, ClientConfig, ClientsConfig}, database::{DatabaseConfig, PgSslMode}, email::{EmailConfig, EmailSmtpMode, EmailTransportKind}, - experimental::ExperimentalConfig, + experimental::{ExperimentalConfig, SessionLimitConfig as ExperimentalSessionLimitConfig}, http::{ BindConfig as HttpBindConfig, HttpConfig, ListenerConfig as HttpListenerConfig, Resource as HttpResource, TlsConfig as HttpTlsConfig, UnixOrTcp, diff --git a/crates/data-model/Cargo.toml b/crates/data-model/Cargo.toml index 7e9be8282..a1ff799bd 100644 --- a/crates/data-model/Cargo.toml +++ b/crates/data-model/Cargo.toml @@ -31,6 +31,7 @@ regex.workspace = true woothee.workspace = true mas-iana.workspace = true +# Added for rustdoc links +mas-config.workspace = true mas-jose.workspace = true oauth2-types.workspace = true - diff --git a/crates/data-model/src/site_config.rs b/crates/data-model/src/site_config.rs index bb92dc3e4..9d164c639 100644 --- a/crates/data-model/src/site_config.rs +++ b/crates/data-model/src/site_config.rs @@ -39,10 +39,12 @@ pub struct SessionExpirationConfig { pub compat_session_inactivity_ttl: Option, } +/// See [`mas_config::ExperimentalSessionLimitConfig`] #[derive(Serialize, Debug, Clone)] pub struct SessionLimitConfig { pub soft_limit: NonZeroU64, pub hard_limit: NonZeroU64, + pub dangerous_hard_limit_eviction: bool, } /// Random site configuration we want accessible in various places. diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index 5e57ce5c0..e36e57a5b 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -4,7 +4,10 @@ // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial // Please see LICENSE files in the repository root for full details. -use std::sync::{Arc, LazyLock}; +use std::{ + collections::HashMap, + sync::{Arc, LazyLock}, +}; use axum::{Json, extract::State, response::IntoResponse}; use axum_extra::typed_header::TypedHeader; @@ -12,16 +15,16 @@ use chrono::Duration; use hyper::StatusCode; use mas_axum_utils::record_error; use mas_data_model::{ - BoxClock, BoxRng, Clock, CompatSession, CompatSsoLoginState, Device, SiteConfig, TokenType, - User, + BoxClock, BoxRng, Clock, CompatSession, CompatSsoLoginState, Device, SessionLimitConfig, + SiteConfig, TokenType, User, }; use mas_matrix::HomeserverConnection; -use mas_policy::{Policy, Requester, ViolationVariant, model::CompatLogin}; +use mas_policy::{Policy, Requester, Violation, ViolationVariant, model::CompatLogin}; use mas_storage::{ - BoxRepository, BoxRepositoryFactory, RepositoryAccess, + BoxRepository, BoxRepositoryFactory, Pagination, RepositoryAccess, compat::{ - CompatAccessTokenRepository, CompatRefreshTokenRepository, CompatSessionRepository, - CompatSsoLoginRepository, + CompatAccessTokenRepository, CompatRefreshTokenRepository, CompatSessionFilter, + CompatSessionRepository, CompatSsoLoginRepository, }, queue::{QueueJobRepositoryExt as _, SyncDevicesJob}, user::{UserPasswordRepository, UserRepository}, @@ -51,6 +54,10 @@ static LOGIN_COUNTER: LazyLock> = LazyLock::new(|| { const TYPE: Key = Key::from_static_str("type"); const RESULT: Key = Key::from_static_str("result"); +/// This matches the `getNinetyDaysAgo()` used in the web UI for "inactive" +/// sessions. +const INACTIVE_SESSION_THRESHOLD: chrono::TimeDelta = Duration::days(90); + #[derive(Debug, Serialize)] #[serde(tag = "type")] enum LoginType { @@ -359,6 +366,7 @@ pub(crate) async fn post( ip_address: activity_tracker.ip(), user_agent: user_agent.clone(), }, + site_config.session_limit.as_ref(), username, password, input.device_id, // TODO check for validity @@ -377,6 +385,7 @@ pub(crate) async fn post( ip_address: activity_tracker.ip(), user_agent: user_agent.clone(), }, + site_config.session_limit.as_ref(), &token, input.device_id, input.initial_device_display_name, @@ -490,12 +499,245 @@ pub(crate) async fn post( })) } +/// Given the violations from [`Policy::evaluate_compat_login`], return the +/// appropriate `RouteError` response. +async fn process_violations_for_compat_login( + clock: &dyn Clock, + repo: &mut BoxRepository, + session_limit_config: Option<&SessionLimitConfig>, + user: &User, + violations: Vec, +) -> Result<(), RouteError> { + // We're using slice syntax here so we can match easily + match &violations[..] { + // If the only violation is having reached the session limit, we might be + // able to resolve the situation. + // + // We don't trigger this if there was some other violation anyway, since + // that means that removing a session wouldn't actually unblock the login. + [ + Violation { + variant: Some(ViolationVariant::TooManySessions { need_to_remove }), + .. + }, + ] => { + // Normally, if we are seeing a `TooManySessions` violation, we would + // expect `session_limit_config` to be filled in but if someone created + // their own policies which emit a `TooManySessions` violation that isn't + // based on the configured `session_limit`, we could also end up here. + // + // If you're using the default policies in MAS, `session_limit_config` being + // `None` would be a programming error. + match session_limit_config { + Some(session_limit_config) => { + let need_to_remove = usize::try_from(*need_to_remove).map_err(|err| { + RouteError::Internal( + anyhow::anyhow!("Unable to convert `need_to_remove` to usize: {err}") + .into(), + ) + })?; + + // When logging in with the compatibility API, there is no way for us to + // display any web UI for people to remove devices, so we instead + // automatically remove their oldest devices (when + // `dangerous_hard_limit_eviction` is configured). + if session_limit_config.dangerous_hard_limit_eviction { + // Find the least recently used (LRU) compat sessions + // + // FIXME: In the future, it would be nice to avoid sessions with + // cryptographic state. What does that mean exactly? Keys + // uploaded for device? The spec says this: + // > For all intents and purposes, non-cryptographic devices are + // > a completely separate concept and do not exist from the + // > perspective of the cryptography layer since they do not + // > have [device] identity keys, so it is impossible to send + // > them decryption keys. + // > + // > -- https://spec.matrix.org/v1.18/client-server-api/#recommended-client-behaviour + // + // FIXME: Instead of finding, then finishing in separate steps, + // we could potentially use + // `repo.compat_session().finish_bulk(...)` if it had the + // ability to limit and order. + let lru_compat_sessions = + find_lru_compat_sessions_flawed(clock, repo, user, need_to_remove) + .await?; + + // For now, we only automatically clean up compatibility sessions. + // If there aren't enough sessions that we could clean up, we just + // throw an error with an explanation. + if lru_compat_sessions.len() < need_to_remove { + return Err(RouteError::PolicyHardSessionLimitReached); + } + + // Remove the sessions (only as much as necessary, `need_to_remove`) + for compat_session in &lru_compat_sessions[0..need_to_remove] { + // Log what's happening so we have some explanation if someone asks + // + // FIXME: In the future, it would probably good to mark the reason + // down in the database for a better paper trail. + tracing::info!( + // So we can easily find logs for a given user + user_id = user.id.to_string(), + username = user.username, + // So we can easily look it up in the MAS database + compat_session_id = compat_session.id.to_string(), + // Make it easier to line up with what the user may be talking + // about + device_id = compat_session + .device + .as_ref() + .map(mas_data_model::Device::as_str), + "Automatically removing compat session for user (`dangerous_hard_limit_eviction`)" + ); + + // Remove the session + repo.compat_session() + .finish(clock, compat_session.to_owned()) + .await?; + } + } else { + // Tell the user about the limit + return Err(RouteError::PolicyHardSessionLimitReached); + } + } + // If we got here, it means they are using their own custom policies + // which don't take into account the configured `session_limit`. + // + // We don't know the actual reason behind the policy emitting the + // violation so we just have to show a generic policy rejected page. + None => { + // FIXME: We should be exposing the violations to the user + return Err(RouteError::PolicyRejected); + } + } + } + // Nothing is wrong + [] => return Ok(()), + // Just throw an error for any other violation + _violations => { + // FIXME: We should be exposing the violations to the user + return Err(RouteError::PolicyRejected); + } + } + + Ok(()) +} + +/// We fetch a minimum number of sessions (2160, more than we need in normal +/// cases) so we can sort by `last_active_at` after it gets back from the +/// database and can get even closer to removing the true oldest sessions. +/// +/// The 2160 number was chosen based on someone having a script that runs every +/// hour for the the 90-day `INACTIVE_SESSION_THRESHOLD`. Additionally, it also +/// aligns nicely with < 0.001% of people on matrix.org having less than 2160 +/// sessions and reasoning how much memory is reasonable to spend on this +/// operation to get things right. Assuming each row is ~1 KiB (pessimistic high +/// bound, see next paragraph below) we end up at ~2 MiB of memory. +/// +/// Each item in the page is `(CompatSession, Option)` where +/// `CompatSession` is 192 bytes plus a couple of strings (device name and user +/// agent) (assume pessimistic 512 total bytes). And `CompatSsoLogin` which is +/// also 192 bytes with a `login_token` string which should be no more than 32 +/// bytes. +const MINIMUM_SESSIONS_TO_FETCH: usize = 2160; + +/// Find the least recently used (LRU) compat sessions +/// +/// The results of this function are flawed (for accounts with more sessions +/// than `MINIMUM_SESSIONS_TO_FETCH`) because we can't order by `last_active_at` +/// and get an absolute sort of actually least recently used sessions. But we do +/// a pretty good job at working around the problem (see internal comments for +/// details). +async fn find_lru_compat_sessions_flawed( + clock: &dyn Clock, + repo: &mut BoxRepository, + user: &User, + // Like a limit we this function may return more more results + num_requested: usize, +) -> Result, RouteError> { + // TODO: In the future, instead of all of this faff, we can simply order + // by `last_active_at` + + let mut edges_to_consider = Vec::new(); + + // First, find the "inactive" sessions + // + // XXX: Since we can't order by `last_active_at` yet, we instead + // filter the list down to "inactive" sessions (`last_active_at` > + // 90 days ago) (this matches the `getNinetyDaysAgo()` used in the + // web UI for "inactive" sessions). And by the nature of + // [`mas_data_model::compat::CompatSession::id`] being a + // `Ulid` (the query is ordered by `compat_session_id`), the + // first bytes are a timestamp so we'll be getting the 'oldest + // created' sessions which is another good proxy. + let inactive_threshold_date = clock.now() - INACTIVE_SESSION_THRESHOLD; + let inactive_compat_session_page = repo + .compat_session() + .list( + CompatSessionFilter::new() + .for_user(user) + .active_only() + .with_last_active_before(inactive_threshold_date), + Pagination::first(std::cmp::max(num_requested, MINIMUM_SESSIONS_TO_FETCH)), + ) + .await?; + edges_to_consider.extend(inactive_compat_session_page.edges); + + // If there aren't enough "inactive" sessions, supplement with active ones + if edges_to_consider.len() < num_requested { + let active_compat_session_page = repo + .compat_session() + .list( + // If we try to use + // `.with_last_active_after(inactive_threshold_date)` + // here, it will exclude all of the rows where + // `last_active_at` is null which we want to include. + CompatSessionFilter::new().for_user(user).active_only(), + Pagination::first(std::cmp::max(num_requested, MINIMUM_SESSIONS_TO_FETCH)), + ) + .await?; + edges_to_consider.extend(active_compat_session_page.edges); + } + + // De-duplicate the sessions across both pages + let compat_session_map = { + let mut compat_session_map = HashMap::new(); + for edge in edges_to_consider { + let (compat_session, _) = edge.node; + compat_session_map.insert(compat_session.id, compat_session); + } + compat_session_map + }; + + // List of compat sessions sorted by `last_active_at` ascending + let sorted_compat_sessions = { + let mut compat_sessions: Vec = + compat_session_map.into_values().collect(); + // Sort by `last_active_at` (ascending) + compat_sessions.sort_by_key(|compat_session| { + ( + // We mainly care about sorting by `last_active_at` + compat_session.last_active_at, + // Tie-break based on `created_at` + compat_session.created_at, + // Tie-break based on `id` for determinism + compat_session.id, + ) + }); + compat_sessions + }; + + Ok(sorted_compat_sessions) +} + async fn token_login( rng: &mut (dyn RngCore + Send), clock: &dyn Clock, repo: &mut BoxRepository, policy: &mut Policy, requester: Requester, + session_limit_config: Option<&SessionLimitConfig>, token: &str, requested_device_id: Option, initial_device_display_name: Option, @@ -597,21 +839,14 @@ async fn token_login( requester, }) .await?; - if !res.valid() { - // If the only violation is that we have too many sessions, then handle that - // separately. - // In the future, we intend to evict some sessions automatically instead. We - // don't trigger this if there was some other violation anyway, since that means - // that removing a session wouldn't actually unblock the login. - if res.violations.len() == 1 { - let violation = &res.violations[0]; - if violation.variant == Some(ViolationVariant::TooManySessions) { - // The only violation is having reached the session limit. - return Err(RouteError::PolicyHardSessionLimitReached); - } - } - return Err(RouteError::PolicyRejected); - } + process_violations_for_compat_login( + clock, + repo, + session_limit_config, + &browser_session.user, + res.violations, + ) + .await?; // We first create the session in the database, commit the transaction, then // create it on the homeserver, scheduling a device sync job afterwards to @@ -645,6 +880,7 @@ async fn user_password_login( repo: &mut BoxRepository, policy: &mut Policy, policy_requester: Requester, + session_limit_config: Option<&SessionLimitConfig>, username: &str, password: String, requested_device_id: Option, @@ -730,21 +966,8 @@ async fn user_password_login( requester: policy_requester, }) .await?; - if !res.valid() { - // If the only violation is that we have too many sessions, then handle that - // separately. - // In the future, we intend to evict some sessions automatically instead. We - // don't trigger this if there was some other violation anyway, since that means - // that removing a session wouldn't actually unblock the login. - if res.violations.len() == 1 { - let violation = &res.violations[0]; - if violation.variant == Some(ViolationVariant::TooManySessions) { - // The only violation is having reached the session limit. - return Err(RouteError::PolicyHardSessionLimitReached); - } - } - return Err(RouteError::PolicyRejected); - } + process_violations_for_compat_login(clock, repo, session_limit_config, &user, res.violations) + .await?; let session = repo .compat_session() @@ -764,6 +987,12 @@ async fn user_password_login( #[cfg(test)] mod tests { + use std::{ + collections::HashSet, + num::NonZeroU64, + ops::{Mul, Sub}, + }; + use hyper::Request; use mas_matrix::{HomeserverConnection, ProvisionRequest}; use rand::distributions::{Alphanumeric, DistString}; @@ -1512,4 +1741,419 @@ mod tests { token } + + /// Test that the `soft_limit` is not enforced for compat login. + /// + /// `soft_limit` is for when we allow the user to remove devices in + /// interactive contexts. With the compatibility login API, there is no + /// opportunity for us to present a web UI. + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_soft_limit_does_not_affect_compat_login(pool: PgPool) { + setup(); + let state = TestState::from_pool_with_site_config( + pool, + SiteConfig { + session_limit: Some(SessionLimitConfig { + // Lowest non-zero value so we don't have to login a bunch (lower + // than `hard_limit`) + soft_limit: NonZeroU64::new(1).unwrap(), + // Some arbitrary high value (more than we login) + hard_limit: NonZeroU64::new(5).unwrap(), + dangerous_hard_limit_eviction: false, + }), + ..test_site_config() + }, + ) + .await + .unwrap(); + + let session_limit_config = state + .site_config + .session_limit + .as_ref() + .expect("Expected `session_limit` configured for this test"); + + assert!( + session_limit_config.soft_limit < session_limit_config.hard_limit, + "`soft_limit` should be lower than the `hard_limit` so we don't run into `hard_limit` \ + (we're testing the `soft_limit`)", + ); + + let _user = user_with_password(&state, "alice", "password", false).await; + + // Keep logging in to add more sessions, more than the `soft_limit` + #[allow(clippy::range_plus_one)] + for _ in 0..(session_limit_config.soft_limit.get() + 1) { + let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({ + "type": "m.login.password", + "identifier": { + "type": "m.id.user", + "user": "alice", + }, + "password": "password", + })); + let response = state.request(request.clone()).await; + response.assert_status(StatusCode::OK); + } + } + + /// Test that the `hard_limit` prevents more sessions + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_hard_limit_compat_login(pool: PgPool) { + setup(); + let state = TestState::from_pool_with_site_config( + pool, + SiteConfig { + session_limit: Some(SessionLimitConfig { + // (doesn't matter) + soft_limit: NonZeroU64::new(1).unwrap(), + // Lowest non-zero value so we don't have to login a bunch + hard_limit: NonZeroU64::new(1).unwrap(), + dangerous_hard_limit_eviction: false, + }), + ..test_site_config() + }, + ) + .await + .unwrap(); + + let session_limit_config = state + .site_config + .session_limit + .as_ref() + .expect("Expected `session_limit` configured for this test"); + + let _user = user_with_password(&state, "alice", "password", false).await; + + // Keep logging in to add more sessions, up to the `hard_limit` + #[allow(clippy::range_plus_one)] + for _ in 0..session_limit_config.hard_limit.get() { + let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({ + "type": "m.login.password", + "identifier": { + "type": "m.id.user", + "user": "alice", + }, + "password": "password", + })); + let response = state.request(request.clone()).await; + response.assert_status(StatusCode::OK); + } + + // One more login will tip us over the `hard_limit` + let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({ + "type": "m.login.password", + "identifier": { + "type": "m.id.user", + "user": "alice", + }, + "password": "password", + })); + let response = state.request(request.clone()).await; + response.assert_status(StatusCode::FORBIDDEN); + let body: serde_json::Value = response.json(); + assert_eq!( + body.get("errcode") + .expect("Expected errror response to include an `errcode`"), + "M_FORBIDDEN", + "Expected `errcode` to be `M_FORBIDDEN`" + ); + } + + /// Test that the `dangerous_hard_limit_eviction` will automatically drop + /// old sessions when we go over the limit + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_dangerous_hard_limit_eviction_old_compat_login(pool: PgPool) { + setup(); + let state = TestState::from_pool_with_site_config( + pool, + SiteConfig { + session_limit: Some(SessionLimitConfig { + // (doesn't matter) + soft_limit: NonZeroU64::new(1).unwrap(), + // Must be at least 2 when `dangerous_hard_limit_eviction` + hard_limit: NonZeroU64::new(2).unwrap(), + // Option under test + dangerous_hard_limit_eviction: true, + }), + ..test_site_config() + }, + ) + .await + .unwrap(); + + let session_limit_config = state + .site_config + .session_limit + .as_ref() + .expect("Expected `session_limit` configured for this test"); + + let user = user_with_password(&state, "alice", "password", false).await; + + let mut login_device_ids: Vec = Vec::new(); + + let do_login = async || { + let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({ + "type": "m.login.password", + "identifier": { + "type": "m.id.user", + "user": "alice", + }, + "password": "password", + })); + let response = state.request(request.clone()).await; + response.assert_status(StatusCode::OK); + let body: serde_json::Value = response.json(); + let device_id = match body + .get("device_id") + .expect("Expected successful login response to include `device_id`") + { + serde_json::value::Value::String(device_id) => device_id.to_owned(), + _ => { + panic!("Expected `device_id` to be a string") + } + }; + + // Wait for `last_active_at` to be set + state.activity_tracker.flush().await; + + // Return the new device + device_id + }; + + // Keep logging in to add more sessions, up to the `hard_limit`. + #[allow(clippy::range_plus_one)] + for _ in 0..session_limit_config.hard_limit.get() { + let device_id = do_login().await; + login_device_ids.push(device_id); + + // Advance time so it appears like each login happens a day after each other + state.clock.advance(Duration::days(1)); + } + let time_after_past_logins = state.clock.now(); + + // Jump to "current time" (anything > INACTIVE_SESSION_THRESHOLD) which will + // make all of those past logins to be considered "inactive" at this point. + state.clock.advance(INACTIVE_SESSION_THRESHOLD.mul(2)); + assert!( + state.clock.now() - time_after_past_logins > INACTIVE_SESSION_THRESHOLD, + "Expected 'current time' login to happen > INACTIVE_SESSION_THRESHOLD from when the past logins happened" + ); + + // Sanity check that the past compat sessions have `last_active_at` set. This is + // important as `last_active_at` starts out null. + let mut repo = state.repository().await.unwrap(); + let compat_session_page = repo + .compat_session() + .list( + CompatSessionFilter::new().for_user(&user).active_only(), + Pagination::first(session_limit_config.hard_limit.get().try_into().unwrap()), + ) + .await + .expect("Should be able to list user's compat sessions"); + for edge in compat_session_page.edges { + let (compat_session, _) = edge.node; + let last_active_at = compat_session + .last_active_at + .expect("We expect compat sessions to have `last_active_at` set for this test"); + assert!( + last_active_at < (state.clock.now().sub(INACTIVE_SESSION_THRESHOLD)), + "Expected past compat sessions to have a `last_active_at` older than the `INACTIVE_SESSION_THRESHOLD`" + ); + } + + // Now the user wants to login in the "current time". + // + // One more login will drop one of our old sessions to make room for the new + // login + let device_id = do_login().await; + login_device_ids.push(device_id); + + // Ensure we still only have two sessions (`session_limit_config.hard_limit`). + // We're sanity checking across all session types. + let session_counts = count_user_sessions_for_limiting(&mut repo, &user) + .await + .unwrap(); + assert_eq!( + session_counts.total, 2, + "Must not have more sessions ({}) than allowed by the `hard_limit` ({}). \ + Expected one of the old sessions to be dropped to make room for the new login", + session_counts.total, session_limit_config.hard_limit, + ); + + // Also ensure that the newest sessions remain (we dropped the oldest) + let compat_session_page = repo + .compat_session() + .list( + CompatSessionFilter::new().for_user(&user).active_only(), + Pagination::first(2), + ) + .await + .expect("Should be able to list user's compat sessions"); + let remaining_active_compat_session_device_ids: HashSet = compat_session_page + .edges + .iter() + .map(|a| { + a.node + .0 + .device + .clone() + .expect("Expected each login should have a device") + .as_str() + .to_owned() + }) + .collect(); + + let most_recent_login_device_ids: HashSet = login_device_ids + .iter() + .rev() + .take(2) + .map(std::borrow::ToOwned::to_owned) + .collect(); + // Sanity check our comparison (ensure we're not comparing an empty set) + assert_eq!( + most_recent_login_device_ids.len(), + 2, + "Expected 2 logins for the next comparison" + ); + + // The remaining sessions should be the most recent sessions + assert!( + most_recent_login_device_ids.is_subset(&remaining_active_compat_session_device_ids), + "Expected the 2 remaining active sessions ({remaining:?}) to include the 2 most recent logins ({recent:?}). (all logins: {login_device_ids:?})", + remaining = remaining_active_compat_session_device_ids, + recent = most_recent_login_device_ids, + ); + } + + /// Test that the `dangerous_hard_limit_eviction` will automatically drop + /// the oldest sessions when we go over the limit even if all of the + /// sessions are recent. + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_dangerous_hard_limit_eviction_recent_compat_login(pool: PgPool) { + setup(); + let state = TestState::from_pool_with_site_config( + pool, + SiteConfig { + session_limit: Some(SessionLimitConfig { + // (doesn't matter) + soft_limit: NonZeroU64::new(1).unwrap(), + // Must be at least 2 when `dangerous_hard_limit_eviction` + hard_limit: NonZeroU64::new(2).unwrap(), + // Option under test + dangerous_hard_limit_eviction: true, + }), + ..test_site_config() + }, + ) + .await + .unwrap(); + + let session_limit_config = state + .site_config + .session_limit + .as_ref() + .expect("Expected `session_limit` configured for this test"); + + let user = user_with_password(&state, "alice", "password", false).await; + + let mut login_device_ids: Vec = Vec::new(); + + // Keep logging in to add more sessions, up to the `hard_limit`. Then one more + // login will drop one of our old sessions to make room for the new login + #[allow(clippy::range_plus_one)] + for _ in 0..(session_limit_config.hard_limit.get() + 1) { + let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({ + "type": "m.login.password", + "identifier": { + "type": "m.id.user", + "user": "alice", + }, + "password": "password", + })); + let response = state.request(request.clone()).await; + response.assert_status(StatusCode::OK); + let body: serde_json::Value = response.json(); + let device_id = match body + .get("device_id") + .expect("Expected successful login response to include `device_id`") + { + serde_json::value::Value::String(device_id) => device_id.to_owned(), + _ => { + panic!("Expected `device_id` to be a string") + } + }; + login_device_ids.push(device_id); + + // This test doesn't really care if `last_active_at` is filled in. Ideally, + // we would explicitly test both scenarios (null and filled in) but either + // is fine. + // + // state.activity_tracker.flush().await; + + // Advance time so that each session ID sorts deterministically after each + // other (ULID includes timestamp). We would have flaky tests without this. + state + .clock + // Each login comes after the next. + .advance(Duration::seconds(1)); + } + + // Ensure we still only have two sessions (`session_limit_config.hard_limit`). + // We're sanity checking across all session types. + let mut repo = state.repository().await.unwrap(); + let session_counts = count_user_sessions_for_limiting(&mut repo, &user) + .await + .unwrap(); + assert_eq!( + session_counts.total, 2, + "Must not have more sessions ({}) than allowed by the `hard_limit` ({}). \ + Expected one of the old sessions to be dropped to make room for the new login", + session_counts.total, session_limit_config.hard_limit, + ); + + // Also ensure that the newest sessions remain (we dropped the oldest) + let compat_session_page = repo + .compat_session() + .list( + CompatSessionFilter::new().for_user(&user).active_only(), + Pagination::first(2), + ) + .await + .expect("Should be able to list user's compat sessions"); + let remaining_active_compat_session_device_ids: HashSet = compat_session_page + .edges + .iter() + .map(|a| { + a.node + .0 + .device + .clone() + .expect("Expected each login should have a device") + .as_str() + .to_owned() + }) + .collect(); + + let most_recent_login_device_ids: HashSet = login_device_ids + .iter() + .rev() + .take(2) + .map(std::borrow::ToOwned::to_owned) + .collect(); + // Sanity check our comparison (ensure we're not comparing an empty set) + assert_eq!( + most_recent_login_device_ids.len(), + 2, + "Expected 2 logins for the next comparison" + ); + + // The remaining sessions should be the most recent sessions + assert!( + most_recent_login_device_ids.is_subset(&remaining_active_compat_session_device_ids), + "Expected the 2 remaining active sessions ({remaining:?}) to include the 2 most recent logins ({recent:?}). (all logins: {login_device_ids:?})", + remaining = remaining_active_compat_session_device_ids, + recent = most_recent_login_device_ids, + ); + } } diff --git a/crates/handlers/src/graphql/tests.rs b/crates/handlers/src/graphql/tests.rs index 7fed79c09..898df9a56 100644 --- a/crates/handlers/src/graphql/tests.rs +++ b/crates/handlers/src/graphql/tests.rs @@ -470,7 +470,10 @@ async fn test_oauth2_client_credentials(pool: PgPool) { let state = { let mut state = state; state.policy_factory = test_utils::policy_factory( - "example.com", + mas_policy::BaseData { + server_name: "example.com".to_owned(), + session_limit: None, + }, serde_json::json!({ "admin_clients": [client_id], }), @@ -596,7 +599,10 @@ async fn test_add_user(pool: PgPool) { let state = { let mut state = state; state.policy_factory = test_utils::policy_factory( - "example.com", + mas_policy::BaseData { + server_name: "example.com".to_owned(), + session_limit: None, + }, serde_json::json!({ "admin_clients": [client_id], }), diff --git a/crates/handlers/src/oauth2/token.rs b/crates/handlers/src/oauth2/token.rs index 696c7d426..49dd7cb68 100644 --- a/crates/handlers/src/oauth2/token.rs +++ b/crates/handlers/src/oauth2/token.rs @@ -1644,7 +1644,10 @@ mod tests { let state = { let mut state = state; state.policy_factory = crate::test_utils::policy_factory( - "example.com", + mas_policy::BaseData { + server_name: "example.com".to_owned(), + session_limit: None, + }, serde_json::json!({ "admin_clients": [client_id] }), diff --git a/crates/handlers/src/test_utils.rs b/crates/handlers/src/test_utils.rs index 521a4848d..619d995a7 100644 --- a/crates/handlers/src/test_utils.rs +++ b/crates/handlers/src/test_utils.rs @@ -69,7 +69,7 @@ pub(crate) fn setup() { } pub(crate) async fn policy_factory( - server_name: &str, + base_data: mas_policy::BaseData, data: serde_json::Value, ) -> Result, anyhow::Error> { let workspace_root = camino::Utf8Path::new(env!("CARGO_MANIFEST_DIR")) @@ -86,7 +86,7 @@ pub(crate) async fn policy_factory( email: "email/violation".to_owned(), }; - let data = mas_policy::Data::new(server_name.to_owned(), None).with_rest(data); + let data = mas_policy::Data::new(base_data).with_rest(data); let policy_factory = PolicyFactory::load(file, data, entrypoints).await?; let policy_factory = Arc::new(policy_factory); @@ -207,8 +207,14 @@ impl TestState { PasswordManager::disabled() }; - let policy_factory = - policy_factory(&site_config.server_name, serde_json::json!({})).await?; + let policy_factory = policy_factory( + mas_policy::BaseData { + server_name: site_config.server_name.clone(), + session_limit: site_config.session_limit.clone(), + }, + serde_json::json!({}), + ) + .await?; let homeserver_connection = Arc::new(MockHomeserverConnection::new(&site_config.server_name)); @@ -365,7 +371,10 @@ impl TestState { let state = { let mut state = self.clone(); state.policy_factory = policy_factory( - "example.com", + mas_policy::BaseData { + server_name: "example.com".to_owned(), + session_limit: None, + }, serde_json::json!({ "admin_clients": [client_id], }), diff --git a/crates/policy/src/lib.rs b/crates/policy/src/lib.rs index 3c20eaed0..d667dd6c0 100644 --- a/crates/policy/src/lib.rs +++ b/crates/policy/src/lib.rs @@ -99,21 +99,18 @@ pub struct Data { } #[derive(Serialize, Debug)] -struct BaseData { - server_name: String, +pub struct BaseData { + pub server_name: String, /// Limits on the number of application sessions that each user can have - session_limit: Option, + pub session_limit: Option, } impl Data { #[must_use] - pub fn new(server_name: String, session_limit: Option) -> Self { + pub fn new(base_data: BaseData) -> Self { Self { - base: BaseData { - server_name, - session_limit, - }, + base: base_data, rest: None, } @@ -512,7 +509,11 @@ mod tests { #[tokio::test] async fn test_register() { - let data = Data::new("example.com".to_owned(), None).with_rest(serde_json::json!({ + let data = Data::new(BaseData { + server_name: "example.com".to_owned(), + session_limit: None, + }) + .with_rest(serde_json::json!({ "allowed_domains": ["element.io", "*.element.io"], "banned_domains": ["staging.element.io"], })); @@ -577,7 +578,10 @@ mod tests { #[tokio::test] async fn test_dynamic_data() { - let data = Data::new("example.com".to_owned(), None); + let data = Data::new(BaseData { + server_name: "example.com".to_owned(), + session_limit: None, + }); #[allow(clippy::disallowed_types)] let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) @@ -641,7 +645,10 @@ mod tests { #[tokio::test] async fn test_big_dynamic_data() { - let data = Data::new("example.com".to_owned(), None); + let data = Data::new(BaseData { + server_name: "example.com".to_owned(), + session_limit: None, + }); #[allow(clippy::disallowed_types)] let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) diff --git a/crates/policy/src/model.rs b/crates/policy/src/model.rs index a3bf24b5f..83ceb08b9 100644 --- a/crates/policy/src/model.rs +++ b/crates/policy/src/model.rs @@ -52,7 +52,10 @@ pub enum ViolationVariant { EmailBanned, /// The user has reached their session limit. - TooManySessions, + TooManySessions { + /// How many devices need to be removed to make room for the new session + need_to_remove: u32, + }, } impl ViolationVariant { @@ -70,7 +73,7 @@ impl ViolationVariant { Self::EmailDomainBanned => "email-domain-banned", Self::EmailNotAllowed => "email-not-allowed", Self::EmailBanned => "email-banned", - Self::TooManySessions => "too-many-sessions", + Self::TooManySessions { .. } => "too-many-sessions", } } } diff --git a/crates/templates/src/context.rs b/crates/templates/src/context.rs index 25123970b..6bbc1177c 100644 --- a/crates/templates/src/context.rs +++ b/crates/templates/src/context.rs @@ -890,7 +890,7 @@ impl TemplateContext for CompatLoginPolicyViolationContext { msg: "user has too many active sessions".to_owned(), redirect_uri: None, field: None, - variant: Some(ViolationVariant::TooManySessions), + variant: Some(ViolationVariant::TooManySessions { need_to_remove: 1 }), }], }, ]) diff --git a/docs/config.schema.json b/docs/config.schema.json index f6d947e48..c2c988a60 100644 --- a/docs/config.schema.json +++ b/docs/config.schema.json @@ -2898,14 +2898,21 @@ "type": "object", "properties": { "soft_limit": { + "description": "Upon login in interactive contexts (like OAuth 2.0 sessions), if the\n soft limit is reached, it will display a policy violation screen\n (web UI) to remove sessions before creating the new session.\n\n This is not enforced in non-interactive contexts (like the legacy\n compability login API) as there is no opportunity for us to show\n some UI for people remove some sessions. See [`hard_limit`] for\n enforcement on that side.\n\n [`hard_limit`]: Self::hard_limit", "type": "integer", "format": "uint64", "minimum": 1 }, "hard_limit": { + "description": "Upon login, when `dangerous_hard_limit_eviction: false`, will refuse the\n new login (policy violation error), otherwise, see\n [`dangerous_hard_limit_eviction`].\n\n The hard limit is enforced in all contexts\n (interactive/non-interactive).\n\n [`dangerous_hard_limit_eviction`]: Self::dangerous_hard_limit_eviction", "type": "integer", "format": "uint64", "minimum": 1 + }, + "dangerous_hard_limit_eviction": { + "description": "Whether we should automatically choose the least recently used devices\n to remove when the [`Self::hard_limit`] is reached; in order to\n allow the new login to continue.\n\n Disabled by default\n\n WARNING: Removing sessions is a potentially damaging operation. Any\n end-to-end encrypted history on the device will be lost and can only\n be recovered if you have another verified active device or have a\n recovery key setup.\n\n When using [`dangerous_hard_limit_eviction`], the [`hard_limit`] must be\n at least 2 to avoid catastrophically losing encrypted history and\n digital identity in pathological cases. Keep in mind this is a bare\n minimum restriction and you can still run into trouble.\n\n This is most applicable in scenarios where your homeserver has many\n legacy bots/scripts that login over and over (which ideally should\n be using [personal access\n tokens](https://github.com/element-hq/matrix-authentication-service/issues/4492))\n and you want to avoid breaking their operation while maintaining some\n level of sanity with the number of devices that people can have.\n\n [`hard_limit`]: Self::hard_limit\n [`dangerous_hard_limit_eviction`]: Self::dangerous_hard_limit_eviction", + "type": "boolean", + "default": false } }, "required": [ diff --git a/policies/authorization_grant/authorization_grant.rego b/policies/authorization_grant/authorization_grant.rego index e7d1e68e5..34334d36b 100644 --- a/policies/authorization_grant/authorization_grant.rego +++ b/policies/authorization_grant/authorization_grant.rego @@ -157,6 +157,9 @@ violation contains {"msg": sprintf( violation contains { "code": "too-many-sessions", "msg": "user has too many active sessions", + # `+ 1` because when you're at 2 sessions, and the limit is 2, you have to make room + # for the new session + "need_to_remove": (input.session_counts.total - data.session_limit.soft_limit) + 1, } if { # Only apply if session limits are enabled in the config data.session_limit != null diff --git a/policies/authorization_grant/authorization_grant_test.rego b/policies/authorization_grant/authorization_grant_test.rego index e2ca74086..4bf6442f5 100644 --- a/policies/authorization_grant/authorization_grant_test.rego +++ b/policies/authorization_grant/authorization_grant_test.rego @@ -223,34 +223,82 @@ test_mas_scopes if { with input.scope as "urn:mas:admin" } -test_session_limiting if { - authorization_grant.allow with input.user as user +# Helper utility to extract the number of sessions that they `need_to_remove`, returns 0 +# if the `too-many-sessions` violation is not found +need_to_remove_sessions(violations) := need if { + some v in violations + v.code == "too-many-sessions" + need := v.need_to_remove +} else := 0 + +# Tests session limiting when using OAuth 2.0 authorization grants +# (interactive, therefore `soft_limit` applies) +# ========================================================================= +test_session_limiting_under_limit if { + result := { + "allow": authorization_grant.allow, + "need_to_remove_sessions": need_to_remove_sessions(authorization_grant.violation), + } with input.user as user with input.session_counts as {"total": 1} with data.session_limit as {"soft_limit": 32, "hard_limit": 64} + result.allow + result.need_to_remove_sessions == 0 +} - authorization_grant.allow with input.user as user +test_session_limiting_under_soft_limit if { + result := { + "allow": authorization_grant.allow, + "need_to_remove_sessions": need_to_remove_sessions(authorization_grant.violation), + } with input.user as user with input.session_counts as {"total": 31} with data.session_limit as {"soft_limit": 32, "hard_limit": 64} + result.allow + result.need_to_remove_sessions == 0 +} - not authorization_grant.allow with input.user as user +test_session_limiting_hit_soft_limit if { + result := { + "allow": authorization_grant.allow, + "need_to_remove_sessions": need_to_remove_sessions(authorization_grant.violation), + } with input.user as user with input.session_counts as {"total": 32} with data.session_limit as {"soft_limit": 32, "hard_limit": 64} + not result.allow + result.need_to_remove_sessions == 1 +} - not authorization_grant.allow with input.user as user +test_session_limiting_over_soft_limit if { + result := { + "allow": authorization_grant.allow, + "need_to_remove_sessions": need_to_remove_sessions(authorization_grant.violation), + } with input.user as user with input.session_counts as {"total": 42} with data.session_limit as {"soft_limit": 32, "hard_limit": 64} + not result.allow + result.need_to_remove_sessions == 11 +} - not authorization_grant.allow with input.user as user +test_session_limiting_over_hard_limit if { + result := { + "allow": authorization_grant.allow, + "need_to_remove_sessions": need_to_remove_sessions(authorization_grant.violation), + } with input.user as user with input.session_counts as {"total": 65} with data.session_limit as {"soft_limit": 32, "hard_limit": 64} + not result.allow + # Only the `soft_limit` applies to the interactive login + result.need_to_remove_sessions == 34 +} + +test_session_limiting_no_limit if { # No limit configured - authorization_grant.allow with input.user as user + result := { + "allow": authorization_grant.allow, + "need_to_remove_sessions": need_to_remove_sessions(authorization_grant.violation), + } with input.user as user with input.session_counts as {"total": 1} with data.session_limit as null - - # Client credentials grant - authorization_grant.allow with input.user as user - with input.session_counts as null - with data.session_limit as {"soft_limit": 32, "hard_limit": 64} + result.allow + result.need_to_remove_sessions == 0 } diff --git a/policies/compat_login/compat_login.rego b/policies/compat_login/compat_login.rego index 4f76842cd..5f1825268 100644 --- a/policies/compat_login/compat_login.rego +++ b/policies/compat_login/compat_login.rego @@ -28,6 +28,9 @@ violation contains {"msg": sprintf( violation contains { "code": "too-many-sessions", "msg": "user has too many active sessions (soft limit)", + # `+ 1` because when you're at 2 sessions, and the limit is 2, you have to make room + # for the new session + "need_to_remove": (input.session_counts.total - data.session_limit.soft_limit) + 1, } if { # Only apply if session limits are enabled in the config data.session_limit != null @@ -49,6 +52,9 @@ violation contains { violation contains { "code": "too-many-sessions", "msg": "user has too many active sessions (hard limit)", + # `+ 1` because when you're at 2 sessions, and the limit is 2, you have to make room + # for the new session + "need_to_remove": (input.session_counts.total - data.session_limit.hard_limit) + 1, } if { # Only apply if session limits are enabled in the config data.session_limit != null diff --git a/policies/compat_login/compat_login_test.rego b/policies/compat_login/compat_login_test.rego index 1b8049844..339b1b684 100644 --- a/policies/compat_login/compat_login_test.rego +++ b/policies/compat_login/compat_login_test.rego @@ -10,90 +10,191 @@ import rego.v1 user := {"username": "john"} +# Helper utility to extract the number of sessions that they `need_to_remove`, returns 0 +# if the `too-many-sessions` violation is not found +need_to_remove_sessions(violations) := need if { + some v in violations + v.code == "too-many-sessions" + need := v.need_to_remove +} else := 0 + # 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 +# (interactive, therefore `soft_limit` applies) +# ========================================================================= +test_session_limiting_sso_under_limit if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user with input.session_counts as {"total": 1} 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} + result.allow + result.need_to_remove_sessions == 0 +} - compat_login.allow with input.user as user +test_session_limiting_sso_barely_under_soft_limit if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user with input.session_counts as {"total": 31} 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} + result.allow + result.need_to_remove_sessions == 0 +} - not compat_login.allow with input.user as user +test_session_limiting_sso_hit_soft_limit if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user with input.session_counts as {"total": 32} 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 result.allow + result.need_to_remove_sessions == 1 +} - not compat_login.allow with input.user as user +test_session_limiting_sso_over_soft_limit if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user with input.session_counts as {"total": 42} 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 result.allow + result.need_to_remove_sessions == 11 +} - not compat_login.allow with input.user as user +test_session_limiting_sso_over_hard_limit if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user with input.session_counts as {"total": 65} 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 result.allow + # Only the `soft_limit` applies to the interactive `m.login.sso` login + result.need_to_remove_sessions == 34 +} + +test_session_limiting_sso_no_limit if { # No limit configured - compat_login.allow with input.user as user + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user with input.session_counts as {"total": 1} with input.login as {"type": "m.login.sso"} with input.session_replaced as false with data.session_limit as null + result.allow + result.need_to_remove_sessions == 0 } -# Test session limiting when using `m.login.password` -test_session_limiting_password if { - compat_login.allow with input.user as user +# Test session limiting when using `m.login.password` (not interactive, therefore +# `hard_limit` applies) +# ========================================================================= +test_session_limiting_password_under_limit if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user with input.session_counts as {"total": 1} 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} + result.allow + result.need_to_remove_sessions == 0 +} - compat_login.allow with input.user as user +test_session_limiting_password_under_hard_limit if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user with input.session_counts as {"total": 63} 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} + result.allow + result.need_to_remove_sessions == 0 +} - not compat_login.allow with input.user as user +test_session_limiting_password_hit_hard_limit if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user with input.session_counts as {"total": 64} 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 result.allow + result.need_to_remove_sessions == 1 +} - not compat_login.allow with input.user as user +test_session_limiting_password_over_hard_limit if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user with input.session_counts as {"total": 65} 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 result.allow + result.need_to_remove_sessions == 2 +} +test_session_limiting_password_no_limit if { # No limit configured - compat_login.allow with input.user as user + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user with input.session_counts as {"total": 1} with input.login as {"type": "m.login.password"} with input.session_replaced as false with data.session_limit as null + result.allow + result.need_to_remove_sessions == 0 } -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 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 +# If the session is replacing an existing session, no need to throw any violations about +# too many sessions +test_no_session_limiting_sso_upon_replacement if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user with input.session_counts as {"total": 65} with input.login as {"type": "m.login.sso"} - with input.session_replaced as false + with input.session_replaced as true with data.session_limit as {"soft_limit": 32, "hard_limit": 64} + result.allow + result.need_to_remove_sessions == 0 +} + +test_no_session_limiting_password_upon_replacement if { + result := { + "allow": compat_login.allow, + "need_to_remove_sessions": need_to_remove_sessions(compat_login.violation), + } with input.user as user + with input.session_counts as {"total": 65} + with input.login as {"type": "m.login.password"} + with input.session_replaced as true + with data.session_limit as {"soft_limit": 32, "hard_limit": 64} + result.allow + result.need_to_remove_sessions == 0 }