From f42756aa290a35fdb3b1542fb23859a7bbef94fd Mon Sep 17 00:00:00 2001 From: Paul Meier Date: Sun, 21 Jun 2026 11:31:59 -0500 Subject: [PATCH] fix(oidc): make SSO callback idempotent + evict stale legacy service worker (#510) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OIDC SSO login intermittently ended on a 403 "Invalid or expired OIDC state — possible CSRF attack" even though the login had already succeeded server-side. Root cause: the (now-removed) legacy vanilla-JS frontend registered a `/sw.js` service worker that, with navigation preload enabled, double-fetched the top-level navigation to `/api/auth/oidc/callback`. The OIDC `state` is single-use, so the first callback consumed it and logged the user in while the duplicate (~0.4s later) found the state gone and returned the 403 the browser rendered. Backend — idempotent callback: after a successful web login, remember `state -> exchange_code` in a short-lived (120s) cache. A duplicate callback whose state was already consumed now replays that same redirect instead of 403-ing, returning the cached result directly without re-running the IdP code exchange (the authorization `code` is single-use too). Keyed by the unguessable 32-byte state, so it adds no new attack surface and fixes the 403 for everyone — including browsers still running a stale legacy service worker. Frontend — evict the stale worker: the current SvelteKit app registers no service worker, so fresh clients can't double-fire. But a browser that previously loaded the legacy frontend still has `/sw.js` registered and controlling pages (and `/sw.js` now 404s, so vendor self-cleanup is inconsistent). killLegacyServiceWorker() runs first in the root layout's onMount: it surgically unregisters only `/sw.js` workers, drops only the legacy `oxicloud-cache-*` caches, and reloads once (guarded). Fixes #510. Co-Authored-By: Claude Opus 4.8 --- .../src/lib/utils/killLegacyServiceWorker.ts | 28 +++++++++++++ frontend/src/routes/+layout.svelte | 3 ++ .../services/auth_application_service.rs | 40 +++++++++++++++---- 3 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 frontend/src/lib/utils/killLegacyServiceWorker.ts diff --git a/frontend/src/lib/utils/killLegacyServiceWorker.ts b/frontend/src/lib/utils/killLegacyServiceWorker.ts new file mode 100644 index 00000000..40d84f0c --- /dev/null +++ b/frontend/src/lib/utils/killLegacyServiceWorker.ts @@ -0,0 +1,28 @@ +export async function killLegacyServiceWorker(): Promise { + if (typeof navigator === 'undefined' || !('serviceWorker' in navigator)) return; + + try { + const registrations = await navigator.serviceWorker.getRegistrations(); + const legacy = registrations.filter((r) => { + const url = r.active?.scriptURL ?? r.waiting?.scriptURL ?? r.installing?.scriptURL ?? ''; + return url.endsWith('/sw.js'); + }); + if (legacy.length === 0) return; + + await Promise.all(legacy.map((r) => r.unregister())); + + if ('caches' in window) { + const keys = await caches.keys(); + await Promise.all( + keys.filter((k) => k.startsWith('oxicloud-cache')).map((k) => caches.delete(k)) + ); + } + + if (navigator.serviceWorker.controller && !sessionStorage.getItem('legacy-sw-killed')) { + sessionStorage.setItem('legacy-sw-killed', '1'); + location.reload(); + } + } catch { + /* best-effort cleanup */ + } +} diff --git a/frontend/src/routes/+layout.svelte b/frontend/src/routes/+layout.svelte index a28f6a20..dd9b5851 100644 --- a/frontend/src/routes/+layout.svelte +++ b/frontend/src/routes/+layout.svelte @@ -9,6 +9,7 @@ import { session } from '$lib/stores/session.svelte'; import { ui } from '$lib/stores/ui.svelte'; import { hashUrlToPath } from '$lib/utils/hashRedirect'; + import { killLegacyServiceWorker } from '$lib/utils/killLegacyServiceWorker'; let { children } = $props(); @@ -33,6 +34,8 @@ let ready = $state(false); onMount(async () => { + await killLegacyServiceWorker(); + // The instant HTML boot splash has done its job — the app is mounted, so // the route (login renders immediately; protected routes show their own // loading state) is already in the DOM behind it. diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index 8f94bfd6..db619cdd 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -137,6 +137,7 @@ pub struct AuthApplicationService { /// Pending one-time token codes for secure token delivery after OIDC callback. /// Auto-expires after 60 seconds via moka TTL; max 10 000 entries for DoS protection. pending_oidc_tokens: Cache, + completed_oidc_logins: Cache, /// Magic-link token repository — populated when the magic-link feature /// is enabled (PR 8+). `None` means redemption endpoints return 503. magic_link_repo: Option>, @@ -181,6 +182,10 @@ impl AuthApplicationService { .max_capacity(10_000) .time_to_live(Duration::from_secs(60)) .build(), + completed_oidc_logins: Cache::builder() + .max_capacity(10_000) + .time_to_live(Duration::from_secs(120)) + .build(), magic_link_repo: None, user_flags_cache: Cache::builder() .max_capacity(10_000) @@ -2020,13 +2025,31 @@ impl AuthApplicationService { ) -> Result { // 0. Validate CSRF state and retrieve PKCE verifier + nonce + optional NC token // (entry is auto-expired by moka TTL — remove returns None if expired) - let flow = self.pending_oidc_flows.remove(state).ok_or_else(|| { - tracing::warn!("OIDC callback with invalid/expired state token"); - DomainError::new( - ErrorKind::AccessDenied, "OIDC", - "Invalid or expired OIDC state — possible CSRF attack. Please try logging in again.", - ) - })?; + let flow = match self.pending_oidc_flows.remove(state) { + Some(flow) => flow, + None => { + if let Some(exchange_code) = self.completed_oidc_logins.get(state) { + tracing::info!( + target: "audit", + event = "oidc.callback_replayed", + reason = "duplicate_callback", + "👮🏻‍♂️ Replayed a recently-completed OIDC login for a duplicate callback (consumed state)", + ); + return Ok(OidcCallbackResult::WebLogin { exchange_code }); + } + tracing::warn!( + target: "audit", + event = "oidc.callback_rejected", + reason = "invalid_or_expired_state", + "👮🏻‍♂️ OIDC callback with invalid/expired state token", + ); + return Err(DomainError::new( + ErrorKind::AccessDenied, + "OIDC", + "Invalid or expired OIDC state — possible CSRF attack. Please try logging in again.", + )); + } + }; let (pkce_verifier, nonce, nc_flow_token) = (flow.pkce_verifier, flow.nonce, flow.nc_flow_token); @@ -2324,6 +2347,9 @@ impl AuthApplicationService { self.pending_oidc_tokens .insert(exchange_code.clone(), PendingOidcToken { auth_response }); + self.completed_oidc_logins + .insert(state.to_string(), exchange_code.clone()); + tracing::info!("OIDC login successful, one-time exchange code generated"); Ok(OidcCallbackResult::WebLogin { exchange_code })