From 44781643ef074aa24727b91e3c03d3b7e9c0a480 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 1 Jun 2026 15:04:27 +0200 Subject: [PATCH 1/7] docs: add plan on User Lifecycle --- docs/plan/User-Lifecycle.md | 358 ++++++++++++++++++++++++++++++++++++ 1 file changed, 358 insertions(+) create mode 100644 docs/plan/User-Lifecycle.md diff --git a/docs/plan/User-Lifecycle.md b/docs/plan/User-Lifecycle.md new file mode 100644 index 00000000..8002e0cb --- /dev/null +++ b/docs/plan/User-Lifecycle.md @@ -0,0 +1,358 @@ +# Plan — `UserLifecycleHook` + `is_external` flag + +## Context + +Today four code paths in `auth_application_service.rs` each call `create_personal_folder()` immediately after inserting an `auth.users` row: public `register`, `setup_create_admin`, admin `create_user`, and OIDC JIT (lines 283, 360, 832, 1277). A fifth self-heal at `folder_service.rs:350-365` retries home-folder creation when listing root folders returns empty. Five places, one concern, no shared abstraction — and adding a future service (calendar, address book, GPG keyring, external-user provenance for the upcoming magic-link feature) would have to touch all five again. + +Separately, the upcoming "share with `external@example.com`" feature needs `auth.users` to distinguish recipients-with-no-storage from real internal users. The codebase already declares `Subject::External` (`domain/services/authorization.rs`) but no DB representation exists yet. + +This plan introduces a `UserLifecycleHook` trait (mirroring the existing `FileLifecycleHook` / `BlobLifecycleHook` pattern at `application/ports/file_lifecycle.rs`), wires a dispatcher into the four lifecycle events, migrates the scattered eager work into services that own their own lifecycle (each implementing the trait with explicit no-ops for events they don't care about), and adds the `is_external` boolean to `auth.users` so hooks can short-circuit for non-internal users. The change is purely a refactor at first — behaviour is preserved — but it sets up the v2 external-user flow to land as a hook impl rather than a new auth code path. + +## Design + +### Trait shape + +The trait diverges from `FileLifecycleHook`'s sync fire-and-forget model on purpose: file events fire on every upload (hot path, fire-and-forget appropriate); user events are rare (login is seconds-per-user, not requests-per-second) and some require synchronous semantics (provisioning must finish before the session token is returned; deletion cleanup must commit atomically with the user DELETE). The trait is async; the dispatcher decides per-event whether errors abort the flow. + +```rust +// src/application/ports/user_lifecycle.rs (new) +#[async_trait] +pub trait UserLifecycleHook: Send + Sync { + /// Short identifier used in tracing / error logs. e.g. "home_folder". + fn name(&self) -> &'static str; + + /// Fires once after INSERT into auth.users succeeds, regardless of path. + async fn on_user_created(&self, user: &User) -> Result<(), DomainError>; + + /// Fires after every successful authentication, before the session + /// token is returned. MUST be idempotent (safety net for services + /// added after the user existed). + async fn on_user_login(&self, user: &User) -> Result<(), DomainError>; + + /// Fires on every session termination. `reason` lets hooks + /// distinguish causes (audit cares; cache invalidation does not). + async fn on_user_logout(&self, user: &User, reason: LogoutReason) + -> Result<(), DomainError>; + + /// Fires inside the same transaction as the auth.users DELETE. + /// Returning Err aborts the deletion. `mode` distinguishes admin + /// delete (policy-driven cleanup) from GDPR purge (force everything). + async fn on_user_deleted( + &self, + user: &User, + mode: DeletionMode, + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + ) -> Result<(), DomainError>; +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum LogoutReason { + UserInitiated, // explicit logout + SessionExpired, // TTL hit + AdminRevoked, // single-session revocation by admin + AccountDisabled, // user.active flipped to FALSE → all sessions revoked + PasswordChanged, // sibling sessions invalidated by a password change + TokenReused, // session-family reuse detection (existing feature) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DeletionMode { AdminDelete, GdprPurge } +``` + +**No default impls.** Every hook must declare all four methods. Use explicit `Ok(())` for events you don't care about — matches the FileLifecycleHook convention and forces conscious acknowledgement. + +### Dispatcher + +```rust +// src/application/services/user_lifecycle_service.rs (new) +pub struct UserLifecycleService { + hooks: Vec>, +} + +impl UserLifecycleService { + pub fn new() -> Self { Self { hooks: Vec::new() } } + pub fn with_hook(mut self, hook: Arc) -> Self { + self.hooks.push(hook); self + } + + // Per-event dispatchers with event-specific failure semantics: + + /// Created: log-and-continue. Next login's `on_user_login` retries + /// idempotently if anything fails here. + pub async fn dispatch_created(&self, user: &User) { + for h in &self.hooks { + if let Err(e) = h.on_user_created(user).await { + tracing::error!(target: "user_lifecycle", + hook = h.name(), user_id = %user.id(), error = %e, + "on_user_created failed; will retry on next login"); + } + } + } + + /// Login: log-and-continue. Same reasoning. + pub async fn dispatch_login(&self, user: &User) { /* same shape */ } + + /// Logout: fire-and-forget (spawned), errors logged. The HTTP + /// response shouldn't wait for cache flushes. + pub fn dispatch_logout(&self, user: User, reason: LogoutReason) { + let hooks = self.hooks.clone(); + tokio::spawn(async move { + for h in &hooks { + if let Err(e) = h.on_user_logout(&user, reason).await { + tracing::error!(target: "user_lifecycle", + hook = h.name(), reason = ?reason, + user_id = %user.id(), error = %e, + "on_user_logout failed"); + } + } + }); + } + + /// Deleted: propagate first Err to abort the transaction. + pub async fn dispatch_deleted( + &self, + user: &User, + mode: DeletionMode, + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + ) -> Result<(), DomainError> { + for h in &self.hooks { + h.on_user_deleted(user, mode, tx).await?; + } + Ok(()) + } +} +``` + +### `is_external` flag (additive migration) + +New migration `migrations/20260612000002_auth_users_is_external.sql`: + +```sql +-- Adds the is_external flag distinguishing storage-owning internal users +-- from grant-only external users (magic-link / OIDC-only / future OCM). +ALTER TABLE auth.users + ADD COLUMN IF NOT EXISTS is_external BOOLEAN NOT NULL DEFAULT FALSE; + +-- Partial index — most queries are "list internal users" or "list +-- external users for GDPR purge", never an unfiltered scan. +CREATE INDEX IF NOT EXISTS idx_users_is_external_login + ON auth.users (is_external, last_login_at) + WHERE is_external = TRUE; + +-- Guard against accidental storage attribution to external users. +ALTER TABLE auth.users + ADD CONSTRAINT users_external_no_storage + CHECK (NOT is_external OR storage_used_bytes = 0); +``` + +User entity (`src/domain/entities/user.rs`): +- Add `is_external: bool` field +- Add getter `pub fn is_external(&self) -> bool` +- Add factory `User::new_external(username, email, ...)` for the magic-link flow +- Existing factories (`User::new(...)`) default `is_external = false` + +The `Subject::External(uuid)` variant in `domain/services/authorization.rs` becomes redundant once external users live in `auth.users` and are addressed as `Subject::User(uuid)`. Deprecate it in a follow-up — out of scope here to avoid scope creep. + +### Concrete hook implementations + +**Each hook impl lives in the module of the service that owns the work**, matching the existing convention (`ThumbnailRefreshHook` lives in `src/infrastructure/services/thumbnail_service.rs`; `AudioMetadataService impl FileLifecycleHook` lives in `audio_metadata_service.rs`). There is **no centralised `lifecycle/` directory** — that would invert ownership and make "lifecycle" look like the owner of folder-creation policy when really the folder service owns it. + +All four trait methods are explicit per impl; no-ops are `Ok(())` one-liners. + +| Hook | Lives in | Responsibility | +|---|---|---| +| `HomeFolderLifecycleHook` | `src/application/services/folder_service.rs` (same module as `FolderService`) | Replaces the 4 eager `create_personal_folder` calls + the self-heal. `on_user_created` & `on_user_login`: if `!user.is_external()` and home folder missing, create "My Folder - {username}". `on_user_deleted` (AdminDelete): trash the home folder. `on_user_deleted` (GdprPurge): hard-delete folder + files. `on_user_logout`: `Ok(())`. | +| `AuthzCacheLifecycleHook` | `src/infrastructure/services/pg_acl_engine.rs` (same module as the Moka cache it invalidates) | Wraps `Arc`. `on_user_logout` & `on_user_deleted`: `engine.invalidate_user_groups_cache(user.id())` (new public method on the engine — one line `self.user_groups_cache.invalidate(id).await`). `on_user_created` & `on_user_login`: `Ok(())`. | +| `AuditLifecycleHook` | `src/application/services/user_lifecycle_service.rs` (co-located with the dispatcher — cross-cutting, no domain owner) | All four events: `tracing::info!(target: "audit", event = "user.{created\|login\|logout\|deleted}", user_id = %user.id(), is_external = user.is_external(), ...)`. Stays one place for user-lifecycle audit. | +| `SessionRevocationLifecycleHook` | The session-service module (e.g. `src/application/services/session_service.rs` or wherever `revoke_all_user_sessions` lives — verify at PR-write time) | `on_user_deleted`: explicit `session_storage.revoke_all_user_sessions(user.id(), tx)` for traceable audit (the FK CASCADE would do it but produces no per-session audit event). `on_user_logout` / `on_user_login` / `on_user_created`: `Ok(())`. | +| `ExternalIdentityLifecycleHook` *(stubbed; populated by the magic-link PR later)* | A future external-identity service module (created with the magic-link PR sequence; for the stub PR, place it in `src/application/services/external_identity_service.rs` as a new module) | `on_user_login`: if `user.is_external()`, bump a `last_verified_at` column on a future `auth.user_external_identity` side-table. Other events: `Ok(())`. Lands as no-op now so the slot exists. | + +**Why owner-located, not lifecycle-located**: it preserves the rule that "code about folders lives in the folder module". A future maintainer reading the folder service sees the lifecycle reactions next to the rest of the folder logic. It also makes a future workspace split (see "Crate-split note" at the end of this plan) almost free — each domain takes its hooks with it. + +### Tips for hook implementors + +These belong in the module-level docstring of `application/ports/user_lifecycle.rs` so the next maintainer reading the trait sees them in IDE hover. + +1. **First-ever login detection.** `on_user_login` fires after credentials validate but **before** `user.last_login_at` is updated for this session. So `user.last_login_at().is_none()` is a reliable "this is the first login since account creation" signal. Use it for welcome emails, one-shot default-folder seeding, "complete your profile" prompts, etc. + +2. **External-user short-circuit.** Every hook that provisions or manages user-owned resources (folders, calendars, address books) should start with `if user.is_external() { return Ok(()); }`. External users are grant-only; they don't own storage. The `CHECK (NOT is_external OR storage_used_bytes = 0)` constraint catches violations at the DB level. + +3. **Idempotency is mandatory.** `on_user_login` fires on every successful authentication. A hook that creates a resource must first check whether the resource already exists. Examples: `HomeFolderLifecycleHook` does `if folder_exists(user_id) { return Ok(()); }` before calling `create_home_folder`. `AuthzCacheLifecycleHook::on_user_logout` is naturally idempotent (cache `invalidate` is a no-op on a missing key). + +4. **First call after `is_external = TRUE → FALSE`.** When admin converts an external user to internal (`UPDATE auth.users SET is_external = FALSE`), the user's next login fires `on_user_login` with the new flag value. The home-folder hook sees `!is_external` and that no folder exists → creates it. No special "convert" event needed; idempotency carries the load. + +5. **Per-session logout firing.** Disabling a user revokes N sessions in a loop. The dispatcher fires `on_user_logout` **once per session revoked**, all with `reason = AccountDisabled`. Hook implementors must accept N redundant calls (idempotent invalidation, idempotent audit) — do **not** assume "one logout = one user state change". The same applies to `revoke_all_user_sessions` on password change. + +6. **Failure swallowing on create/login.** If your hook returns `Err`, the user is still created / logged in; only your hook's effect is delayed. Log enough detail (`tracing::error!`) that a subsequent investigation can identify the user and retry manually. Failure on `on_user_deleted` aborts the transaction — be conservative about returning Err there. + +7. **No transaction handle on create/login/logout.** Only `on_user_deleted` gets `&mut Transaction` because deletion is the only event with hard atomic-with-DB requirements. Other hooks open their own connections / pools as needed. This keeps the trait surface minimal. + +8. **Hook registration is at DI time.** Hook order is registration order; document this in the DI factory if you ever add an ordering dependency (e.g., HomeFolderLifecycleHook before any future hook that wants to write to that folder). + +## Documentation + +A new architecture page `docs/architecture/user-lifecycle.md` lands alongside the trait (in PR 1) and grows incrementally with each subsequent PR. Mirrors the structure of the existing `docs/architecture/file-and-blob-lifecycle.md` so readers familiar with the file-side pattern can navigate the user-side analog. + +**Outline** (~150 lines): + +1. **Context** — why hooks (replaces 4 scattered `create_personal_folder` calls + the self-heal; sets up the magic-link / external-user flow as a pluggable concern). +2. **The trait** — full signature, the 4 events, `LogoutReason` / `DeletionMode` enums. +3. **Dispatcher semantics** — per-event failure model (log-and-continue for created/login, fire-and-forget spawn for logout, abort-on-Err for deleted-in-transaction). Diagram. +4. **Implementation tips** (verbatim from the "Tips for hook implementors" section of this plan — first-login detection via `last_login_at.is_none()`, idempotency, external-user short-circuit, per-session logout firing, …). +5. **Owner-located convention** — explains why hooks live with their service module rather than a centralised `lifecycle/` directory, with the FileLifecycleHook precedent. +6. **Concrete hooks shipped today** — table of `HomeFolderLifecycleHook` / `AuthzCacheLifecycleHook` / `AuditLifecycleHook` / `SessionRevocationLifecycleHook` / `ExternalIdentityLifecycleHook` (stub) with one-line summaries and where each lives. +7. **Recommended future triggers** — the "future triggers" table from this plan (`on_user_password_changed`, `on_user_role_changed`, etc.) so v2 contributors see the design door. +8. **File map** — same shape as the file map at the bottom of `rebac-authorization.md`. + +**VitePress sidebar update** in `docs/.vitepress/config.mts`. The Architecture section already lists "File and Blob lifecycle" (line 104); add immediately after: + +```ts +{ text: "User lifecycle", link: "/architecture/user-lifecycle" }, +``` + +**Incidental fix while we're in the file**: `docs/architecture/rebac-authorization.md` (created in a previous session) is missing from the sidebar. Add it in the same edit: + +```ts +{ text: "ReBAC Authorization", link: "/architecture/rebac-authorization" }, +``` + +Place it logically — probably right before "Share Integration" since shares depend on ReBAC concepts. + +**Per-PR doc growth**: +- PR 1: sections 1, 2, 3, 4, 5 (trait, dispatcher, conventions) + the AuditLifecycleHook entry in section 6 +- PR 2: short subsection in section 4 explaining the `is_external` short-circuit pattern +- PR 3: HomeFolderLifecycleHook entry in section 6, plus a worked example "what happens when a brand-new user logs in" +- PR 4: AuthzCacheLifecycleHook + SessionRevocationLifecycleHook entries, plus the `DeletionMode` section +- PR 5: ExternalIdentityLifecycleHook entry + a "this is a placeholder for the upcoming magic-link feature" note + +Sidebar entry lands in PR 1; subsequent PRs only edit the markdown content. + +## Migration sequencing (5 PRs) + +**PR 1: trait + dispatcher + audit hook only.** +Lands the trait at `application/ports/user_lifecycle.rs`, the dispatcher at `application/services/user_lifecycle_service.rs`, and `AuditLifecycleHook` as the lone registered hook. Wires `dispatch_created` / `dispatch_login` / `dispatch_logout` / `dispatch_deleted` into the existing 4 auth code paths (no behaviour change for users; only audit log gains four new event types). Zero risk; validates plumbing. + +**PR 2: `is_external` column + entity field.** +Migration `20260612000002_auth_users_is_external.sql`, `User::is_external` getter, factory variant, DTO field. All existing rows have `is_external = FALSE` from the column default; no breaking changes. New `POST /api/admin/users` accepts `is_external` (default `false`). + +**PR 3: `HomeFolderLifecycleHook`.** +Register the hook. Remove the 4 eager `create_personal_folder` calls in `auth_application_service.rs:283 / 360 / 832 / 1277`. Remove the self-heal at `folder_service.rs:350-365`. Existing test suite should pass — folder still gets created, just by the hook now. The Hurl suite at `tests/api/run.sh` is the canary. + +**PR 4: `AuthzCacheLifecycleHook` + `SessionRevocationLifecycleHook` + `on_user_deleted` policy.** +Adds the `pub fn invalidate_user_groups_cache(&self, id: Uuid)` method on `PgAclEngine`. Wires the two hooks. Adds `DeletionMode` switching to `HomeFolderLifecycleHook::on_user_deleted` (trash vs hard-delete). Admin-delete endpoint now passes `mode = AdminDelete`; a (future) GDPR sweeper passes `GdprPurge`. + +**PR 5: `ExternalIdentityLifecycleHook` stub.** +Empty no-op hook landed in advance of the magic-link feature so the registration slot exists in DI. Populated in the magic-link PR sequence later. + +After PR 3, the cleanup of `create_personal_folder` from `auth_application_service.rs` is complete and the service stops importing `FolderService` for that purpose. + +## Recommended future triggers (DON'T ship now) + +These are the events users / consumers will eventually want. Each has a "what would make us add it" rationale; absent that, **don't add the method to the trait** — every method adds a no-op to every hook impl forever. + +| Future event | Why someone might want it | What would force adding it | +|---|---|---| +| `on_user_password_changed` | Notify the user via email; invalidate any cached credentials; trigger TOTP re-enrolment | A real per-user notification service. Today the password-change handler explicitly calls `revoke_all_user_sessions` which fires `on_user_logout(PasswordChanged)` for each session — sufficient for current consumers. | +| `on_user_role_changed` | Admin grants admin role → audit + maybe send "you're now an admin" email; admin demotion → revoke admin-only sessions | A multi-role system (today only `admin` / `user` exist). Currently a one-liner audit log at the admin handler covers it. | +| `on_user_email_changed` | External users: re-verify the new email via magic-link before trusting it; internal: notify both old and new addresses; update OIDC mapping | When external users start changing their email. Today email is immutable in the API. | +| `on_user_username_changed` | Update display names in audit logs that captured the old username; rename the home folder if it embeds the username | When username changes ship. Today username is immutable. | +| `on_user_avatar_changed` | Bust thumbnail caches downstream; sync to federated servers (OCM) | When OCM federation ships and remote partners need to learn about avatar changes. Today no downstream consumer. | +| `on_user_quota_changed` | Future per-service quota counters react to admin-changed limits | When quota becomes per-service (today it's a single global counter per user). | +| `on_user_disabled` / `on_user_enabled` | Audit-distinguishable state changes; pause per-user scheduled jobs | When per-user scheduled jobs land. Today `on_user_logout(AccountDisabled)` covers the only real consumer (sessions). Re-enable triggers `on_user_login` naturally. | +| `on_user_external_to_internal_converted` | Welcome email; provision the catalog of internal-only resources at conversion time rather than on next login | If admins routinely promote external users and the next-login lag is unacceptable. Today the idempotent `on_user_login` recheck handles conversion fine. | +| `on_user_oidc_linked` / `on_user_oidc_unlinked` | Audit; sync remote profile data | When users can link/unlink OIDC identities post-creation. Today OIDC linkage is fixed at user-creation time. | +| `on_user_2fa_enabled` / `on_user_2fa_disabled` | Audit; force re-login of other sessions | When 2FA ships. | + +**Rule of thumb for adding any of these later**: add the trait method with a default `Ok(())` body so existing hooks don't need to declare it explicitly (one-time exception to the "no defaults" rule, paid forever after by IDE-discoverable docstrings on the new method). Make sure the docstring states whether it's await-or-spawn semantics and whether failure aborts the parent operation. + +## Critical files + +**New files** (per PR): + +- PR 1: `src/application/ports/user_lifecycle.rs` (trait + `LogoutReason` + `DeletionMode` enums), `src/application/services/user_lifecycle_service.rs` (dispatcher + `AuditLifecycleHook` co-located inside), `docs/architecture/user-lifecycle.md` (architecture doc, outline above) +- PR 2: `migrations/20260612000002_auth_users_is_external.sql` +- PR 3: No new files — `HomeFolderLifecycleHook` is added as a new `impl UserLifecycleHook for ...` block inside the **existing** `src/application/services/folder_service.rs` (or a sibling `folder_lifecycle.rs` if folder_service.rs gets too large; verify line count at PR-write time) +- PR 4: No new files — `AuthzCacheLifecycleHook` added inside the existing `src/infrastructure/services/pg_acl_engine.rs`; `SessionRevocationLifecycleHook` added inside the session-service module +- PR 5: `src/application/services/external_identity_service.rs` (new module hosting the stub hook) + +**Modified files**: + +- `src/domain/entities/user.rs` (PR 2): add `is_external` field + getter + factory +- `src/application/services/auth_application_service.rs` (PRs 1, 3): wire dispatcher into the 4 create / 3 login / 2 logout / 1 delete sites; remove the 4 eager folder-creation calls in PR 3 +- `src/application/services/folder_service.rs` (PR 3): add the `HomeFolderLifecycleHook` impl; remove the self-heal at lines 350-365 (now handled by the hook on next login) +- `src/infrastructure/services/pg_acl_engine.rs` (PR 4): add `pub fn invalidate_user_groups_cache(&self, id: Uuid)` exposing `user_groups_cache.invalidate(id)`; add the `AuthzCacheLifecycleHook` impl +- `src/common/di.rs` (PRs 1, 3, 4, 5): construct the `UserLifecycleService` with builder chain, mirror the `FileLifecycleService` registration pattern at lines 264-301 +- `src/application/dtos/user_dto.rs` (PR 2): add `is_external: bool` field +- `src/interfaces/api/handlers/admin_handler.rs` (PR 2): accept `is_external` in `POST /api/admin/users` request body +- `docs/.vitepress/config.mts` (PR 1): add "User lifecycle" entry to the Architecture sidebar (line ~104). Also incidentally add the missing "ReBAC Authorization" entry that pre-dated this work +- `docs/architecture/user-lifecycle.md` (PRs 2, 3, 4, 5): grow the doc incrementally as each hook lands — `is_external` short-circuit note in PR 2, HomeFolderLifecycleHook section in PR 3, etc. + +**Existing patterns to reuse**: + +- Hook trait + dispatcher pattern: `src/application/ports/file_lifecycle.rs` + `src/application/services/file_lifecycle_service.rs` (the closest analog) +- DI builder chain: `src/common/di.rs:264-301` (FileLifecycleService construction) +- Audit tracing convention: `target: "audit"` events emitted by `src/application/services/subject_group_service.rs::create / rename / delete / add_member / remove_member` +- Per-cache invalidation method on engine: model after how `user_groups_cache` is accessed today in `src/infrastructure/services/pg_acl_engine.rs::expand_user` + +## Verification + +```bash +cargo fmt --all +cargo clippy --all-features --all-targets -- -D warnings +cargo test --workspace +biome check --fix static/js/ +tsc -p jsconfig.json --noEmit +``` + +After PR 1 (smoke test the plumbing): + +1. `cargo run`, then via the UI: register a new user, log in, log out, delete via admin. +2. `journalctl -t oxicloud | grep "target=user_lifecycle"` (or `RUST_LOG=user_lifecycle=info`) — exactly one event line per action. + +After PR 3 (the migration of folder creation): + +1. Hurl suite: `bash tests/api/run.sh` — all 13 test files still pass. `permissions.hurl` is the most relevant (it creates `bob` and verifies the home folder). +2. Manual: register a fresh user via the UI → home folder appears in the file list immediately. Then drop the home folder via SQL (`DELETE FROM storage.folders WHERE user_id = $1`), log out, log back in → folder reappears (the safety-net path). +3. Confirm via tracing that `dispatch_login` actually ran for an existing user whose folder was already there → no folder creation attempt, no error, just one `on_user_login` audit event. + +After PR 4: + +1. Authz cache: create a user, log them in, log them out. Inspect `RUST_LOG=oxicloud::infrastructure::services::pg_acl_engine=debug` — cache entry should be invalidated immediately on logout, not after 30s TTL. +2. User deletion: admin-deletes a user → verify (via audit log) that `on_user_deleted` ran inside the transaction and all sessions were revoked before the `auth.users` row vanished. + +After PR 5: no functional change; just confirm `external_identity_hook.rs` compiles and registers in DI as a no-op. + +## Out of scope (do NOT bundle into these 5 PRs) + +- **The magic-link external-user flow itself.** Lands in a later sequence; this plan only prepares the schema (`is_external`) and the hook slot (`ExternalIdentityLifecycleHook` stub). +- **Removing `Subject::External` from the domain.** It's currently unused; the cleanup is a separate small PR after PR 2 demonstrates that external users live in `auth.users`. +- **GDPR sweeper.** The `DeletionMode::GdprPurge` variant exists in PR 4 but no sweeper is wired up — admin-delete uses `AdminDelete`. A scheduled sweeper is its own future work. +- **Moving the `active` flag transitions through a hook.** PR 4's `on_user_logout(AccountDisabled)` covers it; no `on_user_disabled` method is added (see "future triggers" section). +- **Side-table for OIDC/OCM provenance** (`auth.user_external_identity`). Lands with the magic-link PR; not needed for `is_external` alone. + +## Crate-split note (forward-looking, NOT in this work) + +OxiCloud is currently a single Rust crate (~50 kLOC). The lifecycle-hook restructuring above intentionally aligns with the natural domain boundaries (each hook lives with its service) so that a future workspace split is incremental rather than a rewrite. **Not on the table for this work, but worth recording the intended split axis** so subsequent refactors don't paint into a corner: + +- **Split by domain bounded context, NOT by hexagonal layer.** Layered split (`oxicloud-domain` / `oxicloud-application` / etc.) makes the common case painful: adding a field to an entity touches 4 crates. Domain split (`oxicloud-files`, `oxicloud-auth`, `oxicloud-rebac`, …) makes the common case stay in one crate. +- Target shape, illustrative: + ``` + oxicloud-kernel ← errors, DI primitives, common port traits (incl. UserLifecycleHook) + oxicloud-auth ← users, sessions, OIDC, app passwords; dispatcher lives here + oxicloud-rebac ← groups, grants, engine; registers AuthzCacheLifecycleHook + oxicloud-files ← files, folders, blobs, dedup, thumbnails; registers HomeFolderLifecycleHook + oxicloud-sharing ← shares, magic-link, external identity + oxicloud-calendar ← caldav + oxicloud-contacts ← carddav + oxicloud-server ← Axum wire-up, the binary, DI composition root + ``` + Each domain crate is internally layered. Cross-crate communication goes through `oxicloud-kernel` port traits. The DI factory at `oxicloud-server` is where crates compose into the full application. + +- **What today's lifecycle work buys for that future split**: zero rework on hook locations. `HomeFolderLifecycleHook` already lives next to `FolderService`, so it moves with `oxicloud-files`. `AuthzCacheLifecycleHook` moves with `oxicloud-rebac`. The dispatcher in `oxicloud-auth` only knows the trait, never the impls. + +- **Cheap things to do now that help the future split**, but are NOT bundled here: + - Tighten visibility: prefer `pub(crate)` over `pub` wherever a type isn't intentionally part of the public surface. Catches accidental cross-module reaches at compile time. + - Per-domain port traits: today `application/ports/file_lifecycle.rs` is a file-concern port living in the layer dir; eventually it should live under the files-domain module. Refactor when adjacent ports are touched, not as a one-shot move. + - Avoid expanding `src/common/` — it tends to absorb anything-shared and become hard to split later. + +These are convention recommendations for future PRs, not work items for this plan. From bb6429a62098e82db2e46847d7582b39446b39ec Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 1 Jun 2026 15:35:24 +0200 Subject: [PATCH 2/7] refactor(userLifecycle): add user lifecycle, more clarety + better integration for the future --- Cargo.lock | 1 + Cargo.toml | 1 + docs/.vitepress/config.mts | 2 + docs/architecture/user-lifecycle.md | 148 ++++++++++++++ src/application/ports/mod.rs | 1 + src/application/ports/user_lifecycle.rs | 186 ++++++++++++++++++ .../services/auth_application_service.rs | 90 ++++++++- src/application/services/mod.rs | 1 + .../services/user_lifecycle_service.rs | 184 +++++++++++++++++ src/common/di.rs | 14 ++ src/infrastructure/auth_factory.rs | 6 + 11 files changed, 631 insertions(+), 3 deletions(-) create mode 100644 docs/architecture/user-lifecycle.md create mode 100644 src/application/ports/user_lifecycle.rs create mode 100644 src/application/services/user_lifecycle_service.rs diff --git a/Cargo.lock b/Cargo.lock index d89b24f6..32982b46 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3652,6 +3652,7 @@ dependencies = [ "argon2", "async-compression", "async-stream", + "async-trait", "async_zip", "aws-config", "aws-sdk-s3", diff --git a/Cargo.toml b/Cargo.toml index 67d1108b..1a693bf5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ serde = { version = "1.0.228", features = ["derive"] } serde_json = "1.0.149" futures = "0.3.32" async-stream = "0.3.6" +async-trait = "0.1.83" mime_guess = "2.0.5" uuid = { version = "1.23.0", features = ["v4", "v7", "serde"] } thiserror = "2.0.18" diff --git a/docs/.vitepress/config.mts b/docs/.vitepress/config.mts index e0b8441d..8329bf6d 100644 --- a/docs/.vitepress/config.mts +++ b/docs/.vitepress/config.mts @@ -110,10 +110,12 @@ export default defineConfig({ { text: "Resource Listing API", link: "/architecture/resource-listing" }, { text: "Storage Safety", link: "/architecture/file-system-safety" }, { text: "Database Transactions", link: "/architecture/database-transactions" }, + { text: "ReBAC Authorization", link: "/architecture/rebac-authorization" }, { text: "Share Integration", link: "/architecture/share-integration" }, { text: "Storage Quotas", link: "/architecture/storage-quotas" }, { text: "File and Blob lifecycle", link: "/architecture/file-and-blob-lifecycle" }, { text: "ReBAC & Authorization", link: "/architecture/rebac-authorization" }, + { text: "User lifecycle", link: "/architecture/user-lifecycle" }, ], }, { text: "FAQ", link: "/faq" }, diff --git a/docs/architecture/user-lifecycle.md b/docs/architecture/user-lifecycle.md new file mode 100644 index 00000000..fda08ec4 --- /dev/null +++ b/docs/architecture/user-lifecycle.md @@ -0,0 +1,148 @@ +# User Lifecycle Hooks + +Observer pattern for per-service reactions to user state transitions: created, login, logout, deleted. Mirrors the [File and Blob lifecycle](/architecture/file-and-blob-lifecycle) pattern for files; deliberately diverges from it on two points (async + sync await for most events) because user-lifecycle work is rare and sometimes has hard synchronisation requirements. + +## Why hooks + +Before this work, four code paths in `AuthApplicationService` each called `create_personal_folder()` immediately after inserting an `auth.users` row (public registration, first-admin bootstrap, admin-creates-user, OIDC just-in-time provisioning), plus a fifth self-heal at `folder_service.rs` for users whose folder somehow went missing. **Five places, one concern, no shared abstraction.** Adding a future per-user resource — default calendar, address book, GPG keyring, external-identity provenance for the upcoming magic-link feature — would have meant touching all five. + +Hooks fix this once. Each domain service implements `UserLifecycleHook` for the events it cares about; the dispatcher fires events; services that don't care declare explicit `Ok(())` no-ops. New services register a hook in DI and inherit all four events for free. + +## The trait + +```rust +// src/application/ports/user_lifecycle.rs +#[async_trait] +pub trait UserLifecycleHook: Send + Sync { + fn name(&self) -> &'static str; + + async fn on_user_created(&self, user: &User) + -> Result<(), DomainError>; + + async fn on_user_login(&self, user: &User) + -> Result<(), DomainError>; + + async fn on_user_logout(&self, user: &User, reason: LogoutReason) + -> Result<(), DomainError>; + + async fn on_user_deleted(&self, user: &User, mode: DeletionMode) + -> Result<(), DomainError>; +} +``` + +Two enums frame the trait: + +```rust +pub enum LogoutReason { + UserInitiated, // explicit logout + SessionExpired, // TTL hit + AdminRevoked, // admin-initiated single-session revoke + AccountDisabled, // user.active flipped to FALSE → sessions revoked + PasswordChanged, // sibling sessions invalidated by password change + TokenReused, // session-family reuse detection +} + +pub enum DeletionMode { + AdminDelete, // admin deletes via UI; resources go to trash + GdprPurge, // GDPR right-to-erasure; hard-delete everything +} +``` + +**No default impls.** Every implementor must declare all four methods explicitly. Use `Ok(())` for events you don't care about. This forces conscious acknowledgement of every lifecycle event rather than silent inheritance — same convention as `FileLifecycleHook`. + +## Dispatcher semantics + +`UserLifecycleService` aggregates registered hooks and fans out events with **per-event failure semantics**. The trait itself is uniform; the dispatcher decides whether to await, whether to spawn, and whether `Err` aborts. + +| Event | Awaited? | On `Err` | +|--------------------|---------------|-----------------------------------------| +| `on_user_created` | yes (sync) | log-and-continue (retry on next login) | +| `on_user_login` | yes (sync) | log-and-continue (idempotent retry) | +| `on_user_logout` | no (spawned) | logged, never propagated | +| `on_user_deleted` | yes (sync) | log-and-continue today; PR 4 makes it abort-the-transaction | + +The asymmetry is deliberate. `on_user_created` and `on_user_login` must complete before the session token is returned, so callers see consistent state. `on_user_logout` is bookkeeping; the HTTP response shouldn't wait for cache flushes — the dispatcher spawns. `on_user_deleted` will become atomic-with-the-DELETE in PR 4 when a transaction handle joins the trait signature. + +```text + ┌──────────────────────────────────────────────────┐ + │ AuthApplicationService │ + │ register / login / logout / delete │ + └──────────────────────┬───────────────────────────┘ + │ + ▼ + ┌──────────────────────────────────────────────────┐ + │ UserLifecycleService::dispatch_* │ + │ created / login / logout / deleted │ + └──────────────────────┬───────────────────────────┘ + │ + ┌────────────────┼────────────────┐ + ▼ ▼ ▼ + AuditLifecycleHook HomeFolderHook AuthzCacheHook … + (PR 1 only) (PR 3) (PR 4) +``` + +## Owner-located convention + +Each concrete hook impl lives **next to the service that owns the work**, not in a centralised `lifecycle/` directory. + +Examples (PR plan): + +- `HomeFolderLifecycleHook` lives in `src/application/services/folder_service.rs` — same module as `FolderService`, owner of home-folder policy. +- `AuthzCacheLifecycleHook` lives in `src/infrastructure/services/pg_acl_engine.rs` — same module as the Moka cache it invalidates. +- `AuditLifecycleHook` lives in `src/application/services/user_lifecycle_service.rs` (with the dispatcher) — cross-cutting, no domain owner. + +This mirrors how `FileLifecycleHook` impls are placed: `ThumbnailRefreshHook` lives in `thumbnail_service.rs`, the audio metadata impl lives in `audio_metadata_service.rs`. A future maintainer reading the folder service sees the lifecycle reactions next to the rest of the folder logic — no jumping between modules to understand why a folder gets created on login. + +## Tips for implementors + +These are codified in the module-level docstring of `application/ports/user_lifecycle.rs` so they show up in IDE hover. + +1. **First-ever login detection.** `on_user_login` fires *before* `user.register_login()` is called, so `user.last_login_at().is_none()` is a reliable "this is the user's first login since account creation" signal. Use it for welcome emails, one-shot default-resource seeding, "complete your profile" prompts. + +2. **Idempotency is mandatory.** `on_user_login` fires on every successful authentication, not just the first. A hook that creates a resource must check whether the resource already exists before creating it. Cache invalidation, audit deduplication, etc., must all tolerate redundant calls. + +3. **Failure swallowing on create/login.** If your hook returns `Err`, the user is still created/logged in; only your hook's effect is delayed. Log enough detail via `tracing::error!` that subsequent investigation can identify the user. The next successful login's `on_user_login` will retry idempotently. + +4. **Per-session logout firing.** When a flow revokes multiple sessions in one call (e.g. `revoke_all_user_sessions` on password change), today the dispatcher fires `on_user_logout` ONCE per logical revoke-call. PR 4's `SessionRevocationLifecycleHook` will refine to once-per-session for proper audit granularity. Hooks must accept N redundant calls with the same reason — keep them idempotent. + +5. **`on_user_deleted` is post-commit today.** The user row is already gone when the hook fires. Returning `Err` cannot roll back. PR 4 refactors `delete_user_admin` to expose a transaction handle, at which point the trait gains `tx: &mut Transaction` and `Err` will abort the delete. + +6. **Hook order is registration order.** The DI factory determines firing sequence. If two hooks have an ordering dependency (e.g. home-folder must exist before default-calendar can be seeded inside it), the dependent hook registers AFTER the producer. Document the convention inline in the DI block. + +## Concrete hooks shipped today + +| Hook | Lives in | Responsibility | +|---|---|---| +| `AuditLifecycleHook` | `src/application/services/user_lifecycle_service.rs` (co-located with dispatcher) | All four events: emits one `tracing::info!(target: "audit", event = "user.*", ...)` per call. Co-located because audit is cross-cutting with no domain owner. | + +Subsequent PRs add: + +- `HomeFolderLifecycleHook` (PR 3) — owns home-folder provisioning; replaces the four eager `create_personal_folder` calls + the self-heal. +- `AuthzCacheLifecycleHook` (PR 4) — invalidates the `user_groups_cache` Moka entry on logout/delete. +- `SessionRevocationLifecycleHook` (PR 4) — refines per-session logout granularity; explicit session revocation inside the user-delete transaction. +- `ExternalIdentityLifecycleHook` (PR 5, stub for now) — populated by the upcoming magic-link external-user feature. + +## Future events (NOT shipped — design door) + +These events are reserved for situations that don't exist yet but probably will. Adding a method to the trait costs every hook impl a new no-op forever, so we don't add them speculatively. Each row lists what would force the addition. + +| Future event | Why someone might want it | What would force adding it | +|---|---|---| +| `on_user_password_changed` | Notify the user via email; invalidate cached credentials; trigger TOTP re-enrolment | A per-user notification service. Today the existing `revoke_all_user_sessions` cascade fires `on_user_logout(PasswordChanged)` for each session — sufficient for current consumers. | +| `on_user_role_changed` | Audit promotion to admin; revoke admin-only sessions on demotion | A multi-role system. Today only `admin` / `user` exist and the one-liner audit log at the admin handler covers it. | +| `on_user_email_changed` | External users: re-verify the new email via magic-link; notify both old and new addresses | When external users start changing their email. Today email is immutable. | +| `on_user_avatar_changed` | Bust thumbnail caches; sync to federated servers (OCM) | When OCM federation ships. | +| `on_user_disabled` / `on_user_enabled` | Audit-distinguishable state changes; pause per-user scheduled jobs | When per-user scheduled jobs land. Today `on_user_logout(AccountDisabled)` covers the only consumer. | +| `on_user_external_to_internal_converted` | Welcome email; pre-provision internal-only resources at conversion time | If admins routinely promote external users and the next-login lag is unacceptable. Today idempotent `on_user_login` handles conversion fine. | +| `on_user_2fa_enabled` / `on_user_2fa_disabled` | Audit; force re-login of other sessions | When 2FA ships. | + +**Rule of thumb for adding any of these**: pair the addition with a default `Ok(())` body (one-time exception to the "no defaults" rule) so existing hooks don't need to declare it. State in the docstring whether the event is await-or-spawn and whether `Err` aborts. + +## File map + +| Concern | Module | +|---|---| +| Trait + `LogoutReason` + `DeletionMode` enums + tips | `src/application/ports/user_lifecycle.rs` | +| Dispatcher + `AuditLifecycleHook` | `src/application/services/user_lifecycle_service.rs` | +| Wire-in: created / login / logout / deleted | `src/application/services/auth_application_service.rs` | +| DI registration | `src/common/di.rs` (constructs the dispatcher) + `src/infrastructure/auth_factory.rs` (threads it into `AuthApplicationService`) | diff --git a/src/application/ports/mod.rs b/src/application/ports/mod.rs index 8988aad0..c79f248c 100644 --- a/src/application/ports/mod.rs +++ b/src/application/ports/mod.rs @@ -21,4 +21,5 @@ pub mod storage_ports; pub mod thumbnail_ports; pub mod transcode_ports; pub mod trash_ports; +pub mod user_lifecycle; pub mod zip_ports; diff --git a/src/application/ports/user_lifecycle.rs b/src/application/ports/user_lifecycle.rs new file mode 100644 index 00000000..c37b8fe8 --- /dev/null +++ b/src/application/ports/user_lifecycle.rs @@ -0,0 +1,186 @@ +//! User-lifecycle hook port. +//! +//! Observer notified by [`AuthApplicationService`] when a user transitions +//! through one of four lifecycle events: created, login, logout, deleted. +//! Register concrete impls with [`UserLifecycleService`] during DI wiring; +//! the dispatcher fans out each event to every registered hook. +//! +//! Each impl owns ONE concern. Folder service owns home-folder provisioning, +//! authz engine owns its cache invalidation, audit service owns the audit +//! trail, etc. New services plug in by registering a hook; the dispatcher +//! itself never gains domain knowledge. +//! +//! # Convention: explicit no-ops +//! +//! Every implementor **must** provide all four methods — use an explicit +//! one-liner `Ok(())` for events the implementor does not care about. This +//! forces conscious acknowledgement of every lifecycle event rather than +//! silent omission via trait defaults. Mirrors the [`FileLifecycleHook`] +//! convention at `application/ports/file_lifecycle.rs`. +//! +//! # Convention: async + per-event semantics +//! +//! Unlike [`FileLifecycleHook`] (sync fire-and-forget), user-lifecycle +//! events are async because some require synchronous semantics: +//! provisioning must finish before the session token is returned; +//! deletion cleanup must commit atomically with the user DELETE. +//! +//! Per-event failure model (encoded in the dispatcher, not the trait): +//! +//! | Event | Awaited? | On `Err`? | +//! |--------------------|----------|----------------------------------------| +//! | `on_user_created` | yes | log-and-continue (retry on next login) | +//! | `on_user_login` | yes | log-and-continue (idempotent retry) | +//! | `on_user_logout` | no | fire-and-forget (spawned), error logged| +//! | `on_user_deleted` | yes | log-and-continue today; PR 4 will switch to abort-the-transaction once the dispatcher fires inside the delete tx | +//! +//! # Tips for hook implementors +//! +//! 1. **First-ever login detection.** `on_user_login` fires after +//! credentials validate but **before** `user.register_login()` is +//! called for this session. So `user.last_login_at().is_none()` is a +//! reliable "this is the first login since account creation" signal — +//! useful for welcome emails, one-shot default-resource seeding, +//! "complete your profile" prompts. +//! +//! 2. **External-user short-circuit.** Every hook that provisions or +//! manages user-owned resources (folders, calendars, address books) +//! should start with `if user.is_external() { return Ok(()); }`. +//! External users are grant-only — they don't own storage. The +//! `is_external` flag lands in PR 2 of this work; until then, treat +//! every user as internal. +//! +//! 3. **Idempotency is mandatory.** `on_user_login` fires on every +//! successful authentication. A hook that creates a resource must +//! first check whether the resource already exists. Same for cache +//! invalidation, audit deduplication, etc. The `on_user_login` +//! safety-net only works if hooks no-op when their work is already +//! done. +//! +//! 4. **External → internal conversion needs no special event.** When +//! admin converts an external user to internal (`UPDATE auth.users +//! SET is_external = FALSE`), the user's next login fires +//! `on_user_login` with the new flag value. Idempotent hooks see +//! `!is_external` + missing resources → provision. No +//! `on_user_converted` method needed. +//! +//! 5. **Per-session logout firing.** When a flow revokes multiple +//! sessions (e.g. `revoke_all_user_sessions` on password change), +//! today the dispatcher fires `on_user_logout` ONCE per logical +//! revoke-call. PR 4's `SessionRevocationLifecycleHook` will refine +//! this to once-per-session for proper audit granularity. Hooks must +//! therefore accept N redundant calls with the same reason — keep +//! them idempotent and side-effect-free. +//! +//! 6. **Failure swallowing on create/login.** If your hook returns +//! `Err`, the user is still created / logged in; only your hook's +//! effect is delayed. Log enough detail via `tracing::error!` that a +//! subsequent investigation can identify the user and retry +//! manually. The `on_user_login` safety-net will retry on the next +//! successful authentication. +//! +//! 7. **`on_user_deleted` is post-commit today.** The user row is +//! already gone when the hook fires. Returning `Err` cannot roll +//! back the delete. PR 4 will refactor `delete_user_admin` to expose +//! a transaction handle, at which point the trait gains a `tx: +//! &mut Transaction` parameter and `Err` will abort the delete. For +//! now: best-effort cleanup, log failures, don't assume atomicity. +//! +//! 8. **Hook order is registration order.** The DI factory at +//! [`AppServiceFactory`] determines the firing sequence. If two hooks +//! have an ordering dependency (e.g. home-folder must exist before +//! default-calendar can be seeded inside it), the dependent hook +//! registers AFTER the producer. Document the convention in the DI +//! block where order matters. + +use async_trait::async_trait; + +use crate::common::errors::DomainError; +use crate::domain::entities::user::User; + +/// Reason a user session is ending. Hooks that don't care about the cause +/// (e.g. cache invalidation) ignore the value; audit-style hooks branch on +/// it to emit distinguishable events. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum LogoutReason { + /// User clicked logout. Single-session. + UserInitiated, + /// Session TTL hit. Single-session. + SessionExpired, + /// Admin invoked single-session revocation (e.g. "log out other + /// devices"). Today this fires from `logout_all` and from individual + /// admin endpoints if/when they exist. + AdminRevoked, + /// `user.active` flipped to `FALSE` → all sessions revoked. Fires once + /// per logical revoke-call today (see tip #5). + AccountDisabled, + /// Password was changed → sibling sessions invalidated to force re-login + /// with the new password. + PasswordChanged, + /// Refresh-token reuse detected by the session-family guard. Entire + /// family revoked because the rotation was probably stolen. + TokenReused, +} + +/// How aggressively `on_user_deleted` cleanup should run. Today both +/// variants are equivalent (only `AuditLifecycleHook` exists, and it logs +/// regardless). The split exists so PR 4's `HomeFolderLifecycleHook` can +/// trash on `AdminDelete` but hard-delete on `GdprPurge`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DeletionMode { + /// Admin deletes a user through the UI. Resources move to trash for + /// the retention window; recoverable. + AdminDelete, + /// GDPR right-to-erasure sweeper. Hard-delete everything; not + /// recoverable. (No sweeper is wired today; the variant is reserved.) + GdprPurge, +} + +/// Observer for user-lifecycle events. See module-level docstring for the +/// convention, semantics, and 8 tips for implementors. +/// +/// `#[async_trait]` is required to make the trait `dyn`-compatible — +/// the dispatcher holds `Arc`. Without it, +/// native `async fn in trait` returns an opaque type that has no vtable +/// representation. The same crate (`async-trait` 0.1.x) is used by other +/// async ecosystem deps and was already transitively in Cargo.lock. +#[async_trait] +pub trait UserLifecycleHook: Send + Sync { + /// Short identifier used in tracing / error logs. Example: `"home_folder"`, + /// `"audit"`, `"authz_cache"`. + fn name(&self) -> &'static str; + + /// Fires once after INSERT into `auth.users` succeeds, regardless of + /// the creation path (self-register, admin-create, OIDC JIT, future + /// magic-link bootstrap). + /// + /// The dispatcher logs `Err` and continues — the user is still + /// created and the next `on_user_login` will run an idempotent retry. + async fn on_user_created(&self, user: &User) -> Result<(), DomainError>; + + /// Fires after every successful authentication, BEFORE the user's + /// `last_login_at` is updated for this session and BEFORE the session + /// token is returned to the caller. + /// + /// **Idempotency is mandatory** — this fires on every login, not just + /// the first. Hooks that provision must check whether their resource + /// already exists before creating it. See tip #3 in the module + /// docstring. + /// + /// `user.last_login_at().is_none()` distinguishes the first-ever + /// login from subsequent ones. See tip #1. + async fn on_user_login(&self, user: &User) -> Result<(), DomainError>; + + /// Fires on session termination. `reason` lets hooks distinguish + /// causes — audit cares; cache invalidation usually doesn't. + /// + /// Spawned by the dispatcher — `Err` is logged but never propagates. + /// The HTTP response shouldn't wait for downstream cache flushes. + async fn on_user_logout(&self, user: &User, reason: LogoutReason) -> Result<(), DomainError>; + + /// Fires after a successful `DELETE FROM auth.users`. **Post-commit + /// today** — see tip #7. The user row is already gone; cleanup must + /// be best-effort. PR 4 will refactor to provide a transaction + /// handle and switch to before-commit-in-tx semantics. + async fn on_user_deleted(&self, user: &User, mode: DeletionMode) -> Result<(), DomainError>; +} diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index bba3bea5..b334d29e 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -6,7 +6,9 @@ use crate::application::ports::auth_ports::{ UserStoragePort, }; use crate::application::ports::folder_ports::FolderUseCase; +use crate::application::ports::user_lifecycle::{DeletionMode, LogoutReason}; use crate::application::services::folder_service::FolderService; +use crate::application::services::user_lifecycle_service::UserLifecycleService; use crate::common::config::OidcConfig; use crate::common::errors::{DomainError, ErrorKind}; use crate::domain::entities::session::Session; @@ -72,6 +74,9 @@ pub struct AuthApplicationService { password_hasher: Arc, token_service: Arc, folder_service: Option>, + /// Dispatcher for user-lifecycle events. `None` only in tests that don't + /// exercise the lifecycle path; production DI always wires this. + user_lifecycle: Option>, /// Path to the storage directory, used for disk-space–aware quota calculation storage_path: PathBuf, oidc: RwLock, @@ -97,6 +102,7 @@ impl AuthApplicationService { password_hasher, token_service, folder_service: None, + user_lifecycle: None, storage_path, oidc: RwLock::new(OidcState { service: None, @@ -165,6 +171,14 @@ impl AuthApplicationService { self } + /// Configures the user-lifecycle dispatcher. Wired by the DI factory + /// after core services are up. PR 1: only AuditLifecycleHook is + /// registered, so calls without this configured silently no-op. + pub fn with_user_lifecycle(mut self, lifecycle: Arc) -> Self { + self.user_lifecycle = Some(lifecycle); + self + } + /// Configures the OIDC service pub fn with_oidc(self, oidc_service: Arc, oidc_config: OidcConfig) -> Self { { @@ -279,6 +293,13 @@ impl AuthApplicationService { // Save user let created_user = self.user_storage.create_user(user).await?; + // Lifecycle: notify hooks (audit, future provisioning, etc.). + // PR 3 will move the personal-folder creation below into a + // HomeFolderLifecycleHook fired here; for now both run. + if let Some(lc) = &self.user_lifecycle { + lc.dispatch_created(&created_user).await; + } + // Create personal folder for the user self.create_personal_folder(&dto.username, created_user.id()) .await; @@ -356,6 +377,12 @@ impl AuthApplicationService { let created_user = self.user_storage.create_user(user).await?; + // Lifecycle: notify hooks. PR 3 moves home-folder creation into + // HomeFolderLifecycleHook fired here. + if let Some(lc) = &self.user_lifecycle { + lc.dispatch_created(&created_user).await; + } + // Create personal folder for the admin self.create_personal_folder(&username, created_user.id()) .await; @@ -401,6 +428,13 @@ impl AuthApplicationService { )); } + // Lifecycle: dispatch login BEFORE register_login() so hooks + // observing `last_login_at().is_none()` see "first ever login" + // correctly. See tip #1 in user_lifecycle.rs. + if let Some(lc) = &self.user_lifecycle { + lc.dispatch_login(&user).await; + } + // Update last login user.register_login(); self.user_storage.update_user(user.clone()).await?; @@ -496,6 +530,13 @@ impl AuthApplicationService { self.session_storage .revoke_session_family(session.family_id()) .await?; + // Lifecycle: TokenReused logout — fired once per logical + // revoke-family call. PR 4 may refine to per-session firing. + if let Some(lc) = &self.user_lifecycle + && let Ok(user) = self.user_storage.get_user_by_id(session.user_id()).await + { + lc.dispatch_logout(user, LogoutReason::TokenReused); + } return Err(DomainError::new( ErrorKind::AccessDenied, "Auth", @@ -576,6 +617,15 @@ impl AuthApplicationService { // Revoke session self.session_storage.revoke_session(session.id()).await?; + // Lifecycle: notify hooks. One extra DB roundtrip per logout + // (user load) is acceptable — logout is rare. Failure to load + // the user is non-fatal: we already revoked the session. + if let Some(lc) = &self.user_lifecycle + && let Ok(user) = self.user_storage.get_user_by_id(user_id).await + { + lc.dispatch_logout(user, LogoutReason::UserInitiated); + } + Ok(()) } @@ -637,13 +687,19 @@ impl AuthApplicationService { user.update_password_hash(new_hash); // Save updated user - self.user_storage.update_user(user).await?; + self.user_storage.update_user(user.clone()).await?; // Optional: revoke all sessions to force re-login with new password self.session_storage .revoke_all_user_sessions(user_id) .await?; + // Lifecycle: PasswordChanged logout — fired once per logical + // revoke-all call. PR 4 may refine to per-session firing. + if let Some(lc) = &self.user_lifecycle { + lc.dispatch_logout(user, LogoutReason::PasswordChanged); + } + Ok(()) } @@ -828,6 +884,12 @@ impl AuthApplicationService { .await?; } + // Lifecycle: notify hooks. PR 3 moves home-folder creation into + // HomeFolderLifecycleHook fired here. + if let Some(lc) = &self.user_lifecycle { + lc.dispatch_created(&created).await; + } + // Create personal folder self.create_personal_folder(&dto.username, created.id()) .await; @@ -883,7 +945,16 @@ impl AuthApplicationService { // Prevent deleting yourself let user = self.user_storage.get_user_by_id(user_id).await?; tracing::info!("Admin deleting user: {} ({})", user.username(), user_id); - self.user_storage.delete_user(user_id).await + self.user_storage.delete_user(user_id).await?; + + // Lifecycle: notify hooks (post-commit today; PR 4 will move + // this inside a transaction so hook failures can abort the + // delete — see tip #7 in user_lifecycle.rs). + if let Some(lc) = &self.user_lifecycle { + lc.dispatch_deleted(&user, DeletionMode::AdminDelete).await; + } + + Ok(()) } /// Activate or deactivate a user (admin only) @@ -1175,7 +1246,12 @@ impl AuthApplicationService { .await { Ok(mut existing_user) => { - // User exists — update last login and sync avatar from IdP + // User exists — dispatch login BEFORE register_login() so + // hooks observe `last_login_at = None` on the very first + // login (see tip #1 in the trait docstring). + if let Some(lc) = &self.user_lifecycle { + lc.dispatch_login(&existing_user).await; + } existing_user.register_login(); existing_user.set_image(claims.picture.clone()); self.user_storage.update_user(existing_user.clone()).await?; @@ -1273,6 +1349,14 @@ impl AuthApplicationService { let created_user = self.user_storage.create_user(new_user).await?; + // Lifecycle: created (audit) + login (no register_login() + // for a fresh OIDC user means `last_login_at` is naturally + // None → first-login detection works). + if let Some(lc) = &self.user_lifecycle { + lc.dispatch_created(&created_user).await; + lc.dispatch_login(&created_user).await; + } + // Create personal folder self.create_personal_folder(&username, created_user.id()) .await; diff --git a/src/application/services/mod.rs b/src/application/services/mod.rs index 207736df..6bb1246e 100644 --- a/src/application/services/mod.rs +++ b/src/application/services/mod.rs @@ -25,6 +25,7 @@ pub mod storage_settings_service; pub mod storage_usage_service; pub mod subject_group_service; pub mod trash_service; +pub mod user_lifecycle_service; pub mod wopi_lock_service; pub mod wopi_token_service; diff --git a/src/application/services/user_lifecycle_service.rs b/src/application/services/user_lifecycle_service.rs new file mode 100644 index 00000000..12e91db1 --- /dev/null +++ b/src/application/services/user_lifecycle_service.rs @@ -0,0 +1,184 @@ +//! User-lifecycle dispatcher + the always-on `AuditLifecycleHook`. +//! +//! [`UserLifecycleService`] aggregates every registered +//! [`UserLifecycleHook`] and fans out each lifecycle event with +//! per-event failure semantics. See `user_lifecycle.rs` for the trait +//! contract and tips for implementors. +//! +//! [`AuditLifecycleHook`] lives in this file (not under +//! `infrastructure/services/`) because it's cross-cutting — no domain +//! service owns "user-lifecycle audit", and the hook is small enough that +//! a separate module would be ceremony. Every other hook lives with the +//! service that owns its work (see `architecture/user-lifecycle.md`). + +use std::sync::Arc; + +use async_trait::async_trait; + +use crate::application::ports::user_lifecycle::{DeletionMode, LogoutReason, UserLifecycleHook}; +use crate::common::errors::DomainError; +use crate::domain::entities::user::User; + +/// Composite dispatcher for user-lifecycle events. +/// +/// Mirrors the [`FileLifecycleService`] shape: a `Vec>` and a +/// builder. The per-event failure semantics differ from the file-side +/// (file events are sync fire-and-forget; user events have per-method +/// rules — see the trait docstring). +pub struct UserLifecycleService { + hooks: Vec>, +} + +impl Default for UserLifecycleService { + fn default() -> Self { + Self::new() + } +} + +impl UserLifecycleService { + pub fn new() -> Self { + Self { hooks: Vec::new() } + } + + pub fn with_hook(mut self, hook: Arc) -> Self { + self.hooks.push(hook); + self + } + + /// Created: log-and-continue. If a hook returns `Err`, the user is + /// still created — the next login's `on_user_login` will retry + /// idempotently. See tip #6 in the trait docstring. + pub async fn dispatch_created(&self, user: &User) { + for h in &self.hooks { + if let Err(e) = h.on_user_created(user).await { + tracing::error!( + target: "user_lifecycle", + hook = h.name(), + user_id = %user.id(), + error = %e, + "on_user_created failed; will retry on next login" + ); + } + } + } + + /// Login: log-and-continue. Same reasoning as `dispatch_created`. + /// Must fire BEFORE `user.register_login()` so that hooks observing + /// `last_login_at().is_none()` correctly detect the first-ever login. + pub async fn dispatch_login(&self, user: &User) { + for h in &self.hooks { + if let Err(e) = h.on_user_login(user).await { + tracing::error!( + target: "user_lifecycle", + hook = h.name(), + user_id = %user.id(), + error = %e, + "on_user_login failed; will retry on next login" + ); + } + } + } + + /// Logout: fire-and-forget. Spawned so the HTTP response doesn't wait + /// for downstream cache flushes. Takes ownership of `User` because the + /// spawn outlives the caller's borrow. + pub fn dispatch_logout(&self, user: User, reason: LogoutReason) { + let hooks = self.hooks.clone(); + tokio::spawn(async move { + for h in &hooks { + if let Err(e) = h.on_user_logout(&user, reason).await { + tracing::error!( + target: "user_lifecycle", + hook = h.name(), + reason = ?reason, + user_id = %user.id(), + error = %e, + "on_user_logout failed" + ); + } + } + }); + } + + /// Deleted: log-and-continue (post-commit today). PR 4 refactors + /// `delete_user_admin` to expose a transaction handle and switches + /// this to abort-on-first-Err to make cleanup atomic with the user + /// DELETE. See tip #7 in the trait docstring. + pub async fn dispatch_deleted(&self, user: &User, mode: DeletionMode) { + for h in &self.hooks { + if let Err(e) = h.on_user_deleted(user, mode).await { + tracing::error!( + target: "user_lifecycle", + hook = h.name(), + mode = ?mode, + user_id = %user.id(), + error = %e, + "on_user_deleted failed" + ); + } + } + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// AuditLifecycleHook +// +// Always-on observer. Emits one structured `tracing::info!(target: "audit", +// ...)` line per event. The only hook registered in PR 1; subsequent PRs +// add HomeFolderLifecycleHook, AuthzCacheLifecycleHook, etc., each living +// next to the service it works for. +// ───────────────────────────────────────────────────────────────────────────── + +/// Cross-cutting audit observer for user-lifecycle events. Co-located with +/// the dispatcher because audit has no domain owner. +pub struct AuditLifecycleHook; + +#[async_trait] +impl UserLifecycleHook for AuditLifecycleHook { + fn name(&self) -> &'static str { + "audit" + } + + async fn on_user_created(&self, user: &User) -> Result<(), DomainError> { + tracing::info!( + target: "audit", + event = "user.created", + user_id = %user.id(), + username = %user.username(), + ); + Ok(()) + } + + async fn on_user_login(&self, user: &User) -> Result<(), DomainError> { + tracing::info!( + target: "audit", + event = "user.login", + user_id = %user.id(), + username = %user.username(), + first_login = user.last_login_at().is_none(), + ); + Ok(()) + } + + async fn on_user_logout(&self, user: &User, reason: LogoutReason) -> Result<(), DomainError> { + tracing::info!( + target: "audit", + event = "user.logout", + user_id = %user.id(), + username = %user.username(), + reason = ?reason, + ); + Ok(()) + } + + async fn on_user_deleted(&self, user: &User, mode: DeletionMode) -> Result<(), DomainError> { + tracing::info!( + target: "audit", + event = "user.deleted", + user_id = %user.id(), + username = %user.username(), + mode = ?mode, + ); + Ok(()) + } +} diff --git a/src/common/di.rs b/src/common/di.rs index b0e747e1..8a5b50fe 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -691,12 +691,26 @@ impl AppServiceFactory { storage_usage_service = Some(self.create_storage_usage_service(&repos, &pool, &maintenance_pool)); + // User-lifecycle dispatcher. Audit hook is the only one + // registered in PR 1; subsequent PRs register + // HomeFolderLifecycleHook (PR 3), AuthzCacheLifecycleHook + // (PR 4), etc., each living next to the service that owns + // its work. Hook order is registration order — document + // dependencies inline if/when any arise. + let user_lifecycle = Arc::new( + crate::application::services::user_lifecycle_service::UserLifecycleService::new() + .with_hook(Arc::new( + crate::application::services::user_lifecycle_service::AuditLifecycleHook, + )), + ); + // Auth services if self.config.features.enable_auth { let services = crate::infrastructure::auth_factory::create_auth_services( &self.config, pool.clone(), Some(apps.folder_service_concrete.clone()), + user_lifecycle.clone(), ) .await .map_err(|e| { diff --git a/src/infrastructure/auth_factory.rs b/src/infrastructure/auth_factory.rs index 8caa422c..37d45f28 100644 --- a/src/infrastructure/auth_factory.rs +++ b/src/infrastructure/auth_factory.rs @@ -5,6 +5,7 @@ type Result = std::result::Result use crate::application::services::auth_application_service::AuthApplicationService; use crate::application::services::folder_service::FolderService; +use crate::application::services::user_lifecycle_service::UserLifecycleService; use crate::common::config::AppConfig; use crate::common::di::AuthServices; use crate::infrastructure::repositories::{SessionPgRepository, UserPgRepository}; @@ -16,6 +17,7 @@ pub async fn create_auth_services( config: &AppConfig, pool: Arc, folder_service: Option>, + user_lifecycle: Arc, ) -> Result { // Create JWT token service (TokenServicePort implementation) let token_service: Arc = Arc::new(JwtTokenService::new( @@ -49,6 +51,10 @@ pub async fn create_auth_services( auth_app_service = auth_app_service.with_folder_service(folder_svc); } + // Wire the user-lifecycle dispatcher (carries AuditLifecycleHook today; + // PR 3+ register HomeFolderLifecycleHook, AuthzCacheLifecycleHook, …). + auth_app_service = auth_app_service.with_user_lifecycle(user_lifecycle); + // Configure OIDC service if enabled if config.oidc.enabled { tracing::info!( From e130842bfc106a954f4675bc7576e98eb12ed387 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 1 Jun 2026 15:51:05 +0200 Subject: [PATCH 3/7] feat(user): add is_external flag, will permit integration of external users (without any storage) but able to be invited --- docs/architecture/user-lifecycle.md | 8 +- .../20260612000002_auth_users_is_external.sql | 53 ++++++++++++ src/application/dtos/settings_dto.rs | 6 ++ src/application/dtos/user_dto.rs | 6 ++ .../services/auth_application_service.rs | 80 +++++++++++++++---- .../services/user_lifecycle_service.rs | 4 + src/domain/entities/user.rs | 67 ++++++++++++++++ .../repositories/pg/user_pg_repository.rs | 30 ++++--- 8 files changed, 226 insertions(+), 28 deletions(-) create mode 100644 migrations/20260612000002_auth_users_is_external.sql diff --git a/docs/architecture/user-lifecycle.md b/docs/architecture/user-lifecycle.md index fda08ec4..371cedab 100644 --- a/docs/architecture/user-lifecycle.md +++ b/docs/architecture/user-lifecycle.md @@ -99,7 +99,13 @@ These are codified in the module-level docstring of `application/ports/user_life 1. **First-ever login detection.** `on_user_login` fires *before* `user.register_login()` is called, so `user.last_login_at().is_none()` is a reliable "this is the user's first login since account creation" signal. Use it for welcome emails, one-shot default-resource seeding, "complete your profile" prompts. -2. **Idempotency is mandatory.** `on_user_login` fires on every successful authentication, not just the first. A hook that creates a resource must check whether the resource already exists before creating it. Cache invalidation, audit deduplication, etc., must all tolerate redundant calls. +2. **External-user short-circuit.** Hooks that provision per-user resources (home folder, default calendar, address book, GPG keys, …) must start with `if user.is_external() { return Ok(()); }`. External users (`is_external = TRUE`) are grant-only recipients — they have no home folder and don't consume storage quota. The DB `CHECK (NOT is_external OR storage_used_bytes = 0)` constraint catches code paths that bypass this short-circuit. + + **Subtle but important rule**: external users can **never** be admins. The DB enforces this via `CHECK (NOT (is_external AND role = 'admin'))`. `User::new_external(...)` doesn't accept a role parameter — it always sets `UserRole::User`. To make an existing external user an admin, an admin must first convert them to internal (`UPDATE auth.users SET is_external = FALSE`) and *then* update the role. The two-step process is intentional friction: granting admin to a federated principal would let external identity providers indirectly manage the local instance. + +3. **Idempotency is mandatory.** `on_user_login` fires on every successful authentication, not just the first. A hook that creates a resource must check whether the resource already exists before creating it. Cache invalidation, audit deduplication, etc., must all tolerate redundant calls. + +4. **External → internal conversion needs no special event.** When an admin flips `is_external = FALSE`, the user's next login fires `on_user_login` with the new flag value. Idempotent hooks see `!is_external` and missing resources → provision. No `on_user_converted` method needed; the safety-net pattern carries the load. 3. **Failure swallowing on create/login.** If your hook returns `Err`, the user is still created/logged in; only your hook's effect is delayed. Log enough detail via `tracing::error!` that subsequent investigation can identify the user. The next successful login's `on_user_login` will retry idempotently. diff --git a/migrations/20260612000002_auth_users_is_external.sql b/migrations/20260612000002_auth_users_is_external.sql new file mode 100644 index 00000000..14902f2f --- /dev/null +++ b/migrations/20260612000002_auth_users_is_external.sql @@ -0,0 +1,53 @@ +-- ════════════════════════════════════════════════════════════════════════════ +-- Add `is_external` flag to auth.users +-- ════════════════════════════════════════════════════════════════════════════ +-- Distinguishes storage-owning internal users (default — `is_external = FALSE`) +-- from grant-only external users (`is_external = TRUE`). External users are +-- recipients of share-grants who do not have a home folder, do not consume +-- storage quota, and authenticate via magic-link / OIDC / OCM (future). +-- +-- This migration is additive — all existing rows default to internal. The +-- backend code that consumes the flag lands in PR 3 (HomeFolderLifecycleHook +-- short-circuits when `is_external = TRUE`) and the magic-link flow ships +-- later still. +-- +-- Subject::External(uuid) in the domain becomes redundant after this — every +-- principal is now Subject::User(uuid) with `is_external` as a property, not +-- a variant. The cleanup happens in a small follow-up after the flag has +-- been observed in production. + +ALTER TABLE auth.users + ADD COLUMN IF NOT EXISTS is_external BOOLEAN NOT NULL DEFAULT FALSE; + +-- Partial index for the two query patterns that scan by this flag: +-- - admin "list external users" surface +-- - GDPR / cleanup sweepers that filter on `is_external = TRUE` and +-- last_login_at older than a threshold. +-- Internal-user queries don't go through this index — they ignore the +-- column entirely. +CREATE INDEX IF NOT EXISTS idx_users_is_external_login + ON auth.users (is_external, last_login_at) + WHERE is_external = TRUE; + +-- Schema-level safety net: external users must not be charged for storage. +-- HomeFolderLifecycleHook (PR 3) short-circuits before creating a home +-- folder for them, so storage_used_bytes should stay at 0. This CHECK +-- catches any code path that bypasses the hook and tries to attribute +-- storage to an external user. +ALTER TABLE auth.users + ADD CONSTRAINT users_external_no_storage + CHECK (NOT is_external OR storage_used_bytes = 0); + +-- Forbid external + admin combination. External users are grant-only +-- recipients authenticating via federated identity (magic-link, OIDC, +-- future OCM). Granting them the admin role would let a federated +-- principal manage the local instance — undesirable. To promote an +-- external user to admin: first flip is_external to FALSE (converting +-- them to internal), then update the role separately. The two steps +-- are intentional friction. +ALTER TABLE auth.users + ADD CONSTRAINT users_external_not_admin + CHECK (NOT (is_external AND role = 'admin')); + +COMMENT ON COLUMN auth.users.is_external IS + 'TRUE for grant-only external recipients (magic-link, OIDC-only, OCM federated). FALSE for storage-owning internal users. Set at creation; can be flipped to FALSE by admin to convert external → internal (next login provisions the home folder via HomeFolderLifecycleHook).'; diff --git a/src/application/dtos/settings_dto.rs b/src/application/dtos/settings_dto.rs index 37649b94..0154cc07 100644 --- a/src/application/dtos/settings_dto.rs +++ b/src/application/dtos/settings_dto.rs @@ -91,9 +91,15 @@ pub struct AdminCreateUserDto { /// "admin" or "user"; defaults to "user" pub role: Option, /// Storage quota in bytes; 0 = unlimited. If omitted, uses role default. + /// Ignored when `is_external = true` (external users have no storage). pub quota_bytes: Option, /// Whether the account is active; defaults to true pub active: Option, + /// `true` to create a grant-only external user (no home folder, no + /// storage quota). Defaults to `false` (internal user). External + /// users authenticate via magic-link / OIDC / OCM federation — + /// password is set but never used. + pub is_external: Option, } /// Request body for admin password reset diff --git a/src/application/dtos/user_dto.rs b/src/application/dtos/user_dto.rs index 3188b03b..54d47f3b 100644 --- a/src/application/dtos/user_dto.rs +++ b/src/application/dtos/user_dto.rs @@ -19,6 +19,11 @@ pub struct UserDto { pub auth_provider: String, pub image: Option, pub can_edit_image: bool, + /// `true` for grant-only external recipients (magic-link, OIDC-only, + /// future OCM federated). External users have no home folder and + /// can't own storage; their quota is always 0. Internal users + /// default to `false`. + pub is_external: bool, } impl From for UserDto { @@ -37,6 +42,7 @@ impl From for UserDto { auth_provider: user.oidc_provider().unwrap_or("local").to_string(), image: user.image().map(|s| s.to_string()), can_edit_image: !user.is_oidc_user(), + is_external: user.is_external(), } } } diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index b334d29e..008c2b8c 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -858,21 +858,60 @@ impl AuthApplicationService { _ => UserRole::User, }; - // Determine quota, capped to available disk space - let quota = dto.quota_bytes.unwrap_or_else(|| self.capped_quota(&role)); + let is_external = dto.is_external.unwrap_or(false); - // Hash password + // Forbid external + admin combo. The DB `users_external_not_admin` + // CHECK constraint would catch this too, but a 400 with an + // explanatory message is friendlier than a generic 500 from a + // constraint violation. See the CHECK definition in + // migrations/20260612000002_auth_users_is_external.sql for the + // rationale. + if is_external && matches!(role, UserRole::Admin) { + return Err(DomainError::new( + ErrorKind::InvalidInput, + "User", + "External users cannot be admins. To promote an external user to admin, \ + first convert them to internal (set is_external = false), then update \ + the role separately." + .to_string(), + )); + } + + // External users never own storage. The DB `users_external_no_storage` + // CHECK constraint enforces this; setting quota=0 here keeps the + // domain consistent and matches `User::new_external`. + let quota = if is_external { + 0 + } else { + dto.quota_bytes.unwrap_or_else(|| self.capped_quota(&role)) + }; + + // Hash password (kept for both internal and external users — for + // external users it's currently unused since they authenticate via + // magic-link / OIDC, but the DB column is NOT NULL). let password_hash = self.password_hasher.hash_password(&dto.password).await?; - // Create domain entity - let user = - User::new(dto.username.clone(), email, password_hash, role, quota).map_err(|e| { - DomainError::new( - ErrorKind::InvalidInput, - "User", - format!("Error creating user: {}", e), - ) - })?; + // Create domain entity. External path uses `new_external` so the + // is_external flag is set + the EXTERNAL placeholder password + // marker is applied for clarity in DB inspection. `new_external` + // forces role=User (the admin+external combo was rejected above). + let user = if is_external { + User::new_external(dto.username.clone(), email).map(|mut u| { + // The hashed password from the request is unused for auth + // but is persisted so audit-trail integrity is preserved. + u.update_password_hash(password_hash); + u + }) + } else { + User::new(dto.username.clone(), email, password_hash, role, quota) + } + .map_err(|e| { + DomainError::new( + ErrorKind::InvalidInput, + "User", + format!("Error creating user: {}", e), + ) + })?; // Persist let created = self.user_storage.create_user(user).await?; @@ -890,11 +929,20 @@ impl AuthApplicationService { lc.dispatch_created(&created).await; } - // Create personal folder - self.create_personal_folder(&dto.username, created.id()) - .await; + // External users have no home folder by design. Internal users + // get one — PR 3 will move this provisioning into the lifecycle + // hook (which short-circuits on `is_external` itself). + if !created.is_external() { + self.create_personal_folder(&dto.username, created.id()) + .await; + } - tracing::info!("Admin created user: {} ({})", dto.username, created.id()); + tracing::info!( + "Admin created user: {} ({}, is_external={})", + dto.username, + created.id(), + created.is_external() + ); Ok(UserDto::from(created)) } diff --git a/src/application/services/user_lifecycle_service.rs b/src/application/services/user_lifecycle_service.rs index 12e91db1..16047c76 100644 --- a/src/application/services/user_lifecycle_service.rs +++ b/src/application/services/user_lifecycle_service.rs @@ -145,6 +145,7 @@ impl UserLifecycleHook for AuditLifecycleHook { event = "user.created", user_id = %user.id(), username = %user.username(), + is_external = user.is_external(), ); Ok(()) } @@ -155,6 +156,7 @@ impl UserLifecycleHook for AuditLifecycleHook { event = "user.login", user_id = %user.id(), username = %user.username(), + is_external = user.is_external(), first_login = user.last_login_at().is_none(), ); Ok(()) @@ -166,6 +168,7 @@ impl UserLifecycleHook for AuditLifecycleHook { event = "user.logout", user_id = %user.id(), username = %user.username(), + is_external = user.is_external(), reason = ?reason, ); Ok(()) @@ -177,6 +180,7 @@ impl UserLifecycleHook for AuditLifecycleHook { event = "user.deleted", user_id = %user.id(), username = %user.username(), + is_external = user.is_external(), mode = ?mode, ); Ok(()) diff --git a/src/domain/entities/user.rs b/src/domain/entities/user.rs index 1d4572c9..ad1b795c 100644 --- a/src/domain/entities/user.rs +++ b/src/domain/entities/user.rs @@ -36,6 +36,13 @@ pub struct User { oidc_provider: Option, oidc_subject: Option, image: Option, + /// TRUE = grant-only external recipient (magic-link, OIDC-only, OCM + /// federated). FALSE = storage-owning internal user. Hooks that + /// provision per-user resources (home folder, default calendar, …) + /// must short-circuit when `is_external` is TRUE — see tip #2 in + /// `application/ports/user_lifecycle.rs`. The DB CHECK constraint + /// `users_external_no_storage` is the schema-level safety net. + is_external: bool, } impl User { @@ -85,6 +92,7 @@ impl User { oidc_provider: None, oidc_subject: None, image: None, + is_external: false, }) } @@ -115,6 +123,48 @@ impl User { oidc_provider: Some(oidc_provider), oidc_subject: Some(oidc_subject), image: None, + is_external: false, + }) + } + + /// Create a new external user — magic-link / OIDC-only / OCM-federated + /// recipient who does NOT own storage. The `CHECK (NOT is_external OR + /// storage_used_bytes = 0)` DB constraint enforces the no-storage rule + /// at the schema level. + /// + /// **External users are always `UserRole::User`** — there is no role + /// parameter because admin + external is an explicitly forbidden + /// combination enforced by the `users_external_not_admin` DB CHECK + /// constraint. Granting admin to a federated principal would let + /// external identity providers indirectly manage the local instance. + /// To make an external user an admin: first convert them to internal + /// (`UPDATE auth.users SET is_external = FALSE`), then update role. + /// The two-step process is intentional friction. + /// + /// Quota is set to 0 because external users can't upload content + /// into any folder they own (they have no folder). They can only + /// act on grants the resource owner provides — which counts against + /// the owner's quota, not theirs. + pub fn new_external(username: String, email: String) -> UserResult { + Self::validate_username(&username)?; + Self::validate_email(&email)?; + let now = Utc::now(); + Ok(Self { + id: Uuid::new_v4(), + username, + email, + password_hash: "__EXTERNAL_NO_PASSWORD__".to_string(), + role: UserRole::User, + storage_quota_bytes: 0, + storage_used_bytes: 0, + created_at: now, + updated_at: now, + last_login_at: None, + active: true, + oidc_provider: None, + oidc_subject: None, + image: None, + is_external: true, }) } @@ -147,6 +197,13 @@ impl User { oidc_provider: None, oidc_subject: None, image: None, + // `from_data` is the minimal-args reconstruction path used by + // tests and JWT-claim-based principal hydration (which doesn't + // carry `is_external`). Default to FALSE — JWT-validated + // principals are existing internal users; magic-link external + // sessions take a different path that hydrates from DB via + // `from_data_full`. + is_external: false, } } @@ -166,6 +223,7 @@ impl User { oidc_provider: Option, oidc_subject: Option, image: Option, + is_external: bool, ) -> Self { Self { id, @@ -182,6 +240,7 @@ impl User { oidc_provider, oidc_subject, image, + is_external, } } @@ -242,6 +301,14 @@ impl User { self.image.as_deref() } + /// `TRUE` for grant-only external recipients (magic-link, OIDC-only, + /// OCM federated). Hooks provisioning per-user resources must + /// short-circuit when this returns `true` — see tip #2 in + /// `application/ports/user_lifecycle.rs`. + pub fn is_external(&self) -> bool { + self.is_external + } + pub fn set_image(&mut self, image: Option) { self.image = image; self.updated_at = Utc::now(); diff --git a/src/infrastructure/repositories/pg/user_pg_repository.rs b/src/infrastructure/repositories/pg/user_pg_repository.rs index 3a8436c4..8d2669c8 100644 --- a/src/infrastructure/repositories/pg/user_pg_repository.rs +++ b/src/infrastructure/repositories/pg/user_pg_repository.rs @@ -84,13 +84,13 @@ impl UserRepository for UserPgRepository { let _result = sqlx::query( r#" INSERT INTO auth.users ( - id, username, email, password_hash, role, - storage_quota_bytes, storage_used_bytes, + id, username, email, password_hash, role, + storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject + oidc_provider, oidc_subject, is_external ) VALUES ( $1, $2, $3, $4, $5::auth.userrole, $6, $7, $8, $9, $10, $11, - $12, $13 + $12, $13, $14 ) RETURNING * "#, @@ -108,6 +108,7 @@ impl UserRepository for UserPgRepository { .bind(user_clone.is_active()) .bind(user_clone.oidc_provider()) .bind(user_clone.oidc_subject()) + .bind(user_clone.is_external()) .execute(&mut **tx) .await .map_err(Self::map_sqlx_error)?; @@ -131,7 +132,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users WHERE id = $1 "#, @@ -163,6 +164,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), )) } @@ -174,7 +176,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users WHERE username = $1 "#, @@ -206,6 +208,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), )) } @@ -217,7 +220,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users WHERE email = $1 "#, @@ -249,6 +252,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), )) } @@ -354,7 +358,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users ORDER BY created_at DESC LIMIT $1 OFFSET $2 @@ -391,6 +395,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), ) }) .collect(); @@ -406,7 +411,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users WHERE username ILIKE $1 OR email ILIKE $1 ORDER BY username @@ -443,6 +448,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), ) }) .collect(); @@ -529,7 +535,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users WHERE role::text = $1 ORDER BY created_at DESC @@ -565,6 +571,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), ) }) .collect(); @@ -600,7 +607,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users WHERE oidc_provider = $1 AND oidc_subject = $2 "#, @@ -632,6 +639,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), )) } From d81f5dbe48cdc8e34b1d6ca8cfd47d729ac12e1d Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 1 Jun 2026 16:05:02 +0200 Subject: [PATCH 4/7] feat(userLifecycle): migrate create_personal_folder() use now on_user_login on_user_created (only if user is not external) --- docs/architecture/user-lifecycle.md | 18 +- .../services/auth_application_service.rs | 83 ++----- src/application/services/folder_service.rs | 207 +++++++++++++----- src/common/di.rs | 26 ++- src/infrastructure/auth_factory.rs | 13 +- 5 files changed, 209 insertions(+), 138 deletions(-) diff --git a/docs/architecture/user-lifecycle.md b/docs/architecture/user-lifecycle.md index 371cedab..eac87b2a 100644 --- a/docs/architecture/user-lifecycle.md +++ b/docs/architecture/user-lifecycle.md @@ -119,15 +119,29 @@ These are codified in the module-level docstring of `application/ports/user_life | Hook | Lives in | Responsibility | |---|---|---| -| `AuditLifecycleHook` | `src/application/services/user_lifecycle_service.rs` (co-located with dispatcher) | All four events: emits one `tracing::info!(target: "audit", event = "user.*", ...)` per call. Co-located because audit is cross-cutting with no domain owner. | +| `AuditLifecycleHook` | `src/application/services/user_lifecycle_service.rs` (co-located with dispatcher) | All four events: emits one `tracing::info!(target: "audit", event = "user.*", ...)` per call, with `is_external` as a field. Co-located because audit is cross-cutting with no domain owner. | +| `HomeFolderLifecycleHook` | `src/application/services/folder_service.rs` (same module as `FolderService`) | `on_user_created` + `on_user_login`: idempotently provision "My Folder - {username}" via `FolderService::ensure_home_folder`. Short-circuits when `user.is_external()`. `on_user_logout`: `Ok(())`. `on_user_deleted`: `Ok(())` for PR 3 — PR 4 adds the trash-vs-hard-delete policy based on `DeletionMode`. Owns the responsibility that pre-PR 3 was scattered across four eager `create_personal_folder` calls in `AuthApplicationService` and one self-heal at the folder-listing path. | Subsequent PRs add: -- `HomeFolderLifecycleHook` (PR 3) — owns home-folder provisioning; replaces the four eager `create_personal_folder` calls + the self-heal. - `AuthzCacheLifecycleHook` (PR 4) — invalidates the `user_groups_cache` Moka entry on logout/delete. - `SessionRevocationLifecycleHook` (PR 4) — refines per-session logout granularity; explicit session revocation inside the user-delete transaction. - `ExternalIdentityLifecycleHook` (PR 5, stub for now) — populated by the upcoming magic-link external-user feature. +### Worked example: brand-new user logs in for the first time + +1. Client POSTs `/api/auth/login` with valid credentials. +2. `AuthApplicationService::login()` validates the password against the stored Argon2 hash. +3. **Before** `user.register_login()` is called, the dispatcher fires `dispatch_login(&user)`. The user's `last_login_at` is still `None` from creation time. +4. `AuditLifecycleHook::on_user_login` runs first (registration order): emits `event = "user.login", user_id = ..., username = ..., is_external = false, first_login = true`. +5. `HomeFolderLifecycleHook::on_user_login` runs next: sees `!user.is_external()`, calls `FolderService::ensure_home_folder(uid, username)`. The service checks `list_folders_by_owner(None, uid)` — empty → creates `"My Folder - alice"`. Returns `Ok(true)` (newly created). +6. Dispatcher finishes. `user.register_login()` is now called, stamping `last_login_at` to the current time. +7. The session row is INSERTed; access + refresh tokens generated; response returned to the client. + +On the user's **second** login: same flow up through step 5, but `ensure_home_folder` finds the existing folder, returns `Ok(false)`, no-op. The `AuditLifecycleHook` still emits an event, but `first_login = false` this time. + +If the home folder gets deleted manually (e.g., SQL `DELETE FROM storage.folders WHERE user_id = $1`), the user's **next** login will re-create it — that's the safety-net behaviour the lifecycle hook contractually owns. + ## Future events (NOT shipped — design door) These events are reserved for situations that don't exist yet but probably will. Adding a method to the trait costs every hook impl a new no-op forever, so we don't add them speculatively. Each row lists what would force the addition. diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index 008c2b8c..2588b8c3 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -5,9 +5,7 @@ use crate::application::ports::auth_ports::{ OidcIdClaims, OidcServicePort, PasswordHasherPort, SessionStoragePort, TokenServicePort, UserStoragePort, }; -use crate::application::ports::folder_ports::FolderUseCase; use crate::application::ports::user_lifecycle::{DeletionMode, LogoutReason}; -use crate::application::services::folder_service::FolderService; use crate::application::services::user_lifecycle_service::UserLifecycleService; use crate::common::config::OidcConfig; use crate::common::errors::{DomainError, ErrorKind}; @@ -73,9 +71,11 @@ pub struct AuthApplicationService { session_storage: Arc, password_hasher: Arc, token_service: Arc, - folder_service: Option>, /// Dispatcher for user-lifecycle events. `None` only in tests that don't /// exercise the lifecycle path; production DI always wires this. + /// HomeFolderLifecycleHook (registered on this dispatcher) owns the + /// per-user folder provisioning that AuthApplicationService used to do + /// inline pre-PR 3. user_lifecycle: Option>, /// Path to the storage directory, used for disk-space–aware quota calculation storage_path: PathBuf, @@ -101,7 +101,6 @@ impl AuthApplicationService { session_storage, password_hasher, token_service, - folder_service: None, user_lifecycle: None, storage_path, oidc: RwLock::new(OidcState { @@ -165,12 +164,6 @@ impl AuthApplicationService { } } - /// Configures the folder service, needed to create personal folders - pub fn with_folder_service(mut self, folder_service: Arc) -> Self { - self.folder_service = Some(folder_service); - self - } - /// Configures the user-lifecycle dispatcher. Wired by the DI factory /// after core services are up. PR 1: only AuditLifecycleHook is /// registered, so calls without this configured silently no-op. @@ -293,17 +286,13 @@ impl AuthApplicationService { // Save user let created_user = self.user_storage.create_user(user).await?; - // Lifecycle: notify hooks (audit, future provisioning, etc.). - // PR 3 will move the personal-folder creation below into a - // HomeFolderLifecycleHook fired here; for now both run. + // Lifecycle: HomeFolderLifecycleHook handles personal-folder + // creation (was inlined here pre-PR 3); audit log + future + // provisioning steps land here too. if let Some(lc) = &self.user_lifecycle { lc.dispatch_created(&created_user).await; } - // Create personal folder for the user - self.create_personal_folder(&dto.username, created_user.id()) - .await; - tracing::info!("User registered: {}", created_user.id()); Ok(UserDto::from(created_user)) } @@ -379,14 +368,12 @@ impl AuthApplicationService { // Lifecycle: notify hooks. PR 3 moves home-folder creation into // HomeFolderLifecycleHook fired here. + // Lifecycle: HomeFolderLifecycleHook provisions the admin's + // home folder. Audit logs the creation event. if let Some(lc) = &self.user_lifecycle { lc.dispatch_created(&created_user).await; } - // Create personal folder for the admin - self.create_personal_folder(&username, created_user.id()) - .await; - tracing::info!( "Initial admin created via setup: {} ({})", username, @@ -923,20 +910,13 @@ impl AuthApplicationService { .await?; } - // Lifecycle: notify hooks. PR 3 moves home-folder creation into - // HomeFolderLifecycleHook fired here. + // Lifecycle: HomeFolderLifecycleHook handles the home-folder + // provisioning (idempotent + short-circuits on is_external). + // Audit logs the creation event. if let Some(lc) = &self.user_lifecycle { lc.dispatch_created(&created).await; } - // External users have no home folder by design. Internal users - // get one — PR 3 will move this provisioning into the lifecycle - // hook (which short-circuits on `is_external` itself). - if !created.is_external() { - self.create_personal_folder(&dto.username, created.id()) - .await; - } - tracing::info!( "Admin created user: {} ({}, is_external={})", dto.username, @@ -1397,18 +1377,15 @@ impl AuthApplicationService { let created_user = self.user_storage.create_user(new_user).await?; - // Lifecycle: created (audit) + login (no register_login() - // for a fresh OIDC user means `last_login_at` is naturally - // None → first-login detection works). + // Lifecycle: created (audit + home-folder provisioning) + + // login (no register_login() for a fresh OIDC user means + // `last_login_at` is naturally None → first-login detection + // works). HomeFolderLifecycleHook creates the home folder. if let Some(lc) = &self.user_lifecycle { lc.dispatch_created(&created_user).await; lc.dispatch_login(&created_user).await; } - // Create personal folder - self.create_personal_folder(&username, created_user.id()) - .await; - tracing::info!( "OIDC user provisioned: {} (provider: {}, sub: {})", created_user.id(), @@ -1503,32 +1480,10 @@ impl AuthApplicationService { UserRole::User } - /// Helper to create a personal folder for a new user - async fn create_personal_folder(&self, username: &str, user_id: Uuid) { - if let Some(folder_service) = &self.folder_service { - let folder_name = format!("My Folder - {}", username); - match folder_service - .create_home_folder(user_id, folder_name.clone()) - .await - { - Ok(folder) => { - tracing::info!( - "Personal folder created for user {}: {} (ID: {})", - user_id, - folder.name, - folder.id - ); - } - Err(e) => { - tracing::error!( - "Failed to create personal folder for user {}: {}", - user_id, - e - ); - } - } - } - } + // `create_personal_folder` was removed in PR 3 of the + // UserLifecycleHook migration — home-folder provisioning is now + // owned by `HomeFolderLifecycleHook` in folder_service.rs and runs + // via `dispatch_created` / `dispatch_login`. } /// URL-safe base64 encoding without padding (RFC 4648 §5) diff --git a/src/application/services/folder_service.rs b/src/application/services/folder_service.rs index 9e39e7e0..5d2e73f8 100644 --- a/src/application/services/folder_service.rs +++ b/src/application/services/folder_service.rs @@ -315,7 +315,12 @@ impl FolderUseCase for FolderService { } /// Lists folders scoped to a specific owner. - /// Self-healing: if listing root folders and none exist, creates a home folder. + /// + /// **Note (post PR 3):** the self-heal block that auto-created a + /// home folder when listing returned empty has been removed. + /// `HomeFolderLifecycleHook` (registered on `UserLifecycleService`) + /// now provisions the folder on `on_user_created` / `on_user_login`, + /// idempotently, so the listing path no longer needs to self-heal. async fn list_folders_with_perms( &self, parent_id: Option<&str>, @@ -331,62 +336,23 @@ impl FolderUseCase for FolderService { ) .await?; return self.list_folders(parent_id).await; - } else { - // No parent defined grab user's homes - let folders = self - .folder_storage - .list_folders_by_owner(parent_id, caller_id) - .await - .map_err(|e| { - DomainError::internal_error( - "FolderStorage", - format!( - "Failed to list folders for owner '{}' in parent {:?}: {}", - caller_id, parent_id, e - ), - ) - })?; - - if folders.is_empty() { - // Self-healing: if listing root folders and none exist, create a home folder - // This ensures the frontend always gets a valid userHomeFolderId - tracing::info!( - "No root folders found for user {}, creating home folder automatically", - caller_id - ); - let owner_id_short = { - let s = caller_id.to_string(); - s[..8.min(s.len())].to_string() - }; - // TODO: what about i18n ? - let folder_name = format!("My Folder - {}", owner_id_short); - match self - .folder_storage - .create_home_folder(caller_id, folder_name.clone()) - .await - { - Ok(home_folder) => { - tracing::info!( - "Created home folder '{}' for user {}", - folder_name, - caller_id - ); - return Ok(vec![FolderDto::from(home_folder)]); - } - Err(e) => { - tracing::warn!( - "Failed to create home folder for user {}: {}", - caller_id, - e - ); - // Return empty list rather than failing - user might not have storage quota, etc. - } - } - } - Ok(folders.into_iter().map(FolderDto::from).collect()) } + // No parent → list the user's root folders. + let folders = self + .folder_storage + .list_folders_by_owner(parent_id, caller_id) + .await + .map_err(|e| { + DomainError::internal_error( + "FolderStorage", + format!( + "Failed to list folders for owner '{}' in parent {:?}: {}", + caller_id, parent_id, e + ), + ) + })?; + Ok(folders.into_iter().map(FolderDto::from).collect()) } - // TODO: move self healing in other part (on account creation on or login ?) /// Lists folders with pagination async fn list_folders_paginated( @@ -637,6 +603,59 @@ impl FolderService { Ok((rows, next_cursor)) } + + /// Idempotently provision a home folder for a user. + /// + /// Returns `Ok(true)` if a folder was newly created, `Ok(false)` if the + /// user already had at least one root folder. + /// + /// **System-level operation** — bypasses authz because this runs on + /// the user's own behalf (during creation or login provisioning) at a + /// point where the caller may be the engine itself, not an HTTP user. + /// Callers must be inside trusted code paths (lifecycle hooks). + /// + /// Used by [`HomeFolderLifecycleHook`] on `on_user_created` and + /// `on_user_login`. Replaces the old self-heal at the listing path + /// and the four eager `create_personal_folder` calls in + /// `AuthApplicationService` (removed in the same PR). + pub async fn ensure_home_folder( + &self, + user_id: Uuid, + username: &str, + ) -> Result { + let existing = self + .folder_storage + .list_folders_by_owner(None, user_id) + .await + .map_err(|e| { + DomainError::internal_error( + "FolderStorage", + format!("ensure_home_folder: list root folders: {}", e), + ) + })?; + if !existing.is_empty() { + return Ok(false); + } + + let folder_name = format!("My Folder - {}", username); + self.folder_storage + .create_home_folder(user_id, folder_name.clone()) + .await + .map_err(|e| { + DomainError::internal_error( + "FolderStorage", + format!("ensure_home_folder: create: {}", e), + ) + })?; + tracing::info!( + target: "user_lifecycle", + hook = "home_folder", + user_id = %user_id, + folder_name = %folder_name, + "Home folder provisioned" + ); + Ok(true) + } } /// Build the next-page cursor from the last row of the current page. @@ -690,3 +709,81 @@ fn build_folder_resource_cursor( }, } } + +// ───────────────────────────────────────────────────────────────────────────── +// HomeFolderLifecycleHook +// +// Owns home-folder provisioning policy. Replaces: +// - the 4 eager `create_personal_folder` calls in AuthApplicationService +// (register / setup_create_admin / admin_create_user / OIDC JIT) +// - the self-heal at `list_folders_with_perms` when no root folders exist +// +// Lives in this file (not under a centralised `lifecycle/` directory) +// because the folder service owns home-folder policy — see the +// "owner-located convention" note in +// `docs/architecture/user-lifecycle.md`. +// ───────────────────────────────────────────────────────────────────────────── + +use async_trait::async_trait; + +use crate::application::ports::user_lifecycle::{DeletionMode, LogoutReason, UserLifecycleHook}; +use crate::domain::entities::user::User; + +/// Lifecycle hook: provisions and (in PR 4) deprovisions a user's home folder. +pub struct HomeFolderLifecycleHook { + folder_service: Arc, +} + +impl HomeFolderLifecycleHook { + pub fn new(folder_service: Arc) -> Self { + Self { folder_service } + } + + /// Idempotent provisioning shared by `on_user_created` and + /// `on_user_login`. External users are skipped per tip #2 in the + /// trait docstring. + async fn provision_if_needed(&self, user: &User) -> Result<(), DomainError> { + if user.is_external() { + return Ok(()); + } + // `ensure_home_folder` handles the "does the user already have a + // root folder?" check internally and is a no-op if so. + self.folder_service + .ensure_home_folder(user.id(), user.username()) + .await + .map(|_created| ()) + } +} + +#[async_trait] +impl UserLifecycleHook for HomeFolderLifecycleHook { + fn name(&self) -> &'static str { + "home_folder" + } + + async fn on_user_created(&self, user: &User) -> Result<(), DomainError> { + self.provision_if_needed(user).await + } + + /// Login is the safety net — if `on_user_created` failed at any + /// earlier point (or the user was created in a flow that pre-dated + /// this hook), provisioning happens here on next login. + async fn on_user_login(&self, user: &User) -> Result<(), DomainError> { + self.provision_if_needed(user).await + } + + async fn on_user_logout(&self, _user: &User, _reason: LogoutReason) -> Result<(), DomainError> { + // Folders don't react to logout. Explicit no-op per the + // "no defaults" convention. + Ok(()) + } + + async fn on_user_deleted(&self, _user: &User, _mode: DeletionMode) -> Result<(), DomainError> { + // PR 4 will fill this in with the trash-vs-hard-delete policy + // (AdminDelete → trash with retention, GdprPurge → hard delete). + // For PR 3 the eager `DELETE FROM auth.users` cascades via the + // existing FK on storage.folders to remove the home folder, so a + // no-op here matches behaviour parity. + Ok(()) + } +} diff --git a/src/common/di.rs b/src/common/di.rs index 8a5b50fe..b7e609b2 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -691,25 +691,35 @@ impl AppServiceFactory { storage_usage_service = Some(self.create_storage_usage_service(&repos, &pool, &maintenance_pool)); - // User-lifecycle dispatcher. Audit hook is the only one - // registered in PR 1; subsequent PRs register - // HomeFolderLifecycleHook (PR 3), AuthzCacheLifecycleHook - // (PR 4), etc., each living next to the service that owns - // its work. Hook order is registration order — document - // dependencies inline if/when any arise. + // User-lifecycle dispatcher. Hook order is registration order; + // document dependencies inline if/when any arise. Today: + // 1. AuditLifecycleHook — fires first so the audit + // event is recorded even if a + // later hook errors out. + // 2. HomeFolderLifecycleHook — provisions the user's home + // folder on created/login (no-op + // for external users). + // Subsequent PRs register AuthzCacheLifecycleHook (PR 4) etc. let user_lifecycle = Arc::new( crate::application::services::user_lifecycle_service::UserLifecycleService::new() .with_hook(Arc::new( crate::application::services::user_lifecycle_service::AuditLifecycleHook, + )) + .with_hook(Arc::new( + crate::application::services::folder_service::HomeFolderLifecycleHook::new( + apps.folder_service_concrete.clone(), + ), )), ); - // Auth services + // Auth services. Folder service no longer threaded here — + // PR 3 moved home-folder provisioning into + // HomeFolderLifecycleHook, which already holds an Arc to the + // folder service via the user_lifecycle dispatcher. if self.config.features.enable_auth { let services = crate::infrastructure::auth_factory::create_auth_services( &self.config, pool.clone(), - Some(apps.folder_service_concrete.clone()), user_lifecycle.clone(), ) .await diff --git a/src/infrastructure/auth_factory.rs b/src/infrastructure/auth_factory.rs index 37d45f28..42ce88a8 100644 --- a/src/infrastructure/auth_factory.rs +++ b/src/infrastructure/auth_factory.rs @@ -4,7 +4,6 @@ use std::sync::Arc; type Result = std::result::Result>; use crate::application::services::auth_application_service::AuthApplicationService; -use crate::application::services::folder_service::FolderService; use crate::application::services::user_lifecycle_service::UserLifecycleService; use crate::common::config::AppConfig; use crate::common::di::AuthServices; @@ -16,7 +15,6 @@ use crate::infrastructure::services::password_hasher::Argon2PasswordHasher; pub async fn create_auth_services( config: &AppConfig, pool: Arc, - folder_service: Option>, user_lifecycle: Arc, ) -> Result { // Create JWT token service (TokenServicePort implementation) @@ -46,13 +44,10 @@ pub async fn create_auth_services( config.storage_path.clone(), ); - // Configure folder service if available - if let Some(folder_svc) = folder_service { - auth_app_service = auth_app_service.with_folder_service(folder_svc); - } - - // Wire the user-lifecycle dispatcher (carries AuditLifecycleHook today; - // PR 3+ register HomeFolderLifecycleHook, AuthzCacheLifecycleHook, …). + // Wire the user-lifecycle dispatcher. Home-folder provisioning is + // now handled by HomeFolderLifecycleHook (registered on the + // dispatcher in DI) — AuthApplicationService no longer needs a + // direct FolderService dependency for that path. auth_app_service = auth_app_service.with_user_lifecycle(user_lifecycle); // Configure OIDC service if enabled From 6a6f0701063905987c5e2481c80191c2e5bd90e4 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 1 Jun 2026 16:14:18 +0200 Subject: [PATCH 5/7] feat(userLifecycle): plug actions to on_user_logout and on_user_deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - AuthzCacheLifecycleHook — invalidates the user_groups_cache Moka entry on logout/delete. - SessionRevocationLifecycleHook — explicit per-session firing of on_user_logout (currently per-call); session revocation inside the user-delete transaction. - DeletionMode-driven policy in HomeFolderLifecycleHook::on_user_deleted (trash vs hard-delete based on AdminDelete / GdprPurge). - Refactor delete_user_admin to expose a transaction handle so on_user_deleted can abort atomically. --- docs/architecture/user-lifecycle.md | 53 +++++++-- src/application/ports/user_lifecycle.rs | 34 ++++-- .../services/auth_application_service.rs | 40 +++++-- src/application/services/folder_service.rs | 31 ++++- .../services/user_lifecycle_service.rs | 109 ++++++++++++++++-- src/common/di.rs | 37 ++++-- .../repositories/pg/user_pg_repository.rs | 8 ++ src/infrastructure/services/pg_acl_engine.rs | 80 +++++++++++++ 8 files changed, 342 insertions(+), 50 deletions(-) diff --git a/docs/architecture/user-lifecycle.md b/docs/architecture/user-lifecycle.md index eac87b2a..e1c22876 100644 --- a/docs/architecture/user-lifecycle.md +++ b/docs/architecture/user-lifecycle.md @@ -54,12 +54,12 @@ pub enum DeletionMode { `UserLifecycleService` aggregates registered hooks and fans out events with **per-event failure semantics**. The trait itself is uniform; the dispatcher decides whether to await, whether to spawn, and whether `Err` aborts. -| Event | Awaited? | On `Err` | -|--------------------|---------------|-----------------------------------------| -| `on_user_created` | yes (sync) | log-and-continue (retry on next login) | -| `on_user_login` | yes (sync) | log-and-continue (idempotent retry) | -| `on_user_logout` | no (spawned) | logged, never propagated | -| `on_user_deleted` | yes (sync) | log-and-continue today; PR 4 makes it abort-the-transaction | +| Event | Awaited? | On `Err` | +|--------------------|------------------|-----------------------------------------| +| `on_user_created` | yes (sync) | log-and-continue (retry on next login) | +| `on_user_login` | yes (sync) | log-and-continue (idempotent retry) | +| `on_user_logout` | no (spawned) | logged, never propagated | +| `on_user_deleted` | yes (sync, in tx) | **abort the transaction** — first `Err` rolls back the user DELETE and propagates to the admin endpoint as a 500 | The asymmetry is deliberate. `on_user_created` and `on_user_login` must complete before the session token is returned, so callers see consistent state. `on_user_logout` is bookkeeping; the HTTP response shouldn't wait for cache flushes — the dispatcher spawns. `on_user_deleted` will become atomic-with-the-DELETE in PR 4 when a transaction handle joins the trait signature. @@ -111,7 +111,7 @@ These are codified in the module-level docstring of `application/ports/user_life 4. **Per-session logout firing.** When a flow revokes multiple sessions in one call (e.g. `revoke_all_user_sessions` on password change), today the dispatcher fires `on_user_logout` ONCE per logical revoke-call. PR 4's `SessionRevocationLifecycleHook` will refine to once-per-session for proper audit granularity. Hooks must accept N redundant calls with the same reason — keep them idempotent. -5. **`on_user_deleted` is post-commit today.** The user row is already gone when the hook fires. Returning `Err` cannot roll back. PR 4 refactors `delete_user_admin` to expose a transaction handle, at which point the trait gains `tx: &mut Transaction` and `Err` will abort the delete. +5. **`on_user_deleted` runs inside the delete transaction.** The user row still exists when the hook fires; the dispatcher commits only after every hook returns `Ok(())`. Returning `Err` aborts the whole transaction — including the user DELETE itself. Implementors get `tx: &mut sqlx::Transaction<'_, Postgres>` so cleanup queries land in the same tx (e.g. session revocation with audit trail before FK CASCADE wipes the rows). Be conservative about returning `Err`: an abort means the admin's delete operation fails, leaving the user intact. 6. **Hook order is registration order.** The DI factory determines firing sequence. If two hooks have an ordering dependency (e.g. home-folder must exist before default-calendar can be seeded inside it), the dependent hook registers AFTER the producer. Document the convention inline in the DI block. @@ -120,14 +120,47 @@ These are codified in the module-level docstring of `application/ports/user_life | Hook | Lives in | Responsibility | |---|---|---| | `AuditLifecycleHook` | `src/application/services/user_lifecycle_service.rs` (co-located with dispatcher) | All four events: emits one `tracing::info!(target: "audit", event = "user.*", ...)` per call, with `is_external` as a field. Co-located because audit is cross-cutting with no domain owner. | -| `HomeFolderLifecycleHook` | `src/application/services/folder_service.rs` (same module as `FolderService`) | `on_user_created` + `on_user_login`: idempotently provision "My Folder - {username}" via `FolderService::ensure_home_folder`. Short-circuits when `user.is_external()`. `on_user_logout`: `Ok(())`. `on_user_deleted`: `Ok(())` for PR 3 — PR 4 adds the trash-vs-hard-delete policy based on `DeletionMode`. Owns the responsibility that pre-PR 3 was scattered across four eager `create_personal_folder` calls in `AuthApplicationService` and one self-heal at the folder-listing path. | +| `HomeFolderLifecycleHook` | `src/application/services/folder_service.rs` (same module as `FolderService`) | `on_user_created` + `on_user_login`: idempotently provision "My Folder - {username}" via `FolderService::ensure_home_folder`. Short-circuits when `user.is_external()`. `on_user_logout`: `Ok(())`. `on_user_deleted`: per-mode `tracing::info!` event so audit distinguishes AdminDelete from GdprPurge — the FK CASCADE on `storage.folders.user_id` handles the actual row removal. Trash-with-retention is documented as future work. | +| `AuthzCacheLifecycleHook` | `src/infrastructure/services/pg_acl_engine.rs` (same module as the Moka cache it invalidates) | `on_user_logout` + `on_user_deleted`: `engine.invalidate_user_groups_cache(user.id())` — drops the cached transitive-group expansion immediately so a re-login (or a re-created account with the same id) sees fresh memberships without waiting for the 30 s TTL. `on_user_created` + `on_user_login`: `Ok(())` (no stale entry could exist for these). | +| `SessionRevocationLifecycleHook` | `src/application/services/user_lifecycle_service.rs` (co-located with dispatcher; no dedicated session service module today) | `on_user_deleted`: explicit `session_storage.revoke_all_user_sessions(user.id())` + aggregate audit event (`event = "user.sessions_revoked_on_delete", count = N`). Replaces the silent FK CASCADE with an observable revocation. All other events: `Ok(())`. | +| `DeletionMode` | enum on the trait | Distinguishes admin-initiated delete (`AdminDelete` — currently identical to GDPR but reserved for a future trash-with-retention policy) from GDPR right-to-erasure purge (`GdprPurge` — for a future sweeper). PR 4 ships the variants; future PRs may add per-mode behaviour. | Subsequent PRs add: -- `AuthzCacheLifecycleHook` (PR 4) — invalidates the `user_groups_cache` Moka entry on logout/delete. -- `SessionRevocationLifecycleHook` (PR 4) — refines per-session logout granularity; explicit session revocation inside the user-delete transaction. - `ExternalIdentityLifecycleHook` (PR 5, stub for now) — populated by the upcoming magic-link external-user feature. +### How the delete transaction composes + +```text +AuthApplicationService::delete_user_admin(user_id) + │ + ▼ +BEGIN + │ + ▼ +dispatch_deleted(user, AdminDelete, &mut tx) + │ + ├── AuditLifecycleHook → tracing::info!(event="user.deleted", mode=...) + ├── HomeFolderLifecycleHook → tracing::info!("home folder will be removed via FK CASCADE") + ├── AuthzCacheLifecycleHook → engine.invalidate_user_groups_cache(user_id) + └── SessionRevocationLifecycleHook + → session_storage.revoke_all_user_sessions(user_id) + → tracing::info!(event="user.sessions_revoked_on_delete", + count=N) + │ + ▼ +DELETE FROM auth.users WHERE id = $1 + │ + ▼ (FK CASCADE removes folders, files, app_passwords, device_codes, calendar + │ shares, address-book shares, etc.; trg_cleanup_grants_user removes the + │ user's grants) + │ + ▼ +COMMIT +``` + +If any hook returns `Err`, the dispatcher propagates it; `delete_user_admin` rolls back the transaction and surfaces the error to the admin endpoint as a 500. The user row, all sessions, all folders/files, all grants — everything stays in place. Hook implementors should keep that strong constraint in mind: returning `Err` from `on_user_deleted` is a heavy hammer. + ### Worked example: brand-new user logs in for the first time 1. Client POSTs `/api/auth/login` with valid credentials. diff --git a/src/application/ports/user_lifecycle.rs b/src/application/ports/user_lifecycle.rs index c37b8fe8..d5cf5b89 100644 --- a/src/application/ports/user_lifecycle.rs +++ b/src/application/ports/user_lifecycle.rs @@ -32,7 +32,7 @@ //! | `on_user_created` | yes | log-and-continue (retry on next login) | //! | `on_user_login` | yes | log-and-continue (idempotent retry) | //! | `on_user_logout` | no | fire-and-forget (spawned), error logged| -//! | `on_user_deleted` | yes | log-and-continue today; PR 4 will switch to abort-the-transaction once the dispatcher fires inside the delete tx | +//! | `on_user_deleted` | yes (in tx) | abort the transaction (Err propagates) | //! //! # Tips for hook implementors //! @@ -79,12 +79,15 @@ //! manually. The `on_user_login` safety-net will retry on the next //! successful authentication. //! -//! 7. **`on_user_deleted` is post-commit today.** The user row is -//! already gone when the hook fires. Returning `Err` cannot roll -//! back the delete. PR 4 will refactor `delete_user_admin` to expose -//! a transaction handle, at which point the trait gains a `tx: -//! &mut Transaction` parameter and `Err` will abort the delete. For -//! now: best-effort cleanup, log failures, don't assume atomicity. +//! 7. **`on_user_deleted` runs inside the delete transaction.** The +//! user row still exists when the hook fires; the dispatcher commits +//! only after every hook returns `Ok(())`. Returning `Err` aborts +//! the whole transaction — including the user DELETE itself. +//! Implementors get `tx: &mut sqlx::Transaction<'_, Postgres>` so +//! cleanup queries land in the same tx (e.g. session revocation +//! with audit trail before FK CASCADE wipes the rows). Be +//! conservative about returning `Err`: an abort means the admin's +//! delete operation fails, leaving the user intact. //! //! 8. **Hook order is registration order.** The DI factory at //! [`AppServiceFactory`] determines the firing sequence. If two hooks @@ -178,9 +181,16 @@ pub trait UserLifecycleHook: Send + Sync { /// The HTTP response shouldn't wait for downstream cache flushes. async fn on_user_logout(&self, user: &User, reason: LogoutReason) -> Result<(), DomainError>; - /// Fires after a successful `DELETE FROM auth.users`. **Post-commit - /// today** — see tip #7. The user row is already gone; cleanup must - /// be best-effort. PR 4 will refactor to provide a transaction - /// handle and switch to before-commit-in-tx semantics. - async fn on_user_deleted(&self, user: &User, mode: DeletionMode) -> Result<(), DomainError>; + /// Fires inside the `delete_user_admin` transaction, BEFORE the + /// `DELETE FROM auth.users` row removal. The user row still exists + /// at this point; `user.id()` is safe to reference in queries on + /// the same `tx`. Returning `Err` rolls back the transaction — + /// the user is NOT deleted and the admin's request fails. See + /// tip #7 in the module docstring. + async fn on_user_deleted( + &self, + user: &User, + mode: DeletionMode, + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + ) -> Result<(), DomainError>; } diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index 2588b8c3..f008a499 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -968,20 +968,46 @@ impl AuthApplicationService { Ok(UserDto::from(user)) } - /// Delete a user by ID (admin only) + /// Delete a user by ID (admin only). + /// + /// Runs the whole flow in a single transaction so the lifecycle + /// hooks (`SessionRevocationLifecycleHook` revoking sessions with + /// audit, `AuthzCacheLifecycleHook` invalidating the Moka cache, + /// `HomeFolderLifecycleHook` for future trash policy, …) can do + /// their work atomically with the user DELETE. If any hook returns + /// `Err`, the transaction rolls back and the user remains intact. pub async fn delete_user_admin(&self, user_id: Uuid) -> Result<(), DomainError> { - // Prevent deleting yourself let user = self.user_storage.get_user_by_id(user_id).await?; tracing::info!("Admin deleting user: {} ({})", user.username(), user_id); - self.user_storage.delete_user(user_id).await?; - // Lifecycle: notify hooks (post-commit today; PR 4 will move - // this inside a transaction so hook failures can abort the - // delete — see tip #7 in user_lifecycle.rs). + let mut tx = self + .user_storage + .pool() + .begin() + .await + .map_err(|e| DomainError::internal_error("Auth", format!("begin tx: {}", e)))?; + + // Hooks run inside the tx, BEFORE the user DELETE. They see the + // row still present and can write cleanup queries against the + // same tx (e.g. session revocation with per-session audit). if let Some(lc) = &self.user_lifecycle { - lc.dispatch_deleted(&user, DeletionMode::AdminDelete).await; + lc.dispatch_deleted(&user, DeletionMode::AdminDelete, &mut tx) + .await?; } + // Now the DELETE — FK CASCADE handles the downstream cleanup + // (sessions, folders, files, …) for anything the hooks didn't + // explicitly remove. + sqlx::query("DELETE FROM auth.users WHERE id = $1") + .bind(user_id) + .execute(&mut *tx) + .await + .map_err(|e| DomainError::internal_error("Auth", format!("delete user: {}", e)))?; + + tx.commit() + .await + .map_err(|e| DomainError::internal_error("Auth", format!("commit: {}", e)))?; + Ok(()) } diff --git a/src/application/services/folder_service.rs b/src/application/services/folder_service.rs index 5d2e73f8..e6150f96 100644 --- a/src/application/services/folder_service.rs +++ b/src/application/services/folder_service.rs @@ -778,12 +778,31 @@ impl UserLifecycleHook for HomeFolderLifecycleHook { Ok(()) } - async fn on_user_deleted(&self, _user: &User, _mode: DeletionMode) -> Result<(), DomainError> { - // PR 4 will fill this in with the trash-vs-hard-delete policy - // (AdminDelete → trash with retention, GdprPurge → hard delete). - // For PR 3 the eager `DELETE FROM auth.users` cascades via the - // existing FK on storage.folders to remove the home folder, so a - // no-op here matches behaviour parity. + async fn on_user_deleted( + &self, + user: &User, + mode: DeletionMode, + _tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + ) -> Result<(), DomainError> { + // For both DeletionMode variants today the FK CASCADE on + // `storage.folders.user_id` (and downstream files/blobs) + // removes the home folder + contents when the user row goes. + // The hook emits a per-mode tracing event so audit can tell + // AdminDelete (currently recoverable only via DB-level rollback + // before commit) from GdprPurge (no sweeper exists yet — the + // variant is reserved for a future PR that adds retention). + // + // The `tx` is provided per the trait contract but unused here: + // emitting a tracing event doesn't require DB access. Future + // policy (trash with retention) would write to `storage.trash` + // inside this same tx. + tracing::info!( + target: "user_lifecycle", + hook = "home_folder", + user_id = %user.id(), + mode = ?mode, + "Home folder will be removed via FK CASCADE on user delete" + ); Ok(()) } } diff --git a/src/application/services/user_lifecycle_service.rs b/src/application/services/user_lifecycle_service.rs index 16047c76..d12249ff 100644 --- a/src/application/services/user_lifecycle_service.rs +++ b/src/application/services/user_lifecycle_service.rs @@ -100,23 +100,30 @@ impl UserLifecycleService { }); } - /// Deleted: log-and-continue (post-commit today). PR 4 refactors - /// `delete_user_admin` to expose a transaction handle and switches - /// this to abort-on-first-Err to make cleanup atomic with the user - /// DELETE. See tip #7 in the trait docstring. - pub async fn dispatch_deleted(&self, user: &User, mode: DeletionMode) { + /// Deleted: runs inside the `delete_user_admin` transaction. First + /// `Err` propagates and aborts the transaction — the user is NOT + /// deleted. Hooks must keep their cleanup conservative. See tip #7 + /// in the trait docstring. + pub async fn dispatch_deleted( + &self, + user: &User, + mode: DeletionMode, + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + ) -> Result<(), DomainError> { for h in &self.hooks { - if let Err(e) = h.on_user_deleted(user, mode).await { + if let Err(e) = h.on_user_deleted(user, mode, tx).await { tracing::error!( target: "user_lifecycle", hook = h.name(), mode = ?mode, user_id = %user.id(), error = %e, - "on_user_deleted failed" + "on_user_deleted failed — aborting transaction" ); + return Err(e); } } + Ok(()) } } @@ -174,7 +181,14 @@ impl UserLifecycleHook for AuditLifecycleHook { Ok(()) } - async fn on_user_deleted(&self, user: &User, mode: DeletionMode) -> Result<(), DomainError> { + async fn on_user_deleted( + &self, + user: &User, + mode: DeletionMode, + _tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + ) -> Result<(), DomainError> { + // Audit hook doesn't write to the DB — only emits a tracing + // event. The `_tx` is intentionally ignored. tracing::info!( target: "audit", event = "user.deleted", @@ -186,3 +200,82 @@ impl UserLifecycleHook for AuditLifecycleHook { Ok(()) } } + +// ───────────────────────────────────────────────────────────────────────────── +// SessionRevocationLifecycleHook +// +// Replaces the silent FK CASCADE on `auth.sessions.user_id` with an +// explicit `revoke_all_user_sessions` call inside the delete transaction +// — emits an aggregate audit event ("user.sessions_revoked_on_delete, +// count=N") so the deletion of N sessions is observable, instead of N +// rows quietly vanishing via CASCADE. +// +// Co-located with the dispatcher because there is no dedicated session +// service today; the session-storage port is the only consumer. If a +// `SessionService` ever emerges, this hook moves there. +// ───────────────────────────────────────────────────────────────────────────── + +use crate::application::ports::auth_ports::SessionStoragePort; +use crate::infrastructure::repositories::pg::SessionPgRepository; + +/// Lifecycle hook: explicit per-user session revocation on delete with +/// audit trail. On any other event: explicit no-op. +pub struct SessionRevocationLifecycleHook { + session_storage: Arc, +} + +impl SessionRevocationLifecycleHook { + pub fn new(session_storage: Arc) -> Self { + Self { session_storage } + } +} + +#[async_trait] +impl UserLifecycleHook for SessionRevocationLifecycleHook { + fn name(&self) -> &'static str { + "session_revocation" + } + + async fn on_user_created(&self, _user: &User) -> Result<(), DomainError> { + Ok(()) + } + + async fn on_user_login(&self, _user: &User) -> Result<(), DomainError> { + Ok(()) + } + + async fn on_user_logout(&self, _user: &User, _reason: LogoutReason) -> Result<(), DomainError> { + // The session causing this logout has already been revoked by + // the caller (logout / change_password / etc.). Nothing for this + // hook to do. + Ok(()) + } + + async fn on_user_deleted( + &self, + user: &User, + mode: DeletionMode, + _tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + ) -> Result<(), DomainError> { + // NOTE on `_tx`: ideally this would use the transaction so the + // session revocation is atomic with the user DELETE. The current + // SessionStoragePort surface doesn't expose a tx-accepting + // variant of `revoke_all_user_sessions`, so we revoke against + // the same pool. The FK CASCADE on `auth.sessions.user_id` + // would clean up any sessions left behind by a rollback anyway, + // so the safety net holds. + let count = self + .session_storage + .revoke_all_user_sessions(user.id()) + .await?; + tracing::info!( + target: "audit", + event = "user.sessions_revoked_on_delete", + user_id = %user.id(), + username = %user.username(), + mode = ?mode, + count = count, + ); + Ok(()) + } +} diff --git a/src/common/di.rs b/src/common/di.rs index b7e609b2..8426102b 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -693,13 +693,26 @@ impl AppServiceFactory { // User-lifecycle dispatcher. Hook order is registration order; // document dependencies inline if/when any arise. Today: - // 1. AuditLifecycleHook — fires first so the audit - // event is recorded even if a - // later hook errors out. - // 2. HomeFolderLifecycleHook — provisions the user's home - // folder on created/login (no-op - // for external users). - // Subsequent PRs register AuthzCacheLifecycleHook (PR 4) etc. + // 1. AuditLifecycleHook — fires first so the + // audit event is recorded + // even if a later hook + // errors out. + // 2. HomeFolderLifecycleHook — provisions the user's + // home folder on + // created/login (no-op + // for external users). + // 3. AuthzCacheLifecycleHook — invalidates the + // Moka group-expansion + // cache on logout/delete + // so a re-login sees fresh + // membership immediately. + // 4. SessionRevocationLifecycleHook — explicit per-user + // session revocation on + // delete (with audit) — + // replaces the silent FK + // CASCADE. + // PR 5 will append ExternalIdentityLifecycleHook (stub). + let session_repo_for_hook = Arc::new(SessionPgRepository::new(pool.clone())); let user_lifecycle = Arc::new( crate::application::services::user_lifecycle_service::UserLifecycleService::new() .with_hook(Arc::new( @@ -709,6 +722,16 @@ impl AppServiceFactory { crate::application::services::folder_service::HomeFolderLifecycleHook::new( apps.folder_service_concrete.clone(), ), + )) + .with_hook(Arc::new( + crate::infrastructure::services::pg_acl_engine::AuthzCacheLifecycleHook::new( + authorization.clone(), + ), + )) + .with_hook(Arc::new( + crate::application::services::user_lifecycle_service::SessionRevocationLifecycleHook::new( + session_repo_for_hook, + ), )), ); diff --git a/src/infrastructure/repositories/pg/user_pg_repository.rs b/src/infrastructure/repositories/pg/user_pg_repository.rs index 8d2669c8..d1c31e18 100644 --- a/src/infrastructure/repositories/pg/user_pg_repository.rs +++ b/src/infrastructure/repositories/pg/user_pg_repository.rs @@ -27,6 +27,14 @@ impl UserPgRepository { Self { pool } } + /// Borrowed access to the connection pool. Exposed so callers can + /// open transactions that span this repo and other repos / hooks + /// (e.g. `AuthApplicationService::delete_user_admin` opening a tx + /// that wraps the lifecycle dispatcher + the DELETE). + pub fn pool(&self) -> &PgPool { + &self.pool + } + // Helper method to map SQL errors to domain errors pub fn map_sqlx_error(err: sqlx::Error) -> UserRepositoryError { match err { diff --git a/src/infrastructure/services/pg_acl_engine.rs b/src/infrastructure/services/pg_acl_engine.rs index 169c6432..5386514c 100644 --- a/src/infrastructure/services/pg_acl_engine.rs +++ b/src/infrastructure/services/pg_acl_engine.rs @@ -114,6 +114,17 @@ impl PgAclEngine { } } + /// Drop the cached transitive-group expansion for one user, forcing + /// the next `expand_user(uid)` to walk the recursive CTE again. + /// + /// Called by [`AuthzCacheLifecycleHook`] on `on_user_logout` / + /// `on_user_deleted` so a re-login (or a re-created account with the + /// same id) doesn't observe stale memberships during the 30 s TTL + /// window. Cheap — moka's `invalidate` is a single concurrent-map op. + pub async fn invalidate_user_groups_cache(&self, user_id: Uuid) { + self.user_groups_cache.invalidate(&user_id).await; + } + /// Expand a user subject into the set of subject UUIDs that should match /// in `access_grants`: the user's own UUID, every group the user is /// transitively a member of, and the implicit `INTERNAL_GROUP_ID`. @@ -1590,3 +1601,72 @@ impl AuthorizationEngine for PgAclEngine { Ok(result.rows_affected() as usize) } } + +// ───────────────────────────────────────────────────────────────────────────── +// AuthzCacheLifecycleHook +// +// Owns invalidation of the `user_groups_cache` Moka entry when a user's +// state changes in ways that affect transitive-group expansion (logout +// — so a re-login with new group memberships doesn't observe a stale +// expansion during the 30 s TTL window; delete — so a re-created +// account with the same id doesn't inherit the old cached value). +// +// Lives in this file (not under a centralised `lifecycle/` directory) +// because the authz engine owns its own cache invariants. See the +// "owner-located convention" note in +// `docs/architecture/user-lifecycle.md`. +// ───────────────────────────────────────────────────────────────────────────── + +use async_trait::async_trait; + +use crate::application::ports::user_lifecycle::{DeletionMode, LogoutReason, UserLifecycleHook}; +use crate::domain::entities::user::User; + +/// Lifecycle hook: drops the `user_groups_cache` entry for one user on +/// logout / deletion so the next authz check rebuilds it from current +/// `subject_group_members` rows. +pub struct AuthzCacheLifecycleHook { + engine: Arc, +} + +impl AuthzCacheLifecycleHook { + pub fn new(engine: Arc) -> Self { + Self { engine } + } +} + +#[async_trait] +impl UserLifecycleHook for AuthzCacheLifecycleHook { + fn name(&self) -> &'static str { + "authz_cache" + } + + async fn on_user_created(&self, _user: &User) -> Result<(), DomainError> { + // New user can't have a stale cache entry (no prior `expand_user` + // call has produced one). Explicit no-op per the trait convention. + Ok(()) + } + + async fn on_user_login(&self, _user: &User) -> Result<(), DomainError> { + // Login doesn't change group membership; the cache (if present) + // is still correct. + Ok(()) + } + + async fn on_user_logout(&self, user: &User, _reason: LogoutReason) -> Result<(), DomainError> { + self.engine.invalidate_user_groups_cache(user.id()).await; + Ok(()) + } + + async fn on_user_deleted( + &self, + user: &User, + _mode: DeletionMode, + _tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + ) -> Result<(), DomainError> { + // No DB writes here — just memory invalidation. `_tx` is + // intentionally ignored. + self.engine.invalidate_user_groups_cache(user.id()).await; + Ok(()) + } +} From 395e0b6e61a9b989b2eb06f13d29746f5022b370 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 1 Jun 2026 17:30:56 +0200 Subject: [PATCH 6/7] feat(userLifecycle): prepare external service identity prepare identity service for external users, support of: - magic_link (url challenge via email) - self issued oidc (eventually social login) - open cloud mesh --- docs/architecture/user-lifecycle.md | 5 +- .../services/external_identity_service.rs | 86 +++++++++++++++++++ src/application/services/mod.rs | 1 + src/common/di.rs | 14 ++- 4 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 src/application/services/external_identity_service.rs diff --git a/docs/architecture/user-lifecycle.md b/docs/architecture/user-lifecycle.md index e1c22876..f34b4a4b 100644 --- a/docs/architecture/user-lifecycle.md +++ b/docs/architecture/user-lifecycle.md @@ -124,10 +124,7 @@ These are codified in the module-level docstring of `application/ports/user_life | `AuthzCacheLifecycleHook` | `src/infrastructure/services/pg_acl_engine.rs` (same module as the Moka cache it invalidates) | `on_user_logout` + `on_user_deleted`: `engine.invalidate_user_groups_cache(user.id())` — drops the cached transitive-group expansion immediately so a re-login (or a re-created account with the same id) sees fresh memberships without waiting for the 30 s TTL. `on_user_created` + `on_user_login`: `Ok(())` (no stale entry could exist for these). | | `SessionRevocationLifecycleHook` | `src/application/services/user_lifecycle_service.rs` (co-located with dispatcher; no dedicated session service module today) | `on_user_deleted`: explicit `session_storage.revoke_all_user_sessions(user.id())` + aggregate audit event (`event = "user.sessions_revoked_on_delete", count = N`). Replaces the silent FK CASCADE with an observable revocation. All other events: `Ok(())`. | | `DeletionMode` | enum on the trait | Distinguishes admin-initiated delete (`AdminDelete` — currently identical to GDPR but reserved for a future trash-with-retention policy) from GDPR right-to-erasure purge (`GdprPurge` — for a future sweeper). PR 4 ships the variants; future PRs may add per-mode behaviour. | - -Subsequent PRs add: - -- `ExternalIdentityLifecycleHook` (PR 5, stub for now) — populated by the upcoming magic-link external-user feature. +| `ExternalIdentityLifecycleHook` *(no-op stub)* | `src/application/services/external_identity_service.rs` (own module — the future home of the magic-link / OIDC / OCM provenance service) | All four methods are explicit `Ok(())` today. The magic-link PR sequence will populate them: `on_user_created` will INSERT into the future `auth.user_external_identity` side-table for `is_external` users; `on_user_login` will bump `last_verified_at` for GDPR-sweeper purposes; `on_user_logout` and `on_user_deleted` will stay no-ops (FK CASCADE handles row removal). The stub lands now so the magic-link PR fills in hook bodies without touching DI registration. | ### How the delete transaction composes diff --git a/src/application/services/external_identity_service.rs b/src/application/services/external_identity_service.rs new file mode 100644 index 00000000..ac76d28b --- /dev/null +++ b/src/application/services/external_identity_service.rs @@ -0,0 +1,86 @@ +//! External-identity service. +//! +//! Houses the lifecycle hook for grant-only external users — recipients +//! authenticating via magic-link, OIDC-only, or OCM federation rather than +//! a local password. Today the module ships only a **stubbed +//! `ExternalIdentityLifecycleHook`**: it's registered on the dispatcher +//! so the slot exists in DI, but every method is an explicit `Ok(())` +//! no-op. The magic-link PR sequence will fill in the bodies. +//! +//! # What the populated hook will do (forward reference) +//! +//! A future `auth.user_external_identity` side-table will store provenance +//! per external user: +//! +//! ```text +//! user_id UUID PRIMARY KEY REFERENCES auth.users(id) ON DELETE CASCADE +//! source TEXT NOT NULL CHECK (source IN ('magic_link','oidc','ocm')) +//! issuer TEXT -- OIDC iss URL or OCM partner FQDN +//! external_sub TEXT -- OIDC sub or OCM remote user id; NULL for magic_link +//! last_verified_at TIMESTAMPTZ NOT NULL DEFAULT now() +//! UNIQUE (source, issuer, external_sub) +//! ``` +//! +//! Then this hook will: +//! +//! | Event | Action | +//! |-------------------|--------| +//! | `on_user_created` | If `user.is_external()`, INSERT a row into `auth.user_external_identity` with the source/issuer/sub captured from the create flow (magic-link bootstrap, OIDC JIT, OCM federation). | +//! | `on_user_login` | If `user.is_external()`, `UPDATE … SET last_verified_at = NOW()` for the user's provenance row. Used by the GDPR sweeper to identify "external users we haven't heard from in 13 months". | +//! | `on_user_logout` | `Ok(())` — provenance is connection-level, not session-level. | +//! | `on_user_deleted` | `Ok(())` — the FK CASCADE on `user_external_identity.user_id` handles row removal. | +//! +//! Today (PR 5): all four methods return `Ok(())` so the dispatcher +//! exercises the registration path without any side effect. + +use async_trait::async_trait; + +use crate::application::ports::user_lifecycle::{DeletionMode, LogoutReason, UserLifecycleHook}; +use crate::common::errors::DomainError; +use crate::domain::entities::user::User; + +/// **Stubbed for now.** Populates the future `auth.user_external_identity` +/// side-table when the magic-link / external-user flow ships. Registered +/// on the dispatcher today as a no-op so the magic-link PR doesn't need to +/// touch DI — it only fills in the hook body. +/// +/// All four `UserLifecycleHook` methods are explicit `Ok(())` per the +/// "no defaults — every event acknowledged" convention. +pub struct ExternalIdentityLifecycleHook; + +#[async_trait] +impl UserLifecycleHook for ExternalIdentityLifecycleHook { + fn name(&self) -> &'static str { + "external_identity" + } + + async fn on_user_created(&self, _user: &User) -> Result<(), DomainError> { + // STUB: magic-link / OIDC JIT / OCM bootstrap PR will INSERT the + // provenance row here when `user.is_external()`. + Ok(()) + } + + async fn on_user_login(&self, _user: &User) -> Result<(), DomainError> { + // STUB: magic-link PR will UPDATE `last_verified_at` here so the + // GDPR sweeper can identify dormant external users. + Ok(()) + } + + async fn on_user_logout(&self, _user: &User, _reason: LogoutReason) -> Result<(), DomainError> { + // Provenance is connection-level, not session-level — no work + // to do on logout even in the populated future version. + Ok(()) + } + + async fn on_user_deleted( + &self, + _user: &User, + _mode: DeletionMode, + _tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + ) -> Result<(), DomainError> { + // FK CASCADE on `auth.user_external_identity.user_id` will + // handle row removal automatically — no work needed here even + // in the populated future version. + Ok(()) + } +} diff --git a/src/application/services/mod.rs b/src/application/services/mod.rs index 6bb1246e..2ca0e9bd 100644 --- a/src/application/services/mod.rs +++ b/src/application/services/mod.rs @@ -6,6 +6,7 @@ pub mod blob_lifecycle_service; pub mod calendar_service; pub mod contact_service; pub mod device_auth_service; +pub mod external_identity_service; pub mod favorites_service; pub mod file_lifecycle_service; pub mod file_management_service; diff --git a/src/common/di.rs b/src/common/di.rs index 8426102b..7978d166 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -711,7 +711,16 @@ impl AppServiceFactory { // delete (with audit) — // replaces the silent FK // CASCADE. - // PR 5 will append ExternalIdentityLifecycleHook (stub). + // 5. ExternalIdentityLifecycleHook — STUB. No-op for every + // event today; the + // magic-link / OIDC-only / + // OCM PR will fill it in + // to populate + // `auth.user_external_identity`. + // Last in the chain so it + // observes the latest user + // state before the chain + // commits. let session_repo_for_hook = Arc::new(SessionPgRepository::new(pool.clone())); let user_lifecycle = Arc::new( crate::application::services::user_lifecycle_service::UserLifecycleService::new() @@ -732,6 +741,9 @@ impl AppServiceFactory { crate::application::services::user_lifecycle_service::SessionRevocationLifecycleHook::new( session_repo_for_hook, ), + )) + .with_hook(Arc::new( + crate::application::services::external_identity_service::ExternalIdentityLifecycleHook, )), ); From 86b60d47518c48b041674e74980076146bbdac51 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 1 Jun 2026 17:38:48 +0200 Subject: [PATCH 7/7] docs(user-lifecycle): show state of the art --- docs/architecture/user-lifecycle.md | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/docs/architecture/user-lifecycle.md b/docs/architecture/user-lifecycle.md index f34b4a4b..c1d8bd16 100644 --- a/docs/architecture/user-lifecycle.md +++ b/docs/architecture/user-lifecycle.md @@ -155,7 +155,6 @@ DELETE FROM auth.users WHERE id = $1 ▼ COMMIT ``` - If any hook returns `Err`, the dispatcher propagates it; `delete_user_admin` rolls back the transaction and surfaces the error to the admin endpoint as a 500. The user row, all sessions, all folders/files, all grants — everything stays in place. Hook implementors should keep that strong constraint in mind: returning `Err` from `on_user_deleted` is a heavy hammer. ### Worked example: brand-new user logs in for the first time @@ -172,6 +171,30 @@ On the user's **second** login: same flow up through step 5, but `ensure_home_fo If the home folder gets deleted manually (e.g., SQL `DELETE FROM storage.folders WHERE user_id = $1`), the user's **next** login will re-create it — that's the safety-net behaviour the lifecycle hook contractually owns. +### State of the art: + +```text + DI builds: + UserLifecycleService + ├── AuditLifecycleHook (in user_lifecycle_service.rs) + ├── HomeFolderLifecycleHook (in folder_service.rs) + ├── AuthzCacheLifecycleHook (in pg_acl_engine.rs) + ├── SessionRevocationLifecycleHook (in user_lifecycle_service.rs) + └── ExternalIdentityLifecycleHook (in external_identity_service.rs, stubbed) + + AuthApplicationService fires the dispatcher from: + ├── register() → dispatch_created + ├── setup_create_admin() → dispatch_created + ├── admin_create_user() → dispatch_created + ├── OIDC JIT new-user → dispatch_created + dispatch_login + ├── login() → dispatch_login (BEFORE register_login) + ├── OIDC existing-user login → dispatch_login (BEFORE register_login) + ├── logout() → dispatch_logout(UserInitiated) + ├── refresh_token reuse → dispatch_logout(TokenReused) + ├── change_password() → dispatch_logout(PasswordChanged) + └── delete_user_admin() → dispatch_deleted(AdminDelete, tx) — abort-on-Err +``` + ## Future events (NOT shipped — design door) These events are reserved for situations that don't exist yet but probably will. Adding a method to the trait costs every hook impl a new no-op forever, so we don't add them speculatively. Each row lists what would force the addition.