fix(auth): revoke JWT access on deactivation, deletion and role change
JWT access tokens freeze role/identity at login (access 1h, refresh 7d) and validated tokens are cached for 30s. The Bearer and cookie auth paths trusted claims.role and never re-checked the account, so demoting an admin, or disabling/deleting an account, did not revoke access until the token expired. The app-password path already re-read role/active from the DB; only the JWT/cookie path had the gap. Re-validate the caller against the live user record on the token path via the already-cached get_user_flags (role / is_external / active), bounded by USER_FLAGS_CACHE_TTL and invalidated eagerly on set_user_active / change_user_role / delete_user_admin: - middleware/user.rs: new resolve_live_role helper (+ pure decide_live_role core) — returns the *current* role, rejects deleted (NotFound) and deactivated accounts, and fails open on transient lookup errors (mirrors require_internal_user). Login/refresh remain the canonical active gate. - middleware/auth.rs: auth_middleware (Bearer + cookie) now populates CurrentUser with the live role and rejects revoked accounts (Bearer -> 401 AccountInactive; cookie -> fall through to 401/login redirect). require_admin emits an audit line on denial. - middleware/admin.rs: require_admin / require_authenticated re-check the live record, return the live role, and audit admin denials. Downstream admin gates (dedup_handler, subject_group_handler, OCS) inherit the live role automatically via CurrentUser / require_authenticated. Tests: decide_live_role policy (active / demoted / deactivated / deleted / transient fail-open) and AccountInactive -> 401. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TAzLEQDaLak3dnrEN3YT35
This commit is contained in:
@@ -14,6 +14,7 @@ use crate::application::ports::auth_ports::TokenServicePort;
|
||||
use crate::common::di::AppState;
|
||||
use crate::interfaces::api::cookie_auth::{ACCESS_COOKIE, extract_cookie_value};
|
||||
use crate::interfaces::errors::AppError;
|
||||
use crate::interfaces::middleware::user::{LiveRole, resolve_live_role};
|
||||
|
||||
/// Validate the request's JWT (from the `Authorization: Bearer …` header
|
||||
/// or the access-token cookie) and require `claims.role == "admin"`.
|
||||
@@ -43,19 +44,37 @@ pub async fn require_admin(
|
||||
.validate_token(&token)
|
||||
.map_err(|e| AppError::unauthorized(format!("Invalid token: {}", e)))?;
|
||||
|
||||
if claims.role != "admin" {
|
||||
return Err(AppError::new(
|
||||
StatusCode::FORBIDDEN,
|
||||
"Admin access required",
|
||||
"Forbidden",
|
||||
));
|
||||
}
|
||||
let user_id = Uuid::parse_str(&claims.sub)
|
||||
.map_err(|_| AppError::internal_error("Invalid user ID in token"))?;
|
||||
|
||||
Ok((
|
||||
Uuid::parse_str(&claims.sub)
|
||||
.map_err(|_| AppError::internal_error("Invalid user ID in token"))?,
|
||||
claims.role.clone(),
|
||||
))
|
||||
// Gate on the *live* role, not the JWT claim: a demotion or deactivation
|
||||
// must take effect within the flags-cache TTL rather than surviving until
|
||||
// the token expires.
|
||||
match resolve_live_role(
|
||||
auth.auth_application_service.as_ref(),
|
||||
user_id,
|
||||
&claims.role,
|
||||
)
|
||||
.await
|
||||
{
|
||||
LiveRole::Active(role) if role == "admin" => Ok((user_id, role)),
|
||||
LiveRole::Active(role) => {
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "authz.admin_denied",
|
||||
reason = "not_admin",
|
||||
caller_id = %user_id,
|
||||
role = %role,
|
||||
"👮🏻♂️ admin-only endpoint denied for non-admin caller"
|
||||
);
|
||||
Err(AppError::new(
|
||||
StatusCode::FORBIDDEN,
|
||||
"Admin access required",
|
||||
"Forbidden",
|
||||
))
|
||||
}
|
||||
LiveRole::Revoked => Err(AppError::unauthorized("Account is no longer active")),
|
||||
}
|
||||
}
|
||||
|
||||
/// Validate the request's JWT (any role) and return `(user_id, role)`.
|
||||
@@ -84,9 +103,19 @@ pub async fn require_authenticated(
|
||||
.validate_token(&token)
|
||||
.map_err(|e| AppError::unauthorized(format!("Invalid token: {}", e)))?;
|
||||
|
||||
Ok((
|
||||
Uuid::parse_str(&claims.sub)
|
||||
.map_err(|_| AppError::internal_error("Invalid user ID in token"))?,
|
||||
claims.role.clone(),
|
||||
))
|
||||
let user_id = Uuid::parse_str(&claims.sub)
|
||||
.map_err(|_| AppError::internal_error("Invalid user ID in token"))?;
|
||||
|
||||
// Reject tokens whose account was deactivated/deleted, and return the
|
||||
// caller's live role rather than the (possibly stale) JWT claim.
|
||||
match resolve_live_role(
|
||||
auth.auth_application_service.as_ref(),
|
||||
user_id,
|
||||
&claims.role,
|
||||
)
|
||||
.await
|
||||
{
|
||||
LiveRole::Active(role) => Ok((user_id, role)),
|
||||
LiveRole::Revoked => Err(AppError::unauthorized("Account is no longer active")),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ use crate::common::di::AppState;
|
||||
// Re-export CurrentUser from application layer for use in handlers
|
||||
pub use crate::application::dtos::user_dto::CurrentUser;
|
||||
use crate::application::ports::auth_ports::TokenServicePort;
|
||||
use crate::interfaces::middleware::user::{LiveRole, resolve_live_role};
|
||||
|
||||
/// Marker inserted into request extensions when the user was authenticated
|
||||
/// via the `oxicloud_access` HttpOnly cookie rather than a Bearer/Basic header.
|
||||
@@ -111,6 +112,9 @@ pub enum AuthError {
|
||||
#[error("User not found")]
|
||||
UserNotFound,
|
||||
|
||||
#[error("Account is no longer active")]
|
||||
AccountInactive,
|
||||
|
||||
#[error("Access denied: {0}")]
|
||||
AccessDenied(String),
|
||||
|
||||
@@ -127,6 +131,10 @@ impl IntoResponse for AuthError {
|
||||
AuthError::InvalidToken(msg) => (StatusCode::UNAUTHORIZED, msg),
|
||||
AuthError::TokenExpired => (StatusCode::UNAUTHORIZED, "Token expired".to_string()),
|
||||
AuthError::UserNotFound => (StatusCode::UNAUTHORIZED, "User not found".to_string()),
|
||||
AuthError::AccountInactive => (
|
||||
StatusCode::UNAUTHORIZED,
|
||||
"Account is no longer active".to_string(),
|
||||
),
|
||||
AuthError::AccessDenied(msg) => (StatusCode::FORBIDDEN, msg),
|
||||
AuthError::AuthServiceUnavailable => (
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
@@ -181,11 +189,26 @@ pub async fn auth_middleware(
|
||||
let user_id = Uuid::parse_str(&claims.sub).map_err(|_| {
|
||||
AuthError::InvalidToken("Invalid user ID in token".to_string())
|
||||
})?;
|
||||
// A cryptographically valid token must not outlive the
|
||||
// account: re-check the live record so deactivation,
|
||||
// deletion and demotion take effect within the flags-cache
|
||||
// TTL instead of waiting for token expiry. The returned
|
||||
// role is authoritative — never the frozen JWT claim.
|
||||
let role = match resolve_live_role(
|
||||
auth_service.auth_application_service.as_ref(),
|
||||
user_id,
|
||||
&claims.role,
|
||||
)
|
||||
.await
|
||||
{
|
||||
LiveRole::Active(role) => role,
|
||||
LiveRole::Revoked => return Err(AuthError::AccountInactive),
|
||||
};
|
||||
let current_user = Arc::new(CurrentUser {
|
||||
id: user_id,
|
||||
username: claims.username.clone(),
|
||||
email: claims.email.clone(),
|
||||
role: claims.role.clone(),
|
||||
role,
|
||||
});
|
||||
request.extensions_mut().insert(current_user);
|
||||
tracing::Span::current().record("user_id", user_id.to_string());
|
||||
@@ -280,16 +303,33 @@ pub async fn auth_middleware(
|
||||
let user_id = Uuid::parse_str(&claims.sub).map_err(|_| {
|
||||
AuthError::InvalidToken("Invalid user ID in token".to_string())
|
||||
})?;
|
||||
let current_user = Arc::new(CurrentUser {
|
||||
id: user_id,
|
||||
username: claims.username.clone(),
|
||||
email: claims.email.clone(),
|
||||
role: claims.role.clone(),
|
||||
});
|
||||
request.extensions_mut().insert(current_user);
|
||||
request.extensions_mut().insert(CookieAuthenticated);
|
||||
tracing::Span::current().record("user_id", user_id.to_string());
|
||||
return Ok(next.run(request).await);
|
||||
// Same live-account re-check as the Bearer path. On
|
||||
// revocation we fall through (rather than erroring) so the
|
||||
// browser receives the standard 401 and redirects to
|
||||
// /login, exactly like an invalid or expired cookie.
|
||||
match resolve_live_role(
|
||||
auth_service.auth_application_service.as_ref(),
|
||||
user_id,
|
||||
&claims.role,
|
||||
)
|
||||
.await
|
||||
{
|
||||
LiveRole::Active(role) => {
|
||||
let current_user = Arc::new(CurrentUser {
|
||||
id: user_id,
|
||||
username: claims.username.clone(),
|
||||
email: claims.email.clone(),
|
||||
role,
|
||||
});
|
||||
request.extensions_mut().insert(current_user);
|
||||
request.extensions_mut().insert(CookieAuthenticated);
|
||||
tracing::Span::current().record("user_id", user_id.to_string());
|
||||
return Ok(next.run(request).await);
|
||||
}
|
||||
LiveRole::Revoked => {
|
||||
// Fall through to the unauthenticated 401 / login redirect.
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
tracing::debug!("Cookie token validation failed: {}", e);
|
||||
@@ -344,8 +384,11 @@ fn dav_basic_auth_challenge(message: &'static str) -> Response {
|
||||
|
||||
/// Middleware to verify that the authenticated user has an admin role.
|
||||
///
|
||||
/// Must be applied AFTER auth_middleware, as it depends on
|
||||
/// `CurrentUser` being present in the request extensions.
|
||||
/// Must be applied AFTER auth_middleware, as it depends on `CurrentUser`
|
||||
/// being present in the request extensions. The role carried by
|
||||
/// `CurrentUser` is the *live* role resolved by `auth_middleware` (see
|
||||
/// [`resolve_live_role`]), not the JWT claim, so a demotion is honoured
|
||||
/// here within the flags-cache TTL.
|
||||
pub async fn require_admin(request: Request, next: Next) -> Response {
|
||||
// Get the CurrentUser inserted by auth_middleware
|
||||
if let Some(current_user) = request.extensions().get::<Arc<CurrentUser>>() {
|
||||
@@ -353,13 +396,21 @@ pub async fn require_admin(request: Request, next: Next) -> Response {
|
||||
tracing::debug!("Admin access granted for user: {}", current_user.username);
|
||||
return next.run(request).await;
|
||||
}
|
||||
tracing::warn!(
|
||||
"Admin access denied for user: {} (role: {})",
|
||||
current_user.username,
|
||||
current_user.role
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "authz.admin_denied",
|
||||
reason = "not_admin",
|
||||
caller_id = %current_user.id,
|
||||
role = %current_user.role,
|
||||
"👮🏻♂️ admin-only route denied for non-admin caller"
|
||||
);
|
||||
} else {
|
||||
tracing::warn!("Admin check failed: no authenticated user in request");
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "authz.admin_denied",
|
||||
reason = "unauthenticated",
|
||||
"👮🏻♂️ admin-only route reached with no authenticated user"
|
||||
);
|
||||
}
|
||||
|
||||
// Access denied
|
||||
@@ -415,4 +466,13 @@ mod tests {
|
||||
Some(r#"Basic realm="OxiCloud""#),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn account_inactive_maps_to_401() {
|
||||
// A token that is still cryptographically valid but whose account was
|
||||
// deactivated/deleted must be rejected with 401 (credentials no longer
|
||||
// valid), so browsers redirect to /login rather than seeing a 403.
|
||||
let resp = AuthError::AccountInactive.into_response();
|
||||
assert_eq!(resp.status(), StatusCode::UNAUTHORIZED);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -32,7 +32,8 @@ use uuid::Uuid;
|
||||
|
||||
use crate::application::services::auth_application_service::AuthApplicationService;
|
||||
use crate::common::di::AppState;
|
||||
use crate::domain::entities::user::UserRole;
|
||||
use crate::domain::entities::user::{UserFlags, UserRole};
|
||||
use crate::domain::errors::{DomainError, ErrorKind};
|
||||
use crate::interfaces::errors::AppError;
|
||||
use crate::interfaces::middleware::auth::CurrentUser;
|
||||
|
||||
@@ -102,6 +103,91 @@ pub async fn require_admin_user(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Outcome of re-checking a token-authenticated caller against the live
|
||||
/// user record (see [`resolve_live_role`]).
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum LiveRole {
|
||||
/// The account exists and is active. Carries the caller's *current*
|
||||
/// role string (`"admin"` / `"user"`), which is authoritative and
|
||||
/// supersedes the — possibly stale — JWT `role` claim.
|
||||
Active(String),
|
||||
/// The account is deactivated or deleted: the request must be rejected
|
||||
/// even though its token is still cryptographically valid.
|
||||
Revoked,
|
||||
}
|
||||
|
||||
/// Re-validate a caller carried by a still-valid token against the live
|
||||
/// user record, so deactivation, deletion and role changes take effect
|
||||
/// within [`USER_FLAGS_CACHE_TTL`](crate::application::services::auth_application_service)
|
||||
/// instead of waiting for the token to expire (access 1 h / refresh 7 d by
|
||||
/// default).
|
||||
///
|
||||
/// JWT claims — `role` included — are frozen at login. Without this check a
|
||||
/// demoted admin keeps admin power, and a disabled or deleted account keeps
|
||||
/// full access, until its token expires. Returning the *current* role lets
|
||||
/// every caller stop trusting `claims.role`.
|
||||
///
|
||||
/// Cost: the short-TTL-cached `get_user_flags` (no `image` column), so ~one
|
||||
/// tiny indexed query per user per cache-TTL window; admin role/active
|
||||
/// changes invalidate the entry eagerly for immediate effect.
|
||||
///
|
||||
/// Availability stance mirrors [`require_internal_user`]: a *transient*
|
||||
/// lookup failure fails OPEN with the claim role (a DB blip must not lock
|
||||
/// every authenticated user out, and login/refresh already enforce `active`
|
||||
/// at the canonical layer). A *missing* row (`NotFound`) is a definitive
|
||||
/// revocation and fails CLOSED.
|
||||
pub async fn resolve_live_role(
|
||||
auth: &AuthApplicationService,
|
||||
user_id: Uuid,
|
||||
claim_role: &str,
|
||||
) -> LiveRole {
|
||||
decide_live_role(auth.get_user_flags(user_id).await, user_id, claim_role)
|
||||
}
|
||||
|
||||
/// Pure decision core of [`resolve_live_role`], split out so the
|
||||
/// allow/revoke/fail-open policy is unit-testable without a service or DB.
|
||||
fn decide_live_role(
|
||||
flags: Result<UserFlags, DomainError>,
|
||||
user_id: Uuid,
|
||||
claim_role: &str,
|
||||
) -> LiveRole {
|
||||
match flags {
|
||||
Ok(flags) if flags.active => LiveRole::Active(flags.role.to_string()),
|
||||
Ok(_) => {
|
||||
audit_token_revoked(user_id, "deactivated");
|
||||
LiveRole::Revoked
|
||||
}
|
||||
// The user row is gone — a definitive revocation; fail closed.
|
||||
Err(e) if matches!(e.kind, ErrorKind::NotFound) => {
|
||||
audit_token_revoked(user_id, "deleted");
|
||||
LiveRole::Revoked
|
||||
}
|
||||
// Transient lookup failure (DB blip): fail open on the claim role so
|
||||
// a momentary outage doesn't 401 every authenticated user at once.
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
user_id = %user_id,
|
||||
error = %e,
|
||||
"live-user re-check failed transiently; allowing request on the JWT claim role (fail-open)"
|
||||
);
|
||||
LiveRole::Active(claim_role.to_string())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Audit a request rejected because the token outlived the account's access
|
||||
/// (deactivation or deletion). Anti-enumeration is not a concern — the
|
||||
/// subject is the caller's own account.
|
||||
fn audit_token_revoked(user_id: Uuid, reason: &'static str) {
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "auth.token_revoked",
|
||||
reason = reason,
|
||||
caller_id = %user_id,
|
||||
"👮🏻♂️ valid token presented for an account that is no longer active — rejected"
|
||||
);
|
||||
}
|
||||
|
||||
/// Axum middleware layer that blocks external users from a whole route
|
||||
/// subtree. Apply via `.layer(from_fn_with_state(state, require_internal_user_layer))`
|
||||
/// on the protocol nests (CalDAV / CardDAV / WebDAV) that have no
|
||||
@@ -153,3 +239,52 @@ pub async fn require_internal_user_layer(
|
||||
|
||||
next.run(request).await
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn flags(role: UserRole, active: bool) -> UserFlags {
|
||||
UserFlags {
|
||||
role,
|
||||
is_external: false,
|
||||
active,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn active_admin_yields_current_admin_role() {
|
||||
let live = decide_live_role(Ok(flags(UserRole::Admin, true)), Uuid::nil(), "user");
|
||||
// The live record wins over the (stale) claim — a freshly promoted
|
||||
// user is admin even though their token still says "user".
|
||||
assert_eq!(live, LiveRole::Active("admin".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn active_user_yields_current_user_role() {
|
||||
// A demoted admin: token claim still "admin", live record "user".
|
||||
let live = decide_live_role(Ok(flags(UserRole::User, true)), Uuid::nil(), "admin");
|
||||
assert_eq!(live, LiveRole::Active("user".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deactivated_account_is_revoked() {
|
||||
let live = decide_live_role(Ok(flags(UserRole::Admin, false)), Uuid::nil(), "admin");
|
||||
assert_eq!(live, LiveRole::Revoked);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deleted_account_not_found_is_revoked() {
|
||||
let err = DomainError::new(ErrorKind::NotFound, "User", "no such user");
|
||||
let live = decide_live_role(Err(err), Uuid::nil(), "admin");
|
||||
assert_eq!(live, LiveRole::Revoked);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn transient_error_fails_open_on_claim_role() {
|
||||
// A DB blip must not lock everyone out: allow on the claim role.
|
||||
let err = DomainError::new(ErrorKind::InternalError, "User", "connection reset");
|
||||
let live = decide_live_role(Err(err), Uuid::nil(), "admin");
|
||||
assert_eq!(live, LiveRole::Active("admin".to_string()));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user