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/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/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/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); diff --git a/frontend/src/contexts/GuestIdentityContext.tsx b/frontend/src/contexts/GuestIdentityContext.tsx index 132908d7..20760f60 100644 --- a/frontend/src/contexts/GuestIdentityContext.tsx +++ b/frontend/src/contexts/GuestIdentityContext.tsx @@ -1,6 +1,8 @@ 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 { + GUEST_IDENTITY_CLEARED_EVENT, clearGuestIdentity, getGuestIdentity, storeGuestIdentity, @@ -23,6 +25,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 @@ -44,6 +56,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); @@ -57,15 +70,113 @@ 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 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; + } + 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); + 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 + // 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); + // 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; + 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 // 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); + // 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' || 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 () => { + invitePromiseRef.current = (async () => { try { const response = await guestsService.redeemInvite(slug, inviteToken); storeGuestIdentity(slug, response.guest, response.token); @@ -76,12 +187,34 @@ 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 { + invitePromiseRef.current = null; } })(); - }, [slug, identityMode, identity]); + }, [slug, identityMode]); const openPrompt = useCallback(() => setPromptOpen(true), []); const closePrompt = useCallback(() => { @@ -144,7 +277,12 @@ export const GuestIdentityProvider: React.FC = ({ setIdentity(null); }, [slug, identity]); - const ensureIdentity = useCallback((): Promise => { + const signOut = useCallback((): void => { + clearGuestIdentity(slug); + setIdentity(null); + }, [slug]); + + 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. @@ -155,14 +293,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; @@ -182,6 +332,7 @@ export const GuestIdentityProvider: React.FC = ({ recoverRequest, recoverVerify, forget, + signOut, ensureIdentity, }), [ @@ -199,11 +350,18 @@ export const GuestIdentityProvider: React.FC = ({ recoverRequest, recoverVerify, forget, + signOut, ensureIdentity, ] ); - 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/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..c3f1bc0a --- /dev/null +++ b/frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts @@ -0,0 +1,294 @@ +/** + * 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 { + __resetStorageResolutionForTests, + clearGuestIdentity, + getGuestIdentity, + getGuestToken, + storeGuestIdentity, +} from '../guestIdentityStorage'; + +const SLUG = 'wedding-summer-2026'; +const IDENTITY = { id: 42, name: 'Tina', email: 'tina@example.com', identifier: 'abc-123' }; + +/** 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 { + window.sessionStorage.clear(); +} + +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', () => { + 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); + }); + }); + + 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('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(); + }); + + 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. + 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..876e6286 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'; @@ -12,51 +28,317 @@ 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'; +/** + * 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; + if (resolvedStorage !== undefined) return resolvedStorage; + + for (const pick of [() => window.localStorage, () => window.sessionStorage]) { + let candidate: Storage; + try { + candidate = pick(); + } catch { + 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; + promotionFailed.clear(); +} + +/** + * 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 { - return window.sessionStorage; + 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; + } +} + +/** + * 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 secondaryStore(primary: Storage): Storage | null { + if (!isBrowser) return null; + try { + const candidate = window.sessionStorage; + return candidate && candidate !== primary ? candidate : null; } catch { 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 { + target.setItem(identityKey, identityRaw); + target.setItem(tokenKey, token); + return true; + } catch { + 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; - storage.setItem(`${TOKEN_KEY_PREFIX}${slug}`, token); - storage.setItem(`${IDENTITY_KEY_PREFIX}${slug}`, JSON.stringify(identity)); + const identityRaw = JSON.stringify(identity); + + 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. + // 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; - return storage.getItem(`${TOKEN_KEY_PREFIX}${resolvedSlug}`); + const { token } = readPair(resolvedSlug); + if (token && isExpired(token)) { + clearGuestIdentity(resolvedSlug); + return null; + } + return token; } export function getGuestIdentity(slug?: string | null): GuestIdentity | null { - const storage = getStorage(); - if (!storage) return null; const resolvedSlug = slug || extractSlugFromLocation(); if (!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; } } 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; + 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[] = []; + 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. + } + } + + // `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. + } + } } /**