paul/picpeak
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(guests): invite wins over stored identity; clear server-rejected ones

Browse Source 
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.
This commit is contained in:
Paul Nothaft
2026-09-02 08:07:41 +02:00
parent 51db1e09e9
commit f3f37a8c77
4 changed files with 98 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/config/api.ts
+11 -1
View File
@@ -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) {
frontend/src/contexts/GuestIdentityContext.tsx
+33 -3
View File
@@ -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<GuestIdentityProviderProps> = ({
identityMode,
children,
}) => {
const queryClient = useQueryClient();
const [identity, setIdentity] = useState<GuestIdentity | null>(() => getGuestIdentity(slug));
const [promptOpen, setPromptOpen] = useState(false);
const [recoveryOpen, setRecoveryOpen] = useState(false);
@@ -88,13 +90,41 @@ export const GuestIdentityProvider: React.FC<GuestIdentityProviderProps> = ({
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<number | null>(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<string | 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 () => {
try {
@@ -112,7 +142,7 @@ export const GuestIdentityProvider: React.FC<GuestIdentityProviderProps> = ({
console.warn('Failed to redeem invite token', error);
}
})();
}, [slug, identityMode, identity]);
}, [slug, identityMode]);
const openPrompt = useCallback(() => setPromptOpen(true), []);
const closePrompt = useCallback(() => {
frontend/src/utils/__tests__/guestIdentityStorage.persistence.test.ts
+27
View File
@@ -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.
frontend/src/utils/guestIdentityStorage.ts
+27 -8
View File
@@ -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.
}
}
}