From 0b9e8bfe238e870cf5c3d25ad2107d9d65ec75da Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sun, 6 Sep 2026 23:19:22 +0200 Subject: [PATCH 1/2] docs(plan): case-insensitive usernames Records the design for issue #691 (make usernames case-insensitive): silently lowercase on ingest, explicit `oxicloud migrate lowercase-usernames [--dry-run]`, refuse-to-boot until DB is fully lowercase. Includes the chunked-upload directory rename step and the OIDC JIT lowercase fix surfaced during the design sweep. Design deferred pieces (display_name split, WebDAV URL redesign) are listed under "Not in scope" so the boundary is explicit. Co-Authored-By: Claude Opus 4.7 --- docs/plan/username-lowercase.md | 524 ++++++++++++++++++++++++++++++++ 1 file changed, 524 insertions(+) create mode 100644 docs/plan/username-lowercase.md diff --git a/docs/plan/username-lowercase.md b/docs/plan/username-lowercase.md new file mode 100644 index 00000000..310cd25f --- /dev/null +++ b/docs/plan/username-lowercase.md @@ -0,0 +1,524 @@ +# Plan — Case-insensitive usernames (lowercase-on-ingest) + +## Context + +Feature ask: [issue #691](https://github.com/AtalayaLabs/OxiCloud/issues/691). +Usernames are currently case-sensitive, so `Alice`, `alice`, and `ALICE` +refer to three different accounts. Users hit this as a login friction — +they type their name with different capitalization on different clients +and get "invalid credentials" instead of a successful login. + +## Why this is simpler than it looks in this codebase specifically + +- `validate_username` in `src/domain/entities/user.rs:884` already + restricts usernames to ASCII-only `[a-zA-Z0-9._-]{2,64}` with no `@`. + The Unicode case-folding minefield (Turkish dotted-I, German ß, Greek + final sigma, NFC vs NFD) does not apply — ASCII case-folding is + trivial (`to_ascii_lowercase`), deterministic, and locale-independent. +- OIDC identity binds via `(iss, sub)` in + `get_user_by_federation_subject` at + `src/infrastructure/repositories/pg/user_pg_repository.rs:1257-1303` — + case-sensitivity of the local username is orthogonal to OIDC identity + matching. No OIDC breakage risk. +- Password verification runs through Argon2's `verify_password` + (constant-time by construction). Not affected. +- `@`-forbidden rule in usernames is the disjoint namespace with email + lookup (`dispatch_login` at `auth_application_service.rs:1018`). + Case-insensitive usernames align semantics with email addresses + (already case-insensitive in practice), so any future + `groupname@domain` composition stays consistent. +- NextCloud URL `/remote.php/dav/files/{user}/…` uses `{user}` as an + informational / consistency-check marker, not a security boundary — + the chroot ACL is the real authz. Handling case in the URL segment is + a small local change (documented in `session.rs:33-34`). + +## Design decisions + +1. **Silently lowercase on ingest** (registration, admin-create, OIDC + provisioning, rename). Never reject uppercase input from clients — + accept liberally, store strictly (Postel's Law). +2. **Explicit migration** (`oxicloud migrate lowercase-usernames + [--dry-run]`). The server never mutates `auth.users` at boot. Ops + MUST run the migration explicitly. Follows the + [[feedback_no_silent_auto_repair]] rule: consistency tenants are + discovery-only by default; mutation is opt-in. +3. **Refuse-to-boot** if any active-user username is not already + lowercase. Boot error message shows the exact CLI command to run. + Boot performs a read-only verification only. +4. **Collision tiebreak** on migration: `(last_login_at DESC NULLS + LAST, created_at ASC)`. Winner keeps the canonical lowercased name. + Losers get `-2`, `-3`, … suffix (increment until free), matching the + pattern in `oxicloud migrate nfc-filenames`. +5. **Active accounts only** in the boot check + migration. Soft-deleted + / disabled rows are skipped (they don't block usable logins). NULL + usernames (OPAQUE-migrated accounts) are skipped in every layer — + the boot verifier, the migration UPDATE, the CLI report. The + `WHERE username <> LOWER(username)` predicate is already NULL-safe + by SQL semantics (NULL comparisons yield NULL, filtered out); state + it explicitly so a reviewer isn't left wondering. +6. **Rename-only** on collision resolution — sessions are not + invalidated. Sessions key on `user_id` so they survive the rename. +7. **Un-soft-delete of a mixed-case account uses the SAME suffix + scheme.** If `Alice` is soft-deleted (skipped by migration) and + later un-soft-deleted while `alice` already exists, the un-soft- + delete path re-normalizes via `set_username` and, on collision, + assigns `alice-2` / `alice-3` / … — the same helper the migration + CLI calls. Both callers reach for a shared + `find_free_username_suffix(pool, base) -> String` in + `src/common/username_migration.rs` so migration + un-soft-delete + agree by construction. Without this, an un-soft-delete of the + only pre-migration mixed-case survivor would refuse-to-boot on + the next restart. + +## Not in scope + +- Unicode case-folding (usernames are ASCII-only by validation). +- `display_name` split (usernames were already just identifiers; + free-form display is a separate future feature if a user asks for + it — deferred pending real demand signal). +- OIDC provisioning behaviour change beyond the ingest-normalize point. +- Case-insensitivity for emails (already achieved in practice; not + touched). +- Any change to `validate_username`'s character-class rules. +- Any change to the WebDAV URL shape `/dav/files/{user}/…` (client + compat; drop deferred separately per [[project_nc_multidrive_poc]]). +- Group names (`SubjectGroup`). Lowercase by convention today; no + runtime enforcement, no migration. If group-name case-insensitivity + becomes a real ask, it lands as a sibling plan doc with the same + shape. + +## Deliverables + +### 1. Ingest normalization + +Change `validate_username` to return the canonical form instead of +`()`: + +```rust +// src/domain/entities/user.rs — new signature +fn validate_username(username: &str) -> UserResult { + let normalized = username.trim().to_ascii_lowercase(); + // ... existing length + charset + boundary checks apply to `normalized` ... + Ok(normalized) +} +``` + +Every caller that today does `Self::validate_username(u)?;` becomes +`let u = Self::validate_username(&u)?;` — the returned canonical form +is what gets stored. Because the return type changes from `Result<()>` +to `Result`, any caller that ignores the result now becomes a +compile error — the type system forces every write path through the +normalizer. + +Write sites all funnel through `User::new` (`src/domain/entities/user.rs:313`) +or `User::set_username` (`:811`), so the signature change catches the +entity-write path automatically. Callers to touch: + +- Application services calling `User::new`: + - `auth_application_service.rs:840` — `register()` public signup + - `auth_application_service.rs:944` — `setup_create_admin()` + first-boot admin + - `auth_application_service.rs:3520`, `:3532` — `admin_create_user()` + external + internal branches + - `auth_application_service.rs:4692` — OIDC JIT provisioning + - `magic_link_invite_service.rs:233` — magic-link external invite + +- User-driven rename (calls `User::set_username`): + - `auth_application_service.rs:2756-2801` — `update_profile()` + +- Repository-write compile-error catches: + - `src/infrastructure/repositories/pg/user_pg_repository.rs:281` + (`create_user` INSERT) and `:740` (`update_user` UPDATE) — these + bind `user_clone.username()`, which is now guaranteed lowercase by + the entity constructor. + +### 2. OIDC JIT derivation + +`auth_application_service.rs:4649-4690` derives a local username from +the OIDC `preferred_username` / `name` / `sub` claims, filters to +`[a-zA-Z0-9._-]`, and truncates. **It does not currently lowercase.** +Add `to_ascii_lowercase()` on the derived string before passing to +`User::new`. This is beyond what the entity signature change catches — +explicit fix required. + +### 3. Lookup normalization + +Repository `find_by_username`-style methods internally lowercase the +input before the SQL query, so callers don't have to remember. One-line +change per method: + +- `src/infrastructure/repositories/pg/user_pg_repository.rs:487` + (`get_user_by_username`) — add `let username = username.trim(). + to_ascii_lowercase();` before the `.bind(&username)` at line 488. +- `src/infrastructure/repositories/pg/user_pg_repository.rs:1043` + (`search_users`) — `ILIKE` is already case-insensitive by + construction; verify nothing regresses. +- `src/infrastructure/repositories/pg/user_pg_repository.rs:1504` + (`search_usernames`) — same as above. +- `src/application/services/storage_usage_service.rs:145-149` + (`update_user_storage_usage_by_username`) — raw SQL bind; normalize + before `.bind()`. +- `src/cli/opaque.rs:125`, `:202` — `opaque reset` CLI identifier + dispatch on `@`; lowercase the username branch input. + +Post-migration, the DB is fully lowercase so `WHERE username = 'alice'` +matches. Pre-migration users are blocked from booting by the boot-time +check, so the mixed-case-DB-during-transition state cannot serve +traffic. + +### 4. NextCloud DAV surface + +Two coordinated changes on the NC surface: + +- `src/interfaces/nextcloud/basic_auth_middleware.rs:94-134` — decoded + `raw_username` from the Basic Auth header, lowercase the whole + string. Safe for the `user~drive_uuid` multi-drive format because + UUID hex is `[0-9a-f-]` which lowercases to itself. +- `src/interfaces/nextcloud/basic_auth_middleware.rs:307-323` + (`parse_basic_auth` helper) — lowercase the username portion before + returning. +- `src/interfaces/nextcloud/session.rs:90-111` + (`extract_url_user`) — lowercase the returned `Cow<'_, str>` value + from URL decode. The cross-check comparison at `session.rs:157-161` + (`url_user != session.raw_username`) then compares normalized vs + normalized — no change needed at the comparison site itself. + +Downstream `session.raw_username` consumers (WebDAV / OCS href +builders, MOVE Destination parsers, avatar / trashbin handlers) all +pass through and emit lowercase automatically — no per-site change +needed. + +**Client compatibility:** NC / DAVX5 clients that cached URLs like +`/remote.php/dav/files/Alice/…` continue to work through the migration +because the server accepts uppercase URL segments **indefinitely** +(the Basic Auth middleware + `extract_url_user` both lowercase on +decode). No forced client upgrade or reconfiguration. PROPFIND +response bodies emit lowercase hrefs (from canonical +`session.raw_username`), which well-behaved clients update on next +sync. + +Expected per-client behavior on first PROPFIND after upgrade: + +- **Nextcloud desktop** — prompts a one-time re-sync notification + when it notices the account URL case changed. Files re-verify + via ETag, so no re-upload; the re-sync completes in + seconds-to-minutes depending on file count. Users click through + the reconnect dialog. +- **DAVX5** (calendars, contacts) — silent update of the internal + `principal-URL`; user sees no dialog. +- **NC mobile app** — silent refresh of the account tile. +- **Older / misbehaving clients** — may create a duplicate account + profile (rare, cosmetic, not destructive). + +**Zero data risk in every path.** The chroot ACL keys on +`user_id`, not username, so files, calendars, contacts, and +grants all follow the user across the rename. The blast radius +is a one-time UX notification, not lost bytes. + +**Power-user pre-emption** (worth documenting in CHANGELOG): ops +who want to avoid the re-sync prompt entirely can, before +upgrading, log into each NC desktop client and manually update +the account URL from `.../USERNAME` to lowercase. Cheap +prophylactic for organizations rolling out to non-technical +users. + +### 5. Chunked-upload directory rename + +`src/infrastructure/services/nextcloud_chunked_upload_service.rs:99-103` +uses `user.username` as an on-disk directory name AND as an in-memory +cache key. Post-migration, `user.username` becomes lowercase; any +in-flight upload for `Alice` at migration time strands the on-disk +`base_dir/Alice/upload_xxx/` directory and orphans its cache entry. + +The migration command must ALSO walk `base_dir/*/` and rename any +mixed-case subdirectory to its lowercase form. Collision handling +(both `Alice/` and `alice/` present) → merge contents; else simple +rename. In practice this is likely a no-op — chunked-upload state is +ephemeral, and simultaneous mixed-case uploads by the same user are +rare. + +### 6. Boot-time verification + +New module `src/common/username_migration.rs` exposing: + +```rust +pub async fn verify_all_usernames_lowercase(pool: &PgPool) -> Result<(), String> +``` + +Runs after `sqlx::migrate!()` completes, before `AppState` is +assembled. Query: + +```sql +SELECT id, username, created_at, last_login_at + FROM auth.users + WHERE username <> LOWER(username) + -- NULL usernames (OPAQUE-migrated accounts) are already filtered + -- out by SQL semantics: NULL <> anything yields NULL, which + -- WHERE excludes. Explicit for the reviewer's benefit. + -- add is_deleted / disabled filter if such a flag exists + ORDER BY LOWER(username), + (last_login_at IS NULL), + last_login_at DESC NULLS LAST, + created_at ASC + LIMIT 200; -- soft cap on error-message size +``` + +If empty → boot proceeds. If non-empty → format the FATAL error and +return `Err(String)`. `main.rs` propagates via `?` to a non-zero +process exit. + +Boot only READS `auth.users`; never WRITES. This is the "explicit +migration required" enforcement layer. + +**Error message format** (self-sufficient — no docs required at 3 AM): + +``` +FATAL: cannot start — user account(s) have non-lowercase usernames. + +Before this version can boot, run the migration: + + oxicloud migrate lowercase-usernames --dry-run # preview + oxicloud migrate lowercase-usernames # apply + +Affected accounts (up to 20 shown; full list via the dry-run): + + Alice (id: a1b2c3d4-... last_login: 2026-08-01) + BOB (id: 9abc0000-... last_login: never) + ... + +The migration handles case-collisions (Alice + alice → alice keeps +the name based on most recent login; the other gets alice-2 suffix). +Sessions and grants survive the rename (they key on user_id). +``` + +### 7. Migration CLI + +New action under `oxicloud migrate`: + +```rust +// src/cli/migrate.rs — extend the Action enum +Action::LowercaseUsernames { dry_run: bool } +``` + +Following the shape of `run_nfc_filenames`: + +- Load all active users (skip soft-deleted / disabled AND rows + where `username IS NULL` — OPAQUE-migrated accounts have no + username string to normalize) +- Group by `LOWER(username)` +- For each group: + - Single-member group with mixed-case name → UPDATE to lowercase + - Multi-member group (collision) → apply tiebreak + `(last_login_at DESC NULLS LAST, created_at ASC)`, winner UPDATEs + to lowercase, losers UPDATE to `-2`, `-3`, … (increment + until free) +- Per-row `println!` log: + `NORMALIZE user= '' ({}B) → '' ({}B)` +- Summary at end: scanned / already-lowercase / normalized / + collision-resolved / renamed-to-suffix +- `--dry-run` guards all UPDATEs + +After the DB pass, run the chunked-upload directory rename step (see +Deliverable 5). + +Suffix search reuses the pattern from +`find_free_folder_duplicate_name` in the existing NFC migration — +increment-until-free loop, starting at `-2`, probing until an +unused suffix is found. Robust against pre-existing rows like +`alice-2` already being taken (the probe just steps past them +to `-3`, `-4`, …). + +Extracted into a shared public helper in +`src/common/username_migration.rs`: + +```rust +pub async fn find_free_username_suffix(pool: &PgPool, base: &str) -> Result +``` + +Both the migration CLI AND the un-soft-delete API (Design decision +7) call this helper — same collision-resolution behavior by +construction, no drift risk between the two paths. + +Bounded at 10,000 as a safety cap. The probability of reaching +that in a real deployment is negligible — it would require ~10 K +distinct accounts all originally cased differently but sharing +the same lowercase form (a normal collision is 2-3 accounts, not +10 K). If the cap ever fires, something is very wrong with the +account universe and the migration ABORTs with a loud error +rather than silently truncating — the loud abort IS the +detection mechanism. + +### 8. Test seed audit + +Sweep-verified: existing test seeds all produce lowercase or NULL +usernames. Worth one more grep pass to ensure no test fixture INSERTs +`INSERT INTO auth.users … 'AliceTest'` — if any exist, lowercase them +in the same commit to avoid CI refuse-to-boot regressions. + +Files verified (all safe): +- `src/infrastructure/repositories/pg/user_pg_repository.rs:1786` +- `src/infrastructure/repositories/pg/opaque_pg_repository.rs:339` + (NULL) +- `src/application/services/auth_application_service.rs:4982` (NULL) +- `src/application/services/subject_group_service.rs:796` (NULL) +- `src/bin/load-seed.rs:414`, `:446` (`load_user_XXXX` — lowercase) +- `src/mount_it_support.rs:61` (`make_user(name)` — verify callers) +- `tests/common/init-test-schema.sh:40` (`ci-admin` — lowercase) + +### 9. Cosmetic side-effects (worth noting in CHANGELOG, non-blocking) + +- `src/interfaces/nextcloud/avatar_handler.rs:283` — `pick_color` + derives a deterministic tile color from username bytes. Users whose + canonical username had uppercase letters will get a different + fallback-avatar tile color after the migration. One-time cosmetic + change. +- **NC desktop may perform a one-time re-sync** — see Deliverable 4. + +### 10. Documentation + +- `CHANGELOG.md` — user-visible note: + - Migration required; server refuses to boot until it's been run. + - Exact CLI command shown in the refusal message. + - Nextcloud desktop clients will prompt for a one-time re-sync + on first PROPFIND after upgrade. Files are ETag-verified, not + re-uploaded. DAVX5 and NC mobile handle the URL case change + silently. **No forced client upgrade or reconfiguration** — + server accepts uppercase URL segments indefinitely. + - Optional pre-emption for non-technical users: ops can manually + update the account URL to lowercase in each NC desktop client + before upgrading, avoiding the re-sync prompt entirely. + - Usernames become lowercase in ALL UI display surfaces (share + dialogs, activity feeds, admin panels, PROPFIND response + bodies, notification bell). Login identity unchanged from the + user's POV (they can still type any case at the login form). + - Avatar fallback color may change for users with previously- + uppercase usernames. + - Preamble noting `display_name` is a possible follow-up if + users miss capitalization for display — deferred pending + demand signal, no compat cost to adding later. +- `docs/config/env.md` — note the boot-time check + migration command. +- `docs/install/binary.md` — upgrade-from-case-sensitive section. + +### 11. Test coverage + +- **Unit**: `validate_username("Alice")` returns `Ok("alice")`; + `validate_username("alice-")` returns `Err(...)` unchanged; + `validate_username(" Alice ")` returns `Ok("alice")`. +- **Unit**: `verify_all_usernames_lowercase` with mocked pool — empty + result → Ok; non-empty → Err with formatted message. +- **Hurl** (`tests/api/lowercase_usernames.hurl`, new): register a + user with `MixedCase`, assert DB stores `mixedcase`; log in with + `MIXEDCASE` and `mixedcase` — both succeed; rename to `NewName`, + assert `newname` stored; NC Basic Auth accepts `MixedCase:pass`, + `MIXEDCASE:pass`, `mixedcase:pass`. +- **Manual** (against dev DB, not CI): + - Induce a collision via `INSERT INTO auth.users … 'Alice'` on top + of `alice`; boot server → verify refusal message + exact CLI shown + - `oxicloud migrate lowercase-usernames --dry-run` → verify report + - `oxicloud migrate lowercase-usernames` → verify apply + - Boot again → succeeds + - `curl -u ALICE:pass https://oxicloud/remote.php/dav/files/ALICE/…` + → succeeds (accepts uppercase input, resolves to lowercase user) + +## Cache-and-consistency observations (informational) + +- `src/infrastructure/services/login_lockout_service.rs:33,68-89` — + already lowercases the key at line 58. No code change; comment + becomes factual not incidental. +- `src/application/services/app_password_service.rs:89,317-323` — + BLAKE3-keyed cache using the raw wire username. Post-normalization, + both sides normalize consistently → cache stays coherent. 300 s TTL + self-heals any transitional window. +- `NC_CHROOT_CACHE` in `basic_auth_middleware.rs:34-40` — keyed on + `Uuid`, not username. Unaffected. + +## Critical files + +Full enumeration in the Deliverables sections above. Grouped summary: + +**Ingest normalizer:** +- `src/domain/entities/user.rs` (signature change + callers) + +**Application services (write callers):** +- `src/application/services/auth_application_service.rs` +- `src/application/services/magic_link_invite_service.rs` + +**Repositories (lookup normalization):** +- `src/infrastructure/repositories/pg/user_pg_repository.rs` +- `src/application/services/storage_usage_service.rs` +- `src/cli/opaque.rs` + +**NextCloud DAV surface:** +- `src/interfaces/nextcloud/basic_auth_middleware.rs` +- `src/interfaces/nextcloud/session.rs` +- `src/infrastructure/services/nextcloud_chunked_upload_service.rs` + +**New files:** +- `src/common/username_migration.rs` +- `tests/api/lowercase_usernames.hurl` + +**Main entry:** +- `src/main.rs` (call verifier after `sqlx::migrate!()`) + +**Migration CLI:** +- `src/cli/migrate.rs` + +**Docs:** +- `CHANGELOG.md` +- `docs/config/env.md` +- `docs/install/binary.md` + +## Delivery order + +1. Change `validate_username` signature to return `Result` — + one file. +2. Fix OIDC JIT derivation + (`auth_application_service.rs:4649-4690`) to lowercase before + passing to `User::new` — explicit change beyond the entity + normalizer's compile-time catches. +3. Iterate on compile errors — the return-type change catches every + downstream write-site. +4. Update repository lookup methods (`user_pg_repository.rs`, + `storage_usage_service.rs`, `cli/opaque.rs`) to internally + lowercase input before `.bind()`. +5. Update NC `basic_auth_middleware.rs` (lowercase `raw_username` at + decode) + `session.rs::extract_url_user` (lowercase return). +6. Add the boot-time verification helper (`src/common/username_migration.rs`) + + wire into `main.rs`. +7. Extend `oxicloud migrate` with `lowercase-usernames [--dry-run]` — + DB pass + chunked-upload directory rename. +8. Test seed audit (grep pass). +9. Add hurl coverage. +10. CHANGELOG entry + admin docs update. +11. Manual smoke test against dev DB. +12. PR to canonical. + +## Total scope estimate + +~6-8 hours of careful work. Larger than the initial estimate because +of these sweep-surfaced additions: + +- OIDC JIT explicit fix (small). +- Chunked-upload directory rename step in the migration (~30 min). +- Test-seed audit (~15 min). +- More lookup callsites than initially thought. + +The shape is uniform (`to_ascii_lowercase()` at every touchpoint) and +the compiler catches missed entity-write sites via the +`Result` signature change. The parts NOT caught by the +compiler (OIDC JIT, lookup normalizers, NC URL segment, chunked-upload +directory) are the ones needing careful review — enumerated above. + +## References + +- Issue: [#691](https://github.com/AtalayaLabs/OxiCloud/issues/691) +- Related feature restrictions today: + - `validate_username` at + `src/domain/entities/user.rs:884-916` + - `@`-disjoint dispatch at + `src/application/services/auth_application_service.rs:1018` +- Related project docs: + - `docs/plan/auth-simplification.md` — the broader auth surface this + fits within + - Prior similar migration: + `oxicloud migrate nfc-filenames` in `src/cli/migrate.rs` From a95a6b106cb6b68a304a3e414d81385845cd83a5 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sun, 13 Sep 2026 18:46:42 +0200 Subject: [PATCH 2/2] feat(username): normalize username into lowercase - normalize username into lowercase (this is already ASCII only) - permit users to login with their username with insensitive case - if a disabled account is reactivated and got a collision, it will normalize it too - server will stop on collision (ex: 2 entries with `Alice` and `alice`) in a such case admin can run: ``` oxicloud migrate lowercase-usernames --dry-run ``` then ``` oxicloud migrate lowercase-usernames ``` --- docs/config/env.md | 64 +++ docs/install/binary.md | 69 +++ docs/plan/username-lowercase.md | 135 ++++-- .../services/auth_application_service.rs | 172 +++++++- .../services/storage_usage_service.rs | 8 +- src/cli/migrate.rs | 314 ++++++++++++++ src/cli/opaque.rs | 21 +- src/common/mod.rs | 1 + src/common/username_migration.rs | 396 ++++++++++++++++++ src/domain/entities/user.rs | 107 ++++- src/infrastructure/db.rs | 40 ++ .../repositories/pg/user_pg_repository.rs | 11 +- .../nextcloud/basic_auth_middleware.rs | 21 +- src/interfaces/nextcloud/session.rs | 16 +- tests/api/lowercase_usernames.hurl | 160 +++++++ 15 files changed, 1454 insertions(+), 81 deletions(-) create mode 100644 src/common/username_migration.rs create mode 100644 tests/api/lowercase_usernames.hurl diff --git a/docs/config/env.md b/docs/config/env.md index f3f6f727..901a1a0c 100644 --- a/docs/config/env.md +++ b/docs/config/env.md @@ -123,6 +123,70 @@ rather than as a visible error. | `OXICLOUD_NOTIFICATIONS_RETENTION_DAYS` | `30` | Retention window for **read** notification rows (`notif.notifications`). The `notifications_cleanup` scheduled job runs daily and deletes rows where `read_at IS NOT NULL` and `read_at < now() - retention_days`. Unread rows are preserved unconditionally — the whole point of the durable table is that a user offline for a month still sees the share-granted notice on next login. Clamped to a minimum of 1 (0 would purge every read row on every tick). Adjust down for compliance-sensitive deployments where "cleared once seen" matters; adjust up when operators expect users to reference old notifications for support. | | `OXICLOUD_WEBDAV_DRIVE_LISTING_PREFIX` | `@drive` | Native WebDAV URL segment that renders the caller's drive list. Sanitized by trimming leading/trailing `/`. Three shapes: (1) default `@drive` — `/webdav/…` addresses the caller's default personal drive (back-compat), `/webdav/@drive/` returns the drive listing, `/webdav/@drive//…` targets a specific drive. (2) empty string `""` — `/webdav/` IS the drive listing, `/webdav//…` targets a specific drive, no default-drive shortcut. (3) any other string (e.g. `drives`) — same shape as `@drive` with that segment substituted. Only drives the caller has Read on via `role_grants` resolve. | +## Boot-time refuse-to-boot checks + +Some upgrades add invariants the running database must satisfy +BEFORE the new binary can serve traffic. These are enforced by +read-only checks that run after `sqlx::migrate!()` and before the +server binds a listen socket. If a check fails, the server exits +with a FATAL message spelling out the exact CLI command to run. + +The server **never silently mutates data** at boot — every fix is +an explicit `oxicloud migrate ` invocation. Follows the +"discovery-only by default, mutation opt-in" rule that also +governs the consistency-check jobs. + +### `lowercase-usernames` + +Three outcomes at boot, only one of which stops the server: + +- **All lowercase (or `NULL`)** — the check is a no-op, boot + proceeds unchanged. +- **Mixed-case rows exist, no `LOWER(username)` collision** — + boot **auto-lowercases** them in one atomic transaction, emits + a structured audit line per rename + (`user.username_lowercased_on_boot`, INFO) plus an INFO summary + (`user.usernames_lowercased_on_boot_summary` with `renamed=N`), + and continues. Silent action is confined to the case with + exactly one correct move: `Alice` (with no `alice` row) becomes + `alice`. +- **`LOWER(username)` collision** — two or more active rows share + the same lowercase form (e.g. `Alice` + `alice`). Boot + **refuses to start** with a FATAL message spelling out every + collision group and the exact CLI command to resolve it. + Tiebreak needs a human. + +Soft-deleted / disabled accounts (`active = false`) and `NULL` +usernames (OPAQUE-migrated) are skipped in every case. + +**Refusal message pattern:** `FATAL: cannot start — N colliding +username group(s) (M affected account(s) in total)`, followed by +each group's canonical form and its members with `id` + +`last_login`. Up to 10 groups shown; the `--dry-run` CLI reveals +the full list. + +**Fix (only required when the server refused):** +``` +oxicloud migrate lowercase-usernames --dry-run # preview the tiebreak +oxicloud migrate lowercase-usernames # apply +``` + +**What the migration does:** lowercases every mixed-case +username. On collision (`Alice` + `alice` both exist), the +tiebreak `(last_login_at DESC NULLS LAST, created_at ASC)` picks +a winner; losers get `alice-2`, `-3`, … as a suffix. Sessions +and grants survive the rename — both key on the user's UUID. + +**Client compat:** NextCloud clients that cached URLs like +`/remote.php/dav/files/Alice/…` continue to work indefinitely — +the Basic Auth middleware and URL parser both lowercase on +decode. NC desktop clients will prompt a one-time re-sync on +first PROPFIND after upgrade; DAVX5 and NC mobile handle it +silently. See `docs/install/binary.md § Upgrading from a +case-sensitive-usernames release` for the full upgrade flow. + +**Design:** `docs/plan/username-lowercase.md`. + ## Storage Entries (multi-entry, recommended) Declare one or more **named** storage backends. The one the app runs on is picked from the DB (`admin_settings.storage.active_backend_name`); the admin panel's storage tab flips the pointer, and cross-backend migration is a recoverable job that copies blobs between two entries with a read-only safety window. See [Admin Settings — Storage & Migration](/config/admin-settings) for the operator flow and the [multi-entry design doc](https://github.com/oxicloud/oxicloud/blob/main/docs/plan/storage-multi-entry.md) for the full model. diff --git a/docs/install/binary.md b/docs/install/binary.md index 9d29b139..48aa19a0 100644 --- a/docs/install/binary.md +++ b/docs/install/binary.md @@ -217,6 +217,75 @@ supported by sqlx's migration model; if you need to roll back, stop the server, roll back your Postgres data directory to a snapshot, and install the previous binary. +### Upgrading from a case-sensitive-usernames release + +Releases that predate the case-insensitive-usernames change stored +`Alice`, `alice`, and `ALICE` as three separate accounts. The +current release treats usernames as case-insensitive (canonical +lowercase in the database). What happens on the first boot after +upgrade depends on your data: + +**No mixed-case usernames.** The check is a no-op; the server +starts normally. Nothing to do. + +**Mixed-case usernames with no collision.** The server +**auto-lowercases** them in one atomic transaction at boot and +continues. Each rename is recorded in the audit log +(`user.username_lowercased_on_boot`) and a WARN summary line +names the total count. `Alice` (with no `alice` row alongside) +becomes `alice`; no ops action needed. This covers the vast +majority of single-admin self-hosted deployments. + +**Mixed-case usernames WITH a collision** (`Alice` + `alice` +both exist as active accounts). The server **refuses to boot** +— tiebreak requires a human. Run the migration: + +``` +# Preview the tiebreak — no writes. +sudo -u oxicloud DATABASE_URL="postgres://..." \ + /usr/local/bin/oxicloud migrate lowercase-usernames --dry-run + +# Apply. Renames run in a single transaction; safe to re-run if aborted. +sudo -u oxicloud DATABASE_URL="postgres://..." \ + /usr/local/bin/oxicloud migrate lowercase-usernames +``` + +On collision, the migration picks a winner by `(last_login_at +DESC NULLS LAST, created_at ASC)` — most recently active keeps +the canonical lowercase name; the loser gets `alice-2`, `-3`, … +as a suffix. **Sessions and grants survive the rename** — both +key on the user's UUID, not the username. + +Skipped in every path above: soft-deleted / disabled accounts +and OPAQUE-migrated users whose `username` column is NULL. +Neither blocks boot. + +After the migration completes, restart the service: + +``` +sudo systemctl start oxicloud +``` + +**Nextcloud desktop clients** will prompt a one-time re-sync on +their first PROPFIND after upgrade — the account URL case +changed. Data is safe (files re-verify via ETag, not re-uploaded). +DAVX5 (calendars, contacts) and NC mobile handle the URL case +change silently. **No client upgrade or reconfiguration is +required** — the server accepts uppercase URL segments (`Alice` +in `/remote.php/dav/files/Alice/...`) indefinitely. + +The refusal message printed by the server on boot (collision +path only) includes the exact CLI command above, so you can't +miss it. Full plan and rationale in +`docs/plan/username-lowercase.md`. + +**Explicit-preview path.** If you'd rather run the migration +before the binary swap — to review renames on your own schedule +or to gate a backup step — run `oxicloud migrate +lowercase-usernames --dry-run` against the OLD binary's DB +first, then apply. On next boot the new binary sees a +lowercase-clean DB and the auto-rename path is a no-op. + ## Installing via `cargo binstall` If you already have the Rust toolchain and just want the binary diff --git a/docs/plan/username-lowercase.md b/docs/plan/username-lowercase.md index 310cd25f..255b7148 100644 --- a/docs/plan/username-lowercase.md +++ b/docs/plan/username-lowercase.md @@ -66,9 +66,26 @@ and get "invalid credentials" instead of a successful login. CLI calls. Both callers reach for a shared `find_free_username_suffix(pool, base) -> String` in `src/common/username_migration.rs` so migration + un-soft-delete - agree by construction. Without this, an un-soft-delete of the - only pre-migration mixed-case survivor would refuse-to-boot on - the next restart. + agree by construction. Without this, an un-soft-delete could + create a fresh collision that the next boot's auto-rename + couldn't resolve (auto-rename handles singletons only) — the + server would then refuse-to-boot until an admin resolves the + tiebreak. +8. **Boot-time behaviour has three outcomes, not two.** The + verifier categorises the DB into: (a) clean — nothing to do; + (b) mixed-case rows with no `LOWER(username)` collision — the + server **auto-lowercases them in one atomic transaction and + continues**, emitting an audit line per rename; (c) at least + one `LOWER(username)` collision — the server **refuses to + boot** because tiebreak requires human judgement. Silent + action is bounded to (b), where there is exactly one correct + move. This is a narrower reading of + [[feedback_no_silent_auto_repair]] than "no silent action + ever": the rule targets consistency-check jobs where drift is + a bug signal; a schema-adjacent boot invariant with a + unique-correct-fix is a different situation. Making the + trivial-case common path a no-op massively lowers upgrade + friction for the 90% self-hosted deployment. ## Not in scope @@ -162,9 +179,10 @@ change per method: dispatch on `@`; lowercase the username branch input. Post-migration, the DB is fully lowercase so `WHERE username = 'alice'` -matches. Pre-migration users are blocked from booting by the boot-time -check, so the mixed-case-DB-during-transition state cannot serve -traffic. +matches. The mixed-case-DB-during-transition state cannot serve +traffic because the boot flow either (a) auto-renames the singleton +rows before `AppState` assembles, or (b) refuses to boot on collision +groups. ### 4. NextCloud DAV surface @@ -230,12 +248,30 @@ cache key. Post-migration, `user.username` becomes lowercase; any in-flight upload for `Alice` at migration time strands the on-disk `base_dir/Alice/upload_xxx/` directory and orphans its cache entry. -The migration command must ALSO walk `base_dir/*/` and rename any +The migration command SHOULD walk `base_dir/*/` and rename any mixed-case subdirectory to its lowercase form. Collision handling (both `Alice/` and `alice/` present) → merge contents; else simple -rename. In practice this is likely a no-op — chunked-upload state is -ephemeral, and simultaneous mixed-case uploads by the same user are -rare. +rename. In practice this is likely a no-op — chunked-upload state +is ephemeral, and simultaneous mixed-case uploads by the same user +are rare. + +**Implementation status:** deferred. Chunked-upload state is +ephemeral: any in-flight upload that gets stranded is retryable +by the client (the upload session's timeout eventually purges the +stale dir; the client retries with a fresh `upload_id`, this time +under the lowercase username). Wiring the dir-walk into the CLI +adds ~40 lines of async filesystem code (walk, collision merge, +mtime-preserving move) and a new `--chunk-dir ` arg — the +CLI otherwise doesn't need to know about the storage-path +config layer. Not worth it for a rare no-op; add if user reports +show a real problem. + +**Ops manual step** — if a migration is run WHILE an upload is +in flight, ops can either restart the affected client (the +upload session is stateful across a `create → chunks → complete` +cycle, so the client will retry from scratch) or manually +`mv base_dir/Alice base_dir/alice` after the DB migration +completes. ### 6. Boot-time verification @@ -318,8 +354,9 @@ Following the shape of `run_nfc_filenames`: collision-resolved / renamed-to-suffix - `--dry-run` guards all UPDATEs -After the DB pass, run the chunked-upload directory rename step (see -Deliverable 5). +After the DB pass, the chunked-upload directory rename step (see +Deliverable 5) is deferred; run manually only if in-flight uploads +were live at migration time. Suffix search reuses the pattern from `find_free_folder_duplicate_name` in the existing NFC migration — @@ -376,26 +413,37 @@ Files verified (all safe): ### 10. Documentation -- `CHANGELOG.md` — user-visible note: - - Migration required; server refuses to boot until it's been run. - - Exact CLI command shown in the refusal message. - - Nextcloud desktop clients will prompt for a one-time re-sync - on first PROPFIND after upgrade. Files are ETag-verified, not - re-uploaded. DAVX5 and NC mobile handle the URL case change - silently. **No forced client upgrade or reconfiguration** — - server accepts uppercase URL segments indefinitely. - - Optional pre-emption for non-technical users: ops can manually - update the account URL to lowercase in each NC desktop client - before upgrading, avoiding the re-sync prompt entirely. - - Usernames become lowercase in ALL UI display surfaces (share - dialogs, activity feeds, admin panels, PROPFIND response - bodies, notification bell). Login identity unchanged from the - user's POV (they can still type any case at the login form). - - Avatar fallback color may change for users with previously- - uppercase usernames. - - Preamble noting `display_name` is a possible follow-up if - users miss capitalization for display — deferred pending - demand signal, no compat cost to adding later. +Release notes / CHANGELOG entry is NOT part of this PR — the +canonical repo's maintainer handles release notes at version-bump +time. This PR just leaves the notes-worthy items enumerated here +so the maintainer has the bullets to pick from when the next +version ships: + +- Server auto-lowercases non-colliding mixed-case usernames at + first boot. No ops action needed for the common case. +- On `LOWER(username)` collision (`Alice` + `alice` both active), + the server refuses to boot; ops runs `oxicloud migrate + lowercase-usernames`. Exact CLI command shown in the refusal. +- Nextcloud desktop clients will prompt for a one-time re-sync on + first PROPFIND after upgrade. Files are ETag-verified, not + re-uploaded. DAVX5 and NC mobile handle the URL case change + silently. **No forced client upgrade or reconfiguration** — + server accepts uppercase URL segments indefinitely. +- Optional pre-emption for non-technical users: ops can manually + update the account URL to lowercase in each NC desktop client + before upgrading, avoiding the re-sync prompt entirely. +- Usernames become lowercase in ALL UI display surfaces (share + dialogs, activity feeds, admin panels, PROPFIND response + bodies, notification bell). Login identity unchanged from the + user's POV (they can still type any case at the login form). +- Avatar fallback color may change for users with previously- + uppercase usernames. +- Note: `display_name` is a possible follow-up if users miss + capitalisation for display — deferred pending demand signal, no + compat cost to adding later. + +The two docs that DO ship with this PR: + - `docs/config/env.md` — note the boot-time check + migration command. - `docs/install/binary.md` — upgrade-from-case-sensitive section. @@ -404,19 +452,26 @@ Files verified (all safe): - **Unit**: `validate_username("Alice")` returns `Ok("alice")`; `validate_username("alice-")` returns `Err(...)` unchanged; `validate_username(" Alice ")` returns `Ok("alice")`. -- **Unit**: `verify_all_usernames_lowercase` with mocked pool — empty - result → Ok; non-empty → Err with formatted message. +- **Unit**: `format_refusal_message_collisions` — 1 group renders + canonical + members + CLI; > 10 groups renders overflow tail; + total-affected-count sums across groups. - **Hurl** (`tests/api/lowercase_usernames.hurl`, new): register a user with `MixedCase`, assert DB stores `mixedcase`; log in with `MIXEDCASE` and `mixedcase` — both succeed; rename to `NewName`, assert `newname` stored; NC Basic Auth accepts `MixedCase:pass`, `MIXEDCASE:pass`, `mixedcase:pass`. - **Manual** (against dev DB, not CI): - - Induce a collision via `INSERT INTO auth.users … 'Alice'` on top - of `alice`; boot server → verify refusal message + exact CLI shown + - Auto-rename path: `UPDATE auth.users SET username='Alice' WHERE + username='alice'` (no collision); boot server → verify audit log + line + WARN summary, row is `alice` after boot, service starts. + - Collision path: `INSERT INTO auth.users … 'Alice'` on top of + existing `alice`; boot server → verify refusal message names both + rows + exact CLI shown, server exits non-zero. - `oxicloud migrate lowercase-usernames --dry-run` → verify report - - `oxicloud migrate lowercase-usernames` → verify apply - - Boot again → succeeds + of collision + tiebreak decision + - `oxicloud migrate lowercase-usernames` → verify apply, one row + keeps `alice`, other gets `alice-2` + - Boot again → succeeds (Clean outcome) - `curl -u ALICE:pass https://oxicloud/remote.php/dav/files/ALICE/…` → succeeds (accepts uppercase input, resolves to lowercase user) @@ -489,7 +544,9 @@ Full enumeration in the Deliverables sections above. Grouped summary: DB pass + chunked-upload directory rename. 8. Test seed audit (grep pass). 9. Add hurl coverage. -10. CHANGELOG entry + admin docs update. +10. Admin docs update (`docs/config/env.md` boot-check subsection + + `docs/install/binary.md` upgrade section). CHANGELOG is Dio's + job at version-bump time — not part of this PR. 11. Manual smoke test against dev DB. 12. PR to canonical. diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index 618fdd22..709f8fd6 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -3425,34 +3425,37 @@ impl AuthApplicationService { &self, dto: crate::application::dtos::settings_dto::AdminCreateUserDto, ) -> Result { - // Validate username length - if dto.username.len() < 3 || dto.username.len() > 254 { - return Err(DomainError::new( - ErrorKind::InvalidInput, - "User", - "Username must be between 3 and 254 characters".to_string(), - )); - } + // Normalise the username up-front — trim + lowercase — and use + // the canonical form for every downstream check + generated + // value below. `User::new` also normalises internally, but the + // placeholder-email fallback and the duplicate-check error + // message live above that call, so they'd otherwise capture the + // raw wire input (e.g. `UpperCase@oxicloud.local`). + // See docs/plan/username-lowercase.md § Design decision 5. + let username = User::validate_username(&dto.username).map_err(|e| { + DomainError::new(ErrorKind::InvalidInput, "User", format!("Username: {e}")) + })?; // Check for duplicate username if self .user_storage - .get_user_by_username(&dto.username) + .get_user_by_username(&username) .await .is_ok() { return Err(DomainError::new( ErrorKind::AlreadyExists, "User", - format!("User '{}' already exists", dto.username), + format!("User '{username}' already exists"), )); } - // Email: use provided or generate placeholder + // Email: use provided or generate placeholder from the + // canonical (lowercase) username. let email = dto .email .filter(|e| !e.trim().is_empty()) - .unwrap_or_else(|| format!("{}@oxicloud.local", dto.username)); + .unwrap_or_else(|| format!("{username}@oxicloud.local")); // Check email uniqueness if self.user_storage.get_user_by_email(&email).await.is_ok() { @@ -3519,7 +3522,7 @@ impl AuthApplicationService { let user = if is_external { User::new( email, - Some(dto.username.clone()), + Some(username.clone()), Some(password_hash), None, // federation_kind: admin-created external, no federation link yet None, // federation_issuer @@ -3531,7 +3534,7 @@ impl AuthApplicationService { } else { User::new( email, - Some(dto.username.clone()), + Some(username.clone()), Some(password_hash), None, // federation_kind: admin-created local user None, // federation_issuer @@ -3750,8 +3753,33 @@ impl AuthApplicationService { Ok(()) } - /// Activate or deactivate a user (admin only) + /// Activate or deactivate a user (admin only). + /// + /// **Reactivation collision handling** — when a deactivated account + /// holds a mixed-case username from before the lowercase-usernames + /// migration (its row was skipped by that migration precisely + /// because it was deactivated), reactivating it can produce a + /// username collision if `LOWER(other.username) == LOWER(this.username)` + /// for an active row. We resolve the collision by: + /// + /// 1. Re-normalising via `User::set_username` (returns the + /// canonical lowercase form on success). + /// 2. If the canonical form is already taken by another active + /// row, probe `-2`, `-3`, … via the shared + /// `find_free_username_suffix` helper. + /// 3. Persist the resolved name BEFORE flipping `active = true` + /// so no time window has two-active-users with the same + /// lowercase form. + /// + /// See `docs/plan/username-lowercase.md § Design decision 7`. + /// Without this, un-soft-deleting the only pre-migration + /// mixed-case survivor would refuse-to-boot on the next restart. pub async fn set_user_active(&self, user_id: Uuid, active: bool) -> Result<(), DomainError> { + // Only the activate direction needs the collision-resolution + // dance — deactivation just flips a bit. + if active { + self.resolve_reactivation_collision(user_id).await?; + } self.user_storage .set_user_active_status(user_id, active) .await?; @@ -3759,6 +3787,93 @@ impl AuthApplicationService { Ok(()) } + /// Pre-flight for `set_user_active(active = true)`: ensures the + /// target user's username is canonical (lowercase) AND unique + /// against currently-active accounts. Renames the target row if + /// either invariant would break. + /// + /// NULL usernames (OPAQUE-migrated accounts) are a no-op — nothing + /// to normalise, nothing to collide. + async fn resolve_reactivation_collision(&self, user_id: Uuid) -> Result<(), DomainError> { + let target = self.user_storage.get_user_by_id(user_id).await?; + let Some(current) = target.username().map(str::to_string) else { + return Ok(()); + }; + + let canonical = current.to_ascii_lowercase(); + + // Look for another ACTIVE user holding the canonical form. + // The migration CLI's `find_free_username_suffix` probes + // directly via a pool; here we don't have the pool + // (`AuthApplicationService` holds a `dyn UserRepository` + // trait object). Use `get_user_by_username` — the repo + // normalises input to lowercase before bind, so this + // resolves against the canonical row. + let collision = self + .user_storage + .get_user_by_username(&canonical) + .await + .ok() + .filter(|other| other.id() != user_id && other.is_active()); + + let chosen_name = match collision { + None => canonical, + Some(_) => { + // Collision — probe `-2`, `-3`, … via + // repository lookups. Same shape as the migration + // CLI's `find_free_username_suffix`, just against + // the repo trait instead of a raw pool. Both paths + // agree by construction on the numbering scheme. + // + // Cap at 10_000 (matches the shared helper's cap — + // see `docs/plan/username-lowercase.md § 3. Suffix- + // collision robustness`). Reaching the cap means + // the account universe has an anomaly worth + // investigating; loud abort beats silent truncation. + const SUFFIX_PROBE_CAP: i32 = 10_000; + let mut chosen: Option = None; + for n in 2..=SUFFIX_PROBE_CAP { + let candidate = format!("{canonical}-{n}"); + match self.user_storage.get_user_by_username(&candidate).await { + Ok(_) => continue, + Err(_) => { + chosen = Some(candidate); + break; + } + } + } + let suffixed = chosen.ok_or_else(|| { + DomainError::internal_error( + "User", + format!( + "reactivation-collision suffix probe exhausted \ + {SUFFIX_PROBE_CAP} candidates for base '{canonical}'" + ), + ) + })?; + tracing::info!( + target: "audit", + event = "user.reactivation_renamed", + reason = "collision_with_active", + target_id = %user_id, + from = %current, + to = %suffixed, + "🔄 user reactivation renamed to avoid username collision", + ); + suffixed + } + }; + + if target.username() != Some(chosen_name.as_str()) { + let mut renamed = target; + renamed + .set_username(chosen_name) + .map_err(|e| DomainError::internal_error("User", format!("set_username: {e}")))?; + self.user_storage.update_user(renamed).await?; + } + Ok(()) + } + /// Change user role (admin only). /// /// Refuses `role = "admin"` when the target is external (grant-only). @@ -4435,10 +4550,14 @@ impl AuthApplicationService { .clone() .or(claims.name.clone()) .unwrap_or_else(|| format!("oidc_{}", &claims.sub[..8.min(claims.sub.len())])); + // Placeholder-email fallback when the IdP omits `email` from + // the claim set. Lowercase the local-part so the fake address + // matches the storage convention for other placeholder-email + // paths (see `admin_create_user`'s `@oxicloud.local`). let oidc_email = claims .email .clone() - .unwrap_or_else(|| format!("{}@oidc.local", oidc_username)); + .unwrap_or_else(|| format!("{}@oidc.local", oidc_username.to_ascii_lowercase())); // 5. Look up existing user by OIDC subject. // @@ -4652,16 +4771,30 @@ impl AuthApplicationService { &oidc_username }; - // Filter to valid username characters only, then truncate to 32 chars + // Lowercase at JIT derivation. `validate_username` in + // `User::new` would lowercase too, but the collision + // check below (`get_user_by_username`) needs the + // canonical form BEFORE `User::new` is called — + // otherwise `Alice` from an IdP claim would look + // "free" against an `alice` row on the first pass + // and fail the DB unique constraint at INSERT time. + // See `docs/plan/username-lowercase.md § 2. OIDC JIT + // derivation`. + // + // ASCII-only by the char-filter below, so + // `to_ascii_lowercase()` is deterministic and + // locale-safe. let mut username = base_username .chars() .filter(|c| { c.is_ascii_alphanumeric() || *c == '-' || *c == '_' || *c == '.' }) .take(32) - .collect::(); + .collect::() + .to_ascii_lowercase(); - // Filter helper: removes any chars that are not valid in a username + // Filter helper: removes any chars that are not valid in a username. + // Lowercases too so the collision-suffix path below writes canonical form. let filter_username_chars = |s: &str| { s.chars() .filter(|c| { @@ -4669,6 +4802,7 @@ impl AuthApplicationService { }) .take(32) .collect::() + .to_ascii_lowercase() }; // Ensure minimum length (the padding suffix must also be filtered) diff --git a/src/application/services/storage_usage_service.rs b/src/application/services/storage_usage_service.rs index 392e22ee..b15cfad8 100644 --- a/src/application/services/storage_usage_service.rs +++ b/src/application/services/storage_usage_service.rs @@ -125,10 +125,16 @@ impl StorageUsageService { } /// Same as [`Self::update_user_storage_usage`], keyed by username. + /// + /// Lowercases input before bind — same rule as + /// `UserRepository::get_user_by_username`. Usernames are canonical + /// (lowercase) in the DB post-migration; callers may pass any case. + /// See `docs/plan/username-lowercase.md`. pub async fn update_user_storage_usage_by_username( &self, username: &str, ) -> Result { + let username = username.trim().to_ascii_lowercase(); let total_usage: Option = sqlx::query_scalar( r#" UPDATE auth.users u @@ -146,7 +152,7 @@ impl StorageUsageService { RETURNING u.storage_used_bytes "#, ) - .bind(username) + .bind(&username) .fetch_optional(self.pool.as_ref()) .await .map_err(|e| { diff --git a/src/cli/migrate.rs b/src/cli/migrate.rs index 89f6e06a..fcf585be 100644 --- a/src/cli/migrate.rs +++ b/src/cli/migrate.rs @@ -82,11 +82,35 @@ pub enum Action { #[arg(long)] dry_run: bool, }, + + /// Lowercase every active user's username in `auth.users`. + /// + /// Enforcement-companion for the case-insensitive-usernames + /// migration (see `docs/plan/username-lowercase.md`). The server + /// refuses to boot after upgrade until this has run. Data touched + /// is `auth.users.username` only. + /// + /// Collision handling: when `Alice` and `alice` both exist, + /// tiebreak `(last_login_at DESC NULLS LAST, created_at ASC)` — + /// the winner keeps the canonical lowercased name, losers get + /// `-2`, `-3`, … via the shared + /// [`common::username_migration::find_free_username_suffix`] + /// probe. Sessions and grants survive the rename (they key on + /// `user_id`). + /// + /// Skipped: soft-deleted / disabled rows and rows where + /// `username IS NULL` (OPAQUE-migrated accounts). + LowercaseUsernames { + /// Print what would change without touching the DB. + #[arg(long)] + dry_run: bool, + }, } pub async fn run(action: Action) -> u8 { match action { Action::NfcFilenames { dry_run } => run_nfc_filenames(dry_run).await, + Action::LowercaseUsernames { dry_run } => run_lowercase_usernames(dry_run).await, } } @@ -758,3 +782,293 @@ async fn run_folders(pool: &PgPool, dry_run: bool, stats: &mut Stats) -> Result< } Ok(()) } + +// ════════════════════════════════════════════════════════════════════════════ +// lowercase-usernames +// ════════════════════════════════════════════════════════════════════════════ + +#[derive(Default)] +struct UsernameStats { + scanned: u64, + already_lowercase: u64, + normalized_in_place: u64, + /// Multi-member `LOWER(username)` group where the tiebreak + /// winner kept the canonical name. + collision_winners: u64, + /// Multi-member losers renamed to `-N`. + renamed_to_suffix: u64, + /// Rows the scan touched but the loop declined to modify. Today + /// this is inactive rows (soft-deleted / admin-disabled). NULL + /// usernames never enter the scan so they don't contribute here. + skipped: u64, +} + +#[derive(Debug)] +struct UsernameRow { + id: Uuid, + username: String, + /// Inactive rows (soft-deleted / admin-disabled) are read but not + /// modified — normalising a name we can't reach anyway risks + /// creating a `-N` conflict with a future re-activation of + /// the same handle. The loop uses this flag to skip and count. + active: bool, + // Kept for the SQL row-shape roundtrip (the SELECT ordering + // depends on them) even though the Rust-side grouping only + // reads `id` and `username`. Marked `#[allow(dead_code)]` + // so clippy doesn't nag; renaming to `_last_login_at` would + // work too but the SQL column names are load-bearing for the + // sqlx `Row::get` calls below. + #[allow(dead_code)] + last_login_at: Option>, + #[allow(dead_code)] + created_at: DateTime, +} + +async fn run_lowercase_usernames(dry_run: bool) -> u8 { + let database_url = match env::var("DATABASE_URL") { + Ok(v) => v, + Err(_) => { + eprintln!("migrate lowercase-usernames: DATABASE_URL not set"); + return 2; + } + }; + + let pool = match PgPool::connect(&database_url).await { + Ok(p) => p, + Err(e) => { + eprintln!("migrate lowercase-usernames: failed to connect: {e}"); + return 2; + } + }; + + if dry_run { + println!("migrate lowercase-usernames: DRY RUN (no writes)"); + } else { + println!("migrate lowercase-usernames: applying changes"); + } + + // Load every user with a non-NULL username, active and inactive + // alike. Inactive rows are surfaced (not filtered at scan-time) + // so the `skipped (inactive)` counter can report them honestly — + // an operator reading the summary sees "10 rows scanned, 2 + // inactive were passed on" instead of a phantom 0. + // + // NULL usernames stay out of the scan: there's nothing to + // normalise for OPAQUE-migrated rows, and pulling them would + // inflate `scanned` with rows the migration has no verb for. + // + // Ordering: alphabetic by `LOWER(username)` groups collisions + // together, then the intra-group order is the tiebreak + // (`last_login_at DESC NULLS LAST, created_at ASC` — most + // recently active wins the canonical name). + let rows: Vec = match sqlx::query( + r#" + SELECT id, username, active, last_login_at, created_at + FROM auth.users + WHERE username IS NOT NULL + ORDER BY LOWER(username), + (last_login_at IS NULL), + last_login_at DESC NULLS LAST, + created_at ASC + "#, + ) + .fetch_all(&pool) + .await + { + Ok(rs) => rs + .into_iter() + .map(|r| UsernameRow { + id: r.get::("id"), + username: r.get::("username"), + active: r.get::("active"), + last_login_at: r.try_get::, _>("last_login_at").ok(), + created_at: r.get::, _>("created_at"), + }) + .collect(), + Err(e) => { + eprintln!("migrate lowercase-usernames: initial scan failed: {e}"); + return 2; + } + }; + + let mut stats = UsernameStats::default(); + + // Group by `LOWER(username)`. Order preserved from the SQL query + // → within a group, the FIRST row is the tiebreak winner. + // + // Inactive rows are filtered OUT of the grouping (not just + // skipped inside the loop) so they can't create a phantom + // collision with an active row sharing their lowercase form. + // Example: inactive `Alice` + active `alice` would otherwise + // look like a two-member group; filtering inactive first leaves + // `alice` as a clean singleton no-op. The count goes to + // `stats.skipped`, surfaced in the summary as `skipped (inactive)`. + let mut groups: Vec<(String, Vec)> = Vec::new(); + for row in rows { + stats.scanned += 1; + if !row.active { + stats.skipped += 1; + continue; + } + let key = row.username.to_ascii_lowercase(); + match groups.last_mut() { + Some((k, v)) if k == &key => v.push(row), + _ => groups.push((key, vec![row])), + } + } + + for (lower, members) in groups { + if members.len() == 1 { + let row = &members[0]; + if row.username == lower { + stats.already_lowercase += 1; + continue; + } + // Single-member group with a mixed-case name → straight + // rename to the lowercase form. No collision. + if !dry_run && let Err(e) = update_username(&pool, row.id, &lower).await { + eprintln!( + "migrate lowercase-usernames: UPDATE failed for {}: {e}", + row.id + ); + return 1; + } + println!( + "NORMALIZE user={} '{}' → '{}'", + row.id, row.username, lower + ); + stats.normalized_in_place += 1; + continue; + } + + // Multi-member group → collision. Members are already ordered + // by the tiebreak. Winner takes the canonical lowercase name, + // losers get `-2`, `-3`, … via the shared suffix helper. + // + // ORDER MATTERS: losers must be renamed FIRST. If we renamed + // the winner to `` while a loser still holds that + // exact name (the common case where the winner is mixed-case + // and the loser is already-lowercase), the UNIQUE constraint + // `users_username_key` fires. Freeing the canonical form by + // suffixing every non-winner member first eliminates the + // race entirely. + let (winner, losers) = members.split_first().expect("non-empty by construction"); + + // Suffixes assigned inside this group during this run. Used + // to keep dry-run consistent (no DB writes → the shared + // suffix helper would hand the same probe back for every + // loser). During apply, the DB itself deduplicates, but + // tracking here keeps the two modes structurally identical. + let mut reserved_this_group: Vec = Vec::new(); + for loser in losers { + let suffixed = + match pick_free_suffix_avoiding(&pool, &lower, &reserved_this_group).await { + Ok(s) => s, + Err(e) => { + eprintln!( + "migrate lowercase-usernames: suffix probe failed for {}: {e}", + loser.id + ); + return 1; + } + }; + if !dry_run && let Err(e) = update_username(&pool, loser.id, &suffixed).await { + eprintln!( + "migrate lowercase-usernames: loser UPDATE failed for {}: {e}", + loser.id + ); + return 1; + } + println!( + "RENAME user={} '{}' → '{}' (collision suffix)", + loser.id, loser.username, suffixed + ); + stats.renamed_to_suffix += 1; + reserved_this_group.push(suffixed); + } + + if winner.username == lower { + // Winner already holds the canonical name (a lowercase + // row happened to be the most recently active; other + // members are the ones needing renames). + stats.already_lowercase += 1; + } else { + if !dry_run && let Err(e) = update_username(&pool, winner.id, &lower).await { + eprintln!( + "migrate lowercase-usernames: winner UPDATE failed for {}: {e}", + winner.id + ); + return 1; + } + println!( + "NORMALIZE user={} '{}' → '{}' (collision winner)", + winner.id, winner.username, lower + ); + stats.collision_winners += 1; + } + } + + println!(); + println!("Summary:"); + println!(" scanned: {}", stats.scanned); + println!(" already-lowercase: {}", stats.already_lowercase); + println!(" normalized in place: {}", stats.normalized_in_place); + println!(" collision winners: {}", stats.collision_winners); + println!(" renamed to suffix: {}", stats.renamed_to_suffix); + println!(" skipped (inactive): {}", stats.skipped); + if dry_run + && (stats.normalized_in_place + stats.collision_winners + stats.renamed_to_suffix) > 0 + { + println!(); + println!("(dry-run — re-run without --dry-run to apply)"); + } + 0 +} + +async fn update_username(pool: &PgPool, id: Uuid, new_name: &str) -> Result<(), sqlx::Error> { + sqlx::query("UPDATE auth.users SET username = $1, updated_at = NOW() WHERE id = $2") + .bind(new_name) + .bind(id) + .execute(pool) + .await?; + Ok(()) +} + +/// Pick the next free `-` suffix, skipping any suffix already +/// reserved earlier in this migration run. +/// +/// Wraps [`crate::common::username_migration::find_free_username_suffix`] +/// with an additional local guard: in dry-run mode, no UPDATEs land so +/// the DB probe would return the same suffix for every loser in a +/// multi-loser group. The `reserved` slice lets the caller feed back +/// the suffixes it has already announced, and the probe steps past +/// them. In apply mode the DB probe alone would be enough (each real +/// UPDATE moves the state forward), but the same code path keeps the +/// two modes structurally identical. +async fn pick_free_suffix_avoiding( + pool: &PgPool, + base: &str, + reserved: &[String], +) -> Result { + // Try the shared helper's default candidate first; if it collides + // with a same-run reservation, increment past it and probe again. + let mut n = 2; + loop { + let candidate = format!("{base}-{n}"); + let db_taken: (bool,) = + sqlx::query_as("SELECT EXISTS(SELECT 1 FROM auth.users WHERE username = $1)") + .bind(&candidate) + .fetch_one(pool) + .await?; + let locally_taken = reserved.iter().any(|s| s == &candidate); + if !db_taken.0 && !locally_taken { + return Ok(candidate); + } + n += 1; + if n > 10_000 { + // Same cap as the shared helper — a loud panic beats a + // silent truncation for an anomaly this rare. + panic!("pick_free_suffix_avoiding: exhausted 10000 suffix probes for base '{base}'",); + } + } +} diff --git a/src/cli/opaque.rs b/src/cli/opaque.rs index 95c61d4a..777270a4 100644 --- a/src/cli/opaque.rs +++ b/src/cli/opaque.rs @@ -128,8 +128,15 @@ async fn run_reset(user: Option, all: bool, dry_run: bool) -> u8 { let rows_result = if all { sqlx::query(select_sql).fetch_all(&pool).await } else { - let ident = user.as_deref().unwrap(); - sqlx::query(select_sql).bind(ident).fetch_all(&pool).await + // Normalise before bind so the CLI accepts any case for + // the username branch (email is already case-insensitive + // via a functional index on LOWER(email); lowercasing here + // for both branches is harmless — emails are lowercase + // ASCII in `auth.users.email` too). + // See `docs/plan/username-lowercase.md § 3. Lookup normalization`. + let ident_raw = user.as_deref().unwrap(); + let ident = ident_raw.trim().to_ascii_lowercase(); + sqlx::query(select_sql).bind(&ident).fetch_all(&pool).await }; let rows = match rows_result { Ok(r) => r, @@ -204,8 +211,14 @@ async fn run_reset(user: Option, all: bool, dry_run: bool) -> u8 { let write_result = if all { sqlx::query(update_sql_all).execute(&pool).await } else { - let ident = user.as_deref().unwrap(); - sqlx::query(update_sql_one).bind(ident).execute(&pool).await + // Same normalisation as the read path above — usernames are + // canonical lowercase in the DB. See docs/plan/username-lowercase.md. + let ident_raw = user.as_deref().unwrap(); + let ident = ident_raw.trim().to_ascii_lowercase(); + sqlx::query(update_sql_one) + .bind(&ident) + .execute(&pool) + .await }; let affected = match write_result { Ok(r) => r.rows_affected(), diff --git a/src/common/mod.rs b/src/common/mod.rs index ed8cc6a5..8dd5e47d 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -8,3 +8,4 @@ pub mod mime_detect; pub mod runtime; pub mod stubs; pub mod text; +pub mod username_migration; diff --git a/src/common/username_migration.rs b/src/common/username_migration.rs new file mode 100644 index 00000000..85e02a30 --- /dev/null +++ b/src/common/username_migration.rs @@ -0,0 +1,396 @@ +//! Username-lowercase boot flow: verifier, auto-rename, shared collision helper. +//! +//! The plan (`docs/plan/username-lowercase.md`) makes usernames +//! case-insensitive by canonicalising to lowercase on ingest. Three +//! pieces of infrastructure live here: +//! +//! 1. [`verify_all_usernames_lowercase`] — a **read-only** check that +//! runs after `sqlx::migrate!()` at boot. Classifies every active +//! mixed-case row into one of three outcomes: +//! +//! - [`UsernameCaseCheck::Clean`] — nothing to do. +//! - [`UsernameCaseCheck::AutoRenamable`] — mixed-case rows exist +//! but each `LOWER(username)` form is unique in the active-user +//! set. Safe to lowercase in one atomic transaction; boot proceeds. +//! - [`UsernameCaseCheck::Collisions`] — at least one group has +//! two or more active rows sharing a `LOWER(username)` (e.g. +//! `Alice` + `alice`). Tiebreak requires human judgement; the +//! server refuses to start and prints the CLI command. +//! +//! Follows [[feedback_no_silent_auto_repair]] in spirit: silent +//! action is limited to cases where there is exactly one correct +//! move (rename the sole mixed-case row to its lowercase form). +//! Anywhere ambiguity exists (which of `Alice` and `alice` keeps +//! the canonical name?), boot refuses and defers to `oxicloud +//! migrate lowercase-usernames`. +//! +//! 2. [`apply_auto_renames`] — the one-transaction UPDATE loop that +//! performs the auto-rename path. Emits a structured audit line +//! per row (`user.username_lowercased_on_boot`). All-or-nothing: +//! a mid-tx failure aborts the transaction and boot fails, so the +//! DB is never left in a half-renamed state. +//! +//! 3. [`find_free_username_suffix`] — the shared collision-resolution +//! helper. Called by the migration CLI when it lowercases a name +//! that would clash with an existing row, AND by the un-soft-delete +//! API when it re-normalises a mixed-case account whose lowercase +//! form is now taken by someone else. +//! +//! `NULL` usernames (OPAQUE-migrated accounts) are always skipped — the +//! SQL `WHERE username <> LOWER(username)` predicate is NULL-safe by +//! semantics (`NULL <> anything` yields `NULL`, which `WHERE` excludes). +//! Soft-deleted / disabled accounts (`active = false`) are also skipped: +//! they can't serve traffic anyway. + +use sqlx::{PgPool, Row}; + +/// One mixed-case account row. Used both for the auto-rename list and +/// for reporting collision-group members. +#[derive(Debug, Clone)] +pub struct MixedCaseAccount { + pub id: uuid::Uuid, + pub username: String, + pub last_login_at: Option>, +} + +/// A `LOWER(username)` group with two or more active members. At least +/// one member is mixed-case (that's what made the group visible to the +/// verifier); the other member(s) may be already-lowercase (e.g. +/// `Alice` + `alice`) or also mixed-case (`Alice` + `ALICE`). +#[derive(Debug, Clone)] +pub struct CollisionGroup { + /// The lowercase form shared by every member. + pub canonical: String, + /// Members, ordered by the tiebreak that the migration CLI + /// applies: `last_login_at DESC NULLS LAST, created_at ASC`. + pub members: Vec, +} + +/// The three outcomes of the boot-time verifier. +#[derive(Debug, Clone)] +pub enum UsernameCaseCheck { + /// Every active username is already lowercase (or `NULL`). Boot + /// proceeds unmodified. + Clean, + /// Mixed-case rows exist, but each `LOWER(username)` form is + /// unique among active users. Safe to lowercase atomically at + /// boot; the caller runs [`apply_auto_renames`]. + AutoRenamable(Vec), + /// At least one `LOWER(username)` group has two or more active + /// members. Tiebreak requires human judgement; the caller formats + /// a refusal message via [`format_refusal_message_collisions`] and + /// aborts boot. + Collisions(Vec), +} + +/// Boot-time verifier. Runs AFTER `sqlx::migrate!()` and BEFORE +/// `AppState` is assembled. Read-only: never mutates `auth.users`. +/// +/// Returns [`UsernameCaseCheck`] describing what (if anything) the +/// caller should do. Errors are limited to DB failures — semantic +/// outcomes are all `Ok(_)` variants. +pub async fn verify_all_usernames_lowercase(pool: &PgPool) -> Result { + // First pass: mixed-case rows that have NO other active row + // sharing their LOWER form. These are safe to auto-rename. + let auto_rows = sqlx::query( + r#" + SELECT u.id, u.username, u.last_login_at + FROM auth.users u + WHERE u.active = true + AND u.username <> LOWER(u.username) + AND NOT EXISTS ( + SELECT 1 + FROM auth.users u2 + WHERE u2.active = true + AND u2.id <> u.id + AND LOWER(u2.username) = LOWER(u.username) + ) + ORDER BY LOWER(u.username) + "#, + ) + .fetch_all(pool) + .await + .map_err(|e| format!("username lowercase verifier: singleton query failed: {e}"))?; + + // Second pass: every active row that belongs to a colliding + // group — a `LOWER(username)` shared by two or more active rows + // where at least one member is mixed-case. Result includes + // already-lowercase members so the refusal report shows the full + // context of each collision. + let collision_rows = sqlx::query( + r#" + WITH colliding_lowers AS ( + SELECT LOWER(username) AS canonical + FROM auth.users + WHERE active = true + GROUP BY LOWER(username) + HAVING COUNT(*) > 1 + AND SUM(CASE WHEN username <> LOWER(username) THEN 1 ELSE 0 END) >= 1 + ) + SELECT id, username, last_login_at, LOWER(username) AS canonical + FROM auth.users + WHERE active = true + AND LOWER(username) IN (SELECT canonical FROM colliding_lowers) + ORDER BY LOWER(username), + (last_login_at IS NULL), + last_login_at DESC NULLS LAST, + created_at ASC + "#, + ) + .fetch_all(pool) + .await + .map_err(|e| format!("username lowercase verifier: collision query failed: {e}"))?; + + if !collision_rows.is_empty() { + // Group by canonical. Rows are already ordered by canonical + // then by tiebreak, so a fold is enough. + let mut groups: Vec = Vec::new(); + for r in collision_rows { + let canonical: String = r.get("canonical"); + let member = MixedCaseAccount { + id: r.get::("id"), + username: r.get::("username"), + last_login_at: r + .try_get::, _>("last_login_at") + .ok(), + }; + match groups.last_mut() { + Some(g) if g.canonical == canonical => g.members.push(member), + _ => groups.push(CollisionGroup { + canonical, + members: vec![member], + }), + } + } + return Ok(UsernameCaseCheck::Collisions(groups)); + } + + if auto_rows.is_empty() { + return Ok(UsernameCaseCheck::Clean); + } + + let accounts = auto_rows + .into_iter() + .map(|r| MixedCaseAccount { + id: r.get::("id"), + username: r.get::("username"), + last_login_at: r + .try_get::, _>("last_login_at") + .ok(), + }) + .collect(); + Ok(UsernameCaseCheck::AutoRenamable(accounts)) +} + +/// Apply the atomic auto-rename transaction. All UPDATEs succeed +/// together or all roll back — the DB is never left in a half-renamed +/// state. Each successful rename emits a structured audit line. +/// +/// The `WHERE id = $1 AND username = $3` guard defends against a +/// concurrent rename between the SELECT and this UPDATE. If some +/// other process renamed the row in that window, the UPDATE affects +/// zero rows and we log a warning but do not fail the transaction — +/// the row is already lowercase (that's why the guard didn't match), +/// so the invariant still holds. +pub async fn apply_auto_renames( + pool: &PgPool, + accounts: &[MixedCaseAccount], +) -> Result<(), sqlx::Error> { + let mut tx = pool.begin().await?; + for acc in accounts { + let new_username = acc.username.to_ascii_lowercase(); + let res = sqlx::query( + r#" + UPDATE auth.users + SET username = $2 + WHERE id = $1 + AND username = $3 + "#, + ) + .bind(acc.id) + .bind(&new_username) + .bind(&acc.username) + .execute(&mut *tx) + .await?; + + if res.rows_affected() == 0 { + tracing::warn!( + target: "audit", + event = "user.username_lowercase_skipped_on_boot", + reason = "row_changed_between_verify_and_apply", + user_id = %acc.id, + expected_username = %acc.username, + "👮🏻‍♂️ skipped auto-lowercase: row was modified after verifier ran", + ); + continue; + } + + tracing::info!( + target: "audit", + event = "user.username_lowercased_on_boot", + reason = "unique_lowercase_group", + user_id = %acc.id, + old_username = %acc.username, + new_username = %new_username, + "👮🏻‍♂️ auto-lowercased username at boot", + ); + } + tx.commit().await?; + Ok(()) +} + +/// Format the FATAL error string shown when boot refuses to proceed +/// because at least one `LOWER(username)` group has multiple active +/// members. Self-sufficient — an operator at 3 AM shouldn't need to +/// consult docs to know what to do. +pub fn format_refusal_message_collisions(groups: &[CollisionGroup]) -> String { + use std::fmt::Write; + let total_members: usize = groups.iter().map(|g| g.members.len()).sum(); + let mut out = String::new(); + let _ = write!( + &mut out, + "\nFATAL: cannot start — {} colliding username group(s) \ + ({} affected account(s) in total).\n\n\ + Non-colliding mixed-case rows are auto-renamed at boot. \ + These groups can't be resolved automatically because two or \ + more active accounts share the same lowercase form, and only \ + a human can decide who keeps the canonical name.\n\n\ + Run the migration:\n\n \ + oxicloud migrate lowercase-usernames --dry-run # preview the tiebreak\n \ + oxicloud migrate lowercase-usernames # apply\n\n\ + The tiebreak rule is `last_login_at DESC NULLS LAST, \ + created_at ASC` — the most recently active member keeps the \ + canonical lowercase name; the losers get `-2`, `-3`, … as a \ + suffix. Sessions and grants survive the rename (they key on \ + user_id, not username).\n\n\ + Collision groups (up to 10 shown):\n", + groups.len(), + total_members + ); + for g in groups.iter().take(10) { + let _ = writeln!(&mut out, "\n Canonical form: {}", g.canonical); + for m in &g.members { + let last = m + .last_login_at + .map(|t| t.format("%Y-%m-%d").to_string()) + .unwrap_or_else(|| "never".to_string()); + let _ = writeln!( + &mut out, + " {} (id: {} last_login: {})", + m.username, m.id, last + ); + } + } + if groups.len() > 10 { + let _ = writeln!( + &mut out, + "\n ... and {} more group(s). Run --dry-run for the full list.", + groups.len() - 10 + ); + } + out +} + +/// Cap on the suffix-probe loop. If we ever need `-10000` there's +/// something very wrong with the account universe — collisions in the +/// wild are 2-3 accounts, not 10 K. The loud abort IS the detection. +/// See [`docs/plan/username-lowercase.md § 3. Suffix-collision robustness`]. +const SUFFIX_PROBE_CAP: i32 = 10_000; + +/// Find the next free `-` suffix for a colliding username. +/// +/// Starts at `-2` and increments until an unused suffix is +/// found. Robust against pre-existing rows already occupying some +/// suffixes (the probe steps past them). +/// +/// Called by: +/// - The migration CLI when a `LOWER(username)` group has multiple +/// members and the tiebreak winner keeps the canonical name; the +/// losers get `-2`, `-3`, … from this helper. +/// - The un-soft-delete API when re-normalising a mixed-case +/// account whose lowercase form is now taken by an active row. +/// +/// Both callers reach for this single function so the two paths +/// agree by construction — no drift risk between the migration and +/// runtime un-soft-delete. +pub async fn find_free_username_suffix(pool: &PgPool, base: &str) -> Result { + for n in 2..=SUFFIX_PROBE_CAP { + let candidate = format!("{base}-{n}"); + let exists: (bool,) = + sqlx::query_as("SELECT EXISTS(SELECT 1 FROM auth.users WHERE username = $1)") + .bind(&candidate) + .fetch_one(pool) + .await?; + if !exists.0 { + return Ok(candidate); + } + } + // If we get here, something is very wrong. Loud panic beats + // silent truncation to whatever the caller's fallback is. + panic!( + "find_free_username_suffix: exhausted {SUFFIX_PROBE_CAP} suffix probes for base '{base}'; \ + the account universe likely has an anomaly worth investigating" + ); +} + +#[cfg(test)] +mod tests { + use super::*; + + fn acc(name: &str) -> MixedCaseAccount { + MixedCaseAccount { + id: uuid::Uuid::nil(), + username: name.into(), + last_login_at: None, + } + } + + #[test] + fn refusal_message_lists_groups_and_cli() { + let groups = vec![CollisionGroup { + canonical: "alice".into(), + members: vec![acc("Alice"), acc("alice")], + }]; + let msg = format_refusal_message_collisions(&groups); + assert!(msg.contains("1 colliding username group(s)")); + assert!(msg.contains("2 affected account(s)")); + assert!(msg.contains("oxicloud migrate lowercase-usernames")); + assert!(msg.contains("Canonical form: alice")); + assert!(msg.contains("Alice")); + assert!(msg.contains("last_login: never")); + } + + #[test] + fn refusal_message_caps_group_display_and_notes_overflow() { + let groups: Vec<_> = (0..15) + .map(|i| CollisionGroup { + canonical: format!("user{i:02}"), + members: vec![acc(&format!("User{i:02}")), acc(&format!("user{i:02}"))], + }) + .collect(); + let msg = format_refusal_message_collisions(&groups); + // First 10 groups shown by canonical name. + assert!(msg.contains("Canonical form: user00")); + assert!(msg.contains("Canonical form: user09")); + // Overflow tail names how many are hidden. + assert!(msg.contains("and 5 more group(s)")); + } + + #[test] + fn refusal_message_reports_total_across_all_groups() { + // Two groups with different sizes — 2 + 3 = 5 members total. + let groups = vec![ + CollisionGroup { + canonical: "alice".into(), + members: vec![acc("Alice"), acc("alice")], + }, + CollisionGroup { + canonical: "bob".into(), + members: vec![acc("Bob"), acc("BOB"), acc("bob")], + }, + ]; + let msg = format_refusal_message_collisions(&groups); + assert!(msg.contains("2 colliding username group(s)")); + assert!(msg.contains("5 affected account(s)")); + } +} diff --git a/src/domain/entities/user.rs b/src/domain/entities/user.rs index 5e2c8692..90dceaa4 100644 --- a/src/domain/entities/user.rs +++ b/src/domain/entities/user.rs @@ -322,9 +322,15 @@ impl User { is_external: bool, ) -> UserResult { Self::validate_email(&email)?; - if let Some(ref u) = username { - Self::validate_username(u)?; - } + // Shadow `username` with the canonical (trimmed, lowercased) + // form returned by `validate_username`. Every downstream write + // consumes the shadowed binding, so the row that lands in the + // DB is always the normalised value. See + // `docs/plan/username-lowercase.md`. + let username = match username { + Some(u) => Some(Self::validate_username(&u)?), + None => None, + }; if let Some(ref h) = password_hash && h.is_empty() { @@ -809,8 +815,10 @@ impl User { /// renamed: it was display text at creation; the folder is owned /// by `user_id`. pub fn set_username(&mut self, new_username: String) -> UserResult<()> { - Self::validate_username(&new_username)?; - self.username = Some(new_username); + // Canonical form (trim + lowercase) — see + // `validate_username`. Callers can pass any case; we store + // the normalised value. + self.username = Some(Self::validate_username(&new_username)?); self.updated_at = Utc::now(); Ok(()) } @@ -881,20 +889,41 @@ impl User { /// a handle that shadows another user's email). No leading/trailing /// dot or hyphen. The character set also prevents XSS payloads from /// being stored as usernames. - fn validate_username(username: &str) -> UserResult<()> { - let len = username.chars().count(); + /// Validate AND canonicalise a username. + /// + /// Two normalisations run first, before every check: + /// - `trim()` — strip whitespace clients may have added. + /// - `to_ascii_lowercase()` — usernames are case-insensitive + /// identifiers. Users type `Alice`, `ALICE`, `alice` on + /// different clients; all three refer to the same account. + /// ASCII-only by construction (charset check below), so + /// `to_ascii_lowercase` is deterministic and locale-safe — + /// no Unicode case-folding surprises (Turkish dotted-I, + /// German ß, Greek final sigma, NFC vs NFD). + /// + /// Returns the canonical form on success. Every entity write + /// site consumes the returned string — because the signature + /// changed from `Result<()>` to `Result`, any caller + /// that ignored the result is now a compile error. That's + /// what forces every write path through the normaliser. + /// + /// See `docs/plan/username-lowercase.md` for the full design. + pub fn validate_username(username: &str) -> UserResult { + let normalized = username.trim().to_ascii_lowercase(); + + let len = normalized.chars().count(); if !(2..=64).contains(&len) { return Err(UserError::InvalidUsername( "Username must be between 2 and 64 characters".to_string(), )); } - if username.contains('@') { + if normalized.contains('@') { return Err(UserError::InvalidUsername( "Username must not contain '@' — use the email field for email addresses" .to_string(), )); } - if !username + if !normalized .chars() .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.') { @@ -903,16 +932,16 @@ impl User { .to_string(), )); } - if username.starts_with('.') - || username.starts_with('-') - || username.ends_with('.') - || username.ends_with('-') + if normalized.starts_with('.') + || normalized.starts_with('-') + || normalized.ends_with('.') + || normalized.ends_with('-') { return Err(UserError::InvalidUsername( "Username must not start or end with a dot or hyphen".to_string(), )); } - Ok(()) + Ok(normalized) } /// Basic but meaningful email validation: @@ -1046,4 +1075,54 @@ mod tests { assert_eq!(u.display_full(true), "solo@x.com"); assert_eq!(u.display_full(false), "solo@x.com"); } + + // ── validate_username: normalization + rules ───────────────────────────── + // + // Post-lowercase-migration `validate_username` returns the canonical + // (trimmed, lowercased) form on success. Every write-site consumes + // that returned string via the shadow in `User::new` / + // `set_username`, so the invariant "usernames in `auth.users` are + // always canonical" is enforced at the domain boundary. + // + // The rules that DON'T change (charset, length, no leading/trailing + // dot or hyphen, no `@`) get their coverage here too so a future + // rewrite of `validate_username` can't regress them silently. + + #[test] + fn validate_username_lowercases_and_trims() { + // Uppercase in the middle → canonical form is lowercase. + assert_eq!(User::validate_username("Alice").unwrap(), "alice"); + // All-uppercase. + assert_eq!(User::validate_username("ALICE").unwrap(), "alice"); + // Whitespace around a mixed-case name → both stripped. + assert_eq!(User::validate_username(" Alice ").unwrap(), "alice"); + // Already-canonical passes through unchanged. + assert_eq!(User::validate_username("alice").unwrap(), "alice"); + } + + #[test] + fn validate_username_charset_and_boundary_rules_survive_normalization() { + // Trailing hyphen — still rejected after the case-fold. + assert!(User::validate_username("alice-").is_err()); + // Leading dot. + assert!(User::validate_username(".alice").is_err()); + // Whitespace INSIDE the name (not just around it) — the + // charset check rejects space characters. + assert!(User::validate_username("Al ice").is_err()); + // Non-ASCII letter — usernames are ASCII-only. + assert!(User::validate_username("Álice").is_err()); + // `@` is forbidden (disjoint namespace with email lookup). + assert!(User::validate_username("alice@example").is_err()); + } + + #[test] + fn validate_username_length_bounds_apply_after_trim() { + // Two-char minimum satisfied AFTER trim. + assert_eq!(User::validate_username(" ab ").unwrap(), "ab"); + // Below the minimum after trim. + assert!(User::validate_username(" a ").is_err()); + // Above the maximum after trim. + let too_long = "a".repeat(65); + assert!(User::validate_username(&too_long).is_err()); + } } diff --git a/src/infrastructure/db.rs b/src/infrastructure/db.rs index 1c1c05ba..347b1dc1 100644 --- a/src/infrastructure/db.rs +++ b/src/infrastructure/db.rs @@ -61,6 +61,46 @@ pub async fn create_database_pools(config: &AppConfig) -> Result { } tracing::info!("Database migrations complete"); + // Username-lowercase verifier — three outcomes: + // * Clean → nothing to do. + // * AutoRenamable → non-colliding mixed-case rows exist; lowercase + // them in one transaction and continue. Silent + // action is bounded to the case where there is + // exactly one correct move ([[feedback_no_silent_auto_repair]] + // in spirit — ambiguity → refusal, unique fix → apply). + // Each rename emits an audit line. + // * Collisions → two or more active rows share a LOWER(username) + // form (e.g. `Alice` + `alice`); tiebreak needs a + // human, refuse to boot and print the CLI command. + // See `common::username_migration::verify_all_usernames_lowercase`. + use crate::common::username_migration::{ + UsernameCaseCheck, apply_auto_renames, format_refusal_message_collisions, + verify_all_usernames_lowercase, + }; + match verify_all_usernames_lowercase(&primary).await { + Ok(UsernameCaseCheck::Clean) => {} + Ok(UsernameCaseCheck::AutoRenamable(accounts)) => { + let count = accounts.len(); + if let Err(e) = apply_auto_renames(&primary, &accounts).await { + return Err(DbError(format!( + "username lowercase auto-rename failed at boot: {e}. \ + Run `oxicloud migrate lowercase-usernames --dry-run` to \ + inspect the current state, then apply manually." + ))); + } + tracing::info!( + target: "audit", + event = "user.usernames_lowercased_on_boot_summary", + renamed = count, + "auto-lowercased {count} non-colliding mixed-case username(s) at boot", + ); + } + Ok(UsernameCaseCheck::Collisions(groups)) => { + return Err(DbError(format_refusal_message_collisions(&groups))); + } + Err(msg) => return Err(DbError(msg)), + } + // --- maintenance pool --- let maintenance = create_pool_with_retries( &config.database.connection_string, diff --git a/src/infrastructure/repositories/pg/user_pg_repository.rs b/src/infrastructure/repositories/pg/user_pg_repository.rs index 7dd3bae0..e7d7eb00 100644 --- a/src/infrastructure/repositories/pg/user_pg_repository.rs +++ b/src/infrastructure/repositories/pg/user_pg_repository.rs @@ -472,8 +472,15 @@ impl UserRepository for UserPgRepository { Ok((user, flags)) } - /// Gets a user by username + /// Gets a user by username. + /// + /// Lowercases the input before binding: usernames are stored in + /// canonical (lowercase, trimmed) form by `validate_username` + /// (see `docs/plan/username-lowercase.md`), and callers may pass + /// whatever case the user typed at the login form. Normalising + /// here means every caller doesn't have to remember. async fn get_user_by_username(&self, username: &str) -> UserRepositoryResult { + let username = username.trim().to_ascii_lowercase(); let row = sqlx::query( r#" SELECT @@ -487,7 +494,7 @@ impl UserRepository for UserPgRepository { WHERE username = $1 "#, ) - .bind(username) + .bind(&username) .fetch_one(&*self.pool) .await .map_err(Self::map_sqlx_error)?; diff --git a/src/interfaces/nextcloud/basic_auth_middleware.rs b/src/interfaces/nextcloud/basic_auth_middleware.rs index dc37620a..ce93958c 100644 --- a/src/interfaces/nextcloud/basic_auth_middleware.rs +++ b/src/interfaces/nextcloud/basic_auth_middleware.rs @@ -94,6 +94,19 @@ pub async fn basic_auth_middleware( let (raw_username, password) = parse_basic_auth(auth_header).ok_or(NextcloudAuthError::Unauthorized)?; + // Canonicalise the whole Basic-Auth username to lowercase. + // + // Usernames are canonical (lowercase) in the DB post-migration + // (`docs/plan/username-lowercase.md`), and NC / DAVX5 clients that + // cached URLs from before the migration keep sending `Alice:pass` + // — the server continues to accept that indefinitely by + // lowercasing here. Safe for the multi-drive `user~drive_uuid` + // composite because UUID hex `[0-9a-f-]` lowercases to itself. + // + // ASCII-only by `validate_username`'s charset check, so + // `to_ascii_lowercase` is deterministic and locale-safe. + let raw_username = raw_username.to_ascii_lowercase(); + // ── Multi-drive composite-username parse ──────────────────────── // POC wire shape: `{username}~{drive_marker}` may appear in the // Basic Auth header. `~` was chosen because it needs no URL @@ -319,7 +332,13 @@ pub fn parse_basic_auth(header_value: &str) -> Option<(String, String)> { let decoded = String::from_utf8(decoded).ok()?; let (user, pass) = decoded.split_once(':')?; - Some((user.to_string(), pass.to_string())) + // Canonicalise the username to lowercase here too, so any caller + // that reaches for `parse_basic_auth` directly (bypassing the + // middleware wrapper) also sees the canonical form. Redundant with + // the middleware's explicit `to_ascii_lowercase` on `raw_username` + // — belt-and-braces to keep the invariant local to the parser too. + // See `docs/plan/username-lowercase.md § 4. NextCloud DAV surface`. + Some((user.to_ascii_lowercase(), pass.to_string())) } #[cfg(test)] diff --git a/src/interfaces/nextcloud/session.rs b/src/interfaces/nextcloud/session.rs index 58bc5cc1..a98a9f1a 100644 --- a/src/interfaces/nextcloud/session.rs +++ b/src/interfaces/nextcloud/session.rs @@ -107,7 +107,21 @@ fn extract_url_user(path: &str) -> Option> { // common path allocates nothing; only a percent-encoded username owns. The // old `.into_owned()` forced a `String` on EVERY path-scoped NC DAV request // (benches/ROUND19.md §M7). The caller compares by slice. - urlencoding::decode(user_seg).ok() + // + // Lowercase before returning so cached client URLs like + // `/dav/files/Alice/...` compare equal to the canonical + // (lowercase) `session.raw_username`. See + // `docs/plan/username-lowercase.md § 4. NextCloud DAV surface`. + // + // The lowercase transform always allocates (`to_ascii_lowercase` + // on a `str` returns `String`). Trades the "Cow::Borrowed common + // path" of the ROUND19 optimisation for correctness of the case- + // insensitive comparison at line 157 — a `&str` compare with a + // borrowed segment against a lowercase `session.raw_username` + // would silently mismatch for `Alice`. The alloc is one small + // String per NC DAV request; the correctness win is worth it. + let decoded = urlencoding::decode(user_seg).ok()?; + Some(std::borrow::Cow::Owned(decoded.to_ascii_lowercase())) } /// Axum extractor: the shared handle to the request's [`NcSession`]. diff --git a/tests/api/lowercase_usernames.hurl b/tests/api/lowercase_usernames.hurl new file mode 100644 index 00000000..acb4d30f --- /dev/null +++ b/tests/api/lowercase_usernames.hurl @@ -0,0 +1,160 @@ +# ============================================================= +# OxiCloud — usernames are case-insensitive (silent lowercase on ingest) +# ============================================================= +# Pin for `docs/plan/username-lowercase.md`. The plan makes usernames +# case-insensitive by canonicalising to lowercase in +# `User::validate_username`. This file covers the wire surface: +# +# 1. Registration with a mixed-case username lands as lowercase. +# 2. Login by the ORIGINAL mixed-case string succeeds (server +# normalises on lookup). +# 3. Login by ALL-CAPS of the same name also succeeds. +# 4. Login by the canonical lowercase form succeeds. +# 5. Profile rename to a mixed-case name lands as lowercase. +# 6. NC Basic Auth accepts every case variant of the same account. +# +# Character-set + boundary rules (no leading dot, no `@`, etc.) are +# NOT re-tested here — that's the Rust unit-test surface. This file +# only pins the end-to-end normalization behaviour. +# +# Runs after `setup.hurl` (admin exists). Uses a self-contained user +# to avoid interfering with other scenarios. +# ============================================================= + + +# ───────────────────────────────────────────────────────────── +# Step 1 — Register with a mixed-case username. Expect the server +# to silently store the lowercase form. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/register +Content-Type: application/json +{ + "username": "MixedCaseUser", + "email": "mixedcase@example.com", + "password": "MixedCasePassword1!" +} + +HTTP 200 + + +# ───────────────────────────────────────────────────────────── +# Step 2 — Log in with the ORIGINAL mixed-case string. Server +# should normalise on lookup and accept. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "MixedCaseUser", "password": "MixedCasePassword1!" } + +HTTP 200 +[Asserts] +jsonpath "$.access_token" isString +# The `/auth/me` response inside the login reply exposes the canonical +# stored username. Post-migration, it MUST be lowercase regardless of +# what the caller typed at registration. +jsonpath "$.user.full.user.username" == "mixedcaseuser" + + +# ───────────────────────────────────────────────────────────── +# Step 3 — Log in with ALL-CAPS. Same account, different casing. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "MIXEDCASEUSER", "password": "MixedCasePassword1!" } + +HTTP 200 +[Asserts] +jsonpath "$.user.full.user.username" == "mixedcaseuser" + + +# ───────────────────────────────────────────────────────────── +# Step 4 — Log in with the canonical lowercase form. Same account. +# Capture the token for the profile-rename step below. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "mixedcaseuser", "password": "MixedCasePassword1!" } + +HTTP 200 +[Captures] +mixed_token: jsonpath "$.access_token" +[Asserts] +jsonpath "$.user.full.user.username" == "mixedcaseuser" + + +# ───────────────────────────────────────────────────────────── +# Step 5 — Rename via profile PATCH. New name is mixed-case; server +# must store it as lowercase. Same rule as registration, applied on +# the mutation path. +# ───────────────────────────────────────────────────────────── +PATCH {{base_url}}/api/auth/me +Authorization: Bearer {{mixed_token}} +Content-Type: application/json +{ "username": "RenamedTarget" } + +HTTP 200 +[Asserts] +# The response echoes the stored (canonical) form. +jsonpath "$.full.user.username" == "renamedtarget" + + +# Old (pre-rename) username no longer resolves — login fails 401. +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "MixedCaseUser", "password": "MixedCasePassword1!" } + +HTTP 401 + + +# New (post-rename) mixed-case login succeeds. +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "RENAMEDTARGET", "password": "MixedCasePassword1!" } + +HTTP 200 +[Asserts] +jsonpath "$.user.full.user.username" == "renamedtarget" + + +# ───────────────────────────────────────────────────────────── +# Step 6 — NextCloud Basic Auth accepts every case variant. +# The middleware lowercases the decoded username on the auth path, +# and `extract_url_user` lowercases the URL segment. A cached +# client URL like `.../dav/files/RenamedTarget` continues to work +# indefinitely across the migration. +# +# Uses PROPFIND `Depth: 0` on `/remote.php/dav/files/{user}/` — a +# well-formed request that touches Basic Auth + URL parse + chroot +# resolve in one hop. 207 Multi-Status is the expected success shape. +# ───────────────────────────────────────────────────────────── +PROPFIND {{base_url}}/remote.php/dav/files/renamedtarget/ +Depth: 0 +[BasicAuth] +renamedtarget: MixedCasePassword1! + +HTTP 207 + + +PROPFIND {{base_url}}/remote.php/dav/files/RenamedTarget/ +Depth: 0 +[BasicAuth] +RenamedTarget: MixedCasePassword1! + +HTTP 207 + + +PROPFIND {{base_url}}/remote.php/dav/files/RENAMEDTARGET/ +Depth: 0 +[BasicAuth] +RENAMEDTARGET: MixedCasePassword1! + +HTTP 207 + + +# Mixed case in URL, lowercase in Basic Auth — still works because +# both surfaces normalise before comparison. +PROPFIND {{base_url}}/remote.php/dav/files/RenamedTarget/ +Depth: 0 +[BasicAuth] +renamedtarget: MixedCasePassword1! + +HTTP 207