From 2afe3c47ce2fb438b8ba0377d1a51f4e0fcec42d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 10 May 2026 00:27:16 +0200 Subject: [PATCH] fix(onboarding): scope tour-launcher sessionStorage keys by user id (v1.4.15 H4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both `healthlog-tour-session-dismissed` and `healthlog-tour-referrer` were un-scoped global keys, so a browser shared by two HealthLog identities (admin impersonating a target user, family laptop, etc.) inherited the first user's "tour dismissed for this session" flag — the second user logged in and silently never saw the spotlight tour. Both keys are now suffixed with `:${userId}`, so each identity's state stays independent. The two builders (`tourSessionDismissedKey` and `tourReferrerKey`) are exported so the wizard's `persistAndExit` shares the same wire format and a unit test pins the scheme against future drift. One-off cost: every currently-open tab re-fires the tour once on the next load because the legacy un-prefixed keys are no longer read. Acceptable tax for multi-tenant correctness ahead of v1.5. --- src/app/onboarding/page.tsx | 20 ++++- .../__tests__/tour-launcher-keys.test.ts | 45 +++++++++++ src/components/onboarding/tour-launcher.tsx | 78 ++++++++++++------- 3 files changed, 109 insertions(+), 34 deletions(-) create mode 100644 src/components/onboarding/__tests__/tour-launcher-keys.test.ts diff --git a/src/app/onboarding/page.tsx b/src/app/onboarding/page.tsx index 85a57ce20..266df7455 100644 --- a/src/app/onboarding/page.tsx +++ b/src/app/onboarding/page.tsx @@ -19,6 +19,8 @@ import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Logo } from "@/components/ui/logo"; import { MeasurementForm } from "@/components/measurements/measurement-form"; +import { tourReferrerKey } from "@/components/onboarding/tour-launcher"; +import { useAuth } from "@/hooks/use-auth"; import { useTranslations } from "@/lib/i18n/context"; import { locales, localeLabels, type Locale } from "@/lib/i18n/config"; import { cn } from "@/lib/utils"; @@ -74,6 +76,7 @@ export default function OnboardingPage() { const router = useRouter(); const queryClient = useQueryClient(); const { t, locale, setLocale } = useTranslations(); + const { user } = useAuth(); const [step, setStep] = useState<1 | 2 | 3>(1); const [saving, setSaving] = useState(false); @@ -129,10 +132,19 @@ export default function OnboardingPage() { // render-phase decision can stay pure under // `react-hooks/purity` — the launcher consumes-and-clears the // key on first read. - try { - window.sessionStorage.setItem("healthlog-tour-referrer", "1"); - } catch { - /* ignore — storage disabled or full */ + // + // v1.4.15 H4: key is scoped by user id so a shared browser + // (admin impersonation, family laptop) keeps each user's + // wizard-referrer state independent. Skip the write entirely + // if the auth payload hasn't loaded — the worst-case is the + // tour pops up immediately instead of after the 1.5s grace + // window, which is harmless. + if (user?.id) { + try { + window.sessionStorage.setItem(tourReferrerKey(user.id), "1"); + } catch { + /* ignore — storage disabled or full */ + } } if (opts?.gotoSettingsHash) { diff --git a/src/components/onboarding/__tests__/tour-launcher-keys.test.ts b/src/components/onboarding/__tests__/tour-launcher-keys.test.ts new file mode 100644 index 000000000..c20f1e394 --- /dev/null +++ b/src/components/onboarding/__tests__/tour-launcher-keys.test.ts @@ -0,0 +1,45 @@ +/** + * v1.4.15 H4 — sessionStorage keys used by the dashboard tour + * launcher must be scoped per user id. Without scoping, an admin + * impersonating a second user (or a family member sharing the + * browser) inherited the first user's "tour dismissed for this + * session" flag and the tour silently no-op'd for the second user. + * + * The two key-builders are exported so this lock-test pins the wire + * format. Changing them is a soft compatibility break — every + * already-rendered tab would re-fire the tour for the user once, + * because the un-prefixed legacy keys are never read again. We + * accept that one-off tax in exchange for the multi-tenant + * correctness. + */ +import { describe, expect, it } from "vitest"; +import { + tourReferrerKey, + tourSessionDismissedKey, +} from "../tour-launcher"; + +describe("tour-launcher sessionStorage key scoping", () => { + it("scopes the dismiss key by user id", () => { + expect(tourSessionDismissedKey("u_abc")).toBe( + "healthlog-tour-session-dismissed:u_abc", + ); + }); + + it("scopes the referrer key by user id", () => { + expect(tourReferrerKey("u_abc")).toBe("healthlog-tour-referrer:u_abc"); + }); + + it("returns distinct keys for distinct users", () => { + expect(tourSessionDismissedKey("u_alice")).not.toBe( + tourSessionDismissedKey("u_bob"), + ); + expect(tourReferrerKey("u_alice")).not.toBe(tourReferrerKey("u_bob")); + }); + + it("does not collide with the legacy un-scoped keys", () => { + expect(tourSessionDismissedKey("u_x")).not.toBe( + "healthlog-tour-session-dismissed", + ); + expect(tourReferrerKey("u_x")).not.toBe("healthlog-tour-referrer"); + }); +}); diff --git a/src/components/onboarding/tour-launcher.tsx b/src/components/onboarding/tour-launcher.tsx index b0a96f748..2d2932775 100644 --- a/src/components/onboarding/tour-launcher.tsx +++ b/src/components/onboarding/tour-launcher.tsx @@ -43,9 +43,26 @@ import { useQueryClient } from "@tanstack/react-query"; import { useAuth } from "@/hooks/use-auth"; import { OnboardingTour } from "./tour"; -const SESSION_DISMISS_KEY = "healthlog-tour-session-dismissed"; const POST_WIZARD_GRACE_MS = 1500; -const REFERRER_KEY = "healthlog-tour-referrer"; + +// v1.4.15 H4 — sessionStorage keys are now scoped by user id so an +// admin impersonating a second user does not inherit the first user's +// "tour dismissed for this session" state. The previous global keys +// (`healthlog-tour-session-dismissed`, `healthlog-tour-referrer`) +// kept v1.4.16's multi-tenant prep clean too: anyone using the same +// browser to switch accounts via the typical user-menu sign-out flow +// will not see the tour suppressed under their second identity. +// +// Exported for the unit-test suite so the key-construction stays +// pinned across refactors (and so the keys themselves stay stable — +// changing them would re-fire the tour for everyone). +export function tourSessionDismissedKey(userId: string): string { + return `healthlog-tour-session-dismissed:${userId}`; +} + +export function tourReferrerKey(userId: string): string { + return `healthlog-tour-referrer:${userId}`; +} /** * Whether the user just navigated here from `/onboarding`. Read @@ -56,41 +73,48 @@ const REFERRER_KEY = "healthlog-tour-referrer"; * before navigating, and the launcher consumes-and-clears it on * first mount so a back-button to the wizard and forward again * doesn't double-defer. + * + * `userId` is required — until the auth payload arrives the launcher + * does not consult sessionStorage at all (the gating effect waits + * for `inputsReady` first). */ -function readAndClearJustFromWizard(): boolean { +function readAndClearJustFromWizard(userId: string): boolean { if (typeof window === "undefined") return false; try { - const value = window.sessionStorage.getItem(REFERRER_KEY); + const key = tourReferrerKey(userId); + const value = window.sessionStorage.getItem(key); if (value !== "1") return false; - window.sessionStorage.removeItem(REFERRER_KEY); + window.sessionStorage.removeItem(key); return true; } catch { return false; } } -function readSessionDismissed(): boolean { +function readSessionDismissed(userId: string): boolean { if (typeof window === "undefined") return false; try { - return window.sessionStorage.getItem(SESSION_DISMISS_KEY) === "1"; + return ( + window.sessionStorage.getItem(tourSessionDismissedKey(userId)) === "1" + ); } catch { return false; } } -function writeSessionDismissed() { +function writeSessionDismissed(userId: string) { if (typeof window === "undefined") return; try { - window.sessionStorage.setItem(SESSION_DISMISS_KEY, "1"); + window.sessionStorage.setItem(tourSessionDismissedKey(userId), "1"); } catch { /* ignore — sandboxed iframes etc. */ } } -function clearSessionDismissed() { +function clearSessionDismissed(userId: string) { if (typeof window === "undefined") return; try { - window.sessionStorage.removeItem(SESSION_DISMISS_KEY); + window.sessionStorage.removeItem(tourSessionDismissedKey(userId)); } catch { /* ignore */ } @@ -116,18 +140,6 @@ export function TourLauncher({ ready }: TourLauncherProps) { // closing the tour transitions true→false and we never re-open // unless the restart event fires. const [showTour, setShowTour] = useState(null); - // Lazy initial check for "user just exited the wizard". The - // initialiser runs once at mount — sessionStorage is pure-enough - // there because React only invokes the initialiser one time per - // component instance, and in dev StrictMode's double-call merely - // returns the same `true` (the second call sees the cleared key - // and returns `false`, but the state has already been seeded). We - // accept that tiny behavioural quirk because it only affects dev - // mode and the consequence is "tour doesn't auto-launch on first - // dev mount" — never surfaces in production. - const [justFromWizard] = useState(() => - readAndClearJustFromWizard(), - ); // Track post-wizard deferral as a boolean so the render path stays // pure (no Date.now() comparisons). The effect below reads / clears it. const [deferredFromWizard, setDeferredFromWizard] = useState(false); @@ -154,11 +166,14 @@ export function TourLauncher({ ready }: TourLauncherProps) { decidedFor.flag !== user.onboardingTourCompleted) ) { setDecidedFor({ userId: user.id, flag: user.onboardingTourCompleted }); + // sessionStorage reads happen via per-user keyed helpers, so a + // browser shared by two HealthLog users (admin impersonation, + // family laptop, etc.) keeps each user's tour state independent. if (user.onboardingTourCompleted) { setShowTour(false); - } else if (readSessionDismissed()) { + } else if (readSessionDismissed(user.id)) { setShowTour(false); - } else if (justFromWizard) { + } else if (readAndClearJustFromWizard(user.id)) { // Mark "deferred"; the effect below schedules a fixed-duration // timer + flips `showTour` from inside the timeout callback // (which the set-state-in-effect rule allows). @@ -174,28 +189,31 @@ export function TourLauncher({ ready }: TourLauncherProps) { // set-state-in-effect rule is satisfied. useEffect(() => { if (!deferredFromWizard) return; + const userId = user?.id; + if (!userId) return; const id = window.setTimeout(() => { - if (!readSessionDismissed()) { + if (!readSessionDismissed(userId)) { setShowTour(true); } setDeferredFromWizard(false); }, POST_WIZARD_GRACE_MS); return () => window.clearTimeout(id); - }, [deferredFromWizard]); + }, [deferredFromWizard, user?.id]); // Listen for explicit restart events from Settings → Account. // setState inside a window-event listener is fine under the // set-state-in-effect rule. + const userIdForRestart = user?.id; useEffect(() => { function onRestart() { - clearSessionDismissed(); + if (userIdForRestart) clearSessionDismissed(userIdForRestart); setShowTour(true); } window.addEventListener("healthlog:tour-restart", onRestart); return () => { window.removeEventListener("healthlog:tour-restart", onRestart); }; - }, []); + }, [userIdForRestart]); if (showTour !== true) return null; @@ -210,7 +228,7 @@ export function TourLauncher({ ready }: TourLauncherProps) { // `/api/auth/me` (which the user gets on any nav) will // surface any divergence. setShowTour(false); - writeSessionDismissed(); + if (user?.id) writeSessionDismissed(user.id); try { await fetch("/api/onboarding/tour", { method: "POST",