From ec70b21c6e7483368613baeddb2218b69c924b46 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Fri, 21 Aug 2026 14:06:54 +0200 Subject: [PATCH] refactor(User): clear separation PublicUserDto, FullUserDto, SelfUserDto --- docs/plan/userdto-refactor.md | 416 ++++++++++++++++++ src/application/dtos/user_dto.rs | 256 +++++++++++ src/domain/repositories/user_repository.rs | 45 ++ .../repositories/pg/user_pg_repository.rs | 31 +- 4 files changed, 747 insertions(+), 1 deletion(-) create mode 100644 docs/plan/userdto-refactor.md diff --git a/docs/plan/userdto-refactor.md b/docs/plan/userdto-refactor.md new file mode 100644 index 00000000..17fe98aa --- /dev/null +++ b/docs/plan/userdto-refactor.md @@ -0,0 +1,416 @@ +# UserDto Refactor — Three-Layer Split (Public / Full / Self) + +Establish three DTO shapes for representing a user on the wire, each +with a single unambiguous audience, composed hierarchically so the +overlap between audiences is defined ONCE: + +- **`PublicUserDto`** — public identity. What any authenticated caller + may see about *another* user. Returned by `/api/users/{id}`, share + responses, group members, magic-link invitees, recipient enrichment. +- **`FullUserDto`** = `PublicUserDto` + all fields that BOTH an admin + (viewing another user) AND the subject themselves (viewing + themselves) may see. Returned as `Vec` by + `/api/admin/users`. Closest DTO to the underlying `auth.users` row. +- **`SelfUserDto`** = `FullUserDto` + self-only preferences, + session-scoped flags, and caller-scoped permissions. Returned by + `/api/auth/me` and by the login / refresh / OIDC / magic-link auth + response. + +Composition (`FullUserDto.user: PublicUserDto`, +`SelfUserDto.full: FullUserDto`) means the public-identity contract +has ONE definition; the overlap between admin's view and self's view +is another single definition. Adding a new field naturally finds its +level: + +- Useful to any authenticated caller? → `PublicUserDto`. +- Useful only to admin (about another user) and the subject + themselves? → `FullUserDto`. +- Meaningful only to the caller viewing themselves? → `SelfUserDto`. + +Companion of `docs/plan/sessions.md` (which introduced `is_online` +and motivated widening the DTO for presence). Same principle: pick +the audience first, structure the DTO around it, don't let the same +field mean different things on different endpoints. + +## Why now — the problems this fixes + +Today's single `UserDto` conflates three audiences. Symptoms: + +1. **The "quiet lie"**. `UserDto::has_password` is populated only by + `/api/auth/me`; every other emitter (`From`) leaves it + `false`. A share-recipient DTO on the wire says + `has_password: false` unconditionally, which an attacker + scraping share responses could misread as "this user is + passwordless" when the truth is "we didn't fill this field in + for you". Same pattern for `force_password_change` and + `is_dpop_bound`. See `src/application/dtos/user_dto.rs:134-138` + for the explicit disclaimer — the convention exists precisely + because the field placement is wrong. + +2. **Private signals leak by default**. `last_login_at`, + `notify_on_share`, `ui_preferences`, `preferred_locale`, + `federation_kind`, `email_verified_at`, and + `storage_used_bytes` all ride on `UserDto` and are returned to + any authenticated caller who can see a given user. Group + members can see when their peers last logged in, which IdP they + federate with, and how full their disks are. None of this is + information a share picker or a member listing needs. + +3. **`AdminUserSummaryDto` duplicates a chunk of `UserDto` verbatim** + (id / username / email / role / quotas / last_login_at / active / + federation_* / is_external), then adds three admin-only fields + (has_password / opaque_registered / opaque_migrated). The two + shapes drift naturally as new fields are added — no compile-time + guarantee they stay in sync. + +4. **N+1 in the admin panel**. Because `AdminUserSummaryDto` doesn't + include `image`, the admin users table fires `/api/users/{id}` + per row so `UserVignette` can render the avatar. Composition + (`FullUserDto.user.image`) lets the admin listing seed the SPA's + per-user cache from the list rows directly. + +## Target shapes + +### `PublicUserDto` — public identity (9 fields) + +Applied rule: "would a share picker / group member listing / +recipient enrichment need this? if no, it doesn't belong here." + +```rust +pub struct PublicUserDto { + pub id: String, + pub username: Option, + pub email: String, + pub role: String, // sharee UI renders admin badge + pub image: Option, // avatar + pub is_external: bool, // external badge + pub given_name: Option, // social identity + pub family_name: Option, // social identity + pub is_online: bool, // presence — social signal +} +``` + +Every existing UserDto emitter site (`From`, share responses, +group members, magic-link invitees, sharee-vignette lookup) +returns this slim shape. All private signals below vanish from +those wire paths. + +### `FullUserDto` — admin's view of anyone + self's view of self (13 extras) + +The fields the SUBJECT themselves may know about themselves that +an ADMIN may also know about the subject. Composed on top of +`PublicUserDto`. This is the DTO closest to the underlying +`auth.users` row. + +```rust +pub struct FullUserDto { + /// Public identity — same set any authenticated caller can see. + pub user: PublicUserDto, + /// IdP linkage. Which SSO provider a peer uses is a soft + /// org-affiliation leak; not needed by share pickers. + pub federation_kind: Option, + pub federation_issuer: Option, + /// Subject's own locale preference. Only THEY or an admin + /// managing them needs this — other callers use their own. + pub preferred_locale: Option, + /// Email-verification stamp. Trust signal — meaningful to admin + /// (auditing verification status) and to self (own record), but + /// not to a share picker rendering a vignette. + pub email_verified_at: Option>, + /// Row bookkeeping — not rendered on any non-admin surface today. + pub created_at: DateTime, + pub updated_at: DateTime, + /// Activity signal — private. + pub last_login_at: Option>, + /// Account-active flag — private (a deactivated user couldn't + /// reach `/me` anyway, but admin needs to see it). + pub active: bool, + /// Storage quotas — personal financials. Admin manages others'; + /// self sees own. + pub storage_quota_bytes: i64, + pub storage_used_bytes: i64, + /// Auth capability set — has_password / OPAQUE flags. Kept off + /// public identity because per-user auth adoption leaks through + /// directory endpoints. + pub has_password: bool, + pub opaque_registered: bool, + pub opaque_migrated: bool, +} +``` + +### `SelfUserDto` — /api/auth/me (5 extras) + +Everything the caller may see about themselves that no other +caller (not even an admin) needs to see: pure self-scoped state. + +```rust +pub struct SelfUserDto { + /// Full profile. Every field an admin would see about you is + /// here — same shape as one row of /api/admin/users. + pub full: FullUserDto, + /// Opaque UI preferences bag — my own UI state. Cross-device + /// via PATCH /api/auth/me/profile. + pub ui_preferences: serde_json::Value, + /// Whether I want share-notification emails. + pub notify_on_share: bool, + /// Session-scoped: my current session carries a DPoP thumbprint. + /// SPA skips a redundant /api/auth/dpop/bind on load when true. + pub is_dpop_bound: bool, + /// Admin-set temp-password gate — SPA nav guard blocks everything + /// but /change-password until this flips back. + pub force_password_change: bool, + /// Caller-scoped permission: can I edit my own avatar? False for + /// OIDC users whose avatar comes from the IdP. Only meaningful + /// when caller == subject; nonsense on any other DTO. + pub can_edit_image: bool, +} +``` + +## Endpoint mapping + +| Endpoint | Old shape | New shape | +|---|---|---| +| `/api/auth/me` | `UserDto` (fat) | `SelfUserDto` | +| `/api/auth/login` / `/refresh` / OIDC callback / magic-link redemption | `AuthResponseDto { user: UserDto }` | `AuthResponseDto { user: SelfUserDto }` | +| `/api/admin/users` | `Vec` | `Vec` | +| `/api/users/{id}` | `UserDto` (fat) | `PublicUserDto` (9 fields) | +| Share responses / group members / magic-link invitees / recipient enrichment | `UserDto` (fat) | `PublicUserDto` | + +**Login response ships `SelfUserDto`, not `PublicUserDto`.** The SPA +needs `has_password`, `ui_preferences`, `is_dpop_bound`, and +`force_password_change` immediately post-login to avoid a UI race +with the first `/me` fetch. Same rationale for refresh and +OIDC/magic-link callback: the SPA's post-auth state must be +complete in one round trip. + +## Backend callsite inventory + +**DTO layer** (`src/application/dtos/user_dto.rs`): + +- Replace `UserDto` with `PublicUserDto` (renamed AND slimmed — + same rename forces every consumer to consciously pick the new + shape rather than silently losing fields). +- Add `FullUserDto` and `SelfUserDto`. +- Delete `AdminUserSummaryDto` (superseded by `FullUserDto`). +- `impl From for PublicUserDto` — the entry point. Maps 1:1 + to `User`'s public identity accessors. +- `FullUserDto::build(user: User, flags: UserDerivedFlags)` — + wraps a `PublicUserDto` plus the DB-computed booleans not on + `User` (`has_password`, `opaque_registered`, `opaque_migrated`, + `is_online`). Every other FullUserDto field comes from `User` + directly. Not a `From` impl because the second argument is + needed and Rust's `From` is single-arg. +- `SelfUserDto::build(full: FullUserDto, session_ctx: SessionContext)` + — helper taking a FullUserDto plus caller context (session's + DPoP-bound flag, admin-set force-password-change flag). Same + reason as FullUserDto's builder — not a `From` impl. + +**Repository layer** +(`src/infrastructure/repositories/pg/user_pg_repository.rs`): + +- **Delete `UserListEntry`** — the narrow projection was a perf + optimization; Path B decision supersedes it. +- `list_users` returns `Vec<(User, UserDerivedFlags)>` where + `UserDerivedFlags` is a small named struct in + `domain/repositories/user_repository.rs`: + + ```rust + /// DB-computed booleans about a user that aren't fields on the + /// `User` entity itself — either derived from column presence + /// (`password_hash IS NOT NULL`) or from a cross-table lookup + /// (`auth.sessions.last_seen_at` for `is_online`). Companion + /// to `User` on the list projection: the repo computes both, + /// the application layer packs them into `FullUserDto`. + pub struct UserDerivedFlags { + pub has_password: bool, + pub opaque_registered: bool, + pub opaque_migrated: bool, + pub is_online: bool, + } + ``` + + Not "admin-only" — every field ends up on `FullUserDto`, which + both admin AND self read. The name reflects "derived from the + DB row, not intrinsic to the User entity". +- SELECT widens to include `image` (previously narrowed away + per ROUND12 §Q1) + the `EXISTS(...)` scalar for `is_online`, + bound with `ONLINE_WINDOW.as_secs_f64()` via + `make_interval(secs => $N)` — same pattern as + `session_liveness_gauges.rs:104-114`, single source of truth, + no SQL literal. + +**Handler / service layer**: + +- `/api/auth/me` handler — builds `SelfUserDto` from + `(User, UserDerivedFlags, session_context)`. The + `UserDerivedFlags` for /me comes from a reuse of the + list-repo path scoped to `WHERE id = $me` or a new small + dedicated query (implementer's call — either works). +- `AuthResponseDto` shape follows — `user: SelfUserDto` field. +- Every login/refresh/OIDC/magic-link path that mints an + `AuthResponseDto` computes the same SelfUserDto. +- Admin service `list_users_admin` — returns `Vec`. +- `/api/users/{id}` handler — returns `PublicUserDto`. Every + other public consumer stays on `PublicUserDto`. + +## Frontend callsite inventory + +**Type changes** (`frontend/src/lib/api/types.ts`): + +- Rename `User` interface → `PublicUser` and slim to match new + DTO (9 fields). +- Add `FullUser` interface — `{ user: PublicUser, federation_kind, ... }`. +- Add `SelfUser` interface — `{ full: FullUser, ui_preferences, ... }`. +- Delete `AdminUser` (replaced by `FullUser`). + +**Store changes**: + +- `lib/stores/session.svelte.ts` — reads /me, must handle SelfUser + shape. Recommendation: keep a derived `session.me: SelfUser` for + the full record and shorthand accessors: + `session.user: PublicUser` = `session.me.full.user`, + `session.full: FullUser` = `session.me.full`. + Existing `session.user.username` calls keep working via the + shorthand; new self-only reads go through `session.me.foo` or + `session.full.foo`. + +**Component changes**: + +- Profile / change-password / DPoP-bind pages — reads + `session.me.has_password`, `session.me.is_dpop_bound`, + `session.me.can_edit_image`, `session.full.preferred_locale`, + etc. +- `routes/admin/[[tab]]/+page.svelte` users table — every + `u.username` → `u.user.username`, every `u.last_login_at` / + `u.active` / `u.has_password` stays top-level (FullUserDto + fields). Also **seed `resolveUser` cache with `u.user`** in the + load path — kills the N+1 that motivated widening the query. +- Admin sessions table's `UserVignette` — no change, `user_id` + passed through unchanged; the users-table cache seed above + satisfies the vignette lookup on cross-table navigation. +- `lib/composables/useOwnerCache.ts` / + `lib/api/endpoints/users.ts` — `resolveUser` returns + `PublicUser`. No signature change; the return shape only gets + smaller. Add a `seedUser(u: PublicUser)` export so the admin + table can prime the cache. + +## Phasing + +Each step compiles standalone; each is a reasonable review chunk. + +1. **Introduce the new DTOs** — add `PublicUserDto`, `FullUserDto`, + and `SelfUserDto` alongside the existing `UserDto`. Don't + change `UserDto` yet. Compiles; no behaviour change. +2. **Widen repo projection** — add `is_online` (via EXISTS + subquery) and `image` back to `list_users` SELECT. Introduce + `AdminExtras` struct. `UserListEntry` still exists but is now + redundant (fields also available on `User`). +3. **Migrate the emitter sites** — `/api/auth/me`, + login/refresh/OIDC/magic-link, admin service. Each now builds + the new nested shape. Old `UserDto` still ships every field. +4. **Rename `UserDto` → `PublicUserDto` and slim** — remove the + moved fields. The Rust compiler flags every remaining consumer + that reads a removed field; those either move to `.full.foo` / + `.user.foo` (embedded) or promote themselves to a Self/Full + DTO. +5. **Delete `UserListEntry` + `AdminUserSummaryDto`** — dead after + the cutover. +6. **Frontend** — rename types (`User` → `PublicUser`), add + `FullUser` / `SelfUser`, update session store, all consumers. + Seed `resolveUser` cache from admin table. +7. **Regenerate OpenAPI** — `cargo run --bin generate-openapi` + picks up the new schemas; the shrunken `PublicUserDto` schema + documents the new contract. +8. **Delete obsolete doc comments** — `has_password` / + `is_dpop_bound` / `force_password_change` comments on the old + UserDto explaining "populated only by `/me`" become obsolete + (the field structurally can't exist on non-self emitters). + +## Wire-shape breaking changes + +All in-repo consumers (backend + SPA) migrate in the same commit. +External consumers: none today — `/api/admin/users` is +admin-panel-only, `/me` is SPA-only, share/group endpoints are +SPA-only. Ship as one clean break; skip a `?shape=v2` deprecation +window. + +Every removed field from a public UserDto path (share responses, +group members, magic-link invitees, `/api/users/{id}`) is a +deliberate leak reduction, not a regression. Any FE consumer that +was reading e.g. `sharee.has_password` was reading a "quiet lie" +anyway (always `false`). + +## Testing + +**Backend**: + +- Round-trip tests for each new DTO type (already have for + UserDto; extend to PublicUserDto + FullUserDto + SelfUserDto). +- **Structural quarantine tests** — + `self_user_dto_does_not_leak_ui_preferences_via_public_paths`: + serialize a `SelfUserDto`, assert `ui_preferences` appears + ONLY at top level, not inside `.full.user` or `.full`. Same for + `FullUserDto` — `has_password` at top level of `FullUserDto`, + not inside `.user`. +- Update every service test that constructs `UserDto` fixtures. + +**Frontend**: + +- The TS type system catches every consumer that reads a removed + field. `npm run check` surfaces the whole blast radius on the + first pass — no new test infrastructure needed. +- Add one Vitest integration on the admin users table asserting + the presence dot renders AND `/api/users/{id}` is NOT called + per row (checks `apiFetch` mock call count). + +**Wire-shape guard**: + +- Hurl test hitting `/api/users/{id}` as a non-admin caller, + asserting the response does NOT contain moved fields + (`has_password`, `last_login_at`, `notify_on_share`, + `ui_preferences`, `storage_used_bytes`, `federation_kind`, + `preferred_locale`, `email_verified_at`, `created_at`, etc.). + Anti-regression guard for the whole point of this refactor. + +## Non-goals + +- **Reworking the `User` domain entity** — this refactor is + DTO-shape only. The entity keeps all its fields. +- **Visibility-rule changes on `/api/users/{id}`** — who can see + whom stays as-is; only the field set narrows. +- **Splitting other DTOs** — `SessionSummaryDto`, `FileDto`, etc. + Same principle would apply, but each is a separate design call. +- **Moving avatars out of the row** — planned separately. This + refactor keeps `image` on `PublicUserDto` so the admin-panel + N+1 fix survives. +- **Flattening the nested shape via `#[serde(flatten)]`** — the + three-level wire shape (`me.full.user.username`) is slightly + deeper than a flat DTO would be, but the structural quarantine + is worth the cost. Reconsider if consumer readability suffers. + +## Open questions + +1. **Wire nesting depth on `/me`**: `me.full.user.username` is 3 + levels. Acceptable? Alternative: `#[serde(flatten)]` on + FullUserDto and SelfUserDto so the wire is flat + (`me.username`, `me.has_password`, `me.ui_preferences` all + at top level), while keeping structural quarantine at compile + time only. Simpler for FE consumers, loses runtime + introspectability (a receiver can't tell which fields are + public vs full vs self from the shape). Recommendation: ship + nested; revisit if FE readability suffers. +2. **`created_at` / `updated_at` on FullUserDto** — not rendered + anywhere currently. Keep for compat unless there's a + compelling reason to drop. + +## Memory notes to update on landing + +- Extend `project_sessions_last_seen_at_shipped` with a "led to" + pointer at this refactor. +- New note `project_userdto_three_layer_split` — captures the + PublicUserDto / FullUserDto / SelfUserDto pattern + the + decision rule ("would any authenticated caller need this? + PublicUserDto. would self+admin? FullUserDto. self only? + SelfUserDto."). +- Delete the `AdminUserSummaryDto` and `UserListEntry` + references in earlier memory notes. diff --git a/src/application/dtos/user_dto.rs b/src/application/dtos/user_dto.rs index 9147fafa..3d88b344 100644 --- a/src/application/dtos/user_dto.rs +++ b/src/application/dtos/user_dto.rs @@ -288,6 +288,262 @@ impl From for UserDto { } } +// ──────────────────────────────────────────────────────────────────────── +// Three-layer user DTO family — see docs/plan/userdto-refactor.md. +// +// `PublicUserDto` — public identity. Every authenticated caller may see it. +// Returned by /api/users/{id}, share responses, group +// members, magic-link invitees, recipient enrichment. +// `FullUserDto` — `{ user: PublicUserDto, ...admin+self extras }`. +// Returned as `Vec` by /api/admin/users; +// embedded in `SelfUserDto`. Closest DTO to the +// `auth.users` row. +// `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. +// ──────────────────────────────────────────────────────────────────────── + +/// Public identity — what any authenticated caller may see about ANOTHER +/// user. Returned by `/api/users/{id}` and everywhere a user is +/// referenced by another surface (share responses, group members, +/// magic-link invitees, recipient enrichment). +/// +/// This is the audience-narrowest DTO: adding a field here means every +/// authenticated caller can see it about every visible user. Fields that +/// are meaningful only to the subject themselves (preferences, session +/// state) or only to an admin (auth adoption signals) belong on +/// [`SelfUserDto`] or [`FullUserDto`] respectively. +#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] +pub struct PublicUserDto { + pub id: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub username: Option, + pub email: String, + /// Role string ("admin" | "user"). Kept public because the sharee / + /// group-member vignette renders an admin badge. + pub role: String, + /// Avatar payload (base64 data-URI up to 512 KiB). Public so a share + /// picker can render the recipient's face directly. Will move to a + /// dedicated avatar endpoint in a future refactor — this shape is + /// transitional. + pub image: Option, + /// `true` for grant-only external recipients (magic-link, OIDC-only, + /// future OCM federated). Renders the "external" badge on the vignette. + pub is_external: bool, + /// Optional first/given name. Social identity. + #[serde(skip_serializing_if = "Option::is_none")] + pub given_name: Option, + /// Optional last/family name. Social identity. + #[serde(skip_serializing_if = "Option::is_none")] + pub family_name: 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). Sourced from an EXISTS subquery when the DTO is built + /// from a list-projection path; single-user endpoints that don't + /// enrich presence ship `false`. + #[serde(default)] + pub is_online: bool, +} + +/// Full user record — public identity + all fields BOTH an admin +/// (viewing another user) AND the subject themselves may see. Returned +/// as `Vec` by `/api/admin/users`; embedded in +/// [`SelfUserDto`] for `/api/auth/me`. +/// +/// This is the DTO closest to the underlying `auth.users` row. Adding a +/// field here means an admin looking at any user can see it, and the +/// subject themselves can see it in their `/me` response — but the field +/// stays off the public [`PublicUserDto`] surface. +#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] +pub struct FullUserDto { + /// Public identity — same set every authenticated caller sees. + pub user: PublicUserDto, + /// Which trust chain minted this user's federation identity — + /// `"oidc" | "ocm" | "magic_link"` — or `None` for pure local users. + /// Kept off `PublicUserDto` because a peer's federation kind is a + /// soft org-affiliation leak; only self + admin need it. + #[serde(skip_serializing_if = "Option::is_none")] + pub federation_kind: Option, + /// The authority that minted this user's `federation_subject` — + /// issuer URL for OIDC (id_token `iss` claim), peer domain for OCM, + /// `None` for local users. Same rationale as `federation_kind`. + #[serde(skip_serializing_if = "Option::is_none")] + pub federation_issuer: Option, + /// Subject's own locale preference. Only THEY or an admin managing + /// them needs this — other callers use their own locale. + #[serde(skip_serializing_if = "Option::is_none")] + pub preferred_locale: Option, + /// When the user first demonstrated control of their email. Trust + /// signal — meaningful to admin (auditing verification status) and + /// to self (own record), but not to a share picker rendering a + /// vignette. + #[serde(skip_serializing_if = "Option::is_none")] + pub email_verified_at: Option>, + /// Row bookkeeping. + pub created_at: DateTime, + pub updated_at: DateTime, + /// Activity signal — private to the subject; admin sees it too. + pub last_login_at: Option>, + /// Account-active flag — a deactivated user couldn't reach `/me` + /// anyway, but admin needs to see it. + pub active: bool, + /// Storage quotas — personal financials. Admin manages others'; + /// self sees own. + pub storage_quota_bytes: i64, + pub storage_used_bytes: i64, + /// TRUE when the account has a server-verifiable password + /// (`password_hash IS NOT NULL`). Kept off `PublicUserDto` because + /// per-user auth adoption leaks through directory endpoints. + pub has_password: bool, + /// TRUE when the user has an OPAQUE envelope on file. + pub opaque_registered: bool, + /// TRUE when the user has completed ≥1 successful OPAQUE login. + /// Distinct from `opaque_registered`: an admin can invalidate the + /// envelope leaving the user registered=false but with historical + /// migrated=true. + pub opaque_migrated: bool, +} + +/// Self view — everything the caller may see about themselves. +/// Returned by `/api/auth/me` and by every `AuthResponseDto` path +/// (login / refresh / OIDC callback / magic-link redemption). +/// +/// Composed on top of [`FullUserDto`] so `/me` and `/admin/users` share +/// the SAME "full profile" contract for the fields both need — new +/// self+admin-visible fields go on `FullUserDto` and both endpoints get +/// them together. Fields here are pure self-scoped state: preferences, +/// session-scoped flags, and caller-scoped permissions. +#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] +pub struct SelfUserDto { + /// Full profile — same shape as one row of `/api/admin/users`. + pub full: FullUserDto, + /// Opaque UI preferences bag — my own UI state. Cross-device store + /// for pure UI toggles (view mode, sidebar collapse, hide dotfiles, + /// …). The server never inspects the contents. Always present on + /// the wire; empty bag is `{}`, never `null`. + pub ui_preferences: serde_json::Value, + /// Whether I want share-notification emails. + pub notify_on_share: bool, + /// Session-scoped: my current session carries a DPoP thumbprint. + /// SPA reads this on `session.load()` to skip a redundant + /// `POST /api/auth/dpop/bind` when the session is already bound. + pub is_dpop_bound: bool, + /// Admin-set temp-password gate — SPA nav guard blocks everything + /// but `/change-password` until this flips back. Cleared by a + /// successful `POST /api/auth/change-password`. + pub force_password_change: bool, + /// Caller-scoped permission: can I edit my own avatar? `false` for + /// OIDC users whose avatar comes from the IdP. Only meaningful when + /// caller == subject; nonsense on any other DTO. + pub can_edit_image: bool, +} + +impl From for PublicUserDto { + fn from(user: User) -> Self { + let role = format!("{}", user.role()); + let p = user.into_parts(); + Self { + id: p.id.to_string(), + username: p.username, + email: p.email, + role, + image: p.image, + is_external: p.is_external, + given_name: p.given_name, + family_name: p.family_name, + // Single-user paths that don't enrich presence ship `false`. + // List projections (admin users, sharees enriched with + // presence) build via FullUserDto::build below, which + // overrides this from UserDerivedFlags. + is_online: false, + } + } +} + +impl FullUserDto { + /// Construct a `FullUserDto` from a `User` entity plus the DB-derived + /// flags the entity doesn't carry (`has_password`, OPAQUE flags, + /// `is_online`). Both are typically produced together by the users + /// list repo projection. + /// + /// Not a `From` impl because it takes two arguments; not a `From + /// <(User, UserDerivedFlags)>` because that reads awkwardly at + /// callsites — `FullUserDto::build(user, flags)` is clearer. + pub fn build( + user: User, + flags: crate::domain::repositories::user_repository::UserDerivedFlags, + ) -> Self { + let role = format!("{}", user.role()); + let p = user.into_parts(); + Self { + user: PublicUserDto { + id: p.id.to_string(), + username: p.username, + email: p.email, + role, + image: p.image, + is_external: p.is_external, + given_name: p.given_name, + family_name: p.family_name, + is_online: flags.is_online, + }, + federation_kind: p.federation_kind.map(|k| k.as_str().to_string()), + federation_issuer: p.federation_issuer, + preferred_locale: p.preferred_locale, + email_verified_at: p.email_verified_at, + created_at: p.created_at, + updated_at: p.updated_at, + last_login_at: p.last_login_at, + active: p.active, + storage_quota_bytes: p.storage_quota_bytes, + storage_used_bytes: p.storage_used_bytes, + has_password: flags.has_password, + opaque_registered: flags.opaque_registered, + opaque_migrated: flags.opaque_migrated, + } + } +} + +impl SelfUserDto { + /// Assemble the `/me` response from a `FullUserDto` plus the two + /// session-scoped booleans that can't be derived from `User` alone: + /// the caller's DPoP-binding state (from the JWT `cnf.jkt` claim) + /// and the admin-set force-password-change flag (from the auth + /// service's cache). + /// + /// The other self-only fields (`ui_preferences`, `notify_on_share`, + /// `can_edit_image`) come from `User` and are read off the entity + /// before it's moved into the FullUserDto; this method takes those + /// as explicit parameters so the caller can decide when to read + /// them (typically at the same point they read the DPoP-binding + /// state). + pub fn build( + full: FullUserDto, + ui_preferences: serde_json::Value, + notify_on_share: bool, + is_dpop_bound: bool, + force_password_change: bool, + can_edit_image: bool, + ) -> Self { + Self { + full, + ui_preferences, + notify_on_share, + is_dpop_bound, + force_password_change, + can_edit_image, + } + } +} + +// ──────────────────────────────────────────────────────────────────────── +// End of three-layer user DTO family. +// ──────────────────────────────────────────────────────────────────────── + #[derive(Debug, Serialize, Deserialize, Clone, ToSchema)] pub struct LoginDto { /// Identifier the user typed. Accepts BOTH a username (no `@`) and diff --git a/src/domain/repositories/user_repository.rs b/src/domain/repositories/user_repository.rs index 7bcb4148..3adaddc4 100644 --- a/src/domain/repositories/user_repository.rs +++ b/src/domain/repositories/user_repository.rs @@ -69,6 +69,51 @@ pub struct UserListEntry { /// 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`) +/// or from a cross-table lookup (`auth.sessions.last_seen_at` for +/// `is_online`). Companion to `User` on the list projection: the +/// repo computes both, the application layer packs them into +/// [`FullUserDto`](crate::application::dtos::user_dto::FullUserDto). +/// +/// Not "admin-only" — every field ends up on `FullUserDto`, which +/// 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). +#[derive(Debug, Clone, Copy)] +pub struct UserDerivedFlags { + pub has_password: bool, + pub opaque_registered: bool, + pub opaque_migrated: bool, + pub is_online: bool, } // Conversion from UserRepositoryError to DomainError diff --git a/src/infrastructure/repositories/pg/user_pg_repository.rs b/src/infrastructure/repositories/pg/user_pg_repository.rs index 6feb033e..06b0fd47 100644 --- a/src/infrastructure/repositories/pg/user_pg_repository.rs +++ b/src/infrastructure/repositories/pg/user_pg_repository.rs @@ -858,6 +858,8 @@ impl UserRepository for UserPgRepository { bool, bool, bool, + Option, + bool, ), >( // Auth-credential columns projected as booleans via `IS NOT @@ -871,6 +873,21 @@ impl UserRepository for UserPgRepository { // 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, @@ -879,7 +896,14 @@ impl UserRepository for UserPgRepository { 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 + (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 @@ -889,6 +913,7 @@ impl UserRepository for UserPgRepository { .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)?; @@ -911,6 +936,8 @@ impl UserRepository for UserPgRepository { has_password, opaque_registered, opaque_migrated, + image, + is_online, )| UserListEntry { id, username, @@ -930,6 +957,8 @@ impl UserRepository for UserPgRepository { has_password, opaque_registered, opaque_migrated, + image, + is_online, }, ) .collect())