Skip to content

Commit 0c8017f

Browse files
authored
Add experimental and preliminary policy-driven session limiting when logging in OAuth 2 sessions. (#5221)
2 parents 4753aa8 + 1690570 commit 0c8017f

File tree

18 files changed

+309
-23
lines changed

18 files changed

+309
-23
lines changed

crates/cli/src/commands/debug.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ use std::process::ExitCode;
99
use clap::Parser;
1010
use figment::Figment;
1111
use mas_config::{
12-
ConfigurationSection, ConfigurationSectionExt, DatabaseConfig, MatrixConfig, PolicyConfig,
12+
ConfigurationSection, ConfigurationSectionExt, DatabaseConfig, ExperimentalConfig,
13+
MatrixConfig, PolicyConfig,
1314
};
1415
use mas_storage_pg::PgRepositoryFactory;
1516
use tracing::{info, info_span};
@@ -45,8 +46,12 @@ impl Options {
4546
PolicyConfig::extract_or_default(figment).map_err(anyhow::Error::from_boxed)?;
4647
let matrix_config =
4748
MatrixConfig::extract(figment).map_err(anyhow::Error::from_boxed)?;
49+
let experimental_config =
50+
ExperimentalConfig::extract(figment).map_err(anyhow::Error::from_boxed)?;
4851
info!("Loading and compiling the policy module");
49-
let policy_factory = policy_factory_from_config(&config, &matrix_config).await?;
52+
let policy_factory =
53+
policy_factory_from_config(&config, &matrix_config, &experimental_config)
54+
.await?;
5055

5156
if with_dynamic_data {
5257
let database_config =

crates/cli/src/commands/server.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,9 @@ impl Options {
132132

133133
// Load and compile the WASM policies (and fallback to the default embedded one)
134134
info!("Loading and compiling the policy module");
135-
let policy_factory = policy_factory_from_config(&config.policy, &config.matrix).await?;
135+
let policy_factory =
136+
policy_factory_from_config(&config.policy, &config.matrix, &config.experimental)
137+
.await?;
136138
let policy_factory = Arc::new(policy_factory);
137139

138140
load_policy_factory_dynamic_data_continuously(

crates/cli/src/util.rs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use mas_config::{
1313
PolicyConfig, TemplatesConfig,
1414
};
1515
use mas_context::LogContext;
16-
use mas_data_model::{SessionExpirationConfig, SiteConfig};
16+
use mas_data_model::{SessionExpirationConfig, SessionLimitConfig, SiteConfig};
1717
use mas_email::{MailTransport, Mailer};
1818
use mas_handlers::passwords::PasswordManager;
1919
use mas_matrix::{HomeserverConnection, ReadOnlyHomeserverConnection};
@@ -135,6 +135,7 @@ pub fn test_mailer_in_background(mailer: &Mailer, timeout: Duration) {
135135
pub async fn policy_factory_from_config(
136136
config: &PolicyConfig,
137137
matrix_config: &MatrixConfig,
138+
experimental_config: &ExperimentalConfig,
138139
) -> Result<PolicyFactory, anyhow::Error> {
139140
let policy_file = tokio::fs::File::open(&config.wasm_module)
140141
.await
@@ -147,8 +148,17 @@ pub async fn policy_factory_from_config(
147148
email: config.email_entrypoint.clone(),
148149
};
149150

150-
let data =
151-
mas_policy::Data::new(matrix_config.homeserver.clone()).with_rest(config.data.clone());
151+
let session_limit_config =
152+
experimental_config
153+
.session_limit
154+
.as_ref()
155+
.map(|c| SessionLimitConfig {
156+
soft_limit: c.soft_limit,
157+
hard_limit: c.hard_limit,
158+
});
159+
160+
let data = mas_policy::Data::new(matrix_config.homeserver.clone(), session_limit_config)
161+
.with_rest(config.data.clone());
152162

153163
PolicyFactory::load(policy_file, data, entrypoints)
154164
.await
@@ -225,6 +235,13 @@ pub fn site_config_from_config(
225235
session_expiration,
226236
login_with_email_allowed: account_config.login_with_email_allowed,
227237
plan_management_iframe_uri: experimental_config.plan_management_iframe_uri.clone(),
238+
session_limit: experimental_config
239+
.session_limit
240+
.as_ref()
241+
.map(|c| SessionLimitConfig {
242+
soft_limit: c.soft_limit,
243+
hard_limit: c.hard_limit,
244+
}),
228245
})
229246
}
230247

crates/config/src/sections/experimental.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
55
// Please see LICENSE files in the repository root for full details.
66

7+
use std::num::NonZeroU64;
8+
79
use chrono::Duration;
810
use schemars::JsonSchema;
911
use serde::{Deserialize, Serialize};
@@ -81,6 +83,13 @@ pub struct ExperimentalConfig {
8183
/// validation.
8284
#[serde(skip_serializing_if = "Option::is_none")]
8385
pub plan_management_iframe_uri: Option<String>,
86+
87+
/// Experimental feature to limit the number of application sessions per
88+
/// user.
89+
///
90+
/// Disabled by default.
91+
#[serde(skip_serializing_if = "Option::is_none")]
92+
pub session_limit: Option<SessionLimitConfig>,
8493
}
8594

8695
impl Default for ExperimentalConfig {
@@ -90,6 +99,7 @@ impl Default for ExperimentalConfig {
9099
compat_token_ttl: default_token_ttl(),
91100
inactive_session_expiration: None,
92101
plan_management_iframe_uri: None,
102+
session_limit: None,
93103
}
94104
}
95105
}
@@ -100,9 +110,17 @@ impl ExperimentalConfig {
100110
&& is_default_token_ttl(&self.compat_token_ttl)
101111
&& self.inactive_session_expiration.is_none()
102112
&& self.plan_management_iframe_uri.is_none()
113+
&& self.session_limit.is_none()
103114
}
104115
}
105116

106117
impl ConfigurationSection for ExperimentalConfig {
107118
const PATH: Option<&'static str> = Some("experimental");
108119
}
120+
121+
/// Configuration options for the session limit feature
122+
#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)]
123+
pub struct SessionLimitConfig {
124+
pub soft_limit: NonZeroU64,
125+
pub hard_limit: NonZeroU64,
126+
}

crates/data-model/src/lib.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ pub use self::{
3939
DeviceCodeGrantState, InvalidRedirectUriError, JwksOrJwksUri, Pkce, Session, SessionState,
4040
},
4141
policy_data::PolicyData,
42-
site_config::{CaptchaConfig, CaptchaService, SessionExpirationConfig, SiteConfig},
42+
site_config::{
43+
CaptchaConfig, CaptchaService, SessionExpirationConfig, SessionLimitConfig, SiteConfig,
44+
},
4345
tokens::{
4446
AccessToken, AccessTokenState, RefreshToken, RefreshTokenState, TokenFormatError, TokenType,
4547
},

crates/data-model/src/site_config.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44
// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
55
// Please see LICENSE files in the repository root for full details.
66

7+
use std::num::NonZeroU64;
8+
79
use chrono::Duration;
10+
use serde::Serialize;
811
use url::Url;
912

1013
/// Which Captcha service is being used
@@ -36,6 +39,12 @@ pub struct SessionExpirationConfig {
3639
pub compat_session_inactivity_ttl: Option<Duration>,
3740
}
3841

42+
#[derive(Serialize, Debug, Clone)]
43+
pub struct SessionLimitConfig {
44+
pub soft_limit: NonZeroU64,
45+
pub hard_limit: NonZeroU64,
46+
}
47+
3948
/// Random site configuration we want accessible in various places.
4049
#[allow(clippy::struct_excessive_bools)]
4150
#[derive(Debug, Clone)]
@@ -99,4 +108,7 @@ pub struct SiteConfig {
99108

100109
/// The iframe URL to show in the plan tab of the UI
101110
pub plan_management_iframe_uri: Option<String>,
111+
112+
/// Limits on the number of application sessions that each user can have
113+
pub session_limit: Option<SessionLimitConfig>,
102114
}

crates/handlers/src/oauth2/authorization/consent.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use super::callback::CallbackDestination;
3232
use crate::{
3333
BoundActivityTracker, PreferredLanguage, impl_from_error_for_route,
3434
oauth2::generate_id_token,
35-
session::{SessionOrFallback, load_session_or_fallback},
35+
session::{SessionOrFallback, count_user_sessions_for_limiting, load_session_or_fallback},
3636
};
3737

3838
#[derive(Debug, Error)]
@@ -136,10 +136,13 @@ pub(crate) async fn get(
136136

137137
let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng);
138138

139+
let session_counts = count_user_sessions_for_limiting(&mut repo, &session.user).await?;
140+
139141
let res = policy
140142
.evaluate_authorization_grant(mas_policy::AuthorizationGrantInput {
141143
user: Some(&session.user),
142144
client: &client,
145+
session_counts: Some(session_counts),
143146
scope: &grant.scope,
144147
grant_type: mas_policy::GrantType::AuthorizationCode,
145148
requester: mas_policy::Requester {
@@ -235,10 +238,13 @@ pub(crate) async fn post(
235238
return Err(RouteError::GrantNotPending(grant.id));
236239
}
237240

241+
let session_counts = count_user_sessions_for_limiting(&mut repo, &browser_session.user).await?;
242+
238243
let res = policy
239244
.evaluate_authorization_grant(mas_policy::AuthorizationGrantInput {
240245
user: Some(&browser_session.user),
241246
client: &client,
247+
session_counts: Some(session_counts),
242248
scope: &grant.scope,
243249
grant_type: mas_policy::GrantType::AuthorizationCode,
244250
requester: mas_policy::Requester {

crates/handlers/src/oauth2/device/consent.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use ulid::Ulid;
2727

2828
use crate::{
2929
BoundActivityTracker, PreferredLanguage,
30-
session::{SessionOrFallback, load_session_or_fallback},
30+
session::{SessionOrFallback, count_user_sessions_for_limiting, load_session_or_fallback},
3131
};
3232

3333
#[derive(Deserialize, Debug)]
@@ -103,11 +103,14 @@ pub(crate) async fn get(
103103
.context("Client not found")
104104
.map_err(InternalError::from_anyhow)?;
105105

106+
let session_counts = count_user_sessions_for_limiting(&mut repo, &session.user).await?;
107+
106108
// Evaluate the policy
107109
let res = policy
108110
.evaluate_authorization_grant(mas_policy::AuthorizationGrantInput {
109111
grant_type: mas_policy::GrantType::DeviceCode,
110112
client: &client,
113+
session_counts: Some(session_counts),
111114
scope: &grant.scope,
112115
user: Some(&session.user),
113116
requester: mas_policy::Requester {
@@ -205,11 +208,14 @@ pub(crate) async fn post(
205208
.context("Client not found")
206209
.map_err(InternalError::from_anyhow)?;
207210

211+
let session_counts = count_user_sessions_for_limiting(&mut repo, &session.user).await?;
212+
208213
// Evaluate the policy
209214
let res = policy
210215
.evaluate_authorization_grant(mas_policy::AuthorizationGrantInput {
211216
grant_type: mas_policy::GrantType::DeviceCode,
212217
client: &client,
218+
session_counts: Some(session_counts),
213219
scope: &grant.scope,
214220
user: Some(&session.user),
215221
requester: mas_policy::Requester {

crates/handlers/src/oauth2/token.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,7 @@ async fn client_credentials_grant(
781781
.evaluate_authorization_grant(mas_policy::AuthorizationGrantInput {
782782
user: None,
783783
client,
784+
session_counts: None,
784785
scope: &scope,
785786
grant_type: mas_policy::GrantType::ClientCredentials,
786787
requester: mas_policy::Requester {

crates/handlers/src/session.rs

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,13 @@
88
99
use axum::response::{Html, IntoResponse as _, Response};
1010
use mas_axum_utils::{SessionInfoExt, cookies::CookieJar, csrf::CsrfExt};
11-
use mas_data_model::{BrowserSession, Clock};
11+
use mas_data_model::{BrowserSession, Clock, User};
1212
use mas_i18n::DataLocale;
13-
use mas_storage::{BoxRepository, RepositoryError};
13+
use mas_policy::model::SessionCounts;
14+
use mas_storage::{
15+
BoxRepository, RepositoryError, compat::CompatSessionFilter, oauth2::OAuth2SessionFilter,
16+
personal::PersonalSessionFilter,
17+
};
1418
use mas_templates::{AccountInactiveContext, TemplateContext, Templates};
1519
use rand::RngCore;
1620
use thiserror::Error;
@@ -102,3 +106,62 @@ pub async fn load_session_or_fallback(
102106
maybe_session: Some(session),
103107
})
104108
}
109+
110+
/// Get a count of sessions for the given user, for the purposes of session
111+
/// limiting.
112+
///
113+
/// Includes:
114+
/// - OAuth 2 sessions
115+
/// - Compatibility sessions
116+
/// - Personal sessions (unless owned by a different user)
117+
///
118+
/// # Backstory
119+
///
120+
/// Originally, we were only intending to count sessions with devices in this
121+
/// result, because those are the entries that are expensive for Synapse and
122+
/// also would not hinder use of deviceless clients (like Element Admin, an
123+
/// admin dashboard).
124+
///
125+
/// However, to do so, we would need to count only sessions including device
126+
/// scopes. To do this efficiently, we'd need a partial index on sessions
127+
/// including device scopes.
128+
///
129+
/// It turns out that this can't be done cleanly (as we need to, in Postgres,
130+
/// match scope lists where one of the scopes matches one of 2 known prefixes),
131+
/// at least not without somewhat uncomfortable stored functions.
132+
///
133+
/// So for simplicity's sake, we now count all sessions.
134+
/// For practical use cases, it's not likely to make a noticeable difference
135+
/// (and maybe it's good that there's an overall limit).
136+
pub(crate) async fn count_user_sessions_for_limiting(
137+
repo: &mut BoxRepository,
138+
user: &User,
139+
) -> Result<SessionCounts, RepositoryError> {
140+
let oauth2 = repo
141+
.oauth2_session()
142+
.count(OAuth2SessionFilter::new().active_only().for_user(user))
143+
.await? as u64;
144+
145+
let compat = repo
146+
.compat_session()
147+
.count(CompatSessionFilter::new().active_only().for_user(user))
148+
.await? as u64;
149+
150+
// Only include self-owned personal sessions, not administratively-owned ones
151+
let personal = repo
152+
.personal_session()
153+
.count(
154+
PersonalSessionFilter::new()
155+
.active_only()
156+
.for_actor_user(user)
157+
.for_owner_user(user),
158+
)
159+
.await? as u64;
160+
161+
Ok(SessionCounts {
162+
total: oauth2 + compat + personal,
163+
oauth2,
164+
compat,
165+
personal,
166+
})
167+
}

0 commit comments

Comments
 (0)