diff --git a/changelog.d/next/48.fixed.md b/changelog.d/next/48.fixed.md new file mode 100644 index 0000000000..da0c93fde6 --- /dev/null +++ b/changelog.d/next/48.fixed.md @@ -0,0 +1 @@ +A link that opens the Shop with a pubky.app session hand-off now asks "Continue as this account?" and names the pubky before it signs the tab in. Bitkit sign-in no longer leaves an approved session behind when the browser cannot save it, and it refuses an approval narrower than the Shop permissions. diff --git a/docs/adr/0029-vibe-session-consumer.md b/docs/adr/0029-vibe-session-consumer.md index e9ff2abc5d..8adf88e225 100644 --- a/docs/adr/0029-vibe-session-consumer.md +++ b/docs/adr/0029-vibe-session-consumer.md @@ -34,7 +34,7 @@ Both are read as literal `process.env.NEXT_PUBLIC_*` so Next inlines them. They 1. If a persisted `sessionExport` exists, restore it with the existing retry loop (`HomeserverService.restoreSession` + homeserver environment check). 2. If there is **no** persist, or persist fails with a **definitive auth** error (`AppError` auth category except wrong-environment, or `isPubkyExpiredError`), and consumer mode is on: obtain an export via **fragment → bridge**, then run the same `restoreSession` path. -3. Fragment `#s=` is consumed on the first client pass (`instrumentation-client.ts` via `consumeFragmentSessionExport`) and taken once by restore (`takeFragmentSessionExport`). The hash is stripped with `history.replaceState` even when consumer mode is off. +3. Fragment `#s=` is consumed on the first client pass (`instrumentation-client.ts` via `consumeFragmentSessionExport`) and taken once by restore (`takeFragmentSessionExport`). The hash is stripped with `history.replaceState` even when consumer mode is off. A fragment export that restores is **not applied until the user confirms it**: the Controller's `confirmSessionHandoff` opens `DialogSessionHandoff`, which names the pubky the link would sign the tab in as. Only Continue applies it. Not me, Escape, or a logout declines it; a declined hand-off also suppresses the bridge leg for the tab, so the refused identity is not applied silently by another route. Application declines every hand-off when no confirmer is passed. 4. Bridge: hidden iframe to `${bridgeOrigin}/session-bridge`, `sandbox="allow-scripts allow-same-origin"`. After `load`, post `{ type: 'pubky-session-request', v: 1 }` to `bridgeOrigin`. Accept a reply only if `event.origin === bridgeOrigin && event.source === iframe.contentWindow && data.v === 1`. Load timeout 15 s; reply timeout 3 s from load; one request per load; `AbortSignal`; cleanup of listener / iframe / timers on every path; late messages ignored. ### Contract @@ -75,6 +75,7 @@ RouteGuard and auth-store rehydrate both call `shouldAttemptSessionRestore` (`sr - The homeserver **HttpOnly cookie** binds identity. The consumer never reads or copies that cookie. - Accept `postMessage` only from `bridgeOrigin` and the iframe `contentWindow`. Never `'*'`. - Strip `#s=` before any auth-dependent routing or network. +- A `#s=` hand-off needs the user's confirmation of the named pubky. Any page can link to the Shop with `#s=` for a session whose cookie this browser holds, including one a third party may have planted through a cross-site sign-in, and nothing binds the link to this device: the board opens the Shop without a Shop-issued state or nonce to echo. Binding the hand-off to a same-device nonce needs the board to carry that nonce; until it does, the prompt is the control. - The iframe sandbox allows scripts and same-origin so the bridge page keeps the pubky-app origin; it cannot navigate the parent. ## Consequences diff --git a/docs/ecommerce/step-up-approval.md b/docs/ecommerce/step-up-approval.md index 3b0d3238eb..b3043cefe5 100644 --- a/docs/ecommerce/step-up-approval.md +++ b/docs/ecommerce/step-up-approval.md @@ -103,6 +103,14 @@ Approvals per payment method and feature under Option C: - Manual, blocks launch: test what Pubky Ring displays for an empty-capabilities pubkyauth request (`caps=''`); if it does not visibly distinguish empty from wide, file a Ring issue before launch (the QR/phish-swap row's "low-value empty-caps prompt" reasoning depends on this). - marketplace-service: `create_session` accepts an empty-capabilities token; posting identical bytes twice returns 401 on the second call (integration-level replay test — current `auth.rs:269–374` tests cover verification, not the single-use INSERT path). +## Grant (Bitkit) sessions need no step-up + +A Bitkit sign-in (`pubkyauth://signin_grant`) requests exactly `CAPABILITIES`, and the Shop refuses anything else: `AuthApplication.assertFullGrantSession` signs out and rejects an approved grant session whose `info.capabilities` do not match `capabilitiesMatchFullGrant`, and a stored grant session that restores narrower is signed out and its record removed. Every live grant session therefore already holds `/priv/pubky.app/:rw`, so `canCurrentSessionWrite(PRIVATE_APP_DATA_PATH)` is true and the capability-based `needs_reauth` state cannot occur for it. + +The other `needs_reauth` trigger is a 401/403 on the private document. For a grant session with the full grant, that refusal means the grant itself is no longer honored (revoked in Bitkit, or expired). A step-up approval widens scope; it cannot repair a refused grant. `CommerceApplication.isPrivateAccessDenied` therefore does not report `needs_reauth` for a grant session: watchlist sync reports `error` (the outbox job stays pending) and receipt publication reports `unavailable`, both retried on the next load. + +`MarketplaceReauthDialog` renders only in the `needs_reauth` state, so it never opens for a grant session. A delegated-grant step-up QR (contract row R3.9a) is not built: no state reaches it. `AuthController.getStepUpAuthUrl` still refuses a grant session before any Ring flow starts, so a future grant path that skips the full-grant checks gets an error in the dialog, not a Ring step-up that would replace the grant session with a cookie session while its grant record stays stored. + ## Verification that differed from the brief 1. **`signinWithAuthToken` does not exist.** `pubky.d.ts` has no AuthToken→Session API (only `AuthFlow.awaitApproval` line 188, `awaitToken` line 198, `Pubky.restoreSession` line 831, `Signer.signin` line 1294). Option A's "same bytes sign in to the homeserver" requires re-implementing protocol internals, not an SDK call. diff --git a/src/app/layout.tsx b/src/app/layout.tsx index 3dbbc0dec6..10e4f83356 100644 --- a/src/app/layout.tsx +++ b/src/app/layout.tsx @@ -8,6 +8,7 @@ import { Metadata } from '@/molecules/Metadata/Metadata'; import { StructuredData } from '@/molecules/StructuredData/StructuredData'; import { Toaster } from '@/molecules/Toaster/Toaster'; import { CoordinatorsManager } from '@/organisms/CoordinatorsManager/CoordinatorsManager'; +import { DialogSessionHandoff } from '@/organisms/DialogSessionHandoff/DialogSessionHandoff'; import { DialogSignIn } from '@/organisms/DialogSignIn/DialogSignIn'; import { Header } from '@/organisms/Header/Header'; import { DatabaseProvider } from '@/providers/DatabaseProvider/DatabaseProvider'; @@ -56,6 +57,8 @@ export default function RootLayout({ children }: { children: React.ReactNode }) + {/* Outside RouteGuardProvider: it waits on the restore this dialog answers. */} + diff --git a/src/components/molecules/QrCodeSlot/QrCodeSlot.dom.test.tsx b/src/components/molecules/QrCodeSlot/QrCodeSlot.dom.test.tsx new file mode 100644 index 0000000000..bf9880f0fa --- /dev/null +++ b/src/components/molecules/QrCodeSlot/QrCodeSlot.dom.test.tsx @@ -0,0 +1,29 @@ +import { render } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { QrCodeSlot } from './QrCodeSlot'; + +vi.mock('next/image', () => ({ + __esModule: true, + default: ({ src, alt }: { src: string; alt: string }) => {alt}, +})); + +const RELAY_SECRET = 'c2VjcmV0LWNoYW5uZWwta2V5LWZvci10aGlzLWZsb3c'; +const AUTH_URL = `pubkyauth://signin_grant?caps=%2Fpub%2Fpubky.app%2F%3Arw&relay=https%3A%2F%2Frelay.example%2Finbox&secret=${RELAY_SECRET}&cid=shop.pubky.app`; + +describe('QrCodeSlot with the real QR renderer', () => { + it('keeps the relay secret out of the DOM markup', () => { + const { container } = render( + , + ); + + expect(container.querySelector('svg')).not.toBeNull(); + expect(container.innerHTML).not.toContain(RELAY_SECRET); + expect(container.innerHTML).not.toContain('pubkyauth://'); + }); +}); diff --git a/src/components/molecules/QrCodeSlot/QrCodeSlot.test.tsx b/src/components/molecules/QrCodeSlot/QrCodeSlot.test.tsx index 0dfd5f8527..da4a899a3b 100644 --- a/src/components/molecules/QrCodeSlot/QrCodeSlot.test.tsx +++ b/src/components/molecules/QrCodeSlot/QrCodeSlot.test.tsx @@ -77,7 +77,11 @@ describe('QrCodeSlot', () => { const qr = screen.getByTestId('qrcode-svg'); expect(qr).toHaveAttribute('data-value', 'auth-url'); expect(qr).toHaveAttribute('width', '176'); - expect(screen.getByTestId('qr-auth-url')).toHaveAttribute('data-auth-url', 'auth-url'); + const slotAttributeValues = Array.from( + screen.getByTestId('qr-auth-url').attributes, + (attribute) => attribute.value, + ); + expect(slotAttributeValues).not.toContain('auth-url'); const ringLogo = screen.getByAltText('Pubky Ring'); expect(ringLogo).toHaveAttribute('src', '/images/ring-logo.svg'); diff --git a/src/components/molecules/QrCodeSlot/QrCodeSlot.test.tsx.snap b/src/components/molecules/QrCodeSlot/QrCodeSlot.test.tsx.snap index 1e7fc491aa..27fd1b036f 100644 --- a/src/components/molecules/QrCodeSlot/QrCodeSlot.test.tsx.snap +++ b/src/components/molecules/QrCodeSlot/QrCodeSlot.test.tsx.snap @@ -4,7 +4,6 @@ exports[`QrCodeSlot - Snapshots > matches snapshot for a non-default size 1`] =
matches snapshot for active QR with hover effe
matches snapshot for active QR without hover e
+ {showRingLogo && ( vi.fn()); +vi.mock('@/controllers/auth/auth', () => ({ + AuthController: { answerSessionHandoff }, +})); + +const PUBKY = 'o1gg8yc7mj4ksrzr6ms3s5rs8h7bo8y7ohcq7j88wbkm7ns7tuxo' as Pubky; + +describe('DialogSessionHandoff', () => { + afterEach(() => { + act(() => useSessionHandoffStore.getState().setPendingPubky(null)); + answerSessionHandoff.mockClear(); + }); + + it('renders nothing while no hand-off waits', () => { + render(); + + expect(screen.queryByTestId('session-handoff-dialog')).not.toBeInTheDocument(); + }); + + it('names the account the link would sign in as', () => { + act(() => useSessionHandoffStore.getState().setPendingPubky(PUBKY)); + render(); + + expect(screen.getByRole('heading', { name: 'Continue as this account?' })).toBeInTheDocument(); + expect(screen.getByTestId('session-handoff-pubky')).toHaveTextContent('pubkyo1gg8yc7...7ns7tuxo'); + }); + + it('Continue accepts the hand-off', async () => { + act(() => useSessionHandoffStore.getState().setPendingPubky(PUBKY)); + render(); + + await userEvent.setup().click(screen.getByTestId('session-handoff-accept')); + + expect(answerSessionHandoff).toHaveBeenCalledExactlyOnceWith(true); + }); + + it('Not me declines the hand-off', async () => { + act(() => useSessionHandoffStore.getState().setPendingPubky(PUBKY)); + render(); + + await userEvent.setup().click(screen.getByTestId('session-handoff-decline')); + + expect(answerSessionHandoff).toHaveBeenCalledExactlyOnceWith(false); + }); + + it('dismissing the dialog declines the hand-off', async () => { + act(() => useSessionHandoffStore.getState().setPendingPubky(PUBKY)); + render(); + + await userEvent.setup().keyboard('{Escape}'); + + expect(answerSessionHandoff).toHaveBeenCalledExactlyOnceWith(false); + }); +}); diff --git a/src/components/organisms/DialogSessionHandoff/DialogSessionHandoff.tsx b/src/components/organisms/DialogSessionHandoff/DialogSessionHandoff.tsx new file mode 100644 index 0000000000..e359241590 --- /dev/null +++ b/src/components/organisms/DialogSessionHandoff/DialogSessionHandoff.tsx @@ -0,0 +1,58 @@ +'use client'; + +import { Button } from '@/atoms/Button/Button'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/atoms/Dialog/Dialog'; +import { Typography } from '@/atoms/Typography/Typography'; +import { useSessionHandoff } from '@/hooks/useSessionHandoff/useSessionHandoff'; +import { formatPublicKey, withPubkyPrefix } from '@/libs/utils/utils'; + +/** + * Asks before a `#s=` link signs this tab in. Closing the dialog any way other + * than Continue declines the hand-off. + */ +export function DialogSessionHandoff() { + const { pendingPubky, accept, decline } = useSessionHandoff(); + if (!pendingPubky) return null; + + return ( + { + if (!open) decline(); + }} + > + + + Continue as this account? + + The link you opened signs this browser in to the Shop. Continue only if you opened it from your own Pubky + account. + + + + {formatPublicKey({ key: pendingPubky, length: 16, includePrefix: true })} + + + + + + + + ); +} diff --git a/src/components/organisms/Marketplace/MarketplaceReauthDialog.test.tsx b/src/components/organisms/Marketplace/MarketplaceReauthDialog.test.tsx index 9b63005d41..c756ab2bc2 100644 --- a/src/components/organisms/Marketplace/MarketplaceReauthDialog.test.tsx +++ b/src/components/organisms/Marketplace/MarketplaceReauthDialog.test.tsx @@ -20,26 +20,19 @@ vi.mock('@/hooks/useStepUpReauth/useStepUpReauth', () => ({ useStepUpReauth: () => reauth, })); -const grant = vi.hoisted(() => ({ isGrantSession: false })); -vi.mock('@/hooks/useIsGrantSession/useIsGrantSession', () => ({ - useIsGrantSession: () => grant.isGrantSession, -})); - describe('MarketplaceReauthDialog', () => { beforeEach(() => { - grant.isGrantSession = false; vi.mocked(reauth.start).mockClear(); }); - it('grant session sees refusal not classic qr (step-up)', async () => { - grant.isGrantSession = true; + it('opening starts a fresh step-up flow and shows its QR', async () => { render(); await userEvent.setup().click(screen.getByRole('button', { name: 'Sign in again' })); - expect(screen.getByTestId('grant-session-refusal')).toBeInTheDocument(); - expect(screen.queryByLabelText('Copy authorization link')).not.toBeInTheDocument(); - expect(reauth.start).not.toHaveBeenCalled(); + expect(reauth.start).toHaveBeenCalledOnce(); + expect(screen.getByLabelText('Copy authorization link')).toBeInTheDocument(); + expect(screen.queryByTestId('grant-session-refusal')).not.toBeInTheDocument(); }); it('asks for a sign-in in product language and does not print capability paths', async () => { diff --git a/src/components/organisms/Marketplace/MarketplaceReauthDialog.tsx b/src/components/organisms/Marketplace/MarketplaceReauthDialog.tsx index 0a4c151298..7148605e8e 100644 --- a/src/components/organisms/Marketplace/MarketplaceReauthDialog.tsx +++ b/src/components/organisms/Marketplace/MarketplaceReauthDialog.tsx @@ -5,10 +5,8 @@ import { Copy, KeyRound, Loader2, RefreshCw, Smartphone } from 'lucide-react'; import { Button } from '@/atoms/Button/Button'; import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle, DialogTrigger } from '@/atoms/Dialog/Dialog'; import { Typography } from '@/atoms/Typography/Typography'; -import { useIsGrantSession } from '@/hooks/useIsGrantSession/useIsGrantSession'; import { useStepUpReauth } from '@/hooks/useStepUpReauth/useStepUpReauth'; import { Logger } from '@/libs/logger/logger'; -import { GrantSessionRefusal } from '@/molecules/GrantSessionRefusal/GrantSessionRefusal'; import { QrCodeSlot } from '@/molecules/QrCodeSlot/QrCodeSlot'; import { toast } from '@/molecules/Toaster/use-toast'; @@ -47,14 +45,13 @@ export function MarketplaceReauthDialog({ // Referencing `reauth.start`/`reauth.cancel` directly keeps the effect // dependency-stable: both are useCallback-memoized in the hook. const { start, cancel } = reauth; - const isGrantSession = useIsGrantSession(); useEffect(() => { if (open) { - if (!isGrantSession) start(); + start(); return; } cancel(); - }, [open, start, cancel, isGrantSession]); + }, [open, start, cancel]); const copyUrl = async () => { try { @@ -83,9 +80,7 @@ export function MarketplaceReauthDialog({ Sign in again for this device. - {isGrantSession ? ( - - ) : reauth.status === 'error' ? ( + {reauth.status === 'error' ? (
{reauth.errorMessage} diff --git a/src/components/organisms/Scan/Scan.test.tsx.snap b/src/components/organisms/Scan/Scan.test.tsx.snap index 1b6096cb52..e47234e829 100644 --- a/src/components/organisms/Scan/Scan.test.tsx.snap +++ b/src/components/organisms/Scan/Scan.test.tsx.snap @@ -51,7 +51,6 @@ exports[`Scan Components - Snapshots > ScanContent - Snapshots > matches snapsho > matches snapshot for the QR sign-in layout > { setIsRestoringSession: vi.fn(), init: vi.fn(), }); - const grantSession = () => - asOpaque({ grant: {}, info: { publicKey: asOpaque({ z32: () => 'user-pubky' }) } }); + const grantSession = (capabilities: string[] = CAPABILITIES.split(',')) => + asOpaque({ grant: {}, info: { publicKey: asOpaque({ z32: () => 'user-pubky' }), capabilities } }); it('reload restores grant session from store', async () => { const session = grantSession(); @@ -292,6 +293,38 @@ describe('AuthApplication', () => { expect(cookieRestoreSpy).not.toHaveBeenCalled(); }); + it('a stored grant narrower than the Shop grant is signed out and its record removed', async () => { + const session = grantSession(['/pub/pubky.app/:rw']); + vi.spyOn(HomeserverService, 'restoreGrantSession').mockResolvedValue(session); + const logoutSpy = vi.spyOn(HomeserverService, 'logout').mockResolvedValue(undefined); + const removeSpy = vi.spyOn(HomeserverService, 'removeGrantSession').mockResolvedValue(undefined); + + const result = await AuthApplication.restorePersistedSession({ authStore: grantStore() }); + + expect(result).toEqual({ status: 'signed-out' }); + expect(logoutSpy).toHaveBeenCalledWith({ session }); + expect(removeSpy).toHaveBeenCalledWith('rec-1'); + }); + + it('grant restore drops a #s= export captured on the same load', async () => { + vi.spyOn(HomeserverService, 'restoreGrantSession').mockRejectedValue(createAuthError()); + vi.spyOn(HomeserverService, 'removeGrantSession').mockResolvedValue(undefined); + vibeSessionFragment.resetFragmentSessionExportCache(); + window.history.replaceState(null, '', '/marketplace#s=handoff-export'); + try { + vibeSessionFragment.consumeFragmentSessionExport(); + expect(vibeSessionFragment.hasPendingFragmentSessionExport()).toBe(true); + + await AuthApplication.restorePersistedSession({ authStore: grantStore() }); + + expect(vibeSessionFragment.hasPendingFragmentSessionExport()).toBe(false); + expect(vibeSessionFragment.takeFragmentSessionExport()).toBeNull(); + } finally { + vibeSessionFragment.resetFragmentSessionExportCache(); + window.history.replaceState(null, '', '/'); + } + }); + it('restore never calls save', async () => { vi.spyOn(HomeserverService, 'restoreGrantSession').mockResolvedValue(grantSession()); vi.spyOn(HomeserverService, 'assertUserHomeserverAllowed').mockResolvedValue(undefined); @@ -619,15 +652,52 @@ describe('AuthApplication', () => { const restoreSpy = vi.spyOn(HomeserverService, 'restoreSession').mockResolvedValue(session); vi.spyOn(HomeserverService, 'assertUserHomeserverAllowed').mockResolvedValue(undefined); const authStore = createMockAuthStore(null); + const confirmSessionHandoff = vi.fn().mockResolvedValue(true); - const result = await AuthApplication.restorePersistedSession({ authStore }); + const result = await AuthApplication.restorePersistedSession({ authStore, confirmSessionHandoff }); expect(restoreSpy).toHaveBeenCalledOnce(); expect(restoreSpy).toHaveBeenCalledWith({ sessionExport: FRAGMENT_EXPORT }); + expect(confirmSessionHandoff).toHaveBeenCalledWith('user-pubky'); expect(vibeSessionBridge.requestFromBridge).not.toHaveBeenCalled(); expect(result).toEqual({ status: 'restored', session }); }); + it('a declined #s= hand-off restores nothing and turns the bridge off for the tab', async () => { + vi.mocked(vibeSessionConfig.getVibeSessionBridgeOrigin).mockReturnValue(BRIDGE); + vi.mocked(vibeSessionFragment.takeFragmentSessionExport).mockReturnValue(FRAGMENT_EXPORT); + vi.spyOn(HomeserverService, 'restoreSession').mockResolvedValue(liveSession()); + vi.spyOn(HomeserverService, 'assertUserHomeserverAllowed').mockResolvedValue(undefined); + const confirmSessionHandoff = vi.fn().mockResolvedValue(false); + try { + const result = await AuthApplication.restorePersistedSession({ + authStore: createMockAuthStore(null), + confirmSessionHandoff, + }); + + expect(confirmSessionHandoff).toHaveBeenCalledWith('user-pubky'); + expect(result).toEqual({ status: 'signed-out' }); + expect(vibeSessionBridge.requestFromBridge).not.toHaveBeenCalled(); + expect(vibeSessionAutoRestore.isVibeSessionAutoRestoreSuppressed()).toBe(true); + } finally { + vibeSessionAutoRestore.clearVibeSessionAutoRestoreSuppressed(); + } + }); + + it('a #s= hand-off is declined when no one can confirm it', async () => { + vi.mocked(vibeSessionConfig.getVibeSessionBridgeOrigin).mockReturnValue(BRIDGE); + vi.mocked(vibeSessionFragment.takeFragmentSessionExport).mockReturnValue(FRAGMENT_EXPORT); + vi.spyOn(HomeserverService, 'restoreSession').mockResolvedValue(liveSession()); + vi.spyOn(HomeserverService, 'assertUserHomeserverAllowed').mockResolvedValue(undefined); + try { + const result = await AuthApplication.restorePersistedSession({ authStore: createMockAuthStore(null) }); + + expect(result).toEqual({ status: 'signed-out' }); + } finally { + vibeSessionAutoRestore.clearVibeSessionAutoRestoreSuppressed(); + } + }); + it('restores from a bridge reply when consumer mode is on and nothing is persisted', async () => { vi.mocked(vibeSessionConfig.getVibeSessionBridgeOrigin).mockReturnValue(BRIDGE); vi.mocked(vibeSessionFragment.takeFragmentSessionExport).mockReturnValue(null); @@ -738,7 +808,10 @@ describe('AuthApplication', () => { vi.spyOn(HomeserverService, 'assertUserHomeserverAllowed').mockResolvedValue(undefined); const authStore = createMockAuthStore(null); - const result = await AuthApplication.restorePersistedSession({ authStore }); + const result = await AuthApplication.restorePersistedSession({ + authStore, + confirmSessionHandoff: async () => true, + }); expect(restoreSpy).toHaveBeenCalledWith({ sessionExport: FRAGMENT_EXPORT }); expect(vibeSessionBridge.requestFromBridge).not.toHaveBeenCalled(); diff --git a/src/core/application/auth/auth.ts b/src/core/application/auth/auth.ts index e1a603e4a2..f9db2ffe1f 100644 --- a/src/core/application/auth/auth.ts +++ b/src/core/application/auth/auth.ts @@ -9,7 +9,7 @@ import type { TSingleApprovalCeremonyHooks, TSingleApprovalResult, } from '@/application/auth/auth.types'; -import { CAPABILITIES } from '@/config/app'; +import { CAPABILITIES, capabilitiesMatchFullGrant } from '@/config/app'; import { getCommerceAdapterMode, isDurableCommerceMode } from '@/config/commerce'; import { ValidationErrorCode } from '@/libs/error/error.codes'; import { Err } from '@/libs/error/error.factories'; @@ -23,9 +23,10 @@ import { toAppError, } from '@/libs/error/error.utils'; import { HttpMethod } from '@/libs/http/http.types'; +import { Identity } from '@/libs/identity/identity'; import { Logger } from '@/libs/logger/logger'; import { sleep } from '@/libs/utils/utils'; -import { isVibeSessionBridgeLegSkipped } from '@/libs/vibe-session/auto-restore'; +import { isVibeSessionBridgeLegSkipped, suppressVibeSessionAutoRestore } from '@/libs/vibe-session/auto-restore'; import { requestFromBridge } from '@/libs/vibe-session/bridge'; import { getVibeId, getVibeSessionBridgeOrigin } from '@/libs/vibe-session/config'; import { isPubkyExpiredError } from '@/libs/vibe-session/expired'; @@ -81,7 +82,10 @@ export class AuthApplication { * @param authStore - The auth store object containing state and actions needed for restoration * @returns The restored session, or null if restoration failed */ - static async restorePersistedSession({ authStore }: TRestoreSessionParams): TRestoreSessionResult { + static async restorePersistedSession({ + authStore, + confirmSessionHandoff = async () => false, + }: TRestoreSessionParams): TRestoreSessionResult { // If a restoration is already in progress, return the existing promise if (this.restoreSessionPromise) { return await this.restoreSessionPromise; @@ -95,6 +99,9 @@ export class AuthApplication { try { return await this.restoreGrantSession(grantRecordId); } finally { + // Same bound as the cookie leg: a `#s=` captured on this load must + // not survive into a later restore after this one signs out. + discardFragmentSessionExport(); this.restoreSessionPromise = null; } })(); @@ -114,7 +121,7 @@ export class AuthApplication { // flag for the whole restore+finalization span so no leg can leave a loading gap. this.restoreSessionPromise = (async () => { try { - return await this.runSessionRestore({ persistedExport, consumerOrigin }); + return await this.runSessionRestore({ persistedExport, consumerOrigin, confirmSessionHandoff }); } finally { // The first restore decision of this page load has run (whichever leg // decided it) — drop any cached `#s=` export so a later same-tab @@ -130,9 +137,11 @@ export class AuthApplication { private static async runSessionRestore({ persistedExport, consumerOrigin, + confirmSessionHandoff, }: { persistedExport: string | null; consumerOrigin: string | undefined; + confirmSessionHandoff: (pubky: Pubky) => Promise; }): TRestoreSessionResult { let keepPersistedExport = false; @@ -159,7 +168,15 @@ export class AuthApplication { if (fragmentExport) { const fromFragment = await this.restoreSessionFromExport(fragmentExport); if (fromFragment.session) { - return { status: 'restored', session: fromFragment.session }; + // Any page can link here with a `#s=` for a session the browser holds a + // cookie for, including one a third party planted. Nothing binds the + // hand-off to this device, so the user confirms the identity first. + if (await confirmSessionHandoff(Identity.z32FromSession({ session: fromFragment.session }))) { + return { status: 'restored', session: fromFragment.session }; + } + // Declined: the bridge must not apply an identity the user just refused. + suppressVibeSessionAutoRestore(); + return this.unresolvedConsumerRestore(keepPersistedExport); } } @@ -193,6 +210,14 @@ export class AuthApplication { let session: Session | null = null; try { session = await HomeserverService.restoreGrantSession(recordId); + if (!capabilitiesMatchFullGrant(session.info.capabilities)) { + Logger.warn('Stored grant session does not hold the full Shop grant; removing its record'); + await HomeserverService.logout({ session }).catch((logoutError) => { + Logger.warn('Failed to sign out a narrow grant session', { logoutError }); + }); + await this.removeGrantSession(recordId); + return { status: 'signed-out' }; + } await HomeserverService.assertUserHomeserverAllowed({ publicKey: session.info.publicKey }); return { status: 'restored', session }; } catch (error) { @@ -362,6 +387,23 @@ export class AuthApplication { return HomeserverService.isGrantSession(session); } + /** + * A grant session holds exactly the Shop grant (`CAPABILITIES`), so it never + * needs a step-up re-approval. An approval for anything narrower is signed + * out and refused, the way the Ring token path refuses it. + */ + static async assertFullGrantSession(session: Session): Promise { + if (capabilitiesMatchFullGrant(session.info.capabilities)) return; + await HomeserverService.logout({ session }).catch((logoutError) => { + Logger.warn('Failed to sign out a narrow grant session', { logoutError }); + }); + throw Err.validation( + ValidationErrorCode.INVALID_INPUT, + 'This approval does not include the full Shop permission list. Scan again from Shop.', + { service: ErrorService.Homeserver, operation: 'assertFullGrantSession' }, + ); + } + static async saveGrantSession(session: Session): Promise { return await HomeserverService.saveGrantSession(session); } diff --git a/src/core/application/auth/auth.types.ts b/src/core/application/auth/auth.types.ts index 50a6a37ed8..fd0d7be25f 100644 --- a/src/core/application/auth/auth.types.ts +++ b/src/core/application/auth/auth.types.ts @@ -1,4 +1,5 @@ import { Keypair, type Session } from '@synonymdev/pubky'; +import type { Pubky } from '@/models/models.types'; import type { MarketplaceSessionInfo } from '@/services/marketplace/marketplace-session'; import type { AuthStore } from '@/stores/auth/auth.types'; @@ -14,6 +15,11 @@ export type THomeserverAuthenticateParams = TKeypairParams & TSecretKey; export interface TRestoreSessionParams { authStore: AuthStore; + /** + * Asks the user whether a `#s=` hand-off may sign this tab in as `pubky`. + * Resolves true only on an explicit yes. Absent, every hand-off is declined. + */ + confirmSessionHandoff?: (pubky: Pubky) => Promise; } export type TRestoreSessionOutcome = diff --git a/src/core/application/commerce/commerce.receipts.test.ts b/src/core/application/commerce/commerce.receipts.test.ts index cb47d958a4..559679f170 100644 --- a/src/core/application/commerce/commerce.receipts.test.ts +++ b/src/core/application/commerce/commerce.receipts.test.ts @@ -400,6 +400,16 @@ describe('CommerceApplication.publishOrderReceipts publication status (step-up O expect(fetch).toHaveBeenCalledTimes(2); }); + it('a grant session refused with 403 reports unavailable, not a step-up', async () => { + grantCapableSession(); + vi.spyOn(HomeserverService, 'isCurrentSessionGrant').mockReturnValue(true); + vi.spyOn(CommerceHomeserverService, 'fetchJson').mockRejectedValue(forbiddenError()); + + await expect( + CommerceApplication.publishOrderReceipts(BUYER, [paidOrder('018f47d2-6a27-7c23-a49d-6b21bb770219')]), + ).resolves.toBe('unavailable'); + }); + it('reports needs_reauth when the private read is refused with 403 mid-pass', async () => { grantCapableSession(); vi.spyOn(CommerceHomeserverService, 'fetchJson').mockRejectedValue(forbiddenError()); diff --git a/src/core/application/commerce/commerce.ts b/src/core/application/commerce/commerce.ts index 01982cdad4..123e481778 100644 --- a/src/core/application/commerce/commerce.ts +++ b/src/core/application/commerce/commerce.ts @@ -1464,9 +1464,16 @@ export class CommerceApplication { } } - /** 403 (scope refused) or 401 (session rejected) on the private document. */ + /** + * 403 (scope refused) or 401 (session rejected) on the private document, + * for a session that a step-up approval can widen. A grant session already + * holds the full Shop grant (enforced at sign-in and restore), so a refusal + * means its grant is no longer honored (revoked or expired), which no + * step-up QR can fix: the round fails and retries on the next load. + */ private static isPrivateAccessDenied(error: unknown): boolean { - return hasHttpStatus(error, HttpStatusCode.FORBIDDEN) || hasHttpStatus(error, HttpStatusCode.UNAUTHORIZED); + const denied = hasHttpStatus(error, HttpStatusCode.FORBIDDEN) || hasHttpStatus(error, HttpStatusCode.UNAUTHORIZED); + return denied && !HomeserverService.isCurrentSessionGrant(); } // --------------------------------------------------------------------- diff --git a/src/core/application/commerce/commerce.watchlist.test.ts b/src/core/application/commerce/commerce.watchlist.test.ts index 51599a57ff..10c3386f03 100644 --- a/src/core/application/commerce/commerce.watchlist.test.ts +++ b/src/core/application/commerce/commerce.watchlist.test.ts @@ -92,6 +92,17 @@ describe('CommerceApplication.syncWatchlist capability gating', () => { expect(put).toHaveBeenCalledOnce(); }); + it('a grant session refused with 403 fails the round instead of asking for a step-up', async () => { + vi.spyOn(HomeserverService, 'hasActiveSession').mockReturnValue(true); + vi.spyOn(HomeserverService, 'canCurrentSessionWrite').mockReturnValue(true); + vi.spyOn(HomeserverService, 'isCurrentSessionGrant').mockReturnValue(true); + vi.spyOn(CommerceHomeserverService, 'fetchJson').mockRejectedValue(httpError(403)); + const complete = vi.spyOn(LocalCommerceService, 'completeSyncJob'); + + expect(await CommerceApplication.syncWatchlist(OWNER)).toBe('error'); + expect(complete).not.toHaveBeenCalled(); + }); + it('pulls, merges remote-only watches into Dexie, and pushes nothing when the merge equals remote', async () => { vi.spyOn(HomeserverService, 'hasActiveSession').mockReturnValue(true); vi.spyOn(HomeserverService, 'canCurrentSessionWrite').mockReturnValue(true); diff --git a/src/core/controllers/auth/auth.single-approval.test.ts b/src/core/controllers/auth/auth.single-approval.test.ts index 5b57bda70a..701bda8bdc 100644 --- a/src/core/controllers/auth/auth.single-approval.test.ts +++ b/src/core/controllers/auth/auth.single-approval.test.ts @@ -2,6 +2,7 @@ import type { AuthToken, Session } from '@synonymdev/pubky'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { AuthApplication } from '@/application/auth/auth'; import { BootstrapApplication } from '@/application/bootstrap/bootstrap'; +import { CAPABILITIES } from '@/config/app'; import { clearDatabase } from '@/database/franky/franky.helpers'; import { AuthErrorCode } from '@/libs/error/error.codes'; import { Err } from '@/libs/error/error.factories'; @@ -583,7 +584,7 @@ describe('AuthController single-approval ceremony', () => { describe('AuthController Ring and Bitkit QRs side by side', () => { const grantSession = asOpaque({ - info: { publicKey: { z32: () => 'test-pubky' } }, + info: { publicKey: { z32: () => 'test-pubky' }, capabilities: CAPABILITIES.split(',') }, grant: {}, }); diff --git a/src/core/controllers/auth/auth.test.ts b/src/core/controllers/auth/auth.test.ts index 8e3067659c..6ff36c3c1b 100644 --- a/src/core/controllers/auth/auth.test.ts +++ b/src/core/controllers/auth/auth.test.ts @@ -6,16 +6,22 @@ import { BootstrapApplication } from '@/application/bootstrap/bootstrap'; import { CommerceApplication } from '@/application/commerce/commerce'; import { SettingsApplication } from '@/application/settings/settings'; import { postStreamQueue } from '@/application/stream/posts/muting/post-stream-queue'; +import { CAPABILITIES } from '@/config/app'; import { MUTE_SYNC_CURSOR_STORAGE_PREFIX } from '@/config/mute-sync'; import { AUTH_EPOCH_KEY, bumpAuthEpoch, readAuthEpoch, subscribeSignedOut } from '@/controllers/auth/auth-epoch'; import { resetAuthFinalizationLockForTests } from '@/controllers/auth/auth-finalization-lock'; +import { + clearGrantKeyCleanupPending, + isGrantKeyCleanupPending, + markGrantKeyCleanupPending, +} from '@/controllers/auth/grant-key-cleanup'; import { CommerceController } from '@/controllers/commerce/commerce'; import { NotificationCoordinator } from '@/coordinators/notifications/notifications'; import { StreamCoordinator } from '@/coordinators/streams/stream'; import { TtlCoordinator } from '@/coordinators/ttl/ttl'; import { clearDatabase, clearPrivateData } from '@/database/franky/franky.helpers'; import { AppError } from '@/libs/error/error'; -import { AuthErrorCode, ServerErrorCode } from '@/libs/error/error.codes'; +import { AuthErrorCode, ServerErrorCode, ValidationErrorCode } from '@/libs/error/error.codes'; import { Err } from '@/libs/error/error.factories'; import { ErrorCategory, ErrorService } from '@/libs/error/error.types'; import { Identity } from '@/libs/identity/identity'; @@ -29,6 +35,7 @@ import { NotificationNormalizer } from '@/pipes/notification/notification.normal import { PubkySpecsSingleton } from '@/pipes/pipes.builder'; import { SettingsNormalizer } from '@/pipes/settings/settings.normalizer'; import { grantKeyRemovalFailed, isGrantKeyRemovalError } from '@/services/homeserver/error.utils'; +import { HomeserverService } from '@/services/homeserver/homeserver'; import { useAuthStore } from '@/stores/auth/auth.store'; import type { AuthStore } from '@/stores/auth/auth.types'; import { useCommerceStore } from '@/stores/commerce/commerce.store'; @@ -40,6 +47,7 @@ import { useNotificationStore } from '@/stores/notification/notification.store'; import type { NotificationState } from '@/stores/notification/notification.types'; import { useOnboardingStore } from '@/stores/onboarding/onboarding.store'; import { useSearchStore } from '@/stores/search/search.store'; +import { useSessionHandoffStore } from '@/stores/sessionHandoff/sessionHandoff.store'; import { useSettingsStore } from '@/stores/settings/settings.store'; import { defaultNotificationPreferences, @@ -1148,7 +1156,10 @@ describe('AuthController', () => { const result = await AuthController.restorePersistedSession(); expect(result).toEqual({ status: 'restored' }); - expect(AuthApplication.restorePersistedSession).toHaveBeenCalledWith({ authStore }); + expect(AuthApplication.restorePersistedSession).toHaveBeenCalledWith({ + authStore, + confirmSessionHandoff: expect.any(Function), + }); expect(Identity.z32FromSession).toHaveBeenCalledWith({ session: mockSession }); expect(userIsSignedUpSpy).not.toHaveBeenCalled(); expect(authStore.reset).not.toHaveBeenCalled(); @@ -2396,7 +2407,11 @@ describe('AuthController', () => { }); describe('grant sessions (Bitkit sign-in)', () => { - const grantSession = () => buildMockSession({ grant: asOpaque({}) }); + const grantSession = (capabilities: string[] = CAPABILITIES.split(',')) => + buildMockSession({ + grant: asOpaque({}), + info: asOpaque({ publicKey: { z32: () => 'mock-session-pubky' }, capabilities }), + }); const grantAuthStore = (overrides: Partial = {}): AuthStore => mockAuthStore({ @@ -2417,6 +2432,7 @@ describe('AuthController', () => { beforeEach(() => { storeMocks.resetAuthStore.mockReset(); localStorage.removeItem(AUTH_EPOCH_KEY); + clearGrantKeyCleanupPending(); Object.defineProperty(document, 'cookie', { writable: true, value: '' }); mockClearDatabase.mockResolvedValue(undefined); vi.spyOn(Identity, 'z32FromSession').mockReturnValue(TEST_PUBKY as Pubky); @@ -2448,6 +2464,130 @@ describe('AuthController', () => { ); }); + it('an approval narrower than the Shop grant is signed out and never saved', async () => { + const session = grantSession(['/pub/pubky.app/:rw']); + vi.spyOn(useAuthStore, 'getState').mockReturnValue(grantAuthStore()); + const saveSpy = vi.spyOn(AuthApplication, 'saveGrantSession').mockResolvedValue('rec-1'); + const logoutSpy = vi.spyOn(HomeserverService, 'logout').mockResolvedValue(undefined); + + await expect(approveGrantSignIn(session)).rejects.toMatchObject({ code: ValidationErrorCode.INVALID_INPUT }); + expect(logoutSpy).toHaveBeenCalledWith({ session }); + expect(saveSpy).not.toHaveBeenCalled(); + }); + + it('a failed save signs the grant out, removes its keys and surfaces the failure', async () => { + const order: string[] = []; + const session = grantSession(); + const authStore = grantAuthStore(); + vi.spyOn(useAuthStore, 'getState').mockReturnValue(authStore); + const saveFailure = new Error('IndexedDB write failed'); + vi.spyOn(AuthApplication, 'saveGrantSession').mockRejectedValue(saveFailure); + vi.spyOn(AuthApplication, 'logout').mockImplementation(async () => { + order.push('signout'); + }); + vi.spyOn(AuthApplication, 'clearGrantSessions').mockImplementation(async () => { + order.push('clearAll'); + }); + + const approved = await approveGrantSignIn(session); + await expect(AuthController.initializeAuthenticatedSession({ session: approved })).rejects.toBe(saveFailure); + + expect(order).toEqual(['signout', 'clearAll']); + expect(authStore.init).not.toHaveBeenCalled(); + }); + + /** In-memory BrowserSessionStore behind `clearGrantSessions`: fails `failures` times, then empties. */ + function fakeGrantStore(records: string[], failures: number) { + let remainingFailures = failures; + const clear = vi.spyOn(AuthApplication, 'clearGrantSessions').mockImplementation(async () => { + if (remainingFailures > 0) { + remainingFailures -= 1; + throw grantKeyRemovalFailed('clearGrantSessions', null); + } + records.splice(0); + }); + return { records, clear }; + } + + it('a failed save whose key removal also fails keeps the cleanup, and the next Bitkit sign-in empties the store first', async () => { + const session = grantSession(); + vi.spyOn(useAuthStore, 'getState').mockReturnValue(grantAuthStore()); + vi.spyOn(AuthApplication, 'saveGrantSession').mockRejectedValue(new Error('IndexedDB write failed')); + vi.spyOn(AuthApplication, 'logout').mockResolvedValue(undefined); + const store = fakeGrantStore(['partial-record'], 1); + + const approved = await approveGrantSignIn(session); + await expect(AuthController.initializeAuthenticatedSession({ session: approved })).rejects.toThrow( + 'IndexedDB write failed', + ); + expect(store.records).toEqual(['partial-record']); + expect(isGrantKeyCleanupPending()).toBe(true); + + const order: string[] = []; + store.clear.mockImplementation(async () => { + order.push('clear'); + store.records.splice(0); + }); + vi.spyOn(AuthApplication, 'generateGrantAuthUrl').mockImplementation(async () => { + order.push('new-flow'); + return { + authorizationUrl: 'pubkyauth://signin_grant?x', + awaitApproval: new Promise(() => {}), + cancelAuthFlow: vi.fn(), + }; + }); + await AuthController.getGrantAuthUrl(); + + expect(order).toEqual(['clear', 'new-flow']); + expect(store.records).toEqual([]); + expect(isGrantKeyCleanupPending()).toBe(false); + }); + + it('the next load removes grant keys a failed cleanup left behind', async () => { + vi.spyOn(useAuthStore, 'getState').mockReturnValue(grantAuthStore()); + const store = fakeGrantStore(['stranded'], 1); + markGrantKeyCleanupPending(); + + await expect(AuthController.settlePendingGrantKeyCleanup()).resolves.toBe(false); + expect(isGrantKeyCleanupPending()).toBe(true); + + await expect(AuthController.settlePendingGrantKeyCleanup()).resolves.toBe(true); + expect(store.records).toEqual([]); + expect(isGrantKeyCleanupPending()).toBe(false); + }); + + it('a pending cleanup leaves a signed-in grant session alone until its sign-out removes every key', async () => { + const authStore = grantAuthStore({ session: grantSession(), grantSessionRecordId: 'rec-live' }); + vi.spyOn(useAuthStore, 'getState').mockReturnValue(authStore); + vi.spyOn(AuthApplication, 'logout').mockResolvedValue(undefined); + const store = fakeGrantStore(['rec-live', 'stranded'], 0); + markGrantKeyCleanupPending(); + + await expect(AuthController.settlePendingGrantKeyCleanup()).resolves.toBe(false); + expect(store.clear).not.toHaveBeenCalled(); + expect(isGrantKeyCleanupPending()).toBe(true); + + await AuthController.logout(); + + expect(store.records).toEqual([]); + expect(isGrantKeyCleanupPending()).toBe(false); + }); + + it('a successful grant save drops only the marker it set', async () => { + vi.spyOn(useAuthStore, 'getState').mockReturnValue(grantAuthStore()); + vi.spyOn(AuthApplication, 'saveGrantSession').mockResolvedValue('rec-1'); + + await AuthController.initializeAuthenticatedSession({ session: await approveGrantSignIn(grantSession()) }); + expect(isGrantKeyCleanupPending()).toBe(false); + + vi.spyOn(AuthApplication, 'clearGrantSessions').mockRejectedValue( + grantKeyRemovalFailed('clearGrantSessions', null), + ); + markGrantKeyCleanupPending(); + await AuthController.initializeAuthenticatedSession({ session: await approveGrantSignIn(grantSession()) }); + expect(isGrantKeyCleanupPending()).toBe(true); + }); + it('save aborts after a sign-out since QR start', async () => { const session = grantSession(); const authStore = grantAuthStore(); @@ -2780,6 +2920,50 @@ describe('AuthController', () => { expect(storeMocks.resetAuthStore).not.toHaveBeenCalled(); }); + it('a #s= hand-off waits for the prompt and only a yes passes through', async () => { + vi.spyOn(useAuthStore, 'getState').mockReturnValue(grantAuthStore({ currentUserPubky: null })); + let answer: boolean | undefined; + vi.spyOn(AuthApplication, 'restorePersistedSession').mockImplementation(async ({ confirmSessionHandoff }) => { + answer = await confirmSessionHandoff?.('handoff-pubky' as Pubky); + return { status: 'deferred' }; + }); + + const restoring = AuthController.restorePersistedSession(); + await vi.waitFor(() => expect(useSessionHandoffStore.getState().pendingPubky).toBe('handoff-pubky')); + expect(answer).toBeUndefined(); + AuthController.answerSessionHandoff(true); + await restoring; + + expect(answer).toBe(true); + expect(useSessionHandoffStore.getState().pendingPubky).toBeNull(); + }); + + it('logout declines an unanswered #s= prompt', async () => { + vi.spyOn(useAuthStore, 'getState').mockReturnValue(grantAuthStore({ currentUserPubky: null })); + let answer: boolean | undefined; + vi.spyOn(AuthApplication, 'restorePersistedSession').mockImplementation(async ({ confirmSessionHandoff }) => { + answer = await confirmSessionHandoff?.('handoff-pubky' as Pubky); + return { status: 'deferred' }; + }); + const restoring = AuthController.restorePersistedSession(); + await vi.waitFor(() => expect(useSessionHandoffStore.getState().pendingPubky).toBe('handoff-pubky')); + + vi.spyOn(AuthApplication, 'clearGrantSessions').mockResolvedValue(undefined); + await AuthController.logout(); + await restoring; + + expect(answer).toBe(false); + expect(useSessionHandoffStore.getState().pendingPubky).toBeNull(); + }); + + it('a grant session is refused a step-up and no Ring flow starts', async () => { + vi.spyOn(useAuthStore, 'getState').mockReturnValue(grantAuthStore({ session: grantSession() })); + const ringFlow = vi.spyOn(AuthApplication, 'generateAuthUrl'); + + await expect(AuthController.getStepUpAuthUrl()).rejects.toMatchObject({ code: AuthErrorCode.UNAUTHORIZED }); + expect(ringFlow).not.toHaveBeenCalled(); + }); + it('logout tells other tabs to let go', async () => { vi.spyOn(useAuthStore, 'getState').mockReturnValue(grantAuthStore({ session: grantSession() })); vi.spyOn(AuthApplication, 'logout').mockResolvedValue(undefined); diff --git a/src/core/controllers/auth/auth.ts b/src/core/controllers/auth/auth.ts index bfb2d1e3b3..6e40b58558 100644 --- a/src/core/controllers/auth/auth.ts +++ b/src/core/controllers/auth/auth.ts @@ -27,6 +27,11 @@ import { shouldAbortIdentityPersist, shouldSkipDestructiveCleanup, } from '@/controllers/auth/auth-identity-guard'; +import { + clearGrantKeyCleanupPending, + isGrantKeyCleanupPending, + markGrantKeyCleanupPending, +} from '@/controllers/auth/grant-key-cleanup'; import { CommerceController } from '@/controllers/commerce/commerce'; import { NotificationCoordinator } from '@/coordinators/notifications/notifications'; import { StreamCoordinator } from '@/coordinators/streams/stream'; @@ -55,6 +60,7 @@ import { clearPersistedAuthIdentity, hasPersistedAuthIdentity, readPersistedAuthIdentity, + readPersistedGrantSessionRecordId, } from '@/stores/auth/auth.persisted'; import { useAuthStore } from '@/stores/auth/auth.store'; import { useCommerceStore } from '@/stores/commerce/commerce.store'; @@ -67,6 +73,7 @@ import { useNotificationStore } from '@/stores/notification/notification.store'; import { useOnboardingStore } from '@/stores/onboarding/onboarding.store'; import { ONBOARDING_PERSIST_KEY } from '@/stores/persistedKeys'; import { useSearchStore } from '@/stores/search/search.store'; +import { useSessionHandoffStore } from '@/stores/sessionHandoff/sessionHandoff.store'; import { useSettingsStore } from '@/stores/settings/settings.store'; import type { SettingsState } from '@/stores/settings/settings.types'; import { useSignInStore } from '@/stores/signIn/signIn.store'; @@ -128,6 +135,25 @@ export class AuthController { /** `authFlowGeneration` when each sign-in QR (Ring or Bitkit) started, keyed by its approved session. */ private static sessionFlowGeneration = new WeakMap(); + /** Resolver of the `#s=` hand-off prompt the user has not answered yet. */ + private static pendingSessionHandoff: ((accepted: boolean) => void) | null = null; + + private static async confirmSessionHandoff(pubky: Pubky): Promise { + this.pendingSessionHandoff?.(false); + return await new Promise((resolve) => { + this.pendingSessionHandoff = resolve; + useSessionHandoffStore.getState().setPendingPubky(pubky); + }); + } + + /** The user's answer to the `#s=` hand-off prompt. Only `true` signs the tab in. */ + static answerSessionHandoff(accepted: boolean): void { + const resolve = this.pendingSessionHandoff; + this.pendingSessionHandoff = null; + useSessionHandoffStore.getState().setPendingPubky(null); + resolve?.(accepted); + } + /** * Single-run guard for cleanupLocalState: concurrent Controller invocations * (e.g. a logout racing an in-flight restore) share one run, and once a run @@ -271,7 +297,10 @@ export class AuthController { authStore.setIsRestoringSession(true); let cleanedUp = false; try { - const result = await AuthApplication.restorePersistedSession({ authStore }); + const result = await AuthApplication.restorePersistedSession({ + authStore, + confirmSessionHandoff: (pubky) => this.confirmSessionHandoff(pubky), + }); if (result.status === 'restored') { const { session } = result; const pubky = Identity.z32FromSession({ session }); @@ -527,6 +556,7 @@ export class AuthController { const authStore = useAuthStore.getState(); let persistAborted = false; + let grantSaveError: unknown = null; try { this.cancelAllAuthFlows(); @@ -551,14 +581,21 @@ export class AuthController { if (epochAtStart === undefined || epochAtStart !== readAuthEpoch()) { return false; } - grantSessionRecordId = await AuthApplication.saveGrantSession(session); + // Recorded before the save: a save that fails, or a tab that closes + // mid-save, leaves an obligation the next load can discharge. + const cleanupAlreadyPending = isGrantKeyCleanupPending(); + markGrantKeyCleanupPending(); + try { + grantSessionRecordId = await AuthApplication.saveGrantSession(session); + } catch (error) { + grantSaveError = error; + return false; + } + authStore.init({ session, currentUserPubky: pubky, hasProfile: null, grantSessionRecordId }); + if (!cleanupAlreadyPending) clearGrantKeyCleanupPending(); + return true; } - authStore.init({ - session, - currentUserPubky: pubky, - hasProfile: null, - ...(grantSessionRecordId ? { grantSessionRecordId } : {}), - }); + authStore.init({ session, currentUserPubky: pubky, hasProfile: null }); return true; }); if (!persisted) { @@ -566,6 +603,14 @@ export class AuthController { await AuthApplication.logout({ session }).catch((logoutError) => { Logger.warn('Failed to sign out a session that lost the local-state race', { logoutError }); }); + if (grantSaveError !== null) { + // A failed save can leave a partial record and this flow's delegated + // key in IndexedDB. The grant is signed out above (that needs the + // key), so the key goes now. Sign-out uses the same order. If the + // removal fails too, the pending marker keeps the obligation. + await this.settlePendingGrantKeyCleanup(); + throw grantSaveError; + } throw createCanceledError(); } @@ -882,6 +927,8 @@ export class AuthController { * saved to BrowserSessionStore at completion. */ static async getGrantAuthUrl(): Promise { + // Before the new flow creates its key: the cleanup removes every key. + await this.settlePendingGrantKeyCleanup(); const epochAtStart = readAuthEpoch(); BootstrapApplication.cancelModerationFollow(); const captured = this.captureAuthIdentity(); @@ -912,6 +959,7 @@ export class AuthController { if (this.lostToAnotherSignIn(session)) { return await this.discardLosingApproval(session); } + await AuthApplication.assertFullGrantSession(session); this.grantEpochAtStart.set(session, epochAtStart); return session; }); @@ -924,6 +972,17 @@ export class AuthController { } static async getStepUpAuthUrl(): Promise { + // A grant session already holds the full Shop grant (checked at sign-in + // and restore), and a Ring step-up would swap it for a cookie session + // while its grant record stays stored. Refused here so every caller is + // covered even if a new grant path skips those checks. + if (AuthApplication.isGrantSession(useAuthStore.getState().session)) { + throw Err.auth( + AuthErrorCode.UNAUTHORIZED, + 'Bitkit sign-in already includes every Shop permission. If this keeps asking, sign out and sign in with Bitkit again.', + { service: ErrorService.Local, operation: 'getStepUpAuthUrl' }, + ); + } if (!isSingleApprovalSignInEnabled()) { return this.wrapAuthFlow(() => AuthApplication.generateAuthUrl(), { preserveLocalState: true }); } @@ -1253,6 +1312,8 @@ export class AuthController { // Bump before any await so an in-flight restore (sharing the Application // singleton promise) can tell its restored-branch result is stale. this.logoutGeneration += 1; + // An unanswered `#s=` prompt holds the shared restore this logout joins. + this.answerSessionHandoff(false); // Set before any await so RouteGuard cannot re-bridge between cleanup and this flag. suppressVibeSessionAutoRestore(); AuthApplication.abortInFlightBridgeRequest(); @@ -1346,9 +1407,34 @@ export class AuthController { await withAuthFinalizationLock(async () => { bumpAuthEpoch(); await AuthApplication.clearGrantSessions(); + clearGrantKeyCleanupPending(); }); } + /** + * Discharges a pending grant-key cleanup (see `grant-key-cleanup.ts`): + * removes every stored grant record and key, then drops the marker once the + * store reads back empty. A signed-in grant session in any tab still needs + * its own record, so the cleanup waits for that session's sign-out, which + * removes every key and drops the marker. A failed removal keeps the marker + * for the next attempt. Returns whether nothing is left pending. + */ + static async settlePendingGrantKeyCleanup(): Promise { + if (!isGrantKeyCleanupPending()) return true; + try { + return await withAuthFinalizationLock(async () => { + if (!isGrantKeyCleanupPending()) return true; + if (useAuthStore.getState().grantSessionRecordId || readPersistedGrantSessionRecordId()) return false; + await AuthApplication.clearGrantSessions(); + clearGrantKeyCleanupPending(); + return true; + }); + } catch (error) { + Logger.error('Grant keys left by a failed save are still stored; the cleanup is retried later', { error }); + return false; + } + } + /** * Another tab signed out. A tab still holding a grant session in memory * signs it out and drops local state; cookie sessions are left alone. diff --git a/src/core/controllers/auth/grant-key-cleanup.ts b/src/core/controllers/auth/grant-key-cleanup.ts new file mode 100644 index 0000000000..01d21bfe7f --- /dev/null +++ b/src/core/controllers/auth/grant-key-cleanup.ts @@ -0,0 +1,41 @@ +/** + * Durable obligation to remove grant keys that a failed save left in + * BrowserSessionStore. Set before the cleanup is attempted and cleared only + * once the store reads back empty, so a cleanup that fails (or a tab that + * closes mid-cleanup) is retried on the next load, the next Bitkit sign-in, + * or the next sign-out. Origin-scoped like the store it guards; not an + * account-owned key, so account cleanup must not drop it. + */ +export const GRANT_KEY_CLEANUP_PENDING_KEY = 'pubky-grant-key-cleanup-pending-v1'; + +function storage(): Storage | null { + try { + return typeof localStorage === 'undefined' ? null : localStorage; + } catch { + return null; + } +} + +export function markGrantKeyCleanupPending(): void { + try { + storage()?.setItem(GRANT_KEY_CLEANUP_PENDING_KEY, '1'); + } catch { + // Storage full or disabled: the cleanup below still runs this once. + } +} + +export function isGrantKeyCleanupPending(): boolean { + try { + return storage()?.getItem(GRANT_KEY_CLEANUP_PENDING_KEY) === '1'; + } catch { + return false; + } +} + +export function clearGrantKeyCleanupPending(): void { + try { + storage()?.removeItem(GRANT_KEY_CLEANUP_PENDING_KEY); + } catch { + // Unreadable storage cannot hold the marker either. + } +} diff --git a/src/core/services/homeserver/homeserver.ts b/src/core/services/homeserver/homeserver.ts index b203cfd884..9c258d26fa 100644 --- a/src/core/services/homeserver/homeserver.ts +++ b/src/core/services/homeserver/homeserver.ts @@ -692,6 +692,10 @@ export class HomeserverService { return Boolean(session && session.grant !== undefined); } + static isCurrentSessionGrant(): boolean { + return this.isGrantSession(useAuthStore.getState().selectSession()); + } + /** Persists a completed grant session in IndexedDB and returns its record id. */ static async saveGrantSession(session: Session): Promise { try { diff --git a/src/core/stores/auth/auth.persisted.ts b/src/core/stores/auth/auth.persisted.ts index 30524e1353..9d3399dff6 100644 --- a/src/core/stores/auth/auth.persisted.ts +++ b/src/core/stores/auth/auth.persisted.ts @@ -63,6 +63,18 @@ export function hasPersistedAuthIdentity(): boolean { return readPersistedAuthIdentity().present; } +/** The BrowserSessionStore record id a signed-in grant session (any tab) points at, if one is persisted. */ +export function readPersistedGrantSessionRecordId(): string | null { + try { + const raw = globalThis.localStorage?.getItem(AUTH_PERSIST_KEY); + if (!raw) return null; + const state = (JSON.parse(raw) as { state?: { grantSessionRecordId?: unknown } }).state; + return isNonEmptyString(state?.grantSessionRecordId) ? state.grantSessionRecordId : null; + } catch { + return null; + } +} + /** * Drop the persist blob so a later `init` of a different pubky is not * treated as a foreign clobber. Only call this inside the finalization lock diff --git a/src/core/stores/sessionHandoff/sessionHandoff.store.ts b/src/core/stores/sessionHandoff/sessionHandoff.store.ts new file mode 100644 index 0000000000..5df88bd8c7 --- /dev/null +++ b/src/core/stores/sessionHandoff/sessionHandoff.store.ts @@ -0,0 +1,14 @@ +import { create } from 'zustand'; +import type { Pubky } from '@/models/models.types'; + +type SessionHandoffStore = { + /** Identity a pending `#s=` hand-off would sign this tab in as; null when none is waiting. */ + pendingPubky: Pubky | null; + setPendingPubky: (pubky: Pubky | null) => void; +}; + +// No persistence: a hand-off belongs to one page load. +export const useSessionHandoffStore = create()((set) => ({ + pendingPubky: null, + setPendingPubky: (pubky) => set({ pendingPubky: pubky }), +})); diff --git a/src/hooks/useSessionHandoff/useSessionHandoff.ts b/src/hooks/useSessionHandoff/useSessionHandoff.ts new file mode 100644 index 0000000000..2df5d39a9a --- /dev/null +++ b/src/hooks/useSessionHandoff/useSessionHandoff.ts @@ -0,0 +1,15 @@ +'use client'; + +import { AuthController } from '@/controllers/auth/auth'; +import type { Pubky } from '@/models/models.types'; +import { useSessionHandoffStore } from '@/stores/sessionHandoff/sessionHandoff.store'; + +/** The pending `#s=` hand-off prompt, if any, and the two answers to it. */ +export function useSessionHandoff(): { pendingPubky: Pubky | null; accept: () => void; decline: () => void } { + const pendingPubky = useSessionHandoffStore((state) => state.pendingPubky); + return { + pendingPubky, + accept: () => AuthController.answerSessionHandoff(true), + decline: () => AuthController.answerSessionHandoff(false), + }; +} diff --git a/src/providers/RouteGuardProvider/RouteGuardProvider.suppressed-reload.test.tsx b/src/providers/RouteGuardProvider/RouteGuardProvider.suppressed-reload.test.tsx index 545c5b9902..fb64216450 100644 --- a/src/providers/RouteGuardProvider/RouteGuardProvider.suppressed-reload.test.tsx +++ b/src/providers/RouteGuardProvider/RouteGuardProvider.suppressed-reload.test.tsx @@ -13,6 +13,7 @@ import { RouteGuardProvider } from './RouteGuardProvider'; const mocks = vi.hoisted(() => ({ mockRouterPush: vi.fn(), subscribeCrossTabSignOut: vi.fn(() => () => {}), + settlePendingGrantKeyCleanup: vi.fn().mockResolvedValue(true), restorePersistedSession: vi.fn().mockResolvedValue({ status: 'signed-out' }), pathname: '/home', })); @@ -63,6 +64,7 @@ vi.mock('@/controllers/auth/auth', () => ({ AuthController: { restorePersistedSession: mocks.restorePersistedSession, subscribeCrossTabSignOut: mocks.subscribeCrossTabSignOut, + settlePendingGrantKeyCleanup: mocks.settlePendingGrantKeyCleanup, }, })); diff --git a/src/providers/RouteGuardProvider/RouteGuardProvider.test.tsx b/src/providers/RouteGuardProvider/RouteGuardProvider.test.tsx index 19fb2d4664..3fb6c42580 100644 --- a/src/providers/RouteGuardProvider/RouteGuardProvider.test.tsx +++ b/src/providers/RouteGuardProvider/RouteGuardProvider.test.tsx @@ -19,6 +19,7 @@ const mocks = vi.hoisted(() => { const mockToast = vi.fn(); const mockSetShowSignInDialog = vi.fn(); const restorePersistedSession = vi.fn().mockResolvedValue(true); + const settlePendingGrantKeyCleanup = vi.fn().mockResolvedValue(true); return { subscribeCrossTabSignOut, @@ -30,6 +31,7 @@ const mocks = vi.hoisted(() => { mockToast, mockSetShowSignInDialog, restorePersistedSession, + settlePendingGrantKeyCleanup, consumerEnabled: false, autoRestoreSuppressed: false, // Auth store state defaults @@ -154,6 +156,7 @@ vi.mock('@/controllers/auth/auth', () => ({ AuthController: { restorePersistedSession: mocks.restorePersistedSession, subscribeCrossTabSignOut: mocks.subscribeCrossTabSignOut, + settlePendingGrantKeyCleanup: mocks.settlePendingGrantKeyCleanup, }, })); vi.mock('@/controllers/migration/migration', () => ({ @@ -194,6 +197,28 @@ describe('RouteGuardProvider — migration resync', () => { vi.useRealTimers(); }); + it('retries a pending grant-key cleanup once the auth store has hydrated', async () => { + mocks.hasHydrated = false; + const { rerender } = render( + +
Protected Content
+ , + ); + expect(mocks.settlePendingGrantKeyCleanup).not.toHaveBeenCalled(); + + mocks.hasHydrated = true; + rerender( + +
Protected Content
+ , + ); + await act(async () => { + await vi.runAllTimersAsync(); + }); + + expect(mocks.settlePendingGrantKeyCleanup).toHaveBeenCalledTimes(1); + }); + it('calls MigrationController.resync when wasDbReset is true and user is authenticated', async () => { mocks.wasDbReset = true; mocks.mockResync.mockResolvedValue(undefined); diff --git a/src/providers/RouteGuardProvider/RouteGuardProvider.tsx b/src/providers/RouteGuardProvider/RouteGuardProvider.tsx index d361533083..6c63ee6e4f 100644 --- a/src/providers/RouteGuardProvider/RouteGuardProvider.tsx +++ b/src/providers/RouteGuardProvider/RouteGuardProvider.tsx @@ -68,6 +68,12 @@ export function RouteGuardProvider({ children }: RouteGuardProviderProps) { // Another tab signed out: a grant session held in this tab lets go too. useEffect(() => AuthController.subscribeCrossTabSignOut(), []); + // Grant keys a failed save left behind are removed on the next load. + useEffect(() => { + if (!hasHydrated) return; + void AuthController.settlePendingGrantKeyCleanup(); + }, [hasHydrated]); + // Attempt to restore an existing session snapshot on fresh loads. useEffect(() => { if (!hasHydrated) return; diff --git a/src/server/marketplace-grant/browser-bff.test.ts b/src/server/marketplace-grant/browser-bff.test.ts index fca26a3ab1..88d9517bb2 100644 --- a/src/server/marketplace-grant/browser-bff.test.ts +++ b/src/server/marketplace-grant/browser-bff.test.ts @@ -32,6 +32,7 @@ const getCliFlow = vi.fn(); const acquireCliClaim = vi.fn(); const completeCliClaim = vi.fn(); const abandonCliClaim = vi.fn(); +const abandonLapsedCliClaim = vi.fn(); const renewCliClaim = vi.fn(); const assertCliGrantSchema = vi.fn(); const fetchHomeserverProofDocument = vi.fn(); @@ -56,6 +57,7 @@ vi.mock('./db', async (importOriginal) => { acquireCliClaim: (...args: unknown[]) => acquireCliClaim(...args), completeCliClaim: (...args: unknown[]) => completeCliClaim(...args), abandonCliClaim: (...args: unknown[]) => abandonCliClaim(...args), + abandonLapsedCliClaim: (...args: unknown[]) => abandonLapsedCliClaim(...args), renewCliClaim: (...args: unknown[]) => renewCliClaim(...args), }; }); @@ -769,6 +771,156 @@ describe('browser purchase bootstrap BFF', () => { expect(completeCliClaim).not.toHaveBeenCalled(); }); + it('a claim whose lease lapsed ends the flow and asks for a fresh approval', async () => { + const config = await browserConfig(); + const { bound, row, stateId } = browserFlowRow(config, { status: 'claiming' }); + getCliFlow.mockResolvedValue({ ...row, lease_owner: randomUUID(), lease_until: new Date(Date.now() - 1_000) }); + abandonLapsedCliClaim.mockResolvedValue(true); + const { pollBrowserFlow } = await import('./browser-bff'); + + await expect(pollBrowserFlow(flowRequest(stateId), bound.value, stateId)).rejects.toEqual( + new BffError(422, 'fresh_approval_required'), + ); + expect(abandonLapsedCliClaim).toHaveBeenCalledWith(expect.anything(), stateId); + expect(getGrantStatus).not.toHaveBeenCalled(); + }); + + it('a claim with a live lease stays in progress', async () => { + const config = await browserConfig(); + const { bound, row, stateId } = browserFlowRow(config, { status: 'claiming' }); + getCliFlow.mockResolvedValue({ ...row, lease_owner: randomUUID(), lease_until: new Date(Date.now() + 30_000) }); + abandonLapsedCliClaim.mockResolvedValue(false); + const { pollBrowserFlow } = await import('./browser-bff'); + + await expect(pollBrowserFlow(flowRequest(stateId), bound.value, stateId)).rejects.toEqual( + new BffError(409, 'claim_in_progress'), + ); + }); + + it('cancel ends the flow even when its context cannot be opened', async () => { + const config = await browserConfig(); + const stateId = randomUUID(); + const bound = makeBoundCookie(stateId); + const cliContext = sealCliFlowContext(config, stateId, pubky, { + resultDeliveryId: encodeBase64Url(new Uint8Array(32).fill(4)), + version: 1, + }); + const { row } = browserFlowRow(config, { + tokenHash: hashBoundCookie(config, 1, 'flow', stateId, bound.secret), + contextSealed: cliContext, + }); + getCliFlow.mockResolvedValue({ ...row, state_id: stateId }); + const { cancelBrowserFlow } = await import('./browser-bff'); + + await expect( + cancelBrowserFlow( + jsonRequest(`${ORIGIN}/api/marketplace/bootstrap-flows/${stateId}/cancel`, {}), + bound.value, + stateId, + ), + ).rejects.toEqual(new BffError(403, 'result_denied')); + expect(terminalizeCliFlow).toHaveBeenCalledWith(expect.anything(), stateId, 'cancelled'); + expect(cancelGrant).not.toHaveBeenCalled(); + }); + + it('a flow whose key epoch rotated out can be neither cancelled nor claimed', async () => { + const epoch1 = await browserConfig(); + const { bound, row, stateId } = browserFlowRow(epoch1); + rotateTo(3, STATE_KEY_3, { epoch: 2, key: STATE_KEY_2 }); + getCliFlow.mockResolvedValue(row); + acquireCliClaim.mockResolvedValue(row); + completeServiceFlow(row.flow_id); + const { cancelBrowserFlow, pollBrowserFlow } = await import('./browser-bff'); + const freshApproval = new BffError(422, 'fresh_approval_required'); + + await expect( + cancelBrowserFlow( + jsonRequest(`${ORIGIN}/api/marketplace/bootstrap-flows/${stateId}/cancel`, {}), + bound.value, + stateId, + ), + ).rejects.toEqual(freshApproval); + await expect(pollBrowserFlow(flowRequest(stateId), bound.value, stateId)).rejects.toEqual(freshApproval); + expect(acquireCliClaim).not.toHaveBeenCalled(); + expect(claimGrantResult).not.toHaveBeenCalled(); + }); + + it('cancel ends the flow locally and cancels it at the service', async () => { + const config = await browserConfig(); + const { bound, derived, row, stateId } = browserFlowRow(config); + getCliFlow.mockResolvedValue(row); + cancelGrant.mockResolvedValue(undefined); + const { cancelBrowserFlow } = await import('./browser-bff'); + + await cancelBrowserFlow( + jsonRequest(`${ORIGIN}/api/marketplace/bootstrap-flows/${stateId}/cancel`, {}), + bound.value, + stateId, + ); + + expect(terminalizeCliFlow).toHaveBeenCalledWith(expect.anything(), stateId, 'cancelled'); + expect(cancelGrant).toHaveBeenCalledWith(expect.anything(), row.flow_id, encodeBase64Url(derived.resultDeliveryId)); + }); + + function countingRateLimit(): void { + const buckets = new Map(); + consumeCliRateLimit.mockImplementation(async (_config: unknown, key: string, limit: number) => { + const count = (buckets.get(key) ?? 0) + 1; + buckets.set(key, count); + return count <= limit; + }); + } + + it("challenges a NAT peer creates for someone's pubky never lock the owner out", async () => { + storeInsertedChallenges(); + process.env.VERCEL = '1'; + resetMarketplaceGrantConfigForTests(); + const { createBrowserChallenge } = await import('./browser-bff'); + countingRateLimit(); + const natExit = { 'x-vercel-forwarded-for': '203.0.113.66' }; + + // Five was the old per-pubky allowance; the peer spends it naming the owner's pubky. + for (let i = 0; i < 5; i += 1) await createBrowserChallenge(challengeRequest({ pubky }, natExit)); + + await expect(createBrowserChallenge(challengeRequest({ pubky }, natExit))).resolves.toMatchObject({ + proof_uri: expect.stringContaining(pubky), + }); + }); + + it('rotating a client-written forwarded-for hop does not escape the per-IP bucket', async () => { + storeInsertedChallenges(); + process.env.VERCEL = '1'; + resetMarketplaceGrantConfigForTests(); + const { createBrowserChallenge } = await import('./browser-bff'); + countingRateLimit(); + const config = await browserConfig(); + const spoofed = (i: number) => + challengeRequest({ pubky }, { 'x-vercel-forwarded-for': `10.0.${i}.1, 203.0.113.66` }); + + for (let i = 0; i < config.createPerIpPerMinute; i += 1) await createBrowserChallenge(spoofed(i)); + + await expect(createBrowserChallenge(spoofed(999))).rejects.toEqual(new BffError(429, 'retry_later', 60)); + }); + + it('the per-IP bucket is shared behind one NAT (accepted limit of unauthenticated creation)', async () => { + storeInsertedChallenges(); + process.env.VERCEL = '1'; + resetMarketplaceGrantConfigForTests(); + const { createBrowserChallenge } = await import('./browser-bff'); + countingRateLimit(); + const config = await browserConfig(); + const from = (ip: string, who = pubky) => challengeRequest({ pubky: who }, { 'x-vercel-forwarded-for': ip }); + + for (let i = 0; i < config.createPerIpPerMinute; i += 1) await createBrowserChallenge(from('203.0.113.66')); + + await expect(createBrowserChallenge(from('203.0.113.66', otherPubky))).rejects.toEqual( + new BffError(429, 'retry_later', 60), + ); + await expect(createBrowserChallenge(from('198.51.100.7', otherPubky))).resolves.toMatchObject({ + proof_uri: expect.any(String), + }); + }); + // A9 it('no migration added by the browser bootstrap', () => { const migrations = readdirSync(path.resolve(process.cwd(), 'db/bff')) diff --git a/src/server/marketplace-grant/browser-bff.ts b/src/server/marketplace-grant/browser-bff.ts index 9251043e00..9005a5aa68 100644 --- a/src/server/marketplace-grant/browser-bff.ts +++ b/src/server/marketplace-grant/browser-bff.ts @@ -20,6 +20,7 @@ import { } from './crypto'; import { abandonCliClaim, + abandonLapsedCliClaim, acquireCliClaim, assertCliGrantSchema, bindCliFlow, @@ -96,12 +97,12 @@ export async function createBrowserChallenge(request: Request): Promise { resetMarketplaceGrantConfigForTests(); }); + it("challenges a NAT peer creates for someone's pubky never lock the owner out", async () => { + process.env.VERCEL = '1'; + resetMarketplaceGrantConfigForTests(); + const { createCliChallenge } = await import('./cli-bff'); + const buckets = new Map(); + consumeCliRateLimit.mockImplementation(async (_config: unknown, key: string, limit: number) => { + const count = (buckets.get(key) ?? 0) + 1; + buckets.set(key, count); + return count <= limit; + }); + const request = () => + jsonRequest( + { pubky, result_cpk: pubky, result_delivery_id: deliveryId }, + { 'x-vercel-forwarded-for': '203.0.113.66' }, + ); + + // Five was the old per-pubky allowance; the peer spends it naming the owner's pubky. + for (let i = 0; i < 5; i += 1) await createCliChallenge(request()); + + await expect(createCliChallenge(request())).resolves.toMatchObject({ proof_uri: expect.stringContaining(pubky) }); + expect(buckets.size).toBe(1); + }); + it('returns 404 grant_unavailable when the CLI flag is off', async () => { process.env.SHOP_BFF_CLI_GRANT_ENABLED = 'false'; resetMarketplaceGrantConfigForTests(); diff --git a/src/server/marketplace-grant/cli-bff.ts b/src/server/marketplace-grant/cli-bff.ts index 3c5a807d0a..caffae3460 100644 --- a/src/server/marketplace-grant/cli-bff.ts +++ b/src/server/marketplace-grant/cli-bff.ts @@ -100,9 +100,19 @@ export function requireUuid(value: string): string { return value; } -function firstHop(value: string | null): string { - const hop = value?.split(',')[0]?.trim(); - return hop || ''; +/** + * The hop the platform wrote. Vercel overwrites `x-vercel-forwarded-for` + * with the address it received the request from; a proxy that appends + * instead leaves any client-written hops to the left, so the leftmost hop is + * never trusted. + */ +function lastHop(value: string | null): string { + const hops = + value + ?.split(',') + .map((hop) => hop.trim()) + .filter(Boolean) ?? []; + return hops.at(-1) ?? ''; } function platformRequestIp(request: Request): string { @@ -125,7 +135,7 @@ function xffHopBehindTrustedProxies(forwarded: string | null, trustedProxyCount: export function clientIp(request: Request, trustedProxyCount: number): string { if (process.env.VERCEL === '1') { - return firstHop(request.headers.get('x-vercel-forwarded-for')) || platformRequestIp(request) || '0.0.0.0'; + return lastHop(request.headers.get('x-vercel-forwarded-for')) || platformRequestIp(request) || '0.0.0.0'; } return xffHopBehindTrustedProxies(request.headers.get('x-forwarded-for'), trustedProxyCount) || '0.0.0.0'; } @@ -177,12 +187,15 @@ export async function createCliChallenge(request: Request): Promise<{ } catch { throw new BffError(400, 'invalid_request'); } + // Challenge creation is unauthenticated and may name any pubky, so there is + // no per-pubky bucket: whoever spends it could lock the owner out, from any + // address. The per-IP bucket alone bounds creation. Clients behind one NAT or + // VPN exit share it, as with every per-source limit on unauthenticated input. await rateLimit( config, `cli_challenge_ip:${clientIp(request, config.trustedProxyCount)}`, config.createPerIpPerMinute, ); - await rateLimit(config, `cli_challenge_pubky:${pubky}`, config.createPerPubkyPerMinute); const challengeId = randomUUID(); const nonce = Uint8Array.from(randomBytes(32)); const expiresAt = new Date(Date.now() + config.challengeTtlSeconds * 1000); diff --git a/src/server/marketplace-grant/config.test.ts b/src/server/marketplace-grant/config.test.ts index de5436a38b..acc8515b23 100644 --- a/src/server/marketplace-grant/config.test.ts +++ b/src/server/marketplace-grant/config.test.ts @@ -91,7 +91,6 @@ describe('marketplace grant BFF config', () => { expect(getCliGrantConfig()).toMatchObject({ challengeTtlSeconds: 60, createPerIpPerMinute: 10, - createPerPubkyPerMinute: 5, verifyPerIpPerMinute: 10, statusPerTokenPerMinute: 60, resultPerTokenPerMinute: 30, diff --git a/src/server/marketplace-grant/config.ts b/src/server/marketplace-grant/config.ts index 78db1791d6..3889bbbec2 100644 --- a/src/server/marketplace-grant/config.ts +++ b/src/server/marketplace-grant/config.ts @@ -76,7 +76,6 @@ export type MarketplaceGrantConfig = z.infer; const cliExtrasSchema = z.object({ challengeTtlSeconds: z.coerce.number().int().min(30).max(120).default(60), createPerIpPerMinute: z.coerce.number().int().min(1).default(10), - createPerPubkyPerMinute: z.coerce.number().int().min(1).default(5), verifyPerIpPerMinute: z.coerce.number().int().min(1).default(10), statusPerTokenPerMinute: z.coerce.number().int().min(1).default(60), resultPerTokenPerMinute: z.coerce.number().int().min(1).default(30), @@ -94,7 +93,6 @@ function cliExtrasFromEnv(): z.infer { return cliExtrasSchema.parse({ challengeTtlSeconds: process.env.SHOP_BFF_CLI_GRANT_CHALLENGE_TTL_SECONDS, createPerIpPerMinute: process.env.SHOP_BFF_CLI_GRANT_CREATE_PER_IP_PER_MINUTE, - createPerPubkyPerMinute: process.env.SHOP_BFF_CLI_GRANT_CREATE_PER_PUBKY_PER_MINUTE, verifyPerIpPerMinute: process.env.SHOP_BFF_CLI_GRANT_VERIFY_PER_IP_PER_MINUTE, statusPerTokenPerMinute: process.env.SHOP_BFF_CLI_GRANT_STATUS_PER_TOKEN_PER_MINUTE, resultPerTokenPerMinute: process.env.SHOP_BFF_CLI_GRANT_RESULT_PER_TOKEN_PER_MINUTE, diff --git a/src/server/marketplace-grant/db.ts b/src/server/marketplace-grant/db.ts index 7e002180b5..808d350571 100644 --- a/src/server/marketplace-grant/db.ts +++ b/src/server/marketplace-grant/db.ts @@ -532,6 +532,20 @@ export async function abandonCliClaim(config: MarketplaceGrantConfig, stateId: s `; } +/** + * Abandons a claim whose lease lapsed (its claimer crashed or timed out). + * The database clock decides the lapse, the same rule as the cleanup job. + */ +export async function abandonLapsedCliClaim(config: MarketplaceGrantConfig, stateId: string): Promise { + const result = await grantSql(config)` + UPDATE shop_grant_bff.cli_flow_state + SET status = 'abandoned', context_sealed = NULL, result_token_sealed = NULL, terminal_at = now(), + lease_owner = NULL, lease_until = NULL, version = version + 1 + WHERE state_id = ${stateId} AND status = 'claiming' AND lease_until <= now() + `; + return result.count === 1; +} + export async function consumeCliRateLimit( config: MarketplaceGrantConfig, bucketKey: string,