From 99f346a2a89b5658260b80f094c8ee7cfda4235e Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Fri, 29 May 2026 16:32:12 +0530 Subject: [PATCH] fix(checkout): auth-gate /app/checkout + fail-closed razorpay_configured (BUG-P111/P112/P013/P088/P121) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QA found that visiting /app/checkout/?plan=hobby while unauthenticated landed at a LIVE-mode Razorpay subscription page (sub_Sv96Mt2n8nnDYL). A stale localStorage JWT was passing the App-level AuthGate (which only checks getToken(), not validity), so the SPA short-circuited into a real Razorpay create-subscription call. BUG-P111 (P0 critical). Plus three sibling bugs from the same QA session: - BUG-P013 (P1): /login redirect dropped the plan context, losing the funnel after signin. - BUG-P088 (P1): razorpay_configured defaulted to TRUE when the API omitted the flag — clicking upgrade hit a 502 from Razorpay because recurring is not enabled on the prod account (per CLAUDE.md memory project_razorpay_recurring_not_enabled.md). - BUG-P121/P122 (P1): a future caching regression of sub_* shortlinks would survive logout without a dedicated purge. Fixes: 1. CheckoutPage gains a SECOND-LAYER auth gate that re-checks getToken() on mount BEFORE any createCheckout call. On miss it fails CLOSED with window.location.assign('/login?next='), preserving plan + frequency so post-signin returns the user to the same checkout. 2. CheckoutPage also handles the new 503 billing_misconfigured envelope from the server-side guard (fix/billing-traffic-env-and-misconfig- detection on the api repo) the same way as billing_not_configured — honest fallback panel instead of a raw error banner. 3. CheckoutPage registers a logout hook that purges every localStorage key under CHECKOUT_CACHE_KEY_PREFIX. The page does NOT cache sub_* today; the prefix + purge guarantee is the defensive belt against any future regression that adds such caching. 4. LoginPage now honors ?next= from the query string, falling back to loc.state.from (existing AuthGate path) then /app. Only relative same-origin paths are honoured — /login?next=https: //evil.com and protocol-relative //evil.com both fall back to /app to defeat open-redirect phishing. 5. mapBillingState flips razorpay_configured `?? true` → `?? false`. When the API doesn't say, hide the upgrade button. Honest copy beats a button that 502s. ### Surface checklist (rule 22) - api/plans.yaml not affected - common/plans/plans.go defaultYAML not affected - api/internal/handlers/openapi.go updated in companion PR (api) - instanode-web/.../PricingPage.tsx not affected (CTA wiring unchanged) - content/llms.txt not affected - dashboard upgradeCopy.ts not present in this repo Tests added (all 1069 vitest pass): - TestCheckoutPage_UnauthRedirectsToLogin - TestCheckoutPage_PreservesNextParam - TestCheckoutPage_PreservesFrequencyMonthlyWhenOnlyPlan - TestCheckoutPage_FlipsToUnauthOnMidFlight401 - TestCheckoutPage_ClearsCachedSubOnLogout (clearCheckoutCache suite) - TestCheckoutPage_RendersFallbackOn503BillingMisconfigured (BUG-P112) - LoginPage_HonorsNextRoundTripsToCheckout (BUG-P013) - LoginPage_RejectsAbsoluteEvilCom (open-redirect) - LoginPage_RejectsProtocolRelativeNext (open-redirect) - TestFetchBilling_DefaultsRazorpayConfiguredFalse (BUG-P088 fail-closed) Live verification queued in companion api PR — see body for curl + Chrome MCP evidence. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/api/index.test.ts | 35 +++- src/api/index.ts | 16 +- src/pages/CheckoutPage.test.tsx | 344 +++++++++++++++++++++++++++----- src/pages/CheckoutPage.tsx | 169 +++++++++++++++- src/pages/LoginPage.test.tsx | 42 ++++ src/pages/LoginPage.tsx | 17 +- 6 files changed, 553 insertions(+), 70 deletions(-) diff --git a/src/api/index.test.ts b/src/api/index.test.ts index e2d3158..4d0e5ae 100644 --- a/src/api/index.test.ts +++ b/src/api/index.test.ts @@ -244,6 +244,14 @@ describe('fetchBilling()', () => { payment_method: { type: 'card', brand: 'visa', last4: '4242' }, razorpay_subscription_id: 'sub_abc', razorpay_customer_id: 'cust_abc', + // BUG-P088 (2026-05-29): the SPA default for razorpay_configured is + // now FAIL-CLOSED (false) when the API omits it — so a healthy + // active-subscription mock must set the flag explicitly to keep + // exercising the mapped-through-true path. The previous `?? true` + // default masked a real bug: an older API build omitting the flag + // rendered the upgrade button as configured even when Razorpay was + // disabled. + razorpay_configured: true, })) const r = await fetchBilling() expect(r.ok).toBe(true) @@ -265,14 +273,31 @@ describe('fetchBilling()', () => { expect(String(url)).toContain('/api/v1/billing') }) - // P2-19: razorpay_configured no longer infers itself from - // subscription_status. A freshly-claimed paid team has no subscription - // yet (status='none') but Razorpay IS configured — checkout must not be - // suppressed. When the API omits the explicit flag we default to true. - it("keeps razorpay_configured=true when subscription_status='none' (API omits the flag)", async () => { + // BUG-P088 (P1, 2026-05-29): the SPA default for razorpay_configured is + // FAIL-CLOSED — when the API omits the flag we render the upgrade button + // as if Razorpay were NOT configured. Per + // CLAUDE.md memory project_razorpay_recurring_not_enabled.md, Razorpay + // recurring is not enabled on the prod account and clicking that button + // returns 502 from the create-subscription call. Defaulting to false + // routes those users to the honest "contact support" copy instead. + it("defaults razorpay_configured=false when the API omits the flag (BUG-P088 fail-closed)", async () => { const m = installFetch() m.mockResolvedValueOnce(jsonResponse({ ok: true, tier: 'hobby', subscription_status: 'none' })) const r = await fetchBilling() + expect(r.billing.razorpay_configured).toBe(false) + expect(r.billing.status).toBe('none') + }) + + // P2-19 (kept): the explicit wire value must still be honored when the + // server says razorpay_configured=true. A freshly-claimed paid team has + // no subscription yet (status='none') but Razorpay IS configured — the + // API surfaces the explicit flag so checkout is not suppressed. + it("honors an explicit razorpay_configured=true even when subscription_status='none'", async () => { + const m = installFetch() + m.mockResolvedValueOnce(jsonResponse({ + ok: true, tier: 'hobby', subscription_status: 'none', razorpay_configured: true, + })) + const r = await fetchBilling() expect(r.billing.razorpay_configured).toBe(true) expect(r.billing.status).toBe('none') }) diff --git a/src/api/index.ts b/src/api/index.ts index 1dec3a9..4369993 100644 --- a/src/api/index.ts +++ b/src/api/index.ts @@ -1780,10 +1780,18 @@ function mapBillingState(r: BillingStateResp): BillingDetails { // `subscription_status !== 'none'`. That conflates "the team already // has a subscription" with "checkout is available" — a freshly-claimed // paid team (subscription_status='none') would see checkout suppressed - // even though Razorpay is configured. Prefer the explicit wire field - // when the API sends it; otherwise default to `true` (Razorpay is - // configured in production) so checkout is never wrongly hidden. - razorpay_configured: r.razorpay_configured ?? true, + // even though Razorpay is configured. Prefer the explicit wire field. + // + // BUG-P088 (P1, 2026-05-29): the previous default `?? true` was + // optimistic — an older API build that omits the flag (or a partial + // outage where the field is dropped) would render the upgrade button + // as if Razorpay were live. Per CLAUDE.md memory + // `project_razorpay_recurring_not_enabled.md`, Razorpay recurring is + // NOT enabled on the prod account, so clicking that button surfaces a + // raw 502 from Razorpay's create-subscription call. Default FAIL-CLOSED + // (false) — when the server doesn't say, hide the button. Honest copy + // ("contact support to upgrade") beats a button that 502s. + razorpay_configured: r.razorpay_configured ?? false, subscription_status: r.subscription_status, payment_last4: r.payment_method?.last4, payment_network: r.payment_method?.brand, diff --git a/src/pages/CheckoutPage.test.tsx b/src/pages/CheckoutPage.test.tsx index 898e33f..3c87efc 100644 --- a/src/pages/CheckoutPage.test.tsx +++ b/src/pages/CheckoutPage.test.tsx @@ -1,90 +1,336 @@ -/* CheckoutPage.test.tsx — /app/checkout Razorpay session bootstrap. */ +/* CheckoutPage.test.tsx — coverage for the W12 funnel landing page + + * BUG-P111 / BUG-P112 / BUG-P013 / BUG-P088 / BUG-P121 auth-gate + + * cache-reset fixes (2026-05-29). + * + * The /app/checkout page reads ?plan= and ?frequency= from the URL, + * POSTs to /api/v1/billing/checkout, and either redirects to Razorpay + * or surfaces one of: invalid params, billing-not-configured fallback, + * email-not-verified recovery banner, generic error card, OR — for the + * BUG-P111 fix — redirects to /login?next=… when the second-layer auth + * gate trips. + * + * Tests pin every Status branch in the union — these are the surfaces + * a marketing-page click actually lands on. The unauth + cache-reset + * tests are the new gates that protect against a stale-localStorage JWT + * or back-cache restoration silently reaching a LIVE Razorpay + * subscription URL. */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' import { render, screen, waitFor, cleanup } from '@testing-library/react' import { MemoryRouter } from 'react-router-dom' -import { CheckoutPage } from './CheckoutPage' +import { + CheckoutPage, + buildLoginRedirect, + clearCheckoutCache, + CHECKOUT_CACHE_KEY_PREFIX, +} from './CheckoutPage' vi.mock('../api', async () => { const actual = await vi.importActual('../api') - return { ...actual, createCheckout: vi.fn() } + return { + ...actual, + createCheckout: vi.fn(), + // Default to an authenticated user (token present). Individual tests + // override this via mockReturnValue to exercise the unauth gate. This + // stays closer to production behaviour than mocking localStorage + // directly — the page reads getToken(), not localStorage. + getToken: vi.fn(() => 'test-token'), + } }) vi.mock('../hooks/useDashboardCtx', () => ({ - useDashboardCtx: () => ({ me: { user: { email: 'founder@acme.dev' } } }), + useDashboardCtx: () => ({ + me: { user: { id: 'u1', email: 'aanya@acme.dev', tier: 'free', team_id: 't1', created_at: '', display_name: '', role: 'owner' }, team: null }, + meErr: null, + meLoading: false, + env: 'production', + envs: ['production'], + counts: { resources: 0, deployments: 0, vault: 0, team: 1 }, + }), })) import * as api from '../api' -function renderAt(search: string) { - return render( - - - , - ) +// Spy on window.location.assign — the page imperatively navigates on +// successful checkout AND on the BUG-P111 unauth-redirect. +let originalLocation: Location | null = null +let assignedTo: string | null = null +function installLocationSpy() { + assignedTo = null + if (!originalLocation) originalLocation = window.location + const mock: any = { + ...originalLocation, + assign: (v: string) => { assignedTo = v }, + href: 'http://localhost/', + origin: 'http://localhost', + pathname: '/app/checkout', + search: '', + } + Object.defineProperty(window, 'location', { configurable: true, writable: true, value: mock }) +} +function restoreLocation() { + if (originalLocation) { + try { Object.defineProperty(window, 'location', { configurable: true, writable: true, value: originalLocation }) } catch {} + } } beforeEach(() => { vi.clearAllMocks() - delete (window as any).location - ;(window as any).location = { assign: vi.fn(), origin: 'http://localhost' } + // Re-arm the default getToken mock for each test so a previous test's + // unauth override doesn't bleed into the next test's setup. + ;(api.getToken as any).mockReturnValue('test-token') + installLocationSpy() + // Wipe any localStorage residue so the cache-reset tests start clean. + try { localStorage.clear() } catch {} +}) +afterEach(() => { + cleanup() + restoreLocation() + try { localStorage.clear() } catch {} }) -afterEach(() => cleanup()) + +function renderAt(url: string) { + return render( + + + , + ) +} describe('CheckoutPage', () => { - it('rejects a missing plan param', async () => { - ;(api.createCheckout as any).mockReturnValue(new Promise(() => {})) - renderAt('') - await waitFor(() => expect(screen.getByTestId('checkout-invalid')).toBeTruthy()) - expect(screen.getByTestId('checkout-invalid').textContent).toContain('Missing required') + it('renders loading state on mount with valid params', async () => { + ;(api.createCheckout as any).mockImplementation(() => new Promise(() => {})) + renderAt('/app/checkout?plan=hobby&frequency=monthly') + expect(screen.getByTestId('checkout-loading')).toBeTruthy() + }) + + it('redirects to Razorpay short_url on success', async () => { + ;(api.createCheckout as any).mockResolvedValue({ short_url: 'https://rzp.io/abc' }) + renderAt('/app/checkout?plan=hobby&frequency=monthly') + await waitFor(() => expect(screen.getByTestId('checkout-redirecting')).toBeTruthy()) + expect(assignedTo).toBe('https://rzp.io/abc') + expect(api.createCheckout).toHaveBeenCalledWith('hobby', 'monthly') + }) + + it('surfaces error when short_url is missing from response', async () => { + ;(api.createCheckout as any).mockResolvedValue({}) + renderAt('/app/checkout?plan=hobby&frequency=monthly') + await waitFor(() => expect(screen.getByTestId('checkout-error')).toBeTruthy()) + expect(screen.getByTestId('checkout-error').textContent).toMatch(/missing short_url/i) + }) + + it('renders fallback panel on 503 billing_not_configured', async () => { + ;(api.createCheckout as any).mockRejectedValue({ status: 503, code: 'billing_not_configured' }) + renderAt('/app/checkout?plan=pro&frequency=monthly') + await waitFor(() => expect(screen.getByTestId('checkout-fallback')).toBeTruthy()) + expect(screen.getByTestId('checkout-fallback').textContent).toMatch(/Razorpay not yet configured/i) + }) + + // BUG-P112 server-side guard (PR-2): when the api detects a live key in + // a non-prod deployment it returns 503 billing_misconfigured. The SPA + // should render the same fallback panel (operator action required) so + // QA testers don't reach a real Razorpay subscription URL. + it('renders fallback panel on 503 billing_misconfigured (BUG-P112 server guard)', async () => { + ;(api.createCheckout as any).mockRejectedValue({ status: 503, code: 'billing_misconfigured' }) + renderAt('/app/checkout?plan=pro&frequency=monthly') + await waitFor(() => expect(screen.getByTestId('checkout-fallback')).toBeTruthy()) + expect(api.createCheckout).toHaveBeenCalled() + // No live Razorpay URL was reached — that's the load-bearing assertion. + expect(assignedTo).toBeNull() + }) + + it('renders email_not_verified branch on 403 with email_not_verified code', async () => { + ;(api.createCheckout as any).mockRejectedValue({ + status: 403, + code: 'email_not_verified', + message: 'verify your email', + }) + renderAt('/app/checkout?plan=pro&frequency=monthly') + await waitFor(() => expect(screen.getByTestId('checkout-email-not-verified')).toBeTruthy()) + }) + + it('renders generic error card on unknown rejection', async () => { + ;(api.createCheckout as any).mockRejectedValue(new Error('network down')) + renderAt('/app/checkout?plan=team&frequency=yearly') + await waitFor(() => expect(screen.getByTestId('checkout-error')).toBeTruthy()) + expect(screen.getByTestId('checkout-error').textContent).toMatch(/network down/) + }) + + it('renders error card with default message when rejection has no message', async () => { + ;(api.createCheckout as any).mockRejectedValue({}) + renderAt('/app/checkout?plan=hobby&frequency=monthly') + await waitFor(() => expect(screen.getByTestId('checkout-error')).toBeTruthy()) + expect(screen.getByTestId('checkout-error').textContent).toMatch(/Could not start checkout/i) + }) + + it('renders invalid state when plan is missing', async () => { + renderAt('/app/checkout') + expect(screen.getByTestId('checkout-invalid')).toBeTruthy() + expect(screen.getByTestId('checkout-invalid').textContent).toMatch(/Missing required \?plan=/i) expect(api.createCheckout).not.toHaveBeenCalled() }) - it('rejects an unknown plan', async () => { - renderAt('?plan=enterprise') - await waitFor(() => expect(screen.getByTestId('checkout-invalid').textContent).toContain('Unknown plan')) + it('renders invalid state when plan is unknown', async () => { + renderAt('/app/checkout?plan=bogus&frequency=monthly') + expect(screen.getByTestId('checkout-invalid')).toBeTruthy() + expect(screen.getByTestId('checkout-invalid').textContent).toMatch(/Unknown plan "bogus"/) }) - it('rejects an unknown frequency', async () => { - renderAt('?plan=pro&frequency=weekly') - await waitFor(() => expect(screen.getByTestId('checkout-invalid').textContent).toContain('Unknown frequency')) + it('renders invalid state when frequency is unknown', async () => { + renderAt('/app/checkout?plan=hobby&frequency=quarterly') + expect(screen.getByTestId('checkout-invalid')).toBeTruthy() + expect(screen.getByTestId('checkout-invalid').textContent).toMatch(/Unknown frequency "quarterly"/) }) - it('redirects to the Razorpay short_url on success', async () => { + it('defaults frequency to monthly when only plan is provided', async () => { ;(api.createCheckout as any).mockResolvedValue({ short_url: 'https://rzp.io/x' }) - renderAt('?plan=pro&frequency=monthly') - await waitFor(() => expect(screen.getByTestId('checkout-redirecting')).toBeTruthy()) - expect((window as any).location.assign).toHaveBeenCalledWith('https://rzp.io/x') - expect(api.createCheckout).toHaveBeenCalledWith('pro', 'monthly') + renderAt('/app/checkout?plan=pro') + await waitFor(() => expect(api.createCheckout).toHaveBeenCalledWith('pro', 'monthly')) }) - it('defaults frequency to monthly when omitted', async () => { - ;(api.createCheckout as any).mockResolvedValue({ short_url: 'https://rzp.io/y' }) - renderAt('?plan=hobby') - await waitFor(() => expect(api.createCheckout).toHaveBeenCalledWith('hobby', 'monthly')) + it('accepts hobby_plus and team and yearly variations', async () => { + ;(api.createCheckout as any).mockResolvedValue({ short_url: 'https://rzp.io/x' }) + renderAt('/app/checkout?plan=hobby_plus&frequency=yearly') + await waitFor(() => expect(api.createCheckout).toHaveBeenCalledWith('hobby_plus', 'yearly')) }) - it('errors when the response is missing short_url', async () => { - ;(api.createCheckout as any).mockResolvedValue({}) - renderAt('?plan=pro') - await waitFor(() => expect(screen.getByTestId('checkout-error').textContent).toContain('missing short_url')) + it('cancellation flag suppresses state set after unmount (loading branch)', async () => { + let resolveIt: any + ;(api.createCheckout as any).mockImplementation( + () => new Promise((r) => { resolveIt = r }), + ) + const { unmount } = renderAt('/app/checkout?plan=hobby&frequency=monthly') + unmount() + resolveIt({ short_url: 'https://rzp.io/late' }) + // No assertion failure = success: the post-unmount setState was guarded. + await new Promise((r) => setTimeout(r, 10)) }) - it('renders the fallback panel on 503 billing_not_configured', async () => { - ;(api.createCheckout as any).mockRejectedValue({ status: 503, code: 'billing_not_configured' }) - renderAt('?plan=pro') - await waitFor(() => expect(screen.getByTestId('checkout-fallback')).toBeTruthy()) + // ────────────────────────────────────────────────────────────────────── + // BUG-P111 / BUG-P112 / BUG-P013 — the load-bearing tests. + // ────────────────────────────────────────────────────────────────────── + + describe('BUG-P111 second-layer auth gate', () => { + // TestCheckoutPage_UnauthRedirectsToLogin from the brief. + it('redirects an unauthenticated user to /login?next=… and NEVER calls createCheckout', async () => { + ;(api.getToken as any).mockReturnValue(null) + renderAt('/app/checkout?plan=hobby&frequency=monthly') + // The component flips to the unauthenticated state synchronously + // and the effect issues window.location.assign before any await. + await waitFor(() => expect(screen.getByTestId('checkout-unauthenticated')).toBeTruthy()) + expect(assignedTo).not.toBeNull() + expect(assignedTo!.startsWith('/login?next=')).toBe(true) + // Load-bearing: the API was NEVER called, so no sub_* was minted. + expect(api.createCheckout).not.toHaveBeenCalled() + }) + + // TestCheckoutPage_PreservesNextParam from the brief. + it('preserves plan + frequency in the next= param so post-signin returns the user to the same checkout', async () => { + ;(api.getToken as any).mockReturnValue(null) + renderAt('/app/checkout?plan=pro&frequency=yearly') + await waitFor(() => expect(assignedTo).not.toBeNull()) + const url = new URL(assignedTo!, 'http://localhost') + const next = url.searchParams.get('next') + expect(next).toBeTruthy() + // Decode the inner /app/checkout?plan=…&frequency=… without pinning + // the exact percent-encoding (URLSearchParams handles that). + const innerSearch = new URLSearchParams(next!.split('?')[1] ?? '') + expect(next!.split('?')[0]).toBe('/app/checkout') + expect(innerSearch.get('plan')).toBe('pro') + expect(innerSearch.get('frequency')).toBe('yearly') + }) + + it('preserves frequency=monthly when only plan is provided', async () => { + ;(api.getToken as any).mockReturnValue(null) + renderAt('/app/checkout?plan=hobby') + await waitFor(() => expect(assignedTo).not.toBeNull()) + const url = new URL(assignedTo!, 'http://localhost') + const next = url.searchParams.get('next') + const innerSearch = new URLSearchParams(next!.split('?')[1] ?? '') + expect(innerSearch.get('plan')).toBe('hobby') + expect(innerSearch.get('frequency')).toBe('monthly') + }) + + // Mid-flight token rejection (e.g. logged out from another tab) — + // the page tags 'unauthenticated' so a future analytics hook can + // observe the path. The global call() wrapper already issues the + // server-side invalidation + redirect; the local guard prevents a + // double-redirect race and keeps the assertion testable. + it('flips to unauthenticated on a mid-flight 401 from the API', async () => { + ;(api.createCheckout as any).mockRejectedValue({ status: 401, code: 'unauthorized' }) + renderAt('/app/checkout?plan=hobby&frequency=monthly') + await waitFor(() => expect(screen.getByTestId('checkout-unauthenticated')).toBeTruthy()) + }) }) - it('renders the verify-email banner on a 403 email_not_verified error', async () => { - ;(api.createCheckout as any).mockRejectedValue({ status: 403, code: 'email_not_verified' }) - renderAt('?plan=pro') - await waitFor(() => expect(screen.getByTestId('checkout-email-not-verified')).toBeTruthy()) + // TestCheckoutPage_ClearsCachedSubOnLogout from the brief. + describe('BUG-P121/P122 cache-reset on logout', () => { + it('clearCheckoutCache removes every key with CHECKOUT_CACHE_KEY_PREFIX', () => { + localStorage.setItem(CHECKOUT_CACHE_KEY_PREFIX + 'short_url', 'sub_Sv96Mt2n8nnDYL') + localStorage.setItem(CHECKOUT_CACHE_KEY_PREFIX + 'subscription_id', 'sub_xxx') + // Unrelated key must survive the purge. + localStorage.setItem('instanode.token', 'tok_kept') + clearCheckoutCache() + expect(localStorage.getItem(CHECKOUT_CACHE_KEY_PREFIX + 'short_url')).toBeNull() + expect(localStorage.getItem(CHECKOUT_CACHE_KEY_PREFIX + 'subscription_id')).toBeNull() + expect(localStorage.getItem('instanode.token')).toBe('tok_kept') + }) + + it('clearCheckoutCache is idempotent (no-op when the cache is already empty)', () => { + // Should not throw on a clean cache. + expect(() => clearCheckoutCache()).not.toThrow() + }) + + it('clearCheckoutCache survives a localStorage throw (defence-in-depth)', () => { + const orig = Object.getOwnPropertyDescriptor(window, 'localStorage') + try { + Object.defineProperty(window, 'localStorage', { + configurable: true, + get() { + throw new Error('storage disabled') + }, + }) + // Must not bubble — clearing is best-effort. + expect(() => clearCheckoutCache()).not.toThrow() + } finally { + if (orig) Object.defineProperty(window, 'localStorage', orig) + } + }) }) - it('surfaces a generic error otherwise', async () => { - ;(api.createCheckout as any).mockRejectedValue({ message: 'boom' }) - renderAt('?plan=team') - await waitFor(() => expect(screen.getByTestId('checkout-error').textContent).toContain('boom')) + // BUG-P088: razorpay_configured default is FAIL-CLOSED in the api layer. + // The CheckoutPage doesn't read razorpay_configured itself (it only + // surfaces the server's 503 billing_not_configured / + // billing_misconfigured envelope), but + // TestCheckoutPage_HidesButtonsWhenRazorpayUnconfigured in the brief is + // exercised in src/api/index.test.ts — the mapBillingState default + // flipped from `?? true` to `?? false`. Sentinel test below keeps the + // bug-id discoverable via grep from this file. + describe('BUG-P088 razorpay_configured default (cross-ref)', () => { + it('see src/api/index.test.ts fetchBilling() for the wire-default coverage', () => { + expect(true).toBe(true) + }) + }) + + describe('buildLoginRedirect', () => { + it('builds /login?next= with both plan + frequency', () => { + const r = buildLoginRedirect('hobby', 'monthly') + const url = new URL(r, 'http://localhost') + expect(url.pathname).toBe('/login') + const next = url.searchParams.get('next')! + const innerSearch = new URLSearchParams(next.split('?')[1] ?? '') + expect(next.startsWith('/app/checkout?')).toBe(true) + expect(innerSearch.get('plan')).toBe('hobby') + expect(innerSearch.get('frequency')).toBe('monthly') + }) + + it('omits plan when null (defensive — a hand-typed URL with no plan)', () => { + const r = buildLoginRedirect(null, 'monthly') + const url = new URL(r, 'http://localhost') + const next = url.searchParams.get('next')! + const innerSearch = new URLSearchParams(next.split('?')[1] ?? '') + expect(innerSearch.has('plan')).toBe(false) + expect(innerSearch.get('frequency')).toBe('monthly') + }) }) }) diff --git a/src/pages/CheckoutPage.tsx b/src/pages/CheckoutPage.tsx index 3030d47..6b6da47 100644 --- a/src/pages/CheckoutPage.tsx +++ b/src/pages/CheckoutPage.tsx @@ -1,4 +1,4 @@ -/* CheckoutPage — W12 funnel fix (C1). +/* CheckoutPage — W12 funnel fix (C1) + BUG-P111/P112/P013/P121 hardening. * * The /pricing page CTAs used to point at `/checkout?plan=...&frequency=...`, * a route App.tsx never registered. The catch-all `*` route silently bounced @@ -7,21 +7,60 @@ * /app/checkout (under AuthGate, matching the rest of the auth-required * dashboard surface) and update the PricingPage CTAs to point at it. * + * BUG-P111/P112/P113 (P0 CRITICAL, 2026-05-29) — defence-in-depth auth gate. + * QA found that visiting /app/checkout/?plan=hobby while unauthenticated + * still ended at a LIVE-mode Razorpay subscription page (sub_Sv96Mt2n8nnDYL). + * The App-level AuthGate (App.tsx) only checks getToken() — a stale + * localStorage JWT (e.g. a previous session that the server has since + * invalidated) passes the gate but is rejected by /api/v1/billing/checkout + * with 401. Without an explicit check here, any state in the URL/back-cache + * could short-circuit to a real Razorpay subscription URL. This page now + * performs its OWN getToken() check BEFORE the createCheckout call. If + * unauth: redirect to /login?next= via window.location.assign + * (fail closed). The AuthGate above is the belt; this is the suspenders. + * + * BUG-P013 (P1, 2026-05-29) — preserve next= on redirect. + * A logged-out user clicking /pricing → "Start hobby" used to land at /login + * with no return path; after signin they hit /app/ and had to re-find + * checkout. We now build a /login?next=&frequency=... URL so + * the LoginPage already-built `loc.state.from` + login-callback can round- + * trip the user back to the same plan. + * + * BUG-P121/P122 (P1, 2026-05-29) — no client-side caching of sub_*. + * QA reported the same Razorpay subscription URL appearing across plan + * changes and even across unauth sessions. We DO NOT cache the short_url + * locally — every call to createCheckout returns a freshly-minted sub_* or + * the F7-reused sub from the server. To defend against any FUTURE caching + * regression, we also register a logout hook that clears anything labelled + * with the well-known CHECKOUT_CACHE_KEY prefix. + * * Flow: - * 1. AuthGate (in App.tsx) ensures only logged-in users see this page. - * Unauthenticated visitors are redirected to /login with state.from set - * to /app/checkout?plan=... so post-login they land back here. - * 2. On mount, we read `plan` and `frequency` from the URL search params, + * 1. AuthGate (in App.tsx) is the FIRST layer — unauthenticated visitors + * with NO token at all get redirected to /login from the route guard + * before this component even mounts. + * 2. THIS COMPONENT is the SECOND layer — on mount we re-check + * getToken() and re-render as 'unauthenticated' (which assigns to + * /login?next=...) if it has been cleared between route mount and + * component effect (e.g. logout in another tab triggered a storage + * event). Without this, a browser back-button from Razorpay could + * momentarily display the in-flight redirecting state on an unauth + * browser. + * 3. On mount, we read `plan` and `frequency` from the URL search params, * validate them, then POST /api/v1/billing/checkout with the canonical * contract: {plan, plan_frequency}. The API contract lives in * api/internal/handlers/billing.go:checkoutRequest. - * 3. On success, response has `short_url` — we navigate the browser to it + * 4. On success, response has `short_url` — we navigate the browser to it * so Razorpay's hosted page collects payment. - * 4. On 503 billing_not_configured we render a fallback panel pointing + * 5. On 503 billing_not_configured we render a fallback panel pointing * users at instanode.dev/pricing with a support email — this happens * when the operator hasn't created the Razorpay plan_id for the tier * yet, which is normal during dev / before launch. - * 5. Any other failure is surfaced inline so the user isn't dropped on a + * 6. On 503 billing_misconfigured (BUG-P112 server-side guard, api + * ships in fix/billing-traffic-env-and-misconfig-detection) we render + * the same fallback panel — the operator pointed a non-prod + * deployment at a live Razorpay key and the server fast-failed before + * minting a real subscription. + * 7. Any other failure is surfaced inline so the user isn't dropped on a * blank page. * * Why /app/checkout and not /checkout: every authenticated action on this @@ -34,6 +73,7 @@ import { useEffect, useState } from 'react' import { useSearchParams } from 'react-router-dom' import * as api from '../api' +import { getToken, registerLogoutHook } from '../api' import { isEmailVerifiedError, VerifyEmailBanner } from '../components/VerifyEmailBanner' import { useDashboardCtx } from '../hooks/useDashboardCtx' @@ -44,6 +84,11 @@ import { useDashboardCtx } from '../hooks/useDashboardCtx' const RAZORPAY_FALLBACK_URL = 'https://instanode.dev/pricing' const SUPPORT_EMAIL = 'support@instanode.dev' +// Login redirect target — used when the second-layer auth gate trips. +// Lives outside the component so it survives test re-renders and matches +// the App-level AuthGate's destination exactly. +const LOGIN_PATH = '/login' + // Allowed plans + frequencies — must mirror the API's accepted values // (api/internal/handlers/billing.go:CreateCheckoutAPI). Anything outside // these sets is rejected before we even attempt the network call so the @@ -53,10 +98,42 @@ const ALLOWED_FREQUENCIES = ['monthly', 'yearly'] as const type AllowedPlan = (typeof ALLOWED_PLANS)[number] type AllowedFrequency = (typeof ALLOWED_FREQUENCIES)[number] +// CHECKOUT_CACHE_KEY_PREFIX — every localStorage key we might one day use +// to cache checkout state (subscription_id, short_url, etc.) MUST start +// with this prefix so the logout hook below can purge them in one sweep. +// Today the page does NOT cache the short_url locally — but the prefix + +// purge-on-logout guarantee is the defensive belt against a future +// regression that adds such caching (BUG-P121/P122). +export const CHECKOUT_CACHE_KEY_PREFIX = 'instanode.checkout.' + +// clearCheckoutCache — purge every localStorage entry whose key begins +// with CHECKOUT_CACHE_KEY_PREFIX. Idempotent. Called from the logout +// hook registered at module load below + exported for tests. +export function clearCheckoutCache(): void { + try { + if (typeof localStorage === 'undefined') return + const doomed: string[] = [] + for (let i = 0; i < localStorage.length; i++) { + const k = localStorage.key(i) + if (k && k.startsWith(CHECKOUT_CACHE_KEY_PREFIX)) doomed.push(k) + } + for (const k of doomed) localStorage.removeItem(k) + } catch { + // localStorage may throw in some embedded contexts (private browsing, + // storage quota) — clearing is best-effort. If we can't read it, we + // also can't write into it from this page, so there's nothing to leak. + } +} + +// Register the cache-purge on every logout. The hook registry is +// idempotent (registerLogoutHook dedupes by reference) so this is safe +// even if this module is re-evaluated under HMR / SSR hydration. +registerLogoutHook(clearCheckoutCache) + type Status = | { kind: 'loading' } | { kind: 'redirecting'; shortUrl: string } - | { kind: 'fallback' } // 503 billing_not_configured + | { kind: 'fallback' } // 503 billing_not_configured | billing_misconfigured | { kind: 'invalid'; reason: string } | { kind: 'error'; message: string } // B6-P0 008 (BUGBASH 2026-05-20): api returns 403 with an @@ -64,6 +141,12 @@ type Status = // to upgrade. Surface a recoverable banner with a resend-magic-link // button instead of dropping the user on a generic "Checkout failed". | { kind: 'email_not_verified' } + // BUG-P111/P013 (P0/P1, 2026-05-29): second-layer auth gate. Set when + // getToken() returns null at component mount. Renders nothing visible + // because the effect immediately window.location.assign('/login?next=…') + // — but holding the state lets tests assert "we did not call + // createCheckout" without timing flake. + | { kind: 'unauthenticated' } function isAllowedPlan(p: string | null): p is AllowedPlan { return p !== null && (ALLOWED_PLANS as readonly string[]).includes(p) @@ -72,6 +155,24 @@ function isAllowedFrequency(f: string | null): f is AllowedFrequency { return f !== null && (ALLOWED_FREQUENCIES as readonly string[]).includes(f) } +// buildLoginRedirect — assemble the /login?next= URL that +// preserves the plan + frequency the user was about to buy (BUG-P013). +// Exported for unit testing. +export function buildLoginRedirect(plan: string | null, frequency: string): string { + // Reconstruct the canonical /app/checkout path with the same plan + + // frequency the user was on so the LoginCallback round-trip can drop + // them back here. Use URLSearchParams so future query-param additions + // (e.g. ?promo=) get encoded correctly without hand-formatted ampersands. + const search = new URLSearchParams() + if (plan) search.set('plan', plan) + search.set('frequency', frequency) + const next = `/app/checkout?${search.toString()}` + // /login?next= — LoginPage reads loc.state.from in the React + // Router path, but the OAuth + magic-link round-trips drop state, so we + // surface `next` as a query param too. The login flow normalizes both. + return `${LOGIN_PATH}?next=${encodeURIComponent(next)}` +} + export function CheckoutPage() { const [params] = useSearchParams() const planRaw = params.get('plan') @@ -83,6 +184,26 @@ export function CheckoutPage() { useEffect(() => { let cancelled = false + // ────────────────────────────────────────────────────────────────────── + // BUG-P111 second-layer auth gate (P0). + // The App-level AuthGate (App.tsx) already redirects token-less users + // to /login before this component mounts. BUT: a stale JWT in + // localStorage passes that gate; the server then 401s — and in the + // intervening millisecond the user has seen "Creating your Razorpay + // checkout session…" + we have a real outbound network call. Worse, + // any back/forward-cache restoration of a previously rendered + // 'redirecting' state could surface a leaked sub_* URL to an unauth + // browser (BUG-P121/P122). Re-check explicitly here, before any + // network call, and fail CLOSED with a redirect to /login?next=… so + // the user round-trips to their selected plan post-signin (BUG-P013). + // ────────────────────────────────────────────────────────────────────── + if (!getToken()) { + setStatus({ kind: 'unauthenticated' }) + const dest = buildLoginRedirect(planRaw, frequencyRaw) + window.location.assign(dest) + return + } + // Validate query params before hitting the API. The pricing CTAs always // send valid values, but a hand-typed URL or a stale bookmark could // arrive here with garbage — surface that honestly instead of bouncing. @@ -121,10 +242,26 @@ export function CheckoutPage() { setStatus({ kind: 'error', message: 'Checkout response missing short_url.' }) } catch (e: any) { if (cancelled) return + // 401 from the API: the user's token was rejected mid-flight (e.g. + // logged out from another tab, server-side jti invalidation). The + // global call() wrapper already calls handle401() which fires + // clearToken() + redirects to /login?session_expired=1. We tag the + // state so a test (or a future onAuthError analytics hook) can + // observe the path without us doing a second redirect on top of + // the wrapper's. + if (e?.status === 401) { + setStatus({ kind: 'unauthenticated' }) + return + } // 503 billing_not_configured is the documented path when the // operator hasn't created the Razorpay plan_id for this tier yet. - // Render the friendly fallback instead of a raw error banner. - if (e?.status === 503 && e?.code === 'billing_not_configured') { + // 503 billing_misconfigured (BUG-P112 server-side guard) is the + // sibling — operator wired a live Razorpay key to a non-prod + // deployment. Both render the same fallback panel. + if ( + e?.status === 503 && + (e?.code === 'billing_not_configured' || e?.code === 'billing_misconfigured') + ) { setStatus({ kind: 'fallback' }) return } @@ -151,6 +288,16 @@ export function CheckoutPage() {

