diff --git a/src/application/dtos/user_dto.rs b/src/application/dtos/user_dto.rs index 7c8feaa6..b3c225c1 100644 --- a/src/application/dtos/user_dto.rs +++ b/src/application/dtos/user_dto.rs @@ -1,5 +1,4 @@ use crate::domain::entities::user::User; -use crate::domain::repositories::user_repository::UserListEntry; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use smol_str::SmolStr; @@ -7,287 +6,6 @@ use std::sync::Arc; use utoipa::ToSchema; use uuid::Uuid; -#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] -pub struct UserDto { - pub id: String, - /// Optional handle. `None` for users who have not claimed one - /// (externals, fresh email-only signups). Frontend display callers - /// should walk `username → given/family → email` as their fallback - /// chain. Omitted from JSON when None (consistent with the existing - /// given_name / family_name fields). - #[serde(skip_serializing_if = "Option::is_none")] - pub username: Option, - pub email: String, - pub role: String, - pub storage_quota_bytes: i64, - pub storage_used_bytes: i64, - pub created_at: DateTime, - pub updated_at: DateTime, - pub last_login_at: Option>, - pub active: bool, - /// Which trust chain minted this user's federation identity — - /// `"oidc" | "ocm" | "magic_link"` — or `None` for pure local - /// users. Load-bearing for "is this user OIDC?"-shape predicates: - /// use `federation_kind == "oidc"` rather than string-scraping - /// `federation_issuer`. Serialized only when populated. - /// - /// Mirrors `auth.users.federation_kind` verbatim — same name at - /// DB, entity, and wire layers so there's no translation to reason - /// about. See docs/plan/ocm.md § Identity & auth model. - #[serde(skip_serializing_if = "Option::is_none")] - pub federation_kind: Option, - /// The authority that mints this user's `federation_subject` — - /// issuer URL for OIDC (id_token `iss` claim), peer domain for - /// OCM, `null` for local users (password / OPAQUE only). - /// - /// Renamed from `auth_provider` (which was a `String` with the - /// sentinel `"local"` for non-federated users, and a human-readable - /// label like `"MockSSO"` before Phase B). This shape mirrors the - /// `auth.users.federation_issuer` column directly: nullable when - /// there's no federation involved. FE predicates for "is this user - /// federated?" should read `federation_kind`, not - /// string-compare this value. - /// - /// When populated, FE code that wants a friendly display label - /// looks this value up against `OidcProviderInfoDto.issuer → - /// provider_name` to render the deployment's configured display - /// name; falls back to the raw issuer for foreign IdPs / legacy - /// rows still holding a pre-Phase-B label. - #[serde(skip_serializing_if = "Option::is_none")] - pub federation_issuer: Option, - pub image: Option, - pub can_edit_image: bool, - /// `true` for grant-only external recipients (magic-link, OIDC-only, - /// future OCM federated). External users have no home folder and - /// can't own storage; their quota is always 0. Internal users - /// default to `false`. - pub is_external: bool, - /// Optional first/given name. Populated from the OIDC `given_name` - /// claim at JIT provisioning, or via a profile-edit endpoint. - /// `None` until explicitly set — `skip_serializing_if = "Option::is_none"` - /// keeps the wire format compact for the common case. - #[serde(skip_serializing_if = "Option::is_none")] - pub given_name: Option, - /// Optional last/family name. Same provenance + serde rules as - /// `given_name`. - #[serde(skip_serializing_if = "Option::is_none")] - pub family_name: Option, - /// When the user first demonstrated control of their email (PR 23). - /// `None` = unverified (omitted from JSON). Stamped on the first - /// successful magic-link redemption or OIDC JIT with verified - /// claim. Idempotent — the original timestamp is preserved on - /// subsequent verifications. - #[serde(skip_serializing_if = "Option::is_none")] - pub email_verified_at: Option>, - /// User-chosen locale for server-rendered surfaces (emails, - /// future authenticated HTML). `None` = no preference (the server - /// resolves to `OXICLOUD_DEFAULT_LOCALE` when rendering). Round-trips - /// through `/api/auth/me` and `PATCH /api/auth/me/profile`. - #[serde(skip_serializing_if = "Option::is_none")] - pub preferred_locale: Option, - /// Whether the user wants an email when someone shares a resource - /// with them. `true` (default) = receive share-notification mails; - /// `false` = grants are still created but no email is sent. Honored - /// only on the plain-notification path — magic-link first-invitations - /// to brand-new external users always send, otherwise the recipient - /// could never claim the share. Round-trips through `/api/auth/me` - /// and `PATCH /api/auth/me/profile`. - pub notify_on_share: bool, - /// Opaque UI preferences bag. Cross-device store for pure UI - /// toggles (hide dotfiles, view mode, sidebar collapse, …). The - /// server never inspects the contents — this DTO field just echoes - /// what was PATCHed via `PATCH /api/auth/me/profile`. Shape is a - /// JSON object; the frontend defines the keys it cares about (see - /// `frontend/src/lib/stores/preferences.svelte.ts`). Always present - /// on the wire; empty bag is `{}`, never `null`. - pub ui_preferences: serde_json::Value, - /// Mirrors `auth.users.force_password_change_at_next_login`. Set - /// TRUE by the admin password-reset flow (see - /// `AuthApplicationService::admin_reset_password`) and cleared by - /// a successful self-service `POST /api/auth/change-password`. - /// - /// Populated only by the `/api/auth/me` handler and the login - /// response minter (via a distinct code path). `From` — used - /// by admin listings, share-recipient responses, group-member DTOs, - /// etc. — leaves it at `false`. The flag is a per-session-account - /// concern (does *this* user need to change their password before - /// they can proceed?), not a general user attribute worth - /// surfacing on every list row. - /// - /// The load-bearing consumer is the SPA's session store: on - /// startup and after every refresh, `/me` returns the current - /// flag value and the SPA's nav-guard blocks navigation to - /// anything but the change-password surface until it flips - /// back to false. Backend enforcement is separate (see the - /// `require_no_password_change_pending` middleware) — this DTO - /// field is what the SPA reads to render the mandatory-mode UI. - #[serde(default)] - pub force_password_change: bool, - /// TRUE when the account has a local Argon2id `password_hash` on - /// file. Distinct from `federation_kind`: an OIDC-linked account - /// (`federation_kind == "oidc"`) can ALSO carry a local password if - /// it was set at signup or later — a hybrid posture. The SPA - /// gates the profile page's change-password card on this flag, - /// so hybrid users can rotate their local password even though - /// they normally sign in via SSO. - /// - /// Populated only by the `/api/auth/me` handler. `From` in - /// this file leaves it `false` — other UserDto emitters (admin - /// listings, share-recipient responses, group members) do not - /// need to surface per-user credential state. - #[serde(default)] - pub has_password: bool, - /// TRUE when the caller's current session carries a DPoP JWK - /// thumbprint (`session.dpop_jkt IS NOT NULL`). Sourced from the - /// caller's JWT `cnf.jkt` claim — `is_some()` means the session - /// was bound at token-mint time. - /// - /// Populated only by the `/api/auth/me` handler; other UserDto - /// emitters leave it `false`. The SPA reads this on `session.load()` - /// to skip a redundant `POST /api/auth/dpop/bind` call when the - /// session is already bound (which would 409 and log noisily under - /// the audit stream — see the `already_bound` reject). Only the - /// OIDC / magic-link redirect flows land here as `false` on first - /// visit; password login binds at session-mint time so the very - /// first `/me` after login already reports `true`. - #[serde(default)] - pub is_dpop_bound: bool, -} - -/// Compact row returned by the paginated admin user table. -/// -/// Account-detail fields deliberately do not appear here. In particular, -/// omitting `image` and `ui_preferences` prevents a 100-row page from turning -/// into tens of MiB when users have uploaded avatars. `GET /api/admin/users/:id` -/// remains the full-detail endpoint. -#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] -pub struct AdminUserSummaryDto { - pub id: String, - #[serde(skip_serializing_if = "Option::is_none")] - pub username: Option, - pub email: String, - pub role: String, - pub storage_quota_bytes: i64, - pub storage_used_bytes: i64, - pub last_login_at: Option>, - pub active: bool, - /// See `UserDto::federation_kind` — same semantics, same wire spelling. - #[serde(skip_serializing_if = "Option::is_none")] - pub federation_kind: Option, - /// See `UserDto::federation_issuer` — same semantics, same wire spelling. - #[serde(skip_serializing_if = "Option::is_none")] - pub federation_issuer: Option, - pub is_external: bool, - /// TRUE when the user has a server-verifiable password on file - /// (`password_hash IS NOT NULL`). The admin table uses this - /// alongside `federation_issuer` and `opaque_registered` to render - /// the user's full capability set: a `password` chip lights up - /// here, an OIDC provider name renders the SSO badge, an - /// envelope-on-file flips the OPAQUE chip. A user with none of - /// the three is passwordless (magic-link only — the SPA renders - /// a distinct `passwordless` chip in that case). Admin-only - /// exposure — see the DTO doc for why this isn't on `UserDto`. - #[serde(default)] - pub has_password: bool, - /// Mirrors `UserListEntry::opaque_registered` — TRUE when the user - /// has an OPAQUE envelope on file. Surfaced on the admin table so - /// operators can see per-user rollout progress during the - /// migration window. **Admin-only exposure**: this field is NOT - /// on `UserDto` — putting it there would leak adoption status - /// through every user-directory-adjacent endpoint (share targets, - /// group members, invite listings). `#[serde(default)]` keeps - /// older SPA builds tolerant of the added field. - #[serde(default)] - pub opaque_registered: bool, - /// Mirrors `UserListEntry::opaque_migrated` — TRUE when the user - /// has completed at least one successful OPAQUE login. Distinct - /// from `opaque_registered`: an admin can invalidate the envelope - /// (`clear_registration`) leaving the user registered=false but - /// with a historical migrated=true; the SPA's admin table shows - /// both so this operational nuance is visible. - #[serde(default)] - pub opaque_migrated: bool, -} - -impl From for AdminUserSummaryDto { - fn from(entry: UserListEntry) -> Self { - Self { - id: entry.id.to_string(), - username: entry.username, - email: entry.email, - role: entry.role.to_string(), - storage_quota_bytes: entry.storage_quota_bytes, - storage_used_bytes: entry.storage_used_bytes, - last_login_at: entry.last_login_at, - active: entry.active, - federation_kind: entry.federation_kind, - federation_issuer: entry.federation_issuer, - is_external: entry.is_external, - has_password: entry.has_password, - opaque_registered: entry.opaque_registered, - opaque_migrated: entry.opaque_migrated, - } - } -} - -impl From for UserDto { - fn from(user: User) -> Self { - // `user` is owned and dropped here, so every owned field is MOVED out - // via `into_parts` rather than cloned through the borrowing accessors — - // the accessor form deep-cloned `image` (a data URI up to 512 KiB) and - // the whole `ui_preferences` JSON tree on every `/api/auth/me` and admin - // user listing (benches/ROUND20.md §A2). The two derived values read the - // entity before the move. - let role = format!("{}", user.role()); - let can_edit_image = !user.is_oidc_user(); - // has_password is derivable from the entity — read before the - // move. Cheap (bool from Option::is_some), no extra DB round- - // trip, so From can populate it uniformly rather than - // leaving it false and requiring per-call-site backfill. - let has_password = user.has_password(); - let p = user.into_parts(); - Self { - id: p.id.to_string(), - username: p.username, - email: p.email, - role, - storage_quota_bytes: p.storage_quota_bytes, - storage_used_bytes: p.storage_used_bytes, - created_at: p.created_at, - updated_at: p.updated_at, - last_login_at: p.last_login_at, - active: p.active, - // NULL on both fields for local users (no federation wired). - // FE predicates use `!!federation_kind` for "is federated?" — - // no "local" sentinel string; the null tells the whole story. - federation_kind: p.federation_kind.map(|k| k.as_str().to_string()), - federation_issuer: p.federation_issuer, - image: p.image, - can_edit_image, - is_external: p.is_external, - given_name: p.given_name, - family_name: p.family_name, - email_verified_at: p.email_verified_at, - preferred_locale: p.preferred_locale, - notify_on_share: p.notify_on_share, - ui_preferences: p.ui_preferences, - // Defaults to false. The `/me` handler + the login-response - // minter populate this via a distinct code path (a - // repo read that goes through the auth service's cache); - // admin listings and other UserDto consumers deliberately - // leave it false — the flag is per-session-account state, - // not a general user attribute. - force_password_change: false, - has_password, - // Populated only by `/api/auth/me` — the handler overlays - // the caller's session's actual DPoP binding state after - // this `From` runs. Other UserDto emitters leave - // this at `false` (they lack session context). - is_dpop_bound: false, - } - } -} - // ──────────────────────────────────────────────────────────────────────── // Three-layer user DTO family — see docs/plan/userdto-refactor.md. // @@ -301,9 +19,10 @@ impl From for UserDto { // `SelfUserDto` — `{ full: FullUserDto, ...self-only extras }`. Returned // by /api/auth/me and by every AuthResponseDto path. // -// The fat `UserDto` above is being phased out — the three types will replace -// it and its emitter sites migrate one at a time. Kept temporarily so this -// PR compiles at every checkpoint; deleted at the end of the refactor. +// Adding a field? Decide by audience: +// * Any authenticated caller may see it about another user → `PublicUserDto`. +// * Only admin (about another user) AND self (about self) → `FullUserDto`. +// * Only self about themselves → `SelfUserDto`. // ──────────────────────────────────────────────────────────────────────── /// Public identity — what any authenticated caller may see about ANOTHER @@ -823,7 +542,7 @@ pub struct OidcProviderInfoDto { /// users JIT-provisioned via this IdP. /// /// Populated so the frontend can resolve display: when - /// `UserDto.federation_issuer` equals this `issuer`, render + /// `PublicUserDto.federation_issuer` equals this `issuer`, render /// `provider_name` as the human-friendly label (avoids showing raw /// issuer URLs like `https://sso.example.com/realms/main` in the /// admin badge / profile view). Falls back to the raw issuer when diff --git a/src/application/ports/auth_ports.rs b/src/application/ports/auth_ports.rs index dc75e315..5a7bd955 100644 --- a/src/application/ports/auth_ports.rs +++ b/src/application/ports/auth_ports.rs @@ -3,7 +3,6 @@ use crate::domain::entities::app_password::AppPassword; use crate::domain::entities::device_code::DeviceCode; use crate::domain::entities::session::Session; use crate::domain::entities::user::User; -use crate::domain::repositories::user_repository::UserListEntry; use std::sync::Arc; use uuid::Uuid; @@ -194,15 +193,6 @@ pub trait UserStoragePort: Send + Sync + 'static { include_external: bool, ) -> Result, DomainError>; - /// Narrow user-list projection for management tables. Keeps heavyweight - /// account-detail fields off the database and JSON hot path. - async fn list_user_summaries( - &self, - limit: i64, - offset: i64, - include_external: bool, - ) -> Result, DomainError>; - /// 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/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index 7c8a70aa..fc28c146 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::{ - AuthResponseDto, ChangePasswordDto, FullUserDto, LoginDto, RefreshTokenDto, RegisterDto, - SelfUserDto, UpgradeToInternalDto, UserDto, + AuthResponseDto, ChangePasswordDto, FullUserDto, LoginDto, PublicUserDto, RefreshTokenDto, + RegisterDto, SelfUserDto, UpgradeToInternalDto, }; use crate::application::ports::auth_ports::{ OidcIdClaims, OidcServicePort, PasswordHasherPort, SessionStoragePort, TokenServicePort, @@ -319,11 +319,11 @@ pub enum OidcCallbackResult { #[derive(Debug, Clone)] pub enum RegisterResult { /// Boxed to avoid the `large_enum_variant` clippy warning — - /// `UserDto` is ~250 bytes, the other variants are zero-sized, + /// `PublicUserDto` is ~250 bytes, the other variants are zero-sized, /// so a heap-pointer indirection keeps the enum's stack size /// small. `register` is called once per request; the /// allocation cost is negligible. - Created(Box), + Created(Box), UsernameTaken, EmailTaken, } @@ -876,7 +876,7 @@ impl AuthApplicationService { is_external = false, "🛂 user registered", ); - Ok(RegisterResult::Created(Box::new(UserDto::from( + Ok(RegisterResult::Created(Box::new(PublicUserDto::from( created_user, )))) } @@ -894,7 +894,7 @@ impl AuthApplicationService { username: String, email: String, password: String, - ) -> Result { + ) -> Result { // Validate username if username.len() < 3 || username.len() > 254 { return Err(DomainError::new( @@ -981,7 +981,7 @@ impl AuthApplicationService { username, created_user.id() ); - Ok(UserDto::from(created_user)) + Ok(PublicUserDto::from(created_user)) } pub async fn login( @@ -2081,7 +2081,7 @@ impl AuthApplicationService { &self, caller_id: Uuid, dto: UpgradeToInternalDto, - ) -> Result { + ) -> Result { let mut user = self.user_storage.get_user_by_id(caller_id).await?; // Precondition: caller is currently external. Fast-path 409 so @@ -2182,7 +2182,7 @@ impl AuthApplicationService { lc.dispatch_upgraded_to_internal(&updated).await; } - Ok(UserDto::from(updated)) + Ok(PublicUserDto::from(updated)) } /// Admin-driven external → internal promotion. @@ -2211,7 +2211,7 @@ impl AuthApplicationService { &self, admin_id: Uuid, target_id: Uuid, - ) -> Result { + ) -> Result { let mut user = self.user_storage.get_user_by_id(target_id).await?; if !user.is_external() { @@ -2293,7 +2293,7 @@ impl AuthApplicationService { "👮🏻‍♂️ external user promoted to internal by admin", ); - Ok(UserDto::from(updated)) + Ok(PublicUserDto::from(updated)) } /// `keep_session_id` — when `Some`, revoke every OTHER session for @@ -2548,9 +2548,9 @@ impl AuthApplicationService { Ok(()) } - pub async fn get_user(&self, user_id: Uuid) -> Result { + pub async fn get_user(&self, user_id: Uuid) -> Result { let user = self.user_storage.get_user_by_id(user_id).await?; - Ok(UserDto::from(user)) + Ok(PublicUserDto::from(user)) } /// Cached, image-free lookup of the caller's authorization flags @@ -2699,7 +2699,7 @@ impl AuthApplicationService { caller_id: Uuid, dto: crate::application::dtos::user_dto::UpdateProfileDto, locale_registry: &crate::common::locale::LocaleRegistry, - ) -> Result { + ) -> Result { let mut user = self.user_storage.get_user_by_id(caller_id).await?; // For OIDC-managed users, refuse the patch ONLY when it touches @@ -2875,7 +2875,7 @@ impl AuthApplicationService { if changed.is_empty() && ui_prefs_patch.is_none() { // No-op — return the current user without a DB write. - return Ok(UserDto::from(user)); + return Ok(PublicUserDto::from(user)); } // Persist the typed-field changes first (if any). Skip the @@ -2906,11 +2906,11 @@ impl AuthApplicationService { // Refetch so the returned DTO reflects the merged JSONB bag // (the in-memory `user` above holds the pre-merge value). let refreshed = self.user_storage.get_user_by_id(caller_id).await?; - Ok(UserDto::from(refreshed)) + Ok(PublicUserDto::from(refreshed)) } // Alias for consistency with handler method - pub async fn get_user_by_id(&self, user_id: Uuid) -> Result { + pub async fn get_user_by_id(&self, user_id: Uuid) -> Result { self.get_user(user_id).await } @@ -3003,12 +3003,12 @@ impl AuthApplicationService { target_id: Uuid, expose_system_users: bool, pool: &sqlx::PgPool, - ) -> Result { + ) -> Result { // (1) Self — a single fetch suffices (the check compares the input // UUIDs, so the target read is never needed on this path). if caller_id == target_id { let caller = self.user_storage.get_user_by_id(caller_id).await?; - return Ok(UserDto::from(caller)); + return Ok(PublicUserDto::from(caller)); } // Caller and target are independent point reads (the self-case already @@ -3069,7 +3069,7 @@ impl AuthApplicationService { })?; if related.is_some() { - return Ok(UserDto::from(target)); + return Ok(PublicUserDto::from(target)); } // (3) External callers stop here — no directory enumeration. @@ -3097,12 +3097,12 @@ impl AuthApplicationService { // (4) Internal target + system-address-book exposed: already public. if !target.is_external() && expose_system_users { - return Ok(UserDto::from(target)); + return Ok(PublicUserDto::from(target)); } // (5) Admin caller: always visible. if caller.role() == UserRole::Admin { - return Ok(UserDto::from(target)); + return Ok(PublicUserDto::from(target)); } // (6) No relationship — anti-enumeration NotFound. @@ -3155,7 +3155,7 @@ impl AuthApplicationService { username: &str, expose_system_users: bool, pool: &sqlx::PgPool, - ) -> Result { + ) -> Result { let target = match self.user_storage.get_user_by_username(username).await { Ok(u) => u, Err(e) if e.kind == ErrorKind::NotFound => { @@ -3182,9 +3182,9 @@ impl AuthApplicationService { } // New method to get user by username - needed for admin user handling - pub async fn get_user_by_username(&self, username: &str) -> Result { + pub async fn get_user_by_username(&self, username: &str) -> Result { let user = self.user_storage.get_user_by_username(username).await?; - Ok(UserDto::from(user)) + Ok(PublicUserDto::from(user)) } // Method to count how many admin users exist in the system @@ -3202,9 +3202,13 @@ impl AuthApplicationService { /// sharee search, etc. — never expose external identities. Admin /// surfaces that need the full list should call /// [`list_users_including_external_with_perms`] instead. - pub async fn list_users(&self, limit: i64, offset: i64) -> Result, DomainError> { + pub async fn list_users( + &self, + limit: i64, + offset: i64, + ) -> Result, DomainError> { let users = self.user_storage.list_users(limit, offset, false).await?; - Ok(users.into_iter().map(UserDto::from).collect()) + Ok(users.into_iter().map(PublicUserDto::from).collect()) } /// Admin-only: lists users including external (grant-only) recipients. @@ -3215,10 +3219,10 @@ impl AuthApplicationService { caller_id: Uuid, limit: i64, offset: i64, - ) -> Result, DomainError> { + ) -> Result, DomainError> { self.require_admin_caller(authorization, caller_id).await?; let users = self.user_storage.list_users(limit, offset, true).await?; - Ok(users.into_iter().map(UserDto::from).collect()) + Ok(users.into_iter().map(PublicUserDto::from).collect()) } /// Admin-only user listing. Returns `Vec` — same @@ -3267,9 +3271,13 @@ impl AuthApplicationService { } /// Searches internal users only. See [`list_users`] for the rationale. - pub async fn search_users(&self, query: &str, limit: i64) -> Result, DomainError> { + pub async fn search_users( + &self, + query: &str, + limit: i64, + ) -> Result, DomainError> { let users = self.user_storage.search_users(query, limit, false).await?; - Ok(users.into_iter().map(UserDto::from).collect()) + Ok(users.into_iter().map(PublicUserDto::from).collect()) } /// Username-only search for the NC sharee autocomplete: identical @@ -3375,7 +3383,7 @@ impl AuthApplicationService { pub async fn admin_create_user( &self, dto: crate::application::dtos::settings_dto::AdminCreateUserDto, - ) -> Result { + ) -> Result { // Validate username length if dto.username.len() < 3 || dto.username.len() > 254 { return Err(DomainError::new( @@ -3533,7 +3541,7 @@ impl AuthApplicationService { created.id(), created.is_external() ); - Ok(UserDto::from(created)) + Ok(PublicUserDto::from(created)) } /// Admin-only: reset a user's password. @@ -3630,9 +3638,9 @@ impl AuthApplicationService { } /// Get a single user by ID (for admin panel) - pub async fn get_user_admin(&self, user_id: Uuid) -> Result { + pub async fn get_user_admin(&self, user_id: Uuid) -> Result { let user = self.user_storage.get_user_by_id(user_id).await?; - Ok(UserDto::from(user)) + Ok(PublicUserDto::from(user)) } /// Delete a user by ID (admin only). diff --git a/src/domain/entities/user.rs b/src/domain/entities/user.rs index 37a0e3ed..5e2c8692 100644 --- a/src/domain/entities/user.rs +++ b/src/domain/entities/user.rs @@ -207,7 +207,7 @@ pub struct User { /// Owned decomposition of a [`User`] (mirrors `FileParts` / `FolderParts` / /// `ContactParts`). Lets a consumer MOVE the heap fields out instead of cloning /// them through the borrowing accessors — notably `image` (a data URI up to -/// 512 KiB) and `ui_preferences` (a JSON tree). See `UserDto::from` +/// 512 KiB) and `ui_preferences` (a JSON tree). See `PublicUserDto::from` /// (benches/ROUND20.md §A2). pub struct UserParts { pub id: Uuid, diff --git a/src/domain/repositories/user_repository.rs b/src/domain/repositories/user_repository.rs index 9bc6bec0..5080a51b 100644 --- a/src/domain/repositories/user_repository.rs +++ b/src/domain/repositories/user_repository.rs @@ -1,6 +1,5 @@ use crate::common::errors::DomainError; use crate::domain::entities::user::{User, UserRole}; -use chrono::{DateTime, Utc}; use uuid::Uuid; #[derive(Debug, thiserror::Error)] @@ -26,72 +25,6 @@ pub enum UserRepositoryError { pub type UserRepositoryResult = Result; -/// Narrow projection for user-directory tables that do not need secrets, -/// profile pictures, or the cross-device UI-preferences document. -/// -/// The full [`User`] row intentionally carries all of those fields for account -/// detail and the system address book. Reusing it for the paginated admin -/// table made PostgreSQL detoast and transfer an avatar of up to 512 KiB per -/// row, only for the handler to serialize it back to the browser where the -/// table never reads it. Keeping the projection explicit prevents a future -/// full-row field from silently returning to that hot path. -#[derive(Debug, Clone)] -pub struct UserListEntry { - pub id: Uuid, - pub username: Option, - pub email: String, - pub role: UserRole, - pub storage_quota_bytes: i64, - pub storage_used_bytes: i64, - pub last_login_at: Option>, - pub active: bool, - pub federation_kind: Option, - pub federation_issuer: Option, - pub is_external: bool, - /// TRUE when `auth.users.password_hash IS NOT NULL` — user has a - /// server-verifiable password on file (legacy or admin-set). - /// Distinct from `opaque_registered` (which is the zero-knowledge - /// envelope): a fully-migrated user carries BOTH — password for - /// the fallback / operator flows, envelope for the actual login. - /// A user with `has_password = false AND !opaque_registered AND - /// federation_issuer IS NULL` is passwordless — the only path in is - /// via magic-link (or, for externals, whatever grant they hold). - pub has_password: bool, - /// TRUE when `auth.users.opaque_envelope IS NOT NULL` — the user - /// has completed OPAQUE registration (typically via the Phase 2 - /// silent-migration hook after a successful legacy login). Surfaced - /// on the admin user table so operators can see rollout progress - /// per-user. Admin-only exposure — see `AdminUserSummaryDto`. - pub opaque_registered: bool, - /// TRUE when `auth.users.opaque_migrated_at IS NOT NULL` — the - /// user has completed at least one successful OPAQUE login. Distinct - /// from `opaque_registered` because a user can have an envelope on - /// file without having actually logged in via OPAQUE yet (e.g. - /// admin cleared the envelope, silent-migration hasn't re-run). - pub opaque_migrated: bool, - /// Optional avatar payload (base64, up to 512 KiB per row). Included - /// on the admin list projection so the SPA can seed its per-user - /// `resolveUser` cache from the list row and skip the follow-up - /// `/api/users/{id}` fetch UserVignette would otherwise trigger. - /// The narrow-projection concern that motivated omitting this - /// column originally is retired by that cache-seeding path — the - /// bytes now do useful work per page load instead of being - /// discarded. Deferred: moving avatar storage out of the row - /// entirely (planned refactor); this shape is transitional. - pub image: Option, - /// Presence signal — TRUE when the server observed a request on - /// any of this user's non-revoked sessions within the last - /// [`ONLINE_WINDOW`](crate::application::dtos::session_dto::ONLINE_WINDOW) - /// (5 min). Populated via an `EXISTS(...)` subquery on - /// `auth.sessions` in the list projection — the partial index - /// `idx_sessions_last_seen_at WHERE revoked = FALSE` covers the - /// scan, so per-row cost is ~μs. Surfaces to the FE via - /// `UserDto::is_online` so both `/api/users/{id}` and the admin - /// listing carry it, and the admin table renders a green/grey - /// presence dot next to each vignette. - pub is_online: bool, -} - /// DB-computed booleans about a user that aren't fields on the /// [`User`](crate::domain::entities::user::User) entity itself — /// either derived from column presence (`password_hash IS NOT NULL`) @@ -104,10 +37,8 @@ pub struct UserListEntry { /// both admin AND self read. The name reflects "derived from the DB /// row, not intrinsic to the User entity". /// -/// See `docs/plan/userdto-refactor.md` for the phasing that -/// introduces this type; it will replace [`UserListEntry`] once the -/// list repo is switched from narrow projection to -/// `Vec<(User, UserDerivedFlags)>` (P6 of the refactor). +/// See `docs/plan/userdto-refactor.md` for the design; this type +/// replaced the earlier `UserListEntry` narrow projection as of P6. #[derive(Debug, Clone, Copy)] pub struct UserDerivedFlags { pub has_password: bool, @@ -211,22 +142,6 @@ pub trait UserRepository: Send + Sync + 'static { include_external: bool, ) -> UserRepositoryResult>; - /// 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, - offset: i64, - 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 diff --git a/src/infrastructure/repositories/pg/user_pg_repository.rs b/src/infrastructure/repositories/pg/user_pg_repository.rs index f8d50fda..7dd3bae0 100644 --- a/src/infrastructure/repositories/pg/user_pg_repository.rs +++ b/src/infrastructure/repositories/pg/user_pg_repository.rs @@ -7,7 +7,7 @@ use crate::application::ports::auth_ports::UserStoragePort; use crate::common::errors::DomainError; use crate::domain::entities::user::{User, UserFlags, UserRole}; use crate::domain::repositories::user_repository::{ - StorageStats, UserListEntry, UserRepository, UserRepositoryError, UserRepositoryResult, + StorageStats, UserRepository, UserRepositoryError, UserRepositoryResult, }; use crate::infrastructure::repositories::pg::transaction_utils::with_transaction; @@ -919,135 +919,6 @@ impl UserRepository for UserPgRepository { Ok(users) } - async fn list_user_summaries( - &self, - limit: i64, - offset: i64, - include_external: bool, - ) -> UserRepositoryResult> { - let rows = sqlx::query_as::< - _, - ( - Uuid, - Option, - String, - String, - i64, - i64, - Option>, - bool, - Option, - Option, - bool, - bool, - bool, - bool, - Option, - bool, - ), - >( - // Auth-credential columns projected as booleans via `IS NOT - // NULL` rather than as timestamps / hashes so the row-mapping - // tuple stays small and the wire shape is exactly what the - // admin table needs. Per-row scalar tests — no cost beyond - // the full-table sequential scan the LIMIT/OFFSET already - // pays. `has_password` on the password_hash column tells - // the admin table whether a server-verifiable password is - // on file; combined with the two OPAQUE flags and - // federation_kind / federation_issuer, the SPA derives the - // full "capability set" per user (password / OPAQUE / SSO / - // passwordless). - // - // `image` is projected too — the previous narrow projection - // (ROUND12 §Q1 / ROUND13 §Q1) discarded up to 512 KiB per - // row because the admin table never rendered it. That's now - // reversed: the SPA seeds its per-user `resolveUser` cache - // from these rows to kill the N+1 `/api/users/{id}` fetches - // UserVignette would otherwise trigger. - // - // `is_online` uses an EXISTS scalar subquery against - // `auth.sessions` — the partial index - // `idx_sessions_last_seen_at WHERE revoked = FALSE` covers - // the lookup, so per-row cost is ~μs. The window comes from - // `application::dtos::session_dto::ONLINE_WINDOW` (bound as - // `$4` seconds), same single-source-of-truth pattern the - // `session_liveness_gauges` module uses. - r#" - SELECT - id, username, email, role::text, - storage_quota_bytes, storage_used_bytes, - last_login_at, active, - federation_kind, federation_issuer, is_external, - (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, - image, - 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.as_ref()) - .await - .map_err(Self::map_sqlx_error)?; - - Ok(rows - .into_iter() - .map( - |( - id, - username, - email, - role, - storage_quota_bytes, - storage_used_bytes, - last_login_at, - active, - federation_kind, - federation_issuer, - is_external, - has_password, - opaque_registered, - opaque_migrated, - image, - is_online, - )| UserListEntry { - id, - username, - email, - role: if role == "admin" { - UserRole::Admin - } else { - UserRole::User - }, - storage_quota_bytes, - storage_used_bytes, - last_login_at, - active, - federation_kind, - federation_issuer, - is_external, - has_password, - opaque_registered, - opaque_migrated, - image, - is_online, - }, - ) - .collect()) - } - async fn list_users_with_derived_flags( &self, limit: i64, @@ -1588,17 +1459,6 @@ impl UserStoragePort for UserPgRepository { .map_err(DomainError::from) } - async fn list_user_summaries( - &self, - limit: i64, - offset: i64, - include_external: bool, - ) -> Result, DomainError> { - UserRepository::list_user_summaries(self, limit, offset, include_external) - .await - .map_err(DomainError::from) - } - async fn list_users_with_derived_flags( &self, limit: i64, @@ -1992,26 +1852,27 @@ mod integration_tests { ) .await; - let page = UserRepository::list_user_summaries(&repo, 3, 0, true) + // Migrated from the (now-deleted) `list_user_summaries` + + // `UserListEntry` to `list_users_with_derived_flags`, which + // returns `Vec<(User, UserDerivedFlags)>`. Field checks read + // through the `User` accessors instead of struct-field access. + let page = UserRepository::list_users_with_derived_flags(&repo, 3, 0, true) .await .expect("compact projection query must decode"); - assert_eq!(page.iter().map(|entry| entry.id).collect::>(), ids); - assert_eq!(page[0].username.as_deref(), Some(username_a.as_str())); - assert_eq!(page[0].role, UserRole::Admin); - assert_eq!(page[0].storage_quota_bytes, 10_737_418_240); - assert_eq!(page[1].username, None); - assert!(page[1].is_external); - assert_eq!( - page[1].federation_issuer.as_deref(), - Some("integration-idp") - ); + assert_eq!(page.iter().map(|(u, _)| u.id()).collect::>(), ids); + assert_eq!(page[0].0.username(), Some(username_a.as_str())); + assert_eq!(page[0].0.role(), UserRole::Admin); + assert_eq!(page[0].0.storage_quota_bytes(), 10_737_418_240); + assert_eq!(page[1].0.username(), None); + assert!(page[1].0.is_external()); + assert_eq!(page[1].0.federation_issuer(), Some("integration-idp")); - let internal = UserRepository::list_user_summaries(&repo, 10, 0, false) + let internal = UserRepository::list_users_with_derived_flags(&repo, 10, 0, false) .await .expect("internal compact projection query must decode"); - assert!(internal.iter().any(|entry| entry.id == ids[0])); - assert!(internal.iter().any(|entry| entry.id == ids[2])); - assert!(!internal.iter().any(|entry| entry.id == ids[1])); + assert!(internal.iter().any(|(u, _)| u.id() == ids[0])); + assert!(internal.iter().any(|(u, _)| u.id() == ids[2])); + assert!(!internal.iter().any(|(u, _)| u.id() == ids[1])); sqlx::query("DELETE FROM auth.users WHERE id = ANY($1)") .bind(ids.as_slice()) diff --git a/src/interfaces/api/handlers/admin_handler.rs b/src/interfaces/api/handlers/admin_handler.rs index bb5e12d2..b727b5a1 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::{FullUserDto, UserDto}; +use crate::application::dtos::user_dto::{FullUserDto, PublicUserDto}; 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,10 +42,10 @@ use uuid::Uuid; #[derive(serde::Serialize)] #[serde(untagged)] enum AdminUsersPayload { - /// Fat-`UserDto` per row. Emitted when `?summary=false` — legacy + /// Fat-`PublicUserDto` 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), + Full(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 @@ -1571,7 +1571,7 @@ pub async fn reset_user_password( path = "/api/admin/users/{id}/promote-to-internal", params(("id" = String, Path, description = "Target user id")), responses( - (status = 200, description = "User promoted", body = UserDto), + (status = 200, description = "User promoted", body = PublicUserDto), (status = 400, description = "Magic-link login is disabled on this deployment"), (status = 401, description = "Unauthorized"), (status = 403, description = "Admin required (or target is OIDC-linked)"), diff --git a/src/interfaces/api/handlers/app_password_handler.rs b/src/interfaces/api/handlers/app_password_handler.rs index f2b36b14..ed8a9023 100644 --- a/src/interfaces/api/handlers/app_password_handler.rs +++ b/src/interfaces/api/handlers/app_password_handler.rs @@ -61,7 +61,7 @@ async fn create_app_password( } // Require a claimed username. NextCloud Basic Auth resolves users by - // username; an app password is unusable without one. UserDto carries + // username; an app password is unusable without one. PublicUserDto carries // an empty string when the underlying `users.username` is NULL — the // entity rejects empty strings on construction, so empty here is an // unambiguous signal that the column is NULL. diff --git a/src/interfaces/api/handlers/auth_handler.rs b/src/interfaces/api/handlers/auth_handler.rs index 44cc0171..18d4f129 100644 --- a/src/interfaces/api/handlers/auth_handler.rs +++ b/src/interfaces/api/handlers/auth_handler.rs @@ -12,8 +12,8 @@ use uuid::Uuid; use crate::application::dtos::user_dto::{ AuthResponseDto, ChangePasswordDto, FullUserDto, LoginDto, OidcCallbackQueryDto, - OidcExchangeDto, OidcProviderInfoDto, RefreshTokenDto, RegisterDto, SelfUserDto, SetupAdminDto, - UpgradeToInternalDto, UserDto, + OidcExchangeDto, OidcProviderInfoDto, PublicUserDto, RefreshTokenDto, RegisterDto, SelfUserDto, + SetupAdminDto, UpgradeToInternalDto, }; use crate::application::services::auth_application_service::{OidcCallbackResult, RegisterResult}; use crate::common::di::AppState; @@ -89,7 +89,7 @@ pub fn setup_route() -> Router> { /// the `audit` channel as `auth.register` with `reason` one of /// `created`, `email_taken`, `username_taken`. /// - **SMTP not configured**: there is no welcome-mail cover story, so -/// the classic `201 + UserDto` on success and `409` on collision +/// the classic `201 + PublicUserDto` on success and `409` on collision /// apply. Anti-enumeration would just be misleading UX (telling the /// user to check an email that will never arrive). Email-only /// signup is **503** in this mode because the user would otherwise @@ -106,7 +106,7 @@ pub fn setup_route() -> Router> { request_body = RegisterDto, responses( (status = 200, description = "Uniform registration response (SMTP configured, anti-enumeration mode)"), - (status = 201, description = "User registered successfully (SMTP not configured)", body = UserDto), + (status = 201, description = "User registered successfully (SMTP not configured)", body = PublicUserDto), (status = 400, description = "Validation error (malformed request body)"), (status = 403, description = "Registration disabled (admin setting or OIDC-only mode)"), (status = 409, description = "Username or email already taken (SMTP not configured)"), @@ -280,7 +280,7 @@ pub async fn register( } Ok(resp) } else { - // Classic mode: clear 201 + UserDto so the frontend can + // Classic mode: clear 201 + PublicUserDto so the frontend can // log the user in directly with the password they just // submitted. Unbox the DTO for the JSON serialisation. Ok((StatusCode::CREATED, Json(*user)).into_response()) @@ -659,7 +659,7 @@ pub async fn get_current_user( // `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 + // AND populates the OPAQUE flags on `/me` which the fat-PublicUserDto 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 @@ -860,14 +860,14 @@ pub async fn change_password( /// self-registration policy. Refused with 403 /// `error_type = "RegistrationDomainNotAllowed"`. /// -/// Response: the updated `UserDto` (post-upgrade view — `is_external` +/// Response: the updated `PublicUserDto` (post-upgrade view — `is_external` /// is false, `storage_quota_bytes` is set). #[utoipa::path( post, path = "/api/auth/upgrade-to-internal", request_body = UpgradeToInternalDto, responses( - (status = 200, description = "Upgrade succeeded", body = UserDto), + (status = 200, description = "Upgrade succeeded", body = PublicUserDto), (status = 400, description = "Password missing / too short"), (status = 401, description = "Not authenticated"), (status = 403, description = "OIDC user, or domain not in allowlist"), @@ -965,7 +965,7 @@ pub async fn upgrade_to_internal( path = "/api/auth/me/profile", request_body = crate::application::dtos::user_dto::UpdateProfileDto, responses( - (status = 200, description = "Updated profile (UserDto)", body = UserDto), + (status = 200, description = "Updated profile (PublicUserDto)", body = PublicUserDto), (status = 400, description = "Validation error (e.g. invalid handle format, empty given_name)"), (status = 401, description = "Not authenticated"), (status = 403, description = "OIDC-managed profile — edit at the IdP"), @@ -1249,7 +1249,7 @@ pub struct BackchannelLogoutForm { path = "/api/setup", request_body = SetupAdminDto, responses( - (status = 201, description = "First admin created and system initialized", body = UserDto), + (status = 201, description = "First admin created and system initialized", body = PublicUserDto), (status = 403, description = "System already initialized"), (status = 503, description = "Auth service not configured"), ), diff --git a/src/interfaces/api/handlers/contacts_handler.rs b/src/interfaces/api/handlers/contacts_handler.rs index b1b495f9..22fb9f22 100644 --- a/src/interfaces/api/handlers/contacts_handler.rs +++ b/src/interfaces/api/handlers/contacts_handler.rs @@ -16,7 +16,7 @@ use crate::application::dtos::contact_dto::{ AddressDto, ContactDto, ContactGroupDto, CreateContactDto, CreateContactGroupDto, EmailDto, GroupMembershipDto, PhoneDto, UpdateContactDto, UpdateContactGroupDto, }; -use crate::application::dtos::user_dto::UserDto; +use crate::application::dtos::user_dto::PublicUserDto; use crate::application::ports::carddav_ports::{AddressBookUseCase, ContactUseCase}; use crate::application::services::auth_application_service::AuthApplicationService; use crate::application::services::contact_service::ContactService; @@ -185,14 +185,14 @@ fn if_match_passes(if_match: Option<&str>, stored_etag: &str) -> bool { } } -/// Map a `UserDto` to a `ContactDto` so OxiCloud users appear as contacts +/// Map a `PublicUserDto` to a `ContactDto` so OxiCloud users appear as contacts /// inside the virtual system address book. /// /// `given_name`/`family_name` come from OIDC standard claims at JIT /// provisioning (or NULL for password-only or pre-OIDC users). When /// they're present, prefer a "First Last" full name; otherwise fall /// back to the username (which is always present). -fn user_to_contact(user: UserDto) -> ContactDto { +fn user_to_contact(user: PublicUserDto) -> ContactDto { // Display fallback chain: given+family name → username → email. // Username is `Option` post PR 16; externals start with None. let full_name = match (user.given_name.as_deref(), user.family_name.as_deref()) { @@ -222,8 +222,19 @@ fn user_to_contact(user: UserDto) -> ContactDto { photo_url: user.image.clone(), birthday: None, anniversary: None, - created_at: user.created_at, - updated_at: user.updated_at, + // System-book contacts are VIRTUAL projections of the user + // directory — they have no independent creation history. Stamp + // both timestamps with `Utc::now()` so the ContactDto shape is + // satisfied; CardDAV clients ETag on the vCard content (see + // `etag` below, keyed on the stable user id), not on these + // wrapper timestamps. + // + // Previously read `user.created_at` / `user.updated_at` from the + // fat `UserDto`; those fields moved to `FullUserDto` under the + // three-layer refactor (docs/plan/userdto-refactor.md) and are + // not exposed on the slim `PublicUserDto` this function receives. + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), etag: user.id, } } diff --git a/src/interfaces/api/handlers/users_handler.rs b/src/interfaces/api/handlers/users_handler.rs index b2d0d943..f6aee02e 100644 --- a/src/interfaces/api/handlers/users_handler.rs +++ b/src/interfaces/api/handlers/users_handler.rs @@ -1,6 +1,6 @@ //! User-profile lookup for the frontend. //! -//! `GET /api/users/{id}` returns a [`UserDto`] for the target user iff +//! `GET /api/users/{id}` returns a [`PublicUserDto`] for the target user iff //! the authenticated caller has a legitimate relationship with them. //! The visibility rule lives in //! [`AuthApplicationService::get_user_profile`] — handlers never embed diff --git a/src/interfaces/api/mod.rs b/src/interfaces/api/mod.rs index 03d7008e..9bc4cfcc 100644 --- a/src/interfaces/api/mod.rs +++ b/src/interfaces/api/mod.rs @@ -46,7 +46,7 @@ use crate::application::dtos::trash_dto::{ }; use crate::application::dtos::user_dto::{ AuthResponseDto, ChangePasswordDto, LoginDto, OidcExchangeDto, OidcProviderInfoDto, - RefreshTokenDto, RegisterDto, SetupAdminDto, UserDto, + PublicUserDto, RefreshTokenDto, RegisterDto, SetupAdminDto, }; use crate::application::ports::chunked_upload_ports::{ ChunkUploadResponseDto, CreateUploadResponseDto, UploadStatusResponseDto, @@ -367,7 +367,7 @@ use crate::interfaces::api::handlers::file_handler::MoveFilePayload; PaginationDto, PaginationRequestDto, // User / Auth schemas - UserDto, + PublicUserDto, LoginDto, RegisterDto, SetupAdminDto, @@ -583,7 +583,7 @@ mod tests { "FolderDto", "ShareDto", "TrashedItemDto", - "UserDto", + "PublicUserDto", ] { assert!(schemas.contains_key(name), "missing schema: {name}"); } diff --git a/src/interfaces/nextcloud/ocs_handler.rs b/src/interfaces/nextcloud/ocs_handler.rs index da1c19b1..39e832d8 100644 --- a/src/interfaces/nextcloud/ocs_handler.rs +++ b/src/interfaces/nextcloud/ocs_handler.rs @@ -191,7 +191,15 @@ async fn user_provisioning_response( return Json(ocs_err(997, "Database pool not available")).into_response(); }; - let user_dto = match auth_service + // Two-step lookup: (1) `get_user_profile_by_username_with_perms` + // gates access via the same visibility engine the REST endpoint + // uses; (2) if visibility passes, `get_user_with_derived_flags` + // hydrates the OCS-specific fields (federation_kind / last_login_at + // / active) that live on `FullUserDto` but not on the slim + // `PublicUserDto` returned by the visibility gate. Second call is + // ~1 DB round-trip on the maintenance pool; NC OCS provisioning is + // not on any hot inner loop. + let public = match auth_service .get_user_profile_by_username_with_perms( user.id, &userid, @@ -205,9 +213,27 @@ async fn user_provisioning_response( return Json(ocs_err(404, "User not found")).into_response(); } }; + let target_id = match uuid::Uuid::parse_str(&public.id) { + Ok(u) => u, + Err(_) => { + // Should be unreachable — PublicUserDto.id is always the + // serialised form of a Uuid. Fail closed if this invariant + // is ever violated. + return Json(ocs_err(500, "Malformed user id")).into_response(); + } + }; + let user_dto = match auth_service.get_user_with_derived_flags(target_id).await { + Ok((user, flags)) => crate::application::dtos::user_dto::FullUserDto::build(user, flags), + Err(_) => { + // Visibility already passed above; a miss here would mean + // the user was deleted between the two round-trips. Fall + // back to the 404 shape (anti-enum invariant still holds). + return Json(ocs_err(404, "User not found")).into_response(); + } + }; // Determine groups based on role - let groups = if user_dto.role == "admin" { + let groups = if user_dto.user.role == "admin" { vec!["admin", "users"] } else { vec!["users"] @@ -235,7 +261,7 @@ async fn user_provisioning_response( // Fetch quota from storage usage service let quota: (i64, i64) = match state.storage_usage_service.as_ref() { Some(service) => match service - .get_user_storage_info(uuid::Uuid::parse_str(&user_dto.id).unwrap_or_default()) + .get_user_storage_info(uuid::Uuid::parse_str(&user_dto.user.id).unwrap_or_default()) .await { Ok((used, total)) => (used, total), @@ -256,10 +282,10 @@ async fn user_provisioning_response( "meta": { "status": "ok", "statuscode": statuscode, "message": "OK" }, "data": { "enabled": user_dto.active, - "id": user_dto.username, - "display-name": user_dto.username, - "displayname": user_dto.username, - "email": user_dto.email, + "id": user_dto.user.username, + "display-name": user_dto.user.username, + "displayname": user_dto.user.username, + "email": user_dto.user.email, "phone": "", "address": "", "website": "",