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