diff --git a/docs/plan/auth-simplification.md b/docs/plan/auth-simplification.md new file mode 100644 index 00000000..ee1c93f8 --- /dev/null +++ b/docs/plan/auth-simplification.md @@ -0,0 +1,278 @@ +# Plan — Auth simplification (PR 16–21) + +## Context + +The magic-link work (PRs 6–12) shipped external users as a second principal kind with `username = email`. PR 13 closed the route-level lockouts. Across a design conversation on 2026-06-02 we agreed the resulting model is needlessly two-tier and can be simplified by making **email the identity**, **username an optional handle**, and **credentials (password / OIDC) truly optional and orthogonal**. The proximate motivation: avoid the username-vs-email cross-collision class of bugs we just spent time guarding against, and reduce password-hash density in the DB by letting users sign up email-only with magic-link as their bootstrap path. + +The end state: every user is identified by email; `username`, `password_hash`, and `oidc_subject` are all `Option<…>` columns whose presence is observable but never *required*. Login is a one-line dispatch on `@`-in-input. Magic-link eligibility is a three-branch rule (OIDC always rejected, password rejected by default, no-credential allowed) with one env knob to flip the middle branch. No new auth methods land here — TOTP / WebAuthn / passkey enrolment stays out of scope. The whole work is reorganisation of existing primitives, not new ones. + +## Design decisions (locked in by conversation) + +### Identity model + +| Slot | Type | Meaning | +|---|---|---| +| `email` | `String` (NOT NULL UNIQUE) | The identity. Every login path ultimately resolves here. | +| `username` | `Option` (UNIQUE, NULL allowed) | Optional handle. 2-64 chars inclusive, `[A-Za-z0-9._-]+` (no `@`). Claimable post-creation. Multiple NULLs coexist under the existing UNIQUE index. | +| `password_hash` | `Option` | An Argon2 hash if the user chose one. NULL otherwise. NO placeholder strings. | +| `oidc_subject` | `Option` | The IdP subject claim if the user linked one. NULL otherwise. | +| `is_external` | `bool` | Provisioning origin marker. `true` when created via email-invite from a sharer. Future "promote to internal" flow (separate TODO) flips this. | + +Eligibility predicates derive from the slots: + +```rust +fn has_password(&self) -> bool { self.password_hash.is_some() } +fn has_oidc(&self) -> bool { self.oidc_subject.is_some() } +fn has_login_credential(&self) -> bool { + self.has_password() || self.has_oidc() +} +``` + +No more sentinel strings (`__EXTERNAL_NO_PASSWORD__`, `__OIDC_NO_PASSWORD__`). The schema migration NULLs them out as part of PR 16. + +### Login dispatch + +``` +input contains '@' → lookup by email, verify password +input does not → lookup by username, verify password +``` + +Single DB hit. Unambiguous because `@` is forbidden in `username`. The same dispatcher serves the magic-link send endpoint, but that endpoint only takes email — input without `@` is a 400. + +### Magic-link eligibility ladder + +```rust +pub fn magic_link_eligibility(user: &User, open_to_password_users: bool) -> Eligibility { + if user.has_oidc() { return Reject("oidc_user"); } // unconditional; IdP is the security boundary + if user.has_password() { + return if open_to_password_users { Allow } else { Reject("has_password") }; + } + Allow // no credentials at all +} +``` + +| User state | Magic-link eligible? | +|---|---| +| No password, no OIDC | Yes — always | +| Has password, no OIDC | Default no; `OXICLOUD_MAGIC_LINK_OPEN_TO_PASSWORD_USERS=true` opens it | +| Has OIDC (with or without password) | **No — always.** Flag has no effect. | + +The `oidc_user` reject is unconditional because OIDC is the only path to MFA today (delegated to the IdP), and magic-link must never bypass it. Future native 2FA (TOTP, WebAuthn) lands a third reject branch behind the existing patterns. + +### Registration paths + +| Path | Pre-condition | Effect | +|---|---|---| +| `POST /api/auth/register` with `email + password` | Public registration enabled | User row with password_hash set. JWT issued. | +| `POST /api/auth/register` with `email` only | Public registration enabled | User row with `password_hash = None`. Magic-link mailed to the address for first-session bootstrap. JWT NOT issued by the register call. | +| `POST /api/grants { subject.type: "email" }` | Sharer has Share permission | External user lazily provisioned. Magic-link invitation mailed. Existing PR 9 flow. | +| OIDC JIT | First IdP-mediated login | User row with `oidc_subject` set, `password_hash = None`. JWT issued. | + +All four return the **same** uniform shape regardless of email existence (PR 20 closes the register oracle that survives from the original schema). Real reason recorded in audit. + +### Anti-enumeration is preserved everywhere + +- `POST /api/auth/register` → 200 uniform, audit reasons `created` / `email_taken` / `username_taken` / `disabled` +- `POST /api/auth/magic-link/send` → 200 uniform, audit reasons `sent` / `no_account` / `has_password` / `oidc_user` / `account_deactivated` / `malformed_email` / `rate_limited_email` / `rate_limited_ip` +- `POST /api/auth/login` → 403 uniform `Invalid credentials`, audit reasons `unknown_user` / `bad_password` / `account_deactivated` + +### Migration discipline + +Forward-only migrations. The previous `…000003_users_username_email_login.sql` widening to 254 chars has already been applied to dev / CI environments — squashing with the new shrink-and-NULL migration would break `_sqlx_migrations` checksum tracking. Add a fresh migration file; history records the two-step story honestly. + +### What stays the same + +- JWT structure, session lifetimes, refresh-token rotation +- `is_external` flag and its DB CHECK constraints (`users_external_not_admin`, `users_external_no_storage`) +- All PR 13 route-level lockouts (external users still can't reach CalDAV/CardDAV/WebDAV/NC, can't mint app passwords, can't enumerate groups) +- ReBAC `access_grants` table and all permission semantics +- SMTP-unconfigured ⟹ magic-link unavailable ⟹ external invitations unavailable (existing 503 paths) +- The three rate limiters from PR 12 — caps and keys unchanged + +## PR sequence + +| PR | Subject | Why land separately | +|---|---|---| +| **16** | Schema + entity: nullable `username` / `password_hash`, format CHECK, sentinel cleanup, `User::new` collapse, audit-log Option handling | Foundational — every later PR consumes `Option` columns. Verifiable in isolation by re-running the existing Hurl suite. | +| **17** | Login dispatcher: input `contains('@')` decides email vs. username path. Pure refactor of `AuthApplicationService::login`. | One behavioural axis; isolated test surface. | +| **18** | Optional password at registration: `RegisterDto.password: Option`. Password-less path mints a magic-link to the supplied email. | The "email-only signup" UX. Depends on 16. | +| **19** | `OXICLOUD_MAGIC_LINK_OPEN_TO_PASSWORD_USERS` env knob + `magic_link_eligibility()` refactor with three audit reasons. | Single env-driven policy switch with observability. | +| **20** | Anti-enumeration `register`: uniform 200 regardless of outcome, real reasons in audit channel. | Closes the username/email enumeration oracle that survives from the original schema. | +| **21** | `docs/architecture/auth-model.md` (~250 lines) + sidebar + cross-references. Acceptance gate. | Big-picture documentation of the final identity / credential / login surface. | + +## Critical files + +### New + +- `migrations/20260603000000_username_optional_no_email_shape.sql` — username + password_hash schema cleanup +- `tests/api/auth_login.hurl` — dispatcher coverage (PR 17) +- `tests/api/registration.hurl` — email-only signup + anti-enumeration (PR 18 + PR 20) +- `docs/architecture/auth-model.md` — final architecture page (PR 21) + +### Modified — by PR + +**PR 16:** +- `src/domain/entities/user.rs` — `username: Option`, `password_hash: Option`, `oidc_subject` confirmed `Option`. Collapse `new_password` / `new_oidc` / `new_external` into one `User::new` (or keep three named factories if readability outweighs the duplication). Drop sentinel string comparisons; `has_login_credential` becomes a two-line `Option::is_some` check. +- `src/infrastructure/repositories/pg/user_pg_repository.rs` — Option binding in all 8 SELECT/INSERT/UPDATE sites +- `src/application/dtos/user_dto.rs` — `username: Option` +- `src/application/services/magic_link_invite_service.rs::resolve_or_create_recipient` — pass `None` for username when minting an external +- `src/application/services/auth_application_service.rs` — every audit line emitting `username = %user.username()` switches to a `display_for_audit(&user)` helper that falls back to user_id when `None` +- `src/interfaces/api/handlers/contacts_handler.rs::user_to_contact` — already handles None fallback (from yesterday's given/family work), but verify after the type change +- `src/interfaces/nextcloud/routes.rs::verify_url_user` — when `auth_user.username` is None, return 403 instead of comparing to the URL segment (externals are already PR-13-blocked but the type change forces an explicit branch) +- Every `audit` log line in `auth_handler.rs`, `auth_application_service.rs`, `magic_link_invite_service.rs` — review for `username =` interpolations + +**PR 17:** +- `src/application/services/auth_application_service.rs::login` — dispatch on `@` +- `src/application/dtos/user_dto.rs::LoginDto` — rename `username` field to `username_or_email` with a serde alias for backward compat, or leave the name and document the new semantics +- Frontend `static/login.html` — change the input placeholder from "Username" to "Username or email" + +**PR 18:** +- `src/application/dtos/user_dto.rs::RegisterDto.password: Option` + validation: when present, enforce length minimum +- `src/application/services/auth_application_service.rs::register` — branch on `dto.password.as_ref()`. When None: create user with `password_hash = None`, then call `MagicLinkInviteService::send_login_link(&dto.email)` as a best-effort post-action +- `src/interfaces/api/handlers/auth_handler.rs::register` — uniform 201 either way + +**PR 19:** +- `src/common/config.rs::MagicLinkConfig.open_to_password_users: bool` (default false) + env loader +- `src/application/services/magic_link_invite_service.rs` — new `magic_link_eligibility()` function; replace the single `if user.has_login_credential()` check in `send_login_link` and `issue_invitation` +- `example.env` — new entry with explanation + +**PR 20:** +- `src/application/services/auth_application_service.rs::register` — silence the descriptive error strings; return Ok with the same uniform shape on collision; audit-log the truth +- `src/interfaces/api/handlers/auth_handler.rs::register` — response body becomes the uniform "If the email is available, a confirmation link has been sent." +- Hurl regression: the existing register test in `tests/api/setup.hurl` etc. needs to assert the new uniform shape + +**PR 21:** +- `docs/architecture/auth-model.md` (new file, ~250 lines) +- `docs/.vitepress/config.mts` — sidebar entry after `magic-link-auth` +- `docs/architecture/magic-link-auth.md` — § "Identity model" links to `auth-model.md` +- `docs/architecture/share-integration.md` — already references `magic-link-auth`; chain stays one hop deep + +## Existing patterns to reuse (with paths) + +- **MockEmailSender** at `src/infrastructure/services/mock_email_sender.rs` — every PR 18 / 20 Hurl test uses it via `GET /api/admin/smtp/test/captured?to=…`. Already configured in `tests/common/server.env` (`OXICLOUD_SMTP_MOCK=true`). +- **Audit-log convention** from `CLAUDE.md` § Authorization — every reject emits `tracing::info!(target: "audit", event = ".", reason = "", …)` with structured fields. New reasons in this work: + - `auth.register_rejected` with `email_taken`, `username_taken`, `disabled` + - `auth.magic_link_send` gains `has_password`, `oidc_user` (replacing single `has_credential`) + - `auth.login_rejected` reasons unchanged +- **Migration mirroring** — `migrations/20260612000003_users_username_email_login.sql` (or the actual existing file) is the precedent for username-column changes. New migration sits beside it. +- **Three rate limiters from PR 12** — caps and keys unchanged. No new limiters in this work. +- **Eligibility split pattern** — `Eligibility::Allow / Reject(reason)` enum is new; canonical home is `application/services/magic_link_invite_service.rs` next to the existing `MagicLinkResourceKind`. + +## Verification + +Per-PR (mandatory, every PR): +```bash +cargo fmt --all +cargo clippy --all-features --all-targets -- -D warnings +cargo test --workspace --lib +bash tests/api/run.sh +``` + +Frontend checks when touching `static/`: +```bash +biome check --fix static/ +stylelint static/css/ +tsc -p jsconfig.json --noEmit +``` + +### End-to-end gates + +**After PR 16** (smoke): the existing 14 Hurl files still pass. Bob's flow in `external_users.hurl` works with bob's username now NULL (assertion updates in `Step 11c` and `Step 12`). + +**After PR 17:** +1. Hurl: alice (internal, picked username "alice") logs in via `username = "alice"` → 200 +2. Hurl: alice logs in via `username = "alice@oxicloud.local"` (her email) → 200 +3. Hurl: bob (external, NULL username) logs in via `username = "bob@externalcompany.com"` → 403 (no password); via magic-link path → still works as before +4. Hurl: wrong password on either path → uniform `403 Invalid credentials` + +**After PR 18:** +1. Hurl: `POST /api/auth/register` with `{email, password}` → 201, JWT returned (existing behaviour) +2. Hurl: `POST /api/auth/register` with `{email}` only → 200 with uniform body; `GET /api/admin/smtp/test/captured?to=…` returns the welcome magic-link +3. Hurl: follow the captured URL → session cookies set, redirect to `/#/` +4. Hurl: user now has `password_hash = NULL` in DB; subsequent magic-link send is allowed (eligible) +5. Hurl: user sets a password via `PUT /api/auth/change-password` → next magic-link send to same email returns 200 but NO new mail captured (strict mode + `has_password` audit) + +**After PR 19:** +1. Hurl in default mode: password user gets no mail on magic-link send (existing strict-mode test) +2. Hurl in lenient mode (`OXICLOUD_MAGIC_LINK_OPEN_TO_PASSWORD_USERS=true` in `tests/common/server.env`, or a separate run): mail IS captured for the same scenario +3. Hurl: OIDC user → no mail regardless of flag value (the `oidc_user` audit reason fires unconditionally) +4. Unit test: 16-row matrix of `(has_password, has_oidc, is_external, flag)` × eligibility outcome + +**After PR 20:** +1. Hurl: `POST /api/auth/register` with an already-used email → 200 uniform; no new user row in DB; audit log line `auth.register_rejected reason=email_taken` +2. Hurl: `POST /api/auth/register` with an already-used username → 200 uniform; same shape +3. Hurl: timing comparison (informal) — collision path returns within ±10ms of the success path + +**After PR 21** (acceptance): +- `auth-model.md` renders correctly in VitePress dev (`cd docs && npm run dev`) +- Sidebar shows the new entry under Architecture +- Every audit reason listed in the doc is grep-able to a real `tracing::info!` call in `src/` + +## Out of scope (do NOT bundle) + +Items the conversation explicitly deferred. Each has a clear future trigger. + +- **Native 2FA (TOTP, WebAuthn enrolment) for password users.** Today OIDC delegation is the only MFA path; native enrolment requires UI, recovery codes, and a third reject branch in `magic_link_eligibility()` (`mfa_enrolled`). Listed in `auth-model.md` § "What is deliberately out of scope". +- **`login_strategy` per-user policy enum.** The conversation surfaced this as a future architectural direction. Captured in `auth-model.md` § "Future direction — per-user login strategy" with the matrix (`passwordless`, `password`, `password_or_magic_link`, `password_and_magic_link`, `oidc`, `password_and_totp`, `password_and_webauthn`). No implementation in this work. +- **External-user promotes to internal.** Triggered when an external sets a credential. Today `is_external` stays TRUE post-credential-set; the upgrade flips it to FALSE and provisions a home folder + Internal-group membership + DAV access. Depends on registration UX direction (Design A "claim username when needed" vs. Design B "auto-generate handle") being decided. Separate work. +- **`session_kind` on sessions emitted from magic-link.** A magic-link session today is indistinguishable from a password session. Enables scoped sessions later (Option-B style "magic-link session can only access granted resources"). Not load-bearing for v1. +- **Differentiated session TTL for externals.** Uniform refresh-token expiry today. Future env `OXICLOUD_EXTERNAL_REFRESH_TOKEN_EXPIRY_DAYS`. +- **Open Cloud Mesh (OCM) federation.** Third source for external provisioning. The `ExternalIdentityLifecycleHook::on_user_created` design accommodates the `source` discriminator (`magic_link` / `oidc` / `ocm`). +- **Recovery codes / passkey enrolment.** Tied to native 2FA above. +- **Per-user opt-out of magic-link when lenient mode is on.** Today the env flag is instance-wide. A future per-account toggle (e.g. high-privilege admins disabling magic-link for themselves) would need an `auth.users.magic_link_disabled BOOLEAN` column and one extra branch in eligibility. Listed in `auth-model.md`. + +## Locked-in design decisions (confirmed before PR 16) + +1. **Single `User::new` constructor.** Signature: `User::new(email, password_hash: Option, oidc_subject: Option, is_external: bool, …)`. The three named factories (`new_password` / `new_oidc` / `new_external`) collapse into call-site helpers if needed, but the canonical API is one constructor. +2. **`OXICLOUD_MAGIC_LINK_OPEN_TO_PASSWORD_USERS` is symmetric.** When `true`, both `/api/auth/magic-link/send` AND `/api/grants` email-invitations mail through to password users. Single `magic_link_eligibility()` predicate, same audit reason `has_password` in both code paths. +3. **Email-only signup welcome redemption lands on `/#/files`.** Redemption logic adjustment: NULL `resource_id` + `is_external = false` → `/#/files`; NULL + `is_external = true` → `/#/sharedwithme` (existing rule). One extra branch in `magic_link_handler::redeem`. +4. **`LoginDto.username` field name kept** with docstring updated to "accepts email or username". Frontend already sends the input as `username` regardless of what the user typed. Mentioned in `auth-model.md` as an intentional ambiguity in the API shape. + +## Recommended future event triggers (DON'T ship in this work) + +Same convention as previous plans — a future event ships only when there's a concrete consumer. + +| Future event | What would force it | +|---|---| +| `on_password_set` | When a user first sets a password — useful for audit ("alice is no longer magic-link-eligible" or "alice is now lenient-mode-eligible") and for invalidating any outstanding magic-link tokens she has. Today the new tokens are simply unused; reaping them via this event would be cleaner. | +| `on_oidc_linked` | When a user first links OIDC — useful for the same audit purpose, and especially load-bearing because OIDC linkage permanently disables magic-link. | +| `on_username_claimed` | When a NULL-username user picks one. Audit visibility for the share-modal autocomplete suddenly showing a new entry. | +| `on_credential_revoked` | When a user removes their password or unlinks OIDC. Reverses the eligibility decision. | + +These are doc-only; their absence doesn't block anything. + +## Doc skeleton — `auth-model.md` + +The PR 21 deliverable. Headings only: + +1. Why this page +2. Identity model (email, username, credentials) +3. Credential slots and the eligibility derivation +4. Login paths + - Username + password + - Email + password + - Email + magic-link (with the 3-branch table) + - OIDC redirect +5. Login dispatcher — how the input is interpreted (the `@`-in-input rule) +6. Registration paths + - Email + password + - Email-only + - OIDC JIT + - Email invitation +7. Anti-enumeration — what each endpoint returns +8. Security trade-offs + - Mailbox-as-bypass when `OPEN_TO_PASSWORD_USERS=true` + - Why OIDC is unconditionally excluded + - No native 2FA today; how OIDC delegation provides MFA via IdPs + - Rate-limit caps +9. Audit events — table +10. Migration path for existing instances +11. Future direction — per-user `login_strategy` (sketch with the 7-row matrix) +12. What is deliberately out of scope +13. Related documents (cross-refs) + +Target: ~250 lines, big-picture, no code walkthroughs (in line with `magic-link-auth.md`). + +--- + +**Status**: ready to start at PR 16 on confirmation. No code written yet. No tasks created in the TaskCreate system — those happen per-PR when implementation begins.