diff --git a/scripts/login-continuation.test.mjs b/scripts/login-continuation.test.mjs new file mode 100644 index 0000000..3046546 --- /dev/null +++ b/scripts/login-continuation.test.mjs @@ -0,0 +1,113 @@ +import { test, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { build } from 'esbuild'; +import { pathToFileURL } from 'node:url'; +import { GlobalRegistrator } from '@happy-dom/global-registrator'; +if (!GlobalRegistrator.isRegistered) GlobalRegistrator.register({ url: 'http://localhost/' }); +globalThis.IS_REACT_ACT_ENVIRONMENT = true; +const React = await import('react'); +const { render, cleanup, fireEvent, act, waitFor } = await import('@testing-library/react'); +const { createMemoryRouter, RouterProvider } = await import('react-router-dom'); +const fixture = globalThis.__loginUi = { authenticated: false, listeners: new Set(), verify: async () => {} }; +await build({ + entryPoints: [{in: process.env.LOGIN_SOURCE ?? 'src/pages/Login.tsx', out: 'pages/Login'}, {in: 'src/lib/loginDestination.ts', out: 'lib/loginDestination'}], outdir: 'tmp/login', outbase: 'src', + bundle: true, splitting: true, platform: 'node', format: 'esm', jsx: 'automatic', outExtension: { '.js': '.mjs' }, + external: ['react', 'react/jsx-runtime', 'react-router-dom'], + plugins: [{ name: 'auth-fixture', setup(b) { + b.onResolve({ filter: /\/hooks\/useAuth$|\/components\/auth\/OtpInput$|\/lib\/analytics$/ }, args => ({ path: args.path, namespace: 'fixture' })); + b.onLoad({ filter: /.*/, namespace: 'fixture' }, ({ path }) => ({ contents: + path.endsWith('/useAuth') ? `import {useSyncExternalStore} from 'react'; export function useAuth(){const f=globalThis.__loginUi; const isAuthenticated=useSyncExternalStore(cb=>{f.listeners.add(cb);return()=>f.listeners.delete(cb)},()=>f.authenticated);return {isAuthenticated,isLoading:false,startOtp:async()=>{},verifyOtp:code=>f.verify(code)}}` : + path.endsWith('/OtpInput') ? `import {createElement} from 'react'; export const OtpInput=({onComplete})=>createElement('button',{onClick:()=>onComplete('123456')},'Verify fixture OTP');` : + 'export const trackSignupStarted=()=>{}; export const trackSignupCompleted=()=>{};' })); + }}], +}); +const { Login } = await import(pathToFileURL(`${process.cwd()}/tmp/login/pages/Login.mjs`)); +const { loginDestination } = await import(pathToFileURL(`${process.cwd()}/tmp/login/lib/loginDestination.mjs`)); +afterEach(() => { cleanup(); fixture.authenticated = false; fixture.verify = async () => {}; fixture.listeners.clear(); }); +const authenticate = () => { fixture.authenticated = true; for (const cb of fixture.listeners) cb(); }; +function mount({ entry = { pathname: '/login' }, embedded = false } = {}) { + const router = createMemoryRouter([ + { path: '/login', element: React.createElement(Login, { embedded }) }, + { path: '*', element: React.createElement('div', null, 'Destination') }, + ], { initialEntries: ['/previous', entry], initialIndex: 1 }); + const view = render(React.createElement(React.StrictMode, null, React.createElement(RouterProvider, { router }))); + return { router, view }; +} +async function otp(view) { + fireEvent.change(view.getByLabelText('Email address'), { target: { value: 'test@example.test' } }); + fireEvent.submit(view.getByRole('button', { name: 'Send Code' }).closest('form')); + await waitFor(() => assert.ok(view.getByText('Verify fixture OTP'))); + fireEvent.click(view.getByText('Verify fixture OTP')); +} + +test('destination preserves guarded pathname, search, hash and action state', () => { + assert.deepEqual(loginDestination('', { from: { pathname: '/list/abc', search: '?view=all', hash: '#item', state: { openShare: true } } }), { to: '/list/abc?view=all#item', state: { openShare: true } }); +}); +test('known runbook retains precedence and invalid runbooks fall back to guarded route', () => { + assert.equal(loginDestination('?template=release-checklist', { from: { pathname: '/profile' } }).to, '/templates/release-checklist/use'); + assert.equal(loginDestination('?template=invalid', { from: { pathname: '/profile' } }).to, '/profile'); +}); +test('missing, malformed, external and login-loop destinations safely use home', () => { + for (const state of [undefined, null, {}, {from: '/list/a'}, {from: {pathname: 5}}, ...['https://evil.test','//evil.test','/a/..//evil.test','/\\evil.test','/login','/login/','/LOGIN','/a/../login','/login?x=1','/list/a\n'].map(pathname => ({from:{pathname}}))]) { + assert.equal(loginDestination('', state).to, '/d', JSON.stringify(state)); + } +}); +test('already-authenticated login resumes intent and replaces history', async () => { + fixture.authenticated = true; + const {router} = mount({entry: {pathname:'/login',state:{from:{pathname:'/list/abc',search:'?q=1',hash:'#last',state:{openShare:true}}}}}); + await waitFor(() => assert.equal(router.state.location.pathname, '/list/abc')); + assert.equal(router.state.location.search, '?q=1'); assert.equal(router.state.location.hash, '#last'); + assert.deepEqual(router.state.location.state, {openShare:true}); + await act(async () => router.navigate(-1)); assert.equal(router.state.location.pathname,'/previous'); + await act(async () => router.navigate(1)); assert.equal(router.state.location.pathname,'/list/abc'); +}); +test('OTP authentication resumes the originally guarded route', async () => { + fixture.verify = async () => authenticate(); + const {router,view} = mount({entry:{pathname:'/login',state:{from:{pathname:'/invitations'}}}}); + await otp(view); + await waitFor(() => assert.equal(router.state.location.pathname, '/invitations')); +}); +test('late successful OTP after navigating away cannot hijack the newer page', async () => { + let finish; + fixture.verify = () => new Promise(resolve => { finish = () => { authenticate(); resolve(); }; }); + const {router,view} = mount({entry:{pathname:'/login',state:{from:{pathname:'/list/old'}}}}); + await otp(view); + await act(async () => router.navigate('/privacy')); + await act(async () => finish()); + assert.equal(router.state.location.pathname, '/privacy'); +}); +test('embedded login leaves navigation to its parent after OTP', async () => { + fixture.verify = async () => authenticate(); + const {router,view} = mount({embedded:true}); + await otp(view); + await act(async () => {}); + assert.equal(router.state.location.pathname, '/login'); +}); +test('App gives Login sole responsibility for post-auth destination', () => { + const source = readFileSync('src/App.tsx','utf8'); + assert.match(source, /path="\/login" element=\{\}/); + assert.doesNotMatch(source, /templateLoginDestination/); +}); + +test('rendered login resumes a selected runbook without stale route state', async () => { + fixture.authenticated = true; + const { router } = mount({entry:{pathname:'/login',search:'?template=release-checklist',state:{from:{pathname:'/profile',state:{old:true}}}}}); + await waitFor(() => assert.equal(router.state.location.pathname, '/templates/release-checklist/use')); + assert.equal(router.state.location.state, null); +}); +test('failed OTP remains on login with intended route available for a retry', async () => { + fixture.verify = async () => { throw new Error('Invalid code'); }; + const from = {pathname:'/list/retry',search:'?view=all'}; + const { router,view } = mount({entry:{pathname:'/login',state:{from}}}); + const originalError = console.error; + console.error = () => {}; + try { await otp(view); await act(async () => {}); } + finally { console.error = originalError; } + assert.equal(router.state.location.pathname, '/login'); + assert.deepEqual(router.state.location.state, {from}); + fixture.verify = async () => authenticate(); + fireEvent.click(view.getByText('Verify fixture OTP')); + await waitFor(() => assert.equal(router.state.location.pathname, '/list/retry')); + assert.equal(router.state.location.search,'?view=all'); +}); diff --git a/src/App.tsx b/src/App.tsx index a8ac85a..b436919 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -17,7 +17,6 @@ import { CookieConsent } from './components/CookieConsent' import { useSwipeBack } from './hooks/useSwipeBack' import { useNativeLinks } from './hooks/useNativeLinks' import { initPushNotifications } from './lib/pushNotifications' -import { templateLoginDestination } from '../convex/lib/templateCatalog' import { incrementMetric } from './lib/observability' // Lazy-loaded routes for better code splitting @@ -278,7 +277,7 @@ function App() { > {/* Public routes - accessible without authentication */} } /> - : } /> + } /> } /> } /> } /> @@ -311,3 +310,4 @@ function App() { } export default App + diff --git a/src/lib/loginDestination.ts b/src/lib/loginDestination.ts new file mode 100644 index 0000000..70f844b --- /dev/null +++ b/src/lib/loginDestination.ts @@ -0,0 +1,23 @@ +import { templateLoginDestination } from '../../convex/lib/templateCatalog'; + +/** Resume a guarded route without accepting external URLs or login loops. */ +export function loginDestination(search: string, state: unknown): { to: string; state?: unknown } { + const template = templateLoginDestination(search); + if (template !== '/d') return { to: template }; + + const from = state && typeof state === 'object' && 'from' in state ? state.from : null; + if (!from || typeof from !== 'object' || !('pathname' in from)) return { to: '/d' }; + const pathname = from.pathname; + if (typeof pathname !== 'string' || !pathname.startsWith('/') || pathname.startsWith('//') || + /[\\?#]/.test(pathname) || [...pathname].some(character => character.charCodeAt(0) <= 32)) return { to: '/d' }; + const target = new URL(pathname, 'https://boop.invalid'); + if (target.origin !== 'https://boop.invalid' || target.pathname.startsWith('//') || + /^\/login\/*$/i.test(target.pathname)) return { to: '/d' }; + + const query = 'search' in from && typeof from.search === 'string' && from.search.startsWith('?') ? from.search : ''; + const hash = 'hash' in from && typeof from.hash === 'string' && from.hash.startsWith('#') ? from.hash : ''; + return { + to: `${target.pathname}${query}${hash}`, + state: 'state' in from ? from.state : undefined, + }; +} diff --git a/src/pages/Login.tsx b/src/pages/Login.tsx index 446eee4..fa380c8 100644 --- a/src/pages/Login.tsx +++ b/src/pages/Login.tsx @@ -5,16 +5,16 @@ * 1. User enters email, clicks "Send Code" * 2. User enters OTP code from email, clicks "Verify" * - * On successful verification, user is redirected to Home. + * On successful verification, resume the intended page or selected runbook. */ import { useState, type FormEvent } from "react"; -import { useNavigate, useLocation, Link } from "react-router-dom"; +import { Navigate, useLocation, Link } from "react-router-dom"; import { useAuth } from "../hooks/useAuth"; import { OtpInput } from "../components/auth/OtpInput"; import { trackSignupStarted, trackSignupCompleted } from "../lib/analytics"; -import { templateLoginDestination } from "../../convex/lib/templateCatalog"; +import { loginDestination } from "../lib/loginDestination"; type LoginStep = "email" | "otp"; @@ -34,7 +34,6 @@ function getGreeting(): string { export function Login({ embedded = false }: LoginProps) { const { startOtp, verifyOtp, isLoading, isAuthenticated } = useAuth(); - const navigate = useNavigate(); const location = useLocation(); const [step, setStep] = useState("email"); @@ -44,8 +43,8 @@ export function Login({ embedded = false }: LoginProps) { // Redirect if already authenticated (unless embedded) if (isAuthenticated && !embedded) { - navigate(templateLoginDestination(location.search), { replace: true }); - return null; + const destination = loginDestination(location.search, location.state); + return ; } const validateEmail = (email: string): boolean => { @@ -87,11 +86,9 @@ export function Login({ embedded = false }: LoginProps) { try { await verifyOtp(code); trackSignupCompleted("free"); - // On successful verification, useAuth will update isAuthenticated - // If not embedded, redirect to app; if embedded, parent handles navigation - if (!embedded) { - navigate(templateLoginDestination(location.search), { replace: true }); - } + // Auth state drives the redirect while this page is still mounted. + // A late OTP response must not override navigation away from login. + // Embedded callers own their navigation. } catch (err) { console.error("Failed to verify OTP:", err); // Server sanitizes /auth/verify errors, so its message is safe to show. @@ -193,3 +190,4 @@ export function Login({ embedded = false }: LoginProps) { ); } +