fix(frontend): three dead ends a guest cannot get out of
**1. The cached PIN could never be cleared after a host reset.** `/recover` clears a rejected cached PIN only when the submitted name is the one this device belongs to — narrowed on this branch so a guest who mistypes their own name does not lose the only copy of their PIN (the server keeps just the bcrypt). But it compared against `DISPLAY_NAME_KEY`, which `clearAuth` deletes for shared-device privacy — one step BEFORE the guest ever reaches that screen: host taps "PIN zurücksetzen" -> the backend also revokes every session for that user -> the guest's next request 401s -> clearAuth -> redirect to /join -> they go to /recover, where the field is pre-filled with the dead PIN and the guard can never fire again Since a 4-digit value auto-submits, every correction burns another of the four wrong-PIN attempts the shared venue IP allows per 15 minutes. The PIN's owner is now stored WITH the PIN and survives alongside it, with a fallback to the auth display name for devices that cached a PIN before this key existed. **2. Every layout-level SSE handler waited on the `/me/context` retry.** The retry was awaited inside the same `onMount` that registers `pin-reset`, `user-hidden`/`user-shown`, `event-closed`/`event-opened` and `event-updated`. Worst case is a 20s timeout + 2s backoff + a second 20s timeout: ~42s with an empty handler list, on exactly the wifi the retry exists for. Five of the six self-heal; `pin-reset` does not, and a missed one leaves a dead PIN displayed in "Mein Konto" and pre-filling /recover — the same state as (1), reached from the other end. Detached, since nothing below reads its result. **3. `crypto.randomUUID` was on the join critical path.** It needs Safari >= 15.4 / Chrome >= 92 AND a secure context. The queue already depended on it, so an old phone previously joined and browsed and only failed at upload — degraded but survivable. Minting an idempotency key at join turned that into a `TypeError` caught by the generic handler and rendered as "Ein Fehler ist aufgetreten." on every retry: cannot join, cannot browse, and /recover is no help because there is no account yet. The one screen where a hard failure has no way out at all. Falls back to `crypto.getRandomValues` with the RFC 4122 version and variant bits set; `Math.random` is deliberately NOT a further fallback, since a collision between two guests would replay one guest's join or upload onto another. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -19,7 +19,9 @@ import {
|
|||||||
getRole,
|
getRole,
|
||||||
getUserId,
|
getUserId,
|
||||||
clearAuth,
|
clearAuth,
|
||||||
clearPin
|
clearPin,
|
||||||
|
getPinOwner,
|
||||||
|
getDisplayName
|
||||||
} from './auth';
|
} from './auth';
|
||||||
|
|
||||||
/** Build a JWT-shaped string (header.payload.sig) with the given claims. */
|
/** Build a JWT-shaped string (header.payload.sig) with the given claims. */
|
||||||
@@ -115,3 +117,35 @@ describe('auth — JWT claim decode', () => {
|
|||||||
expect(getRole()).toBeNull();
|
expect(getRole()).toBeNull();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('auth — the cached PIN knows whose it is', () => {
|
||||||
|
// A host PIN reset also revokes the guest's sessions, so the guest reaches /recover only AFTER
|
||||||
|
// clearAuth has run. /recover clears a rejected cached PIN only when the submitted name matches
|
||||||
|
// this device's — so if the owner name did not survive clearAuth, the dead PIN could never be
|
||||||
|
// cleared and would keep pre-filling the field.
|
||||||
|
it('the PIN owner survives clearAuth, exactly as the PIN itself does', () => {
|
||||||
|
setAuth('a.b.c', '1234', 'uid-1', 'Alice');
|
||||||
|
expect(getPinOwner()).toBe('Alice');
|
||||||
|
|
||||||
|
clearAuth();
|
||||||
|
|
||||||
|
expect(getPin(), 'the PIN is deliberately kept so the guest can recover').toBe('1234');
|
||||||
|
expect(getDisplayName(), 'the display name is wiped for shared-device privacy').toBeNull();
|
||||||
|
expect(getPinOwner(), 'so the PIN owner must be stored separately, or the pair is broken').toBe(
|
||||||
|
'Alice'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('clearPin drops the owner too — no orphan pointing at a PIN that is gone', () => {
|
||||||
|
setAuth('a.b.c', '1234', 'uid-1', 'Alice');
|
||||||
|
clearPin();
|
||||||
|
expect(getPin()).toBeNull();
|
||||||
|
expect(getPinOwner()).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('no cached PIN means no owner, even if a display name is present', () => {
|
||||||
|
setAuth('a.b.c', null, 'uid-1', 'Alice');
|
||||||
|
expect(getPin()).toBeNull();
|
||||||
|
expect(getPinOwner()).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -3,6 +3,23 @@ import { browser } from '$app/environment';
|
|||||||
|
|
||||||
const TOKEN_KEY = 'eventsnap_jwt';
|
const TOKEN_KEY = 'eventsnap_jwt';
|
||||||
const PIN_KEY = 'eventsnap_pin';
|
const PIN_KEY = 'eventsnap_pin';
|
||||||
|
/**
|
||||||
|
* Whose PIN `PIN_KEY` holds — and it is a SEPARATE key from `DISPLAY_NAME_KEY` on purpose.
|
||||||
|
*
|
||||||
|
* `/recover` only clears a rejected cached PIN when the name submitted is the one this device
|
||||||
|
* belongs to, so that a guest who mistypes their own name does not lose the only copy of their PIN
|
||||||
|
* (the server keeps just the bcrypt). That check read `DISPLAY_NAME_KEY` — which `clearAuth`
|
||||||
|
* deletes, for shared-device privacy, one step BEFORE the guest ever reaches `/recover`:
|
||||||
|
*
|
||||||
|
* host taps "PIN zurücksetzen" → the backend also revokes every session for that user
|
||||||
|
* (`host.rs`, `Session::delete_all_for_user`) → the guest's next request 401s → `clearAuth`
|
||||||
|
* → redirect to /join → the guest goes to /recover, where the field is PRE-FILLED with the
|
||||||
|
* dead PIN and can never be cleared, because the name it would be compared against is gone
|
||||||
|
*
|
||||||
|
* Since `clearAuth` deliberately keeps the PIN so the guest can recover, it must keep the PIN's
|
||||||
|
* owner too, or the pair is inconsistent and the guard is unreachable exactly when it is needed.
|
||||||
|
*/
|
||||||
|
const PIN_OWNER_KEY = 'eventsnap_pin_owner';
|
||||||
const USER_ID_KEY = 'eventsnap_user_id';
|
const USER_ID_KEY = 'eventsnap_user_id';
|
||||||
const DISPLAY_NAME_KEY = 'eventsnap_display_name';
|
const DISPLAY_NAME_KEY = 'eventsnap_display_name';
|
||||||
|
|
||||||
@@ -43,9 +60,22 @@ export function getPin(): string | null {
|
|||||||
export function clearPin(): void {
|
export function clearPin(): void {
|
||||||
if (!browser) return;
|
if (!browser) return;
|
||||||
localStorage.removeItem(PIN_KEY);
|
localStorage.removeItem(PIN_KEY);
|
||||||
|
localStorage.removeItem(PIN_OWNER_KEY);
|
||||||
currentPin.set(null);
|
currentPin.set(null);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The display name the cached PIN belongs to, or `null` if there is no cached PIN.
|
||||||
|
*
|
||||||
|
* Survives `clearAuth` alongside the PIN itself — see [`PIN_OWNER_KEY`]. Falls back to the auth
|
||||||
|
* display name for devices that cached a PIN before this key existed.
|
||||||
|
*/
|
||||||
|
export function getPinOwner(): string | null {
|
||||||
|
if (!browser) return null;
|
||||||
|
if (localStorage.getItem(PIN_KEY) === null) return null;
|
||||||
|
return localStorage.getItem(PIN_OWNER_KEY) ?? readAuth(DISPLAY_NAME_KEY);
|
||||||
|
}
|
||||||
|
|
||||||
export function getUserId(): string | null {
|
export function getUserId(): string | null {
|
||||||
return readAuth(USER_ID_KEY);
|
return readAuth(USER_ID_KEY);
|
||||||
}
|
}
|
||||||
@@ -81,6 +111,8 @@ export function setAuth(
|
|||||||
localStorage.setItem(TOKEN_KEY, jwt);
|
localStorage.setItem(TOKEN_KEY, jwt);
|
||||||
if (pin) {
|
if (pin) {
|
||||||
localStorage.setItem(PIN_KEY, pin);
|
localStorage.setItem(PIN_KEY, pin);
|
||||||
|
// Stored with the PIN, not derived from it later — see `PIN_OWNER_KEY`.
|
||||||
|
if (displayName) localStorage.setItem(PIN_OWNER_KEY, displayName);
|
||||||
currentPin.set(pin);
|
currentPin.set(pin);
|
||||||
}
|
}
|
||||||
localStorage.setItem(USER_ID_KEY, userId);
|
localStorage.setItem(USER_ID_KEY, userId);
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import { openDB, type IDBPDatabase } from 'idb';
|
import { openDB, type IDBPDatabase } from 'idb';
|
||||||
|
import { uuid } from '$lib/uuid';
|
||||||
import { writable, get } from 'svelte/store';
|
import { writable, get } from 'svelte/store';
|
||||||
import { getToken, getUserId, clearAuth, onClearAuth, onSetAuth } from './auth';
|
import { getToken, getUserId, clearAuth, onClearAuth, onSetAuth } from './auth';
|
||||||
import { onSseEvent } from './sse';
|
import { onSseEvent } from './sse';
|
||||||
@@ -885,7 +886,7 @@ export async function addToQueue(
|
|||||||
|
|
||||||
// This id is also the server-side idempotency key (`client_upload_id`), so it is minted
|
// This id is also the server-side idempotency key (`client_upload_id`), so it is minted
|
||||||
// exactly ONCE per file here and reused by every retry — see uploadItem.
|
// exactly ONCE per file here and reused by every retry — see uploadItem.
|
||||||
const id = crypto.randomUUID();
|
const id = uuid();
|
||||||
const entry: QueueEntry = {
|
const entry: QueueEntry = {
|
||||||
id,
|
id,
|
||||||
userId,
|
userId,
|
||||||
|
|||||||
47
frontend/src/lib/uuid.test.ts
Normal file
47
frontend/src/lib/uuid.test.ts
Normal file
@@ -0,0 +1,47 @@
|
|||||||
|
// @vitest-environment jsdom
|
||||||
|
import { describe, it, expect, vi, afterEach } from 'vitest';
|
||||||
|
import { uuid } from './uuid';
|
||||||
|
|
||||||
|
const V4 = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/;
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.unstubAllGlobals();
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('uuid', () => {
|
||||||
|
it('uses crypto.randomUUID when it is available', () => {
|
||||||
|
const spy = vi.spyOn(crypto, 'randomUUID');
|
||||||
|
expect(uuid()).toMatch(V4);
|
||||||
|
expect(spy).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
// The case that matters: an idempotency key is now minted on the JOIN path, and a TypeError
|
||||||
|
// there surfaces as a generic error the guest cannot get past — no account yet, so /recover is
|
||||||
|
// no help either. `crypto.randomUUID` needs a secure context and Safari >= 15.4.
|
||||||
|
it('falls back to getRandomValues when randomUUID is missing', () => {
|
||||||
|
const real = crypto.getRandomValues.bind(crypto);
|
||||||
|
vi.stubGlobal('crypto', {
|
||||||
|
getRandomValues: real
|
||||||
|
// randomUUID deliberately absent
|
||||||
|
});
|
||||||
|
|
||||||
|
const id = uuid();
|
||||||
|
expect(id, 'the fallback must still produce a well-formed v4 UUID').toMatch(V4);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('the fallback sets the version and variant bits, not just random hex', () => {
|
||||||
|
// Every byte 0x00 — so version/variant nibbles can only be right if they are set explicitly.
|
||||||
|
vi.stubGlobal('crypto', {
|
||||||
|
getRandomValues: (a: Uint8Array) => a.fill(0)
|
||||||
|
});
|
||||||
|
expect(uuid()).toBe('00000000-0000-4000-8000-000000000000');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('produces distinct values through the fallback', () => {
|
||||||
|
const real = crypto.getRandomValues.bind(crypto);
|
||||||
|
vi.stubGlobal('crypto', { getRandomValues: real });
|
||||||
|
const ids = new Set(Array.from({ length: 200 }, () => uuid()));
|
||||||
|
expect(ids.size, 'a collision would replay one guest join or upload onto another').toBe(200);
|
||||||
|
});
|
||||||
|
});
|
||||||
37
frontend/src/lib/uuid.ts
Normal file
37
frontend/src/lib/uuid.ts
Normal file
@@ -0,0 +1,37 @@
|
|||||||
|
/**
|
||||||
|
* A v4 UUID, with a fallback for environments where `crypto.randomUUID` is missing.
|
||||||
|
*
|
||||||
|
* `crypto.randomUUID` needs Safari ≥ 15.4 / Chrome ≥ 92 **and a secure context**. Production is
|
||||||
|
* HTTPS (Caddy terminates TLS for `{$DOMAIN}`), so the realistic gap is an old phone — but the
|
||||||
|
* consequence changed when idempotency keys moved onto the join path. Previously such a device
|
||||||
|
* joined and browsed fine and only failed at upload, a degraded but survivable experience. Now the
|
||||||
|
* `TypeError` is thrown inside `handleJoin`'s `try` and surfaces as the generic
|
||||||
|
* "Ein Fehler ist aufgetreten.", identically on every retry: the guest cannot join, cannot browse,
|
||||||
|
* and cannot use `/recover` either, because they have no account yet. It is the one screen in the
|
||||||
|
* app where a hard failure leaves no way out at all.
|
||||||
|
*
|
||||||
|
* The fallback is `crypto.getRandomValues` (universally available, no secure-context requirement)
|
||||||
|
* with the version and variant bits set per RFC 4122 §4.4. `Math.random` is deliberately NOT a
|
||||||
|
* further fallback: these values are idempotency keys, and a collision between two guests would
|
||||||
|
* replay one guest's join or upload to another. If there is no CSPRNG at all, throwing is correct.
|
||||||
|
*/
|
||||||
|
export function uuid(): string {
|
||||||
|
if (typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function') {
|
||||||
|
return crypto.randomUUID();
|
||||||
|
}
|
||||||
|
|
||||||
|
const bytes = new Uint8Array(16);
|
||||||
|
crypto.getRandomValues(bytes);
|
||||||
|
bytes[6] = (bytes[6] & 0x0f) | 0x40; // version 4
|
||||||
|
bytes[8] = (bytes[8] & 0x3f) | 0x80; // variant 10xx
|
||||||
|
|
||||||
|
const hex: string[] = [];
|
||||||
|
for (const b of bytes) hex.push(b.toString(16).padStart(2, '0'));
|
||||||
|
return [
|
||||||
|
hex.slice(0, 4).join(''),
|
||||||
|
hex.slice(4, 6).join(''),
|
||||||
|
hex.slice(6, 8).join(''),
|
||||||
|
hex.slice(8, 10).join(''),
|
||||||
|
hex.slice(10, 16).join('')
|
||||||
|
].join('-');
|
||||||
|
}
|
||||||
@@ -126,21 +126,39 @@
|
|||||||
// and, outside it, every SSE listener registration — still has to run.
|
// and, outside it, every SSE listener registration — still has to run.
|
||||||
const worthRetrying = !(err instanceof ApiError && err.status === 401);
|
const worthRetrying = !(err instanceof ApiError && err.status === 401);
|
||||||
if (worthRetrying) {
|
if (worthRetrying) {
|
||||||
try {
|
// DETACHED, not awaited. Everything below — including every SSE listener
|
||||||
await new Promise((r) => setTimeout(r, 2000));
|
// registered outside this block — used to sit behind it, and the worst case is
|
||||||
const ctx = await api.get<MeContextDto>('/me/context');
|
// a 20 s request timeout + 2 s backoff + a second 20 s timeout: ~42 s during
|
||||||
isBanned.set(ctx.is_banned);
|
// which `pin-reset`, `user-hidden`/`user-shown`, `event-closed`/`event-opened`
|
||||||
eventState.set({
|
// and `event-updated` are dispatched to an empty handler list. On `main` the
|
||||||
uploadsLocked: ctx.uploads_locked,
|
// exposure was one timeout; awaiting the retry doubled it, on exactly the wifi
|
||||||
galleryReleased: ctx.gallery_released
|
// the retry exists for.
|
||||||
});
|
//
|
||||||
void releaseResolvedParks({
|
// Five of the six self-heal (`/feed`'s mount re-reads them, and the upload
|
||||||
banned: ctx.is_banned,
|
// queue binds `event-opened`/`user-shown` at module scope, so parked photos
|
||||||
uploadsOpen: !ctx.uploads_locked && !ctx.gallery_released
|
// still release). `pin-reset` does NOT: nothing else clears the cached
|
||||||
});
|
// plaintext PIN, so a missed one leaves a dead PIN displayed in "Mein Konto"
|
||||||
} catch {
|
// and pre-filling /recover.
|
||||||
// Still down. The "Erneut" button on the parked row remains the way back.
|
//
|
||||||
}
|
// Detaching costs nothing — the retry only writes stores and releases parks,
|
||||||
|
// and no code below reads its result.
|
||||||
|
void (async () => {
|
||||||
|
try {
|
||||||
|
await new Promise((r) => setTimeout(r, 2000));
|
||||||
|
const ctx = await api.get<MeContextDto>('/me/context');
|
||||||
|
isBanned.set(ctx.is_banned);
|
||||||
|
eventState.set({
|
||||||
|
uploadsLocked: ctx.uploads_locked,
|
||||||
|
galleryReleased: ctx.gallery_released
|
||||||
|
});
|
||||||
|
void releaseResolvedParks({
|
||||||
|
banned: ctx.is_banned,
|
||||||
|
uploadsOpen: !ctx.uploads_locked && !ctx.gallery_released
|
||||||
|
});
|
||||||
|
} catch {
|
||||||
|
// Still down. The "Erneut" button on the parked row remains the way back.
|
||||||
|
}
|
||||||
|
})();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
void refreshQuota();
|
void refreshQuota();
|
||||||
|
|||||||
@@ -3,6 +3,7 @@
|
|||||||
import { goto } from '$app/navigation';
|
import { goto } from '$app/navigation';
|
||||||
import { api, ApiError } from '$lib/api';
|
import { api, ApiError } from '$lib/api';
|
||||||
import { setAuth } from '$lib/auth';
|
import { setAuth } from '$lib/auth';
|
||||||
|
import { uuid } from '$lib/uuid';
|
||||||
import { markGuideSeen } from '$lib/onboarding';
|
import { markGuideSeen } from '$lib/onboarding';
|
||||||
import { focusTrap } from '$lib/actions/focus-trap';
|
import { focusTrap } from '$lib/actions/focus-trap';
|
||||||
|
|
||||||
@@ -61,7 +62,7 @@
|
|||||||
// a retry within this page view idempotent, which is the common case.
|
// a retry within this page view idempotent, which is the common case.
|
||||||
}
|
}
|
||||||
if (!id) {
|
if (!id) {
|
||||||
id = crypto.randomUUID();
|
id = uuid();
|
||||||
try {
|
try {
|
||||||
localStorage.setItem(JOIN_KEY_STORAGE, id);
|
localStorage.setItem(JOIN_KEY_STORAGE, id);
|
||||||
} catch {
|
} catch {
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
<script lang="ts">
|
<script lang="ts">
|
||||||
import { goto, afterNavigate } from '$app/navigation';
|
import { goto, afterNavigate } from '$app/navigation';
|
||||||
import { api, ApiError } from '$lib/api';
|
import { api, ApiError } from '$lib/api';
|
||||||
import { setAuth, getPin, getToken, clearPin, getDisplayName } from '$lib/auth';
|
import { setAuth, getPin, getToken, clearPin, getPinOwner } from '$lib/auth';
|
||||||
import { markGuideSeen } from '$lib/onboarding';
|
import { markGuideSeen } from '$lib/onboarding';
|
||||||
import { browser } from '$app/environment';
|
import { browser } from '$app/environment';
|
||||||
import IconButton from '$lib/components/IconButton.svelte';
|
import IconButton from '$lib/components/IconButton.svelte';
|
||||||
@@ -95,8 +95,14 @@
|
|||||||
// So clear only when the evidence actually points at a stale cache: the name they
|
// So clear only when the evidence actually points at a stale cache: the name they
|
||||||
// submitted is the one this device belongs to, AND the PIN that was rejected is the
|
// submitted is the one this device belongs to, AND the PIN that was rejected is the
|
||||||
// cached one. Any other 401 leaves stored state untouched.
|
// cached one. Any other 401 leaves stored state untouched.
|
||||||
|
// `getPinOwner()`, NOT `getDisplayName()`. A host PIN reset also revokes the guest's
|
||||||
|
// sessions, so by the time they reach this screen `clearAuth` has already deleted the
|
||||||
|
// display name — and the guard could never fire again on that device. The dead PIN
|
||||||
|
// then pre-fills this field forever, and since a 4-digit value auto-submits, every
|
||||||
|
// correction burns another of the four wrong-PIN attempts the shared venue IP allows
|
||||||
|
// per 15 minutes. The PIN's owner is stored with the PIN and survives with it.
|
||||||
const submittedOwnName =
|
const submittedOwnName =
|
||||||
getDisplayName()?.trim().toLowerCase() === displayName.trim().toLowerCase();
|
getPinOwner()?.trim().toLowerCase() === displayName.trim().toLowerCase();
|
||||||
const submittedCachedPin = getPin() !== null && pin.trim() === getPin();
|
const submittedCachedPin = getPin() !== null && pin.trim() === getPin();
|
||||||
if (e.status === 401 && submittedOwnName && submittedCachedPin) clearPin();
|
if (e.status === 401 && submittedOwnName && submittedCachedPin) clearPin();
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
Reference in New Issue
Block a user