From 5ecdbf4c08097921399031f861711d3cfd926148 Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Wed, 12 Aug 2026 22:17:29 -0700 Subject: [PATCH] fix(auth): retire verification code across services --- docs/officers/EMERGENCY_AND_RECOVERY.md | 33 +- src/pages/login/VerifyEmailAction.test.tsx | 340 ++++++++++++++++++++- src/pages/login/VerifyEmailAction.tsx | 81 ++++- 3 files changed, 420 insertions(+), 34 deletions(-) diff --git a/docs/officers/EMERGENCY_AND_RECOVERY.md b/docs/officers/EMERGENCY_AND_RECOVERY.md index d76187ff..a59c82aa 100644 --- a/docs/officers/EMERGENCY_AND_RECOVERY.md +++ b/docs/officers/EMERGENCY_AND_RECOVERY.md @@ -166,13 +166,13 @@ After the exact #118 and #153 live proofs exist: ### Verification link page — SOURCE ONLY, NOT LIVE -**Status:** [#194](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/194) is a verification-only source change. It is not a live procedure until the exact website revision is published, `runmprc.com` is verified, and [#119](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/119) proves a safe Firebase handler choice. Firebase uses one handler for verification, password reset, and email recovery. The partial route must not be enabled globally. +**Status:** [#194](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/194) is a verification-only source change. [#571](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/571) adds a source safeguard when the website's current sign-in service becomes unavailable or changes. Neither change makes this a live procedure. It remains unavailable until the exact website revision is published, `runmprc.com` is verified, and [#119](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/119) proves a safe Firebase handler choice. Firebase uses one handler for verification, password reset, and email recovery. The partial route must not be enabled globally. **Purpose:** let a member check one private verification link without exposing its code or letting an automated mail scanner change the account. **Approver:** identity/platform owner plus membership lead. The communications owner and a second Firebase owner approve the provider/template decision. -**Prerequisites:** exact #194 merge and website proof; protected release proof; private #119 proof that reset and recovery links still work; an isolated Firebase project; a made-up account and safe email sink. A source test, preview, or real member link is not enough. +**Prerequisites:** exact #194 and #571 merges and website proof; protected release proof; private #119 proof that reset and recovery links still work; an isolated Firebase project; a made-up account and safe email sink. A source test, preview, or real member link is not enough. ```mermaid flowchart LR @@ -183,10 +183,12 @@ flowchart LR Result -- "Verified or already complete" --> Account["Continue to My Account"] Result -- "Different account" --> SignOut["Sign out; reopen original link"] Result -- "Cannot be used" --> New["Request one new email"] - Result -- "Temporary" --> Retry["Check connection; retry once"] + Result -- "Temporary on same page" --> Retry["Check connection; retry once"] + Context["Sign-in service changes after Checking begins"] --> Retire["Page retires this private link"] + Retire --> New ``` -In words: opening the page removes the private code but changes nothing; the member chooses one button, then follows one plain result without sharing the link or account details. +In words: opening the page removes the private code but changes nothing; the member chooses one button, then follows one plain result. If the website's sign-in service is lost or changes after **Checking verification...** begins, the page hides the earlier result, retires this link, and asks for one new email. It never sends the same private code to a replacement service. An offline click stops before Checking begins and sends no request. Officer steps after every prerequisite has proof: @@ -195,19 +197,24 @@ Officer steps after every prerequisite has proof: 3. Ask them to confirm the address ends with `/auth/action` and contains no `?` or `#` before choosing anything. 4. Ask them to choose **Verify email** once. 5. If the page says verified or already complete, ask them to continue to My Account. -6. If it says a different account is signed in, ask them to open My Account, sign out, and reopen the original message. -7. If the link cannot be used, ask them to request one new verification email from My Account. -8. If verification is temporarily unavailable, ask them to check the connection and use the one retry. -9. If the second attempt is not clear, stop and open a redacted incident. -10. Record only the time, clean route `/auth/action`, and exact plain result. +6. If the page says a different account is signed in, ask them to open My Account. +7. After step 6, ask them to sign out. +8. After step 7, ask them to reopen the original message. +9. If the link cannot be used, ask them to request one new verification email from My Account. +10. If verification is temporarily unavailable, ask them to check the connection. +11. After step 10, ask them to use **Try verification again** once on the same page. +12. If **Verify email** reappears after **Checking verification...** began, do not choose it again. +13. If the current attempt is not clear, stop. +14. After step 13, open a redacted incident. +15. Record only the time, clean route `/auth/action`, and exact plain result. -**Expected result:** page load makes no action-code check and changes no account; the private query and fragment disappear; one deliberate action gives a fixed result; no address, code, provider error, role, membership, payment, or profile value appears or changes. +**Expected result:** page load makes no action-code check and changes no account; the private query and fragment disappear; one deliberate action gives a fixed result; and no address, code, provider error, role, membership, payment, or profile value appears or changes. If the current website sign-in service is lost or replaced after **Checking verification...** begins, the old result disappears, the page makes this link unusable, and no replacement service receives its code. A request already sent through the earlier service cannot be cancelled, so the member must use one new email instead of reusing this link. An offline click never reaches Checking and makes no provider request. -**Stop conditions:** the address still contains `?` or `#`; anyone asks for a link/code/address/screenshot; the page changes an account before the button; a non-verification action opens here; more than one retry; a real-member/provider test; or missing provider/live proof. +**Stop conditions:** the address still contains `?` or `#`; anyone asks for a link/code/address/screenshot; the page changes an account before the button; **Verify email** reappears after **Checking verification...** began; an old result appears or changes after the link becomes unusable; a non-verification action opens here; more than one same-page temporary retry; a real-member/provider test; or missing provider/live proof. -**Success proof:** exact #194 pull request and merge; green synthetic no-network tests; dated keyboard and screen-reader check; exact website publication and `runmprc.com` revision; private #119 staging proof for verification, reset, recovery, reuse, expiration, and rollback. Keep each proof separate. +**Success proof:** exact #194 and #571 pull requests and merges; green synthetic no-network tests, including readiness loss, service replacement before and after a click, retired-link behavior, late old results, and same-service wrapper checks; dated keyboard and screen-reader check; exact website publication and `runmprc.com` revision; private #119 staging proof for verification, reset, recovery, reuse, expiration, and rollback. Keep each proof separate. -**Undo:** publish one reviewed frontend revert or safe roll-forward. The Firebase owners separately restore the previously proven default/multi-mode handler. Never delete an account or edit verification state by hand. +**Undo:** publish one reviewed frontend revert or safe roll-forward for #571. The Firebase owners separately restore the previously proven default/multi-mode handler. Never delete an account or edit verification state by hand. **Escalation:** identity/platform owner plus membership lead; add communications for sender/delivery, security for an exposed code or wrong mutation, and the provider owner for handler/template failure. diff --git a/src/pages/login/VerifyEmailAction.test.tsx b/src/pages/login/VerifyEmailAction.test.tsx index 900896be..92d49006 100644 --- a/src/pages/login/VerifyEmailAction.test.tsx +++ b/src/pages/login/VerifyEmailAction.test.tsx @@ -24,6 +24,24 @@ const mockUseServiceLocator = useServiceLocator as jest.MockedFunction< >; const verifyEmailAction = jest.fn(); +type VerificationIdentityService = { + verifyEmailAction: jest.Mock; +}; + +function createDeferred() { + let resolve!: (value: T | PromiseLike) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + return { promise, reject, resolve }; +} + +function createIdentityService(action: jest.Mock): VerificationIdentityService { + return { verifyEmailAction: action }; +} + function RouterLocationWitness() { const currentLocation = useLocation(); return ( @@ -51,20 +69,22 @@ function SameRouteNavigationProbe() { ); } -function setReadyServices() { +function setServiceContext( + identityService: VerificationIdentityService | null, + isReady = identityService !== null, +) { mockUseServiceLocator.mockReturnValue({ - isReady: true, - services: { - identityService: { verifyEmailAction }, - }, + isReady, + services: identityService === null ? null : { identityService }, } as unknown as ReturnType); } -function renderAction( - target = '/auth/action?mode=verifyEmail&oobCode=synthetic-action-code', -) { - window.history.replaceState(null, '', target); - return render( +function setReadyServices() { + setServiceContext(createIdentityService(verifyEmailAction)); +} + +function ActionRouteTree() { + return ( Fixed account destination} /> - , + ); } +function renderAction( + target = '/auth/action?mode=verifyEmail&oobCode=synthetic-action-code', +) { + window.history.replaceState(null, '', target); + return render(); +} + describe('VerifyEmailAction', () => { beforeEach(() => { jest.clearAllMocks(); @@ -363,6 +390,297 @@ describe('VerifyEmailAction', () => { expect(document.body).not.toHaveTextContent('Email verified.'); }); + describe('AUTH-MAIL-002C2A current identity-service boundary', () => { + test('allows a current service after an offline click made no provider request', async () => { + const online = jest.spyOn(window.navigator, 'onLine', 'get') + .mockReturnValue(false); + const verifyA = jest.fn().mockResolvedValue('verified'); + const verifyB = jest.fn().mockResolvedValue('already-complete'); + setServiceContext(createIdentityService(verifyA)); + const page = renderAction(); + + fireEvent.click(screen.getByRole('button', { name: 'Verify email' })); + expect(verifyA).not.toHaveBeenCalled(); + expect(screen.getByRole('status')).toHaveTextContent( + 'Verification is temporarily unavailable.', + ); + + online.mockReturnValue(true); + setServiceContext(createIdentityService(verifyB)); + page.rerender(); + const currentButton = screen.getByRole('button', { name: 'Verify email' }); + + expect(verifyA).not.toHaveBeenCalled(); + expect(verifyB).not.toHaveBeenCalled(); + expect(currentButton).toBeEnabled(); + fireEvent.click(currentButton); + expect(verifyB).toHaveBeenCalledTimes(1); + expect(verifyB).toHaveBeenCalledWith('synthetic-action-code'); + expect(await screen.findByText( + 'Email verification is already complete.', + )).toBeInTheDocument(); + }); + + test.each([ + ['service replacement', createIdentityService(jest.fn()), true], + ['readiness loss and recovery', null, false], + ])( + 'retires a provider-sent code after unavailable and %s', + async (_label, nextService, nextReady) => { + const verifyA = jest.fn().mockResolvedValue('unavailable'); + const serviceA = createIdentityService(verifyA); + setServiceContext(serviceA); + const page = renderAction(); + + fireEvent.click(screen.getByRole('button', { name: 'Verify email' })); + expect(await screen.findByText( + 'Verification is temporarily unavailable.', + )).toBeInTheDocument(); + expect(verifyA).toHaveBeenCalledTimes(1); + + setServiceContext(nextService, nextReady); + page.rerender(); + + expect(screen.getByRole('status')).toHaveTextContent( + 'This verification link cannot be used.', + ); + expect(screen.queryByRole('button', { + name: /try verification|verify email|verification/i, + })).not.toBeInTheDocument(); + if (nextService) { + expect(nextService.verifyEmailAction).not.toHaveBeenCalled(); + } else { + setServiceContext(serviceA); + page.rerender(); + expect(screen.getByRole('status')).toHaveTextContent( + 'This verification link cannot be used.', + ); + expect(screen.queryByRole('button', { + name: /try verification|verify email|verification/i, + })).not.toBeInTheDocument(); + } + expect(verifyA).toHaveBeenCalledTimes(1); + }, + ); + + test.each([ + ['readiness is lost', false, true], + ['services become unavailable', true, false], + ])( + 'invalidates a pending result when %s', + async (_label, nextReady, keepServices) => { + const attemptA = createDeferred(); + const verifyA = jest.fn(() => attemptA.promise); + const serviceA = createIdentityService(verifyA); + setServiceContext(serviceA); + const page = renderAction(); + + fireEvent.click(screen.getByRole('button', { name: 'Verify email' })); + expect(screen.getByRole('button', { name: 'Checking verification...' })) + .toBeDisabled(); + + setServiceContext(keepServices ? serviceA : null, nextReady); + page.rerender(); + + expect(screen.getByRole('status')).toHaveTextContent( + 'This verification link cannot be used.', + ); + expect(screen.queryByRole('button', { name: /verify email|verification/i })) + .not.toBeInTheDocument(); + expect(screen.queryByText('Email verified.')).not.toBeInTheDocument(); + + setServiceContext(serviceA); + page.rerender(); + expect(screen.getByRole('status')).toHaveTextContent( + 'This verification link cannot be used.', + ); + expect(screen.queryByRole('button', { name: /verify email|verification/i })) + .not.toBeInTheDocument(); + + await act(async () => { + attemptA.resolve('verified'); + await attemptA.promise; + }); + + expect(verifyA).toHaveBeenCalledTimes(1); + expect(screen.getByRole('status')).toHaveTextContent( + 'This verification link cannot be used.', + ); + expect(screen.queryByText('Email verified.')).not.toBeInTheDocument(); + }, + ); + + test('retires a code on service replacement while a late old success stays inert', async () => { + const attemptA = createDeferred(); + const verifyA = jest.fn(() => attemptA.promise); + const verifyB = jest.fn().mockResolvedValue('verified'); + setServiceContext(createIdentityService(verifyA)); + const page = renderAction(); + + fireEvent.click(screen.getByRole('button', { name: 'Verify email' })); + + setServiceContext(createIdentityService(verifyB)); + page.rerender(); + + expect(verifyA).toHaveBeenCalledTimes(1); + expect(verifyB).not.toHaveBeenCalled(); + expect(screen.getByRole('status')).toHaveTextContent( + 'This verification link cannot be used.', + ); + expect(screen.queryByRole('button', { name: /verify email|verification/i })) + .not.toBeInTheDocument(); + + await act(async () => { + attemptA.resolve('verified'); + await attemptA.promise; + }); + + expect(verifyB).not.toHaveBeenCalled(); + expect(screen.getByRole('status')).toHaveTextContent( + 'This verification link cannot be used.', + ); + expect(screen.queryByText('Email verified.')).not.toBeInTheDocument(); + }); + + test('does not inspect a late hostile rejection after service replacement', async () => { + const attemptA = createDeferred(); + const verifyA = jest.fn(() => attemptA.promise); + const verifyB = jest.fn().mockResolvedValue('verified'); + const hostileRead = jest.fn(() => { + throw new Error('hostile rejection property was read'); + }); + const hostileRejection = new Proxy({}, { get: hostileRead }); + const consoleSpies = (['debug', 'error', 'info', 'log', 'warn'] as const) + .map((method) => jest.spyOn(console, method) + .mockImplementation(() => undefined)); + setServiceContext(createIdentityService(verifyA)); + const page = renderAction(); + + fireEvent.click(screen.getByRole('button', { name: 'Verify email' })); + setServiceContext(createIdentityService(verifyB)); + page.rerender(); + expect(screen.getByRole('status')).toHaveTextContent( + 'This verification link cannot be used.', + ); + + await act(async () => { + attemptA.reject(hostileRejection); + try { + await attemptA.promise; + } catch { + // The component owns the rejection; the test only flushes its turn. + } + }); + + expect(hostileRead).not.toHaveBeenCalled(); + consoleSpies.forEach((spy) => expect(spy).not.toHaveBeenCalled()); + expect(verifyB).not.toHaveBeenCalled(); + expect(screen.getByRole('status')).toHaveTextContent( + 'This verification link cannot be used.', + ); + expect(screen.queryByText('Email verified.')).not.toBeInTheDocument(); + }); + + test('does not reuse a sent code after A to B to A context changes', async () => { + const firstA = createDeferred(); + const verifyA = jest.fn(() => firstA.promise); + const serviceA = createIdentityService(verifyA); + const serviceB = createIdentityService(jest.fn()); + setServiceContext(serviceA); + const page = renderAction(); + + fireEvent.click(screen.getByRole('button', { name: 'Verify email' })); + setServiceContext(serviceB); + page.rerender(); + setServiceContext(serviceA); + page.rerender(); + + expect(verifyA).toHaveBeenCalledTimes(1); + expect(serviceB.verifyEmailAction).not.toHaveBeenCalled(); + expect(screen.getByRole('status')).toHaveTextContent( + 'This verification link cannot be used.', + ); + expect(screen.queryByRole('button', { name: /verify email|verification/i })) + .not.toBeInTheDocument(); + + await act(async () => { + firstA.resolve('verified'); + await firstA.promise; + }); + + expect(verifyA).toHaveBeenCalledTimes(1); + expect(screen.getByRole('status')).toHaveTextContent( + 'This verification link cannot be used.', + ); + expect(screen.queryByText('Email verified.')).not.toBeInTheDocument(); + }); + + test('preserves a current result across a wrapper-only service change', async () => { + const serviceA = createIdentityService(verifyEmailAction); + setServiceContext(serviceA); + const page = renderAction(); + fireEvent.click(screen.getByRole('button', { name: 'Verify email' })); + expect(await screen.findByText('Email verified.')).toBeInTheDocument(); + + setServiceContext(serviceA); + page.rerender(); + + expect(screen.getByText('Email verified.')).toBeInTheDocument(); + expect(verifyEmailAction).toHaveBeenCalledTimes(1); + expect(screen.queryByRole('button', { name: 'Verify email' })) + .not.toBeInTheDocument(); + }); + + test.each([ + ['readiness loss', null, false], + ['service replacement', createIdentityService(jest.fn()), true], + ])( + 'retires a code and hides a terminal result after %s', + async (_label, nextService, nextReady) => { + const serviceA = createIdentityService(verifyEmailAction); + setServiceContext(serviceA); + const page = renderAction(); + fireEvent.click(screen.getByRole('button', { name: 'Verify email' })); + expect(await screen.findByText('Email verified.')).toBeInTheDocument(); + + setServiceContext(nextService, nextReady); + page.rerender(); + + expect(screen.queryByText('Email verified.')).not.toBeInTheDocument(); + expect(screen.getByRole('status')).toHaveTextContent( + 'This verification link cannot be used.', + ); + expect(screen.queryByRole('button', { + name: /verify email|verification/i, + })).not.toBeInTheDocument(); + if (nextService) { + expect(nextService.verifyEmailAction).not.toHaveBeenCalled(); + } + }, + ); + + test('uses a replacement service when no provider attempt has started', async () => { + const verifyA = jest.fn().mockResolvedValue('verified'); + const verifyB = jest.fn().mockResolvedValue('already-complete'); + setServiceContext(createIdentityService(verifyA)); + const page = renderAction(); + + setServiceContext(createIdentityService(verifyB)); + page.rerender(); + const currentButton = screen.getByRole('button', { name: 'Verify email' }); + + expect(verifyA).not.toHaveBeenCalled(); + expect(verifyB).not.toHaveBeenCalled(); + expect(currentButton).toBeEnabled(); + fireEvent.click(currentButton); + expect(verifyB).toHaveBeenCalledTimes(1); + expect(verifyB).toHaveBeenCalledWith('synthetic-action-code'); + expect(await screen.findByText( + 'Email verification is already complete.', + )).toBeInTheDocument(); + }); + }); + test('uses a scoped 48px native action with visible keyboard focus', () => { renderAction(); const button = screen.getByRole('button', { name: 'Verify email' }); diff --git a/src/pages/login/VerifyEmailAction.tsx b/src/pages/login/VerifyEmailAction.tsx index 227b8dde..3e399622 100644 --- a/src/pages/login/VerifyEmailAction.tsx +++ b/src/pages/login/VerifyEmailAction.tsx @@ -18,6 +18,11 @@ type ViewState = | 'checking' | EmailVerificationActionResult; +interface VerificationAttempt { + contextGeneration: number; + token: symbol; +} + function readVerificationCode(search: string): string | null { try { const params = new URLSearchParams(search); @@ -66,20 +71,59 @@ function VerifyEmailAction() { const location = useLocation(); const navigate = useNavigate(); const { services, isReady } = useServiceLocator(); + const identityService = isReady ? services?.identityService ?? null : null; + const committedContextRef = useRef({ + generation: 0, + identityService, + }); + const [renderedContextGeneration, setRenderedContextGeneration] = useState(0); + const contextMatches = committedContextRef.current.identityService + === identityService + && committedContextRef.current.generation === renderedContextGeneration; const actionCodeRef = useRef(undefined); if (actionCodeRef.current === undefined) { actionCodeRef.current = readVerificationCode(location.search); } - const [viewState, setViewState] = useState( + const [storedViewState, setViewState] = useState( actionCodeRef.current === null ? 'unusable' : 'ready', ); const [historyScrubbed, setHistoryScrubbed] = useState(false); - const inFlightRef = useRef(false); + const activeAttemptRef = useRef(null); + const providerAttemptStartedRef = useRef(false); const initialScrubCompleteRef = useRef(false); const mountedRef = useRef(true); const requestVersionRef = useRef(0); const scrubFailedRef = useRef(false); const resultRef = useRef(null); + let viewState = storedViewState; + if (!contextMatches) { + viewState = providerAttemptStartedRef.current ? 'unusable' : 'ready'; + } + + useLayoutEffect(() => { + const committedContext = committedContextRef.current; + if (committedContext.identityService === identityService) return; + + const nextContext = { + generation: committedContext.generation + 1, + identityService, + }; + committedContextRef.current = nextContext; + requestVersionRef.current += 1; + activeAttemptRef.current = null; + + if (providerAttemptStartedRef.current) { + // A code already sent through one identity service is never reused after + // readiness or service identity changes. The first provider may already + // have consumed it, and this page cannot prove another service shares the + // same authority. + actionCodeRef.current = null; + setViewState('unusable'); + } else { + setViewState(actionCodeRef.current === null ? 'unusable' : 'ready'); + } + setRenderedContextGeneration(nextContext.generation); + }, [identityService]); useLayoutEffect(() => { const hasNavigationState = location.search !== '' @@ -162,11 +206,14 @@ function VerifyEmailAction() { const runVerification = async () => { const actionCode = actionCodeRef.current; + const committedContext = committedContextRef.current; if ( - inFlightRef.current + !contextMatches + || activeAttemptRef.current?.contextGeneration + === committedContext.generation || !historyScrubbed - || !isReady - || !services + || !identityService + || committedContext.identityService !== identityService || actionCode === null || actionCode === undefined ) { @@ -178,24 +225,38 @@ function VerifyEmailAction() { return; } - inFlightRef.current = true; + const attemptToken = Symbol('verification-attempt'); + activeAttemptRef.current = { + contextGeneration: committedContext.generation, + token: attemptToken, + }; + providerAttemptStartedRef.current = true; const requestVersion = requestVersionRef.current; setViewState('checking'); let result: EmailVerificationActionResult; try { - result = await services.identityService.verifyEmailAction(actionCode); + result = await identityService.verifyEmailAction(actionCode); } catch { result = 'unavailable'; } finally { - inFlightRef.current = false; + if (activeAttemptRef.current?.token === attemptToken) { + activeAttemptRef.current = null; + } } - if (mountedRef.current && requestVersion === requestVersionRef.current) { + if ( + mountedRef.current + && requestVersion === requestVersionRef.current + && committedContextRef.current.generation + === committedContext.generation + && committedContextRef.current.identityService === identityService + ) { if (result !== 'unavailable') actionCodeRef.current = null; setViewState(result); } }; - const providerReady = historyScrubbed && isReady && services !== null; + const providerReady = historyScrubbed && contextMatches + && identityService !== null; const preparing = viewState === 'ready' && !providerReady; const checking = viewState === 'checking'; const showAction = viewState === 'ready' || viewState === 'unavailable';