diff --git a/src/application/dtos/user_dto.rs b/src/application/dtos/user_dto.rs index 3d88b344..7c8feaa6 100644 --- a/src/application/dtos/user_dto.rs +++ b/src/application/dtos/user_dto.rs @@ -681,7 +681,12 @@ impl UpdateProfileDto { #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] pub struct AuthResponseDto { - pub user: UserDto, + /// Full self view — identical shape to `/api/auth/me`. Every + /// login / refresh / OIDC-callback / magic-link redemption ships + /// this so the SPA's post-auth state matches its post-`/me` state + /// (no UI race between `AuthResponseDto` and the first `/me` + /// fetch). See `docs/plan/userdto-refactor.md` § Endpoint mapping. + pub user: SelfUserDto, pub access_token: String, pub refresh_token: String, pub token_type: String, diff --git a/src/application/ports/auth_ports.rs b/src/application/ports/auth_ports.rs index e7f2deff..dc75e315 100644 --- a/src/application/ports/auth_ports.rs +++ b/src/application/ports/auth_ports.rs @@ -123,6 +123,36 @@ pub trait UserStoragePort: Send + Sync + 'static { /// Gets a user by ID async fn get_user_by_id(&self, id: Uuid) -> Result; + /// Fetch the full `User` + [`UserDerivedFlags`] in one query. See + /// [`UserRepository::get_user_with_derived_flags`](crate::domain::repositories::user_repository::UserRepository::get_user_with_derived_flags) + /// for the contract and the rationale for the single-query shape. + async fn get_user_with_derived_flags( + &self, + id: Uuid, + ) -> Result< + ( + User, + crate::domain::repositories::user_repository::UserDerivedFlags, + ), + DomainError, + >; + + /// Paginated admin user listing with derived flags. See + /// [`UserRepository::list_users_with_derived_flags`](crate::domain::repositories::user_repository::UserRepository::list_users_with_derived_flags) + /// for the contract and rationale. + async fn list_users_with_derived_flags( + &self, + limit: i64, + offset: i64, + include_external: bool, + ) -> Result< + Vec<( + User, + crate::domain::repositories::user_repository::UserDerivedFlags, + )>, + DomainError, + >; + /// Batch-loads users by id. Order is unspecified; missing ids are /// silently dropped. Used by group-recipient expansion in /// `RecipientNotificationService` to avoid N+1 lookups when notifying diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index e042dcee..7c8a70aa 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -1,6 +1,6 @@ use crate::application::dtos::user_dto::{ - AdminUserSummaryDto, AuthResponseDto, ChangePasswordDto, LoginDto, RefreshTokenDto, - RegisterDto, UpgradeToInternalDto, UserDto, + AuthResponseDto, ChangePasswordDto, FullUserDto, LoginDto, RefreshTokenDto, RegisterDto, + SelfUserDto, UpgradeToInternalDto, UserDto, }; use crate::application::ports::auth_ports::{ OidcIdClaims, OidcServicePort, PasswordHasherPort, SessionStoragePort, TokenServicePort, @@ -1320,12 +1320,16 @@ impl AuthApplicationService { session = session.with_dpop_jkt(jkt); } + // Build the SelfUserDto BEFORE `session` moves into + // `create_session` — the builder reads `session.dpop_jkt()`. + let user_id = user.id(); + let user_dto = self.build_self_user_dto(user_id, &session).await?; self.session_storage.create_session(session).await?; // Authentication response - let force_password_change = self.read_force_password_change(user.id()).await; + let force_password_change = self.read_force_password_change(user_id).await; Ok(AuthResponseDto { - user: UserDto::from(user), + user: user_dto, access_token, refresh_token, token_type: "Bearer".to_string(), @@ -1334,6 +1338,45 @@ impl AuthApplicationService { }) } + /// Assemble a `SelfUserDto` for the given user + the session that + /// mints them. Called by every `AuthResponseDto` path + /// (login / refresh / OIDC / magic-link) so the wire shape stays + /// consistent across login flavours and matches what `/api/auth/me` + /// would return. + /// + /// Costs one wide SELECT (`get_user_with_derived_flags`) even when + /// the caller already has a `User` in hand — acceptable because + /// `/login`, `/refresh`, and the OIDC/magic-link callbacks are + /// not hot inner loops. In exchange the composition stays uniform + /// across all four callsites and OPAQUE / `is_online` flags land + /// on the wire without a second lookup at each site. + /// + /// `is_dpop_bound` is derived from the session's own DPoP + /// thumbprint — the session was just constructed, so this reads + /// exactly the binding that will govern subsequent requests. + async fn build_self_user_dto( + &self, + user_id: Uuid, + session: &crate::domain::entities::session::Session, + ) -> Result { + let (user, flags) = + UserStoragePort::get_user_with_derived_flags(&*self.user_storage, user_id).await?; + let can_edit_image = !user.is_oidc_user(); + let ui_preferences = user.ui_preferences().clone(); + let notify_on_share = user.notify_on_share(); + let force_password_change = self.read_force_password_change(user_id).await; + let is_dpop_bound = session.dpop_jkt().is_some(); + let full = FullUserDto::build(user, flags); + Ok(SelfUserDto::build( + full, + ui_preferences, + notify_on_share, + is_dpop_bound, + force_password_change, + can_edit_image, + )) + } + /// Read `force_password_change_at_next_login` for the given user, /// with fail-open semantics on repo error (returns `false` and /// logs a warn). Every callsite that builds an `AuthResponseDto` @@ -1586,22 +1629,29 @@ impl AuthApplicationService { let access_token = self.token_service .generate_access_token(&user, Some(session.id()), None)?; + // Snapshot fields still needed for logging + DTO before + // `session` and `user` are consumed by the storage call and + // the DTO builder below. + let user_id = user.id(); + let user_display = user.display_for_audit().to_string(); + let is_external = user.is_external(); + let user_dto = self.build_self_user_dto(user_id, &session).await?; self.session_storage.create_session(session).await?; tracing::info!( target: "audit", event = "magic_link.redeemed", - user_id = %user.id(), - username = %user.display_for_audit(), - is_external = user.is_external(), + user_id = %user_id, + username = %user_display, + is_external = is_external, resource_kind = ?mlt.resource_kind(), resource_id = ?mlt.resource_id(), cross_browser_confirmed = cross_browser_confirmed, ); - let force_password_change = self.read_force_password_change(user.id()).await; + let force_password_change = self.read_force_password_change(user_id).await; let auth = AuthResponseDto { - user: UserDto::from(user), + user: user_dto, access_token, refresh_token, token_type: "Bearer".to_string(), @@ -1789,6 +1839,12 @@ impl AuthApplicationService { session.dpop_jkt(), )?; + // Build the SelfUserDto before `new_session` is consumed by + // the rotate call — the builder reads `session.dpop_jkt()` + // to compute `is_dpop_bound`. + let user_id = user.id(); + let user_dto = self.build_self_user_dto(user_id, &new_session).await?; + self.session_storage .rotate_session(session.id(), new_session) .await?; @@ -1798,9 +1854,9 @@ impl AuthApplicationService { // initial login. The SPA's post-refresh flow (silent, on // its own timer) can then route the user to change-password // without waiting for an explicit re-login. - let force_password_change = self.read_force_password_change(user.id()).await; + let force_password_change = self.read_force_password_change(user_id).await; Ok(AuthResponseDto { - user: UserDto::from(user), + user: user_dto, access_token, refresh_token: new_refresh_token, token_type: "Bearer".to_string(), @@ -2871,6 +2927,26 @@ impl AuthApplicationService { UserStoragePort::get_user_by_id(&*self.user_storage, user_id).await } + /// Load the full `User` entity + `UserDerivedFlags` for the given + /// id in ONE query. See + /// [`UserRepository::get_user_with_derived_flags`](crate::domain::repositories::user_repository::UserRepository::get_user_with_derived_flags) + /// for the shape and the SELECT that drives it. Used by + /// `/api/auth/me` to build a `SelfUserDto` and by future admin + /// single-user views to build a `FullUserDto` without paying two + /// round-trips. + pub async fn get_user_with_derived_flags( + &self, + user_id: Uuid, + ) -> Result< + ( + crate::domain::entities::user::User, + crate::domain::repositories::user_repository::UserDerivedFlags, + ), + DomainError, + > { + UserStoragePort::get_user_with_derived_flags(&*self.user_storage, user_id).await + } + /// Login-style identifier lookup: dispatches on `@` in the input /// (email path when present, username path when not), identical /// to `login()`'s dispatch. Exposed so the OPAQUE login handler @@ -3145,22 +3221,30 @@ impl AuthApplicationService { Ok(users.into_iter().map(UserDto::from).collect()) } - /// Admin-only compact listing. The detail endpoint retains the complete - /// [`UserDto`]; this path projects only what the management table renders so - /// PostgreSQL never detoasts or transfers avatars/preferences for a page. + /// Admin-only user listing. Returns `Vec` — same + /// `FullUserDto` shape [`SelfUserDto`] embeds, so the FE reads + /// admin table rows and `/me` responses through identical field + /// paths. Includes the avatar (`user.image`) and presence + /// (`user.is_online`) so the admin table renders the vignette + + /// green dot without per-row follow-up fetches to + /// `/api/users/{id}` (the N+1 that motivated the widening — see + /// `docs/plan/userdto-refactor.md` § N+1). pub async fn list_user_summaries_including_external_with_perms( &self, authorization: &A, caller_id: Uuid, limit: i64, offset: i64, - ) -> Result, DomainError> { + ) -> Result, DomainError> { self.require_admin_caller(authorization, caller_id).await?; - let users = self + let rows = self .user_storage - .list_user_summaries(limit, offset, true) + .list_users_with_derived_flags(limit, offset, true) .await?; - Ok(users.into_iter().map(AdminUserSummaryDto::from).collect()) + Ok(rows + .into_iter() + .map(|(user, flags)| FullUserDto::build(user, flags)) + .collect()) } /// Service-layer gate for administrator-scoped user-directory operations. @@ -4654,11 +4738,17 @@ impl AuthApplicationService { let access_token = self.token_service .generate_access_token(&user, Some(session.id()), None)?; + // Build the SelfUserDto before `session` is consumed by the + // storage call — the builder reads `session.dpop_jkt()` + // (None here since OIDC callbacks land unbound and the SPA + // finishes binding post-redirect). + let user_id = user.id(); + let user_dto = self.build_self_user_dto(user_id, &session).await?; self.session_storage.create_session(session).await?; - let force_password_change = self.read_force_password_change(user.id()).await; + let force_password_change = self.read_force_password_change(user_id).await; let auth_response = AuthResponseDto { - user: UserDto::from(user), + user: user_dto, access_token, refresh_token, token_type: "Bearer".to_string(), diff --git a/src/domain/repositories/user_repository.rs b/src/domain/repositories/user_repository.rs index 3adaddc4..9bc6bec0 100644 --- a/src/domain/repositories/user_repository.rs +++ b/src/domain/repositories/user_repository.rs @@ -139,6 +139,20 @@ pub trait UserRepository: Send + Sync + 'static { /// Gets a user by ID async fn get_user_by_id(&self, id: Uuid) -> UserRepositoryResult; + /// Fetch the full `User` entity + the [`UserDerivedFlags`] in a + /// single query. Used by `/api/auth/me` and future admin single-user + /// views — anywhere the caller needs both the row itself AND the + /// derived booleans (`has_password`, OPAQUE flags, `is_online`) to + /// build a [`FullUserDto`](crate::application::dtos::user_dto::FullUserDto) + /// or [`SelfUserDto`](crate::application::dtos::user_dto::SelfUserDto). + /// Single query is cheaper than `get_user_by_id` + separate lookups + /// for OPAQUE state + `is_online`; the EXISTS subquery is cheap + /// thanks to the partial index `idx_sessions_last_seen_at`. + async fn get_user_with_derived_flags( + &self, + id: Uuid, + ) -> UserRepositoryResult<(User, UserDerivedFlags)>; + /// Batch-loads a set of users by id, preserving no particular order /// and silently skipping ids that don't match any row. Caller is /// responsible for de-duplicating the input vec. Returns an empty @@ -200,6 +214,12 @@ pub trait UserRepository: Send + Sync + 'static { /// Lists the columns needed by compact user-management tables. Unlike /// [`Self::list_users`], this never fetches password hashes, OIDC subjects, /// avatars, names, locale state, or UI preferences. + /// + /// **Deprecated** — [`Self::list_users_with_derived_flags`] supersedes + /// this: it returns the full `User` entity + [`UserDerivedFlags`] so + /// the application layer can build [`FullUserDto`](crate::application::dtos::user_dto::FullUserDto) + /// directly. Kept only until P6 of `docs/plan/userdto-refactor.md` + /// removes `UserListEntry` + the last remaining caller. async fn list_user_summaries( &self, limit: i64, @@ -207,6 +227,23 @@ pub trait UserRepository: Send + Sync + 'static { include_external: bool, ) -> UserRepositoryResult>; + /// Paginated admin user listing — full `User` entity + the derived + /// booleans (`has_password`, OPAQUE flags, `is_online`) in one wide + /// SELECT. Called by the admin service to build + /// `Vec` for `/api/admin/users` without paying two + /// round-trips per row (once for User, once for derived flags). + /// + /// Same `include_external` semantics as [`Self::list_users`]: + /// admin management UI passes `true`; every other caller passes + /// `false` so external / grant-only users stay off internal-user + /// surfaces. + async fn list_users_with_derived_flags( + &self, + limit: i64, + offset: i64, + include_external: bool, + ) -> UserRepositoryResult>; + /// Searches users by username or email (SQL ILIKE) with a limit. /// See [`list_users`] for the meaning of `include_external`. async fn search_users( diff --git a/src/infrastructure/repositories/pg/user_pg_repository.rs b/src/infrastructure/repositories/pg/user_pg_repository.rs index 06b0fd47..f8d50fda 100644 --- a/src/infrastructure/repositories/pg/user_pg_repository.rs +++ b/src/infrastructure/repositories/pg/user_pg_repository.rs @@ -388,6 +388,90 @@ impl UserRepository for UserPgRepository { )) } + async fn get_user_with_derived_flags( + &self, + id: Uuid, + ) -> UserRepositoryResult<( + User, + crate::domain::repositories::user_repository::UserDerivedFlags, + )> { + // Same column set as `get_user_by_id` plus the three IS-NOT-NULL + // derivations for auth-capability flags AND the EXISTS scalar + // for `is_online`. The `interval` argument is bound as `$2` + // (seconds, `ONLINE_WINDOW.as_secs_f64()`) via + // `make_interval(secs => $2)` — same pattern as + // `session_liveness_gauges.rs`. Partial index + // `idx_sessions_last_seen_at WHERE revoked = FALSE` covers the + // EXISTS scan, so per-row cost is ~μs. + let row = sqlx::query( + r#" + SELECT + id, username, email, password_hash, role::text as role_text, + storage_quota_bytes, storage_used_bytes, + created_at, updated_at, last_login_at, active, + federation_kind, federation_issuer, federation_subject, image, is_external, + given_name, family_name, email_verified_at, preferred_locale, notify_on_share, + ui_preferences, + (password_hash IS NOT NULL) AS has_password, + (opaque_envelope IS NOT NULL) AS opaque_registered, + (opaque_migrated_at IS NOT NULL) AS opaque_migrated, + EXISTS ( + SELECT 1 FROM auth.sessions s + WHERE s.user_id = auth.users.id + AND s.revoked = FALSE + AND s.last_seen_at > NOW() - make_interval(secs => $2) + ) AS is_online + FROM auth.users + WHERE id = $1 + "#, + ) + .bind(id) + .bind(crate::application::dtos::session_dto::ONLINE_WINDOW.as_secs_f64()) + .fetch_one(&*self.pool) + .await + .map_err(Self::map_sqlx_error)?; + + let role_str: Option = row.try_get("role_text").unwrap_or(None); + let role = match role_str.as_deref() { + Some("admin") => UserRole::Admin, + _ => UserRole::User, + }; + + let user = User::from_data_full( + row.get("id"), + row.get("username"), + row.get("email"), + row.get("password_hash"), + role, + row.get("storage_quota_bytes"), + row.get("storage_used_bytes"), + row.get("created_at"), + row.get("updated_at"), + row.get("last_login_at"), + row.get("active"), + row.get::, _>("federation_kind") + .as_deref() + .and_then(crate::domain::entities::user::FederationKind::parse), + row.get("federation_issuer"), + row.get("federation_subject"), + row.get("image"), + row.get("is_external"), + row.get("given_name"), + row.get("family_name"), + row.get("email_verified_at"), + row.get("preferred_locale"), + row.get("notify_on_share"), + row.get::("ui_preferences"), + ); + let flags = crate::domain::repositories::user_repository::UserDerivedFlags { + has_password: row.get("has_password"), + opaque_registered: row.get("opaque_registered"), + opaque_migrated: row.get("opaque_migrated"), + is_online: row.get("is_online"), + }; + Ok((user, flags)) + } + /// Gets a user by username async fn get_user_by_username(&self, username: &str) -> UserRepositoryResult { let row = sqlx::query( @@ -964,6 +1048,110 @@ impl UserRepository for UserPgRepository { .collect()) } + async fn list_users_with_derived_flags( + &self, + limit: i64, + offset: i64, + include_external: bool, + ) -> UserRepositoryResult< + Vec<( + User, + crate::domain::repositories::user_repository::UserDerivedFlags, + )>, + > { + // Full `User` column set (matches `get_user_by_id`) + the four + // derived booleans (IS-NOT-NULL for auth capability, EXISTS for + // `is_online`) in one SELECT. Same rationale as the single-user + // `get_user_with_derived_flags` variant. Widened over the older + // `list_user_summaries` projection because the FE now consumes + // the full user profile from these rows (killing the per-row + // `/api/users/{id}` fetch the admin table used to fire for + // avatars — see docs/plan/userdto-refactor.md § N+1). + // + // `interval` bound as `$4` seconds + // (`ONLINE_WINDOW.as_secs_f64()`), same pattern as + // `session_liveness_gauges.rs` and `get_user_with_derived_flags`. + let rows = sqlx::query( + r#" + SELECT + id, username, email, password_hash, role::text as role_text, + storage_quota_bytes, storage_used_bytes, + created_at, updated_at, last_login_at, active, + federation_kind, federation_issuer, federation_subject, image, is_external, + given_name, family_name, email_verified_at, preferred_locale, notify_on_share, + ui_preferences, + (password_hash IS NOT NULL) AS has_password_flag, + (opaque_envelope IS NOT NULL) AS opaque_registered, + (opaque_migrated_at IS NOT NULL) AS opaque_migrated, + EXISTS ( + SELECT 1 FROM auth.sessions s + WHERE s.user_id = auth.users.id + AND s.revoked = FALSE + AND s.last_seen_at > NOW() - make_interval(secs => $4) + ) AS is_online + FROM auth.users + WHERE ($3 OR is_external = FALSE) + ORDER BY created_at DESC, id DESC + LIMIT $1 OFFSET $2 + "#, + ) + .bind(limit) + .bind(offset) + .bind(include_external) + .bind(crate::application::dtos::session_dto::ONLINE_WINDOW.as_secs_f64()) + .fetch_all(&*self.pool) + .await + .map_err(Self::map_sqlx_error)?; + + // Note: the `has_password_flag` alias avoids colliding with the + // `password_hash` column selected above (the tuple destructure + // in `list_user_summaries` uses a shorter projection so it + // could reuse the raw `has_password` alias; here we keep both). + Ok(rows + .into_iter() + .map(|row| { + let role_str: Option = row.try_get("role_text").unwrap_or(None); + let role = match role_str.as_deref() { + Some("admin") => UserRole::Admin, + _ => UserRole::User, + }; + let user = User::from_data_full( + row.get("id"), + row.get("username"), + row.get("email"), + row.get("password_hash"), + role, + row.get("storage_quota_bytes"), + row.get("storage_used_bytes"), + row.get("created_at"), + row.get("updated_at"), + row.get("last_login_at"), + row.get("active"), + row.get::, _>("federation_kind") + .as_deref() + .and_then(crate::domain::entities::user::FederationKind::parse), + row.get("federation_issuer"), + row.get("federation_subject"), + row.get("image"), + row.get("is_external"), + row.get("given_name"), + row.get("family_name"), + row.get("email_verified_at"), + row.get("preferred_locale"), + row.get("notify_on_share"), + row.get::("ui_preferences"), + ); + let flags = crate::domain::repositories::user_repository::UserDerivedFlags { + has_password: row.get("has_password_flag"), + opaque_registered: row.get("opaque_registered"), + opaque_migrated: row.get("opaque_migrated"), + is_online: row.get("is_online"), + }; + (user, flags) + }) + .collect()) + } + async fn search_users( &self, query: &str, @@ -1331,6 +1519,21 @@ impl UserStoragePort for UserPgRepository { .map_err(DomainError::from) } + async fn get_user_with_derived_flags( + &self, + id: Uuid, + ) -> Result< + ( + User, + crate::domain::repositories::user_repository::UserDerivedFlags, + ), + DomainError, + > { + UserRepository::get_user_with_derived_flags(self, id) + .await + .map_err(DomainError::from) + } + async fn get_users_by_ids(&self, ids: Vec) -> Result, DomainError> { UserRepository::get_users_by_ids(self, ids) .await @@ -1396,6 +1599,23 @@ impl UserStoragePort for UserPgRepository { .map_err(DomainError::from) } + async fn list_users_with_derived_flags( + &self, + limit: i64, + offset: i64, + include_external: bool, + ) -> Result< + Vec<( + User, + crate::domain::repositories::user_repository::UserDerivedFlags, + )>, + DomainError, + > { + UserRepository::list_users_with_derived_flags(self, limit, offset, include_external) + .await + .map_err(DomainError::from) + } + async fn search_users( &self, query: &str, diff --git a/src/interfaces/api/handlers/admin_handler.rs b/src/interfaces/api/handlers/admin_handler.rs index 0cb66c26..bb5e12d2 100644 --- a/src/interfaces/api/handlers/admin_handler.rs +++ b/src/interfaces/api/handlers/admin_handler.rs @@ -22,7 +22,7 @@ use crate::application::dtos::settings_dto::{ TestOidcConnectionDto, TestStorageConnectionDto, UpdateUserActiveDto, UpdateUserQuotaDto, UpdateUserRoleDto, }; -use crate::application::dtos::user_dto::{AdminUserSummaryDto, UserDto}; +use crate::application::dtos::user_dto::{FullUserDto, UserDto}; use crate::application::ports::authorization_ports::AuthorizationEngine; use crate::application::ports::plugin_ports::{LogQuery, PluginManagementPort, PluginMgmtError}; // JobStoreProvider is used only by the storage-migration shims below, @@ -42,8 +42,17 @@ use uuid::Uuid; #[derive(serde::Serialize)] #[serde(untagged)] enum AdminUsersPayload { + /// Fat-`UserDto` per row. Emitted when `?summary=false` — legacy + /// path retained until the FE drops the `summary=false` query + /// (rare; the SPA uses `summary=true` for the paginated table). Full(Vec), - Summary(Vec), + /// `FullUserDto` per row — same shape one row of the /me + /// response's embedded `full` carries. Emitted when + /// `?summary=true`. The FE seeds `resolveUser` cache from + /// `row.user` here (kills the per-row `/api/users/{id}` fetch). + /// The old `AdminUserSummaryDto` returned here has been replaced + /// by `FullUserDto`; see `docs/plan/userdto-refactor.md`. + Summary(Vec), } #[derive(serde::Serialize)] diff --git a/src/interfaces/api/handlers/auth_handler.rs b/src/interfaces/api/handlers/auth_handler.rs index 2f9bad63..44cc0171 100644 --- a/src/interfaces/api/handlers/auth_handler.rs +++ b/src/interfaces/api/handlers/auth_handler.rs @@ -11,9 +11,9 @@ use utoipa::ToSchema; use uuid::Uuid; use crate::application::dtos::user_dto::{ - AuthResponseDto, ChangePasswordDto, LoginDto, OidcCallbackQueryDto, OidcExchangeDto, - OidcProviderInfoDto, RefreshTokenDto, RegisterDto, SetupAdminDto, UpgradeToInternalDto, - UserDto, + AuthResponseDto, ChangePasswordDto, FullUserDto, LoginDto, OidcCallbackQueryDto, + OidcExchangeDto, OidcProviderInfoDto, RefreshTokenDto, RegisterDto, SelfUserDto, SetupAdminDto, + UpgradeToInternalDto, UserDto, }; use crate::application::services::auth_application_service::{OidcCallbackResult, RegisterResult}; use crate::common::di::AppState; @@ -626,7 +626,7 @@ pub async fn refresh_token( get, path = "/api/auth/me", responses( - (status = 200, description = "Current user profile", body = UserDto), + (status = 200, description = "Current user profile", body = SelfUserDto), (status = 401, description = "Not authenticated"), ), security(("bearerAuth" = [])), @@ -654,35 +654,58 @@ pub async fn get_current_user( // never count against this envelope — collaborating in a team drive // costs no personal bytes. The matching cap is // `storage_quota_bytes` (admin-only mutation). - let mut user = auth_service + // + // Single-query fetch: `get_user_with_derived_flags` returns the full + // `User` entity + `UserDerivedFlags` (has_password / OPAQUE flags / + // is_online) in one round-trip. That collapses what used to be a + // `get_user_by_id` + separate credential lookups into one wire trip, + // AND populates the OPAQUE flags on `/me` which the fat-UserDto path + // never did (it left them at false — the "quiet lie" that motivated + // this refactor, see `docs/plan/userdto-refactor.md`). + let (user, flags) = auth_service .auth_application_service - .get_user_by_id(user_id) + .get_user_with_derived_flags(user_id) .await?; + // Read the fields we need before moving `user` into FullUserDto below. + // Ordering matters: `can_edit_image` and the self-only bag fields + // must be captured while `user` is still borrowable; the + // `FullUserDto::build` call downstream consumes the entity. + let can_edit_image = !user.is_oidc_user(); + let ui_preferences = user.ui_preferences().clone(); + let notify_on_share = user.notify_on_share(); + // Overlay the cached `force_password_change` flag (see UserFlags). - // `From` defaults to false; the SPA reads this field on - // startup to decide whether to enter mandatory change-password - // mode. Using the cached path (`get_user_flags` → `user_flags_cache`) + // Using the cached path (`get_user_flags` → `user_flags_cache`) // avoids a second DB round-trip on this hot endpoint. - if let Ok(flags) = auth_service + let force_password_change = auth_service .auth_application_service .get_user_flags(user_id) .await - { - user.force_password_change = flags.force_password_change; - } + .map(|f| f.force_password_change) + .unwrap_or(false); // Session-binding state — read from the JWT `cnf.jkt` claim // (surfaced by the auth middleware into `CurrentUser.dpop_jkt`). // Present ⇒ the session that minted this JWT was bound; absent ⇒ - // the session is unbound and the SPA should call `/dpop/bind` - // to attach the browser's keypair (OIDC / magic-link redirect - // flow). Skips an otherwise-redundant `POST /dpop/bind` on every - // page load which would return 409 `already_bound` and litter - // the audit stream. - user.is_dpop_bound = auth_user.dpop_jkt.is_some(); + // the session is unbound and the SPA should call `/dpop/bind` to + // attach the browser's keypair (OIDC / magic-link redirect flow). + // Skips an otherwise-redundant `POST /dpop/bind` on every page load + // which would return 409 `already_bound` and litter the audit + // stream. + let is_dpop_bound = auth_user.dpop_jkt.is_some(); - Ok((StatusCode::OK, Json(user))) + let full = FullUserDto::build(user, flags); + let self_dto = SelfUserDto::build( + full, + ui_preferences, + notify_on_share, + is_dpop_bound, + force_password_change, + can_edit_image, + ); + + Ok((StatusCode::OK, Json(self_dto))) } /// DTO for updating the user's profile image. @@ -1820,10 +1843,12 @@ pub async fn oidc_exchange( tracing::info!( "OIDC token exchange successful for user: {}", auth_response + .user + .full .user .username .as_deref() - .unwrap_or(&auth_response.user.email) + .unwrap_or(&auth_response.user.full.user.email) ); // Set HttpOnly cookies for the browser diff --git a/src/interfaces/api/handlers/magic_link_handler.rs b/src/interfaces/api/handlers/magic_link_handler.rs index aa2c07bf..8a376387 100644 --- a/src/interfaces/api/handlers/magic_link_handler.rs +++ b/src/interfaces/api/handlers/magic_link_handler.rs @@ -625,7 +625,7 @@ fn redirect_target(redemption: &MagicLinkRedemption) -> String { (Some(MagicLinkResourceKind::Folder), Some(folder_id)) => { format!("/files/{}", folder_id) } - _ if redemption.auth.user.is_external => "/shared-with-me".to_string(), + _ if redemption.auth.user.full.user.is_external => "/shared-with-me".to_string(), _ => "/files".to_string(), } }