From a21c4d3bf5904ddab96c90ca2d0590076897a21c Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Tue, 1 Sep 2026 22:40:23 +0200 Subject: [PATCH] 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. + } + } } /**