From 4137d8677da79e07028ca296a1f7aeab28baf322 Mon Sep 17 00:00:00 2001 From: Scott Idem Date: Fri, 27 Mar 2026 13:45:38 -0400 Subject: [PATCH] =?UTF-8?q?refactor(idaa):=20simplify=20Novi=20verificatio?= =?UTF-8?q?n=20=E2=80=94=20remove=20reactive=20UUID,=20dedupe,=20rate-limi?= =?UTF-8?q?t?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UUID is set by Novi via iframe src at page load and never changes within a session (impersonation = full iframe reload). Reading it once from window.location.search eliminates reactive noise from SvelteKit client-side navigation causing spurious re-verification runs. Removed: - verify_dep $derived.by (reactive UUID + site_cfg narrowing) - dedupe snapshot + last_effect_* tracking variables - verify_backoff_attempts and exponential backoff retry logic - novi_rate_limited_until writes and UUID-change guards - ~80 lines of complexity Kept: - site_cfg_json read outside untrack (effect still re-runs when API key loads async) - verify_in_flight concurrency guard - TTL cache (prevents duplicate calls on SWR site_cfg updates) - All permission upgrade and store write logic NOTE: If Novi adds dynamic impersonation (no full reload), see comment at url_uuid declaration for what to restore. Co-Authored-By: Claude Sonnet 4.6 --- src/routes/idaa/(idaa)/+layout.svelte | 227 +++++--------------------- 1 file changed, 42 insertions(+), 185 deletions(-) diff --git a/src/routes/idaa/(idaa)/+layout.svelte b/src/routes/idaa/(idaa)/+layout.svelte index f6d1ff22..76c9ea40 100644 --- a/src/routes/idaa/(idaa)/+layout.svelte +++ b/src/routes/idaa/(idaa)/+layout.svelte @@ -26,45 +26,26 @@ interface Props { let { data, children }: Props = $props(); -// Small derived containing only fields that should trigger verification. -// $derived.by() evaluates the body reactively; $derived(() => {}) would store the -// function itself (never called) and verification would never fire. -let verify_dep = $derived.by(() => { - const uuid = data.url.searchParams.get('uuid'); - const site_cfg = $ae_loc.site_cfg_json || {}; - return { - uuid, - api_key: site_cfg.novi_idaa_api_key ?? null, - api_root: site_cfg.novi_api_root_url ?? 'https://www.idaa.org/api', - admin_li: site_cfg.novi_admin_li ?? [], - trusted_li: site_cfg.novi_trusted_li ?? [], - ttl_ms: site_cfg.novi_verified_ttl_ms ?? null - }; -}); +// UUID is set by Novi when loading the iframe — fixed for this page's lifetime. +// Impersonation causes a full iframe reload (new page load), not a SvelteKit navigation, +// so reading this once is correct and avoids reactive noise from client-side navigation. +// NOTE: If Novi ever adds dynamic impersonation (no full reload), this needs revisiting — +// reintroduce $derived.by on data.url and the UUID-change guards removed in this commit. +const url_uuid = browser ? new URLSearchParams(window.location.search).get('uuid') : null; -// True while verification is in flight OR while waiting for site config to load. -// Pre-initialized to true if a UUID is present so there is no flash of "Access Denied" -// on first render before the effect has a chance to run. -let novi_verifying: boolean = $state( - typeof window !== 'undefined' && - !!new URLSearchParams(window.location.search).get('uuid') -); +// True while the Novi API call is in flight. +// Pre-initialized to true when a UUID is present to prevent an "Access Denied" flash +// before the effect has a chance to run on first render. +let novi_verifying: boolean = $state(!!url_uuid); -// Cache / rate-limit/backoff configuration -const VERIFIED_TTL_MS_DEFAULT = 5 * 60 * 1000; // 5 minutes by default -let verify_backoff_attempts: Record = {}; // Concurrency guard — separate from novi_verifying (the UI spinner). -// novi_verifying is pre-initialized to true when a UUID is in the URL to prevent -// an "Access Denied" flash before the effect runs. Using novi_verifying as the -// in-flight guard would cause the effect to see it as "already running" and -// exit immediately, leaving the spinner stuck forever. +// Do NOT use novi_verifying as a concurrency guard: it is pre-initialized to true, +// which would cause the guard to fire immediately and skip verification entirely. let verify_in_flight = false; -// Short-window dedupe to avoid noisy re-runs from unrelated store churn -let last_effect_ts: number = 0; -let last_effect_uuid: string | null = null; -let last_effect_snapshot: string | null = null; -// Effect 1: Set URL origin and params (unchanged from original) +const VERIFIED_TTL_MS_DEFAULT = 5 * 60 * 1000; // 5 minutes + +// Effect 1: Set URL origin and params $effect(() => { untrack(() => { $ae_loc.url_origin = data.url.origin; @@ -77,114 +58,52 @@ $effect(() => { }); // Effect 2: Novi UUID verification -// Only fires when a uuid is present in the URL (i.e. the Novi iframe path). -// Non-Novi sign-in paths (User/Pass, shared passcode) will never have a uuid param, -// so this block won't run for them — their permissions are unaffected. +// The only reactive dependency is $ae_loc.site_cfg_json — the API key arrives async +// via SWR background fetch and may not be populated on first render. Reading it outside +// untrack() ensures the effect re-runs when the config loads. +// The UUID is not reactive (read once above via window.location.search). $effect(() => { if (!browser) return; - // Track only the small derived object so unrelated changes in `$ae_loc` - // don't re-run this effect. - const { uuid, api_key, api_root, admin_li, trusted_li, ttl_ms } = verify_dep; - - // NOTE: avoid reading `$ae_loc` here (it would register as a dependency - // and cause noisy re-runs). We'll log a concise message after the - // short-window dedupe so repeated pre-dedupe triggers are not shown. - - // Short-window dedupe: if the effect re-runs within 500ms and the uuid - // and a small snapshot of relevant flags haven't changed, skip. - try { - const now = Date.now(); - const has_api_key = api_key ? '1' : '0'; - // Snapshot must NOT read $idaa_loc — that would subscribe the effect to idaa_loc - // writes (which happen after verification) and cause a needless re-run cycle. - // uuid + api_key presence is sufficient to detect a meaningful change. - const snapshot = `${uuid}:${has_api_key}`; - if (now - last_effect_ts < 500 && uuid === last_effect_uuid && snapshot === last_effect_snapshot) { - if (log_lvl) console.debug('IDAA verify effect: deduped noisy re-run'); - return; - } - last_effect_ts = now; - last_effect_uuid = uuid; - last_effect_snapshot = snapshot; - } catch (e) { - /* ignore snapshot errors */ - } - - // Post-dedupe debug: only log when this invocation is proceeding past - // the dedupe guard. Keep the log minimal to avoid subscribing to large - // store objects. - if (log_lvl) { - try { - console.log(`[IDAA verify effect] run ts=${new Date().toISOString()} uuid=${uuid} novi_verified=${$idaa_loc.novi_verified} novi_uuid=${$idaa_loc.novi_uuid}`); - } catch (e) { - console.debug('[IDAA verify effect] log error', e); - } - } + const site_cfg = $ae_loc.site_cfg_json || {}; + const api_key: string | null = site_cfg.novi_idaa_api_key ?? null; + const api_root: string = site_cfg.novi_api_root_url ?? 'https://www.idaa.org/api'; + const admin_li: string[] = site_cfg.novi_admin_li ?? []; + const trusted_li: string[] = site_cfg.novi_trusted_li ?? []; + const ttl_ms: number = site_cfg.novi_verified_ttl_ms ?? VERIFIED_TTL_MS_DEFAULT; untrack(() => { - if (!uuid) { - // No UUID in URL — non-Novi path, nothing to do here. + if (!url_uuid) { + // No UUID in URL — non-Novi path (user/pass or shared passcode sign-in). $idaa_loc.novi_verified = false; novi_verifying = false; return; } - // If a verification call is already in flight, skip starting another. - // NOTE: do not use novi_verifying here — it is pre-initialized to true when - // a UUID is present (to prevent the Access Denied flash) and would cause this - // guard to exit immediately before any API call is ever made. - if (verify_in_flight) { - if (log_lvl) console.log(`IDAA Layout: verification already in progress for ${uuid}, skipping.`); - return; - } + if (verify_in_flight) return; - // Track whether this is a different UUID than the last verified one. - // Impersonation changes the UUID mid-session; a changed UUID is always a new - // identity and must bypass TTL cache and rate-limits from the previous UUID. - const uuid_changed = uuid !== $idaa_loc.novi_uuid; + // TTL cache: skip if this UUID was recently verified. + // Prevents duplicate API calls when site_cfg_json updates multiple times (SWR pattern). const now = Date.now(); - const cfg_ttl_ms = ttl_ms ?? VERIFIED_TTL_MS_DEFAULT; - - // TTL cache: skip re-verification if this exact UUID was recently confirmed. - // Never skip when the UUID has changed (impersonation → always re-verify). if ( - !uuid_changed && $idaa_loc.novi_verified && + $idaa_loc.novi_uuid === url_uuid && $idaa_loc.novi_verified_ts && - now - $idaa_loc.novi_verified_ts < cfg_ttl_ms + now - $idaa_loc.novi_verified_ts < ttl_ms ) { - if (log_lvl) console.log(`IDAA Layout: cached verification valid for ${uuid} (${Math.round((now - $idaa_loc.novi_verified_ts) / 1000)}s old)`); + if (log_lvl) console.log(`IDAA Layout: cached verification valid for ${url_uuid}`); novi_verifying = false; return; } - // Rate limit: only apply when retrying the *same* UUID. - // A changed UUID (impersonation) bypasses rate-limits — it's a fresh identity. - if (!uuid_changed && $idaa_loc.novi_rate_limited_until && now < $idaa_loc.novi_rate_limited_until) { - if (log_lvl) console.warn(`IDAA Layout: verification skipped — rate-limited until ${new Date($idaa_loc.novi_rate_limited_until).toISOString()}`); - novi_verifying = false; - return; - } + // Load admin/trusted lists before calling verify. + // Only override when site_cfg provides them — don't wipe hardcoded defaults with []. + if (admin_li?.length) $idaa_loc.novi_admin_li = admin_li; + if (trusted_li?.length) $idaa_loc.novi_trusted_li = trusted_li; - // Load admin/trusted lists from site config first — needed by verify function. - // Only override if site_cfg_json actually provides them; falling back to [] would - // silently overwrite the hardcoded defaults in ae_idaa_stores.ts. - if (admin_li?.length) { - $idaa_loc.novi_admin_li = admin_li; - } - if (trusted_li?.length) { - $idaa_loc.novi_trusted_li = trusted_li; - } - - const novi_api_key = api_key ?? null; - const novi_api_root_url = api_root; - - // Fire-and-forget the async verification. After the first await, Svelte's - // reactive tracking no longer applies, so writes to stores are safe. verify_in_flight = true; novi_verifying = true; - verify_novi_uuid(uuid, novi_api_key, novi_api_root_url); + verify_novi_uuid(url_uuid, api_key, api_root); }); }); @@ -201,22 +120,17 @@ async function verify_novi_uuid( console.log(`IDAA Layout: Starting Novi UUID verification for ${uuid}...`); if (!api_key) { // No Novi API key in site config. All-or-nothing means no UUID-based access. - console.warn( - 'IDAA Layout: Novi API key not configured. UUID-based access denied.' - ); + console.warn('IDAA Layout: Novi API key not configured. UUID-based access denied.'); $idaa_loc.novi_uuid = null; $idaa_loc.novi_email = null; $idaa_loc.novi_full_name = null; $idaa_loc.novi_verified = false; + verify_in_flight = false; novi_verifying = false; return; } try { - if (log_lvl > 1) { - console.log(`IDAA Layout: Verifying Novi UUID ${uuid} via API...`); - } - const headers = new Headers(); headers.append('Authorization', `Basic ${api_key}`); const response = await fetch(`${api_root_url}/customers/${uuid}`, { @@ -225,52 +139,7 @@ async function verify_novi_uuid( }); if (!response.ok) { - // Rate-limited — perform exponential backoff and honor Retry-After when present - if (response.status === 429) { - const ra = response.headers.get('Retry-After'); - let retryAfterMs: number | null = null; - if (ra) { - // Retry-After can be seconds or HTTP-date - const raInt = parseInt(ra, 10); - if (!Number.isNaN(raInt)) { - retryAfterMs = raInt * 1000; - } else { - const raDate = Date.parse(ra); - if (!Number.isNaN(raDate)) { - retryAfterMs = raDate - Date.now(); - } - } - } - - const attempts = (verify_backoff_attempts[uuid] || 0) + 1; - verify_backoff_attempts[uuid] = attempts; - const expBackoff = Math.min(2 ** attempts * 1000, 5 * 60 * 1000); // cap 5m - const delay = retryAfterMs && retryAfterMs > 0 ? Math.max(retryAfterMs, expBackoff) : expBackoff; - $idaa_loc.novi_rate_limited_until = Date.now() + delay; - console.warn(`IDAA Layout: Novi API 429 for ${uuid}, backing off ${Math.round(delay/1000)}s (attempt ${attempts})`); - - // Schedule a retry after delay (small padding) - setTimeout(() => { - // Only retry if the UUID hasn't changed since we started backing off - const currentUuid = data.url.searchParams.get('uuid'); - if (currentUuid === uuid) { - console.log(`IDAA Layout: retrying Novi verification for ${uuid} after backoff`); - // set flag and call verify - novi_verifying = true; - verify_novi_uuid(uuid, api_key, api_root_url); - } else { - if (log_lvl) console.log(`IDAA Layout: uuid changed, not retrying ${uuid}`); - } - }, delay + 100); - - // End this attempt - novi_verifying = false; - return; - } - - throw new Error( - `Novi API returned ${response.status} for UUID ${uuid}` - ); + throw new Error(`Novi API returned ${response.status} for UUID ${uuid}`); } const result = await response.json(); @@ -295,9 +164,6 @@ async function verify_novi_uuid( $idaa_loc.novi_full_name = verified_name; $idaa_loc.novi_verified = true; $idaa_loc.novi_verified_ts = Date.now(); - $idaa_loc.novi_rate_limited_until = null; - // reset backoff attempts on success - verify_backoff_attempts[uuid] = 0; console.log( `IDAA Layout: Novi UUID verified. Name: ${verified_name}, Email: ${verified_email}` @@ -315,21 +181,12 @@ async function verify_novi_uuid( // PERMISSION UPGRADE STRATEGY: only apply if higher than current level. // This prevents a global 'manager' from being downgraded by the IDAA layout. const current_level = $ae_loc.access_type || 'anonymous'; - if ( - ae_util.compare_access_levels(target_novi_level, current_level) === - 1 - ) { + if (ae_util.compare_access_levels(target_novi_level, current_level) === 1) { console.log( `IDAA Layout: Upgrading access from ${current_level} to ${target_novi_level} (Novi verified)` ); const perms = ae_util.process_permission_checks(target_novi_level); $ae_loc = { ...$ae_loc, ...perms }; - } else { - if (log_lvl > 1) { - console.log( - `IDAA Layout: Keeping current access ${current_level} (Novi level ${target_novi_level} is not an upgrade)` - ); - } } // Reset BB query filters to safe defaults in case they were left in a non-default state.