perf(auth): cached image-free user-flags lookup for per-request guards
Every WebDAV / CalDAV / CardDAV request paid one full-row user fetch in
require_internal_user_layer just to read `is_external` (and the NC Basic
Auth middleware repeated it right after its own cache hit). That SELECT
includes the `image` column — a data URI of up to 512 KiB — so a sync
client issuing hundreds of PROPFINDs per minute dragged hundreds of MB
of avatar bytes out of Postgres to evaluate a boolean.
- New `UserFlags { role, is_external, active }` + a repo query selecting
only those three columns (inherent method, mirroring `update_image`).
- `AuthApplicationService::get_user_flags`: moka cache, 30 s TTL,
10k capacity. `change_user_role` / `set_user_active` invalidate
eagerly, so admin changes still apply immediately; anything else is
visible within the TTL — preserving the documented "no token rotation
needed" semantics at a per-request cost of zero DB round-trips when
warm.
- `require_internal_user`, `require_admin_user` and the NC Basic Auth
external check now go through the flags lookup.
https://claude.ai/code/session_01Dp3oWon5GBMVn4j3QXZdgx
This commit is contained in:
@@ -11,7 +11,7 @@ use crate::common::config::OidcConfig;
|
|||||||
use crate::common::errors::{DomainError, ErrorKind};
|
use crate::common::errors::{DomainError, ErrorKind};
|
||||||
use crate::domain::entities::magic_link_token::{MagicLinkResourceKind, MagicLinkStatus};
|
use crate::domain::entities::magic_link_token::{MagicLinkResourceKind, MagicLinkStatus};
|
||||||
use crate::domain::entities::session::Session;
|
use crate::domain::entities::session::Session;
|
||||||
use crate::domain::entities::user::{User, UserRole};
|
use crate::domain::entities::user::{User, UserFlags, UserRole};
|
||||||
use crate::domain::repositories::magic_link_token_repository::MagicLinkTokenRepository;
|
use crate::domain::repositories::magic_link_token_repository::MagicLinkTokenRepository;
|
||||||
use crate::infrastructure::repositories::pg::SessionPgRepository;
|
use crate::infrastructure::repositories::pg::SessionPgRepository;
|
||||||
use crate::infrastructure::repositories::pg::UserPgRepository;
|
use crate::infrastructure::repositories::pg::UserPgRepository;
|
||||||
@@ -140,8 +140,20 @@ pub struct AuthApplicationService {
|
|||||||
/// Magic-link token repository — populated when the magic-link feature
|
/// Magic-link token repository — populated when the magic-link feature
|
||||||
/// is enabled (PR 8+). `None` means redemption endpoints return 503.
|
/// is enabled (PR 8+). `None` means redemption endpoints return 503.
|
||||||
magic_link_repo: Option<Arc<dyn MagicLinkTokenRepository>>,
|
magic_link_repo: Option<Arc<dyn MagicLinkTokenRepository>>,
|
||||||
|
/// Per-user authorization flags (`role` / `is_external` / `active`),
|
||||||
|
/// consulted by middleware guards on every WebDAV / CalDAV / CardDAV
|
||||||
|
/// request. The short TTL keeps the "role changes apply without token
|
||||||
|
/// rotation" property within seconds while removing one DB round-trip
|
||||||
|
/// per request; the known mutation paths (`change_user_role`,
|
||||||
|
/// `set_user_active`) also invalidate eagerly.
|
||||||
|
user_flags_cache: Cache<Uuid, UserFlags>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// TTL for [`AuthApplicationService::user_flags_cache`]. Upper bound on how
|
||||||
|
/// long a role / external / active change can take to be observed by the
|
||||||
|
/// per-request guards when it bypasses the eager invalidation paths.
|
||||||
|
const USER_FLAGS_CACHE_TTL: Duration = Duration::from_secs(30);
|
||||||
|
|
||||||
impl AuthApplicationService {
|
impl AuthApplicationService {
|
||||||
pub fn new(
|
pub fn new(
|
||||||
user_storage: Arc<UserPgRepository>,
|
user_storage: Arc<UserPgRepository>,
|
||||||
@@ -170,6 +182,10 @@ impl AuthApplicationService {
|
|||||||
.time_to_live(Duration::from_secs(60))
|
.time_to_live(Duration::from_secs(60))
|
||||||
.build(),
|
.build(),
|
||||||
magic_link_repo: None,
|
magic_link_repo: None,
|
||||||
|
user_flags_cache: Cache::builder()
|
||||||
|
.max_capacity(10_000)
|
||||||
|
.time_to_live(USER_FLAGS_CACHE_TTL)
|
||||||
|
.build(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1141,6 +1157,24 @@ impl AuthApplicationService {
|
|||||||
Ok(UserDto::from(user))
|
Ok(UserDto::from(user))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Cached, image-free lookup of the caller's authorization flags
|
||||||
|
/// (`role` / `is_external` / `active`). This is the per-request fast
|
||||||
|
/// path for middleware guards: the full `get_user` row fetch drags the
|
||||||
|
/// `image` column (a data URI of up to 512 KiB) across the wire, which
|
||||||
|
/// a sync client issuing hundreds of DAV requests per minute paid on
|
||||||
|
/// every single one just to read a boolean.
|
||||||
|
///
|
||||||
|
/// Staleness is bounded by [`USER_FLAGS_CACHE_TTL`]; role and active
|
||||||
|
/// changes made through this service invalidate the entry eagerly.
|
||||||
|
pub async fn get_user_flags(&self, user_id: Uuid) -> Result<UserFlags, DomainError> {
|
||||||
|
if let Some(flags) = self.user_flags_cache.get(&user_id) {
|
||||||
|
return Ok(flags);
|
||||||
|
}
|
||||||
|
let flags = self.user_storage.get_user_flags(user_id).await?;
|
||||||
|
self.user_flags_cache.insert(user_id, flags);
|
||||||
|
Ok(flags)
|
||||||
|
}
|
||||||
|
|
||||||
/// Apply a profile update on behalf of the calling user (PR 24).
|
/// Apply a profile update on behalf of the calling user (PR 24).
|
||||||
///
|
///
|
||||||
/// Hard rules:
|
/// Hard rules:
|
||||||
@@ -1797,7 +1831,9 @@ impl AuthApplicationService {
|
|||||||
pub async fn set_user_active(&self, user_id: Uuid, active: bool) -> Result<(), DomainError> {
|
pub async fn set_user_active(&self, user_id: Uuid, active: bool) -> Result<(), DomainError> {
|
||||||
self.user_storage
|
self.user_storage
|
||||||
.set_user_active_status(user_id, active)
|
.set_user_active_status(user_id, active)
|
||||||
.await
|
.await?;
|
||||||
|
self.user_flags_cache.invalidate(&user_id);
|
||||||
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Change user role (admin only)
|
/// Change user role (admin only)
|
||||||
@@ -1809,7 +1845,9 @@ impl AuthApplicationService {
|
|||||||
format!("Invalid role: {}. Must be 'admin' or 'user'", role),
|
format!("Invalid role: {}. Must be 'admin' or 'user'", role),
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
self.user_storage.change_role(user_id, role).await
|
self.user_storage.change_role(user_id, role).await?;
|
||||||
|
self.user_flags_cache.invalidate(&user_id);
|
||||||
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Update user's storage quota (admin only)
|
/// Update user's storage quota (admin only)
|
||||||
|
|||||||
@@ -20,6 +20,18 @@ impl std::fmt::Display for UserRole {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Authorization-relevant account flags, fetched without the heavyweight
|
||||||
|
/// profile columns. The full user row drags `image` along — a data URI of
|
||||||
|
/// up to 512 KiB — which per-request guards (`require_internal_user`,
|
||||||
|
/// `require_admin_user`, the NC Basic Auth external check) must never pay
|
||||||
|
/// for just to read a boolean or a role.
|
||||||
|
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||||
|
pub struct UserFlags {
|
||||||
|
pub role: UserRole,
|
||||||
|
pub is_external: bool,
|
||||||
|
pub active: bool,
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone)]
|
#[derive(Debug, Clone)]
|
||||||
pub struct User {
|
pub struct User {
|
||||||
id: Uuid,
|
id: Uuid,
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ use uuid::Uuid;
|
|||||||
|
|
||||||
use crate::application::ports::auth_ports::UserStoragePort;
|
use crate::application::ports::auth_ports::UserStoragePort;
|
||||||
use crate::common::errors::DomainError;
|
use crate::common::errors::DomainError;
|
||||||
use crate::domain::entities::user::{User, UserRole};
|
use crate::domain::entities::user::{User, UserFlags, UserRole};
|
||||||
use crate::domain::repositories::user_repository::{
|
use crate::domain::repositories::user_repository::{
|
||||||
StorageStats, UserRepository, UserRepositoryError, UserRepositoryResult,
|
StorageStats, UserRepository, UserRepositoryError, UserRepositoryResult,
|
||||||
};
|
};
|
||||||
@@ -51,6 +51,40 @@ impl UserPgRepository {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Fetch only the authorization-relevant flags of a user. Not part of
|
||||||
|
/// the `UserRepository` trait — called directly from
|
||||||
|
/// `AuthApplicationService::get_user_flags`.
|
||||||
|
///
|
||||||
|
/// Deliberately selects three tiny columns instead of the full row:
|
||||||
|
/// the full-row SELECT includes `image` (a data URI of up to 512 KiB),
|
||||||
|
/// which per-request middleware guards were paying on every WebDAV /
|
||||||
|
/// CalDAV / CardDAV request just to read `is_external` or `role`.
|
||||||
|
pub async fn get_user_flags(&self, id: Uuid) -> UserRepositoryResult<UserFlags> {
|
||||||
|
let row = sqlx::query(
|
||||||
|
r#"
|
||||||
|
SELECT role::text as role_text, is_external, active
|
||||||
|
FROM auth.users
|
||||||
|
WHERE id = $1
|
||||||
|
"#,
|
||||||
|
)
|
||||||
|
.bind(id)
|
||||||
|
.fetch_one(&*self.pool)
|
||||||
|
.await
|
||||||
|
.map_err(Self::map_sqlx_error)?;
|
||||||
|
|
||||||
|
let role_str: Option<String> = row.try_get("role_text").unwrap_or(None);
|
||||||
|
let role = match role_str.as_deref() {
|
||||||
|
Some("admin") => UserRole::Admin,
|
||||||
|
_ => UserRole::User,
|
||||||
|
};
|
||||||
|
|
||||||
|
Ok(UserFlags {
|
||||||
|
role,
|
||||||
|
is_external: row.get("is_external"),
|
||||||
|
active: row.get("active"),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
/// Updates a user's profile image (URL or data URI). Not part of the
|
/// Updates a user's profile image (URL or data URI). Not part of the
|
||||||
/// `UserRepository` trait — called directly from `AuthApplicationService`.
|
/// `UserRepository` trait — called directly from `AuthApplicationService`.
|
||||||
pub async fn update_image(
|
pub async fn update_image(
|
||||||
|
|||||||
@@ -4,9 +4,11 @@
|
|||||||
//! so handlers compose them uniformly as one-liners. They assume the
|
//! so handlers compose them uniformly as one-liners. They assume the
|
||||||
//! caller has already been authenticated by the
|
//! caller has already been authenticated by the
|
||||||
//! [`AuthUser`](super::auth::AuthUser) extractor, and pull the current
|
//! [`AuthUser`](super::auth::AuthUser) extractor, and pull the current
|
||||||
//! user state from the database via `AuthApplicationService` so role /
|
//! user flags via `AuthApplicationService::get_user_flags` — a
|
||||||
//! external-flag changes take effect on the next request without
|
//! lightweight, short-TTL-cached lookup (no `image` column) — so role /
|
||||||
//! waiting for token rotation.
|
//! external-flag changes take effect within seconds without waiting
|
||||||
|
//! for token rotation, while the hot DAV paths stop paying one full-row
|
||||||
|
//! DB fetch per request.
|
||||||
//!
|
//!
|
||||||
//! ```ignore
|
//! ```ignore
|
||||||
//! let caller_id = auth_user.id;
|
//! let caller_id = auth_user.id;
|
||||||
@@ -30,6 +32,7 @@ use uuid::Uuid;
|
|||||||
|
|
||||||
use crate::application::services::auth_application_service::AuthApplicationService;
|
use crate::application::services::auth_application_service::AuthApplicationService;
|
||||||
use crate::common::di::AppState;
|
use crate::common::di::AppState;
|
||||||
|
use crate::domain::entities::user::UserRole;
|
||||||
use crate::interfaces::errors::AppError;
|
use crate::interfaces::errors::AppError;
|
||||||
use crate::interfaces::middleware::auth::CurrentUser;
|
use crate::interfaces::middleware::auth::CurrentUser;
|
||||||
|
|
||||||
@@ -56,8 +59,8 @@ pub async fn require_internal_user(
|
|||||||
auth: &AuthApplicationService,
|
auth: &AuthApplicationService,
|
||||||
caller_id: Uuid,
|
caller_id: Uuid,
|
||||||
) -> Result<(), AppError> {
|
) -> Result<(), AppError> {
|
||||||
match auth.get_user_by_id(caller_id).await {
|
match auth.get_user_flags(caller_id).await {
|
||||||
Ok(dto) if dto.is_external => Err(AppError::new(
|
Ok(flags) if flags.is_external => Err(AppError::new(
|
||||||
StatusCode::FORBIDDEN,
|
StatusCode::FORBIDDEN,
|
||||||
"External users cannot access this endpoint",
|
"External users cannot access this endpoint",
|
||||||
"Forbidden",
|
"Forbidden",
|
||||||
@@ -70,9 +73,11 @@ pub async fn require_internal_user(
|
|||||||
/// admins, `Err(403)` otherwise.
|
/// admins, `Err(403)` otherwise.
|
||||||
///
|
///
|
||||||
/// The check pulls the role from the user record (not from JWT
|
/// The check pulls the role from the user record (not from JWT
|
||||||
/// claims) so a role change takes effect on the next request without
|
/// claims) so a role change takes effect within the flags-cache TTL —
|
||||||
/// waiting for token rotation. Mirrors [`require_internal_user`]'s
|
/// or immediately when changed through `change_user_role`, which
|
||||||
/// shape so handlers compose either of them as a one-liner via `?`.
|
/// invalidates the entry — without waiting for token rotation. Mirrors
|
||||||
|
/// [`require_internal_user`]'s shape so handlers compose either of
|
||||||
|
/// them as a one-liner via `?`.
|
||||||
///
|
///
|
||||||
/// Use this in handlers that already have an
|
/// Use this in handlers that already have an
|
||||||
/// [`AuthUser`](super::auth::AuthUser) extractor (and thus a validated
|
/// [`AuthUser`](super::auth::AuthUser) extractor (and thus a validated
|
||||||
@@ -82,12 +87,12 @@ pub async fn require_admin_user(
|
|||||||
auth: &AuthApplicationService,
|
auth: &AuthApplicationService,
|
||||||
caller_id: Uuid,
|
caller_id: Uuid,
|
||||||
) -> Result<(), AppError> {
|
) -> Result<(), AppError> {
|
||||||
let user = auth
|
let flags = auth
|
||||||
.get_user_by_id(caller_id)
|
.get_user_flags(caller_id)
|
||||||
.await
|
.await
|
||||||
.map_err(AppError::from)?;
|
.map_err(AppError::from)?;
|
||||||
|
|
||||||
if user.role != "admin" {
|
if flags.role != UserRole::Admin {
|
||||||
return Err(AppError::new(
|
return Err(AppError::new(
|
||||||
StatusCode::FORBIDDEN,
|
StatusCode::FORBIDDEN,
|
||||||
"Admin access required",
|
"Admin access required",
|
||||||
|
|||||||
@@ -64,8 +64,7 @@ pub async fn basic_auth_middleware(
|
|||||||
|
|
||||||
// Check account lockout before attempting password verification (saves CPU).
|
// Check account lockout before attempting password verification (saves CPU).
|
||||||
// The lockout is per (account, IP), see #323 for rationale.
|
// The lockout is per (account, IP), see #323 for rationale.
|
||||||
let client_ip =
|
let client_ip = crate::interfaces::middleware::rate_limit::extract_client_ip(&request);
|
||||||
crate::interfaces::middleware::rate_limit::extract_client_ip(&request);
|
|
||||||
if let Some(auth_svc) = state.auth_service.as_ref()
|
if let Some(auth_svc) = state.auth_service.as_ref()
|
||||||
&& let Err(secs) = auth_svc.login_lockout.check(&username, &client_ip)
|
&& let Err(secs) = auth_svc.login_lockout.check(&username, &client_ip)
|
||||||
{
|
{
|
||||||
@@ -103,11 +102,11 @@ pub async fn basic_auth_middleware(
|
|||||||
// this is the belt-and-braces check in case one slipped
|
// this is the belt-and-braces check in case one slipped
|
||||||
// through (e.g. user later flipped to is_external).
|
// through (e.g. user later flipped to is_external).
|
||||||
if let Some(auth_svc) = state.auth_service.as_ref()
|
if let Some(auth_svc) = state.auth_service.as_ref()
|
||||||
&& let Ok(user) = auth_svc
|
&& let Ok(flags) = auth_svc
|
||||||
.auth_application_service
|
.auth_application_service
|
||||||
.get_user_by_id(user_id)
|
.get_user_flags(user_id)
|
||||||
.await
|
.await
|
||||||
&& user.is_external
|
&& flags.is_external
|
||||||
{
|
{
|
||||||
tracing::info!(
|
tracing::info!(
|
||||||
target: "audit",
|
target: "audit",
|
||||||
|
|||||||
Reference in New Issue
Block a user