From 6a434aebe60df4d8797112762981b01cc6c58d0f Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sun, 9 Aug 2026 10:55:19 +0200 Subject: [PATCH] refactor(dpop): apply clippy --- src/application/dtos/session_dto.rs | 27 +++++++++++++-- .../services/auth_application_service.rs | 13 ++++---- src/interfaces/api/handlers/admin_handler.rs | 33 +++++++++++-------- .../api/handlers/opaque_auth_handler.rs | 7 ++-- src/interfaces/middleware/dpop.rs | 1 - src/main.rs | 14 ++++---- 6 files changed, 62 insertions(+), 33 deletions(-) diff --git a/src/application/dtos/session_dto.rs b/src/application/dtos/session_dto.rs index b6a4056a..df95b0da 100644 --- a/src/application/dtos/session_dto.rs +++ b/src/application/dtos/session_dto.rs @@ -21,6 +21,29 @@ use uuid::Uuid; use crate::domain::entities::session::Session; +/// Authenticated-caller context — the caller's identity + session- +/// bound signals a service method might key off. Constructed at the +/// handler boundary from `AuthUser` and passed through unchanged; +/// keeps service signatures flat instead of accumulating parallel +/// `caller_id`, `caller_jkt`, `caller_ip` parameters. Every +/// caller-context field lives here, one place to extend. +/// +/// Not admin-specific — any handler that needs caller context can +/// build one from `AuthUser`. Admin methods just happen to be the +/// first callers (sessions panel's `is_current` comparison and +/// audit lines). +#[derive(Debug, Clone)] +pub struct SessionCaller<'a> { + /// AuthZ subject — used by `require_admin_caller` and audit lines. + pub id: Uuid, + /// Caller's own DPoP thumbprint from the JWT `cnf.jkt` claim. + /// Enables the sessions panel's "you are here" highlight + /// ([`SessionSummaryDto::is_current`]) — `None` when the caller + /// logged in via an unbound path (legacy password without DPoP, + /// pre-bind OIDC redirect, etc.). + pub dpop_jkt: Option<&'a str>, +} + /// Wire shape for `GET /api/admin/sessions`. Contains everything the /// admin table renders and **nothing the raw session entity would /// leak** (refresh token, OIDC ID-token, full DPoP thumbprint). @@ -75,9 +98,7 @@ impl SessionSummaryDto { let is_revoked = s.is_revoked(); let is_expired = s.is_expired(); let jkt = s.dpop_jkt().map(|s| s.to_owned()); - let dpop_jkt_prefix = jkt - .as_ref() - .map(|t| t.chars().take(8).collect::()); + let dpop_jkt_prefix = jkt.as_ref().map(|t| t.chars().take(8).collect::()); let is_current = match (jkt.as_deref(), caller_jkt) { (Some(row), Some(caller)) => row == caller, _ => false, diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index ee700931..0be0e65b 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -2962,14 +2962,13 @@ impl AuthApplicationService { pub async fn admin_list_sessions_with_perms( &self, authorization: &A, - caller_id: Uuid, - caller_dpop_jkt: Option<&str>, + caller: crate::application::dtos::session_dto::SessionCaller<'_>, user_id_filter: Option, include_revoked: bool, limit: i64, offset: i64, ) -> Result, DomainError> { - self.require_admin_caller(authorization, caller_id).await?; + self.require_admin_caller(authorization, caller.id).await?; let sessions = self .session_storage .list_sessions_paginated(user_id_filter, include_revoked, limit, offset) @@ -2979,7 +2978,7 @@ impl AuthApplicationService { .map(|s| { crate::application::dtos::session_dto::SessionSummaryDto::from_session( s, - caller_dpop_jkt, + caller.dpop_jkt, ) }) .collect()) @@ -2994,10 +2993,10 @@ impl AuthApplicationService { pub async fn admin_revoke_session_with_perms( &self, authorization: &A, - caller_id: Uuid, + caller: crate::application::dtos::session_dto::SessionCaller<'_>, session_id: Uuid, ) -> Result<(), DomainError> { - self.require_admin_caller(authorization, caller_id).await?; + self.require_admin_caller(authorization, caller.id).await?; // Resolve target user for the audit line before revocation — // once the session row is revoked the user_id is still readable // but the ORDER is stable this way. @@ -3011,7 +3010,7 @@ impl AuthApplicationService { tracing::info!( target: "audit", event = "admin.session_revoked", - caller_id = %caller_id, + caller_id = %caller.id, session_id = %session_id, target_user_id = target_user_id.map(|u| u.to_string()).unwrap_or_default(), "👮🏻‍♂️ Admin revoked session", diff --git a/src/interfaces/api/handlers/admin_handler.rs b/src/interfaces/api/handlers/admin_handler.rs index cdfbe976..090cb362 100644 --- a/src/interfaces/api/handlers/admin_handler.rs +++ b/src/interfaces/api/handlers/admin_handler.rs @@ -18,9 +18,9 @@ use crate::application::dtos::plugin_dto::{ use crate::application::dtos::settings_dto::{ AdminCreateUserDto, AdminResetPasswordDto, DashboardStatsDto, DriveKindUsageDto, ListSessionsQueryDto, ListUsersQueryDto, MigrationStateDto, SaveOidcSettingsDto, - SaveStorageSettingsDto, - SendSmtpTestDto, SmtpInfoDto, SmtpTestResultDto, StartMigrationDto, TestOidcConnectionDto, - TestStorageConnectionDto, UpdateUserActiveDto, UpdateUserQuotaDto, UpdateUserRoleDto, + SaveStorageSettingsDto, SendSmtpTestDto, SmtpInfoDto, SmtpTestResultDto, StartMigrationDto, + TestOidcConnectionDto, TestStorageConnectionDto, UpdateUserActiveDto, UpdateUserQuotaDto, + UpdateUserRoleDto, }; use crate::application::dtos::user_dto::{AdminUserSummaryDto, UserDto}; use crate::application::ports::authorization_ports::AuthorizationEngine; @@ -1238,20 +1238,23 @@ pub async fn list_sessions( None => None, }; - // Pass the caller's DPoP thumbprint so the DTO can flag which - // row is the admin's own current session (`is_current = true`). - // Rendered as a "this is you" badge — prevents the admin from - // accidentally revoking the session they're clicking from. - // `None` when the admin is unbound (rare — legacy / migration - // window sessions), in which case no row highlights. - let caller_jkt = auth_user.dpop_jkt.as_deref(); + // Pass the caller's DPoP thumbprint through the SessionCaller + // wrapper so the DTO can flag which row is the admin's own + // current session (`is_current = true`). Rendered as a "this + // is you" badge — prevents accidentally revoking the session + // the click came from. `None` when the admin is unbound (rare + // — legacy / migration-window sessions), in which case no row + // highlights. + let caller = crate::application::dtos::session_dto::SessionCaller { + id: auth_user.id, + dpop_jkt: auth_user.dpop_jkt.as_deref(), + }; let sessions = auth .auth_application_service .admin_list_sessions_with_perms( state.authorization.as_ref(), - auth_user.id, - caller_jkt, + caller, user_id_filter, include_revoked, limit, @@ -1293,8 +1296,12 @@ pub async fn revoke_session( .as_ref() .ok_or_else(|| AppError::internal_error("Auth service not configured"))?; + let caller = crate::application::dtos::session_dto::SessionCaller { + id: auth_user.id, + dpop_jkt: auth_user.dpop_jkt.as_deref(), + }; auth.auth_application_service - .admin_revoke_session_with_perms(state.authorization.as_ref(), auth_user.id, session_id) + .admin_revoke_session_with_perms(state.authorization.as_ref(), caller, session_id) .await .map_err(AppError::from)?; diff --git a/src/interfaces/api/handlers/opaque_auth_handler.rs b/src/interfaces/api/handlers/opaque_auth_handler.rs index f9a4525f..988c7abc 100644 --- a/src/interfaces/api/handlers/opaque_auth_handler.rs +++ b/src/interfaces/api/handlers/opaque_auth_handler.rs @@ -770,8 +770,11 @@ pub async fn login_ke3( // `user_agent` land populated instead of NULL (admin panel would // otherwise render "—"). Both are per-session and only refresh // on rotation, matching the login pattern. - let client_ip = - crate::interfaces::middleware::trusted_proxy::client_ip_from_parts(&headers, Some(peer), false); + let client_ip = crate::interfaces::middleware::trusted_proxy::client_ip_from_parts( + &headers, + Some(peer), + false, + ); let user_agent = headers .get(axum::http::header::USER_AGENT) .and_then(|v| v.to_str().ok()) diff --git a/src/interfaces/middleware/dpop.rs b/src/interfaces/middleware/dpop.rs index 2308d8c3..e6e7cbe2 100644 --- a/src/interfaces/middleware/dpop.rs +++ b/src/interfaces/middleware/dpop.rs @@ -412,5 +412,4 @@ mod tests { ("http".to_owned(), "localhost".to_owned()) ); } - } diff --git a/src/main.rs b/src/main.rs index 865d9e59..c25fcc0c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1396,13 +1396,13 @@ async fn run() -> Result<(), Box> { // Installs the recorder BEFORE the main listener starts serving so // the first request's counter increments are captured (recorder // install is racy vs first emit — order matters). - if let Some(metrics_addr) = config.metrics_listen { - if let Err(err) = oxicloud::interfaces::metrics::spawn(metrics_addr).await { - // Fail loudly: operators asked for metrics; not surfacing - // this would hide a misconfigured scrape endpoint. - tracing::error!("Prometheus /metrics setup failed: {err}"); - return Err(err); - } + if let Some(metrics_addr) = config.metrics_listen + && let Err(err) = oxicloud::interfaces::metrics::spawn(metrics_addr).await + { + // Fail loudly: operators asked for metrics; not surfacing + // this would hide a misconfigured scrape endpoint. + tracing::error!("Prometheus /metrics setup failed: {err}"); + return Err(err); } let socket = make_socket(&addr, reuse_port)?;