Session hard limit automatic eviction in non-interactive contexts (dangerous_hard_limit_eviction) (#5607)

Part of https://github.com/element-hq/matrix-authentication-service/issues/4339 / https://github.com/element-hq/backend-internal/issues/199 tracking work to limit number of devices.

Cargo.lock

@@ -3259,6 +3259,7 @@ dependencies = [
  "base64ct",
  "chrono",
  "crc",
+ "mas-config",
  "mas-iana",
  "mas-jose",
  "oauth2-types",

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"

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

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<dyn std::error::Error + Send + Sync + 'static>> {
+        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<figment::error::Error>> {
+        // 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(())
+    }
 }

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,

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
-

crates/data-model/src/site_config.rs

@@ -39,10 +39,12 @@ pub struct SessionExpirationConfig {
     pub compat_session_inactivity_ttl: Option<Duration>,
 }
 
+/// 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.

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<Counter<u64>> = 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<Violation>,
+) -> 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<CompatSsoLogin>)` 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<Vec<CompatSession>, 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<mas_data_model::CompatSession> =
+            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<String>,
     initial_device_display_name: Option<String>,
@@ -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<String>,
@@ -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<String> = 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<String> = 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<String> = 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<String> = 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<String> = 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<String> = 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,
+        );
+    }
 }

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

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

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<Arc<PolicyFactory>, 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],
                 }),

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<SessionLimitConfig>,
+    pub session_limit: Option<SessionLimitConfig>,
 }
 
 impl Data {
     #[must_use]
-    pub fn new(server_name: String, session_limit: Option<SessionLimitConfig>) -> 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"))

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

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

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": [

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

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
 }

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

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
 }