diff --git a/docs/plan/userdto-refactor.md b/docs/plan/userdto-refactor.md new file mode 100644 index 00000000..fdf8de8e --- /dev/null +++ b/docs/plan/userdto-refactor.md @@ -0,0 +1,427 @@ +# UserDto Refactor — Three-Layer Split (Public / Full / Self) + +> **Status — SHIPPED 2026-08-21.** All eight phases landed and all gates +> pass: `cargo clippy --all-targets --all-features -D warnings` clean, +> `cargo fmt --check` clean, `cargo test three_layer_quarantine` (2/2 +> structural-quarantine tests pass), `npm run check` (593 files, 0 +> errors, 0 warnings), `npm run test:unit` (414 pass / 1 skipped / 0 +> failed), OpenAPI regenerated at `resources/gen/openapi.json`. See the +> [Phasing](#phasing) section below for the per-step outcome. The doc +> is retained as the reference for anyone extending the three-layer +> shape (new field → decide by audience per the rule in the opening +> section). + +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/frontend/src/lib/api/endpoints/admin.test.ts b/frontend/src/lib/api/endpoints/admin.test.ts index 5c4a6977..f234f36e 100644 --- a/frontend/src/lib/api/endpoints/admin.test.ts +++ b/frontend/src/lib/api/endpoints/admin.test.ts @@ -64,10 +64,7 @@ describe('admin mutate-based endpoints', () => { describe('admin read endpoints', () => { it('call apiJson for the listing/settings reads', async () => { await admin.listUsers(25, 0); - expect(jsonMock).toHaveBeenCalledWith( - '/api/admin/users?limit=25&offset=0&summary=true', - expect.anything() - ); + expect(jsonMock).toHaveBeenCalledWith('/api/admin/users?limit=25&offset=0', expect.anything()); await admin.getDashboard(); await admin.getSmtpInfo(); await admin.getOidcSettings(); diff --git a/frontend/src/lib/api/endpoints/admin.ts b/frontend/src/lib/api/endpoints/admin.ts index 2cf9ee48..9d1eae3d 100644 --- a/frontend/src/lib/api/endpoints/admin.ts +++ b/frontend/src/lib/api/endpoints/admin.ts @@ -12,7 +12,7 @@ import type { DriveMember, DriveMemberSubject, DriveRole, - User + FullUser } from '$lib/api/types'; const JSON_HEADERS = { 'Content-Type': 'application/json' }; @@ -277,19 +277,24 @@ export function revokeAdminSession(sessionId: string): Promise { // ── Users ─────────────────────────────────────────────────────────────── -/** List the compact rows rendered by the management table; full account - * details remain available through {@link getUserAdmin}. */ +/** List admin users — always returns `FullUser` rows. The former + * `?summary` toggle is retired; a single canonical shape carries + * the vignette + admin-visible extras the table needs. Single-user + * details still available via {@link getUserAdmin}. */ export function listUsers(limit: number, offset: number): Promise { - return apiJson(`/api/admin/users?limit=${limit}&offset=${offset}&summary=true`, { + return apiJson(`/api/admin/users?limit=${limit}&offset=${offset}`, { credentials: 'same-origin' }); } /** * Admin-scoped single-user lookup — `GET /api/admin/users/{id}`. - * Returns the full `User` DTO including `storage_quota_bytes` + - * `storage_used_bytes` which the non-admin `/api/users/{id}` - * response omits for privacy. + * Returns the full `FullUser` DTO (public identity in `.user` + + * admin-visible extras like `email_verified_at` / `has_password` / + * `opaque_registered` / `last_login_at` / quotas at top level) — + * same shape as one row of `/api/admin/users` list. The peer-view + * `/api/users/{id}` returns the slim `PublicUser` which omits those + * admin-only signals. * * Result promises are cached per id at module scope so multiple * callers for the same user (e.g. the admin drives table with N @@ -301,14 +306,14 @@ export function listUsers(limit: number, offset: number): Promise>(); +const adminUserCache = new Map>(); -export function getUserAdmin(id: string): Promise { +export function getUserAdmin(id: string): Promise { const hit = adminUserCache.get(id); if (hit) return hit; - const pending = (async (): Promise => { + const pending = (async (): Promise => { try { - return await apiJson(`/api/admin/users/${encodeURIComponent(id)}`, { + return await apiJson(`/api/admin/users/${encodeURIComponent(id)}`, { credentials: 'same-origin' }); } catch { @@ -387,9 +392,30 @@ export interface DriveKindUsage { } export interface AdminDashboard { + // ── User accounts (static breakdown of auth.users) ── + // All four are counts of the same table under different + // predicates. Rendered as one grouped section on the dashboard. total_users: number; active_users: number; admin_users: number; + /** Grant-only accounts (magic-link / OIDC-only / OCM recipients). + * Filtered out of `total_users` / `active_users` — those count + * operational seats. Surfaced here as its own metric because + * external-heavy deployments (public-share collab, invited-only + * shops) need the invited population at a glance. */ + external_users: number; + // ── Live activity (projection over auth.sessions) ── + // Both change minute-to-minute — a whole different cadence from + // the account counts above. Rendered as a separate section on + // the dashboard with the presence-dot visual cue. + /** Distinct users behind non-revoked sessions active in the last + * 5 min. Same 5-min window as the `oxicloud_sessions_online_users` + * Prometheus gauge; single source of truth on the backend. */ + online_users: number; + /** Non-revoked sessions active in the last 5 min. Ratio + * `online_sessions / online_users` is the multi-device factor + * (browser + desktop + phone). */ + online_sessions: number; server_version: string; drive_usage: DriveKindUsage[]; auth_enabled: boolean; diff --git a/frontend/src/lib/api/endpoints/auth.ts b/frontend/src/lib/api/endpoints/auth.ts index 1e8fff43..b6685dd6 100644 --- a/frontend/src/lib/api/endpoints/auth.ts +++ b/frontend/src/lib/api/endpoints/auth.ts @@ -5,7 +5,7 @@ */ import { ApiError, apiFetch } from '$lib/api/client'; import { getCsrfHeaders } from '$lib/api/csrf'; -import type { AuthResponse, User } from '$lib/api/types'; +import type { AuthResponse, SelfUser } from '$lib/api/types'; /** * Best-effort parse of the backend `ErrorResponse` shape @@ -45,7 +45,7 @@ const JSON_HEADERS = { 'Content-Type': 'application/json' }; * Failure to build a proof (no keypair, missing WebCrypto) falls back to a * headerless request — the server still accepts it for unbound sessions. */ -export async function fetchMe(): Promise { +export async function fetchMe(): Promise { // Build + sign a DPoP proof, send with the header, harvest any // `DPoP-Nonce` off the response into the shared client cache // (so the NEXT apiFetch call reuses it — no wasted round trip). @@ -80,7 +80,7 @@ export async function fetchMe(): Promise { if (dpopMod && dpopMod.isDpopNonceChallenge(res)) res = await send(); if (res.status === 401) return null; if (!res.ok) throw new Error(`/api/auth/me failed: ${res.status}`); - return (await res.json()) as User; + return (await res.json()) as SelfUser; } /** @@ -408,7 +408,7 @@ export async function setupAdmin(email: string, password: string): Promise * failure, not an expired access token. Returns the user on success, null on * any failure so the caller can fall through to the normal login UI. */ -export async function exchangeOidcCode(code: string): Promise { +export async function exchangeOidcCode(code: string): Promise { try { const res = await fetch('/api/auth/oidc/exchange', { method: 'POST', @@ -417,7 +417,7 @@ export async function exchangeOidcCode(code: string): Promise { body: JSON.stringify({ code }) }); if (!res.ok) return null; - const data = (await res.json()) as { user?: User }; + const data = (await res.json()) as { user?: SelfUser }; return data.user ?? null; } catch { return null; @@ -463,7 +463,7 @@ export async function register(email: string, password?: string, username?: stri * authenticated; a 401 here IS a genuine "session expired" and the * refresh interceptor is the right response. */ -export async function upgradeToInternal(password?: string): Promise { +export async function upgradeToInternal(password?: string): Promise { const body: Record = {}; if (password) body.password = password; const res = await apiFetch('/api/auth/upgrade-to-internal', { @@ -482,7 +482,7 @@ export async function upgradeToInternal(password?: string): Promise { message ); } - return (await res.json()) as User; + return (await res.json()) as SelfUser; } export type MagicLinkResult = 'sent' | 'unavailable'; diff --git a/frontend/src/lib/api/endpoints/profile.ts b/frontend/src/lib/api/endpoints/profile.ts index a172cc3f..6305b215 100644 --- a/frontend/src/lib/api/endpoints/profile.ts +++ b/frontend/src/lib/api/endpoints/profile.ts @@ -1,7 +1,7 @@ /** Profile / account endpoints — ported from views/profile/profile.js. */ import { apiFetch } from '$lib/api/client'; import { getCsrfHeaders } from '$lib/api/csrf'; -import type { User } from '$lib/api/types'; +import type { SelfUser } from '$lib/api/types'; import { t } from '$lib/i18n/index.svelte'; const JSON_HEADERS = { 'Content-Type': 'application/json' }; @@ -24,7 +24,7 @@ export interface ProfilePatch { ui_preferences?: Record; } -export async function updateProfile(patch: ProfilePatch): Promise { +export async function updateProfile(patch: ProfilePatch): Promise { const res = await apiFetch('/api/auth/me/profile', { method: 'PATCH', credentials: 'same-origin', @@ -57,7 +57,7 @@ export async function updateProfile(patch: ProfilePatch): Promise { } throw new Error(err.message || err.error || `profile update failed: ${res.status}`); } - return (await res.json()) as User; + return (await res.json()) as SelfUser; } export async function changePassword(currentPw: string, newPw: string): Promise { diff --git a/frontend/src/lib/api/endpoints/users.ts b/frontend/src/lib/api/endpoints/users.ts index ff9b9be7..06e71b28 100644 --- a/frontend/src/lib/api/endpoints/users.ts +++ b/frontend/src/lib/api/endpoints/users.ts @@ -16,15 +16,23 @@ export interface ResolvedUser { email: string; image: string | null; isExternal: boolean; + /** Presence — TRUE when the server observed a request on any of this + * user's non-revoked sessions within the last 5 min (backend + * `PublicUserDto.is_online`). Drives the presence dot overlay on + * `` / ``. `false` when the caller's + * source didn't compute presence (a bare `resolveUser(id)` from + * pre-3-layer callers, an older backend build) — dot stays dark. */ + isOnline: boolean; } -/** Subset of the backend `UserDto` we consume here. */ -interface UserDtoShape { +/** Subset of the backend `PublicUserDto` we consume here. */ +interface PublicUserShape { id: string; username?: string | null; email?: string | null; image?: string | null; is_external: boolean; + is_online?: boolean; } // id → in-flight/resolved lookup (the Promise is cached so concurrent callers @@ -41,13 +49,14 @@ export function resolveUser(id: string): Promise { credentials: 'same-origin' }); if (!res.ok) return null; - const u = (await res.json()) as UserDtoShape; + const u = (await res.json()) as PublicUserShape; return { id: u.id, name: u.username?.trim() || u.email || u.id, email: u.email ?? '', image: u.image ?? null, - isExternal: u.is_external + isExternal: u.is_external, + isOnline: u.is_online ?? false }; } catch { return null; @@ -57,3 +66,37 @@ export function resolveUser(id: string): Promise { cache.set(id, pending); return pending; } + +/** + * Prime the resolver cache from data the caller already has in hand. + * When a list endpoint (e.g. `/api/admin/users`) ships full + * `PublicUser` rows, the admin page seeds this cache in its load path + * so every subsequent `resolveUser(id)` call (from `UserVignette` + * mounted per-row) hits the cache synchronously — no per-row + * `/api/users/{id}` follow-up fetch. Kills the N+1 that motivated + * widening `/api/admin/users` to include the avatar (see + * `docs/plan/userdto-refactor.md` § N+1). + * + * No-op when the id is already cached (in-flight or resolved). This + * makes seeding safe to call unconditionally — never clobbers an + * authoritative in-flight lookup with a stale seed. + */ +export function seedUser(u: { + id: string; + username?: string | null; + email: string; + image?: string | null; + is_external: boolean; + is_online?: boolean; +}): void { + if (cache.has(u.id)) return; + const resolved: ResolvedUser = { + id: u.id, + name: u.username?.trim() || u.email || u.id, + email: u.email, + image: u.image ?? null, + isExternal: u.is_external, + isOnline: u.is_online ?? false + }; + cache.set(u.id, Promise.resolve(resolved)); +} diff --git a/frontend/src/lib/api/types.ts b/frontend/src/lib/api/types.ts index c99f2909..9c967495 100644 --- a/frontend/src/lib/api/types.ts +++ b/frontend/src/lib/api/types.ts @@ -179,148 +179,116 @@ export interface TrashResourcesResponse { export type Role = 'user' | 'admin'; -/** Wire shape of `UserDto` (backend: src/application/dtos/user_dto.rs). */ -export interface User { +// ───────────────────────────────────────────────────────────────────────── +// Three-layer user family — mirrors src/application/dtos/user_dto.rs. +// See docs/plan/userdto-refactor.md. +// +// `PublicUser` — public identity. Every authenticated caller may see it. +// Returned by /api/users/{id}, share responses, group +// members, magic-link invitees, recipient enrichment. +// `FullUser` — `{ user: PublicUser, ...admin+self extras }`. Returned +// as Vec by /api/admin/users; embedded in `SelfUser`. +// `SelfUser` — `{ full: FullUser, ...self-only extras }`. Returned by +// /api/auth/me and by every auth response. +// +// Adding a field? Decide by audience: +// * Any authenticated caller may see it about another user → PublicUser. +// * Only admin (about another user) AND self (about self) → FullUser. +// * Only self about themselves → SelfUser. +// ───────────────────────────────────────────────────────────────────────── + +/** Public identity — 9 fields visible to any authenticated caller. */ +export interface PublicUser { id: string; username?: string; email: string; role: string; - storage_quota_bytes: number; - storage_used_bytes: number; + image?: string | null; + is_external: boolean; + given_name?: string; + family_name?: string; + /** Presence — TRUE when the server observed a request on any of this + * user's non-revoked sessions within the last 5 min. Populated on + * list endpoints; single-user public paths default to `false`. + * Backwards-compat: missing on older backend builds → `false`. */ + is_online?: boolean; +} + +/** 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 `SelfUser` for `/api/auth/me`. */ +export interface FullUser { + user: PublicUser; + /** IdP linkage. Load-bearing "is federated?" predicate: + * `full.federation_kind === 'oidc'`. */ + federation_kind?: 'oidc' | 'ocm' | 'magic_link'; + /** Authority that minted the OIDC/OCM identity — issuer URL for OIDC, + * peer domain for OCM. FE that wants a friendly label maps this + * against `OidcProviders.issuer → provider_name`. */ + federation_issuer?: string; + preferred_locale?: string; + email_verified_at?: string; created_at: string; updated_at: string; last_login_at?: string | null; active: boolean; - /** - * Which trust chain minted this user's federation identity. `null` - * (omitted from wire) for local users (password / OPAQUE only). - * `"oidc" | "ocm" | "magic_link"` for federated users. Predicate: - * `!user.federation_kind` = local; `user.federation_kind === 'oidc'` - * = OIDC user. Mirrors `auth.users.federation_kind` verbatim. - */ - federation_kind?: 'oidc' | 'ocm' | 'magic_link'; - /** - * Authority that minted this user's OIDC/OCM identity — issuer URL - * for OIDC (id_token `iss`), peer domain for OCM. `null` (omitted) - * for local users. FE that wants a friendly display label maps this - * against `OidcProviders.issuer → provider_name` when they match; - * shows the raw value otherwise. Renamed from the historical - * `auth_provider` (which held a display label pre-Phase-B and a - * `"local"` sentinel for non-federated users — both are gone). - */ - federation_issuer?: string; - image?: string | null; - can_edit_image: boolean; - is_external: boolean; - given_name?: string; - family_name?: string; - email_verified_at?: string; - preferred_locale?: string; - notify_on_share: boolean; - /** - * Opaque UI preferences bag. Server-side JSONB column that persists - * pure UI toggles (hide-dotfiles, view mode, sidebar collapse, …) - * across devices. The server never inspects the contents — the SPA - * defines the keys (see `lib/stores/preferences.svelte.ts` for the - * typed view). Always an object on the wire (empty bag is `{}`, - * never `null` or missing). - * - * When PATCHing back to the server via - * `PATCH /api/auth/me/profile { ui_preferences: {...} }`, the - * server SHALLOW-merges — only the keys present in the patch are - * touched, so partial writes from one device don't clobber - * preferences set on another. Set a key to `null` in the patch to - * delete it from the bag. - */ - ui_preferences: Record; - /** - * Mirrors `auth.users.force_password_change_at_next_login`. Only - * populated by `GET /api/auth/me` (see the backend UserDto doc for - * why other UserDto call-sites default to false). When true, the - * SPA MUST lock navigation to the password-change surface — the - * root layout's guard + the backend's `require_no_password_change_pending` - * middleware together enforce this. Optional on the wire because - * older backend builds omit it and `#[serde(default)]` maps - * missing → `false`. - */ - force_password_change?: boolean; - /** - * 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 - * (hybrid posture — SSO for daily login, local password as - * fallback). The profile page's change-password card gates on this - * flag rather than on the federation shape so hybrid users can - * rotate their local credential. Optional on the wire for older- - * backend compatibility; missing → `false` (safe default: hide the - * card). - */ - has_password?: boolean; - /** - * TRUE when the caller's current session is DPoP-bound (row's - * `dpop_jkt IS NOT NULL`). Populated only by `/api/auth/me`; other - * User-emitting endpoints leave it unset. - * - * The session store reads this to skip a redundant - * `POST /api/auth/dpop/bind` call — the endpoint returns 409 - * `already_bound` on repeated attempts (anti-downgrade invariant) - * and each rejection logs at audit INFO, so a naive "bind on - * every load" pattern was cluttering the audit stream. We only - * fire bind now when there's actual work to do (fresh OIDC / - * magic-link session that landed unbound). - */ - is_dpop_bound?: boolean; + storage_quota_bytes: number; + storage_used_bytes: number; + /** TRUE when the account has a local Argon2id `password_hash` on file. + * Distinct from `federation_kind`: an OIDC-linked account can ALSO + * carry a local password (hybrid). */ + has_password: boolean; + /** TRUE when the user has an OPAQUE envelope on file. Admin-visible + * rollout signal — kept off `PublicUser` so directory endpoints don't + * leak OPAQUE adoption. */ + opaque_registered: boolean; + /** TRUE when the user has completed ≥1 OPAQUE login. Distinct from + * `opaque_registered` — envelope-on-file vs successful-login. */ + opaque_migrated: boolean; } -/** Fields rendered by the paginated admin table. Full account details remain - * available from the detail endpoint; this shape keeps avatars and preference - * documents off every listing page. - * - * The two OPAQUE flags below are ADMIN-ONLY signals: they surface per-user - * OPAQUE rollout progress in the admin table. The backend deliberately keeps - * them off `UserDto` (`/api/auth/me`, share-recipient DTOs, group members) - * so a non-admin can't enumerate the adoption set through third-party - * endpoints. Both optional on the wire — older backend builds omit them and - * `#[serde(default)]` maps missing → `false`. */ -export type AdminUserSummary = Pick< - User, - | 'id' - | 'username' - | 'email' - | 'role' - | 'storage_quota_bytes' - | 'storage_used_bytes' - | 'last_login_at' - | 'active' - | 'federation_kind' - | 'federation_issuer' - | 'is_external' -> & { - /** TRUE = user has a server-verifiable password on file (legacy or - * admin-set). Combined with `opaque_registered` and `federation_kind`, - * the admin table derives the full auth capability set — a user with - * `has_password=false`, `opaque_registered=false` AND - * `federation_kind === undefined` (no federation) is passwordless - * (magic-link only, which is the default for externals). */ - has_password?: boolean; - /** TRUE = user has an OPAQUE envelope on file (Phase 2 silent migration - * succeeded, or the user completed a manual re-registration). */ - opaque_registered?: boolean; - /** TRUE = user has completed at least one successful OPAQUE login. - * Distinct from `opaque_registered` — the envelope may have been - * cleared by an admin reset while a stale migrated=true remains as - * historical signal (backend clears both atomically today, but the - * two-flag shape keeps the option open for a future policy split). */ - opaque_migrated?: boolean; -}; +/** Self view — everything the caller may see about themselves. + * Returned by `/api/auth/me` and every `AuthResponse` (login / refresh / + * OIDC callback / magic-link redemption ships this so the SPA's post-auth + * state matches its post-`/me` state with no UI race). */ +export interface SelfUser { + full: FullUser; + /** Opaque UI-preferences bag. Cross-device store for pure UI toggles + * (view mode, sidebar collapse, hide-dotfiles, …). Server never + * inspects contents; the SPA defines the keys (see + * `lib/stores/preferences.svelte.ts`). Always an object on the wire + * — empty bag is `{}`, never `null`. PATCH via `/api/auth/me/profile` + * shallow-merges; setting a key to `null` removes it. */ + ui_preferences: Record; + /** Whether the user wants share-notification emails. */ + notify_on_share: boolean; + /** Session-scoped: my current session is DPoP-bound. SPA reads this + * on `session.load()` to skip a redundant `/api/auth/dpop/bind` call + * (409 `already_bound` otherwise, noisy in the audit stream). */ + is_dpop_bound: boolean; + /** 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`. */ + force_password_change: boolean; + /** Caller-scoped: 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. */ + can_edit_image: boolean; +} + +/** Backwards-compat alias while migrating call-sites. Prefer `PublicUser` + * for public-identity contexts (sharee, group member, invitee) or + * `SelfUser` when reading `/api/auth/me`. Delete once no consumers reference + * the bare `User` name. */ +export type User = PublicUser; export interface AdminUsersPage { total: number; - users: AdminUserSummary[]; + users: FullUser[]; } export interface AuthResponse { - user: User; + user: SelfUser; access_token: string; refresh_token: string; token_type: string; diff --git a/frontend/src/lib/components/AppShell.svelte b/frontend/src/lib/components/AppShell.svelte index 595b5637..74f21ad1 100644 --- a/frontend/src/lib/components/AppShell.svelte +++ b/frontend/src/lib/components/AppShell.svelte @@ -437,11 +437,11 @@ { mode: 'dark', icon: 'moon', label: t('user_menu.theme.dark', 'Dark') } ]; - const storagePct = $derived( - session.user && session.user.storage_quota_bytes > 0 - ? Math.min(100, (session.user.storage_used_bytes / session.user.storage_quota_bytes) * 100) - : 0 - ); + const storagePct = $derived.by(() => { + const full = session.me?.full; + if (!full || full.storage_quota_bytes <= 0) return 0; + return Math.min(100, (full.storage_used_bytes / full.storage_quota_bytes) * 100); + }); const initials = $derived(userInitials(session.user?.username || session.user?.email)); @@ -655,12 +655,12 @@
- {#if session.user.storage_quota_bytes > 0} - {Math.round(storagePct)}% · {formatBytes(session.user.storage_used_bytes)} / {formatBytes( - session.user.storage_quota_bytes + {#if (session.me?.full.storage_quota_bytes ?? 0) > 0} + {Math.round(storagePct)}% · {formatBytes(session.me?.full.storage_used_bytes ?? 0)} / {formatBytes( + session.me?.full.storage_quota_bytes ?? 0 )} {:else} - {formatBytes(session.user.storage_used_bytes)} + {formatBytes(session.me?.full.storage_used_bytes ?? 0)} {/if}
@@ -903,18 +903,18 @@
- {#if session.user.storage_quota_bytes > 0} + {#if (session.me?.full.storage_quota_bytes ?? 0) > 0} {t( 'storage.used', { percentage: Math.round(storagePct), - used: formatBytes(session.user.storage_used_bytes), - total: formatBytes(session.user.storage_quota_bytes) + used: formatBytes(session.me?.full.storage_used_bytes ?? 0), + total: formatBytes(session.me?.full.storage_quota_bytes ?? 0) }, '{{percentage}}% used ({{used}} / {{total}})' )} {:else} - {formatBytes(session.user.storage_used_bytes)} + {formatBytes(session.me?.full.storage_used_bytes ?? 0)} {/if}
diff --git a/frontend/src/lib/components/AppShell.test.ts b/frontend/src/lib/components/AppShell.test.ts index 1439ecff..04089694 100644 --- a/frontend/src/lib/components/AppShell.test.ts +++ b/frontend/src/lib/components/AppShell.test.ts @@ -26,16 +26,27 @@ const children = createRawSnippet(() => ({ beforeEach(() => { vi.clearAllMocks(); pageState.url = new URL('http://localhost/files'); - session.user = { - id: '1', - username: 'admin', - email: 'a@x.test', - given_name: 'A', - family_name: 'B', - role: 'admin', - storage_used_bytes: 10, - storage_quota_bytes: 100, - is_external: false + // Post the three-layer UserDto refactor, `session.user` is a + // derived accessor over `session.me.full.user`; only `session.me` + // is settable. Fixture composes the nested shape — public identity + // (username/email/name) on `.full.user`, admin+self extras + // (storage_*, has_password) on `.full`, self-only bag (ui_prefs, + // dpop_bound, force_password_change, can_edit_image) at the top. + // See docs/plan/userdto-refactor.md. + session.me = { + full: { + user: { + id: '1', + username: 'admin', + email: 'a@x.test', + given_name: 'A', + family_name: 'B', + role: 'admin', + is_external: false + }, + storage_used_bytes: 10, + storage_quota_bytes: 100 + } } as never; }); diff --git a/frontend/src/lib/components/EmptyState.svelte b/frontend/src/lib/components/EmptyState.svelte index 4403103f..1e0c606b 100644 --- a/frontend/src/lib/components/EmptyState.svelte +++ b/frontend/src/lib/components/EmptyState.svelte @@ -18,7 +18,19 @@ let { icon, title, hint, error = false, children }: Props = $props(); -
+ +
{#if icon}{/if} {#if title}

{title}

{/if} {#if hint}

{hint}

{/if} diff --git a/frontend/src/lib/components/UserVignette.svelte b/frontend/src/lib/components/UserVignette.svelte index a1c0fe12..57cd999e 100644 --- a/frontend/src/lib/components/UserVignette.svelte +++ b/frontend/src/lib/components/UserVignette.svelte @@ -33,6 +33,7 @@ const label = $derived(resolved?.name ?? fallbackLabel ?? userId); const email = $derived(resolved?.email || fallbackSublabel || ''); const isExternal = $derived(resolved?.isExternal ?? false); + const isOnline = $derived(resolved?.isOnline ?? false); const image = $derived(resolved?.image ?? null); const colorIndex = $derived(avatarColorIndex(userId)); const initials = $derived(userInitials(label)); @@ -50,6 +51,22 @@ {/if} + {#if isOnline} + + + {/if} {label} @@ -128,6 +145,27 @@ font-size: 9px; } + /* Presence dot — top-right, symmetric with `.uv__badge` at + bottom-right so the two corners don't collide. Slightly smaller + (10x10 vs the badge's 16x16) because it's a pure signal — no + icon, no text. The 2px `--color-bg-surface` border creates a + visual gap between dot and avatar so the green pops out cleanly + regardless of avatar palette (photo, dark initials, light + initials). `box-sizing: border-box` keeps the inner circle's + green footprint at 6x6 — same visual weight the sessions-table + dot has. See `docs/plan/sessions.md` § UI. */ + .uv__presence { + position: absolute; + right: -2px; + top: -2px; + width: 10px; + height: 10px; + border-radius: 50%; + background: var(--color-success-alt); + border: 2px solid var(--color-bg-surface); + box-sizing: border-box; + } + .uv__text { display: flex; flex-direction: column; diff --git a/frontend/src/lib/stores/preferences.svelte.ts b/frontend/src/lib/stores/preferences.svelte.ts index e9e141d0..a905da65 100644 --- a/frontend/src/lib/stores/preferences.svelte.ts +++ b/frontend/src/lib/stores/preferences.svelte.ts @@ -69,12 +69,15 @@ const PATCH_DEBOUNCE_MS = 500; class PreferencesStore { /** - * The typed view of the bag. Derived from `session.user?.ui_preferences` - * so signing in / out / refresh flips it in lockstep with the session. + * The typed view of the bag. Derived from `session.me?.ui_preferences` + * (moved from public `User.ui_preferences` to `SelfUser.ui_preferences` + * as part of the three-layer UserDto refactor — the bag is self-only + * state, not something other authenticated callers should see). + * Signing in / out / refresh flips it in lockstep with the session. * Reads pass through DEFAULTS for any missing key. */ private bag = $derived>( - (session.user?.ui_preferences as Record | undefined) ?? {} + (session.me?.ui_preferences as Record | undefined) ?? {} ); // ── Typed accessors ────────────────────────────────────────── @@ -100,11 +103,14 @@ class PreferencesStore { * `jsonb_strip_nulls` after the merge). */ set(patch: Partial>): void { - if (!session.user) return; + if (!session.me) return; - // Optimistic local write — mutate the reactive user shallowly. + // Optimistic local write — mutate the reactive me shallowly. + // `ui_preferences` lives on `SelfUser` (self-only), not on the + // public `User` slice, so the mutation stays at the SelfUser + // level. The nested `full` / `full.user` blocks are untouched. const nextBag = { - ...((session.user.ui_preferences as Record | undefined) ?? {}), + ...((session.me.ui_preferences as Record | undefined) ?? {}), ...patch }; // Strip any explicit-null locally so the derived getters see the @@ -114,7 +120,7 @@ class PreferencesStore { for (const [k, v] of Object.entries(patch)) { if (v === null) delete (nextBag as Record)[k]; } - session.user = { ...session.user, ui_preferences: nextBag }; + session.me = { ...session.me, ui_preferences: nextBag }; // Accumulate keys so successive `set` calls before the debounce // fires collapse into a single PATCH body — matters for @@ -131,16 +137,20 @@ class PreferencesStore { this.pendingPatch = {}; if (Object.keys(patch).length === 0) return; - const previousUser = session.user; + // `session.user` is a derived read-through on `session.me.full.user` + // — the source of truth is `session.me: SelfUser`. Snapshot + assign + // there so the optimistic update / rollback matches the store shape + // (see `docs/plan/userdto-refactor.md` for the layering). + const previousMe = session.me; try { const updated = await updateProfile({ ui_preferences: patch }); - session.user = updated; + session.me = updated; } catch { // Roll back to whatever the server last confirmed. The // optimistic local mutation is discarded and the derived // `hideDotfiles` / other getters snap back on the next // reactivity tick. - session.user = previousUser; + session.me = previousMe; ui.notify( t('preferences.save_failed', "Couldn't save your preference. Please try again."), 'error' diff --git a/frontend/src/lib/stores/session.svelte.test.ts b/frontend/src/lib/stores/session.svelte.test.ts index 20325726..85450069 100644 --- a/frontend/src/lib/stores/session.svelte.test.ts +++ b/frontend/src/lib/stores/session.svelte.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import type { User } from '$lib/api/types'; +import type { SelfUser } from '$lib/api/types'; // `vi.mock` is hoisted above imports, so the spy it references must be created // with `vi.hoisted` (a plain top-level const isn't initialised yet when the @@ -14,7 +14,14 @@ vi.mock('$lib/api/endpoints/auth', () => ({ import { session } from './session.svelte'; -const userWithUsage = (used: number) => ({ storage_used_bytes: used }) as unknown as User; +// `storage_used_bytes` moved to `FullUser` (embedded inside `SelfUser`) +// as part of the three-layer UserDto refactor +// (`docs/plan/userdto-refactor.md`). Build a minimal SelfUser shape that +// satisfies the type checker without hand-populating every field the +// production shape carries — the test only cares about the usage read +// path (`session.me.full.storage_used_bytes`). +const userWithUsage = (used: number) => + ({ full: { storage_used_bytes: used } }) as unknown as SelfUser; describe('session.refresh', () => { beforeEach(() => { @@ -25,7 +32,7 @@ describe('session.refresh', () => { it('pulls the fresh storage usage into the reactive user (upload/delete sync)', async () => { fetchMeMock.mockResolvedValue(userWithUsage(2048)); await session.refresh(); - expect(session.user?.storage_used_bytes).toBe(2048); + expect(session.me?.full.storage_used_bytes).toBe(2048); }); it('leaves the current user intact when the probe returns null', async () => { @@ -33,7 +40,7 @@ describe('session.refresh', () => { await session.refresh(); fetchMeMock.mockResolvedValue(null); await session.refresh(); - expect(session.user?.storage_used_bytes).toBe(2048); + expect(session.me?.full.storage_used_bytes).toBe(2048); }); it('leaves the current user intact when the probe throws', async () => { @@ -41,6 +48,6 @@ describe('session.refresh', () => { await session.refresh(); fetchMeMock.mockRejectedValue(new Error('network')); await session.refresh(); - expect(session.user?.storage_used_bytes).toBe(2048); + expect(session.me?.full.storage_used_bytes).toBe(2048); }); }); diff --git a/frontend/src/lib/stores/session.svelte.ts b/frontend/src/lib/stores/session.svelte.ts index bc2022c1..a46e1e4d 100644 --- a/frontend/src/lib/stores/session.svelte.ts +++ b/frontend/src/lib/stores/session.svelte.ts @@ -11,17 +11,39 @@ import { setLogoutInProgress } from '$lib/api/client'; import { hasSessionHint } from '$lib/api/csrf'; import { seedNonceFromCookie } from '$lib/auth/dpop-proof'; import { drives } from '$lib/stores/drives.svelte'; -import type { User } from '$lib/api/types'; +import type { PublicUser, SelfUser } from '$lib/api/types'; import { ensureActiveUser } from '$lib/utils/localStoragePrefs'; +/** + * Session store — the authenticated user and derived flags. + * + * Post the three-layer UserDto refactor (`docs/plan/userdto-refactor.md`), + * `/api/auth/me` returns `SelfUser` (composed: + * `SelfUser.full.user: PublicUser`). Two shorthand accessors keep every + * existing consumer readable: + * + * - `session.user` → `PublicUser` (via `me.full.user`). Every callsite + * that read `session.user.username / email / id / role / image / + * is_external / given_name / family_name / is_online` keeps working. + * - `session.me` → full `SelfUser`. New code that needs self-only or + * admin-visible fields (`has_password`, `is_dpop_bound`, `active`, + * `ui_preferences`, `federation_kind`, `last_login_at`, quotas, …) + * reads through `session.me.full.foo` or `session.me.foo`. + */ class SessionStore { - user = $state(null); + /** Full `/api/auth/me` payload. Null when unauthenticated. */ + me = $state(null); loaded = $state(false); homeFolderId = $state(null); homeFolderName = $state(null); - isExternalUser = $derived(this.user?.is_external ?? false); - isAuthenticated = $derived(this.user !== null); + /** Public-identity shorthand — same fields any authenticated caller + * can see. Every legacy `session.user.foo` read (username, email, id, + * role, image, is_external, given_name, family_name, is_online) still + * works via this derived accessor. */ + user = $derived(this.me?.full.user ?? null); + isExternalUser = $derived(this.me?.full.user.is_external ?? false); + isAuthenticated = $derived(this.me !== null); /** * TRUE when the backend has set `force_password_change_at_next_login` * on this account — an admin picked a temporary password and the @@ -33,7 +55,7 @@ class SessionStore { * flag (or a malformed `/me` response) doesn't accidentally * quarantine every user. */ - mustChangePassword = $derived(this.user?.force_password_change === true); + mustChangePassword = $derived(this.me?.force_password_change === true); /** * Resolve the session once. Probes /api/auth/me; on 401 it makes a single @@ -41,15 +63,15 @@ class SessionStore { * what to do with an unauthenticated result. Idempotent: subsequent calls * return the cached result (so client-side navigation doesn't re-probe). */ - async load(): Promise { - if (this.loaded) return this.user; + async load(): Promise { + if (this.loaded) return this.me; // No JS-visible session hint ⇒ nothing to probe. The server sets // `oxicloud_csrf` alongside the HttpOnly session cookies and clears // it on logout, so a missing hint means no session. Skips the // doomed 2× /me + /refresh burst that would otherwise fire on // every first landing / post-logout re-mount with no cookies. if (!hasSessionHint()) { - this.user = null; + this.me = null; this.loaded = true; return null; } @@ -71,25 +93,25 @@ class SessionStore { // otherwise clutter the audit stream. Fire-and-forget // so a slow IndexedDB open doesn't stall app boot. if (me.is_dpop_bound === false) void bindDpopIfPossible(); - } else this.user = null; + } else this.me = null; } catch { - this.user = null; + this.me = null; } this.loaded = true; - return this.user; + return this.me; } /** * Set the authenticated user AND run per-user localStorage cleanup * (see `$lib/utils/localStoragePrefs::ensureActiveUser`). Direct - * `session.user = …` assignments skip the cleanup — always call + * `session.me = …` assignments skip the cleanup — always call * `setUser` on login-flow entry points (form login, OIDC exchange, * existing-session probe) so a switch-account flow inside the same * tab observes the wipe. */ - setUser(user: User): void { - this.user = user; - ensureActiveUser(user.id); + setUser(me: SelfUser): void { + this.me = me; + ensureActiveUser(me.full.user.id); // Any successful login clears the session-teardown gate. Without // this, a logout → login within the same SPA session leaves the // gate stuck at `true` — the login POST is exempted via @@ -115,7 +137,7 @@ class SessionStore { async refresh(): Promise { try { const me = await fetchMe(); - if (me) this.user = me; + if (me) this.me = me; } catch { /* keep the existing user on a transient /api/auth/me failure */ } @@ -142,7 +164,7 @@ class SessionStore { } reset(): void { - this.user = null; + this.me = null; this.homeFolderId = null; this.homeFolderName = null; // Mark the store as `loaded` so any subsequent `session.load()` — diff --git a/frontend/src/routes/admin/[[tab]]/+page.svelte b/frontend/src/routes/admin/[[tab]]/+page.svelte index 63243dfe..919a3770 100644 --- a/frontend/src/routes/admin/[[tab]]/+page.svelte +++ b/frontend/src/routes/admin/[[tab]]/+page.svelte @@ -63,6 +63,7 @@ type StorageTestResult } from '$lib/api/endpoints/admin'; import { createDrive, updateDrivePolicies } from '$lib/api/endpoints/drives'; + import { seedUser } from '$lib/api/endpoints/users'; import { ensureResolvers, resolveRecipient, @@ -70,7 +71,7 @@ type Recipient } from '$lib/api/endpoints/recipients'; import type { - AdminUserSummary, + FullUser, Drive, DriveMember, DrivePolicies, @@ -156,11 +157,11 @@ deleteUserModal !== null && deleteUserEmailInput.trim().toLowerCase() === deleteUserModal.email.toLowerCase() ); - function openDeleteUser(u: AdminUserSummary) { + function openDeleteUser(u: FullUser) { deleteUserModal = { - userId: u.id, - username: u.username || u.email, - email: u.email + userId: u.user.id, + username: u.user.username || u.user.email, + email: u.user.email }; deleteUserEmailInput = ''; } @@ -817,7 +818,7 @@ } // Users - let users = $state([]); + let users = $state([]); let total = $state(0); let pageIndex = $state(0); let usersError = $state(null); @@ -923,6 +924,12 @@ const page = await listUsers(PAGE_SIZE, pageIndex * PAGE_SIZE); users = page.users; total = page.total; + // Seed the per-user resolver cache with the row's `PublicUser` + // slice so every `UserVignette` mounted per row hits the cache + // synchronously — no per-row `/api/users/{id}` follow-up. + // Kills the N+1 that motivated widening `/api/admin/users` to + // carry the avatar (docs/plan/userdto-refactor.md § N+1). + for (const row of page.users) seedUser(row.user); } catch (e) { usersError = errorMessage(e); } @@ -1003,49 +1010,49 @@ } /** True for the signed-in admin's own row — guards self-destructive actions. */ - function isSelf(u: AdminUserSummary): boolean { - return u.id === currentAdminId; + function isSelf(u: FullUser): boolean { + return u.user.id === currentAdminId; } /** OIDC/SSO-provisioned account (no local password to reset). */ - function isOidcUser(u: AdminUserSummary): boolean { + function isOidcUser(u: FullUser): boolean { return u.federation_kind === 'oidc'; } /** Used-quota percentage (0 when unlimited) for the per-user progress bar. */ - function quotaPct(u: AdminUserSummary): number { + function quotaPct(u: FullUser): number { return u.storage_quota_bytes > 0 ? (u.storage_used_bytes / u.storage_quota_bytes) * 100 : 0; } - async function toggleRole(u: AdminUserSummary) { + async function toggleRole(u: FullUser) { if (isSelf(u)) return; - const role = u.role === 'admin' ? 'user' : 'admin'; + const role = u.user.role === 'admin' ? 'user' : 'admin'; if (!(await showConfirm(t('admin.confirm_role', { role }, 'Change role to {{role}}?')))) return; try { - await setUserRole(u.id, role); + await setUserRole(u.user.id, role); await loadUsers(); } catch (e) { reportError(e); } } - async function toggleActive(u: AdminUserSummary) { + async function toggleActive(u: FullUser) { if (isSelf(u) && u.active) return; const msg = u.active ? t('admin.confirm_deactivate', 'Deactivate this user?') : t('admin.confirm_activate', 'Activate this user?'); if (!(await showConfirm(msg))) return; try { - await setUserActive(u.id, !u.active); + await setUserActive(u.user.id, !u.active); await loadUsers(); } catch (e) { reportError(e); } } - function openQuota(u: AdminUserSummary) { + function openQuota(u: FullUser) { quotaModalError = null; quotaModal = { - userId: u.id, - username: u.username || u.email, + userId: u.user.id, + username: u.user.username || u.user.email, initialBytes: u.storage_quota_bytes }; } @@ -1069,8 +1076,8 @@ } } - function openReset(u: AdminUserSummary) { - resetModal = { userId: u.id, username: u.username || u.email }; + function openReset(u: FullUser) { + resetModal = { userId: u.user.id, username: u.user.username || u.user.email }; resetPassword = ''; resetError = null; } @@ -1094,7 +1101,7 @@ } } - function removeUser(u: AdminUserSummary) { + function removeUser(u: FullUser) { if (isSelf(u)) return; openDeleteUser(u); } @@ -1103,20 +1110,20 @@ // provisions a home drive + flips the is_external flag; irreversible // via the admin UI (there's no demote endpoint on purpose). Backend // refuses when magic-link login is disabled — surfaced as a toast. - async function promoteExternal(u: AdminUserSummary) { - if (!u.is_external) return; + async function promoteExternal(u: FullUser) { + if (!u.user.is_external) return; if ( !(await showConfirm( t( 'admin.confirm_promote_user', - { name: u.username || u.email }, + { name: u.user.username || u.user.email }, 'Promote {{name}} to an internal user? This provisions a home drive and gives the account a normal storage envelope. The account keeps its identity; magic-link login stays the way in unless a password is set later.' ) )) ) return; try { - await promoteUserToInternal(u.id); + await promoteUserToInternal(u.user.id); await loadUsers(); } catch (e) { reportError(e); @@ -1240,8 +1247,13 @@ .map(async (d) => { const ownerMember = nextMembers[d.id]?.find((m) => m.subject.type === 'user'); if (!ownerMember) return; - const user = await getUserAdmin(ownerMember.subject.id); - if (user) nextOwners[d.id] = user; + // `getUserAdmin` returns `FullUser` (admin-visible extras + // + nested `.user: PublicUser`). The drive row only reads + // public-identity fields (username, email, image) so keep + // the map typed as `PublicUser` and unwrap the embedded + // public block on insert. See docs/plan/userdto-refactor.md. + const full = await getUserAdmin(ownerMember.subject.id); + if (full) nextOwners[d.id] = full.user; }) ); personalDriveOwners = nextOwners; @@ -1723,6 +1735,16 @@ {:else if !dashboard}

{t('common.loading', 'Loading…')}

{:else} + + + +

{t('admin.section_accounts', 'User accounts')}

{dashboard.total_users}{t('admin.total_users', 'Total users')} @@ -1733,11 +1755,66 @@
{dashboard.admin_users}{t('admin.admin_users', 'Admins')}
-
- v{dashboard.server_version}{t('admin.version', 'Version')} +
+ {dashboard.external_users}{t( + 'admin.external_users', + 'External' + )}
+ +

+ {t('admin.section_activity', 'Live activity')} + + {t('admin.live', 'live')} + +

+
+
+ + + {dashboard.online_users} + + {t('admin.online_users', 'Online users')} +
+
+ + + {dashboard.online_sessions} + + {t('admin.online_sessions', 'Online sessions')} +
+
+ + +

{t('admin.section_system', 'System')}

@@ -1761,6 +1838,9 @@ {t('admin.quotas', 'Quotas')}
+
+ v{dashboard.server_version}{t('admin.version', 'Version')} +
{#if dashboard.users_over_quota > 0} @@ -1802,13 +1882,17 @@ row.kind === 'personal' ? t('admin.quota_personal', 'Personal drives') : t('admin.quota_shared', 'Shared drives')} + {@const total = row.unlimited_count + row.capped_count} {@const pct = row.capped_quota_bytes && row.capped_quota_bytes > 0 ? (row.used_bytes / row.capped_quota_bytes) * 100 : null} {#if row.capped_count > 0 || row.unlimited_count > 0} - {label} + + {total} + {label} + {#if row.capped_quota_bytes !== null && pct !== null} {formatBytes(row.used_bytes)} / {formatBytes(row.capped_quota_bytes)} @@ -2680,15 +2764,15 @@ - {#each users as u (u.id)} + {#each users as u (u.user.id)} {@const pct = quotaPct(u)}
{#if isSelf(u)} {t('admin.you_badge', 'you')} @@ -2703,11 +2787,11 @@ badge is `white-space: nowrap` so the badge label itself never wraps mid-word either. -->
- - {#if u.role === 'admin'}{/if} - {u.role} + + {#if u.user.role === 'admin'}{/if} + {u.user.role} - {#if u.is_external} + {#if u.user.is_external}
- {#if u.is_external} + {#if u.user.is_external} {:else} @@ -2926,7 +3010,7 @@
-
{timeAgo(session.user.last_login_at)}
+
{timeAgo(session.me?.full.last_login_at)}
@@ -752,20 +762,20 @@

{t('profile.storage', 'Storage')}

-
{formatBytes(session.user.storage_used_bytes)}
+
{formatBytes(session.me?.full.storage_used_bytes ?? 0)}
{t('profile.used', 'Used')}
- {session.user.storage_quota_bytes > 0 - ? formatBytes(session.user.storage_quota_bytes) + {(session.me?.full.storage_quota_bytes ?? 0) > 0 + ? formatBytes(session.me?.full.storage_quota_bytes ?? 0) : '∞'}
{t('profile.quota', 'Quota')}
- {session.user.storage_quota_bytes > 0 ? `${storagePct}%` : '—'} + {(session.me?.full.storage_quota_bytes ?? 0) > 0 ? `${storagePct}%` : '—'}
{t('profile.usage', 'Usage')}
diff --git a/frontend/src/routes/profile/page.test.ts b/frontend/src/routes/profile/page.test.ts index 204a7744..10a2cf9b 100644 --- a/frontend/src/routes/profile/page.test.ts +++ b/frontend/src/routes/profile/page.test.ts @@ -1,10 +1,15 @@ import { it, expect, vi, beforeEach } from 'vitest'; import { render, screen, fireEvent, waitFor } from '@testing-library/svelte'; -const { session, ui } = vi.hoisted(() => ({ - session: { - loaded: true, - load: vi.fn(), +// Test-double session store. Post the three-layer UserDto refactor +// (docs/plan/userdto-refactor.md), production `session.user` is a +// derived accessor over `session.me.full.user`. The stub here mirrors +// that shape: `me` carries the whole SelfUser tree, and `user` mirrors +// `me.full.user` so any legacy `session.user.foo` read on the tested +// page keeps working through the mock without reproducing the derived +// mechanism. +const buildSelfMe = () => ({ + full: { user: { id: '1', username: 'admin', @@ -12,14 +17,41 @@ const { session, ui } = vi.hoisted(() => ({ given_name: 'A', family_name: 'B', role: 'admin', + is_external: false + }, + storage_used_bytes: 100, + storage_quota_bytes: 1000, + has_password: true + } +}); + +const { session, ui } = vi.hoisted(() => { + const me = { + full: { + user: { + id: '1', + username: 'admin', + email: 'a@x.test', + given_name: 'A', + family_name: 'B', + role: 'admin', + is_external: false + }, storage_used_bytes: 100, storage_quota_bytes: 1000, - is_external: false, has_password: true } - }, - ui: { notify: vi.fn() } -})); + }; + return { + session: { + loaded: true, + load: vi.fn(), + me, + user: me.full.user + }, + ui: { notify: vi.fn() } + }; +}); vi.mock('$lib/stores/session.svelte', () => ({ session })); vi.mock('$lib/stores/ui.svelte', () => ({ ui })); vi.mock('$lib/stores/dialogs.svelte', () => ({ confirmDialog: vi.fn() })); @@ -44,20 +76,13 @@ const m = (fn: unknown) => fn as ReturnType; beforeEach(() => { vi.clearAllMocks(); - // Reset the shared session each test (handlers may mutate session.user). + // Reset the shared session each test (handlers may mutate session.me + // on save / refresh). `me` is the SelfUser tree; `user` mirrors + // `me.full.user` for legacy `session.user.foo` reads. session.loaded = true; - session.user = { - id: '1', - username: 'admin', - email: 'a@x.test', - given_name: 'A', - family_name: 'B', - role: 'admin', - storage_used_bytes: 100, - storage_quota_bytes: 1000, - is_external: false, - has_password: true - }; + const me = buildSelfMe(); + session.me = me; + session.user = me.full.user; m(profile.listAppPasswords).mockResolvedValue([]); m(profile.updateProfile).mockResolvedValue(undefined); m(getOidcProviders).mockResolvedValue({ password_login_enabled: true }); diff --git a/frontend/static/locales/en.json b/frontend/static/locales/en.json index 88557131..fc20b4d3 100644 --- a/frontend/static/locales/en.json +++ b/frontend/static/locales/en.json @@ -546,6 +546,7 @@ "empty_hidden_hint": "Files whose name starts with '.' are hidden. Toggle the setting to see them.", "show_hidden": "Show hidden files", "upload_dotfile_hidden": "{{n}} file(s) uploaded but hidden by your dotfile preference.", + "upload_folder_not_ready": "Folder is still loading — please try again in a moment.", "rename_dotfile_hidden": "Renamed to '{{name}}' — now hidden by your preference.", "new_folder_dotfile_hidden": "Created folder '{{name}}' — hidden by your dotfile preference.", "dotfiles_hidden_toast": "Dotfiles hidden", @@ -836,6 +837,17 @@ "total_users": "Total Users", "active_users": "Active Users", "admins": "Admins", + "external_users": "External", + "external_users_tooltip": "Grant-only accounts — magic-link, OIDC-only, OCM recipients", + "online_users": "Online users", + "online_users_tooltip": "Distinct users with a session active in the last 5 minutes", + "online_sessions": "Online sessions", + "online_sessions_tooltip": "Non-revoked sessions active in the last 5 minutes — multi-device users contribute more than one", + "section_accounts": "User accounts", + "section_activity": "Live activity", + "section_system": "System", + "live": "live", + "live_tooltip": "Reflects sessions active in the last 5 minutes", "version": "Version", "storage_overview": "Storage Overview", "used": "Used", diff --git a/frontend/static/locales/fr.json b/frontend/static/locales/fr.json index 998c1e4a..a2ef3ce1 100644 --- a/frontend/static/locales/fr.json +++ b/frontend/static/locales/fr.json @@ -459,6 +459,7 @@ "empty_hidden_hint": "Les fichiers dont le nom commence par '.' sont masqués. Modifiez le réglage pour les afficher.", "show_hidden": "Afficher les fichiers masqués", "upload_dotfile_hidden": "{{n}} fichier(s) téléversé(s) mais masqué(s) par votre préférence.", + "upload_folder_not_ready": "Le dossier est encore en cours de chargement — merci de réessayer dans un instant.", "rename_dotfile_hidden": "Renommé en \"{{name}}\" — désormais masqué par votre préférence.", "new_folder_dotfile_hidden": "Dossier \"{{name}}\" créé — masqué par votre préférence.", "dotfiles_hidden_toast": "Fichiers masqués", @@ -801,6 +802,17 @@ "total_users": "Utilisateurs totaux", "active_users": "Utilisateurs actifs", "admins": "Admins", + "external_users": "Externes", + "external_users_tooltip": "Comptes invités — magic-link, OIDC seulement, destinataires OCM", + "online_users": "Utilisateurs en ligne", + "online_users_tooltip": "Utilisateurs distincts ayant une session active dans les 5 dernières minutes", + "online_sessions": "Sessions en ligne", + "online_sessions_tooltip": "Sessions non révoquées actives dans les 5 dernières minutes — les utilisateurs multi-appareils en contribuent plusieurs", + "section_accounts": "Comptes utilisateurs", + "section_activity": "Activité en direct", + "section_system": "Système", + "live": "en direct", + "live_tooltip": "Reflète les sessions actives dans les 5 dernières minutes", "version": "Version", "storage_overview": "Aperçu du stockage", "used": "Utilisé", diff --git a/src/application/dtos/settings_dto.rs b/src/application/dtos/settings_dto.rs index 0d2cc9da..7917d519 100644 --- a/src/application/dtos/settings_dto.rs +++ b/src/application/dtos/settings_dto.rs @@ -108,14 +108,15 @@ pub struct AdminResetPasswordDto { pub new_password: String, } -/// Query parameters for listing users +/// Query parameters for listing users. `/api/admin/users` used to +/// bifurcate on `?summary=` (flat `PublicUserDto` vs nested +/// `FullUserDto`); that split was retired — the endpoint now always +/// returns `FullUserDto`. Unknown query params are ignored, so +/// existing callers still passing `?summary=true` keep working. #[derive(Debug, Serialize, Deserialize)] pub struct ListUsersQueryDto { pub limit: Option, pub offset: Option, - /// Return only the fields rendered by the paginated management table. - /// Defaults to `false` so existing API clients keep the full user shape. - pub summary: Option, } /// Query parameters for the admin sessions listing. @@ -163,10 +164,39 @@ pub struct DashboardStatsDto { pub auth_enabled: bool, pub oidc_configured: bool, pub quotas_enabled: bool, - // User stats + // ── User accounts (static breakdown of auth.users) ── + // All four are counts of the SAME table under different + // predicates. `active`, `admin`, `external` are all subsets of + // `total`. `external` is disjoint from `admin` by DB constraint + // (`users_external_not_admin`). The dashboard renders these as + // one grouped section separate from the live-activity section + // below, so admins don't confuse "as-of-now row count" with + // "who's here right now". pub total_users: i64, pub active_users: i64, pub admin_users: i64, + /// Grant-only accounts (magic-link / OIDC-only / OCM recipients). + /// Filtered out of `total_users` / `active_users` since those + /// columns count operational seats (see the SELECT comment). Here + /// as its own metric because operators of external-heavy + /// deployments (public shares, invited-collab shops) need to see + /// the invited population at a glance. + pub external_users: i64, + // ── Live activity (projection over auth.sessions) ── + // Both fields change minute-to-minute, unlike the user counts + // above which only move on register/deactivate/role-toggle. + // Same 5-min window as the Prometheus gauges + // (`oxicloud_sessions_online[_users]` in + // `session_liveness_gauges.rs`), computed via the shared + // `ONLINE_WINDOW` constant so per-user badges + aggregate + // counts + this dashboard number stay consistent by construction. + /// Distinct users behind non-revoked sessions active in the last + /// 5 min. Answers "how many humans are here right now?". + pub online_users: i64, + /// Non-revoked sessions active in the last 5 min. Answers "how + /// many concurrent connections must I serve?". Ratio + /// `online_sessions / online_users` is the multi-device factor. + pub online_sessions: i64, // ── Per-drive-kind quota accounting ── // One row per drive kind (personal, shared). Pre-dedup, logical // file sizes summed from `drives.used_bytes` (personal rolls up diff --git a/src/application/dtos/user_dto.rs b/src/application/dtos/user_dto.rs index 9147fafa..6822535d 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,283 @@ use std::sync::Arc; use utoipa::ToSchema; use uuid::Uuid; +// ──────────────────────────────────────────────────────────────────────── +// 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. +// +// 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 +/// 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 UserDto { +pub struct PublicUserDto { 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, + /// Role string ("admin" | "user"). Kept public because the sharee / + /// group-member vignette renders an admin badge. 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, + /// 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, - 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`. + /// future OCM federated). Renders the "external" badge on the vignette. 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. + /// Optional first/given name. Social identity. #[serde(skip_serializing_if = "Option::is_none")] pub given_name: Option, - /// Optional last/family name. Same provenance + serde rules as - /// `given_name`. + /// Optional last/family name. Social identity. #[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. + /// 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 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, + pub is_online: bool, } -/// Compact row returned by the paginated admin user table. +/// 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`. /// -/// 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. +/// 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 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. +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, - /// See `UserDto::federation_issuer` — same semantics, same wire spelling. + /// 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, - 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)] + /// 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, - /// 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)] + /// TRUE when the user has an OPAQUE envelope on file. 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)] + /// 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, } -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, - } - } +/// 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 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. +impl PublicUserDto { + /// Construct a `PublicUserDto` from a `User` entity + an explicit + /// `is_online` signal. + /// + /// **Why not `From`?** The `User` entity models a row in + /// `auth.users`; `is_online` is a cross-table lookup on + /// `auth.sessions` (see the EXISTS subquery in + /// `list_users_with_derived_flags` and `get_user_with_derived_flags` + /// on the user repo). A `From` impl couldn't compute it + /// honestly — it would have to ship a `false` default that lies to + /// the FE presence dot on every emitter that didn't remember to + /// override. Making presence a required constructor argument + /// removes that footgun: every callsite has to declare its intent. + /// + /// Two shapes at the callsite: + /// + /// - Presence matters (single-user `/api/users/{id}`, list + /// projections, self-view): pair with + /// `user_storage.get_user_with_derived_flags(id)` and pass + /// `flags.is_online`. + /// - Presence is out of scope (register / update-profile response, + /// post-mutation echo where the FE ignores the field): pass + /// `false` with a short comment explaining why. The receiver's + /// presence read is a no-op — no dot lights up on the stale + /// value. + pub fn new(user: User, is_online: bool) -> Self { 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, + image: p.image, + is_external: p.is_external, + given_name: p.given_name, + family_name: p.family_name, + is_online, + } + } +} + +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, - // 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, + 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 @@ -425,7 +420,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, @@ -562,7 +562,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 @@ -606,3 +606,142 @@ pub struct OidcUserInfoDto { pub name: Option, pub groups: Vec, } + +#[cfg(test)] +mod three_layer_quarantine { + use super::*; + use serde_json::Value; + + /// Structural-quarantine guard for `SelfUserDto`. The self-only + /// bag (`ui_preferences`, `notify_on_share`, `is_dpop_bound`, + /// `force_password_change`, `can_edit_image`) MUST live at the + /// top level, NOT nested inside `.full` or `.full.user`. If a + /// future refactor accidentally moves one of them down, the + /// wire shape leaks it through every `PublicUserDto` / + /// `FullUserDto` emitter (share responses, group members, + /// `/api/admin/users`, magic-link invitees) — exactly what the + /// three-layer split exists to prevent. Fails loudly here. + #[test] + fn self_only_fields_stay_at_top_level_of_self_user_dto() { + let self_dto = SelfUserDto { + full: FullUserDto { + user: PublicUserDto { + id: "00000000-0000-0000-0000-000000000001".into(), + username: None, + email: "self@example.invalid".into(), + role: "user".into(), + image: None, + is_external: false, + given_name: None, + family_name: None, + is_online: false, + }, + federation_kind: None, + federation_issuer: None, + preferred_locale: None, + email_verified_at: None, + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + last_login_at: None, + active: true, + storage_quota_bytes: 0, + storage_used_bytes: 0, + has_password: true, + opaque_registered: false, + opaque_migrated: false, + }, + ui_preferences: serde_json::json!({}), + notify_on_share: true, + is_dpop_bound: false, + force_password_change: false, + can_edit_image: true, + }; + let json: Value = serde_json::to_value(&self_dto).expect("SelfUserDto serialises"); + assert!( + json.get("ui_preferences").is_some(), + "top-level ui_preferences" + ); + assert!( + json.get("full") + .expect("full block") + .get("ui_preferences") + .is_none(), + "ui_preferences must NOT appear inside `.full`" + ); + assert!( + json.pointer("/full/user/ui_preferences").is_none(), + "ui_preferences must NOT appear inside `.full.user`" + ); + // Same guard for the other self-only fields. + for k in [ + "notify_on_share", + "is_dpop_bound", + "force_password_change", + "can_edit_image", + ] { + assert!(json.get(k).is_some(), "{k} at top level"); + assert!( + json.pointer(&format!("/full/{k}")).is_none(), + "{k} must NOT nest in .full" + ); + assert!( + json.pointer(&format!("/full/user/{k}")).is_none(), + "{k} must NOT nest in .full.user" + ); + } + } + + /// Structural-quarantine guard for `FullUserDto`. Admin-visible + /// extras (`has_password`, OPAQUE flags, `federation_*`, + /// `last_login_at`, `active`, quotas, `preferred_locale`, + /// `email_verified_at`) MUST live at the top level of + /// `FullUserDto`, NOT inside `.user`. If a future refactor + /// accidentally lifts one of them onto `PublicUserDto` (the + /// embedded `user` field), it leaks through `/api/users/{id}` + /// and every other public directory endpoint. + #[test] + fn admin_only_fields_stay_at_top_level_of_full_user_dto() { + let full = FullUserDto { + user: PublicUserDto { + id: "00000000-0000-0000-0000-000000000002".into(), + username: Some("bob".into()), + email: "bob@example.invalid".into(), + role: "user".into(), + image: None, + is_external: false, + given_name: None, + family_name: None, + is_online: false, + }, + federation_kind: None, + federation_issuer: None, + preferred_locale: None, + email_verified_at: None, + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + last_login_at: None, + active: true, + storage_quota_bytes: 10_737_418_240, + storage_used_bytes: 0, + has_password: true, + opaque_registered: false, + opaque_migrated: false, + }; + let json: Value = serde_json::to_value(&full).expect("FullUserDto serialises"); + for k in [ + "has_password", + "opaque_registered", + "opaque_migrated", + "last_login_at", + "active", + "storage_quota_bytes", + "storage_used_bytes", + ] { + assert!(json.get(k).is_some(), "{k} at top level of FullUserDto"); + assert!( + json.pointer(&format!("/user/{k}")).is_none(), + "{k} must NOT nest in .user" + ); + } + } +} diff --git a/src/application/ports/auth_ports.rs b/src/application/ports/auth_ports.rs index e7f2deff..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; @@ -123,6 +122,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 @@ -164,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 e042dcee..618fdd22 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, 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,8 +876,9 @@ impl AuthApplicationService { is_external = false, "🛂 user registered", ); - Ok(RegisterResult::Created(Box::new(UserDto::from( + Ok(RegisterResult::Created(Box::new(PublicUserDto::new( created_user, + false, )))) } @@ -894,7 +895,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 +982,7 @@ impl AuthApplicationService { username, created_user.id() ); - Ok(UserDto::from(created_user)) + Ok(PublicUserDto::new(created_user, false)) } pub async fn login( @@ -1320,12 +1321,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 +1339,69 @@ 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 { + // Session-context flavour — delegates to the shared builder + // with the DPoP-bound flag derived from the session row's + // thumbprint. See [`build_self_user_dto_for_id`] for the + // handler-context flavour. + self.build_self_user_dto_for_id(user_id, session.dpop_jkt().is_some()) + .await + } + + /// Handler-context variant of [`build_self_user_dto`]. Called by + /// every endpoint that returns a `SelfUserDto` from a REST handler + /// (`GET /me`, `PATCH /me/profile`, `POST /upgrade-to-internal`) + /// so the wire shape is byte-for-byte identical across them — + /// avoids a "quiet lie" where a client PATCHes one shape and + /// reads another on the very next `/me`. + /// + /// `is_dpop_bound` is passed in by the handler because the JWT + /// `cnf.jkt` claim is where handler-scope code learns the caller's + /// binding state (via `auth_user.dpop_jkt.is_some()`). Session- + /// mint paths use [`build_self_user_dto`] and derive the flag from + /// the freshly-created `Session` row instead. + pub async fn build_self_user_dto_for_id( + &self, + user_id: Uuid, + is_dpop_bound: bool, + ) -> 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 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 +1654,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 +1864,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 +1879,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(), @@ -2025,7 +2106,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 @@ -2126,7 +2207,7 @@ impl AuthApplicationService { lc.dispatch_upgraded_to_internal(&updated).await; } - Ok(UserDto::from(updated)) + Ok(PublicUserDto::new(updated, false)) } /// Admin-driven external → internal promotion. @@ -2155,7 +2236,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() { @@ -2237,7 +2318,7 @@ impl AuthApplicationService { "👮🏻‍♂️ external user promoted to internal by admin", ); - Ok(UserDto::from(updated)) + Ok(PublicUserDto::new(updated, false)) } /// `keep_session_id` — when `Some`, revoke every OTHER session for @@ -2492,9 +2573,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::new(user, false)) } /// Cached, image-free lookup of the caller's authorization flags @@ -2643,7 +2724,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 @@ -2819,7 +2900,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::new(user, false)); } // Persist the typed-field changes first (if any). Skip the @@ -2850,11 +2931,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::new(refreshed, false)) } // 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 } @@ -2871,6 +2952,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 @@ -2927,12 +3028,23 @@ 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). + // + // Both branches use `get_user_with_derived_flags` (not the narrow + // `get_user_by_id`) so `PublicUserDto.is_online` on the wire + // reflects the same EXISTS subquery the admin list uses. Without + // this the FE presence dot would only light up on list-derived + // paths (admin seed); single fetches from share pickers / group + // members would show every user as offline regardless of real + // state. See `docs/plan/userdto-refactor.md`. if caller_id == target_id { - let caller = self.user_storage.get_user_by_id(caller_id).await?; - return Ok(UserDto::from(caller)); + let (caller, flags) = self + .user_storage + .get_user_with_derived_flags(caller_id) + .await?; + return Ok(PublicUserDto::new(caller, flags.is_online)); } // Caller and target are independent point reads (the self-case already @@ -2940,16 +3052,26 @@ impl AuthApplicationService { // overlap them with `join!` instead of two serial round-trips. // `caller_res?` first preserves the caller-error precedence of the old // sequential form. (benches/ROUND23.md §P1) + // + // `caller` uses the narrow `get_user_by_id` because we only read + // `is_external()` off it for the visibility gate; nothing about + // the caller ships on the wire. Only `target` needs the wider + // projection. let (caller_res, target_res) = tokio::join!( self.user_storage.get_user_by_id(caller_id), - self.user_storage.get_user_by_id(target_id) + self.user_storage.get_user_with_derived_flags(target_id) ); let caller = caller_res?; // Anti-enumeration: NotFound for everything that doesn't pass. // Convert a real NotFound on `target` to the same anonymous 404, // so existence isn't leaked through differential responses. - let target = match target_res { + // + // Destructure the (User, UserDerivedFlags) tuple immediately so + // `target` keeps its historical `User` shape (accessors still + // work below); the flags come along as `target_flags` for the + // `is_online` propagation into the returned `PublicUserDto`. + let (target, target_flags) = match target_res { Ok(u) => u, Err(e) if e.kind == ErrorKind::NotFound => { tracing::info!( @@ -2993,7 +3115,7 @@ impl AuthApplicationService { })?; if related.is_some() { - return Ok(UserDto::from(target)); + return Ok(PublicUserDto::new(target, target_flags.is_online)); } // (3) External callers stop here — no directory enumeration. @@ -3021,12 +3143,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::new(target, target_flags.is_online)); } // (5) Admin caller: always visible. if caller.role() == UserRole::Admin { - return Ok(UserDto::from(target)); + return Ok(PublicUserDto::new(target, target_flags.is_online)); } // (6) No relationship — anti-enumeration NotFound. @@ -3079,7 +3201,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 => { @@ -3106,9 +3228,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::new(user, false)) } // Method to count how many admin users exist in the system @@ -3125,42 +3247,46 @@ impl AuthApplicationService { /// out so that internal-user surfaces — system address book, OCS /// 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> { - let users = self.user_storage.list_users(limit, offset, false).await?; - Ok(users.into_iter().map(UserDto::from).collect()) - } - - /// Admin-only: lists users including external (grant-only) recipients. - /// Used by the admin user-management UI. - pub async fn list_users_including_external_with_perms( + /// [`list_user_summaries_including_external_with_perms`] instead. + pub async fn list_users( &self, - authorization: &A, - caller_id: Uuid, limit: i64, offset: i64, - ) -> 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()) + ) -> Result, DomainError> { + let users = self.user_storage.list_users(limit, offset, false).await?; + Ok(users + .into_iter() + .map(|u| PublicUserDto::new(u, false)) + .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). This is the sole + /// admin-visible listing path; the former flat + /// `list_users_including_external_with_perms` variant was + /// retired when `?summary` was dropped. 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. @@ -3183,9 +3309,16 @@ 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(|u| PublicUserDto::new(u, false)) + .collect()) } /// Username-only search for the NC sharee autocomplete: identical @@ -3215,8 +3348,8 @@ impl AuthApplicationService { // `interfaces/api/routes.rs::admin_router`) — but every admin // method here still calls `require_admin_caller` as a // defense-in-depth check, matching the pattern - // `list_users_including_external_with_perms` established. If a - // handler is ever wired outside the /admin subtree, the AuthZ + // `list_user_summaries_including_external_with_perms` established. + // If a handler is ever wired outside the /admin subtree, the AuthZ // still holds. /// List sessions for the admin panel. `user_id_filter = Some(uuid)` @@ -3291,7 +3424,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( @@ -3449,7 +3582,16 @@ impl AuthApplicationService { created.id(), created.is_external() ); - Ok(UserDto::from(created)) + // Return `FullUserDto` — same shape as `GET /api/admin/users/{id}` + // and one row of the admin list. Admin surfaces uniformly return + // FullUserDto so the SPA / test asserts don't need to know which + // admin endpoint they came from. Fresh user has no session yet + // (`is_online = false`) and no OPAQUE registration; `has_password` + // reflects whatever the admin passed in the DTO. + let created_id = created.id(); + let (user, flags) = + UserStoragePort::get_user_with_derived_flags(&*self.user_storage, created_id).await?; + Ok(FullUserDto::build(user, flags)) } /// Admin-only: reset a user's password. @@ -3545,10 +3687,20 @@ impl AuthApplicationService { Ok(()) } - /// Get a single user by ID (for admin panel) - 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)) + /// Get a single user by ID (for admin panel). + /// + /// Returns `FullUserDto` — same shape as one row of + /// `/api/admin/users` — so admin single-user views (detail modal, + /// per-user edit page) render the same fields the list surfaces. + /// The single-row admin view is the canonical observation surface + /// for admin-visible signals like `email_verified_at` / + /// `has_password` / `opaque_registered` / `last_login_at` — none + /// of which live on the peer-view `PublicUserDto`. See + /// `docs/plan/userdto-refactor.md`. + pub async fn get_user_admin(&self, user_id: Uuid) -> Result { + let (user, flags) = + UserStoragePort::get_user_with_derived_flags(&*self.user_storage, user_id).await?; + Ok(FullUserDto::build(user, flags)) } /// Delete a user by ID (admin only). @@ -4654,11 +4806,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/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 7bcb4148..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,49 +25,26 @@ 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. +/// 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). /// -/// 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). +/// 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 design; this type +/// replaced the earlier `UserListEntry` narrow projection as of P6. +#[derive(Debug, Clone, Copy)] +pub struct UserDerivedFlags { 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, + pub is_online: bool, } // Conversion from UserRepositoryError to DomainError @@ -94,6 +70,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 @@ -152,15 +142,22 @@ 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. - async fn list_user_summaries( + /// 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>; + ) -> UserRepositoryResult>; /// Searches users by username or email (SQL ILIKE) with a limit. /// See [`list_users`] for the meaning of `include_external`. diff --git a/src/infrastructure/repositories/pg/user_pg_repository.rs b/src/infrastructure/repositories/pg/user_pg_repository.rs index 6feb033e..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; @@ -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( @@ -835,103 +919,107 @@ impl UserRepository for UserPgRepository { Ok(users) } - async fn list_user_summaries( + async fn list_users_with_derived_flags( &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, - ), - >( - // 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). + ) -> 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, role::text, + id, username, email, password_hash, role::text as 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, + 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 - FROM auth.users - WHERE ($3 OR is_external = FALSE) - ORDER BY created_at DESC, id DESC - LIMIT $1 OFFSET $2 + (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) - .fetch_all(self.pool.as_ref()) + .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( - |( - id, - username, - email, + .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, - storage_quota_bytes, - storage_used_bytes, - last_login_at, - active, - federation_kind, - federation_issuer, - is_external, - has_password, - opaque_registered, - opaque_migrated, - )| 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, - }, - ) + 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()) } @@ -1302,6 +1390,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 @@ -1356,13 +1459,19 @@ impl UserStoragePort for UserPgRepository { .map_err(DomainError::from) } - async fn list_user_summaries( + async fn list_users_with_derived_flags( &self, limit: i64, offset: i64, include_external: bool, - ) -> Result, DomainError> { - UserRepository::list_user_summaries(self, limit, offset, include_external) + ) -> 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) } @@ -1743,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 0cb66c26..e1985f77 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, 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, @@ -39,16 +39,14 @@ use crate::interfaces::middleware::auth::AuthUser; use std::sync::Arc; use uuid::Uuid; -#[derive(serde::Serialize)] -#[serde(untagged)] -enum AdminUsersPayload { - Full(Vec), - Summary(Vec), -} - +/// Response envelope for `GET /api/admin/users`. `users` is always +/// `Vec` — same shape one row of `/me`'s embedded +/// `full` block carries; the FE seeds `resolveUser` cache from +/// `row.user` (kills the per-row `/api/users/{id}` fetch). See +/// `docs/plan/userdto-refactor.md`. #[derive(serde::Serialize)] struct AdminUsersPageResponse { - users: AdminUsersPayload, + users: Vec, total: i64, limit: i64, offset: i64, @@ -931,6 +929,51 @@ pub async fn get_dashboard_stats( .await .map_err(|e| AppError::internal_error(format!("Database query failed: {}", e)))?; + // External account count — distinct query (not FILTERed into + // `stats_row` above) because `stats_row` scopes to + // `is_external = false` for the operational-seat counts. + // Externals form their own population; the dashboard renders them + // as a separate stat card in the "User accounts" section. + let external_users: i64 = + sqlx::query_scalar(r#"SELECT COUNT(*)::INT8 FROM auth.users WHERE is_external = true"#) + .fetch_one(db_pool.as_ref()) + .await + .map_err(|e| AppError::internal_error(format!("External user count failed: {}", e)))?; + + // Live-activity counts — projection over auth.sessions, same + // `ONLINE_WINDOW` (5 min) the Prometheus gauges use so the + // dashboard number, admin-table green dot, and + // `oxicloud_sessions_online` scrape all agree by construction. + // Bound as `$1 = window_secs` via `make_interval(secs => $1)` + // to keep the single-source-of-truth pattern (no SQL literal + // for the window). Both queries hit the partial index + // `idx_sessions_last_seen_at WHERE revoked = FALSE` so per-run + // cost is ~μs even at tens of thousands of session rows. + let online_window_secs: f64 = + crate::application::dtos::session_dto::ONLINE_WINDOW.as_secs_f64(); + let online_sessions: i64 = sqlx::query_scalar( + r#" + SELECT COUNT(*)::INT8 FROM auth.sessions + WHERE revoked = FALSE + AND last_seen_at > NOW() - make_interval(secs => $1) + "#, + ) + .bind(online_window_secs) + .fetch_one(db_pool.as_ref()) + .await + .map_err(|e| AppError::internal_error(format!("Online session count failed: {}", e)))?; + let online_users: i64 = sqlx::query_scalar( + r#" + SELECT COUNT(DISTINCT user_id)::INT8 FROM auth.sessions + WHERE revoked = FALSE + AND last_seen_at > NOW() - make_interval(secs => $1) + "#, + ) + .bind(online_window_secs) + .fetch_one(db_pool.as_ref()) + .await + .map_err(|e| AppError::internal_error(format!("Online user count failed: {}", e)))?; + use sqlx::Row; // Per-drive-kind quota panel: @@ -1015,6 +1058,9 @@ pub async fn get_dashboard_stats( total_users: stats_row.get("total_users"), active_users: stats_row.get("active_users"), admin_users: stats_row.get("admin_users"), + external_users, + online_users, + online_sessions, drive_usage, users_over_80_percent: stats_row.get("users_over_80"), users_over_quota: stats_row.get("users_over_quota"), @@ -1037,13 +1083,20 @@ pub async fn get_dashboard_stats( // ============================================================================ /// GET /api/admin/users?limit=50&offset=0 — list all users +/// +/// Always returns `Vec` — the shape one row of the +/// `/me` response's embedded `full` block carries. The former +/// `?summary` toggle (flat `PublicUserDto` vs nested `FullUserDto`) +/// has been retired: admin listing is low-volume and the FE always +/// asked for the nested shape anyway, so the two-shape split served +/// no caller and only invited jq-path bugs. See +/// `docs/plan/userdto-refactor.md`. #[utoipa::path( get, path = "/api/admin/users", params( ("limit" = Option, Query, description = "Max users to return (default 100, max 500)"), - ("offset" = Option, Query, description = "Pagination offset"), - ("summary" = Option, Query, description = "Return the compact management-table projection") + ("offset" = Option, Query, description = "Pagination offset") ), responses( (status = 200, description = "List of users"), @@ -1071,31 +1124,16 @@ pub async fn list_users( // internal-only variant is used by system address book / sharee // search, where surfacing externals would leak identities. See // `auth_application_service::list_users` doc for the split. - let users = if query.summary.unwrap_or(false) { - AdminUsersPayload::Summary( - auth.auth_application_service - .list_user_summaries_including_external_with_perms( - state.authorization.as_ref(), - auth_user.id, - limit, - offset, - ) - .await - .map_err(AppError::from)?, + let users = auth + .auth_application_service + .list_user_summaries_including_external_with_perms( + state.authorization.as_ref(), + auth_user.id, + limit, + offset, ) - } else { - AdminUsersPayload::Full( - auth.auth_application_service - .list_users_including_external_with_perms( - state.authorization.as_ref(), - auth_user.id, - limit, - offset, - ) - .await - .map_err(AppError::from)?, - ) - }; + .await + .map_err(AppError::from)?; let total = auth .auth_application_service @@ -1562,7 +1600,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 2f9bad63..236721b4 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, LoginDto, OidcCallbackQueryDto, OidcExchangeDto, - OidcProviderInfoDto, RefreshTokenDto, RegisterDto, SetupAdminDto, UpgradeToInternalDto, - UserDto, + 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()) @@ -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" = [])), @@ -652,37 +652,21 @@ pub async fn get_current_user( // Semantics (`docs/plan/drive.md` §7): `storage_used_bytes` is the SUM // of `used_bytes` across the user's personal drives only. Shared drives // 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 + // costs no personal bytes. + // + // Delegate to the shared `build_self_user_dto_for_id` — same code + // path `PATCH /me/profile` and `POST /upgrade-to-internal` use so + // all three self endpoints ship byte-for-byte identical shapes. + // The DPoP-bound signal comes from the JWT `cnf.jkt` claim + // (surfaced by the auth middleware into `AuthUser.dpop_jkt`); + // when present the session that minted this JWT is bound and + // the SPA can skip a redundant `/dpop/bind` call. + let self_dto = auth_service .auth_application_service - .get_user_by_id(user_id) + .build_self_user_dto_for_id(user_id, auth_user.dpop_jkt.is_some()) .await?; - // 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`) - // avoids a second DB round-trip on this hot endpoint. - if let Ok(flags) = auth_service - .auth_application_service - .get_user_flags(user_id) - .await - { - user.force_password_change = flags.force_password_change; - } - - // 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(); - - Ok((StatusCode::OK, Json(user))) + Ok((StatusCode::OK, Json(self_dto))) } /// DTO for updating the user's profile image. @@ -837,14 +821,16 @@ pub async fn change_password( /// self-registration policy. Refused with 403 /// `error_type = "RegistrationDomainNotAllowed"`. /// -/// Response: the updated `UserDto` (post-upgrade view — `is_external` -/// is false, `storage_quota_bytes` is set). +/// Response: the updated `SelfUserDto` (same shape as `GET /me`) so the SPA +/// absorbs the post-upgrade state — new `storage_quota_bytes`, +/// `is_external = false`, updated OPAQUE / auth capability flags — in one +/// round trip without a follow-up `/me` fetch. #[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 — returns SelfUserDto (same shape as GET /me)", body = SelfUserDto), (status = 400, description = "Password missing / too short"), (status = 401, description = "Not authenticated"), (status = 403, description = "OIDC user, or domain not in allowlist"), @@ -855,9 +841,10 @@ pub async fn change_password( )] pub async fn upgrade_to_internal( State(state): State>, - CurrentUserId(user_id): CurrentUserId, + auth_user: AuthUser, Json(dto): Json, ) -> Result { + let user_id = auth_user.id; let auth_service = state .auth_service .as_ref() @@ -905,7 +892,11 @@ pub async fn upgrade_to_internal( } } - let updated = auth_service + // Apply the upgrade. Service returns the updated `PublicUserDto`; + // we discard it and rebuild the full self view via the shared + // `build_self_user_dto_for_id` helper so the wire shape matches + // `GET /me` and `PATCH /me/profile` byte-for-byte. + let _ = auth_service .auth_application_service .upgrade_to_internal(user_id, dto) .await @@ -924,7 +915,11 @@ pub async fn upgrade_to_internal( _ => AppError::from(err), })?; - Ok((StatusCode::OK, Json(updated))) + let self_dto = auth_service + .auth_application_service + .build_self_user_dto_for_id(user_id, auth_user.dpop_jkt.is_some()) + .await?; + Ok((StatusCode::OK, Json(self_dto))) } /// Update the caller's profile (PR 24). @@ -942,7 +937,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 (SelfUserDto) — same shape as GET /me so the SPA sees the just-written state without a follow-up fetch", body = SelfUserDto), (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"), @@ -953,20 +948,39 @@ pub async fn upgrade_to_internal( )] pub async fn update_profile( State(state): State>, - CurrentUserId(user_id): CurrentUserId, + auth_user: AuthUser, Json(dto): Json, ) -> Result { + let user_id = auth_user.id; let auth_service = state .auth_service .as_ref() .ok_or_else(|| AppError::internal_error("Authentication service not configured"))?; - let updated = auth_service + // Apply the patch. The service returns the updated `PublicUserDto` + // internally; we discard it and re-fetch the full self view below + // so the response matches `GET /me`'s `SelfUserDto` shape. + // + // Why SelfUserDto instead of PublicUserDto: a self-write endpoint + // whose response mirrors GET /me lets the SPA update its session + // store in one round trip. Returning a slim PublicUserDto would + // force the SPA to follow up with GET /me anyway to observe the + // just-written `ui_preferences` / `notify_on_share` / etc — those + // fields live on SelfUserDto only, not on the public identity + // slice. Same shape for both endpoints avoids "quiet lie" reads + // where a client PATCHes and then reads a stale local value. + let _ = auth_service .auth_application_service .update_profile_with_perms(user_id, dto, &state.locale_registry) .await?; - Ok((StatusCode::OK, Json(updated))) + // Rebuild via the shared helper so the wire shape matches + // `GET /me` and `POST /upgrade-to-internal` byte-for-byte. + let self_dto = auth_service + .auth_application_service + .build_self_user_dto_for_id(user_id, auth_user.dpop_jkt.is_some()) + .await?; + Ok((StatusCode::OK, Json(self_dto))) } // TODO: add utoipa @@ -1226,7 +1240,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"), ), @@ -1820,10 +1834,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/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/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(), } } 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": "", diff --git a/tests/api/admin_user_ops.hurl b/tests/api/admin_user_ops.hurl index 0abbe1bf..2f779695 100644 --- a/tests/api/admin_user_ops.hurl +++ b/tests/api/admin_user_ops.hurl @@ -56,7 +56,7 @@ Content-Type: application/json HTTP 201 [Captures] -charlie_id: jsonpath "$.id" +charlie_id: jsonpath "$.user.id" # ───────────────────────────────────────────────────────────── @@ -89,7 +89,7 @@ Authorization: Bearer {{charlie_token_v1}} HTTP 200 [Asserts] -jsonpath "$.storage_quota_bytes" == 209715200 +jsonpath "$.full.storage_quota_bytes" == 209715200 # ───────────────────────────────────────────────────────────── @@ -108,7 +108,7 @@ Authorization: Bearer {{charlie_token_v1}} HTTP 200 [Asserts] -jsonpath "$.role" == "admin" +jsonpath "$.full.user.role" == "admin" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/auth_login.hurl b/tests/api/auth_login.hurl index c553613a..86f14651 100644 --- a/tests/api/auth_login.hurl +++ b/tests/api/auth_login.hurl @@ -19,7 +19,7 @@ Content-Type: application/json HTTP 200 [Asserts] jsonpath "$.access_token" exists -jsonpath "$.user.email" == "{{email}}" +jsonpath "$.user.full.user.email" == "{{email}}" # ───────────────────────────────────────────────────────────── @@ -34,7 +34,7 @@ Content-Type: application/json HTTP 200 [Asserts] jsonpath "$.access_token" exists -jsonpath "$.user.email" == "{{email}}" +jsonpath "$.user.full.user.email" == "{{email}}" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/auth_magic_link_login.hurl b/tests/api/auth_magic_link_login.hurl index da3e7197..e927c37d 100644 --- a/tests/api/auth_magic_link_login.hurl +++ b/tests/api/auth_magic_link_login.hurl @@ -142,10 +142,10 @@ Authorization: Bearer {{alice_magic_access_token}} HTTP 200 [Asserts] -jsonpath "$.email" == "{{email}}" -jsonpath "$.username" == "{{username}}" +jsonpath "$.full.user.email" == "{{email}}" +jsonpath "$.full.user.username" == "{{username}}" [Captures] -admin_user_id: jsonpath "$.id" +admin_user_id: jsonpath "$.full.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/auth_session_lifecycle.hurl b/tests/api/auth_session_lifecycle.hurl index df61b796..621afe88 100644 --- a/tests/api/auth_session_lifecycle.hurl +++ b/tests/api/auth_session_lifecycle.hurl @@ -78,7 +78,7 @@ Authorization: Bearer {{access_v2}} HTTP 200 [Asserts] -jsonpath "$.username" == "{{username}}" +jsonpath "$.full.user.username" == "{{username}}" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/auth_upgrade_to_internal.hurl b/tests/api/auth_upgrade_to_internal.hurl index ff429ec8..2e59aaa7 100644 --- a/tests/api/auth_upgrade_to_internal.hurl +++ b/tests/api/auth_upgrade_to_internal.hurl @@ -52,7 +52,7 @@ Content-Type: application/json HTTP 201 [Captures] -bob_user_id: jsonpath "$.id" +bob_user_id: jsonpath "$.user.id" # ───────────────────────────────────────────────────────────── @@ -73,8 +73,8 @@ Authorization: Bearer {{bob_token}} HTTP 200 [Asserts] -jsonpath "$.is_external" == true -jsonpath "$.storage_quota_bytes" == 0 +jsonpath "$.full.user.is_external" == true +jsonpath "$.full.storage_quota_bytes" == 0 # ───────────────────────────────────────────────────────────── @@ -88,8 +88,8 @@ Content-Type: application/json HTTP 200 [Asserts] -jsonpath "$.is_external" == false -jsonpath "$.storage_quota_bytes" > 0 +jsonpath "$.full.user.is_external" == false +jsonpath "$.full.storage_quota_bytes" > 0 # ───────────────────────────────────────────────────────────── @@ -100,8 +100,8 @@ Authorization: Bearer {{bob_token}} HTTP 200 [Asserts] -jsonpath "$.is_external" == false -jsonpath "$.storage_quota_bytes" > 0 +jsonpath "$.full.user.is_external" == false +jsonpath "$.full.storage_quota_bytes" > 0 # ───────────────────────────────────────────────────────────── @@ -179,7 +179,7 @@ Content-Type: application/json HTTP 201 [Captures] -carol_user_id: jsonpath "$.id" +carol_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login Content-Type: application/json @@ -204,7 +204,7 @@ Authorization: Bearer {{carol_token}} HTTP 200 [Asserts] -jsonpath "$.is_external" == true +jsonpath "$.full.user.is_external" == true # ───────────────────────────────────────────────────────────── diff --git a/tests/api/calendar.hurl b/tests/api/calendar.hurl index e53c192e..108330e4 100644 --- a/tests/api/calendar.hurl +++ b/tests/api/calendar.hurl @@ -41,7 +41,7 @@ Content-Type: application/json HTTP 200 [Captures] alice_token: jsonpath "$.access_token" -alice_user_id: jsonpath "$.user.id" +alice_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── @@ -120,7 +120,7 @@ Content-Type: application/json HTTP 200 [Captures] bob_token: jsonpath "$.access_token" -bob_user_id: jsonpath "$.user.id" +bob_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/contacts.hurl b/tests/api/contacts.hurl index 7057b488..a02021d5 100644 --- a/tests/api/contacts.hurl +++ b/tests/api/contacts.hurl @@ -24,7 +24,7 @@ Content-Type: application/json HTTP 200 [Captures] token: jsonpath "$.access_token" -admin_user_id: jsonpath "$.user.id" +admin_user_id: jsonpath "$.user.full.user.id" [Asserts] jsonpath "$.access_token" isString jsonpath "$.token_type" == "Bearer" @@ -344,7 +344,7 @@ Content-Type: application/json HTTP 200 [Captures] bob_token: jsonpath "$.access_token" -bob_user_id: jsonpath "$.user.id" +bob_user_id: jsonpath "$.user.full.user.id" # Step 17 — Bob's book listing does NOT include Alice's book. diff --git a/tests/api/cross_drive_copy.hurl b/tests/api/cross_drive_copy.hurl index b317705a..a804dd9e 100644 --- a/tests/api/cross_drive_copy.hurl +++ b/tests/api/cross_drive_copy.hurl @@ -71,7 +71,7 @@ Content-Type: application/json HTTP 200 [Captures] owner_token: jsonpath "$.access_token" -owner_user_id: jsonpath "$.user.id" +owner_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/cross_drive_move.hurl b/tests/api/cross_drive_move.hurl index c23b4eca..40b0bdd6 100644 --- a/tests/api/cross_drive_move.hurl +++ b/tests/api/cross_drive_move.hurl @@ -67,7 +67,7 @@ Content-Type: application/json HTTP 200 [Captures] owner_token: jsonpath "$.access_token" -owner_user_id: jsonpath "$.user.id" +owner_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/dav_error_mapping.hurl b/tests/api/dav_error_mapping.hurl index 1815397f..49d7ef20 100644 --- a/tests/api/dav_error_mapping.hurl +++ b/tests/api/dav_error_mapping.hurl @@ -249,7 +249,7 @@ Content-Type: application/json HTTP * [Captures] -alice_id: jsonpath "$.id" +alice_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login diff --git a/tests/api/default_caldav_carddav.hurl b/tests/api/default_caldav_carddav.hurl index f67cb071..d849f6f4 100644 --- a/tests/api/default_caldav_carddav.hurl +++ b/tests/api/default_caldav_carddav.hurl @@ -103,7 +103,7 @@ Content-Type: application/json HTTP * [Captures] -fresh_user_id: jsonpath "$.id" +fresh_user_id: jsonpath "$.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/drive_policies.hurl b/tests/api/drive_policies.hurl index 98f388eb..2aabcf0f 100644 --- a/tests/api/drive_policies.hurl +++ b/tests/api/drive_policies.hurl @@ -65,7 +65,7 @@ Content-Type: application/json HTTP 200 [Captures] owner_token: jsonpath "$.access_token" -owner_user_id: jsonpath "$.user.id" +owner_user_id: jsonpath "$.user.full.user.id" # Provision `dp_intruder` — a second internal user used only to @@ -91,7 +91,7 @@ Content-Type: application/json HTTP 200 [Captures] intruder_token: jsonpath "$.access_token" -intruder_user_id: jsonpath "$.user.id" +intruder_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/drive_quota.hurl b/tests/api/drive_quota.hurl index e15e5ee1..c1358cfd 100644 --- a/tests/api/drive_quota.hurl +++ b/tests/api/drive_quota.hurl @@ -60,7 +60,7 @@ Content-Type: application/json HTTP 200 [Captures] owner_token: jsonpath "$.access_token" -owner_user_id: jsonpath "$.user.id" +owner_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/drive_read_only.hurl b/tests/api/drive_read_only.hurl index 6f94540f..4045fc2a 100644 --- a/tests/api/drive_read_only.hurl +++ b/tests/api/drive_read_only.hurl @@ -82,7 +82,7 @@ Content-Type: application/json HTTP 200 [Captures] owner_token: jsonpath "$.access_token" -owner_user_id: jsonpath "$.user.id" +owner_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── @@ -107,7 +107,7 @@ Content-Type: application/json HTTP 200 [Captures] -target_user_id: jsonpath "$.user.id" +target_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/drives_foundation.hurl b/tests/api/drives_foundation.hurl index 55202224..2ba30b40 100644 --- a/tests/api/drives_foundation.hurl +++ b/tests/api/drives_foundation.hurl @@ -31,7 +31,7 @@ Content-Type: application/json HTTP 200 [Captures] admin_token: jsonpath "$.access_token" -admin_user_id: jsonpath "$.user.id" +admin_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── @@ -70,7 +70,7 @@ Content-Type: application/json HTTP 201 [Captures] -alice_user_id: jsonpath "$.id" +alice_user_id: jsonpath "$.user.id" POST {{base_url}}/api/admin/users Authorization: Bearer {{admin_token}} @@ -79,7 +79,7 @@ Content-Type: application/json HTTP 201 [Captures] -bob_user_id: jsonpath "$.id" +bob_user_id: jsonpath "$.user.id" # Alice's first login fires `PersonalDriveLifecycleHook::on_user_login` diff --git a/tests/api/drives_membership.hurl b/tests/api/drives_membership.hurl index 5d454a5c..05cc3f8b 100644 --- a/tests/api/drives_membership.hurl +++ b/tests/api/drives_membership.hurl @@ -64,7 +64,7 @@ Content-Type: application/json HTTP 200 [Captures] admin_token: jsonpath "$.access_token" -admin_user_id: jsonpath "$.user.id" +admin_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── @@ -113,7 +113,7 @@ Content-Type: application/json HTTP 201 [Captures] -alice_user_id: jsonpath "$.id" +alice_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login Content-Type: application/json @@ -470,7 +470,7 @@ Content-Type: application/json HTTP 201 [Captures] -bob_user_id: jsonpath "$.id" +bob_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login Content-Type: application/json @@ -657,7 +657,7 @@ Content-Type: application/json HTTP 201 [Captures] -carol_user_id: jsonpath "$.id" +carol_user_id: jsonpath "$.user.id" # 24a — Owner grants Carol Owner role (Owner-creates-Owner). @@ -836,7 +836,7 @@ Content-Type: application/json HTTP 201 [Captures] -dave_user_id: jsonpath "$.id" +dave_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login Content-Type: application/json diff --git a/tests/api/external_users.hurl b/tests/api/external_users.hurl index ab10d5b2..51fde096 100644 --- a/tests/api/external_users.hurl +++ b/tests/api/external_users.hurl @@ -23,7 +23,7 @@ Content-Type: application/json HTTP 200 [Captures] alice_token: jsonpath "$.access_token" -alice_user_id: jsonpath "$.user.id" +alice_user_id: jsonpath "$.user.full.user.id" GET {{base_url}}/api/folders Authorization: Bearer {{alice_token}} @@ -226,13 +226,16 @@ Authorization: Bearer {{bob_access_token}} HTTP 200 [Asserts] +# `/api/users/{id}` returns the slim `PublicUserDto` (9 fields) — +# id / email / username / role / image / is_external / given_name / +# family_name / is_online. Admin-visible fields like +# `email_verified_at` moved to `FullUserDto` under the three-layer +# refactor (docs/plan/userdto-refactor.md) and are checked below +# via `/api/admin/users`. jsonpath "$.id" == "{{bob_user_id}}" jsonpath "$.is_external" == true jsonpath "$.email" == "bob@externalcompany.com" jsonpath "$.username" not exists -# PR 23 — bob redeemed his invitation magic-link in Step 8, so his -# email_verified_at was stamped at that time and stays set. -jsonpath "$.email_verified_at" exists # 11d — bob CAN look up Alice (his granter) — shared-grant relationship # lets the external recipient resolve the sharer's display name + @@ -244,14 +247,37 @@ HTTP 200 [Asserts] jsonpath "$.id" == "{{alice_user_id}}" jsonpath "$.is_external" == false -# Setup admin is auto-verified at creation. `setup_create_admin` stamps + +# 11c/d/verify — admin (alice) observes email_verified_at on both +# users via `GET /api/admin/users/{id}` — returns `FullUserDto` +# (public identity in `.user` + admin-visible extras at top level). +# +# `email_verified_at` lives on `FullUserDto` (admin+self-visible), +# not on `PublicUserDto` — peer views via `/api/users/{id}` never +# expose it. The admin single-user endpoint is the correct +# observation surface. See `docs/plan/userdto-refactor.md` for the +# three-layer split. +# +# Setup admin auto-verified rationale: `setup_create_admin` stamps # `email_verified_at = NOW()` — admin fiat counts as verification, -# matching the OIDC-JIT convention. Rationale: an operator running the -# first-run wizard is authoritative by construction (they set the -# password at the console on a fresh install). Without this, flipping +# matching the OIDC-JIT convention. An operator running the first-run +# wizard is authoritative by construction. Without this, flipping # `OXICLOUD_REQUIRE_VERIFIED_EMAIL=true` on an existing deployment -# would lock the sole admin out of their own instance. The admin login -# exemption is a second layer of defense; this stamp is the primary. +# would lock the sole admin out of their own instance. +GET {{base_url}}/api/admin/users/{{bob_user_id}} +Authorization: Bearer {{alice_token}} + +HTTP 200 +[Asserts] +jsonpath "$.user.id" == "{{bob_user_id}}" +jsonpath "$.email_verified_at" exists + +GET {{base_url}}/api/admin/users/{{alice_user_id}} +Authorization: Bearer {{alice_token}} + +HTTP 200 +[Asserts] +jsonpath "$.user.id" == "{{alice_user_id}}" jsonpath "$.email_verified_at" exists # 11e — bob CANNOT enumerate unrelated users. A random UUID returns 404 diff --git a/tests/api/files-folders.hurl b/tests/api/files-folders.hurl index 13505130..d1dcaf26 100644 --- a/tests/api/files-folders.hurl +++ b/tests/api/files-folders.hurl @@ -19,11 +19,11 @@ Content-Type: application/json HTTP 200 [Captures] token: jsonpath "$.access_token" -admin_user_id: jsonpath "$.user.id" +admin_user_id: jsonpath "$.user.full.user.id" [Asserts] jsonpath "$.access_token" isString jsonpath "$.token_type" == "Bearer" -jsonpath "$.user.id" isString +jsonpath "$.user.full.user.id" isString # ───────────────────────────────────────────────────────────── diff --git a/tests/api/grant_cleanup.hurl b/tests/api/grant_cleanup.hurl index 01d211e2..4353bd1a 100644 --- a/tests/api/grant_cleanup.hurl +++ b/tests/api/grant_cleanup.hurl @@ -29,7 +29,7 @@ Content-Type: application/json HTTP 200 [Captures] alice_token: jsonpath "$.access_token" -alice_user_id: jsonpath "$.user.id" +alice_user_id: jsonpath "$.user.full.user.id" GET {{base_url}}/api/folders @@ -57,7 +57,7 @@ Content-Type: application/json HTTP 201 [Captures] -mallory_user_id: jsonpath "$.id" +mallory_user_id: jsonpath "$.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/grants.hurl b/tests/api/grants.hurl index b02d4d58..6cb3ddcc 100644 --- a/tests/api/grants.hurl +++ b/tests/api/grants.hurl @@ -23,7 +23,7 @@ Content-Type: application/json HTTP 200 [Captures] alice_token: jsonpath "$.access_token" -alice_user_id: jsonpath "$.user.id" +alice_user_id: jsonpath "$.user.full.user.id" GET {{base_url}}/api/folders Authorization: Bearer {{alice_token}} @@ -45,7 +45,7 @@ Content-Type: application/json HTTP 201 [Captures] -dave_user_id: jsonpath "$.id" +dave_user_id: jsonpath "$.user.id" POST {{base_url}}/api/admin/users Authorization: Bearer {{alice_token}} @@ -54,7 +54,7 @@ Content-Type: application/json HTTP 201 [Captures] -eve_user_id: jsonpath "$.id" +eve_user_id: jsonpath "$.user.id" # ───────────────────────────────────────────────────────────── @@ -371,7 +371,7 @@ Content-Type: application/json HTTP 201 [Captures] -adam_user_id: jsonpath "$.id" +adam_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login Content-Type: application/json @@ -1044,7 +1044,7 @@ Content-Type: application/json HTTP 201 [Captures] -frank_user_id: jsonpath "$.id" +frank_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login Content-Type: application/json diff --git a/tests/api/grants_nested_groups.hurl b/tests/api/grants_nested_groups.hurl index 480edbb3..579d234d 100644 --- a/tests/api/grants_nested_groups.hurl +++ b/tests/api/grants_nested_groups.hurl @@ -44,7 +44,7 @@ Content-Type: application/json HTTP 201 [Captures] -henry_user_id: jsonpath "$.id" +henry_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login Content-Type: application/json diff --git a/tests/api/groups_effective_members.hurl b/tests/api/groups_effective_members.hurl index 21f3dcb1..078f1a97 100644 --- a/tests/api/groups_effective_members.hurl +++ b/tests/api/groups_effective_members.hurl @@ -76,7 +76,7 @@ Content-Type: application/json HTTP 201 [Captures] -dora_id: jsonpath "$.id" +dora_id: jsonpath "$.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/nc_login_flow_v2_drive_picker.hurl b/tests/api/nc_login_flow_v2_drive_picker.hurl index a3e6bc94..0fcd7d1f 100644 --- a/tests/api/nc_login_flow_v2_drive_picker.hurl +++ b/tests/api/nc_login_flow_v2_drive_picker.hurl @@ -48,7 +48,7 @@ Content-Type: application/json HTTP 200 [Captures] admin_token: jsonpath "$.access_token" -admin_user_id: jsonpath "$.user.id" +admin_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/nc_multidrive_move_regression.hurl b/tests/api/nc_multidrive_move_regression.hurl index 9c826370..3868ff91 100644 --- a/tests/api/nc_multidrive_move_regression.hurl +++ b/tests/api/nc_multidrive_move_regression.hurl @@ -45,7 +45,7 @@ Content-Type: application/json HTTP 200 [Captures] jwt: jsonpath "$.access_token" -admin_user_id: jsonpath "$.user.id" +admin_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/nc_second_user_setup.hurl b/tests/api/nc_second_user_setup.hurl index 05fd69f6..d1f5a88e 100644 --- a/tests/api/nc_second_user_setup.hurl +++ b/tests/api/nc_second_user_setup.hurl @@ -50,5 +50,5 @@ Content-Type: application/json HTTP 200 [Asserts] jsonpath "$.access_token" exists -jsonpath "$.user.username" == "bob" -jsonpath "$.user.email" == "bob@example.com" +jsonpath "$.user.full.user.username" == "bob" +jsonpath "$.user.full.user.email" == "bob@example.com" diff --git a/tests/api/nc_webdav_patch_consistency.hurl b/tests/api/nc_webdav_patch_consistency.hurl index e23f1d78..cc108e67 100644 --- a/tests/api/nc_webdav_patch_consistency.hurl +++ b/tests/api/nc_webdav_patch_consistency.hurl @@ -45,7 +45,7 @@ Content-Type: application/json HTTP 200 [Captures] admin_jwt: jsonpath "$.access_token" -admin_user_id: jsonpath "$.user.id" +admin_user_id: jsonpath "$.user.full.user.id" # ═════════════════════════════════════════════════════════════ @@ -71,7 +71,7 @@ Content-Type: application/json HTTP 201 [Captures] -editor_user_id: jsonpath "$.id" +editor_user_id: jsonpath "$.user.id" POST {{base_url}}/api/admin/users Authorization: Bearer {{admin_jwt}} @@ -85,7 +85,7 @@ Content-Type: application/json HTTP 201 [Captures] -viewer_user_id: jsonpath "$.id" +viewer_user_id: jsonpath "$.user.id" POST {{base_url}}/api/admin/users Authorization: Bearer {{admin_jwt}} @@ -99,7 +99,7 @@ Content-Type: application/json HTTP 201 [Captures] -outsider_user_id: jsonpath "$.id" +outsider_user_id: jsonpath "$.user.id" # ───────────────────────────────────────────────────────────── @@ -434,7 +434,7 @@ Content-Type: application/json HTTP 201 [Captures] -quota_owner_id: jsonpath "$.id" +quota_owner_id: jsonpath "$.user.id" PUT {{base_url}}/api/admin/users/{{quota_owner_id}}/quota diff --git a/tests/api/nc_webdav_put_gaps.hurl b/tests/api/nc_webdav_put_gaps.hurl index d6d0657c..f8829b30 100644 --- a/tests/api/nc_webdav_put_gaps.hurl +++ b/tests/api/nc_webdav_put_gaps.hurl @@ -40,7 +40,7 @@ Content-Type: application/json HTTP 200 [Captures] admin_jwt: jsonpath "$.access_token" -admin_user_id: jsonpath "$.user.id" +admin_user_id: jsonpath "$.user.full.user.id" # ═════════════════════════════════════════════════════════════ @@ -65,7 +65,7 @@ Content-Type: application/json HTTP 201 [Captures] -editor_user_id: jsonpath "$.id" +editor_user_id: jsonpath "$.user.id" POST {{base_url}}/api/admin/users Authorization: Bearer {{admin_jwt}} @@ -79,7 +79,7 @@ Content-Type: application/json HTTP 201 [Captures] -viewer_user_id: jsonpath "$.id" +viewer_user_id: jsonpath "$.user.id" # ───────────────────────────────────────────────────────────── @@ -351,7 +351,7 @@ Content-Type: application/json HTTP 201 [Captures] -quota_owner_id: jsonpath "$.id" +quota_owner_id: jsonpath "$.user.id" PUT {{base_url}}/api/admin/users/{{quota_owner_id}}/quota diff --git a/tests/api/nc_webdav_quota_properties.hurl b/tests/api/nc_webdav_quota_properties.hurl index 99775a07..6db91de5 100644 --- a/tests/api/nc_webdav_quota_properties.hurl +++ b/tests/api/nc_webdav_quota_properties.hurl @@ -85,7 +85,7 @@ Content-Type: application/json HTTP 200 [Captures] ncq_owner_jwt: jsonpath "$.access_token" -ncq_owner_id: jsonpath "$.user.id" +ncq_owner_id: jsonpath "$.user.full.user.id" POST {{base_url}}/api/auth/app-passwords Authorization: Bearer {{ncq_owner_jwt}} diff --git a/tests/api/playlists.hurl b/tests/api/playlists.hurl index d8956818..664f9345 100644 --- a/tests/api/playlists.hurl +++ b/tests/api/playlists.hurl @@ -45,7 +45,7 @@ Content-Type: application/json HTTP 200 [Captures] alice_token: jsonpath "$.access_token" -alice_user_id: jsonpath "$.user.id" +alice_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── @@ -126,7 +126,7 @@ Content-Type: application/json HTTP 200 [Captures] bob_token: jsonpath "$.access_token" -bob_user_id: jsonpath "$.user.id" +bob_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/registration.hurl b/tests/api/registration.hurl index c50a2c37..1a78984a 100644 --- a/tests/api/registration.hurl +++ b/tests/api/registration.hurl @@ -55,7 +55,7 @@ Content-Type: application/json HTTP 200 [Captures] charlie_token: jsonpath "$.access_token" -charlie_user_id: jsonpath "$.user.id" +charlie_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── @@ -139,16 +139,16 @@ Authorization: Bearer {{pr18_access_token}} HTTP 200 [Asserts] -jsonpath "$.email" == "pr18-emailonly@example.com" -jsonpath "$.is_external" == false -jsonpath "$.username" not exists +jsonpath "$.full.user.email" == "pr18-emailonly@example.com" +jsonpath "$.full.user.is_external" == false +jsonpath "$.full.user.username" not exists # PR 23 — the user redeemed the welcome magic-link in Step 5b, so # email_verified_at is stamped (the click IS the proof of inbox # control, regardless of whether the redemption went through the # direct or cross-browser-confirm path). -jsonpath "$.email_verified_at" exists +jsonpath "$.full.email_verified_at" exists [Captures] -pr18_user_id: jsonpath "$.id" +pr18_user_id: jsonpath "$.full.user.id" # ───────────────────────────────────────────────────────────── @@ -162,9 +162,9 @@ Content-Type: application/json HTTP 200 [Asserts] -jsonpath "$.id" == "{{pr18_user_id}}" -jsonpath "$.username" not exists -jsonpath "$.given_name" not exists +jsonpath "$.full.user.id" == "{{pr18_user_id}}" +jsonpath "$.full.user.username" not exists +jsonpath "$.full.user.given_name" not exists # ───────────────────────────────────────────────────────────── @@ -178,9 +178,9 @@ Content-Type: application/json HTTP 200 [Asserts] -jsonpath "$.given_name" == "Pee Are" -jsonpath "$.family_name" == "Eighteen" -jsonpath "$.username" not exists +jsonpath "$.full.user.given_name" == "Pee Are" +jsonpath "$.full.user.family_name" == "Eighteen" +jsonpath "$.full.user.username" not exists # ───────────────────────────────────────────────────────────── @@ -220,7 +220,7 @@ Content-Type: application/json HTTP 200 [Asserts] -jsonpath "$.username" == "pr18handle" +jsonpath "$.full.user.username" == "pr18handle" # ───────────────────────────────────────────────────────────── @@ -263,7 +263,7 @@ Content-Type: application/json HTTP 200 [Asserts] -jsonpath "$.given_name" == "Pr18@Handle" +jsonpath "$.full.user.given_name" == "Pr18@Handle" # ───────────────────────────────────────────────────────────── @@ -276,10 +276,10 @@ Authorization: Bearer {{pr18_access_token}} HTTP 200 [Asserts] -jsonpath "$.username" == "pr18handle" -jsonpath "$.given_name" == "Pr18@Handle" -jsonpath "$.family_name" == "Eighteen" -jsonpath "$.email_verified_at" exists +jsonpath "$.full.user.username" == "pr18handle" +jsonpath "$.full.user.given_name" == "Pr18@Handle" +jsonpath "$.full.user.family_name" == "Eighteen" +jsonpath "$.full.email_verified_at" exists # ───────────────────────────────────────────────────────────── diff --git a/tests/api/regression_595_unlimited_user_quota.hurl b/tests/api/regression_595_unlimited_user_quota.hurl index 928f75f1..f7a7fb35 100644 --- a/tests/api/regression_595_unlimited_user_quota.hurl +++ b/tests/api/regression_595_unlimited_user_quota.hurl @@ -80,7 +80,7 @@ Content-Type: application/json HTTP 200 [Captures] user_token: jsonpath "$.access_token" -user_user_id: jsonpath "$.user.id" +user_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/api/role_grants.hurl b/tests/api/role_grants.hurl index e5a218e1..b7742d8f 100644 --- a/tests/api/role_grants.hurl +++ b/tests/api/role_grants.hurl @@ -34,7 +34,7 @@ Content-Type: application/json HTTP 200 [Captures] admin_token: jsonpath "$.access_token" -admin_user_id: jsonpath "$.user.id" +admin_user_id: jsonpath "$.user.full.user.id" GET {{base_url}}/api/folders Authorization: Bearer {{admin_token}} @@ -56,7 +56,7 @@ Content-Type: application/json HTTP 201 [Captures] -renee_user_id: jsonpath "$.id" +renee_user_id: jsonpath "$.user.id" POST {{base_url}}/api/admin/users @@ -66,7 +66,7 @@ Content-Type: application/json HTTP 201 [Captures] -sam_user_id: jsonpath "$.id" +sam_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login diff --git a/tests/api/storage_cleanup_check.sh b/tests/api/storage_cleanup_check.sh index 36f55219..ec416797 100755 --- a/tests/api/storage_cleanup_check.sh +++ b/tests/api/storage_cleanup_check.sh @@ -75,15 +75,18 @@ log "Probe blob and thumbnail confirmed present on disk." # subsequent trash-empty triggers garbage_collect() to remove the # now-orphaned blob files from disk. -# /api/admin/users returns { users: [...], total, limit, offset } +# /api/admin/users returns { users: [FullUserDto…], total, limit, offset } +# — public identity nests under `.user`; admin-visible extras +# (`storage_used_bytes`, `last_login_at`, …) sit at the top level of +# each row. See `docs/plan/userdto-refactor.md`. USERS_JSON=$(curl -sf -H "$AUTH" "$base_url/api/admin/users?limit=500") ADMIN_USER_ID=$(echo "$USERS_JSON" \ - | jq -r --arg u "$username" '.users[] | select(.username == $u) | .id') + | jq -r --arg u "$username" '.users[] | select(.user.username == $u) | .user.id') [[ -z "$ADMIN_USER_ID" || "$ADMIN_USER_ID" == "null" ]] && fail "could not resolve admin user id" OTHER_USER_IDS=$(echo "$USERS_JSON" \ - | jq -r --arg admin_id "$ADMIN_USER_ID" '.users[] | select(.id != $admin_id) | .id') + | jq -r --arg admin_id "$ADMIN_USER_ID" '.users[] | select(.user.id != $admin_id) | .user.id') OTHER_USER_COUNT=0 while IFS= read -r uid; do diff --git a/tests/api/subject_groups.hurl b/tests/api/subject_groups.hurl index b0c2ca90..70b3dc68 100644 --- a/tests/api/subject_groups.hurl +++ b/tests/api/subject_groups.hurl @@ -35,7 +35,7 @@ Content-Type: application/json HTTP 201 [Captures] -grace_user_id: jsonpath "$.id" +grace_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login Content-Type: application/json @@ -268,7 +268,7 @@ Content-Type: application/json HTTP 201 [Captures] -helper_user_id: jsonpath "$.id" +helper_user_id: jsonpath "$.user.id" POST {{base_url}}/api/groups/{{engineers_id}}/members diff --git a/tests/api/trash_per_drive.hurl b/tests/api/trash_per_drive.hurl index 4b0fbdeb..5011ed6c 100644 --- a/tests/api/trash_per_drive.hurl +++ b/tests/api/trash_per_drive.hurl @@ -51,7 +51,7 @@ Content-Type: application/json HTTP 201 [Captures] -owner_user_id: jsonpath "$.id" +owner_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login Content-Type: application/json @@ -209,7 +209,7 @@ Content-Type: application/json HTTP 201 [Captures] -viewer_user_id: jsonpath "$.id" +viewer_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login Content-Type: application/json diff --git a/tests/api/user_envelope_quota.hurl b/tests/api/user_envelope_quota.hurl index ea141db0..7ab4bc4b 100644 --- a/tests/api/user_envelope_quota.hurl +++ b/tests/api/user_envelope_quota.hurl @@ -64,7 +64,7 @@ Content-Type: application/json HTTP 200 [Captures] owner_token: jsonpath "$.access_token" -owner_user_id: jsonpath "$.user.id" +owner_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── @@ -92,7 +92,7 @@ Authorization: Bearer {{owner_token}} HTTP 200 [Asserts] -jsonpath "$.storage_used_bytes" == 0 +jsonpath "$.full.storage_used_bytes" == 0 # ───────────────────────────────────────────────────────────── @@ -173,7 +173,7 @@ Authorization: Bearer {{owner_token}} HTTP 200 [Asserts] -jsonpath "$.storage_used_bytes" == 0 +jsonpath "$.full.storage_used_bytes" == 0 # ───────────────────────────────────────────────────────────── @@ -202,7 +202,7 @@ retry-interval: 200ms HTTP 200 [Asserts] -jsonpath "$.storage_used_bytes" == 32 +jsonpath "$.full.storage_used_bytes" == 32 # Confirm the sweep agrees with the delta — both code paths must @@ -217,7 +217,7 @@ Authorization: Bearer {{owner_token}} HTTP 200 [Asserts] -jsonpath "$.storage_used_bytes" == 32 +jsonpath "$.full.storage_used_bytes" == 32 # ───────────────────────────────────────────────────────────── @@ -247,7 +247,7 @@ Authorization: Bearer {{owner_token}} HTTP 200 [Asserts] -jsonpath "$.storage_used_bytes" == 0 +jsonpath "$.full.storage_used_bytes" == 0 # ───────────────────────────────────────────────────────────── diff --git a/tests/api/webdav_patch_consistency.hurl b/tests/api/webdav_patch_consistency.hurl index e3842c87..399b5819 100644 --- a/tests/api/webdav_patch_consistency.hurl +++ b/tests/api/webdav_patch_consistency.hurl @@ -174,7 +174,7 @@ Content-Type: application/json HTTP 201 [Captures] -quota_owner_id: jsonpath "$.id" +quota_owner_id: jsonpath "$.user.id" PUT {{base_url}}/api/admin/users/{{quota_owner_id}}/quota diff --git a/tests/api/webdav_permissions.hurl b/tests/api/webdav_permissions.hurl index 57cf811e..558ca16a 100644 --- a/tests/api/webdav_permissions.hurl +++ b/tests/api/webdav_permissions.hurl @@ -35,7 +35,7 @@ Content-Type: application/json HTTP 200 [Captures] admin_token: jsonpath "$.access_token" -admin_user_id: jsonpath "$.user.id" +admin_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── @@ -54,7 +54,7 @@ Content-Type: application/json HTTP 201 [Captures] -bob_user_id: jsonpath "$.id" +bob_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login diff --git a/tests/api/webdav_quota_properties.hurl b/tests/api/webdav_quota_properties.hurl index d092578f..1b3381f1 100644 --- a/tests/api/webdav_quota_properties.hurl +++ b/tests/api/webdav_quota_properties.hurl @@ -151,7 +151,7 @@ Content-Type: application/json HTTP 200 [Captures] wq_owner_token: jsonpath "$.access_token" -wq_owner_id: jsonpath "$.user.id" +wq_owner_id: jsonpath "$.user.full.user.id" POST {{base_url}}/api/drives diff --git a/tests/api/wopi_authz.hurl b/tests/api/wopi_authz.hurl index 144e0df7..cf01f5e5 100644 --- a/tests/api/wopi_authz.hurl +++ b/tests/api/wopi_authz.hurl @@ -39,7 +39,7 @@ Content-Type: application/json HTTP 200 [Captures] alice_token: jsonpath "$.access_token" -alice_user_id: jsonpath "$.user.id" +alice_user_id: jsonpath "$.user.full.user.id" GET {{base_url}}/api/folders @@ -65,7 +65,7 @@ Content-Type: application/json HTTP 201 [Captures] -bob_user_id: jsonpath "$.id" +bob_user_id: jsonpath "$.user.id" POST {{base_url}}/api/auth/login diff --git a/tests/api/wopi_shared_drive.hurl b/tests/api/wopi_shared_drive.hurl index 11c7ab36..695e256d 100644 --- a/tests/api/wopi_shared_drive.hurl +++ b/tests/api/wopi_shared_drive.hurl @@ -47,7 +47,7 @@ Content-Type: application/json HTTP 200 [Captures] admin_token: jsonpath "$.access_token" -admin_user_id: jsonpath "$.user.id" +admin_user_id: jsonpath "$.user.full.user.id" # ───────────────────────────────────────────────────────────── diff --git a/tests/e2e/spa/files.spec.ts b/tests/e2e/spa/files.spec.ts index ef7e88f5..467a8140 100644 --- a/tests/e2e/spa/files.spec.ts +++ b/tests/e2e/spa/files.spec.ts @@ -74,7 +74,16 @@ test('upload a file via the hidden file input', async ({ page }) => { // Navigate straight into the (empty) folder by ID (the route keys on folder // id, not name) — avoids the crowded root listing and click ambiguity. await page.goto(`/files/${created.id}`); - await expect(page.getByTestId('files-upload-file-input')).toBeAttached({ timeout: 15_000 }); + // The hidden file input renders unconditionally on mount, so waiting on + // `toBeAttached` fires BEFORE the page's `load()` populates `currentId` + // from the URL. Firing `setInputFiles` in that window used to race + // `load()` and post the upload with `folderId: null`, silently landing + // the file in the caller's home root — the guard in + // `guardUploadFolderReady` now refuses that upload with a toast. Wait + // for the empty-state hook instead — `ResourceList` only renders + // `EmptyState` once `load()` has definitively completed with zero + // items, so it doubles as a "folder is ready to accept uploads" signal. + await expect(page.getByTestId('empty-state')).toBeVisible({ timeout: 15_000 }); // Now inside the folder; upload a text file by setting the hidden input. const f = SAMPLE_FILES.text(); await page.getByTestId('files-upload-file-input').setInputFiles({ diff --git a/tests/oidc/link_unlink.hurl b/tests/oidc/link_unlink.hurl index b8e8b3e6..958b7efe 100644 --- a/tests/oidc/link_unlink.hurl +++ b/tests/oidc/link_unlink.hurl @@ -98,11 +98,11 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.username" == "{{username}}" +jsonpath "$.full.user.username" == "{{username}}" # federation_kind is skip_serializing_if=Option::is_none, so a # local user's response OMITS the field entirely. -jsonpath "$.federation_kind" not exists -jsonpath "$.federation_issuer" not exists +jsonpath "$.full.federation_kind" not exists +jsonpath "$.full.federation_issuer" not exists # ───────────────────────────────────────────────────────────── @@ -202,8 +202,8 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.federation_kind" not exists -jsonpath "$.federation_issuer" not exists +jsonpath "$.full.federation_kind" not exists +jsonpath "$.full.federation_issuer" not exists # ═════════════════════════════════════════════════════════════ @@ -252,8 +252,8 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.federation_kind" not exists -jsonpath "$.federation_issuer" not exists +jsonpath "$.full.federation_kind" not exists +jsonpath "$.full.federation_issuer" not exists # ═════════════════════════════════════════════════════════════ @@ -307,9 +307,9 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.username" == "{{username}}" -jsonpath "$.federation_kind" == "oidc" -jsonpath "$.federation_issuer" == "{{oidc_issuer}}" +jsonpath "$.full.user.username" == "{{username}}" +jsonpath "$.full.federation_kind" == "oidc" +jsonpath "$.full.federation_issuer" == "{{oidc_issuer}}" # Scenario 9 — unlink success (admin has a password, so the @@ -326,8 +326,8 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.federation_kind" not exists -jsonpath "$.federation_issuer" not exists +jsonpath "$.full.federation_kind" not exists +jsonpath "$.full.federation_issuer" not exists # ═════════════════════════════════════════════════════════════ @@ -380,7 +380,7 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.federation_kind" == "oidc" +jsonpath "$.full.federation_kind" == "oidc" # Unlink to reset state before the auto-link scenarios. @@ -447,9 +447,9 @@ HTTP 200 [Asserts] # Auto-link resolved to the pre-existing admin, NOT a fresh # JIT-provisioned user. The load-bearing assertion. -jsonpath "$.user.username" == "{{username}}" -jsonpath "$.user.federation_kind" == "oidc" -jsonpath "$.user.federation_issuer" == "{{oidc_issuer}}" +jsonpath "$.user.full.user.username" == "{{username}}" +jsonpath "$.user.full.federation_kind" == "oidc" +jsonpath "$.user.full.federation_issuer" == "{{oidc_issuer}}" [Captures] # Fresh cookies replace the password session's; capture the # new CSRF for the unlink below. @@ -463,9 +463,9 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.username" == "{{username}}" -jsonpath "$.federation_kind" == "oidc" -jsonpath "$.federation_issuer" == "{{oidc_issuer}}" +jsonpath "$.full.user.username" == "{{username}}" +jsonpath "$.full.federation_kind" == "oidc" +jsonpath "$.full.federation_issuer" == "{{oidc_issuer}}" # Reset admin state before the next scenario (auto-link would @@ -535,7 +535,7 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.federation_kind" not exists +jsonpath "$.full.federation_kind" not exists # Reset fake IdP state (email_verified back to true, sub back @@ -599,7 +599,7 @@ X-CSRF-Token: {{autolink_csrf_token}} HTTP 201 [Captures] -alias_user_id: jsonpath "$.id" +alias_user_id: jsonpath "$.user.id" # Point the fake IdP at a fresh sub with admin's email. Both @@ -636,7 +636,7 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.federation_kind" not exists +jsonpath "$.full.federation_kind" not exists # Cleanup — delete the collider so later scenarios see the same @@ -701,8 +701,8 @@ Content-Type: application/json HTTP 200 [Asserts] -jsonpath "$.user.username" == "oidc_user" -jsonpath "$.user.federation_kind" == "oidc" +jsonpath "$.user.full.user.username" == "oidc_user" +jsonpath "$.user.full.federation_kind" == "oidc" [Captures] # Fresh CSRF from the OIDC session cookies — the admin CSRFs # won't validate against these new cookies. @@ -730,5 +730,5 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.federation_kind" == "oidc" -jsonpath "$.federation_issuer" == "{{oidc_issuer}}" +jsonpath "$.full.federation_kind" == "oidc" +jsonpath "$.full.federation_issuer" == "{{oidc_issuer}}" diff --git a/tests/oidc/oidc.hurl b/tests/oidc/oidc.hurl index 026523fb..2aa1802d 100644 --- a/tests/oidc/oidc.hurl +++ b/tests/oidc/oidc.hurl @@ -172,7 +172,7 @@ Content-Type: application/json HTTP 200 [Captures] -oidc_session_user: jsonpath "$.user.username" +oidc_session_user: jsonpath "$.user.full.user.username" # Snapshotted so Step 7's refresh can prove the tokens rotated # rather than being re-issued unchanged. The refresh handler in # auth_handler.rs always rotates all three cookies (access JWT, @@ -184,8 +184,8 @@ initial_access_token: jsonpath "$.access_token" initial_refresh_token: jsonpath "$.refresh_token" initial_csrf_token: cookie "oxicloud_csrf" [Asserts] -jsonpath "$.user.username" == "oidc_user" -jsonpath "$.user.email" == "oidc@example.com" +jsonpath "$.user.full.user.username" == "oidc_user" +jsonpath "$.user.full.user.email" == "oidc@example.com" jsonpath "$.access_token" isString # Multiple Set-Cookie headers come back as a list of values, so # `contains` only matches whole-element strings. Each cookie shows up @@ -211,10 +211,10 @@ HTTP 200 # Stash the user id for the re-login check in Step 10 below — a # second OIDC flow with the same `sub` must resolve back to this # exact user, not silently create a duplicate. -oidc_user_id: jsonpath "$.id" +oidc_user_id: jsonpath "$.full.user.id" [Asserts] -jsonpath "$.username" == "oidc_user" -jsonpath "$.email" == "oidc@example.com" +jsonpath "$.full.user.username" == "oidc_user" +jsonpath "$.full.user.email" == "oidc@example.com" # Post the federation-identity rename (docs/plan/ocm.md § Schema # rename) UserDto exposes federation_kind + federation_issuer as # separate nullable fields. Local users have both null; OIDC users @@ -222,24 +222,24 @@ jsonpath "$.email" == "oidc@example.com" # the fake IdP (tests/oidc/fake_idp/server.js) that URL is the # issuer published in its discovery document, which matches # `oidc_issuer` from test.env. -jsonpath "$.federation_kind" == "oidc" -jsonpath "$.federation_issuer" == "{{oidc_issuer}}" +jsonpath "$.full.federation_kind" == "oidc" +jsonpath "$.full.federation_issuer" == "{{oidc_issuer}}" # Full claim round-trip — the fake IdP (tests/oidc/fake_idp/server.js) # pins these values and OxiCloud must persist each one verbatim during # JIT provisioning (see auth_application_service.rs around line 2257). # A regression that drops, swaps, or truncates a claim trips here. # Note the field name flip on the API side: OIDC `picture` becomes # UserDto.image (a URL or data URI). -jsonpath "$.given_name" == "OIDC" -jsonpath "$.family_name" == "Test" -jsonpath "$.image" == "https://example.com/oidc-test-user.png" +jsonpath "$.full.user.given_name" == "OIDC" +jsonpath "$.full.user.family_name" == "Test" +jsonpath "$.full.user.image" == "https://example.com/oidc-test-user.png" # Group-to-role mapping. server-with-oidc.env sets # OXICLOUD_OIDC_ADMIN_GROUPS=admin-users; the fake IdP's claims include # `groups: ["admin-users"]`. The JIT path intersects the claim against # the env and promotes the new user from `user` to `admin`. A # regression here would silently strip (or wrongly grant) admin rights # for every SSO deployment that uses group-based role mapping. -jsonpath "$.role" == "admin" +jsonpath "$.full.user.role" == "admin" # ───────────────────────────────────────────────────────────── @@ -274,7 +274,7 @@ refreshed_refresh_token: jsonpath "$.refresh_token" # the (freshly-rotated) `oxicloud_csrf` cookie on the browser. refreshed_csrf_token: cookie "oxicloud_csrf" [Asserts] -jsonpath "$.user.username" == "oidc_user" +jsonpath "$.user.full.user.username" == "oidc_user" jsonpath "$.access_token" isString jsonpath "$.refresh_token" isString # All three cookies must rotate. If any value were re-used, a @@ -296,7 +296,7 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.username" == "oidc_user" +jsonpath "$.full.user.username" == "oidc_user" # ───────────────────────────────────────────────────────────── @@ -412,8 +412,8 @@ HTTP 200 [Asserts] # Same local id — proves the existing-user resolver matched on `sub` # (or `oidc_provider + oidc_subject`) instead of minting a new row. -jsonpath "$.user.id" == "{{oidc_user_id}}" -jsonpath "$.user.username" == "oidc_user" +jsonpath "$.user.full.user.id" == "{{oidc_user_id}}" +jsonpath "$.user.full.user.username" == "oidc_user" # Role from the prior JIT-provisioned admin survives the re-login. # Two regressions this catches: (a) the existing-user branch wiping # the role to a default `user`; (b) the existing-user branch @@ -421,7 +421,7 @@ jsonpath "$.user.username" == "oidc_user" # IdP still emits `groups: ["admin-users"]`, OXICLOUD_OIDC_ADMIN_GROUPS # still resolves to "admin"). Either way, the role should remain # `admin` — otherwise we have a silent admin demotion on every login. -jsonpath "$.user.role" == "admin" +jsonpath "$.user.full.user.role" == "admin" # ───────────────────────────────────────────────────────────── @@ -876,7 +876,7 @@ Content-Type: application/json HTTP 200 [Asserts] -jsonpath "$.user.username" == "oidc_user" +jsonpath "$.user.full.user.username" == "oidc_user" # ───────────────────────────────────────────────────────────── @@ -889,7 +889,7 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.username" == "oidc_user" +jsonpath "$.full.user.username" == "oidc_user" # ───────────────────────────────────────────────────────────── diff --git a/tests/oidc/sso-only.hurl b/tests/oidc/sso-only.hurl index 87a33c0b..4f0b2230 100644 --- a/tests/oidc/sso-only.hurl +++ b/tests/oidc/sso-only.hurl @@ -123,10 +123,10 @@ Content-Type: application/json HTTP 200 [Asserts] -jsonpath "$.user.username" == "oidc_user" +jsonpath "$.user.full.user.username" == "oidc_user" # Group-to-role mapping worked — this is now the admin (and the only # user). -jsonpath "$.user.role" == "admin" +jsonpath "$.user.full.user.role" == "admin" # ───────────────────────────────────────────────────────────── @@ -138,7 +138,7 @@ GET {{base_url}}/api/auth/me HTTP 200 [Asserts] -jsonpath "$.username" == "oidc_user" +jsonpath "$.full.user.username" == "oidc_user" # ───────────────────────────────────────────────────────────── diff --git a/tests/webdav/test_nextcloud_chunked_upload_cap.sh b/tests/webdav/test_nextcloud_chunked_upload_cap.sh index a2e4dd96..0ed426ef 100755 --- a/tests/webdav/test_nextcloud_chunked_upload_cap.sh +++ b/tests/webdav/test_nextcloud_chunked_upload_cap.sh @@ -132,9 +132,9 @@ echo " app password minted (id=$APP_PASSWORD_ID)" # failure path. `storage_quota_bytes == 0` is the unlimited # sentinel (see `check_storage_quota`); we read it back here in # case a prior test set a real value. -ADMIN_ID=$(rest_get "/api/auth/me" | jq -r '.id') +ADMIN_ID=$(rest_get "/api/auth/me" | jq -r '.full.user.id') [[ -n "$ADMIN_ID" && "$ADMIN_ID" != "null" ]] || fail "Failed to read admin user id" -ORIGINAL_ADMIN_QUOTA=$(rest_get "/api/auth/me" | jq -r '.storage_quota_bytes // 0') +ORIGINAL_ADMIN_QUOTA=$(rest_get "/api/auth/me" | jq -r '.full.storage_quota_bytes // 0') # Single cleanup on exit: # - restore admin's original storage envelope (in case Case 3 @@ -247,7 +247,7 @@ echo "[3/3] QUOTA REJECTION — envelope tightened to (used + 100 B), PUT 200 B # `current + 100` — leaves enough headroom that MKCOL passes # (`used + 0 = used < used + 100`) while a 200 B chunk PUT # overflows by exactly 100 (`used + 0 + 200 > used + 100`). -CURRENT_USED=$(rest_get "/api/auth/me" | jq -r '.storage_used_bytes') +CURRENT_USED=$(rest_get "/api/auth/me" | jq -r '.full.storage_used_bytes') [[ -n "$CURRENT_USED" && "$CURRENT_USED" != "null" ]] || fail "Failed to read current used_bytes" TIGHT_QUOTA=$(( CURRENT_USED + 100 ))