Checkout

+ {status.kind === 'unauthenticated' && ( + // We've already issued window.location.assign('/login?next=…') + // synchronously in the effect — this marker is visible for the + // brief flash before the browser navigates, AND it gives tests + // a stable assertion target. Use a neutral message so a screen + // reader doesn't read "Checkout failed" during the redirect. +

+ Sign in to continue to checkout… +

+ )} {status.kind === 'loading' && (

Creating your Razorpay checkout session… diff --git a/src/pages/LoginPage.test.tsx b/src/pages/LoginPage.test.tsx index c9bc2f5..e3739b8 100644 --- a/src/pages/LoginPage.test.tsx +++ b/src/pages/LoginPage.test.tsx @@ -24,6 +24,9 @@ function renderAt(url = '/login') { } /> APP HOME} /> + {/* BUG-P013 round-trip route — when /login?next=/app/checkout?… + preserves the plan + frequency, post-signin must land here. */} + CHECKOUT PAGE} /> , ) @@ -128,4 +131,43 @@ describe('LoginPage', () => { await userEvent.click(screen.getByTestId('login-submit')) await waitFor(() => expect(screen.getByTestId('login-error').textContent).toContain('network down')) }) + + // BUG-P013 (P1, 2026-05-29) — PAT signin with ?next=/app/checkout?... must + // round-trip the user back to the checkout page they were on, NOT the + // generic /app dashboard. The CheckoutPage's BUG-P111 fix calls + // window.location.assign('/login?next=…') which drops React Router + // state, so LoginPage MUST read the query param. + it('honors ?next= and round-trips to /app/checkout after PAT signin', async () => { + ;(api.fetchMe as any).mockResolvedValue({ ok: true }) + ;(window as any).location.search = '?next=%2Fapp%2Fcheckout%3Fplan%3Dhobby%26frequency%3Dmonthly' + renderAt('/login?next=%2Fapp%2Fcheckout%3Fplan%3Dhobby%26frequency%3Dmonthly') + await userEvent.click(screen.getByTestId('toggle-token-form')) + await userEvent.type(screen.getByTestId('token-input'), 'ink_secret') + await userEvent.click(screen.getByTestId('login-submit')) + await waitFor(() => expect(screen.getByText('CHECKOUT PAGE')).toBeTruthy()) + }) + + // Open-redirect safety — never honour an absolute external URL even + // if a phishing link drops it into ?next=. + it('rejects ?next=https://evil.com and falls back to /app', async () => { + ;(api.fetchMe as any).mockResolvedValue({ ok: true }) + ;(window as any).location.search = '?next=https%3A%2F%2Fevil.com' + renderAt('/login?next=https%3A%2F%2Fevil.com') + await userEvent.click(screen.getByTestId('toggle-token-form')) + await userEvent.type(screen.getByTestId('token-input'), 'ink_secret') + await userEvent.click(screen.getByTestId('login-submit')) + await waitFor(() => expect(screen.getByText('APP HOME')).toBeTruthy()) + }) + + // Protocol-relative URLs are equally dangerous (//evil.com → browser + // treats as scheme-inherit absolute). Reject those too. + it('rejects protocol-relative ?next=//evil.com and falls back to /app', async () => { + ;(api.fetchMe as any).mockResolvedValue({ ok: true }) + ;(window as any).location.search = '?next=%2F%2Fevil.com' + renderAt('/login?next=%2F%2Fevil.com') + await userEvent.click(screen.getByTestId('toggle-token-form')) + await userEvent.type(screen.getByTestId('token-input'), 'ink_secret') + await userEvent.click(screen.getByTestId('login-submit')) + await waitFor(() => expect(screen.getByText('APP HOME')).toBeTruthy()) + }) }) diff --git a/src/pages/LoginPage.tsx b/src/pages/LoginPage.tsx index 57ea752..dc761c1 100644 --- a/src/pages/LoginPage.tsx +++ b/src/pages/LoginPage.tsx @@ -77,7 +77,22 @@ export function LoginPage() { setToken(token.trim()) try { await fetchMe() - const dest = loc.state?.from && loc.state.from !== '/login' ? loc.state.from : '/app' + // BUG-P013 (P1, 2026-05-29): when CheckoutPage's second-layer auth + // gate redirects an unauth user it issues `/login?next=` (a hard window.location.assign, so React Router state is + // dropped). Read `next=` from the query string FIRST, fall back to + // `loc.state.from` (the App-level AuthGate path), then `/app`. + // /login itself is never a valid landing — reject so a stale + // bookmark doesn't trap the user in a loop. + const queryNext = typeof window !== 'undefined' + ? new URLSearchParams(window.location.search).get('next') + : null + const candidate = queryNext ?? loc.state?.from ?? '/app' + // Only honour relative same-origin paths so a forged + // /login?next=https://evil.com cannot phish the post-signin nav. + const dest = candidate.startsWith('/') && !candidate.startsWith('//') && candidate !== '/login' + ? candidate + : '/app' navigate(dest, { replace: true }) } catch (e: any) { clearToken()