diff --git a/src/app/baseball/(auth)/demo/page.tsx b/src/app/baseball/(auth)/demo/page.tsx index 215e1cb18..26d52e364 100644 --- a/src/app/baseball/(auth)/demo/page.tsx +++ b/src/app/baseball/(auth)/demo/page.tsx @@ -2,8 +2,8 @@ import { Suspense, useEffect, useMemo, useState } from 'react'; import Link from 'next/link'; -import { useRouter } from 'next/navigation'; -import { ArrowRight, Brain, ClipboardList, Users } from 'lucide-react'; +import { useRouter, useSearchParams } from 'next/navigation'; +import { ArrowRight, Brain, ClipboardList, Clock, Users } from 'lucide-react'; import { Input } from '@/components/ui/input'; import { Button } from '@/components/ui/button'; import { createClient } from '@/lib/supabase/client'; @@ -14,6 +14,7 @@ import { isBaseballDemoAvailable, isBaseballDemoSession, } from '@/app/baseball/actions/demo-access'; +import { probeSignedIn, raceWithTimeout } from '@/lib/demo/gate-probe'; import { AuthBezel, AuthCard, @@ -62,6 +63,8 @@ function validateProgram(v: string): string | undefined { function DemoGateContent() { const router = useRouter(); + const searchParams = useSearchParams(); + const sessionExpired = searchParams.get('message') === 'demo_session_expired'; // Form state const [name, setName] = useState(''); @@ -92,18 +95,28 @@ function DemoGateContent() { useEffect(() => { let active = true; async function checkAuth() { + // probeSignedIn races supabase.auth.getUser() against a hard 4s + // deadline and treats ANY failure — timeout, thrown error, or a + // refresh-token error surfaced through getUser()'s own `error` field — + // as signed out. Without this, a stale/expired httpOnly Supabase + // cookie can leave getUser()'s internal refresh hanging forever, and + // "Checking sign-in status" never resolves (#918). + const user = await probeSignedIn(supabase); + if (!active || !user) return; try { - const { data: { user } } = await supabase.auth.getUser(); - if (!user) return; // The client can't detect the shared demo account itself (its email - // is a server-only secret) — verify it server-side. - const { isDemo } = await isBaseballDemoSession(); + // is a server-only secret) — verify it server-side. Same hard + // timeout applied here: a stuck server action must not hang this + // check either. + const { isDemo } = await raceWithTimeout(isBaseballDemoSession()); if (active && isDemo) setIsDemoSession(true); - } finally { - if (active) setCheckingAuth(false); + } catch { + // Timeout or thrown error — fall through to the form below. } } - void checkAuth(); + void checkAuth().finally(() => { + if (active) setCheckingAuth(false); + }); return () => { active = false; }; }, [supabase]); @@ -111,8 +124,15 @@ function DemoGateContent() { let active = true; async function checkAvailability() { try { - const { enabled } = await isBaseballDemoAvailable(); + // Same hard-timeout guard as the auth check above — a stuck + // kill-switch read must not leave the spinner up forever. Fails + // open (demo treated as enabled) so a slow read shows the form + // instead of a permanent spinner; the real kill-switch is still + // enforced server-side on submit. + const { enabled } = await raceWithTimeout(isBaseballDemoAvailable()); if (active) setDemoEnabled(enabled); + } catch { + if (active) setDemoEnabled(true); } finally { if (active) setCheckingAvailability(false); } @@ -225,6 +245,13 @@ function DemoGateContent() {

+ {/* Friendly notice when bounced here after a demo session timed out (#918) */} + {sessionExpired && ( + + Your demo session timed out. Enter your info again to jump right back in. + + )} + {checkingAuth || checkingAvailability ? (
diff --git a/src/app/baseball/actions/__tests__/demo-access.test.ts b/src/app/baseball/actions/__tests__/demo-access.test.ts index ea5b97ffb..c9f5e4b9a 100644 --- a/src/app/baseball/actions/__tests__/demo-access.test.ts +++ b/src/app/baseball/actions/__tests__/demo-access.test.ts @@ -24,6 +24,7 @@ const mocks = vi.hoisted(() => ({ }, error: null, })), + updateUser: vi.fn(async () => ({ data: { user: null }, error: null })), adminFrom: vi.fn(), insert: vi.fn(async () => ({ data: null, error: null })), logLogin: vi.fn(async () => ({})), @@ -56,6 +57,7 @@ vi.mock('@/lib/supabase/server', () => ({ auth: { getUser: mocks.getUser, signInWithPassword: mocks.signInWithPassword, + updateUser: mocks.updateUser, }, })), })); @@ -116,6 +118,26 @@ beforeEach(() => { }, error: null, }); + mocks.updateUser.mockResolvedValue({ data: { user: null }, error: null }); +}); + +describe('enterBaseballDemo — is_demo metadata stamp (#918)', () => { + it('stamps user_metadata.is_demo = true right after sign-in, before redirecting', async () => { + await expect(enterBaseballDemo(VALID_INPUT)).rejects.toThrow('REDIRECT:/baseball/dashboard?demo=1'); + + expect(mocks.updateUser).toHaveBeenCalledWith({ data: { is_demo: true } }); + const signInOrder = mocks.signInWithPassword.mock.invocationCallOrder[0]!; + const updateOrder = mocks.updateUser.mock.invocationCallOrder[0]!; + expect(signInOrder).toBeLessThan(updateOrder); + }); + + it('never blocks demo entry when the metadata stamp fails', async () => { + mocks.updateUser.mockRejectedValue(new Error('gotrue hiccup')); + + await expect(enterBaseballDemo(VALID_INPUT)).rejects.toThrow('REDIRECT:/baseball/dashboard?demo=1'); + + expect(mocks.signInWithPassword).toHaveBeenCalledTimes(1); + }); }); describe('enterBaseballDemo — rate limit', () => { diff --git a/src/app/baseball/actions/demo-access.ts b/src/app/baseball/actions/demo-access.ts index 837cb72fc..eaa299815 100644 --- a/src/app/baseball/actions/demo-access.ts +++ b/src/app/baseball/actions/demo-access.ts @@ -173,6 +173,18 @@ async function enterBaseballDemoImpl( }; } + // --- 6b. Stamp the session as a demo session ------------------------------ + // `user_metadata.is_demo` is readable by BOTH client and server code + // (unlike the demo email, which stays a server-only secret) — the shared + // idle-timeout flow (middleware + useSessionActivity) reads it to route + // an expired demo session back to /baseball/demo instead of the password + // login (#918). Best-effort: a failure here must never block entry. + try { + await supabase.auth.updateUser({ data: { is_demo: true } }); + } catch { + // Worst case the session_expired redirect falls back to /baseball/login. + } + // --- 7. Mirror into the admin_events feed (best-effort) ------------------ // userId is the shared demo account's real uuid; userEmail is the // visitor's email so admins can identify who entered. The diff --git a/src/app/golf/(auth)/demo/page.tsx b/src/app/golf/(auth)/demo/page.tsx index 3a974396f..df8657d23 100644 --- a/src/app/golf/(auth)/demo/page.tsx +++ b/src/app/golf/(auth)/demo/page.tsx @@ -3,7 +3,7 @@ import { Suspense, useState, useEffect, useMemo } from 'react'; import Image from 'next/image'; import Link from 'next/link'; -import { useRouter } from 'next/navigation'; +import { useRouter, useSearchParams } from 'next/navigation'; import { LazyMotion, m, useReducedMotion } from 'framer-motion'; import { loadFeatures } from '@/lib/motion/load-features'; import { AlertCircle, Loader2, ArrowRight, BarChart2, Users, Brain } from 'lucide-react'; @@ -15,6 +15,7 @@ import { useMediaQuery } from '@/hooks/use-media-query'; import { createClient } from '@/lib/supabase/client'; import { DEMO_LANDING_PATH } from '@/lib/demo/config'; import { enterDemo } from '@/app/golf/actions/demo-access'; +import { probeSignedIn } from '@/lib/demo/gate-probe'; // --------------------------------------------------------------------------- // Value-prop pill items shown below the headline @@ -56,6 +57,8 @@ function validateSchool(v: string): string | undefined { function DemoGateContent() { const prefersReducedMotion = useReducedMotion(); const router = useRouter(); + const searchParams = useSearchParams(); + const sessionExpired = searchParams.get('message') === 'demo_session_expired'; const isDesktop = useMediaQuery('(min-width: 768px)'); // Form state @@ -77,20 +80,24 @@ function DemoGateContent() { const supabase = useMemo(() => createClient(), []); useEffect(() => { + let active = true; async function checkAuth() { - try { - const { data: { user } } = await supabase.auth.getUser(); - // Any already-authenticated visitor gets a "continue" shortcut. We can't - // reliably detect the shared demo account client-side (its email is a - // server-only secret), so we don't special-case it here. - if (user) { - setIsDemoUser(true); - } - } finally { - setCheckingAuth(false); - } + // probeSignedIn races supabase.auth.getUser() against a hard 4s + // deadline and treats ANY failure — timeout, thrown error, or a + // refresh-token error surfaced through getUser()'s own `error` field — + // as signed out. Without this, a stale/expired httpOnly Supabase + // cookie can leave getUser()'s internal refresh hanging forever, and + // this "Checking sign-in status" spinner never resolves (#918). + const user = await probeSignedIn(supabase); + if (!active) return; + // Any already-authenticated visitor gets a "continue" shortcut. We can't + // reliably detect the shared demo account client-side (its email is a + // server-only secret), so we don't special-case it here. + if (user) setIsDemoUser(true); + setCheckingAuth(false); } void checkAuth(); + return () => { active = false; }; }, [supabase]); // Derived inline errors (only shown after touch) @@ -273,6 +280,16 @@ function DemoGateContent() {

+ {/* Friendly notice when bounced here after a demo session timed out (#918) */} + {sessionExpired && ( +
+ Your demo session timed out. Enter your info again to jump right back in. +
+ )} + {/* Already-signed-in shortcut */} {checkingAuth ? (
diff --git a/src/app/golf/actions/__tests__/demo-access.test.ts b/src/app/golf/actions/__tests__/demo-access.test.ts new file mode 100644 index 000000000..0cd2f90e1 --- /dev/null +++ b/src/app/golf/actions/__tests__/demo-access.test.ts @@ -0,0 +1,177 @@ +// ============================================================================= +// src/app/golf/actions/__tests__/demo-access.test.ts +// +// #918 — GolfHelm demo gate hardening. Covers: +// 1. Rate-limit denial (per-IP DEMO_GATE throttle), before touching DB/auth. +// 2. Missing demo credentials short-circuit. +// 3. Lead-row capture into golf_demo_sessions before sign-in. +// 4. The is_demo user_metadata stamp set right after sign-in — the signal +// the shared idle-timeout flow (middleware + useSessionActivity) reads +// to route an expired demo session back to /golf/demo instead of the +// dead-end password login. +// ============================================================================= + +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + signInWithPassword: vi.fn(async () => ({ + data: { + session: { access_token: 'tok' }, + user: { id: 'demo-user-1', email: 'demo@golfhelmdemo.com' }, + }, + error: null, + })), + updateUser: vi.fn(async () => ({ data: { user: null }, error: null })), + getUser: vi.fn(async () => ({ data: { user: null }, error: null })), + adminFrom: vi.fn(), + insert: vi.fn(async () => ({ data: null, error: null })), + logLogin: vi.fn(async () => ({})), + checkRateLimit: vi.fn(async () => ({ allowed: true, remaining: 4, resetAt: Date.now() + 60_000 })), + formatTimeRemaining: vi.fn(() => '15 minutes'), + getDemoCoachCredentials: vi.fn( + (): { email: string; password: string } | null => ({ email: 'demo@golfhelmdemo.com', password: 'Demo2026' }), + ), + redirect: vi.fn((path: string) => { + throw new Error(`REDIRECT:${path}`); + }), +})); +mocks.adminFrom.mockImplementation(() => ({ insert: mocks.insert })); + +vi.mock('next/headers', () => ({ + headers: vi.fn(async () => new Map([ + ['x-forwarded-for', '203.0.113.7'], + ['user-agent', 'TestAgent/1.0 (TestOS)'], + ['referer', 'https://example.com/landing'], + ])), +})); + +vi.mock('next/navigation', () => ({ redirect: mocks.redirect })); + +vi.mock('@/lib/supabase/server', () => ({ + createClient: vi.fn(async () => ({ + auth: { + getUser: mocks.getUser, + signInWithPassword: mocks.signInWithPassword, + updateUser: mocks.updateUser, + }, + })), +})); + +vi.mock('@/lib/supabase/admin', () => ({ + createAdminClient: vi.fn(() => ({ from: mocks.adminFrom })), +})); + +vi.mock('@/lib/admin-logger', () => ({ logLogin: mocks.logLogin })); + +vi.mock('@/lib/auth/rate-limit', () => ({ + checkRateLimit: mocks.checkRateLimit, + RATE_LIMITS: { DEMO_GATE: { maxAttempts: 5, windowMs: 5 * 60_000, blockDurationMs: 15 * 60_000 } }, + formatTimeRemaining: mocks.formatTimeRemaining, +})); + +vi.mock('@/lib/demo/config', () => ({ + DEMO_LANDING_PATH: '/golf/dashboard', +})); + +vi.mock('@/lib/demo/config.server', () => ({ + getDemoCoachCredentials: mocks.getDemoCoachCredentials, + isDemoCoachEmail: vi.fn(() => false), +})); + +import { enterDemo } from '@/app/golf/actions/demo-access'; + +const VALID_INPUT = { + name: 'Coach Rivera', + email: 'coach@example.edu', + school: 'Example University Golf', +}; + +beforeEach(() => { + vi.clearAllMocks(); + mocks.adminFrom.mockImplementation(() => ({ insert: mocks.insert })); + mocks.insert.mockResolvedValue({ data: null, error: null }); + mocks.checkRateLimit.mockResolvedValue({ allowed: true, remaining: 4, resetAt: Date.now() + 60_000 }); + mocks.getDemoCoachCredentials.mockReturnValue({ email: 'demo@golfhelmdemo.com', password: 'Demo2026' }); + mocks.signInWithPassword.mockResolvedValue({ + data: { + session: { access_token: 'tok' }, + user: { id: 'demo-user-1', email: 'demo@golfhelmdemo.com' }, + }, + error: null, + }); + mocks.updateUser.mockResolvedValue({ data: { user: null }, error: null }); +}); + +describe('enterDemo — rate limit', () => { + it('denies entry once the per-IP DEMO_GATE limit is exceeded, before touching the DB or auth', async () => { + mocks.checkRateLimit.mockResolvedValue({ allowed: false, remaining: 0, resetAt: Date.now() + 900_000 }); + + const result = await enterDemo(VALID_INPUT); + + expect(result.success).toBe(false); + if (!result.success) expect(result.error).toMatch(/too many demo attempts/i); + expect(mocks.insert).not.toHaveBeenCalled(); + expect(mocks.signInWithPassword).not.toHaveBeenCalled(); + }); +}); + +describe('enterDemo — missing credentials', () => { + it('short-circuits with a graceful message when demo credentials are unconfigured', async () => { + mocks.getDemoCoachCredentials.mockReturnValue(null); + + const result = await enterDemo(VALID_INPUT); + + expect(result.success).toBe(false); + if (!result.success) expect(result.error).toMatch(/not available/i); + expect(mocks.signInWithPassword).not.toHaveBeenCalled(); + }); +}); + +describe('enterDemo — lead-row capture', () => { + it('inserts a golf_demo_sessions row with visitor + request metadata before signing in', async () => { + await expect(enterDemo(VALID_INPUT)).rejects.toThrow('REDIRECT:/golf/dashboard?demo=1'); + + expect(mocks.adminFrom).toHaveBeenCalledWith('golf_demo_sessions'); + expect(mocks.insert).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'Coach Rivera', + email: 'coach@example.edu', + school: 'Example University Golf', + ip: '203.0.113.7', + user_agent: 'TestAgent/1.0 (TestOS)', + referrer: 'https://example.com/landing', + metadata: {}, + }), + ); + const insertOrder = mocks.insert.mock.invocationCallOrder[0]!; + const signInOrder = mocks.signInWithPassword.mock.invocationCallOrder[0]!; + expect(insertOrder).toBeLessThan(signInOrder); + }); + + it('never blocks demo entry when the tracking insert fails', async () => { + mocks.insert.mockRejectedValue(new Error('relation does not exist')); + + await expect(enterDemo(VALID_INPUT)).rejects.toThrow('REDIRECT:/golf/dashboard?demo=1'); + + expect(mocks.signInWithPassword).toHaveBeenCalledTimes(1); + }); +}); + +describe('enterDemo — is_demo metadata stamp (#918)', () => { + it('stamps user_metadata.is_demo = true right after sign-in, before redirecting', async () => { + await expect(enterDemo(VALID_INPUT)).rejects.toThrow('REDIRECT:/golf/dashboard?demo=1'); + + expect(mocks.updateUser).toHaveBeenCalledWith({ data: { is_demo: true } }); + const signInOrder = mocks.signInWithPassword.mock.invocationCallOrder[0]!; + const updateOrder = mocks.updateUser.mock.invocationCallOrder[0]!; + expect(signInOrder).toBeLessThan(updateOrder); + }); + + it('never blocks demo entry when the metadata stamp fails', async () => { + mocks.updateUser.mockRejectedValue(new Error('gotrue hiccup')); + + await expect(enterDemo(VALID_INPUT)).rejects.toThrow('REDIRECT:/golf/dashboard?demo=1'); + + expect(mocks.signInWithPassword).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/app/golf/actions/demo-access.ts b/src/app/golf/actions/demo-access.ts index 71105e2c6..6d8c3d21a 100644 --- a/src/app/golf/actions/demo-access.ts +++ b/src/app/golf/actions/demo-access.ts @@ -157,6 +157,18 @@ async function enterDemoImpl(input: EnterDemoInput): Promise { }; } + // --- 5b. Stamp the session as a demo session ------------------------------ + // `user_metadata.is_demo` is readable by BOTH client and server code + // (unlike the demo email, which stays a server-only secret) — the shared + // idle-timeout flow (middleware + useSessionActivity) reads it to route + // an expired demo session back to /golf/demo instead of the password + // login (#918). Best-effort: a failure here must never block entry. + try { + await supabase.auth.updateUser({ data: { is_demo: true } }); + } catch { + // Worst case the session_expired redirect falls back to /golf/login. + } + // --- 6. Mirror into the admin_events feed -------------------------------- // userId is the shared demo account's real uuid; userEmail is the // visitor's email so admins can identify them. The golf_demo_sessions diff --git a/src/lib/auth/__tests__/session-activity.test.ts b/src/lib/auth/__tests__/session-activity.test.ts new file mode 100644 index 000000000..2253b4bee --- /dev/null +++ b/src/lib/auth/__tests__/session-activity.test.ts @@ -0,0 +1,159 @@ +// ============================================================================= +// src/lib/auth/__tests__/session-activity.test.ts +// +// #918 — useSessionActivity's idle-timeout logout must: +// 1. Route a demo-context session (user_metadata.is_demo) back to the +// sport's /demo gate with a friendly message, not the dead-end +// password /login. +// 2. Leave a normal (non-demo) idle-timeout and the /admin idle-timeout +// path completely unchanged. +// 3. Never hang: the demo-detection probe (getUser()) is hard-timeout +// guarded, so a stuck refresh-token exchange still lets the logout +// redirect complete instead of leaving the tab stuck mid-signout. +// ============================================================================= + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { renderHook, waitFor, act } from '@testing-library/react'; +import { SESSION_IDLE_COOKIE } from '@/lib/auth/session-idle-shared'; + +const replaceMock = vi.fn(); +vi.mock('next/navigation', () => ({ + useRouter: () => ({ replace: replaceMock }), +})); + +interface FakeUser { + id: string; + user_metadata: Record; +} + +const mocks = vi.hoisted(() => ({ + getUser: vi.fn(async (): Promise<{ data: { user: FakeUser | null }; error: unknown }> => ({ + data: { user: null }, + error: null, + })), + signOut: vi.fn(async () => ({ error: null })), +})); + +vi.mock('@/lib/supabase/client', () => ({ + createClient: vi.fn(() => ({ + auth: { + getUser: mocks.getUser, + signOut: mocks.signOut, + }, + })), +})); + +import { useSessionActivity } from '@/lib/auth/session-activity'; + +/** A `sb_last_activity` value 10 minutes in the past — past the 5-min idle window. */ +const STALE = Date.now() - 10 * 60 * 1000; + +function setPath(pathname: string) { + window.history.pushState({}, '', pathname); +} + +function setStaleActivityCookie() { + document.cookie = `${SESSION_IDLE_COOKIE}=${STALE}; path=/`; +} + +function clearCookies() { + document.cookie = `${SESSION_IDLE_COOKIE}=; path=/; max-age=0`; +} + +describe('useSessionActivity — idle-timeout logout demo routing (#918)', () => { + beforeEach(() => { + vi.clearAllMocks(); + clearCookies(); + mocks.getUser.mockResolvedValue({ data: { user: null }, error: null }); + mocks.signOut.mockResolvedValue({ error: null }); + }); + + afterEach(() => { + clearCookies(); + }); + + it('routes an idle-expired golf demo session to /golf/demo?message=demo_session_expired', async () => { + setPath('/golf/dashboard'); + setStaleActivityCookie(); + mocks.getUser.mockResolvedValue({ + data: { user: { id: 'demo-1', user_metadata: { is_demo: true } } }, + error: null, + }); + + renderHook(() => useSessionActivity()); + + await waitFor(() => expect(replaceMock).toHaveBeenCalled()); + + expect(mocks.signOut).toHaveBeenCalled(); + expect(replaceMock).toHaveBeenCalledWith('/golf/demo?message=demo_session_expired'); + }); + + it('routes an idle-expired baseball demo session to /baseball/demo?message=demo_session_expired', async () => { + setPath('/baseball/dashboard/command-center'); + setStaleActivityCookie(); + mocks.getUser.mockResolvedValue({ + data: { user: { id: 'demo-2', user_metadata: { is_demo: true } } }, + error: null, + }); + + renderHook(() => useSessionActivity()); + + await waitFor(() => expect(replaceMock).toHaveBeenCalled()); + expect(replaceMock).toHaveBeenCalledWith('/baseball/demo?message=demo_session_expired'); + }); + + it('leaves a NON-demo idle-timeout on the normal /login redirect unchanged', async () => { + setPath('/golf/dashboard'); + setStaleActivityCookie(); + mocks.getUser.mockResolvedValue({ + data: { user: { id: 'real-coach-1', user_metadata: {} } }, + error: null, + }); + + renderHook(() => useSessionActivity()); + + await waitFor(() => expect(replaceMock).toHaveBeenCalled()); + expect(replaceMock).toHaveBeenCalledWith('/golf/login?message=session_expired'); + }); + + it('leaves the /admin idle-timeout path unchanged even for a demo-flagged user', async () => { + setPath('/admin/errors'); + setStaleActivityCookie(); + // /admin has no sport prefix, so the demo probe never even applies here — + // this proves the admin branch still wins regardless. + mocks.getUser.mockResolvedValue({ + data: { user: { id: 'demo-1', user_metadata: { is_demo: true } } }, + error: null, + }); + + renderHook(() => useSessionActivity()); + + await waitFor(() => expect(replaceMock).toHaveBeenCalled()); + const [url] = replaceMock.mock.calls[0] as [string]; + expect(url.startsWith('/golf/login?')).toBe(true); + expect(url).toContain('message=session_expired'); + expect(url).toContain('returnTo=%2Fadmin%2Ferrors'); + }); + + it('never hangs the logout: a getUser() that hangs forever still completes the redirect', async () => { + vi.useFakeTimers(); + try { + setPath('/golf/dashboard'); + setStaleActivityCookie(); + // Simulates the exact failure mode this fix targets — a stuck + // refresh-token exchange whose promise never settles. + mocks.getUser.mockReturnValue(new Promise(() => {})); + + renderHook(() => useSessionActivity()); + + // Advance past the probe's hard 4s deadline. + await act(async () => { + await vi.advanceTimersByTimeAsync(4100); + }); + + expect(replaceMock).toHaveBeenCalledWith('/golf/login?message=session_expired'); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/src/lib/auth/session-activity.ts b/src/lib/auth/session-activity.ts index 9d43ea4b5..25c069eeb 100644 --- a/src/lib/auth/session-activity.ts +++ b/src/lib/auth/session-activity.ts @@ -23,6 +23,7 @@ import { isSessionIdleExpired, parseLastActivity, } from '@/lib/auth/session-idle-shared'; +import { isDemoMetadataUser, probeSignedIn } from '@/lib/demo/gate-probe'; const ACTIVITY_CHECK_INTERVAL_MS = 60 * 1000; // Re-check every minute @@ -83,6 +84,23 @@ export function useSessionActivity() { const supabase = useMemo(() => createClient(), []); const handleLogout = useCallback(async () => { + const path = window.location.pathname; + const sport = path.startsWith('/golf') + ? 'golf' + : path.startsWith('/lifting') + ? 'lifting' + : path.startsWith('/baseball') + ? 'baseball' + : null; + + // A demo visitor never had a password — /login is a dead end for them. + // Resolve demo-context BEFORE signing out (the metadata this reads only + // exists on the still-live session); probeSignedIn is hard-timeout-guarded + // so a stuck check can never block the logout itself (#918). + const isDemo = + (sport === 'golf' || sport === 'baseball') && + isDemoMetadataUser(await probeSignedIn(supabase)); + clearLastActivity(); try { await supabase.auth.signOut(); @@ -90,19 +108,20 @@ export function useSessionActivity() { /* best-effort — still redirect to the login screen below */ } - const path = window.location.pathname; if (path.startsWith('/admin')) { const params = new URLSearchParams({ message: 'session_expired', returnTo: path }); router.replace(`/golf/login?${params.toString()}`); return; } - const sport = path.startsWith('/golf') - ? 'golf' - : path.startsWith('/lifting') - ? 'lifting' - : 'baseball'; + + if (isDemo && (sport === 'golf' || sport === 'baseball')) { + // replace() so the back button doesn't return to the timed-out page. + router.replace(`/${sport}/demo?message=demo_session_expired`); + return; + } + // replace() so the back button doesn't return to the timed-out page. - router.replace(`/${sport}/login?message=session_expired`); + router.replace(`/${sport ?? 'baseball'}/login?message=session_expired`); }, [router, supabase]); const checkSessionTimeout = useCallback(async () => { diff --git a/src/lib/demo/__tests__/gate-probe.test.ts b/src/lib/demo/__tests__/gate-probe.test.ts new file mode 100644 index 000000000..766085e55 --- /dev/null +++ b/src/lib/demo/__tests__/gate-probe.test.ts @@ -0,0 +1,164 @@ +// ============================================================================= +// src/lib/demo/__tests__/gate-probe.test.ts +// +// #918 — the demo gate's "Checking sign-in status" probe must resolve to +// signed-out on ANY error/timeout, never hang. Covers: +// 1. raceWithTimeout: fast resolve/reject pass through; a slow promise is +// pre-empted at the deadline. +// 2. probeSignedIn: happy path, a `getUser()` error field, a thrown/rejected +// getUser(), AND a getUser() that never settles (the actual failure mode +// a stuck refresh-token exchange produces) all resolve to `null` — never +// hang, never throw. +// 3. isDemoMetadataUser: pure metadata check used by both the probe callers +// and the middleware's demo-context redirect. +// ============================================================================= + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { + raceWithTimeout, + probeSignedIn, + isDemoMetadataUser, + DemoAuthProbeTimeoutError, + DEMO_AUTH_PROBE_TIMEOUT_MS, + type AuthProbeClient, +} from '@/lib/demo/gate-probe'; + +describe('raceWithTimeout — pass-through when the promise settles quickly', () => { + // Real timers here — nothing about these two assertions is time-based, and + // mixing fake timers with an already-settled/pre-rejected promise is a + // known source of flaky "unhandled rejection" noise unrelated to the + // behavior under test. + it('resolves with the value when the promise settles before the deadline', async () => { + await expect(raceWithTimeout(Promise.resolve('ok'), 1000)).resolves.toBe('ok'); + }); + + it('rejects with the original error when the promise rejects before the deadline', async () => { + await expect(raceWithTimeout(Promise.reject(new Error('boom')), 1000)).rejects.toThrow('boom'); + }); +}); + +describe('raceWithTimeout — timeout behavior', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('rejects with DemoAuthProbeTimeoutError once the deadline elapses on a promise that never settles', async () => { + const neverSettles = new Promise(() => {}); + const result = raceWithTimeout(neverSettles, 4000); + const assertion = expect(result).rejects.toBeInstanceOf(DemoAuthProbeTimeoutError); + await vi.advanceTimersByTimeAsync(4000); + await assertion; + }); + + it('defaults to DEMO_AUTH_PROBE_TIMEOUT_MS when no timeout is given', async () => { + const neverSettles = new Promise(() => {}); + const result = raceWithTimeout(neverSettles); + const assertion = expect(result).rejects.toBeInstanceOf(DemoAuthProbeTimeoutError); + await vi.advanceTimersByTimeAsync(DEMO_AUTH_PROBE_TIMEOUT_MS); + await assertion; + }); +}); + +describe('probeSignedIn', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + function clientResolving( + result: Awaited>, + ): AuthProbeClient { + return { auth: { getUser: vi.fn(async () => result) } }; + } + + it('returns the user on the happy path', async () => { + const client = clientResolving({ data: { user: { id: 'u1', email: 'demo@example.com' } }, error: null }); + const result = probeSignedIn(client); + await vi.advanceTimersByTimeAsync(0); + await expect(result).resolves.toEqual({ id: 'u1', email: 'demo@example.com' }); + }); + + it('returns null when getUser() resolves with no user', async () => { + const client = clientResolving({ data: { user: null }, error: null }); + const result = probeSignedIn(client); + await vi.advanceTimersByTimeAsync(0); + await expect(result).resolves.toBeNull(); + }); + + it('returns null when getUser() resolves with an error field (e.g. a refresh-token failure)', async () => { + const client = clientResolving({ + data: { user: { id: 'u1' } }, + error: { message: 'Invalid Refresh Token: Already Used' }, + }); + const result = probeSignedIn(client); + await vi.advanceTimersByTimeAsync(0); + await expect(result).resolves.toBeNull(); + }); + + it('returns null (never throws) when getUser() rejects', async () => { + const client: AuthProbeClient = { + auth: { getUser: vi.fn(async () => { throw new Error('network blip'); }) }, + }; + const result = probeSignedIn(client); + await vi.advanceTimersByTimeAsync(0); + await expect(result).resolves.toBeNull(); + }); + + it('returns null (never hangs) when getUser() never settles — the stuck refresh-token exchange case', async () => { + const client: AuthProbeClient = { + auth: { + getUser: vi.fn( + () => new Promise<{ data: { user: null }; error: unknown }>(() => {}), + ), + }, + }; + const result = probeSignedIn(client, 4000); + const assertion = expect(result).resolves.toBeNull(); + await vi.advanceTimersByTimeAsync(4000); + await assertion; + }); + + it('resolves well within a caller-visible deadline even on a hung getUser()', async () => { + const client: AuthProbeClient = { + auth: { + getUser: vi.fn( + () => new Promise<{ data: { user: null }; error: unknown }>(() => {}), + ), + }, + }; + const result = probeSignedIn(client, 100); + const assertion = expect(result).resolves.toBeNull(); + await vi.advanceTimersByTimeAsync(100); + await assertion; + }); +}); + +describe('isDemoMetadataUser', () => { + it('is true when user_metadata.is_demo is exactly true', () => { + expect(isDemoMetadataUser({ id: 'u1', user_metadata: { is_demo: true } })).toBe(true); + }); + + it('is false for a real (non-demo) user', () => { + expect(isDemoMetadataUser({ id: 'u1', user_metadata: { full_name: 'Coach Rivera' } })).toBe(false); + }); + + it('is false for a truthy-but-not-boolean-true value (e.g. the string "true")', () => { + expect(isDemoMetadataUser({ id: 'u1', user_metadata: { is_demo: 'true' } })).toBe(false); + }); + + it('is false when user_metadata is absent', () => { + expect(isDemoMetadataUser({ id: 'u1' })).toBe(false); + }); + + it('is false for null/undefined user', () => { + expect(isDemoMetadataUser(null)).toBe(false); + expect(isDemoMetadataUser(undefined)).toBe(false); + }); +}); diff --git a/src/lib/demo/gate-probe.ts b/src/lib/demo/gate-probe.ts new file mode 100644 index 000000000..964ae9f99 --- /dev/null +++ b/src/lib/demo/gate-probe.ts @@ -0,0 +1,104 @@ +/** + * Demo gate — client-safe "am I already signed in" probe + demo-session + * detection. #918. + * + * Both the GolfHelm and BaseballHelm demo gates (and the shared idle-timeout + * flow) need to answer two questions without ever risking an infinite hang: + * + * 1. Is the visitor already signed in? (`probeSignedIn`) + * 2. Is the CURRENT session a demo session? (`isDemoMetadataUser`) + * + * Both a stale/expired httpOnly Supabase auth cookie and a network blip can + * leave `supabase.auth.getUser()`'s internal refresh-token exchange hanging + * indefinitely — the browser `fetch` GoTrue issues internally has no timeout + * of its own, so the returned promise may simply never settle. `probeSignedIn` + * races the real check against a hard deadline and treats ANY failure — + * timeout, thrown error, or a refresh-token/network error surfaced through + * `getUser()`'s own `error` field — as "signed out", so a caller built on top + * of it always resolves and never hangs on a spinner forever. + * + * No `'use server'` / `'use client'` directive — this is a plain module safe + * to import from client components (demo gate pages, the session-activity + * hook) AND from the Edge-runtime middleware. + */ + +/** Hard deadline for the demo gate's "already signed in" probe. */ +export const DEMO_AUTH_PROBE_TIMEOUT_MS = 4000; + +export class DemoAuthProbeTimeoutError extends Error { + constructor() { + super('demo-gate-auth-probe-timeout'); + this.name = 'DemoAuthProbeTimeoutError'; + } +} + +/** + * Races `promise` against a hard deadline. Rejects with a + * {@link DemoAuthProbeTimeoutError} if `timeoutMs` elapses first. Clears the + * timer either way so a fast-resolving promise never leaks a pending timeout. + */ +export function raceWithTimeout( + promise: Promise, + timeoutMs: number = DEMO_AUTH_PROBE_TIMEOUT_MS, +): Promise { + return new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new DemoAuthProbeTimeoutError()), timeoutMs); + promise.then( + (value) => { + clearTimeout(timer); + resolve(value); + }, + (err) => { + clearTimeout(timer); + reject(err); + }, + ); + }); +} + +/** Minimal user shape both probe callers care about. */ +export interface ProbedUser { + id: string; + email?: string | null; + user_metadata?: Record | null; +} + +/** The minimal shape of a Supabase client's `auth.getUser()` this module needs. */ +export interface AuthProbeClient { + auth: { + getUser: () => Promise<{ data: { user: ProbedUser | null }; error: unknown }>; + }; +} + +/** + * "Am I already signed in?" probe. Treats ANY failure — hard timeout, a + * thrown error, or a refresh-token/network error surfaced through + * `getUser()`'s `error` field — as signed out (returns `null`), so a demo + * gate built on top of this never gets stuck on "Checking sign-in status". + */ +export async function probeSignedIn( + supabase: AuthProbeClient, + timeoutMs: number = DEMO_AUTH_PROBE_TIMEOUT_MS, +): Promise { + try { + const { data, error } = await raceWithTimeout(supabase.auth.getUser(), timeoutMs); + if (error) return null; + return data.user ?? null; + } catch { + return null; + } +} + +/** + * True when `user`'s metadata marks it as a demo session. Both `enterDemo` + * (golf) and `enterBaseballDemo` (baseball) stamp `user_metadata.is_demo = + * true` onto the shared demo account right after sign-in specifically so + * this check works WITHOUT exposing the demo account's real email to client + * code — unlike the email, "this is a demo session" carries no secret. + * + * Pure/sync — safe to call from the client, the server, and Edge middleware + * alike on a `user` object already in hand (no extra network round-trip). + */ +export function isDemoMetadataUser(user: ProbedUser | null | undefined): boolean { + return user?.user_metadata?.is_demo === true; +} diff --git a/src/lib/supabase/__tests__/middleware-demo-session-expired.test.ts b/src/lib/supabase/__tests__/middleware-demo-session-expired.test.ts new file mode 100644 index 000000000..871c4406b --- /dev/null +++ b/src/lib/supabase/__tests__/middleware-demo-session-expired.test.ts @@ -0,0 +1,135 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { NextRequest } from 'next/server'; + +/** + * #918 — an idle-expired DEMO session has nowhere to "sign in again" (the + * visitor never had a password), so /login is a dead end for it. The + * idle-timeout redirect must detect a demo-context session (via + * `user_metadata.is_demo`, or an email-match fallback for a session + * established before this fix shipped) and route back to the sport's own + * /demo gate with a friendly `message=demo_session_expired` instead. + * + * A NON-demo idle-expired session must be completely unaffected — still + * routes to /login?message=session_expired&returnTo=... exactly as before. + */ + +const mockGetUser = vi.fn(); +const mockSignOut = vi.fn(); + +vi.mock('@supabase/ssr', () => ({ + createServerClient: vi.fn(() => ({ + auth: { + getUser: mockGetUser, + signOut: mockSignOut, + }, + from: vi.fn(), + })), +})); + +import { updateSession } from '@/lib/supabase/middleware'; + +function buildRequest(pathname: string, cookieHeader?: string) { + const headers = new Headers({ 'user-agent': 'Mozilla/5.0 (test)' }); + if (cookieHeader) headers.set('cookie', cookieHeader); + return new NextRequest(`https://app.example.com${pathname}`, { headers }); +} + +const STALE = Date.now() - 10 * 60 * 1000; // 10 min ago > 5 min idle window + +describe('updateSession — demo-context idle-timeout redirect (#918)', () => { + beforeEach(() => { + mockGetUser.mockReset(); + mockSignOut.mockReset(); + mockSignOut.mockResolvedValue({ error: null }); + vi.stubEnv('NEXT_PUBLIC_SUPABASE_URL', 'https://xyz.supabase.co'); + vi.stubEnv('NEXT_PUBLIC_SUPABASE_ANON_KEY', 'anon-key'); + vi.stubEnv('DEMO_COACH_EMAIL', 'demo@golfhelmdemo.com'); + vi.stubEnv('DEMO_COACH_PASSWORD', 'irrelevant-for-this-test'); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it('routes an idle-expired golf demo session (is_demo metadata) back to /golf/demo, not /golf/login', async () => { + mockGetUser.mockResolvedValue({ + data: { user: { id: 'demo-1', email: 'demo@golfhelmdemo.com', user_metadata: { is_demo: true } } }, + error: null, + }); + + const req = buildRequest('/golf/dashboard', `sb_last_activity=${STALE}`); + const res = await updateSession(req); + + const location = res.headers.get('location'); + expect(location).not.toBeNull(); + const url = new URL(location!); + expect(url.pathname).toBe('/golf/demo'); + expect(url.searchParams.get('message')).toBe('demo_session_expired'); + expect(url.searchParams.get('returnTo')).toBeNull(); + expect(url.pathname).not.toBe('/golf/login'); + }); + + it('falls back to an email match for a demo session established before is_demo metadata shipped', async () => { + // No user_metadata.is_demo at all — only the email matches the + // server-only demo credentials (a session that pre-dates the fix). + mockGetUser.mockResolvedValue({ + data: { user: { id: 'demo-1', email: 'demo@golfhelmdemo.com', user_metadata: {} } }, + error: null, + }); + + const req = buildRequest('/golf/dashboard', `sb_last_activity=${STALE}`); + const res = await updateSession(req); + + const url = new URL(res.headers.get('location')!); + expect(url.pathname).toBe('/golf/demo'); + expect(url.searchParams.get('message')).toBe('demo_session_expired'); + }); + + it('routes an idle-expired baseball demo session back to /baseball/demo', async () => { + mockGetUser.mockResolvedValue({ + data: { + user: { + id: 'demo-2', + email: 'demo-coach@baseballhelmdemo.com', + user_metadata: { is_demo: true }, + }, + }, + error: null, + }); + + const req = buildRequest('/baseball/dashboard/command-center', `sb_last_activity=${STALE}`); + const res = await updateSession(req); + + const url = new URL(res.headers.get('location')!); + expect(url.pathname).toBe('/baseball/demo'); + expect(url.searchParams.get('message')).toBe('demo_session_expired'); + }); + + it('leaves a NON-demo idle-expired session on the normal /login redirect unchanged', async () => { + mockGetUser.mockResolvedValue({ + data: { user: { id: 'real-coach-1', email: 'coach@university.edu', user_metadata: {} } }, + error: null, + }); + + const req = buildRequest('/golf/dashboard', `sb_last_activity=${STALE}`); + const res = await updateSession(req); + + const url = new URL(res.headers.get('location')!); + expect(url.pathname).toBe('/golf/login'); + expect(url.searchParams.get('message')).toBe('session_expired'); + expect(url.searchParams.get('returnTo')).toBe('/golf/dashboard'); + }); + + it('a real user whose metadata happens to hold is_demo: false is NOT treated as demo', async () => { + mockGetUser.mockResolvedValue({ + data: { user: { id: 'real-coach-2', email: 'coach@university.edu', user_metadata: { is_demo: false } } }, + error: null, + }); + + const req = buildRequest('/golf/dashboard', `sb_last_activity=${STALE}`); + const res = await updateSession(req); + + const url = new URL(res.headers.get('location')!); + expect(url.pathname).toBe('/golf/login'); + }); +}); diff --git a/src/lib/supabase/middleware.ts b/src/lib/supabase/middleware.ts index 162225e5c..dc4a79633 100644 --- a/src/lib/supabase/middleware.ts +++ b/src/lib/supabase/middleware.ts @@ -9,6 +9,9 @@ import { isSessionIdleExpired, parseLastActivity, } from '@/lib/auth/session-idle-shared'; +import { isDemoMetadataUser, type ProbedUser } from '@/lib/demo/gate-probe'; +import { isDemoCoachEmail } from '@/lib/demo/config.server'; +import { isBaseballDemoCoachEmail } from '@/lib/demo/baseball-config.server'; /** * STAFF_CAPABILITY_ROUTES — the middleware mirror of every nav-registry entry @@ -57,6 +60,27 @@ function getSportFromPath(pathname: string): 'baseball' | 'golf' | 'lifting' | n return null; } +/** + * True when `user`'s CURRENT session belongs to a shared demo account for a + * sport that has a demo gate (golf, baseball — lifting has none). An expired + * demo session has nowhere to "sign in again" (the visitor never had a + * password), so the idle-timeout redirect below routes these back to the + * sport's /demo gate instead of the dead-end password login (#918). + * + * Checks `user_metadata.is_demo` first (set by `enterDemo` / + * `enterBaseballDemo` right after sign-in, readable without a secret) and + * falls back to an email match against the server-only demo credentials — + * covers a session established before this fix shipped, which won't carry + * the metadata flag yet. + */ +function isDemoContextUser(user: ProbedUser, sport: 'baseball' | 'golf' | 'lifting' | null): boolean { + if (sport !== 'golf' && sport !== 'baseball') return false; + if (isDemoMetadataUser(user)) return true; + return sport === 'golf' + ? isDemoCoachEmail(user.email) + : isBaseballDemoCoachEmail(user.email); +} + /** * Native app requests use the same auth/access policy as desktop web for app * routes, but are kept away from marketing/pricing surfaces. @@ -568,16 +592,26 @@ export async function updateSession(request: NextRequest) { } const loginUrl = request.nextUrl.clone(); - // /admin has no login route of its own — it reuses /golf/login (same as - // evaluateAdminGate's 'redirect-login' decision above) with returnTo - // bringing them back to /admin after signing in again. - loginUrl.pathname = sport ? `/${sport}/login` : '/golf/login'; - loginUrl.search = ''; - loginUrl.searchParams.set('message', 'session_expired'); - // Already gated on isIdleGatedRoute via the enclosing `if` above, so - // returnTo is always set here — send the user back to the exact page - // they were bounced from once they re-authenticate. - loginUrl.searchParams.set('returnTo', pathname); + if (isDemoContextUser(user, sport)) { + // A demo visitor never had a password — there is nothing for them to + // "sign in again" with, so /login is a dead end. Route back to the + // sport's own /demo gate (which re-enters the shared account) with a + // friendly message instead (#918). + loginUrl.pathname = `/${sport}/demo`; + loginUrl.search = ''; + loginUrl.searchParams.set('message', 'demo_session_expired'); + } else { + // /admin has no login route of its own — it reuses /golf/login (same as + // evaluateAdminGate's 'redirect-login' decision above) with returnTo + // bringing them back to /admin after signing in again. + loginUrl.pathname = sport ? `/${sport}/login` : '/golf/login'; + loginUrl.search = ''; + loginUrl.searchParams.set('message', 'session_expired'); + // Already gated on isIdleGatedRoute via the enclosing `if` above, so + // returnTo is always set here — send the user back to the exact page + // they were bounced from once they re-authenticate. + loginUrl.searchParams.set('returnTo', pathname); + } const res = NextResponse.redirect(loginUrl); // Propagate any cookie removals signOut wrote onto supabaseResponse...