diff --git a/frontend/src/routes/login/+page.svelte b/frontend/src/routes/login/+page.svelte index 81450502..813a8e2d 100644 --- a/frontend/src/routes/login/+page.svelte +++ b/frontend/src/routes/login/+page.svelte @@ -319,6 +319,11 @@ 'auth.login_error_already_linked_elsewhere', 'A local account with this email already exists and is linked to a different SSO identity. Contact your administrator.' ); + case 'email_ambiguous': + return t( + 'auth.login_error_email_ambiguous', + 'Multiple local accounts match this email address. Contact your administrator to resolve.' + ); case 'callback_denied': return t( 'auth.login_error_callback_denied', diff --git a/frontend/src/routes/profile/+page.svelte b/frontend/src/routes/profile/+page.svelte index 842a5bf8..a5d5555a 100644 --- a/frontend/src/routes/profile/+page.svelte +++ b/frontend/src/routes/profile/+page.svelte @@ -453,8 +453,10 @@ // Map the stable reason keys to translated messages. Falls // back to a generic message for keys we don't recognise // (forward-compatible with new refusal reasons). + // 10 s dwell (vs the 4 s default) — the copy is long enough + // that the default vanishes before the user finishes reading. const msg = ssoLinkErrorMessage(linkError); - ui.notify(msg, 'error'); + ui.notify(msg, 'error', 10000); } if (linked !== null || linkError !== null) { const stripped = new URL(page.url); @@ -537,13 +539,14 @@ if (me) session.user = me; ui.notify(t('profile.sso_unlinked_success', 'Single sign-on disconnected.'), 'info'); } catch (err) { - if (err instanceof ApiError && err.errorType === 'AccessDenied') { + if (err instanceof ApiError && err.errorType === 'NoAlternativeAuth') { ui.notify( t( 'profile.sso_unlink_no_alt_auth', 'Set a password first — otherwise you would be locked out.' ), - 'error' + 'error', + 10000 ); } else { errorToast(err); diff --git a/migrations/20261011000000_users_normalized_email_index.sql b/migrations/20261011000000_users_normalized_email_index.sql new file mode 100644 index 00000000..f8731e44 --- /dev/null +++ b/migrations/20261011000000_users_normalized_email_index.sql @@ -0,0 +1,44 @@ +-- Identity-lookup email column + b-tree index. Powers +-- `list_users_by_normalized_email`, the auto-link ambiguity detector +-- added alongside the OIDC-linking work +-- (see docs/plan/oidc-account-linking.md § Auto-link). +-- +-- Normalization matches `common::text::normalize_email_for_link`: +-- lowercase + strip `+alias` sub-addressing from the local part. +-- Unconditional storage: every row carries the normalized form, even +-- when it equals `email`. The alternative (only populate when the +-- normalized form differs, then `WHERE email = $1 OR +-- identity_lookup_email = $1`) saves a few bytes per row but forces +-- a two-branch lookup on every call. Storage cost is minimal +-- (~30 bytes/user); simplicity of the always-populated lookup wins. +-- +-- Storing the normalized form as its own column (rather than doing +-- the computation in the WHERE clause) buys three things: +-- 1. Plain-equality SQL — the lookup is a one-liner, not a +-- SPLIT_PART/LOWER expression tree that's easy to mis-copy. +-- 2. Debuggable — operators can `SELECT username, email, +-- identity_lookup_email FROM auth.users` and immediately see +-- why two users collide under normalization. +-- 3. Automatic — GENERATED ALWAYS AS ... STORED means PostgreSQL +-- itself keeps the column in sync on every INSERT/UPDATE of +-- `email`. No trigger, no application code, no drift risk. +-- +-- Table rewrite cost: ALTER TABLE ADD COLUMN with a GENERATED +-- expression forces a full-table rewrite (each row needs the +-- computed value stored). Brief, exclusive lock. Fine at OxiCloud's +-- expected sizes (self-hosted, hundreds to tens of thousands of +-- users); would need a batched backfill on a million-row deployment. +ALTER TABLE auth.users +ADD COLUMN identity_lookup_email TEXT +GENERATED ALWAYS AS ( + LOWER( + SPLIT_PART(SPLIT_PART(email, '@', 1), '+', 1) + || '@' + || SPLIT_PART(email, '@', 2) + ) +) STORED; + +-- Regular b-tree index on the stored column. O(log n) probes for the +-- auto-link ambiguity check on the OIDC callback path. +CREATE INDEX idx_users_identity_lookup_email + ON auth.users(identity_lookup_email); diff --git a/src/application/ports/auth_ports.rs b/src/application/ports/auth_ports.rs index f020f697..4a1c6399 100644 --- a/src/application/ports/auth_ports.rs +++ b/src/application/ports/auth_ports.rs @@ -109,6 +109,14 @@ pub trait UserStoragePort: Send + Sync + 'static { /// Gets a user by email async fn get_user_by_email(&self, email: &str) -> Result; + /// Returns every user whose email normalizes to `normalized_email` + /// (see `UserRepository::list_users_by_normalized_email` for the + /// full contract and the auto-link ambiguity-detection use case). + async fn list_users_by_normalized_email( + &self, + normalized_email: &str, + ) -> Result, DomainError>; + /// Updates an existing user async fn update_user(&self, user: User) -> Result; diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index 4fe8524b..774cba4a 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -3844,26 +3844,38 @@ impl AuthApplicationService { } Err(_) => { // User doesn't exist by federation subject — try to - // match by email. Two possible outcomes: - // * Email matches an existing local user AND the - // auto-link decision tree accepts → auto-link, - // yield the linked user (falls through to session - // mint below). - // * Email matches AND auto-link refuses (config off, - // email not verified, already linked elsewhere) → - // return "contact admin" error (self-service link - // flow remains available). - // * No email match → JIT provision (existing branch). - // - // NOTE (MVP scope): exact-match lookup only. If OxiCloud - // stores `alice+work@example.com` but the IdP returns - // `alice@example.com`, the exact match misses even - // though they normalise to the same value. The user - // falls through to the "contact admin" refusal and can - // self-serve via the profile link flow. - let matched_user = self.user_storage.get_user_by_email(&oidc_email).await.ok(); + // match by email under the same normalization the + // self-service link flow uses (lowercase + strip + // `+alias`). Three possible outcomes: + // * 0 matches → JIT provision (existing branch). + // * 1 match → run the auto-link decision tree. + // * >1 match → refuse `email_ambiguous`. Two local + // rows collapsing to the same normalized email + // (`alice@example.com` + `alice+work@example.com`) + // mean we can't safely pick one to auto-link; + // admin must resolve. + let normalized = crate::common::text::normalize_email_for_link(&oidc_email); + let candidates = self + .user_storage + .list_users_by_normalized_email(&normalized) + .await + .unwrap_or_default(); - if let Some(matched) = matched_user { + if candidates.len() > 1 { + tracing::info!( + target: "audit", + event = "federation.auto_link_refused", + reason = "email_ambiguous", + normalized_email = %normalized, + candidate_count = candidates.len(), + "🔗 auto-link refused — multiple local users normalize to the IdP email", + ); + return Ok(OidcCallbackResult::AutoLinkRefused { + reason: "email_ambiguous", + }); + } + + if let Some(matched) = candidates.into_iter().next() { // Auto-link decision tree — see // docs/plan/oidc-account-linking.md § Auto-link. let can_auto_link = oidc_config.auto_link_email_match diff --git a/src/domain/repositories/user_repository.rs b/src/domain/repositories/user_repository.rs index 0782df59..7bcb4148 100644 --- a/src/domain/repositories/user_repository.rs +++ b/src/domain/repositories/user_repository.rs @@ -107,6 +107,24 @@ pub trait UserRepository: Send + Sync + 'static { /// Gets a user by email async fn get_user_by_email(&self, email: &str) -> UserRepositoryResult; + /// Returns every user whose email normalizes to `normalized_email`. + /// + /// Normalization matches `common::text::normalize_email_for_link` — + /// lowercase + strip `+alias` sub-addressing — so + /// `Alice+work@Example.com` and `alice@example.com` collapse to the + /// same key. Used by the OIDC auto-link decision tree to detect + /// ambiguity: two local rows normalizing to the IdP-returned email + /// means we can't safely pick one to auto-link, and the callback + /// must refuse (`email_ambiguous`). + /// + /// Caller passes the already-normalized value; the SQL applies the + /// same normalization to the stored side symmetrically so casing + /// and `+alias` differences on either side collapse. + async fn list_users_by_normalized_email( + &self, + normalized_email: &str, + ) -> UserRepositoryResult>; + /// Updates an existing user async fn update_user(&self, user: User) -> UserRepositoryResult; diff --git a/src/infrastructure/repositories/pg/user_pg_repository.rs b/src/infrastructure/repositories/pg/user_pg_repository.rs index d56ca2c3..6feb033e 100644 --- a/src/infrastructure/repositories/pg/user_pg_repository.rs +++ b/src/infrastructure/repositories/pg/user_pg_repository.rs @@ -498,6 +498,76 @@ impl UserRepository for UserPgRepository { )) } + /// Returns every user whose email normalizes to `normalized_email`. + /// + /// Looks up against `auth.users.identity_lookup_email`, a stored + /// GENERATED column populated by PostgreSQL from the same + /// normalization `common::text::normalize_email_for_link` applies + /// on the caller side. See migration + /// 20261011000000_users_normalized_email_index.sql — the b-tree + /// index on that column makes this an O(log n) probe. + async fn list_users_by_normalized_email( + &self, + normalized_email: &str, + ) -> UserRepositoryResult> { + let rows = sqlx::query( + r#" + SELECT + id, username, email, password_hash, role::text as role_text, + storage_quota_bytes, storage_used_bytes, + created_at, updated_at, last_login_at, active, + federation_kind, federation_issuer, federation_subject, image, is_external, + given_name, family_name, email_verified_at, preferred_locale, notify_on_share, + ui_preferences + FROM auth.users + WHERE identity_lookup_email = $1 + "#, + ) + .bind(normalized_email) + .fetch_all(&*self.pool) + .await + .map_err(Self::map_sqlx_error)?; + + let users = rows + .into_iter() + .map(|row| { + let role_str: Option = row.try_get("role_text").unwrap_or(None); + let role = match role_str.as_deref() { + Some("admin") => UserRole::Admin, + _ => UserRole::User, + }; + User::from_data_full( + row.get("id"), + row.get("username"), + row.get("email"), + row.get("password_hash"), + role, + row.get("storage_quota_bytes"), + row.get("storage_used_bytes"), + row.get("created_at"), + row.get("updated_at"), + row.get("last_login_at"), + row.get("active"), + row.get::, _>("federation_kind") + .as_deref() + .and_then(crate::domain::entities::user::FederationKind::parse), + row.get("federation_issuer"), + row.get("federation_subject"), + row.get("image"), + row.get("is_external"), + row.get("given_name"), + row.get("family_name"), + row.get("email_verified_at"), + row.get("preferred_locale"), + row.get("notify_on_share"), + row.get::("ui_preferences"), + ) + }) + .collect(); + + Ok(users) + } + /// Batch loads users by id in one query (avoids N+1 for group- /// recipient expansion). Missing ids are silently skipped — the /// caller treats absent rows as "no such recipient", same as @@ -1250,6 +1320,15 @@ impl UserStoragePort for UserPgRepository { .map_err(DomainError::from) } + async fn list_users_by_normalized_email( + &self, + normalized_email: &str, + ) -> Result, DomainError> { + UserRepository::list_users_by_normalized_email(self, normalized_email) + .await + .map_err(DomainError::from) + } + async fn update_user(&self, user: User) -> Result { UserRepository::update_user(self, user) .await diff --git a/src/interfaces/api/handlers/auth_handler.rs b/src/interfaces/api/handlers/auth_handler.rs index d5f6173d..35f57799 100644 --- a/src/interfaces/api/handlers/auth_handler.rs +++ b/src/interfaces/api/handlers/auth_handler.rs @@ -1610,6 +1610,11 @@ pub async fn oidc_callback( // JSON body would leave the user staring at raw JSON. The SPA // login page reads `?login_error=` on mount, renders a // localized notice, and strips the param via history.replaceState. + // + // Reasons currently emitted (see auth_application_service.rs + // auto-link decision tree): auto_link_disabled, + // auto_link_email_not_verified, already_linked_elsewhere, + // email_ambiguous. OidcCallbackResult::AutoLinkRefused { reason } => { let config = auth_app.oidc_config().unwrap(); let frontend_url = config.frontend_url.trim_end_matches('/'); diff --git a/tests/oidc/link_unlink.hurl b/tests/oidc/link_unlink.hurl index 31e41bb7..b8e8b3e6 100644 --- a/tests/oidc/link_unlink.hurl +++ b/tests/oidc/link_unlink.hurl @@ -47,19 +47,19 @@ # the self-service link flow. See auth_handler.rs # AutoLinkRefused arm. # +# 3. Auto-link refused — email_ambiguous. Admin creates a +# second local user whose email `admin+work@example.com` +# normalizes to the same key as admin's `admin@example.com`. +# The fake IdP then returns `admin@example.com` for a fresh +# sub; auto-link's normalized fan-out finds >1 candidate +# and refuses via /login?login_error=email_ambiguous. +# Cleans up the second user afterwards so subsequent +# scenarios aren't affected. +# # [OIDC-only user] # 10. `oidc_user` unlink refused (would lock them out) with # error_type NoAlternativeAuth. # -# NOT covered (backend gap; not a Hurl gap): -# 3. Auto-link refused — email_ambiguous. The current -# auto-link path uses `get_user_by_email` (exact match), -# not a normalized-email lookup. It CAN'T see two rows -# normalizing to the same value, so the ambiguity branch -# in the plan doc is unreachable from wire input. Needs a -# `list_users_by_normalized_email` repo method before -# Hurl can exercise it — separate PR. -# # NOT covered (config gap; would need a second server boot): # 4. Auto-link disabled by OXICLOUD_OIDC_AUTO_LINK_EMAIL_MATCH=false. # Server boots with the flag ON in server-with-oidc.env; @@ -566,7 +566,107 @@ HTTP 200 # ═════════════════════════════════════════════════════════════ -# Step 11 (Scenario 10) — Unlink refused for the OIDC-only user. +# Step 11 (Scenario 3) — Auto-link refused, email_ambiguous. +# ═════════════════════════════════════════════════════════════ +# Create a second local user whose email normalizes to admin's. +# `admin@example.com` and `alice+work@admin_example.com` don't +# collide; we need `admin+work@example.com` — same local base +# and same domain as admin. Then the fake IdP returns +# `admin@example.com` (verified) for a fresh sub. The auto-link +# decision tree's list_users_by_normalized_email finds 2 +# candidates → refuses `email_ambiguous` → callback redirects to +# /login?login_error=email_ambiguous. Both local rows survive +# untouched (no auto-link happens on either). +# +# Cleanup at the tail deletes the second user so subsequent +# scenarios see the same starting state (admin unlinked, no +# stray federation candidates). +# ═════════════════════════════════════════════════════════════ + + +# Create the collider. `autolink_csrf_token` from Step 9 is still +# valid — admin session cookies haven't rotated since (Step 10's +# refused callback set no new cookies). +POST {{base_url}}/api/admin/users +Content-Type: application/json +X-CSRF-Token: {{autolink_csrf_token}} +{ + "username": "admin_alias", + "email": "admin+work@example.com", + "password": "TestPassword1!", + "role": "user" +} + +HTTP 201 +[Captures] +alias_user_id: jsonpath "$.id" + + +# Point the fake IdP at a fresh sub with admin's email. Both +# admin@example.com and admin+work@example.com normalize to +# admin@example.com — auto-link must see both and refuse. +POST {{oidc_issuer}}/control/set-sub +Content-Type: application/json +{ "sub": "sub-ambiguous" } + +HTTP 200 + + +POST {{oidc_issuer}}/control/set-email +Content-Type: application/json +{ "email": "{{email}}" } + +HTTP 200 + + +GET {{base_url}}/api/auth/oidc/authorize +[Options] +location: true +location-trusted: true + +HTTP 200 +[Asserts] +url matches "^http://localhost:8087/login\\?login_error=email_ambiguous$" + + +# Belt-and-braces invariant: neither admin nor admin_alias got +# federation columns populated. The refusal fires BEFORE +# link_federation_identity. +GET {{base_url}}/api/auth/me + +HTTP 200 +[Asserts] +jsonpath "$.federation_kind" not exists + + +# Cleanup — delete the collider so later scenarios see the same +# initial state. Uses the current admin session cookies + CSRF. +DELETE {{base_url}}/api/admin/users/{{alias_user_id}} +X-CSRF-Token: {{autolink_csrf_token}} + +HTTP * +[Asserts] +status < 400 + + +# Reset IdP back to defaults before the oidc_user re-login step +# (which needs the real TEST_USER_SUB + oidc@example.com). +POST {{oidc_issuer}}/control/set-sub +Content-Type: application/json +{} + +HTTP 200 + + +POST {{oidc_issuer}}/control/set-email +Content-Type: application/json +{} + +HTTP 200 + + +# ═════════════════════════════════════════════════════════════ +# Step 12 (Scenario 10) — Unlink refused for the OIDC-only user. # ═════════════════════════════════════════════════════════════ # Fresh OIDC login as `oidc_user` (the JIT-provisioned # federated principal from oidc.hurl). Uses the reset default