From b23a35a214ee96753b2a50e28261600d3d420504 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Tue, 21 Oct 2025 09:34:47 +0100 Subject: [PATCH 1/7] Rename record_personal_session function --- crates/handlers/src/activity_tracker/mod.rs | 4 ++-- crates/handlers/src/oauth2/introspection.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/handlers/src/activity_tracker/mod.rs b/crates/handlers/src/activity_tracker/mod.rs index 1cbaec877..e1c6b976f 100644 --- a/crates/handlers/src/activity_tracker/mod.rs +++ b/crates/handlers/src/activity_tracker/mod.rs @@ -113,8 +113,8 @@ impl ActivityTracker { } } - /// Record activity in a personal access token session. - pub async fn record_personal_access_token_session( + /// Record activity in a personal session. + pub async fn record_personal_session( &self, clock: &dyn Clock, session: &PersonalSession, diff --git a/crates/handlers/src/oauth2/introspection.rs b/crates/handlers/src/oauth2/introspection.rs index 754eaa942..17f508921 100644 --- a/crates/handlers/src/oauth2/introspection.rs +++ b/crates/handlers/src/oauth2/introspection.rs @@ -700,7 +700,7 @@ pub(crate) async fn post( }; activity_tracker - .record_personal_access_token_session(&clock, &session, ip) + .record_personal_session(&clock, &session, ip) .await; INTROSPECTION_COUNTER.add( From c74150f8dfba4a362a97b54a8fb3d2a8dd9e0f4a Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Tue, 21 Oct 2025 09:36:50 +0100 Subject: [PATCH 2/7] Accept PATs on the Admin API --- crates/handlers/src/activity_tracker/bound.rs | 11 +- crates/handlers/src/admin/call_context.rs | 127 ++++++++++++++---- .../src/admin/v1/personal_sessions/add.rs | 11 +- .../src/admin/v1/personal_sessions/mod.rs | 21 +++ .../admin/v1/personal_sessions/regenerate.rs | 9 +- 5 files changed, 135 insertions(+), 44 deletions(-) diff --git a/crates/handlers/src/activity_tracker/bound.rs b/crates/handlers/src/activity_tracker/bound.rs index 14d36fb7c..8f7acbdde 100644 --- a/crates/handlers/src/activity_tracker/bound.rs +++ b/crates/handlers/src/activity_tracker/bound.rs @@ -6,7 +6,9 @@ use std::net::IpAddr; -use mas_data_model::{BrowserSession, Clock, CompatSession, Session}; +use mas_data_model::{ + BrowserSession, Clock, CompatSession, Session, personal::session::PersonalSession, +}; use crate::activity_tracker::ActivityTracker; @@ -37,6 +39,13 @@ impl Bound { .await; } + /// Record activity in a personal session. + pub async fn record_personal_session(&self, clock: &dyn Clock, session: &PersonalSession) { + self.tracker + .record_personal_session(clock, session, self.ip) + .await; + } + /// Record activity in a compatibility session. pub async fn record_compat_session(&self, clock: &dyn Clock, session: &CompatSession) { self.tracker diff --git a/crates/handlers/src/admin/call_context.rs b/crates/handlers/src/admin/call_context.rs index ebe0e02e5..2630dfe1b 100644 --- a/crates/handlers/src/admin/call_context.rs +++ b/crates/handlers/src/admin/call_context.rs @@ -16,8 +16,9 @@ use axum_extra::TypedHeader; use headers::{Authorization, authorization::Bearer}; use hyper::StatusCode; use mas_axum_utils::record_error; -use mas_data_model::{BoxClock, Session, User}; +use mas_data_model::{BoxClock, Session, TokenType, User, personal::session::PersonalSession}; use mas_storage::{BoxRepository, RepositoryError}; +use oauth2_types::scope::Scope; use ulid::Ulid; use super::response::ErrorResponse; @@ -41,6 +42,10 @@ pub enum Rejection { #[error("Invalid repository operation")] Repository(#[from] RepositoryError), + /// The access token was not of the correct type for the Admin API + #[error("Invalid type of access token")] + InvalidAccessTokenType, + /// The access token could not be found in the database #[error("Unknown access token")] UnknownAccessToken, @@ -90,7 +95,8 @@ impl IntoResponse for Rejection { | Rejection::TokenExpired | Rejection::SessionRevoked | Rejection::UserLocked - | Rejection::MissingScope => StatusCode::UNAUTHORIZED, + | Rejection::MissingScope + | Rejection::InvalidAccessTokenType => StatusCode::UNAUTHORIZED, Rejection::RepositorySetup(_) | Rejection::Repository(_) @@ -113,7 +119,7 @@ pub struct CallContext { pub repo: BoxRepository, pub clock: BoxClock, pub user: Option, - pub session: Session, + pub session: CallerSession, } impl FromRequestParts for CallContext @@ -154,28 +160,76 @@ where })?; let token = token.token(); + let token_type = TokenType::check(token).or(Err(Rejection::InvalidAccessTokenType))?; - // Look for the access token in the database - let token = repo - .oauth2_access_token() - .find_by_token(token) - .await? - .ok_or(Rejection::UnknownAccessToken)?; + let session = match token_type { + TokenType::AccessToken => { + // Look for the access token in the database + let token = repo + .oauth2_access_token() + .find_by_token(token) + .await? + .ok_or(Rejection::UnknownAccessToken)?; - // Look for the associated session in the database - let session = repo - .oauth2_session() - .lookup(token.session_id) - .await? - .ok_or_else(|| Rejection::LoadSession(token.session_id))?; + // Look for the associated session in the database + let session = repo + .oauth2_session() + .lookup(token.session_id) + .await? + .ok_or_else(|| Rejection::LoadSession(token.session_id))?; - // Record the activity on the session - activity_tracker - .record_oauth2_session(&clock, &session) - .await; + if !session.is_valid() { + return Err(Rejection::SessionRevoked); + } + + if !token.is_valid(clock.now()) { + return Err(Rejection::TokenExpired); + } + + // Record the activity on the session + activity_tracker + .record_oauth2_session(&clock, &session) + .await; + + CallerSession::OAuth2Session(session) + } + TokenType::PersonalAccessToken => { + // Look for the access token in the database + let token = repo + .personal_access_token() + .find_by_token(token) + .await? + .ok_or(Rejection::UnknownAccessToken)?; + + // Look for the associated session in the database + let session = repo + .personal_session() + .lookup(token.session_id) + .await? + .ok_or_else(|| Rejection::LoadSession(token.session_id))?; + + if !session.is_valid() { + return Err(Rejection::SessionRevoked); + } + + if !token.is_valid(clock.now()) { + return Err(Rejection::TokenExpired); + } + + // Record the activity on the session + activity_tracker + .record_personal_session(&clock, &session) + .await; + + CallerSession::PersonalSession(session) + } + _other => { + return Err(Rejection::InvalidAccessTokenType); + } + }; // Load the user if there is one - let user = if let Some(user_id) = session.user_id { + let user = if let Some(user_id) = session.user_id() { let user = repo .user() .lookup(user_id) @@ -193,17 +247,9 @@ where return Err(Rejection::UserLocked); } - if !session.is_valid() { - return Err(Rejection::SessionRevoked); - } - - if !token.is_valid(clock.now()) { - return Err(Rejection::TokenExpired); - } - // For now, we only check that the session has the admin scope // Later we might want to check other route-specific scopes - if !session.scope.contains("urn:mas:admin") { + if !session.scope().contains("urn:mas:admin") { return Err(Rejection::MissingScope); } @@ -215,3 +261,26 @@ where }) } } + +/// The session representing the caller of the Admin API; +/// could either be an OAuth session or a personal session. +pub enum CallerSession { + OAuth2Session(Session), + PersonalSession(PersonalSession), +} + +impl CallerSession { + pub fn scope(&self) -> &Scope { + match self { + CallerSession::OAuth2Session(session) => &session.scope, + CallerSession::PersonalSession(session) => &session.scope, + } + } + + pub fn user_id(&self) -> Option { + match self { + CallerSession::OAuth2Session(session) => session.user_id, + CallerSession::PersonalSession(session) => Some(session.actor_user_id), + } + } +} diff --git a/crates/handlers/src/admin/v1/personal_sessions/add.rs b/crates/handlers/src/admin/v1/personal_sessions/add.rs index 5a7bb5a0e..5da3929e5 100644 --- a/crates/handlers/src/admin/v1/personal_sessions/add.rs +++ b/crates/handlers/src/admin/v1/personal_sessions/add.rs @@ -8,7 +8,7 @@ use axum::{Json, response::IntoResponse}; use chrono::Duration; use hyper::StatusCode; use mas_axum_utils::record_error; -use mas_data_model::{BoxRng, TokenType, personal::session::PersonalSessionOwner}; +use mas_data_model::{BoxRng, TokenType}; use oauth2_types::scope::Scope; use schemars::JsonSchema; use serde::Deserialize; @@ -19,6 +19,7 @@ use crate::{ call_context::CallContext, model::{InconsistentPersonalSession, PersonalSession}, response::{ErrorResponse, SingleResponse}, + v1::personal_sessions::personal_session_owner_from_caller, }, impl_from_error_for_route, }; @@ -100,13 +101,7 @@ pub async fn handler( NoApi(mut rng): NoApi, Json(params): Json, ) -> Result<(StatusCode, Json>), RouteError> { - let owner = if let Some(user_id) = session.user_id { - // User-owned session - PersonalSessionOwner::User(user_id) - } else { - // No admin user means this is a client-owned session - PersonalSessionOwner::OAuth2Client(session.client_id) - }; + let owner = personal_session_owner_from_caller(&session); let actor_user = repo .user() diff --git a/crates/handlers/src/admin/v1/personal_sessions/mod.rs b/crates/handlers/src/admin/v1/personal_sessions/mod.rs index d876d6544..37c591b09 100644 --- a/crates/handlers/src/admin/v1/personal_sessions/mod.rs +++ b/crates/handlers/src/admin/v1/personal_sessions/mod.rs @@ -9,6 +9,8 @@ mod list; mod regenerate; mod revoke; +use mas_data_model::personal::session::PersonalSessionOwner; + pub use self::{ add::{doc as add_doc, handler as add}, get::{doc as get_doc, handler as get}, @@ -16,3 +18,22 @@ pub use self::{ regenerate::{doc as regenerate_doc, handler as regenerate}, revoke::{doc as revoke_doc, handler as revoke}, }; +use crate::admin::call_context::CallerSession; + +/// Given the [`CallerSession`] of a caller of the Admin API, +/// return the [`PersonalSessionOwner`] that should own created personal +/// sessions. +fn personal_session_owner_from_caller(caller: &CallerSession) -> PersonalSessionOwner { + match caller { + CallerSession::OAuth2Session(session) => { + if let Some(user_id) = session.user_id { + PersonalSessionOwner::User(user_id) + } else { + PersonalSessionOwner::OAuth2Client(session.client_id) + } + } + CallerSession::PersonalSession(session) => { + PersonalSessionOwner::User(session.actor_user_id) + } + } +} diff --git a/crates/handlers/src/admin/v1/personal_sessions/regenerate.rs b/crates/handlers/src/admin/v1/personal_sessions/regenerate.rs index e639222e4..e6c70679f 100644 --- a/crates/handlers/src/admin/v1/personal_sessions/regenerate.rs +++ b/crates/handlers/src/admin/v1/personal_sessions/regenerate.rs @@ -8,7 +8,7 @@ use axum::{Json, response::IntoResponse}; use chrono::Duration; use hyper::StatusCode; use mas_axum_utils::record_error; -use mas_data_model::{BoxRng, TokenType, personal::session::PersonalSessionOwner}; +use mas_data_model::{BoxRng, TokenType}; use schemars::JsonSchema; use serde::Deserialize; use tracing::error; @@ -19,6 +19,7 @@ use crate::{ model::{InconsistentPersonalSession, PersonalSession}, params::UlidPathParam, response::{ErrorResponse, SingleResponse}, + v1::personal_sessions::personal_session_owner_from_caller, }, impl_from_error_for_route, }; @@ -111,11 +112,7 @@ pub async fn handler( // If the owner is not the current caller, then currently we reject the // regeneration. - let caller = if let Some(user_id) = caller_session.user_id { - PersonalSessionOwner::User(user_id) - } else { - PersonalSessionOwner::OAuth2Client(caller_session.client_id) - }; + let caller = personal_session_owner_from_caller(&caller_session); if session.owner != caller { return Err(RouteError::SessionNotYours); } From a7d83540c2d218485180c6e866f0247bc39ee813 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Wed, 22 Oct 2025 12:59:49 +0100 Subject: [PATCH 3/7] Pass through the TokenFormatError --- crates/handlers/src/admin/call_context.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/handlers/src/admin/call_context.rs b/crates/handlers/src/admin/call_context.rs index 2630dfe1b..56707d2df 100644 --- a/crates/handlers/src/admin/call_context.rs +++ b/crates/handlers/src/admin/call_context.rs @@ -16,7 +16,9 @@ use axum_extra::TypedHeader; use headers::{Authorization, authorization::Bearer}; use hyper::StatusCode; use mas_axum_utils::record_error; -use mas_data_model::{BoxClock, Session, TokenType, User, personal::session::PersonalSession}; +use mas_data_model::{ + BoxClock, Session, TokenFormatError, TokenType, User, personal::session::PersonalSession, +}; use mas_storage::{BoxRepository, RepositoryError}; use oauth2_types::scope::Scope; use ulid::Ulid; @@ -44,7 +46,11 @@ pub enum Rejection { /// The access token was not of the correct type for the Admin API #[error("Invalid type of access token")] - InvalidAccessTokenType, + InvalidAccessTokenType( + #[source] + #[from] + Option, + ), /// The access token could not be found in the database #[error("Unknown access token")] @@ -96,7 +102,7 @@ impl IntoResponse for Rejection { | Rejection::SessionRevoked | Rejection::UserLocked | Rejection::MissingScope - | Rejection::InvalidAccessTokenType => StatusCode::UNAUTHORIZED, + | Rejection::InvalidAccessTokenType(_) => StatusCode::UNAUTHORIZED, Rejection::RepositorySetup(_) | Rejection::Repository(_) @@ -160,7 +166,7 @@ where })?; let token = token.token(); - let token_type = TokenType::check(token).or(Err(Rejection::InvalidAccessTokenType))?; + let token_type = TokenType::check(token)?; let session = match token_type { TokenType::AccessToken => { @@ -224,7 +230,7 @@ where CallerSession::PersonalSession(session) } _other => { - return Err(Rejection::InvalidAccessTokenType); + return Err(Rejection::InvalidAccessTokenType(None)); } }; From f51747a666bf6e6c91feeafb24dc3472a1f55cb2 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Wed, 22 Oct 2025 13:04:39 +0100 Subject: [PATCH 4/7] Check validity of token owner --- crates/handlers/src/admin/call_context.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/handlers/src/admin/call_context.rs b/crates/handlers/src/admin/call_context.rs index 56707d2df..08e535cc9 100644 --- a/crates/handlers/src/admin/call_context.rs +++ b/crates/handlers/src/admin/call_context.rs @@ -17,7 +17,8 @@ use headers::{Authorization, authorization::Bearer}; use hyper::StatusCode; use mas_axum_utils::record_error; use mas_data_model::{ - BoxClock, Session, TokenFormatError, TokenType, User, personal::session::PersonalSession, + BoxClock, Session, TokenFormatError, TokenType, User, + personal::session::{PersonalSession, PersonalSessionOwner}, }; use mas_storage::{BoxRepository, RepositoryError}; use oauth2_types::scope::Scope; @@ -222,6 +223,23 @@ where return Err(Rejection::TokenExpired); } + // Check the validity of the owner of the personal session + match session.owner { + PersonalSessionOwner::User(owner_user_id) => { + let owner_user = repo + .user() + .lookup(owner_user_id) + .await? + .ok_or_else(|| Rejection::LoadUser(owner_user_id))?; + if !owner_user.is_valid() { + return Err(Rejection::UserLocked); + } + } + PersonalSessionOwner::OAuth2Client(_) => { + // nop: Client owners are always valid + } + } + // Record the activity on the session activity_tracker .record_personal_session(&clock, &session) From c8ed12512eb154fb73c6205281e7686fea7ba281 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Wed, 22 Oct 2025 13:15:12 +0100 Subject: [PATCH 5/7] Relax the validity check of the token actor --- crates/data-model/src/users.rs | 14 ++++++++++++++ crates/handlers/src/admin/call_context.rs | 19 ++++++++++++++----- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/crates/data-model/src/users.rs b/crates/data-model/src/users.rs index 5221f4867..7c7da6293 100644 --- a/crates/data-model/src/users.rs +++ b/crates/data-model/src/users.rs @@ -30,6 +30,20 @@ impl User { pub fn is_valid(&self) -> bool { self.locked_at.is_none() && self.deactivated_at.is_none() } + + /// Returns `true` if the user is a valid actor, for example + /// of a personal session. + /// + /// Currently: this is `true` unless the user is deactivated. + /// + /// This is a weaker form of validity: `is_valid` always implies + /// `is_valid_actor`, but some users (currently: locked users) + /// can be valid actors for personal sessions but aren't valid + /// except through administrative access. + #[must_use] + pub fn is_valid_actor(&self) -> bool { + self.deactivated_at.is_none() + } } impl User { diff --git a/crates/handlers/src/admin/call_context.rs b/crates/handlers/src/admin/call_context.rs index 08e535cc9..732ea41e0 100644 --- a/crates/handlers/src/admin/call_context.rs +++ b/crates/handlers/src/admin/call_context.rs @@ -264,11 +264,20 @@ where None }; - // If there is a user for this session, check that it is not locked - if let Some(user) = &user - && !user.is_valid() - { - return Err(Rejection::UserLocked); + if let CallerSession::PersonalSession(_) = &session { + // For personal sessions: check that the actor is valid enough + // to be an actor. + // unwrap: personal sessions always have an actor user + if !user.as_ref().unwrap().is_valid_actor() { + return Err(Rejection::UserLocked); + } + } else { + // If there is a user for this session, check that it is not locked + if let Some(user) = &user + && !user.is_valid() + { + return Err(Rejection::UserLocked); + } } // For now, we only check that the session has the admin scope From 84450a7bfb8b327bd0b6d09838b26e936580d505 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Wed, 22 Oct 2025 13:29:45 +0100 Subject: [PATCH 6/7] remove redundant #[source] --- crates/handlers/src/admin/call_context.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/handlers/src/admin/call_context.rs b/crates/handlers/src/admin/call_context.rs index 732ea41e0..6c103823b 100644 --- a/crates/handlers/src/admin/call_context.rs +++ b/crates/handlers/src/admin/call_context.rs @@ -47,11 +47,7 @@ pub enum Rejection { /// The access token was not of the correct type for the Admin API #[error("Invalid type of access token")] - InvalidAccessTokenType( - #[source] - #[from] - Option, - ), + InvalidAccessTokenType(#[from] Option), /// The access token could not be found in the database #[error("Unknown access token")] From e5a54f2d685ad3f8e4f4de6c5f50dfe55eda9856 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Wed, 22 Oct 2025 13:29:53 +0100 Subject: [PATCH 7/7] Restructure user validity check --- crates/handlers/src/admin/call_context.rs | 36 +++++++++++++---------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/crates/handlers/src/admin/call_context.rs b/crates/handlers/src/admin/call_context.rs index 6c103823b..1cffe682e 100644 --- a/crates/handlers/src/admin/call_context.rs +++ b/crates/handlers/src/admin/call_context.rs @@ -255,27 +255,31 @@ where .lookup(user_id) .await? .ok_or_else(|| Rejection::LoadUser(user_id))?; + + match session { + CallerSession::OAuth2Session(_) => { + // For OAuth2 sessions: check that the user is valid enough + // to be a user. + if !user.is_valid() { + return Err(Rejection::UserLocked); + } + } + CallerSession::PersonalSession(_) => { + // For personal sessions: check that the actor is valid enough + // to be an actor. + if !user.is_valid_actor() { + return Err(Rejection::UserLocked); + } + } + } + Some(user) } else { + // Double check we're not using a PersonalSession + assert!(matches!(session, CallerSession::OAuth2Session(_))); None }; - if let CallerSession::PersonalSession(_) = &session { - // For personal sessions: check that the actor is valid enough - // to be an actor. - // unwrap: personal sessions always have an actor user - if !user.as_ref().unwrap().is_valid_actor() { - return Err(Rejection::UserLocked); - } - } else { - // If there is a user for this session, check that it is not locked - if let Some(user) = &user - && !user.is_valid() - { - return Err(Rejection::UserLocked); - } - } - // For now, we only check that the session has the admin scope // Later we might want to check other route-specific scopes if !session.scope().contains("urn:mas:admin") {