From a21c4d3bf5904ddab96c90ca2d0590076897a21c Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Tue, 1 Sep 2026 22:40:23 +0200 Subject: [PATCH 1/8] fix(guests): keep guest identity across a tab close Closes #1265. The guest JWT and profile lived in sessionStorage, so the practical lifetime of an identity was "until this tab closes". GUEST_TOKEN_TTL was raised to 30 days in #1216 specifically to stop identity churn, but it governs how long the token stays valid, not how long the browser keeps it -- so it was almost never reached. A guest who closed the tab and came back through the same emailed link got the registration prompt again, and the ?invite= token in that link is single-use and already redeemed, so it could not put them back. Typing the same name inserted a second gallery_guests row: their earlier likes then belonged to an identity they could no longer act as, and could not be removed. Moved to localStorage, which is the reporter's suggestion and the one that lines up with the TTL that already exists. This does not reopen the objection #1216 raised. Deduplicating on a typed email was rejected there because anyone knowing an address could claim that person's identity, and answering differently for a known address leaks which addresses are in the gallery. This grants nothing to anyone -- it only stops the browser discarding a token it was already given. Gallery ACCESS stays in sessionStorage (galleryAuthStorage.ts) and is untouched, so a returning visitor still has to pass the gallery password before a stored identity means anything. Two things the storage swap alone would have got wrong: - Anyone with a gallery open at upgrade time would be treated as a new guest on their next reload -- the exact duplicate-row bug this fixes, fired once per in-flight guest. getGuestToken/getGuestIdentity now move a pre-#1265 sessionStorage entry across on first read. It moves rather than copies, and a fresh registration in the current tab always wins over a stale copy. clearGuestIdentity clears both stores, so "forget me" cannot be undone by a leftover being migrated back. - Identity now surviving a tab close means a second person on a shared device can be greeted by the previous visitor's name. Their only exit was "Forget me", which soft-deletes the guest row and anonymizes their feedback -- it would erase the wrong person's selections. Added a non-destructive signOut() and a "Not you?" control next to it, which only clears the identity on this device. Storage access already funnelled through one getStorage() accessor, so the swap is a one-line change there; it falls back to sessionStorage when localStorage throws (Safari private mode, blocked by policy) rather than dropping identity entirely. 6 tests. 3 fail against the old implementation, including the core "survives a tab close" case; the other 3 pin the migration and the both-stores clear. Note: the new "Not you?" string is added to en and de. i18n:ci is already red on main (11,405 missing keys) because the extractor there manages six locales while only en/de are kept at parity; this adds 4 entries of that same class. PR #1267 fixes the check itself. --- .../src/components/gallery/GalleryLayout.tsx | 13 +++ .../src/contexts/GuestIdentityContext.tsx | 17 ++++ frontend/src/i18n/locales/de.json | 1 + frontend/src/i18n/locales/en.json | 1 + .../guestIdentityStorage.persistence.test.ts | 98 ++++++++++++++++++ frontend/src/utils/guestIdentityStorage.ts | 99 +++++++++++++++++-- 6 files changed, 219 insertions(+), 10 deletions(-) create mode 100644 frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts diff --git a/frontend/src/components/gallery/GalleryLayout.tsx b/frontend/src/components/gallery/GalleryLayout.tsx index 7888a27e..9de82ac5 100644 --- a/frontend/src/components/gallery/GalleryLayout.tsx +++ b/frontend/src/components/gallery/GalleryLayout.tsx @@ -829,6 +829,19 @@ export const GalleryLayout: React.FC = ({ > {t('gallery.footer.forgetMe', 'Forget me ({{name}})', { name: guestIdentity.identity.name })} + {/* Non-destructive counterpart to "Forget me". Identity now + survives a tab close (#1265), so someone else on a shared + device can be greeted by the previous visitor's name — + and "Forget me" would delete that person's selections. + This only clears the identity on this device. */} + | + )} diff --git a/frontend/src/contexts/GuestIdentityContext.tsx b/frontend/src/contexts/GuestIdentityContext.tsx index 132908d7..1ba5fb71 100644 --- a/frontend/src/contexts/GuestIdentityContext.tsx +++ b/frontend/src/contexts/GuestIdentityContext.tsx @@ -23,6 +23,16 @@ interface GuestIdentityContextValue { recoverRequest: (email: string) => Promise; recoverVerify: (email: string, code: string) => Promise; forget: () => Promise; + /** + * Drop the stored identity on THIS device without touching the server. + * + * Distinct from forget(), which soft-deletes the guest row and anonymizes + * their feedback. Now that identity survives a tab close (#1265), a second + * person on a shared computer can be greeted as whoever used it last — and + * their only previous exit was forget(), which would erase that person's + * name and selections. This is the non-destructive way out. + */ + signOut: () => void; /** * Used by feedback components. Returns the current identity, or opens the * prompt and waits until the user registers (or cancels, in which case it @@ -144,6 +154,11 @@ export const GuestIdentityProvider: React.FC = ({ setIdentity(null); }, [slug, identity]); + const signOut = useCallback((): void => { + clearGuestIdentity(slug); + setIdentity(null); + }, [slug]); + const ensureIdentity = useCallback((): Promise => { if (identityMode !== 'guest') { // In simple mode, there is no per-person identity. Return a synthetic @@ -182,6 +197,7 @@ export const GuestIdentityProvider: React.FC = ({ recoverRequest, recoverVerify, forget, + signOut, ensureIdentity, }), [ @@ -199,6 +215,7 @@ export const GuestIdentityProvider: React.FC = ({ recoverRequest, recoverVerify, forget, + signOut, ensureIdentity, ] ); diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index 5ef5c4dd..a2c54391 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -1120,6 +1120,7 @@ "footer": { "forgetMeConfirm": "Ihr Name und Ihre Auswahl werden aus dieser Galerie entfernt.", "forgetMe": "Mich vergessen ({{name}})", + "notYou": "Nicht Sie?", "socials": "Soziale Netzwerke" }, "photosCount_one": "{{count}} Foto", diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 0c19f1e2..f4762a49 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -614,6 +614,7 @@ "footer": { "forgetMeConfirm": "Your name and selections will be removed from this gallery.", "forgetMe": "Forget me ({{name}})", + "notYou": "Not you?", "socials": "Social media" }, "photosCount_one": "{{count}} photo", diff --git a/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts b/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts new file mode 100644 index 00000000..edee73b1 --- /dev/null +++ b/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts @@ -0,0 +1,98 @@ +/** + * Guest identity has to survive a tab close (#1265). + * + * It used to live in sessionStorage, so a guest who closed the tab and came + * back through the same emailed link lost their identity. The `?invite=` token + * in that link is single-use and already redeemed, so re-registering was the + * only way back in — and that inserted a second gallery_guests row, orphaning + * the likes they had already made. + * + * The first test here is the one that matters: it fails on the old + * implementation, because closing a tab clears sessionStorage. + */ + +import { beforeEach, describe, expect, it } from 'vitest'; +import { + clearGuestIdentity, + getGuestIdentity, + getGuestToken, + storeGuestIdentity, +} from '../guestIdentityStorage'; + +const SLUG = 'wedding-summer-2026'; +const IDENTITY = { id: 42, name: 'Tina', email: 'tina@example.com', identifier: 'abc-123' }; +const TOKEN = 'header.payload.signature'; + +/** A tab close clears sessionStorage and leaves localStorage alone. */ +function closeTab(): void { + window.sessionStorage.clear(); +} + +describe('guest identity persistence (#1265)', () => { + beforeEach(() => { + window.localStorage.clear(); + window.sessionStorage.clear(); + }); + + it('survives a tab close, so a returning guest is still recognised', () => { + storeGuestIdentity(SLUG, IDENTITY as never, TOKEN); + + closeTab(); + + expect(getGuestToken(SLUG)).toBe(TOKEN); + expect(getGuestIdentity(SLUG)).toMatchObject({ id: 42, email: 'tina@example.com' }); + }); + + it('does not write the token to sessionStorage, where a tab close would drop it', () => { + storeGuestIdentity(SLUG, IDENTITY as never, TOKEN); + + expect(window.sessionStorage.getItem(`guest_token_${SLUG}`)).toBeNull(); + expect(window.localStorage.getItem(`guest_token_${SLUG}`)).toBe(TOKEN); + }); + + it('keeps identities of different galleries independent', () => { + storeGuestIdentity(SLUG, IDENTITY as never, TOKEN); + storeGuestIdentity('other-gallery', { ...IDENTITY, id: 7, name: 'Sam' } as never, 'other.token'); + + expect(getGuestToken(SLUG)).toBe(TOKEN); + expect(getGuestToken('other-gallery')).toBe('other.token'); + expect(getGuestIdentity('other-gallery')).toMatchObject({ id: 7 }); + }); + + describe('migration from the pre-#1265 sessionStorage location', () => { + it('adopts an identity left in sessionStorage by the previous build', () => { + // Exactly what the old implementation would have written. + window.sessionStorage.setItem(`guest_token_${SLUG}`, TOKEN); + window.sessionStorage.setItem(`guest_identity_${SLUG}`, JSON.stringify(IDENTITY)); + + expect(getGuestToken(SLUG)).toBe(TOKEN); + + // Moved, not copied — otherwise a later "forget me" could be undone by + // the stale copy being migrated back. + expect(window.sessionStorage.getItem(`guest_token_${SLUG}`)).toBeNull(); + expect(window.localStorage.getItem(`guest_token_${SLUG}`)).toBe(TOKEN); + }); + + it('lets a fresh registration win over a stale sessionStorage copy', () => { + window.sessionStorage.setItem(`guest_token_${SLUG}`, 'stale.token'); + storeGuestIdentity(SLUG, IDENTITY as never, TOKEN); + + expect(getGuestToken(SLUG)).toBe(TOKEN); + }); + }); + + it('clears both stores, so a cleared identity cannot be migrated back', () => { + // A guest who registered before the upgrade and again after it has a copy + // in each store; "forget me" has to remove both. + window.sessionStorage.setItem(`guest_token_${SLUG}`, 'legacy.token'); + window.sessionStorage.setItem(`guest_identity_${SLUG}`, JSON.stringify(IDENTITY)); + storeGuestIdentity(SLUG, IDENTITY as never, TOKEN); + + clearGuestIdentity(SLUG); + + expect(getGuestToken(SLUG)).toBeNull(); + expect(getGuestIdentity(SLUG)).toBeNull(); + expect(window.sessionStorage.getItem(`guest_token_${SLUG}`)).toBeNull(); + expect(window.localStorage.getItem(`guest_token_${SLUG}`)).toBeNull(); + }); +}); diff --git a/frontend/src/utils/guestIdentityStorage.ts b/frontend/src/utils/guestIdentityStorage.ts index 89c26379..4ed98a28 100644 --- a/frontend/src/utils/guestIdentityStorage.ts +++ b/frontend/src/utils/guestIdentityStorage.ts @@ -1,10 +1,26 @@ /** * Per-gallery guest identity persistence. * - * Stores the guest JWT and profile in sessionStorage, keyed by gallery slug, - * so multiple open tabs of the same gallery share identity but different - * browser contexts (and different galleries in the same context) stay - * independent. + * Stores the guest JWT and profile in localStorage, keyed by gallery slug, so + * a guest who closes the tab and comes back through the same emailed link is + * still recognised as themselves. + * + * This used to be sessionStorage, which meant the practical lifetime of the + * identity was "until this tab closes" — the GUEST_TOKEN_TTL of 30 days + * (raised in #1216 precisely to stop identity churn) was almost never reached. + * A returning guest got the registration prompt again, and because the + * `?invite=` token in their link is single-use and already redeemed, they had + * no way back to their own identity. Typing the same name again inserted a + * second gallery_guests row, so their earlier likes belonged to an identity + * they could no longer act as (#1265). + * + * Why this does not reopen the objection #1216 raised: deduplicating on a + * typed email was rejected because anyone who knows an address could claim + * that person's identity. This grants nothing to anyone — it only stops the + * browser discarding a token it was already given. Note also that gallery + * ACCESS lives in sessionStorage (galleryAuthStorage.ts) and is unaffected, so + * a returning visitor still has to pass the gallery password before a stored + * identity means anything. */ import type { GuestIdentity } from '../services/guests.service'; @@ -17,12 +33,54 @@ const isBrowser = typeof window !== 'undefined'; const getStorage = (): Storage | null => { if (!isBrowser) return null; try { - return window.sessionStorage; + return window.localStorage; } catch { - return null; + // Safari in private mode, or storage blocked by policy. Fall back to + // sessionStorage rather than dropping identity entirely — that is the old + // behaviour, which is degraded but still works within a single tab. + try { + return window.sessionStorage; + } catch { + return null; + } } }; +/** + * Move a pre-#1265 identity out of sessionStorage on first read. + * + * Without this, everyone with a gallery open at upgrade time is treated as a + * new guest the moment they reload — which is the exact duplicate-row bug this + * change exists to stop, fired once per in-flight guest. + */ +function migrateFromSessionStorage(slug: string): void { + if (!isBrowser) return; + const target = getStorage(); + if (!target || target === window.sessionStorage) return; + let legacy: Storage; + try { + legacy = window.sessionStorage; + } catch { + return; + } + const tokenKey = `${TOKEN_KEY_PREFIX}${slug}`; + const identityKey = `${IDENTITY_KEY_PREFIX}${slug}`; + try { + const legacyToken = legacy.getItem(tokenKey); + // Only migrate when the new store has nothing — a fresh registration in + // this tab must always win over a stale copy left in sessionStorage. + if (legacyToken && !target.getItem(tokenKey)) { + target.setItem(tokenKey, legacyToken); + const legacyIdentity = legacy.getItem(identityKey); + if (legacyIdentity) target.setItem(identityKey, legacyIdentity); + } + legacy.removeItem(tokenKey); + legacy.removeItem(identityKey); + } catch { + // Best-effort: a failed migration just means the guest re-registers. + } +} + export function storeGuestIdentity(slug: string, identity: GuestIdentity, token: string): void { const storage = getStorage(); if (!storage || !slug) return; @@ -35,6 +93,7 @@ export function getGuestToken(slug?: string | null): string | null { if (!storage) return null; const resolvedSlug = slug || extractSlugFromLocation(); if (!resolvedSlug) return null; + migrateFromSessionStorage(resolvedSlug); return storage.getItem(`${TOKEN_KEY_PREFIX}${resolvedSlug}`); } @@ -43,6 +102,7 @@ export function getGuestIdentity(slug?: string | null): GuestIdentity | null { if (!storage) return null; const resolvedSlug = slug || extractSlugFromLocation(); if (!resolvedSlug) return null; + migrateFromSessionStorage(resolvedSlug); const raw = storage.getItem(`${IDENTITY_KEY_PREFIX}${resolvedSlug}`); if (!raw) return null; try { @@ -53,10 +113,29 @@ export function getGuestIdentity(slug?: string | null): GuestIdentity | null { } export function clearGuestIdentity(slug: string): void { - const storage = getStorage(); - if (!storage || !slug) return; - storage.removeItem(`${TOKEN_KEY_PREFIX}${slug}`); - storage.removeItem(`${IDENTITY_KEY_PREFIX}${slug}`); + if (!slug) return; + // Clear BOTH stores: a copy left behind in sessionStorage would be migrated + // straight back on the next read, silently undoing "forget me". + const stores: Storage[] = []; + const primary = getStorage(); + if (primary) stores.push(primary); + if (isBrowser) { + try { + if (window.sessionStorage && !stores.includes(window.sessionStorage)) { + stores.push(window.sessionStorage); + } + } catch { + // sessionStorage unavailable — nothing to clear there. + } + } + for (const storage of stores) { + try { + storage.removeItem(`${TOKEN_KEY_PREFIX}${slug}`); + storage.removeItem(`${IDENTITY_KEY_PREFIX}${slug}`); + } catch { + // Best-effort. + } + } } /** From 51db1e09e901d8e7494e0182cad6ac6b9b6209d8 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 2 Sep 2026 07:44:09 +0200 Subject: [PATCH 2/8] fix(guests): expire stale tokens, sync tabs, survive unwritable storage Codex review round 1 on #1268. All three findings are consequences of the storage move itself. Expired tokens now read as absent. GUEST_TOKEN_TTL is 30 days and sessionStorage almost never survived that long, so 'stored but expired' was unreachable before; persisting the token makes it routine. Nothing else clears it -- the 401 handler in config/api.ts only drops gallery_event_ -- so the visitor was shown as signed in while every like 401'd, and ensureIdentity() short-circuited so recovery was never offered. The signature is still the server's business; an unparseable token is left alone. Tabs now stay in step. localStorage is shared where sessionStorage gave each tab its own copy, so 'Not you?' or a registration in one tab silently changed the token every other tab sends while they still displayed the old name -- their likes would land on the new guest, the exact misattribution this branch set out to stop. A storage listener rehydrates the others. Storage is probed for writability, not just readability. A store that reads but throws on setItem (quota, private mode) sailed past the read-only guard, and storeGuestIdentity threw after the server had created the guest: failed registration, retry, duplicate row. Writes are also wrapped so a storage failure degrades to a per-session identity instead of rejecting registration. --- .../src/contexts/GuestIdentityContext.tsx | 21 ++++ .../guestIdentityStorage.persistence.test.ts | 65 ++++++++++- frontend/src/utils/guestIdentityStorage.ts | 102 ++++++++++++++++-- 3 files changed, 176 insertions(+), 12 deletions(-) diff --git a/frontend/src/contexts/GuestIdentityContext.tsx b/frontend/src/contexts/GuestIdentityContext.tsx index 1ba5fb71..cb4d9552 100644 --- a/frontend/src/contexts/GuestIdentityContext.tsx +++ b/frontend/src/contexts/GuestIdentityContext.tsx @@ -67,6 +67,27 @@ export const GuestIdentityProvider: React.FC = ({ setIdentity(getGuestIdentity(slug)); }, [slug]); + // Keep tabs in step. The identity now lives in localStorage, which is shared + // across tabs — where sessionStorage gave each tab its own copy. So "Not + // you?" or a fresh registration in one tab silently changes the token the + // axios interceptor sends from every other tab, while those tabs still show + // the old name. Their likes would then be recorded against the new guest: + // the same misattribution this change set out to stop. + // + // `storage` fires only in the OTHER tabs, which is exactly the audience that + // needs to catch up. A null key means the whole store was cleared. + useEffect(() => { + if (typeof window === 'undefined') return; + const onStorage = (event: StorageEvent) => { + if (event.key && event.key !== `guest_token_${slug}` && event.key !== `guest_identity_${slug}`) { + return; + } + setIdentity(getGuestIdentity(slug)); + }; + window.addEventListener('storage', onStorage); + return () => window.removeEventListener('storage', onStorage); + }, [slug]); + // When an invite token is present on the URL (?invite=xxx), redeem it once // on mount. The server returns a guest token we can persist. useEffect(() => { diff --git a/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts b/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts index edee73b1..88c25719 100644 --- a/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts +++ b/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts @@ -13,6 +13,7 @@ import { beforeEach, describe, expect, it } from 'vitest'; import { + __resetStorageResolutionForTests, clearGuestIdentity, getGuestIdentity, getGuestToken, @@ -21,7 +22,15 @@ import { const SLUG = 'wedding-summer-2026'; const IDENTITY = { id: 42, name: 'Tina', email: 'tina@example.com', identifier: 'abc-123' }; -const TOKEN = 'header.payload.signature'; + +/** A JWT-shaped token whose `exp` is `secondsFromNow` away. Signature is irrelevant. */ +function tokenExpiringIn(secondsFromNow: number): string { + const claims = { exp: Math.floor(Date.now() / 1000) + secondsFromNow }; + const payload = btoa(JSON.stringify(claims)).replace(/\+/g, '-').replace(/\//g, '_'); + return `header.${payload}.signature`; +} + +const TOKEN = tokenExpiringIn(30 * 24 * 60 * 60); /** A tab close clears sessionStorage and leaves localStorage alone. */ function closeTab(): void { @@ -32,6 +41,7 @@ describe('guest identity persistence (#1265)', () => { beforeEach(() => { window.localStorage.clear(); window.sessionStorage.clear(); + __resetStorageResolutionForTests(); }); it('survives a tab close, so a returning guest is still recognised', () => { @@ -81,6 +91,59 @@ describe('guest identity persistence (#1265)', () => { }); }); + describe('an expired token must not look like a signed-in guest', () => { + // Persisting the token makes "stored but expired" reachable for the first + // time — sessionStorage almost never survived the 30-day TTL. Nothing else + // clears it: the 401 handler in config/api.ts only drops + // `gallery_event_`, so without this the visitor is shown as signed + // in while every like silently 401s. + it('drops an identity whose token has expired', () => { + storeGuestIdentity(SLUG, IDENTITY as never, tokenExpiringIn(-60)); + + expect(getGuestToken(SLUG)).toBeNull(); + expect(getGuestIdentity(SLUG)).toBeNull(); + // Purged, not just hidden, so the next read does no work. + expect(window.localStorage.getItem(`guest_token_${SLUG}`)).toBeNull(); + }); + + it('keeps a token that is still valid', () => { + storeGuestIdentity(SLUG, IDENTITY as never, tokenExpiringIn(3600)); + + expect(getGuestToken(SLUG)).not.toBeNull(); + expect(getGuestIdentity(SLUG)).toMatchObject({ id: 42 }); + }); + + it('leaves an unparseable token alone, so the server stays the authority', () => { + storeGuestIdentity(SLUG, IDENTITY as never, 'not-a-jwt'); + + expect(getGuestToken(SLUG)).toBe('not-a-jwt'); + }); + }); + + it('does not reject registration when the store refuses writes', () => { + // localStorage that reads fine but throws on write (quota exhausted). + // Throwing here would surface as a failed registration *after* the server + // created the guest — the visitor retries and gets a duplicate row. + const real = window.localStorage; + const throwing = { + getItem: real.getItem.bind(real), + removeItem: real.removeItem.bind(real), + key: real.key.bind(real), + get length() { return real.length; }, + clear: real.clear.bind(real), + setItem: () => { throw new DOMException('QuotaExceededError'); }, + } as unknown as Storage; + Object.defineProperty(window, 'localStorage', { value: throwing, configurable: true }); + __resetStorageResolutionForTests(); + + expect(() => storeGuestIdentity(SLUG, IDENTITY as never, TOKEN)).not.toThrow(); + // Fell back to sessionStorage rather than losing the identity entirely. + expect(window.sessionStorage.getItem(`guest_token_${SLUG}`)).toBe(TOKEN); + + Object.defineProperty(window, 'localStorage', { value: real, configurable: true }); + __resetStorageResolutionForTests(); + }); + it('clears both stores, so a cleared identity cannot be migrated back', () => { // A guest who registered before the upgrade and again after it has a copy // in each store; "forget me" has to remove both. diff --git a/frontend/src/utils/guestIdentityStorage.ts b/frontend/src/utils/guestIdentityStorage.ts index 4ed98a28..78008d2c 100644 --- a/frontend/src/utils/guestIdentityStorage.ts +++ b/frontend/src/utils/guestIdentityStorage.ts @@ -30,22 +30,87 @@ const IDENTITY_KEY_PREFIX = 'guest_identity_'; const isBrowser = typeof window !== 'undefined'; +/** + * A store is only usable if it can be WRITTEN, not merely read. + * + * Guarding the property access alone is not enough: there are browsers and + * states (quota exhausted, Safari private mode historically) where + * `window.localStorage` resolves fine but `setItem` throws. That would sail + * past a read-only guard, and then storeGuestIdentity() would throw *after* + * the server had already created the guest — the registration would report + * failure, the visitor would try again, and the retry would insert the second + * gallery_guests row this whole change exists to prevent. + */ +function isUsable(storage: Storage): boolean { + const probe = '__picpeak_probe__'; + try { + storage.setItem(probe, '1'); + storage.removeItem(probe); + return true; + } catch { + return false; + } +} + +// Resolved once. getGuestToken() runs on every API request through the axios +// interceptor, and probing a write each time would be wasteful. +let resolvedStorage: Storage | null | undefined; + const getStorage = (): Storage | null => { if (!isBrowser) return null; - try { - return window.localStorage; - } catch { - // Safari in private mode, or storage blocked by policy. Fall back to - // sessionStorage rather than dropping identity entirely — that is the old - // behaviour, which is degraded but still works within a single tab. + if (resolvedStorage !== undefined) return resolvedStorage; + + for (const pick of [() => window.localStorage, () => window.sessionStorage]) { + let candidate: Storage; try { - return window.sessionStorage; + candidate = pick(); } catch { - return null; + continue; + } + if (candidate && isUsable(candidate)) { + resolvedStorage = candidate; + return resolvedStorage; } } + // sessionStorage is the degraded fallback: identity lasts one tab, which is + // the pre-#1265 behaviour rather than no identity at all. + resolvedStorage = null; + return resolvedStorage; }; +/** Test seam — storage availability is resolved once per page load. */ +export function __resetStorageResolutionForTests(): void { + resolvedStorage = undefined; +} + +/** + * Treat a token past its `exp` as absent. + * + * GUEST_TOKEN_TTL is 30 days, and until now sessionStorage almost never + * survived long enough to reach it. Persisting the token makes "stored but + * expired" a reachable state, and nothing clears it: the 401 handler in + * config/api.ts only drops `gallery_event_`. Without this the visitor is + * shown as signed in while every like silently 401s, and ensureIdentity() + * short-circuits so they are never offered recovery. + * + * The signature is not verified here — that is the server's job. This only + * reads the expiry so the client stops presenting an identity the backend has + * already stopped honouring. A token we cannot parse is left alone rather than + * discarded, so the server stays the authority on anything ambiguous. + */ +function isExpired(token: string): boolean { + const payload = token.split('.')[1]; + if (!payload) return false; + try { + const base64 = payload.replace(/-/g, '+').replace(/_/g, '/'); + const claims = JSON.parse(atob(base64)) as { exp?: number }; + if (typeof claims.exp !== 'number') return false; + return claims.exp * 1000 <= Date.now(); + } catch { + return false; + } +} + /** * Move a pre-#1265 identity out of sessionStorage on first read. * @@ -84,8 +149,15 @@ function migrateFromSessionStorage(slug: string): void { export function storeGuestIdentity(slug: string, identity: GuestIdentity, token: string): void { const storage = getStorage(); if (!storage || !slug) return; - storage.setItem(`${TOKEN_KEY_PREFIX}${slug}`, token); - storage.setItem(`${IDENTITY_KEY_PREFIX}${slug}`, JSON.stringify(identity)); + try { + storage.setItem(`${TOKEN_KEY_PREFIX}${slug}`, token); + storage.setItem(`${IDENTITY_KEY_PREFIX}${slug}`, JSON.stringify(identity)); + } catch { + // Never let a storage failure reject registration: the guest row already + // exists server-side by this point, so throwing here would show the + // visitor an error and make them register again, creating a duplicate. + // Losing persistence degrades them to a per-session identity instead. + } } export function getGuestToken(slug?: string | null): string | null { @@ -94,7 +166,12 @@ export function getGuestToken(slug?: string | null): string | null { const resolvedSlug = slug || extractSlugFromLocation(); if (!resolvedSlug) return null; migrateFromSessionStorage(resolvedSlug); - return storage.getItem(`${TOKEN_KEY_PREFIX}${resolvedSlug}`); + const token = storage.getItem(`${TOKEN_KEY_PREFIX}${resolvedSlug}`); + if (token && isExpired(token)) { + clearGuestIdentity(resolvedSlug); + return null; + } + return token; } export function getGuestIdentity(slug?: string | null): GuestIdentity | null { @@ -103,6 +180,9 @@ export function getGuestIdentity(slug?: string | null): GuestIdentity | null { const resolvedSlug = slug || extractSlugFromLocation(); if (!resolvedSlug) return null; migrateFromSessionStorage(resolvedSlug); + // An identity whose token has expired must not be presented as signed in — + // clearGuestIdentity() has already run inside getGuestToken() in that case. + if (!getGuestToken(resolvedSlug)) return null; const raw = storage.getItem(`${IDENTITY_KEY_PREFIX}${resolvedSlug}`); if (!raw) return null; try { From f3f37a8c77b5f3338062b95bc1084c428d4cf8b2 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 2 Sep 2026 08:07:41 +0200 Subject: [PATCH 3/8] fix(guests): invite wins over stored identity; clear server-rejected ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review round 2 on #1268. Four findings, all reachable only because the identity now persists. An explicit ?invite= now takes precedence. The redeem effect skipped when an identity already existed, which was harmless while identity died with the tab. Persisted, it means opening guest B's invite on a browser where guest A once visited restores A, never redeems B's invite, and files B's likes under A. A ref keeps it to one redemption per token. Guest-scoped caches are invalidated when the identity changes. my-feedback, gallery-photos and photo-feedback are keyed by slug and photo id, never by guest, so they outlived an identity change and showed the previous guest's likes while requests already carried the new token. Now reachable three ways: another tab, 'Not you?', and an invite redeemed over an existing identity. An identity the server has rejected is dropped. resolveGuest nulls req.guest for a soft-deleted or merged-away row even when the JWT is validly signed and unexpired, and the route answers GUEST_IDENTITY_REQUIRED — no client-side expiry check can catch that. Self-limiting when identity died with the tab; persisted, it would fail every like for up to 30 days while the footer still showed the guest's name. The write fallback now covers the real write, not just the probe. A one-byte probe fits in a nearly-full store that still rejects a JWT plus profile, which left the context believing it was signed in with nothing persisted. --- frontend/src/config/api.ts | 12 ++++++- .../src/contexts/GuestIdentityContext.tsx | 36 +++++++++++++++++-- .../guestIdentityStorage.persistence.test.ts | 27 ++++++++++++++ frontend/src/utils/guestIdentityStorage.ts | 35 +++++++++++++----- 4 files changed, 98 insertions(+), 12 deletions(-) diff --git a/frontend/src/config/api.ts b/frontend/src/config/api.ts index e26a241a..ced3ca2e 100644 --- a/frontend/src/config/api.ts +++ b/frontend/src/config/api.ts @@ -5,7 +5,7 @@ import { inferGallerySlugFromLocation, resolveSlugFromRequestUrl, } from '../utils/galleryAuthStorage'; -import { getGuestToken } from '../utils/guestIdentityStorage'; +import { clearGuestIdentity, getGuestToken } from '../utils/guestIdentityStorage'; import { getApiBaseUrl } from '../utils/url'; // Maintenance mode callback @@ -181,6 +181,16 @@ api.interceptors.response.use( if (!isImageRequest && galleryMatch && galleryMatch[1]) { const gallerySlug = galleryMatch[1]; sessionStorage.removeItem(`gallery_event_${gallerySlug}`); + // A guest identity the server no longer accepts must not stay on + // the device. The JWT can be perfectly valid and unexpired while + // its row has been soft-deleted or merged away by an admin, so no + // client-side expiry check catches it. This was self-limiting when + // identity died with the tab; now it would persist for the full + // 30-day TTL, silently failing every like while the footer still + // shows the guest's name (#1265). + if (error.response?.data?.code === 'GUEST_IDENTITY_REQUIRED') { + clearGuestIdentity(gallerySlug); + } } // Don't redirect - let the component handle the auth state } else if (galleryMatch) { diff --git a/frontend/src/contexts/GuestIdentityContext.tsx b/frontend/src/contexts/GuestIdentityContext.tsx index cb4d9552..36bac6c9 100644 --- a/frontend/src/contexts/GuestIdentityContext.tsx +++ b/frontend/src/contexts/GuestIdentityContext.tsx @@ -1,4 +1,5 @@ import React, { createContext, useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react'; +import { useQueryClient } from '@tanstack/react-query'; import { guestsService, GuestIdentity } from '../services/guests.service'; import { clearGuestIdentity, @@ -54,6 +55,7 @@ export const GuestIdentityProvider: React.FC = ({ identityMode, children, }) => { + const queryClient = useQueryClient(); const [identity, setIdentity] = useState(() => getGuestIdentity(slug)); const [promptOpen, setPromptOpen] = useState(false); const [recoveryOpen, setRecoveryOpen] = useState(false); @@ -88,13 +90,41 @@ export const GuestIdentityProvider: React.FC = ({ return () => window.removeEventListener('storage', onStorage); }, [slug]); + // Guest-scoped caches are keyed by slug and photo id, never by guest id, so + // they survive an identity change and would keep showing the previous + // guest's likes, favourites and ratings while requests already carry the new + // token. Reachable three ways now: another tab signing in or out, "Not + // you?", and a ?invite= redemption over an existing identity. + const previousIdentityId = useRef(identity?.id ?? null); + useEffect(() => { + const currentId = identity?.id ?? null; + if (previousIdentityId.current === currentId) return; + previousIdentityId.current = currentId; + queryClient.invalidateQueries({ queryKey: ['my-feedback', slug] }); + queryClient.invalidateQueries({ queryKey: ['gallery-photos', slug] }); + // Every mounted photo, regardless of id. + queryClient.invalidateQueries({ queryKey: ['photo-feedback', slug] }); + }, [identity, slug, queryClient]); + // When an invite token is present on the URL (?invite=xxx), redeem it once // on mount. The server returns a guest token we can persist. + // + // Deliberately NOT skipped when an identity already exists. It used to be, + // which was harmless while identity died with the tab — but now that it + // persists, opening guest B's invite link on a browser where guest A once + // visited would restore A, skip the redemption entirely, and file B's likes + // under A. An explicit invite is the strongest statement of who the visitor + // is, so it wins over whatever the browser happens to be holding. + // + // The ref keeps it to one redemption per token: `identity` is no longer in + // the dependency list precisely because redeeming sets it. + const redeemedInviteRef = useRef(null); useEffect(() => { - if (identityMode !== 'guest' || identity) return; + if (identityMode !== 'guest') return; const params = new URLSearchParams(window.location.search); const inviteToken = params.get('invite'); - if (!inviteToken) return; + if (!inviteToken || redeemedInviteRef.current === inviteToken) return; + redeemedInviteRef.current = inviteToken; (async () => { try { @@ -112,7 +142,7 @@ export const GuestIdentityProvider: React.FC = ({ console.warn('Failed to redeem invite token', error); } })(); - }, [slug, identityMode, identity]); + }, [slug, identityMode]); const openPrompt = useCallback(() => setPromptOpen(true), []); const closePrompt = useCallback(() => { diff --git a/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts b/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts index 88c25719..4017d945 100644 --- a/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts +++ b/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts @@ -144,6 +144,33 @@ describe('guest identity persistence (#1265)', () => { __resetStorageResolutionForTests(); }); + it('falls back to sessionStorage when the probe fits but the real write does not', () => { + // The probe writes one byte; a JWT plus a profile is far larger, so a + // nearly-full store passes the probe and still rejects the real write. + // Without the retry the context believes it is signed in with nothing + // persisted — which 401s at once and re-registers on reload. + const real = window.localStorage; + const probeOnly = { + getItem: real.getItem.bind(real), + removeItem: real.removeItem.bind(real), + key: real.key.bind(real), + get length() { return real.length; }, + clear: real.clear.bind(real), + setItem: (k: string, _v: string) => { + if (k === '__picpeak_probe__') return; // probe fits + throw new DOMException('QuotaExceededError'); // the real payload does not + }, + } as unknown as Storage; + Object.defineProperty(window, 'localStorage', { value: probeOnly, configurable: true }); + __resetStorageResolutionForTests(); + + expect(() => storeGuestIdentity(SLUG, IDENTITY as never, TOKEN)).not.toThrow(); + expect(window.sessionStorage.getItem(`guest_token_${SLUG}`)).toBe(TOKEN); + + Object.defineProperty(window, 'localStorage', { value: real, configurable: true }); + __resetStorageResolutionForTests(); + }); + it('clears both stores, so a cleared identity cannot be migrated back', () => { // A guest who registered before the upgrade and again after it has a copy // in each store; "forget me" has to remove both. diff --git a/frontend/src/utils/guestIdentityStorage.ts b/frontend/src/utils/guestIdentityStorage.ts index 78008d2c..f1881312 100644 --- a/frontend/src/utils/guestIdentityStorage.ts +++ b/frontend/src/utils/guestIdentityStorage.ts @@ -149,14 +149,33 @@ function migrateFromSessionStorage(slug: string): void { export function storeGuestIdentity(slug: string, identity: GuestIdentity, token: string): void { const storage = getStorage(); if (!storage || !slug) return; - try { - storage.setItem(`${TOKEN_KEY_PREFIX}${slug}`, token); - storage.setItem(`${IDENTITY_KEY_PREFIX}${slug}`, JSON.stringify(identity)); - } catch { - // Never let a storage failure reject registration: the guest row already - // exists server-side by this point, so throwing here would show the - // visitor an error and make them register again, creating a duplicate. - // Losing persistence degrades them to a per-session identity instead. + const write = (target: Storage): boolean => { + try { + target.setItem(`${TOKEN_KEY_PREFIX}${slug}`, token); + target.setItem(`${IDENTITY_KEY_PREFIX}${slug}`, JSON.stringify(identity)); + return true; + } catch { + return false; + } + }; + + if (write(storage)) return; + + // The probe in getStorage() only proves a one-byte write fits; a JWT plus a + // profile is far larger, so a nearly-full store can pass the probe and still + // reject the real write. Retry in sessionStorage rather than leaving the + // context believing it is signed in with nothing persisted — that state + // 401s immediately and re-registers (another duplicate row) on reload. + if (isBrowser) { + try { + if (window.sessionStorage && window.sessionStorage !== storage) { + write(window.sessionStorage); + } + } catch { + // No store will take it. The identity lasts as long as this page does, + // which is strictly better than rejecting a registration the server has + // already completed. + } } } From e9babf65e7f9083cbd18de884e19941dec1900f0 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 2 Sep 2026 08:25:46 +0200 Subject: [PATCH 4/8] fix(guests): rebuild consumers on identity switch; repair fallback reads Codex review round 3 on #1268. Three of these were defects in the round 1-2 fixes themselves. Consumers holding local feedback state are now rebuilt on an identity switch. Invalidating queries was not enough: six gallery layouts seed their liked set behind a mount-only likedSeededRef ('so refetches don't clobber in-session optimistic toggles') and PhotoLightbox keeps its own copy, so a refetch left the previous guest's hearts on screen. The provider re-keys its subtree, which covers all seven without touching them. Deliberately only on a switch away from an established identity -- remounting on first sign-in would tear down the gallery under the click that triggered the prompt and drop the pending action. The storage fallback now repoints reads. storeGuestIdentity wrote to sessionStorage when localStorage rejected the real write but left resolvedStorage on localStorage, so every later read missed: x-guest-token was never sent and the identity vanished on reload. The fallback looked like it worked while achieving nothing. Clearing an identity now notifies this tab. Native storage events fire only in other documents, so the interceptor dropping a server-rejected identity left the provider still showing that guest and ensureIdentity() still handing it out. A same-tab event completes the loop. Cross-tab adoption resolves pending callers. A tab parked on the prompt awaiting ensureIdentity() while another tab registers now completes exactly as register() does, instead of hanging forever and registering a second guest if the visitor submits the still-open prompt. --- .../src/contexts/GuestIdentityContext.tsx | 59 +++++++++++++++++-- frontend/src/utils/guestIdentityStorage.ts | 24 +++++++- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/frontend/src/contexts/GuestIdentityContext.tsx b/frontend/src/contexts/GuestIdentityContext.tsx index 36bac6c9..7efa8cc5 100644 --- a/frontend/src/contexts/GuestIdentityContext.tsx +++ b/frontend/src/contexts/GuestIdentityContext.tsx @@ -2,6 +2,7 @@ import React, { createContext, useCallback, useContext, useEffect, useMemo, useR import { useQueryClient } from '@tanstack/react-query'; import { guestsService, GuestIdentity } from '../services/guests.service'; import { + GUEST_IDENTITY_CLEARED_EVENT, clearGuestIdentity, getGuestIdentity, storeGuestIdentity, @@ -80,14 +81,43 @@ export const GuestIdentityProvider: React.FC = ({ // needs to catch up. A null key means the whole store was cleared. useEffect(() => { if (typeof window === 'undefined') return; + + const adopt = () => { + const next = getGuestIdentity(slug); + setIdentity(next); + if (next) { + // A caller may be parked on the prompt waiting for ensureIdentity(). + // Another tab just answered the question, so complete them exactly as + // register() does — otherwise the action hangs forever and submitting + // the still-open prompt registers a second guest. + setPromptOpen(false); + setRecoveryOpen(false); + pendingResolvers.current.forEach((r) => r(next)); + pendingResolvers.current = []; + pendingRejecters.current = []; + } + }; + const onStorage = (event: StorageEvent) => { if (event.key && event.key !== `guest_token_${slug}` && event.key !== `guest_identity_${slug}`) { return; } - setIdentity(getGuestIdentity(slug)); + adopt(); }; + // Fires in THIS tab, where `storage` does not — e.g. the axios interceptor + // dropping an identity the server has rejected. + const onLocalClear = (event: Event) => { + const detail = (event as CustomEvent<{ slug?: string }>).detail; + if (detail?.slug && detail.slug !== slug) return; + adopt(); + }; + window.addEventListener('storage', onStorage); - return () => window.removeEventListener('storage', onStorage); + window.addEventListener(GUEST_IDENTITY_CLEARED_EVENT, onLocalClear); + return () => { + window.removeEventListener('storage', onStorage); + window.removeEventListener(GUEST_IDENTITY_CLEARED_EVENT, onLocalClear); + }; }, [slug]); // Guest-scoped caches are keyed by slug and photo id, never by guest id, so @@ -96,14 +126,29 @@ export const GuestIdentityProvider: React.FC = ({ // token. Reachable three ways now: another tab signing in or out, "Not // you?", and a ?invite= redemption over an existing identity. const previousIdentityId = useRef(identity?.id ?? null); + // Invalidating queries is not enough on its own: six gallery layouts seed + // their liked-photo set behind a `likedSeededRef` that is deliberately + // mount-only ("so refetches don't clobber in-session optimistic toggles"), + // and PhotoLightbox holds its own copy. A refetch therefore leaves the + // previous guest's hearts on screen. Bumping this generation re-keys the + // subtree so every such consumer is rebuilt. + const [identityGeneration, setIdentityGeneration] = useState(0); useEffect(() => { const currentId = identity?.id ?? null; - if (previousIdentityId.current === currentId) return; + const previousId = previousIdentityId.current; + if (previousId === currentId) return; previousIdentityId.current = currentId; + queryClient.invalidateQueries({ queryKey: ['my-feedback', slug] }); queryClient.invalidateQueries({ queryKey: ['gallery-photos', slug] }); // Every mounted photo, regardless of id. queryClient.invalidateQueries({ queryKey: ['photo-feedback', slug] }); + + // Only on a SWITCH away from an established identity — signing in for the + // first time (null -> A) must not remount, or registering from the prompt + // would tear down the gallery under the very click that triggered it and + // drop the pending action. + if (previousId !== null) setIdentityGeneration((g) => g + 1); }, [identity, slug, queryClient]); // When an invite token is present on the URL (?invite=xxx), redeem it once @@ -271,7 +316,13 @@ export const GuestIdentityProvider: React.FC = ({ ] ); - return {children}; + return ( + + {/* Re-keyed on an identity switch so consumers holding local feedback + state are rebuilt rather than showing the previous guest's. */} + {children} + + ); }; export function useGuestIdentity(): GuestIdentityContextValue { diff --git a/frontend/src/utils/guestIdentityStorage.ts b/frontend/src/utils/guestIdentityStorage.ts index f1881312..bc8e3b59 100644 --- a/frontend/src/utils/guestIdentityStorage.ts +++ b/frontend/src/utils/guestIdentityStorage.ts @@ -28,6 +28,9 @@ import type { GuestIdentity } from '../services/guests.service'; const TOKEN_KEY_PREFIX = 'guest_token_'; const IDENTITY_KEY_PREFIX = 'guest_identity_'; +/** Same-tab counterpart to the native cross-tab `storage` event. */ +export const GUEST_IDENTITY_CLEARED_EVENT = 'picpeak:guest-identity-cleared'; + const isBrowser = typeof window !== 'undefined'; /** @@ -169,7 +172,14 @@ export function storeGuestIdentity(slug: string, identity: GuestIdentity, token: if (isBrowser) { try { if (window.sessionStorage && window.sessionStorage !== storage) { - write(window.sessionStorage); + if (write(window.sessionStorage)) { + // Repoint reads at the store that actually accepted the write. + // Without this the identity is written to sessionStorage while + // getGuestToken() keeps reading localStorage, so x-guest-token is + // never sent and the identity is lost on reload — the fallback + // would look like it worked while achieving nothing. + resolvedStorage = window.sessionStorage; + } } } catch { // No store will take it. The identity lasts as long as this page does, @@ -235,6 +245,18 @@ export function clearGuestIdentity(slug: string): void { // Best-effort. } } + + // `storage` events fire only in OTHER documents, so a clear triggered inside + // this tab (the axios interceptor dropping a server-rejected identity) would + // leave the provider still showing the guest and ensureIdentity() still + // handing it out. Announce it locally too. + if (isBrowser) { + try { + window.dispatchEvent(new CustomEvent(GUEST_IDENTITY_CLEARED_EVENT, { detail: { slug } })); + } catch { + // CustomEvent unavailable — the provider simply refreshes on next mount. + } + } } /** From 7d51aa3db9a814b34a63a49d28607610ef29b15a Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 2 Sep 2026 08:26:36 +0200 Subject: [PATCH 5/8] fix(guests): don't answer feedback with a stale identity mid-invite Last open finding from codex round 3 on #1268. Invite redemption is async and the gallery stays interactive while it runs, so a like clicked in that window resolved against the persisted identity and was filed under the wrong guest permanently. ensureIdentity() now waits on the in-flight redemption and re-reads the result before falling back to the stored identity or the prompt. --- .../src/contexts/GuestIdentityContext.tsx | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/frontend/src/contexts/GuestIdentityContext.tsx b/frontend/src/contexts/GuestIdentityContext.tsx index 7efa8cc5..2f841d71 100644 --- a/frontend/src/contexts/GuestIdentityContext.tsx +++ b/frontend/src/contexts/GuestIdentityContext.tsx @@ -164,6 +164,11 @@ export const GuestIdentityProvider: React.FC = ({ // The ref keeps it to one redemption per token: `identity` is no longer in // the dependency list precisely because redeeming sets it. const redeemedInviteRef = useRef(null); + // Redemption is async, and the gallery stays interactive while it runs. A + // like clicked in that window would otherwise resolve against the persisted + // identity and be filed under the wrong guest, permanently. ensureIdentity() + // waits on this instead. + const invitePromiseRef = useRef | null>(null); useEffect(() => { if (identityMode !== 'guest') return; const params = new URLSearchParams(window.location.search); @@ -171,7 +176,7 @@ export const GuestIdentityProvider: React.FC = ({ if (!inviteToken || redeemedInviteRef.current === inviteToken) return; redeemedInviteRef.current = inviteToken; - (async () => { + invitePromiseRef.current = (async () => { try { const response = await guestsService.redeemInvite(slug, inviteToken); storeGuestIdentity(slug, response.guest, response.token); @@ -185,6 +190,8 @@ export const GuestIdentityProvider: React.FC = ({ // Silently fail invalid invites; user will fall back to normal prompt. // eslint-disable-next-line no-console console.warn('Failed to redeem invite token', error); + } finally { + invitePromiseRef.current = null; } })(); }, [slug, identityMode]); @@ -255,7 +262,7 @@ export const GuestIdentityProvider: React.FC = ({ setIdentity(null); }, [slug]); - const ensureIdentity = useCallback((): Promise => { + const ensureIdentity = useCallback(async (): Promise => { if (identityMode !== 'guest') { // In simple mode, there is no per-person identity. Return a synthetic // "null" identity that callers will ignore. @@ -266,14 +273,26 @@ export const GuestIdentityProvider: React.FC = ({ identifier: '', } as GuestIdentity); } - if (identity) return Promise.resolve(identity); + // An invite naming this visitor is mid-flight: wait for it rather than + // answering with the identity the browser happened to be holding. + if (invitePromiseRef.current) { + try { + await invitePromiseRef.current; + } catch { + // Redemption failed — fall through to the stored identity / prompt. + } + const redeemed = getGuestIdentity(slug); + if (redeemed) return redeemed; + } + + if (identity) return identity; return new Promise((resolve, reject) => { pendingResolvers.current.push(resolve); pendingRejecters.current.push(reject); setPromptOpen(true); }); - }, [identityMode, identity]); + }, [identityMode, identity, slug]); const isRequired = identityMode === 'guest' && !identity; From 7a1ea842e4b18dfe8a0a7387e4a3e36966db8db4 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 2 Sep 2026 09:42:40 +0200 Subject: [PATCH 6/8] fix(guests): read identity from whichever store holds it, write it as a pair Two defects in the storage fallback, both reproduced: The quota fallback repointed reads at sessionStorage through module state, which a reload discards. The next page load probed localStorage, passed the one-byte probe, tried to promote the pair and was refused on the same quota, swallowed that, and read an empty localStorage: the identity sat one store over, unreadable, and the guest re-registered. Reads are now read-through: primary store first, sessionStorage second, promoting into the primary only when it will take the pair and leaving it where it fits when it will not. No module state has to remember which store won. The migration wrote the token before the profile, so a store that accepted the first write and refused the second left a token with no profile: x-guest-token was sent while the provider prompted to register, producing a second row with two live tokens. Every write is now profile-first and rolls back on failure, so a store holds the whole pair or none of it. --- .../guestIdentityStorage.persistence.test.ts | 106 +++++++++ frontend/src/utils/guestIdentityStorage.ts | 208 ++++++++++++------ 2 files changed, 251 insertions(+), 63 deletions(-) diff --git a/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts b/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts index 4017d945..c3f1bc0a 100644 --- a/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts +++ b/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts @@ -171,6 +171,112 @@ describe('guest identity persistence (#1265)', () => { __resetStorageResolutionForTests(); }); + describe('the sessionStorage fallback has to be readable after a reload', () => { + // Making a store that accepts the one-byte probe and nothing else, exactly + // like a nearly-full localStorage. + const installProbeOnlyLocalStorage = () => { + const real = window.localStorage; + const probeOnly = { + getItem: real.getItem.bind(real), + removeItem: real.removeItem.bind(real), + key: real.key.bind(real), + get length() { return real.length; }, + clear: real.clear.bind(real), + setItem: (k: string, _v: string) => { + if (k === '__picpeak_probe__') return; + throw new DOMException('QuotaExceededError'); + }, + } as unknown as Storage; + Object.defineProperty(window, 'localStorage', { value: probeOnly, configurable: true }); + __resetStorageResolutionForTests(); + return () => { + Object.defineProperty(window, 'localStorage', { value: real, configurable: true }); + __resetStorageResolutionForTests(); + }; + }; + + it('still finds the identity on the next page load', () => { + const restore = installProbeOnlyLocalStorage(); + storeGuestIdentity(SLUG, IDENTITY as never, TOKEN); + + // A reload discards module state: storage is re-resolved, localStorage + // passes the probe again, and the identity is sitting in sessionStorage. + __resetStorageResolutionForTests(); + + expect(getGuestToken(SLUG)).toBe(TOKEN); + expect(getGuestIdentity(SLUG)).toMatchObject({ id: 42 }); + restore(); + }); + + it('does not keep retrying the refused promotion on every read', () => { + const restore = installProbeOnlyLocalStorage(); + storeGuestIdentity(SLUG, IDENTITY as never, TOKEN); + __resetStorageResolutionForTests(); + + // First read: tries localStorage, is refused, remembers that. + getGuestToken(SLUG); + let writes = 0; + const store = window.localStorage; + const original = store.setItem; + store.setItem = ((k: string, v: string) => { writes++; return original.call(store, k, v); }) as never; + getGuestToken(SLUG); + getGuestIdentity(SLUG); + expect(writes).toBe(0); + restore(); + }); + }); + + describe('a store never holds a token without its profile', () => { + // localStorage that takes the token key and refuses the profile key — + // the partial acceptance a nearly-full store can produce. A token without + // a profile means x-guest-token is sent while the provider prompts to + // register: the second gallery_guests row, with two live tokens. + const installTokenOnlyLocalStorage = () => { + const real = window.localStorage; + const tokenOnly = { + getItem: real.getItem.bind(real), + removeItem: real.removeItem.bind(real), + key: real.key.bind(real), + get length() { return real.length; }, + clear: real.clear.bind(real), + setItem: (k: string, v: string) => { + if (k.startsWith('guest_identity_')) throw new DOMException('QuotaExceededError'); + real.setItem(k, v); + }, + } as unknown as Storage; + Object.defineProperty(window, 'localStorage', { value: tokenOnly, configurable: true }); + __resetStorageResolutionForTests(); + return () => { + Object.defineProperty(window, 'localStorage', { value: real, configurable: true }); + __resetStorageResolutionForTests(); + }; + }; + + it('on registration, falls back as a pair and leaves no token behind', () => { + const restore = installTokenOnlyLocalStorage(); + storeGuestIdentity(SLUG, IDENTITY as never, TOKEN); + + expect(window.localStorage.getItem(`guest_token_${SLUG}`)).toBeNull(); + expect(window.sessionStorage.getItem(`guest_token_${SLUG}`)).toBe(TOKEN); + expect(getGuestToken(SLUG)).toBe(TOKEN); + expect(getGuestIdentity(SLUG)).toMatchObject({ id: 42 }); + restore(); + }); + + it('on migration, leaves the legacy pair intact rather than moving half of it', () => { + const restore = installTokenOnlyLocalStorage(); + window.sessionStorage.setItem(`guest_token_${SLUG}`, TOKEN); + window.sessionStorage.setItem(`guest_identity_${SLUG}`, JSON.stringify(IDENTITY)); + + expect(getGuestToken(SLUG)).toBe(TOKEN); + expect(getGuestIdentity(SLUG)).toMatchObject({ id: 42 }); + // Nothing half-moved: the token did not land in localStorage on its own. + expect(window.localStorage.getItem(`guest_token_${SLUG}`)).toBeNull(); + expect(window.sessionStorage.getItem(`guest_token_${SLUG}`)).toBe(TOKEN); + restore(); + }); + }); + it('clears both stores, so a cleared identity cannot be migrated back', () => { // A guest who registered before the upgrade and again after it has a copy // in each store; "forget me" has to remove both. diff --git a/frontend/src/utils/guestIdentityStorage.ts b/frontend/src/utils/guestIdentityStorage.ts index bc8e3b59..876e6286 100644 --- a/frontend/src/utils/guestIdentityStorage.ts +++ b/frontend/src/utils/guestIdentityStorage.ts @@ -84,6 +84,7 @@ const getStorage = (): Storage | null => { /** Test seam — storage availability is resolved once per page load. */ export function __resetStorageResolutionForTests(): void { resolvedStorage = undefined; + promotionFailed.clear(); } /** @@ -115,87 +116,167 @@ function isExpired(token: string): boolean { } /** - * Move a pre-#1265 identity out of sessionStorage on first read. - * - * Without this, everyone with a gallery open at upgrade time is treated as a - * new guest the moment they reload — which is the exact duplicate-row bug this - * change exists to stop, fired once per in-flight guest. + * The secondary store: sessionStorage, whenever it is distinct from the store + * reads are resolved against. Two things can live there — a pre-#1265 + * identity the previous build wrote, and the quota fallback below. */ -function migrateFromSessionStorage(slug: string): void { - if (!isBrowser) return; - const target = getStorage(); - if (!target || target === window.sessionStorage) return; - let legacy: Storage; +function secondaryStore(primary: Storage): Storage | null { + if (!isBrowser) return null; try { - legacy = window.sessionStorage; + const candidate = window.sessionStorage; + return candidate && candidate !== primary ? candidate : null; } catch { - return; + return null; } +} + +/** + * Write the pair to one store, PROFILE FIRST. A store that is nearly full can + * accept the first write and reject the second, and "token stored, profile + * missing" is the one partial state that produces the duplicate-row bug this + * file exists to stop: the interceptor sends x-guest-token while the provider + * sees no identity, prompts, and registers a second guest. A profile without + * a token is inert. Anything half-written is removed again before reporting + * failure, so a store either holds the whole pair or none of it. + */ +function writePair(target: Storage, slug: string, identityRaw: string, token: string): boolean { const tokenKey = `${TOKEN_KEY_PREFIX}${slug}`; const identityKey = `${IDENTITY_KEY_PREFIX}${slug}`; try { - const legacyToken = legacy.getItem(tokenKey); - // Only migrate when the new store has nothing — a fresh registration in - // this tab must always win over a stale copy left in sessionStorage. - if (legacyToken && !target.getItem(tokenKey)) { - target.setItem(tokenKey, legacyToken); - const legacyIdentity = legacy.getItem(identityKey); - if (legacyIdentity) target.setItem(identityKey, legacyIdentity); - } - legacy.removeItem(tokenKey); - legacy.removeItem(identityKey); + target.setItem(identityKey, identityRaw); + target.setItem(tokenKey, token); + return true; } catch { - // Best-effort: a failed migration just means the guest re-registers. + try { + target.removeItem(tokenKey); + target.removeItem(identityKey); + } catch { + // Nothing more to do. + } + return false; } } +// Slugs whose pair could not be promoted into the primary store on this page +// load. getGuestToken() runs on every API request; once the primary store has +// refused the write there is no point retrying it per request. +const promotionFailed = new Set(); + +/** + * Read the pair, looking in the primary store first and the secondary one + * second. Finding it in the secondary store covers two cases that used to be + * one-way migrations and are now read-through: + * + * - a pre-#1265 identity the previous build left in sessionStorage. Without + * this everyone with a gallery open at upgrade time is treated as a new + * guest on reload — the exact duplicate-row bug, fired once per guest; + * - the quota fallback in storeGuestIdentity(). That used to repoint reads + * at sessionStorage through module state, which a reload discards: the + * next page load probed localStorage, passed, and read nothing while the + * identity sat unreadable one store over. Reads now find it wherever it + * demonstrably fits. + * + * A pair found in the secondary store is promoted into the primary one when + * that store will take it (MOVED, not copied — a stale copy would otherwise + * be migrated straight back after "forget me"), and left where it is when it + * will not. A pair already in the primary store always wins over the + * secondary copy: a fresh registration in this tab beats a stale leftover. + */ +function readPair(slug: string): { token: string | null; identityRaw: string | null } { + const primary = getStorage(); + if (!primary) return { token: null, identityRaw: null }; + const tokenKey = `${TOKEN_KEY_PREFIX}${slug}`; + const identityKey = `${IDENTITY_KEY_PREFIX}${slug}`; + + let token: string | null = null; + let identityRaw: string | null = null; + try { + token = primary.getItem(tokenKey); + identityRaw = primary.getItem(identityKey); + } catch { + return { token: null, identityRaw: null }; + } + + const secondary = secondaryStore(primary); + if (!secondary) return { token, identityRaw }; + + let secondaryToken: string | null = null; + let secondaryIdentity: string | null = null; + try { + secondaryToken = secondary.getItem(tokenKey); + secondaryIdentity = secondary.getItem(identityKey); + } catch { + return { token, identityRaw }; + } + + if (token) { + // Primary wins. Drop the leftover so it can never be promoted later. + if (secondaryToken || secondaryIdentity) { + try { + secondary.removeItem(tokenKey); + secondary.removeItem(identityKey); + } catch { + // Best-effort. + } + } + return { token, identityRaw }; + } + + if (!secondaryToken) return { token: null, identityRaw }; + + if (!promotionFailed.has(slug) && secondaryIdentity && writePair(primary, slug, secondaryIdentity, secondaryToken)) { + try { + secondary.removeItem(tokenKey); + secondary.removeItem(identityKey); + } catch { + // The copy stays behind; the primary now wins on every later read. + } + } else if (secondaryIdentity) { + promotionFailed.add(slug); + } + return { token: secondaryToken, identityRaw: secondaryIdentity }; +} + export function storeGuestIdentity(slug: string, identity: GuestIdentity, token: string): void { const storage = getStorage(); if (!storage || !slug) return; - const write = (target: Storage): boolean => { - try { - target.setItem(`${TOKEN_KEY_PREFIX}${slug}`, token); - target.setItem(`${IDENTITY_KEY_PREFIX}${slug}`, JSON.stringify(identity)); - return true; - } catch { - return false; - } - }; + const identityRaw = JSON.stringify(identity); - if (write(storage)) return; + if (writePair(storage, slug, identityRaw, token)) { + // A fresh registration supersedes anything the secondary store holds. + const secondary = secondaryStore(storage); + if (secondary) { + try { + secondary.removeItem(`${TOKEN_KEY_PREFIX}${slug}`); + secondary.removeItem(`${IDENTITY_KEY_PREFIX}${slug}`); + } catch { + // Best-effort. + } + } + return; + } // The probe in getStorage() only proves a one-byte write fits; a JWT plus a // profile is far larger, so a nearly-full store can pass the probe and still // reject the real write. Retry in sessionStorage rather than leaving the // context believing it is signed in with nothing persisted — that state // 401s immediately and re-registers (another duplicate row) on reload. - if (isBrowser) { - try { - if (window.sessionStorage && window.sessionStorage !== storage) { - if (write(window.sessionStorage)) { - // Repoint reads at the store that actually accepted the write. - // Without this the identity is written to sessionStorage while - // getGuestToken() keeps reading localStorage, so x-guest-token is - // never sent and the identity is lost on reload — the fallback - // would look like it worked while achieving nothing. - resolvedStorage = window.sessionStorage; - } - } - } catch { - // No store will take it. The identity lasts as long as this page does, - // which is strictly better than rejecting a registration the server has - // already completed. - } + // readPair() looks there on every read, including after a reload, so no + // module state has to remember which store took the write. + const secondary = secondaryStore(storage); + if (secondary) { + promotionFailed.add(slug); + writePair(secondary, slug, identityRaw, token); } + // Otherwise no store will take it. The identity lasts as long as this page + // does, which is strictly better than rejecting a registration the server + // has already completed. } export function getGuestToken(slug?: string | null): string | null { - const storage = getStorage(); - if (!storage) return null; const resolvedSlug = slug || extractSlugFromLocation(); if (!resolvedSlug) return null; - migrateFromSessionStorage(resolvedSlug); - const token = storage.getItem(`${TOKEN_KEY_PREFIX}${resolvedSlug}`); + const { token } = readPair(resolvedSlug); if (token && isExpired(token)) { clearGuestIdentity(resolvedSlug); return null; @@ -204,18 +285,18 @@ export function getGuestToken(slug?: string | null): string | null { } export function getGuestIdentity(slug?: string | null): GuestIdentity | null { - const storage = getStorage(); - if (!storage) return null; const resolvedSlug = slug || extractSlugFromLocation(); if (!resolvedSlug) return null; - migrateFromSessionStorage(resolvedSlug); - // An identity whose token has expired must not be presented as signed in — - // clearGuestIdentity() has already run inside getGuestToken() in that case. - if (!getGuestToken(resolvedSlug)) return null; - const raw = storage.getItem(`${IDENTITY_KEY_PREFIX}${resolvedSlug}`); - if (!raw) return null; + const { token, identityRaw } = readPair(resolvedSlug); + // An identity whose token has expired must not be presented as signed in. + if (!token) return null; + if (isExpired(token)) { + clearGuestIdentity(resolvedSlug); + return null; + } + if (!identityRaw) return null; try { - return JSON.parse(raw) as GuestIdentity; + return JSON.parse(identityRaw) as GuestIdentity; } catch { return null; } @@ -223,6 +304,7 @@ export function getGuestIdentity(slug?: string | null): GuestIdentity | null { export function clearGuestIdentity(slug: string): void { if (!slug) return; + promotionFailed.delete(slug); // Clear BOTH stores: a copy left behind in sessionStorage would be migrated // straight back on the next read, silently undoing "forget me". const stores: Storage[] = []; From f2f40893c15d768438212f75219e245f1bc2e0d3 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 2 Sep 2026 09:42:40 +0200 Subject: [PATCH 7/8] fix(guests): drop a stored identity when a spent invite names someone else A guest coming back through their own already-redeemed link is the ordinary #1265 case, and the identity the device holds is theirs. The same link opened on a shared device that holds another guest's identity is not: the redemption 409s, ensureIdentity() falls through to the stored identity, and the visitor's likes are filed under the previous person. The two cases were indistinguishable client-side, so the 409/410 body now carries the invite's guest_id. On a mismatch the stored identity is cleared and the visitor is asked who they are. A response without guest_id keeps the previous behaviour. --- backend/src/routes/galleryGuests.js | 12 +++++++--- .../src/contexts/GuestIdentityContext.tsx | 22 ++++++++++++++++++- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/backend/src/routes/galleryGuests.js b/backend/src/routes/galleryGuests.js index 567d281c..49feff21 100644 --- a/backend/src/routes/galleryGuests.js +++ b/backend/src/routes/galleryGuests.js @@ -350,8 +350,12 @@ router.post('/:slug/guest/redeem', verifyGalleryAccess, async (req, res) => { .where({ token: inviteToken, event_id: event.id }) .first(); if (!invite) return { error: 'not_found' }; - if (invite.revoked_at) return { error: 'revoked' }; - if (invite.redeemed_at) return { error: 'already_redeemed' }; + // Spent and revoked invites name their guest, so the client can tell + // "this guest came back through their own link" (keep the identity the + // device holds) from "someone else's link on a device that holds + // another guest's identity" (drop it) — see GuestIdentityContext. + if (invite.revoked_at) return { error: 'revoked', guestId: invite.guest_id }; + if (invite.redeemed_at) return { error: 'already_redeemed', guestId: invite.guest_id }; const guest = await trx('gallery_guests') .where({ id: invite.guest_id, is_deleted: false }) @@ -380,7 +384,9 @@ router.post('/:slug/guest/redeem', verifyGalleryAccess, async (req, res) => { already_redeemed: 409, guest_missing: 404, }; - return res.status(statusMap[result.error] || 400).json({ error: result.error }); + const body = { error: result.error }; + if (result.guestId != null) body.guest_id = Number(result.guestId); + return res.status(statusMap[result.error] || 400).json(body); } const token = signGuestToken({ diff --git a/frontend/src/contexts/GuestIdentityContext.tsx b/frontend/src/contexts/GuestIdentityContext.tsx index 2f841d71..20760f60 100644 --- a/frontend/src/contexts/GuestIdentityContext.tsx +++ b/frontend/src/contexts/GuestIdentityContext.tsx @@ -187,7 +187,27 @@ export const GuestIdentityProvider: React.FC = ({ const newUrl = window.location.pathname + (newSearch ? `?${newSearch}` : '') + window.location.hash; window.history.replaceState({}, '', newUrl); } catch (error) { - // Silently fail invalid invites; user will fall back to normal prompt. + // A spent (409) or revoked (410) invite is the normal way a guest comes + // back through their own emailed link, and the identity this device + // holds is then theirs — keep it. But the same link opened on a shared + // device that holds SOMEONE ELSE's identity must not quietly act as + // that someone: the server names the invite's guest on those two + // responses precisely so the two cases can be told apart. On a + // mismatch the stored identity is dropped, so the visitor is asked + // who they are instead of having their likes filed under the previous + // person. A response without guest_id (older backend) keeps today's + // behaviour. + const status = (error as { response?: { status?: number; data?: { guest_id?: unknown } } }) + .response; + const invitedGuestId = status?.data?.guest_id; + if ((status?.status === 409 || status?.status === 410) && typeof invitedGuestId === 'number') { + const stored = getGuestIdentity(slug); + if (stored && stored.id !== invitedGuestId) { + clearGuestIdentity(slug); + setIdentity(null); + } + } + // Otherwise fail silently; the visitor falls back to the normal prompt. // eslint-disable-next-line no-console console.warn('Failed to redeem invite token', error); } finally { From 28f14955e2bdeb8bfed1ca3b2c0111ce07e540c2 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 2 Sep 2026 09:42:40 +0200 Subject: [PATCH 8/8] fix(gallery): clear the guest identity on gallery logout The gallery password is one shared secret per event and does not distinguish people. With the guest identity outliving the tab, logging out and letting the next person enter that password greeted them by the previous guest's name, with "forget me" - which erases that guest's selections server-side - one click away. Logout is the leaving-this-device signal, so it now drops the local identity too. Server row untouched. --- frontend/src/contexts/GalleryAuthContext.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frontend/src/contexts/GalleryAuthContext.tsx b/frontend/src/contexts/GalleryAuthContext.tsx index e6d35931..5512d10c 100644 --- a/frontend/src/contexts/GalleryAuthContext.tsx +++ b/frontend/src/contexts/GalleryAuthContext.tsx @@ -11,6 +11,7 @@ import { setActiveGallerySlug, storeGalleryToken, } from '../utils/galleryAuthStorage'; +import { clearGuestIdentity } from '../utils/guestIdentityStorage'; import type { GalleryAccessLevel } from '../types'; interface GalleryEvent { @@ -350,6 +351,12 @@ export const GalleryAuthProvider: React.FC = ({ childr sessionStorage.removeItem(`gallery_event_${currentSlug}`); sessionStorage.removeItem(`gallery_access_level_${currentSlug}`); clearGalleryToken(currentSlug); + // Logout is the "I am leaving this device" signal. Now that the guest + // identity outlives the tab (#1265), leaving it behind would greet the + // next person who enters the shared gallery password by this guest's + // name — with "forget me", which erases THIS guest's selections + // server-side, one click away. Local only; the server row is untouched. + clearGuestIdentity(currentSlug); } authService.galleryLogout(currentSlug || undefined); setIsAuthenticated(false);