cjw/Oxicloud
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

plan: auth simplification

Browse Source
This commit is contained in:
Edouard Vanbelle
2026-06-02 20:02:27 +02:00
parent 803849fe2e
commit 37b1c2703b
1 changed files with 278 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plan/auth-simplification.md
+278
View File
@@ -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<String>` (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<String>` | An Argon2 hash if the user chose one. NULL otherwise. NO placeholder strings. |
| `oidc_subject` | `Option<String>` | 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<String>` 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<String>`. 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<String>`, `password_hash: Option<String>`, `oidc_subject` confirmed `Option<String>`. 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<String>`
- `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<String>` + 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 = "<domain>.<verb>", reason = "<key>", …)` 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<String>, oidc_subject: Option<String>, 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.