diff --git a/changelog.d/next/purchase-session-other-tab.fixed.md b/changelog.d/next/purchase-session-other-tab.fixed.md new file mode 100644 index 0000000000..c9ecb1cfa3 --- /dev/null +++ b/changelog.d/next/purchase-session-other-tab.fixed.md @@ -0,0 +1 @@ +When your marketplace approval expires in one tab, Shop no longer deletes a newer approval you made in another tab, so the next reload stays connected. Signing out still removes every saved approval. diff --git a/docs/ecommerce/step-up-approval.md b/docs/ecommerce/step-up-approval.md index d7d2ac4483..bd3aec9e01 100644 --- a/docs/ecommerce/step-up-approval.md +++ b/docs/ecommerce/step-up-approval.md @@ -121,6 +121,13 @@ The sections above describe the service token as empty-capability. That is no lo A refused establish is never stored; the current session stays and the dialog shows why. +- **Clearing and overwriting the persisted session.** `localStorage` and the BFF session cookie are shared across tabs, so another tab may hold a newer bearer there. Every path that removes or overwrites the record touches only what it owns: + - `clearSession` (TTL margin in `getActiveSession`, revocation in Inventory automations, checkout hold expiry, a losing or failed sign-in, a step-up for another identity) removes the persisted record only when it still carries the in-memory bearer, and asks the BFF to unpair only that session's `session_id` (`DELETE /api/marketplace/session?session_id=…`, which deletes the bridge only while it pairs that session and keeps the shared cookie otherwise). With nothing in memory it removes nothing. + - A 401 (`MarketplaceTransactionService`, Inventory Studio's purchase bearer) and a session minted for another pubky clear through `clearSessionIfBearer`, only while the bearer the request carried is still the in-memory one. + - `restorePersistedSession` expires memory before reading the slot, and removes a malformed, other-account, expired or unexpected record only while the slot still holds exactly the record it read. + - `writePersistedSession` (both establish writers) does not overwrite a different bearer that expires later: another tab minted after this tab's request left. + - Sign-out and account switch (`AuthController` local-state cleanup) call `clearForSignOut`, the one path that removes a record it did not write and unpairs the cookie unscoped, because no purchase bearer may stay at rest for the user who is leaving. + ## Re-approval routing for Bitkit and Pubky Ring sign-ins 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 homeserver-capability `needs_reauth` state cannot occur for it. diff --git a/src/app/api/marketplace/session/route.test.ts b/src/app/api/marketplace/session/route.test.ts new file mode 100644 index 0000000000..843c7b4a05 --- /dev/null +++ b/src/app/api/marketplace/session/route.test.ts @@ -0,0 +1,41 @@ +/** @vitest-environment node */ +import { NextRequest } from 'next/server'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { DELETE } from './route'; + +const bff = vi.hoisted(() => ({ cleared: true })); + +vi.mock('@/server/marketplace-grant/bff', async (importOriginal) => ({ + ...(await importOriginal()), + clearSession: async () => bff.cleared, +})); + +function del(): NextRequest { + return new NextRequest('https://shop.example/api/marketplace/session?session_id=x', { method: 'DELETE' }); +} + +function deletedCookies(response: Response): string[] { + return response.headers + .getSetCookie() + .filter((cookie) => /Max-Age=0|Expires=Thu, 01 Jan 1970/i.test(cookie)) + .map((cookie) => cookie.split('=')[0]); +} + +describe('DELETE /api/marketplace/session', () => { + beforeEach(() => { + bff.cleared = true; + }); + + it('keeps the shared session cookie when the bridge belongs to another tab’s session', async () => { + bff.cleared = false; + const response = await DELETE(del()); + expect(response.status).toBe(204); + expect(deletedCookies(response)).toEqual([]); + }); + + it('drops the session and flow cookies once the bridge is unpaired', async () => { + const response = await DELETE(del()); + expect(response.status).toBe(204); + expect(deletedCookies(response).sort()).toEqual(['__Host-shop-bff-session', '__Host-shop-marketplace-grant']); + }); +}); diff --git a/src/app/api/marketplace/session/route.ts b/src/app/api/marketplace/session/route.ts index 42b8a467f8..b7d18bef4b 100644 --- a/src/app/api/marketplace/session/route.ts +++ b/src/app/api/marketplace/session/route.ts @@ -19,11 +19,13 @@ export async function POST(request: NextRequest) { export async function DELETE(request: NextRequest) { try { - await clearSession(request, request.cookies.get(SESSION_COOKIE)?.value); + const cleared = await clearSession(request, request.cookies.get(SESSION_COOKIE)?.value); const response = new NextResponse(null, { status: 204 }); response.headers.set('cache-control', 'no-store, private'); - response.cookies.delete(SESSION_COOKIE); - response.cookies.delete(FLOW_COOKIE); + if (cleared) { + response.cookies.delete(SESSION_COOKIE); + response.cookies.delete(FLOW_COOKIE); + } return response; } catch (error) { return grantError(error); diff --git a/src/core/application/commerce/commerce.ts b/src/core/application/commerce/commerce.ts index daeb9ea98f..3f6a8f8daf 100644 --- a/src/core/application/commerce/commerce.ts +++ b/src/core/application/commerce/commerce.ts @@ -1168,16 +1168,29 @@ export class CommerceApplication { } /** - * Drops the Marketplace Transaction Service session from memory and from - * `localStorage`. Part of the sign-out teardown: the bearer token must not - * survive the user it was minted for (this is the single cleanup point). - * The published-receipt memo backs the user-visible `published` status, so - * it is cleared here too — session teardown matches the store reset, and a - * later account re-reads its receipts instead of trusting a prior session. - * The Lock Server creator frontend session is wiped here for the same reason. + * Drops the Marketplace Transaction Service session this tab holds, with + * only its own persisted record (a newer bearer another tab persisted + * stays). The published-receipt memo backs the user-visible `published` + * status, so it is cleared here too — session teardown matches the store + * reset, and a later account re-reads its receipts instead of trusting a + * prior session. The Lock Server creator frontend session is wiped here for + * the same reason. */ static clearMarketplaceSession(): void { MarketplaceSessionService.clearSession('cleared'); + this.clearSessionScopedState(); + } + + /** + * Sign-out and account switch: also removes a purchase bearer another tab + * persisted, because none may outlive the user who is leaving. + */ + static clearMarketplaceSessionForSignOut(): void { + MarketplaceSessionService.clearForSignOut(); + this.clearSessionScopedState(); + } + + private static clearSessionScopedState(): void { LocksFrontendSessionStore.clear(); this.publishedReceiptUrls.clear(); this.ownReviewHomeserverMisses.clear(); diff --git a/src/core/application/commerce/inventory-automations.ts b/src/core/application/commerce/inventory-automations.ts index 8fe36c00e3..e380cd3257 100644 --- a/src/core/application/commerce/inventory-automations.ts +++ b/src/core/application/commerce/inventory-automations.ts @@ -355,7 +355,7 @@ export class CommerceInventoryAutomationsApplication { } const identity = MarketplaceSessionService.getActiveSession(); if (identity && (identity.sessionId === id || (kind === 'purchase' && !identity.sessionId))) { - MarketplaceSessionService.clearSession('cleared'); + MarketplaceSessionService.clearSessionIfBearer(identity.token, 'cleared'); } } } diff --git a/src/core/application/commerce/inventory.test.ts b/src/core/application/commerce/inventory.test.ts index 431612ddcd..24bc4795c1 100644 --- a/src/core/application/commerce/inventory.test.ts +++ b/src/core/application/commerce/inventory.test.ts @@ -326,7 +326,7 @@ describe('CommerceInventoryApplication', () => { it('drops the purchase session, not the Studio slot, when the service rejects its bearer', async () => { purchaseSessionOnly(capturedParity.parity_request.homeserver_verified); - const clearPurchase = vi.spyOn(MarketplaceSessionService, 'clearSession').mockImplementation(() => {}); + const clearPurchase = vi.spyOn(MarketplaceSessionService, 'clearSessionIfBearer').mockImplementation(() => {}); const clearStudio = vi.spyOn(MarketplaceInventorySessionService, 'clearSession').mockImplementation(() => {}); vi.mocked(MarketplaceShopClientService.adjustInventory).mockResolvedValue({ ok: false, @@ -341,7 +341,7 @@ describe('CommerceInventoryApplication', () => { }); expect(result.status).toBe('grant-needed'); - expect(clearPurchase).toHaveBeenCalledWith('rejected'); + expect(clearPurchase).toHaveBeenCalledWith(TOKEN, 'rejected'); expect(clearStudio).not.toHaveBeenCalled(); }); diff --git a/src/core/controllers/auth/auth.test.ts b/src/core/controllers/auth/auth.test.ts index ab15a6e958..d2a1e03fad 100644 --- a/src/core/controllers/auth/auth.test.ts +++ b/src/core/controllers/auth/auth.test.ts @@ -1896,7 +1896,7 @@ describe('AuthController', () => { const clearCookiesSpy = await spyOnClearCookies(); const clearAllQueryClientsSpy = await spyOnClearAllQueryClients(); const resetSpy = vi.spyOn(PubkySpecsSingleton, 'reset'); - const clearMarketplaceSessionSpy = vi.spyOn(CommerceApplication, 'clearMarketplaceSession'); + const clearMarketplaceSessionSpy = vi.spyOn(CommerceApplication, 'clearMarketplaceSessionForSignOut'); const resetTtlSpy = vi.spyOn(TtlCoordinator, 'resetInstance'); const resetStreamSpy = vi.spyOn(StreamCoordinator, 'resetInstance'); const resetNotifCoordSpy = vi.spyOn(NotificationCoordinator, 'resetInstance'); diff --git a/src/core/controllers/auth/auth.ts b/src/core/controllers/auth/auth.ts index 5e10af135b..b81b650d94 100644 --- a/src/core/controllers/auth/auth.ts +++ b/src/core/controllers/auth/auth.ts @@ -858,8 +858,8 @@ export class AuthController { // Reset singletons PubkySpecsSingleton.reset(); - // The marketplace transaction-service bearer token lives in memory only; drop it with the user. - CommerceController.clearMarketplaceSession(); + // No marketplace bearer may stay at rest for the user who is leaving. + CommerceController.clearMarketplaceSessionForSignOut(); // Same rule for the encrypted-messaging homeserver session and its live link handles. MessagingApplication.clearMessagingSession(); useMessagingStore.getState().clearMessagingEnabled(); diff --git a/src/core/controllers/commerce/commerce.restore-guard.test.ts b/src/core/controllers/commerce/commerce.restore-guard.test.ts index d644636784..8a1ce512e7 100644 --- a/src/core/controllers/commerce/commerce.restore-guard.test.ts +++ b/src/core/controllers/commerce/commerce.restore-guard.test.ts @@ -33,7 +33,7 @@ function otherTabPersists(capabilities: string): string { describe('purchase-session restore never downgrades memory or the store mirror', () => { beforeEach(() => { - MarketplaceSessionService.clearSession(); + MarketplaceSessionService.clearForSignOut(); useCommerceStore.getState().reset(); }); @@ -99,3 +99,34 @@ describe('purchase-session restore never downgrades memory or the store mirror', expect(MarketplaceSessionService.getActiveSession()?.token).toBe(OTHER_TAB_TOKEN); }); }); + +describe('clearing the purchase session from the controller', () => { + beforeEach(() => { + MarketplaceSessionService.clearForSignOut(); + useCommerceStore.getState().reset(); + }); + + function thisTabHoldsAndOtherTabPersistsNewer(): string { + const info = MarketplaceSessionService.establishClaimedGrantSession( + { token: WIDE_TOKEN, pubky: PUBKY, capabilities: parity, expiresAt: inOneDay() }, + PUBKY, + ); + CommerceController.writeMarketplaceSessionStore(info); + return otherTabPersists(parity); + } + + it('a failed or losing sign-in clears only this tab’s bearer', () => { + const newer = thisTabHoldsAndOtherTabPersistsNewer(); + CommerceController.clearMarketplaceSession(); + expect(MarketplaceSessionService.getActiveSession()).toBeNull(); + expect(window.localStorage.getItem(MARKETPLACE_SESSION_STORAGE_KEY)).toBe(newer); + }); + + it('sign-out leaves no purchase bearer at rest, whichever tab persisted it', () => { + thisTabHoldsAndOtherTabPersistsNewer(); + CommerceController.clearMarketplaceSessionForSignOut(); + expect(MarketplaceSessionService.getActiveSession()).toBeNull(); + expect(useCommerceStore.getState().marketplaceSession).toBeNull(); + expect(window.localStorage.getItem(MARKETPLACE_SESSION_STORAGE_KEY)).toBeNull(); + }); +}); diff --git a/src/core/controllers/commerce/commerce.ts b/src/core/controllers/commerce/commerce.ts index 4aa9613658..d98acdcbee 100644 --- a/src/core/controllers/commerce/commerce.ts +++ b/src/core/controllers/commerce/commerce.ts @@ -266,8 +266,10 @@ export class CommerceController { } /** - * Drops the purchase bearer, the inventory bearer, and both store mirrors. - * Sign-out and failed sign-in go through here. Identity 401 uses + * Drops the purchase bearer this tab holds (only its own persisted + * record), the inventory bearer, and both store mirrors. A failed or losing + * sign-in goes through here; sign-out uses + * {@link clearMarketplaceSessionForSignOut}. Identity 401 uses * `onMarketplaceSessionEnded` and must not reach this. Checkout TTL uses * `clearIdentitySession` so a hold expiry cannot log the seller out of * Inventory Studio. @@ -278,6 +280,13 @@ export class CommerceController { this.clearInventorySession(); } + /** Sign-out and account switch (`AuthController` local-state cleanup). */ + static clearMarketplaceSessionForSignOut(): void { + CommerceApplication.clearMarketplaceSessionForSignOut(); + this.clearMarketplaceSessionStore(); + this.clearInventorySession(); + } + /** Identity checkout bearer only. Leaves `pubky.marketplace.inventory-session.v1` in place. */ static clearIdentitySession(): void { CommerceApplication.clearMarketplaceSession(); diff --git a/src/core/services/marketplace/marketplace-grant-client.ts b/src/core/services/marketplace/marketplace-grant-client.ts index 7aac421ed4..50f752be42 100644 --- a/src/core/services/marketplace/marketplace-grant-client.ts +++ b/src/core/services/marketplace/marketplace-grant-client.ts @@ -56,8 +56,15 @@ export async function pairMarketplaceBffSession(session: { if (!response.ok) throw new Error('marketplace_session_pair_failed'); } -export async function clearMarketplaceBffSession(): Promise { - await fetch('/api/marketplace/session', { +/** + * Unpairs the BFF session. With `ownedSessionId` the BFF unpairs only when + * its bridge still holds that marketplace session: the cookie is shared + * across tabs, and another tab may have paired a newer one. Without it (sign- + * out) the bridge is removed whatever it holds. + */ +export async function clearMarketplaceBffSession(ownedSessionId?: string): Promise { + const query = ownedSessionId === undefined ? '' : `?session_id=${encodeURIComponent(ownedSessionId)}`; + await fetch(`/api/marketplace/session${query}`, { method: 'DELETE', credentials: 'same-origin', }).catch(() => undefined); diff --git a/src/core/services/marketplace/marketplace-inventory-session.ts b/src/core/services/marketplace/marketplace-inventory-session.ts index aacdb1142f..2a1e0c6155 100644 --- a/src/core/services/marketplace/marketplace-inventory-session.ts +++ b/src/core/services/marketplace/marketplace-inventory-session.ts @@ -247,7 +247,7 @@ export class MarketplaceInventorySessionService { /** Drops the session behind a bearer the service refused. */ static clearRejectedBearer(bearer: InventoryBearer): void { if (bearer.source === 'purchase') { - MarketplaceSessionService.clearSession('rejected'); + MarketplaceSessionService.clearSessionIfBearer(bearer.token, 'rejected'); return; } this.clearSession('rejected'); diff --git a/src/core/services/marketplace/marketplace-session.persistence.test.ts b/src/core/services/marketplace/marketplace-session.persistence.test.ts new file mode 100644 index 0000000000..4a04f58748 --- /dev/null +++ b/src/core/services/marketplace/marketplace-session.persistence.test.ts @@ -0,0 +1,183 @@ +import { afterEach, beforeEach, describe, expect, it, type MockInstance, vi } from 'vitest'; +import captured from '@/test/fixtures/auth/marketplace-grant-priv-parity.staging.json'; +import { MARKETPLACE_SESSION_STORAGE_KEY, MarketplaceSessionService } from './marketplace-session'; + +vi.mock('@/config/commerce', async () => { + const actual = await vi.importActual('@/config/commerce'); + return { + ...actual, + getCommerceAdapterMode: () => 'transaction-service', + getMarketplaceUrl: () => 'http://127.0.0.1:8080', + }; +}); + +const config = vi.hoisted(() => ({ grantFlow: true })); +vi.mock('@/libs/runtime-config/runtime-config', async (importOriginal) => ({ + ...(await importOriginal()), + getMarketplaceGrantFlowEnabled: () => config.grantFlow, +})); + +const PUBKY = 'y'.repeat(52); +const THIS_TAB = { token: 'A'.repeat(43), sessionId: '11111111-1111-4111-8111-111111111111' }; +const OTHER_TAB = { token: 'B'.repeat(43), sessionId: '22222222-2222-4222-8222-222222222222' }; +const parity = captured.parity_request.homeserver_verified; +const HOUR = 3_600_000; + +function iso(ms: number): string { + return new Date(ms).toISOString(); +} + +/** This tab holds a session expiring in `ttlMs`, persisted by its own writer. */ +function thisTabHolds(ttlMs: number) { + return MarketplaceSessionService.establishClaimedGrantSession( + { ...THIS_TAB, pubky: PUBKY, capabilities: parity, expiresAt: iso(Date.now() + ttlMs) }, + PUBKY, + ); +} + +/** Another tab's writer leaves its newer bearer in the shared slot. */ +function otherTabPersists(ttlMs: number): string { + const blob = JSON.stringify({ ...OTHER_TAB, pubky: PUBKY, capabilities: parity, expiresAt: iso(Date.now() + ttlMs) }); + window.localStorage.setItem(MARKETPLACE_SESSION_STORAGE_KEY, blob); + return blob; +} + +function stored(): string | null { + return window.localStorage.getItem(MARKETPLACE_SESSION_STORAGE_KEY); +} + +let fetchSpy: MockInstance; + +function bffDeletes(): string[] { + return fetchSpy.mock.calls.filter(([, init]) => init?.method === 'DELETE').map(([url]) => String(url)); +} + +describe('purchase-session persistence: a tab only removes the record it owns', () => { + beforeEach(() => { + config.grantFlow = true; + MarketplaceSessionService.clearForSignOut(); + window.localStorage.clear(); + fetchSpy = vi.spyOn(globalThis, 'fetch').mockImplementation(async () => new Response(null, { status: 204 })); + }); + + afterEach(() => { + vi.useRealTimers(); + fetchSpy.mockRestore(); + }); + + it('expiry in this tab keeps the newer bearer another tab persisted', () => { + vi.useFakeTimers(); + thisTabHolds(HOUR); + vi.advanceTimersByTime(30 * 60_000); + const newer = otherTabPersists(24 * HOUR); + fetchSpy.mockClear(); + + vi.advanceTimersByTime(30 * 60_000); + expect(MarketplaceSessionService.getActiveSession()).toBeNull(); + + expect(stored()).toBe(newer); + expect(bffDeletes()).toEqual([`/api/marketplace/session?session_id=${THIS_TAB.sessionId}`]); + }); + + it('adopting the newer persisted bearer while memory expires keeps it persisted', () => { + vi.useFakeTimers(); + thisTabHolds(HOUR); + vi.advanceTimersByTime(30 * 60_000); + const newer = otherTabPersists(24 * HOUR); + vi.advanceTimersByTime(30 * 60_000); + + const restored = MarketplaceSessionService.restorePersistedSession(PUBKY); + + expect(restored?.capabilities).toBe(parity); + expect(MarketplaceSessionService.getActiveSession()?.token).toBe(OTHER_TAB.token); + expect(stored()).toBe(newer); + }); + + it('expiry still removes this tab’s own record', () => { + vi.useFakeTimers(); + thisTabHolds(HOUR); + vi.advanceTimersByTime(HOUR); + + expect(MarketplaceSessionService.getActiveSession()).toBeNull(); + expect(stored()).toBeNull(); + }); + + it('a 401 for the bearer a request carried never clears a newer session adopted meanwhile', () => { + thisTabHolds(HOUR); + const newer = otherTabPersists(24 * HOUR); + vi.useFakeTimers(); + vi.advanceTimersByTime(HOUR); + MarketplaceSessionService.restorePersistedSession(PUBKY); + + MarketplaceSessionService.clearSessionIfBearer(THIS_TAB.token, 'rejected'); + + expect(MarketplaceSessionService.getActiveSession()?.token).toBe(OTHER_TAB.token); + expect(stored()).toBe(newer); + }); + + it('a 401 for the current bearer removes it and only its record', () => { + thisTabHolds(HOUR); + MarketplaceSessionService.clearSessionIfBearer(THIS_TAB.token, 'rejected'); + expect(MarketplaceSessionService.getActiveSession()).toBeNull(); + expect(stored()).toBeNull(); + }); + + it('an explicit clear in this tab leaves another tab’s newer record', () => { + thisTabHolds(HOUR); + const newer = otherTabPersists(24 * HOUR); + MarketplaceSessionService.clearSession('cleared'); + expect(stored()).toBe(newer); + }); + + it('a clear with nothing in memory removes nothing and unpairs nothing', () => { + const newer = otherTabPersists(24 * HOUR); + fetchSpy.mockClear(); + MarketplaceSessionService.clearSession('cleared'); + expect(stored()).toBe(newer); + expect(bffDeletes()).toEqual([]); + }); + + it('a session without an id never asks the BFF to unpair the shared cookie', () => { + MarketplaceSessionService.establishClaimedGrantSession( + { token: THIS_TAB.token, pubky: PUBKY, capabilities: parity, expiresAt: iso(Date.now() + HOUR) }, + PUBKY, + ); + fetchSpy.mockClear(); + MarketplaceSessionService.clearSession('rejected'); + expect(bffDeletes()).toEqual([]); + }); + + it('a mint that lands after another tab persisted a newer bearer does not overwrite it', () => { + const newer = otherTabPersists(24 * HOUR); + thisTabHolds(HOUR); + expect(MarketplaceSessionService.getActiveSession()?.token).toBe(THIS_TAB.token); + expect(stored()).toBe(newer); + }); + + it('a mint overwrites an older record', () => { + otherTabPersists(HOUR); + thisTabHolds(24 * HOUR); + expect(JSON.parse(stored()!)).toMatchObject({ token: THIS_TAB.token }); + }); + + it('sign-out removes whatever bearer is at rest and unpairs the cookie unscoped', () => { + thisTabHolds(HOUR); + otherTabPersists(24 * HOUR); + fetchSpy.mockClear(); + + MarketplaceSessionService.clearForSignOut(); + + expect(MarketplaceSessionService.getActiveSession()).toBeNull(); + expect(stored()).toBeNull(); + expect(bffDeletes()).toEqual(['/api/marketplace/session']); + }); + + it('restore drops an expired record it read, and only that record', () => { + vi.useFakeTimers(); + const expired = otherTabPersists(10_000); + vi.advanceTimersByTime(20_000); + expect(stored()).toBe(expired); + expect(MarketplaceSessionService.restorePersistedSession(PUBKY)).toBeNull(); + expect(stored()).toBeNull(); + }); +}); diff --git a/src/core/services/marketplace/marketplace-session.ts b/src/core/services/marketplace/marketplace-session.ts index d0fc0bf7ef..4c60aa02a5 100644 --- a/src/core/services/marketplace/marketplace-session.ts +++ b/src/core/services/marketplace/marketplace-session.ts @@ -363,18 +363,21 @@ export class MarketplaceSessionService { */ static restorePersistedSession(expectedPubky: string): MarketplaceSessionInfo | null { if (!isDurableCommerceMode(getCommerceAdapterMode())) return null; + // Expire memory before reading the slot, so the expiry cleanup runs + // against the old bearer and never against the candidate read below. + this.getActiveSession(); const raw = this.readStorage(); if (raw === null) return null; const parsed = sessionResponseSchema.safeParse(this.parseJson(raw)); if (!parsed.success || parsed.data.pubky !== expectedPubky) { - this.removePersistedSession(); + this.removePersistedRecordIfUnchanged(raw); return null; } const { token, sessionId, pubky, capabilities, expiresAt } = parsed.data; const expiresAtMs = Date.parse(expiresAt); if (Date.now() >= expiresAtMs - SESSION_EXPIRY_MARGIN_MS) { - this.removePersistedSession(); + this.removePersistedRecordIfUnchanged(raw); return null; } const rejection = this.replacementRejection( @@ -384,7 +387,7 @@ export class MarketplaceSessionService { 'restorePersistedSession', ); if (rejection === 'unexpected_capabilities') { - this.removePersistedSession(); + this.removePersistedRecordIfUnchanged(raw); const current = this.getActiveSession(); return current?.pubky === expectedPubky ? this.toPublicInfo(current) : null; } @@ -417,15 +420,46 @@ export class MarketplaceSessionService { return this.session; } - /** Drops the session from memory AND storage. Called on sign-out and on server-side 401. */ + /** + * Drops the in-memory session (TTL margin, revocation, a refused bearer) + * and only the persisted record and BFF pairing that belong to it. + * `localStorage` and the BFF cookie are shared across tabs: another tab may + * already hold a newer bearer there, and it must survive this tab's expiry. + * Sign-out and account switch use {@link clearForSignOut} instead. + */ static clearSession(reason: MarketplaceSessionEndedReason = 'cleared'): void { + const ended = this.session; + this.session = null; + resetMarketplaceNotificationDiagnostics(); + if (!ended) return; + this.removePersistedSessionIfOwned(ended.token); + if (getMarketplaceGrantFlowEnabled() && ended.sessionId) void clearMarketplaceBffSession(ended.sessionId); + this.notifySessionEnded({ reason, issuedAt: ended.issuedAt }); + } + + /** + * Drops the session only when `token` is still the in-memory bearer. A + * 401 answers the bearer a request carried; if the session was replaced + * while the request was in flight, the newer one stays. + */ + static clearSessionIfBearer(token: string, reason: MarketplaceSessionEndedReason): void { + if (this.session?.token !== token) return; + this.clearSession(reason); + } + + /** + * Sign-out and account switch: the user leaves, so no purchase bearer may + * stay at rest for this browser, whichever tab persisted it. The only path + * that removes a record it did not write. + */ + static clearForSignOut(): void { const ended = this.session; this.session = null; resetMarketplaceNotificationDiagnostics(); this.removePersistedSession(); if (getMarketplaceGrantFlowEnabled()) void clearMarketplaceBffSession(); if (!ended) return; - this.notifySessionEnded({ reason, issuedAt: ended.issuedAt }); + this.notifySessionEnded({ reason: 'cleared', issuedAt: ended.issuedAt }); } private static toPublicInfo(session: StoredMarketplaceSession): MarketplaceSessionInfo { @@ -524,8 +558,18 @@ export class MarketplaceSessionService { // localStorage access is wrapped because browsers can refuse it (disabled // storage, private-mode quirks); a session that cannot persist is still a // working in-memory session, so persistence failures only log. + /** + * Persists a freshly minted session unless the slot already holds a + * different bearer that outlives it: another tab minted after this tab's + * request left, and its newer record must not be overwritten. + */ private static writePersistedSession(session: z.infer): void { if (typeof window === 'undefined') return; + const stored = this.persistedBearer(); + if (stored && stored.token !== session.token && stored.expiresAtMs > Date.parse(session.expiresAt)) { + Logger.warn('Kept a newer marketplace session another tab persisted.'); + return; + } try { window.localStorage.setItem(MARKETPLACE_SESSION_STORAGE_KEY, JSON.stringify(session)); } catch { @@ -533,6 +577,30 @@ export class MarketplaceSessionService { } } + /** The bearer and expiry of the persisted record, or null when there is none to compare. */ + private static persistedBearer(): { token: string; expiresAtMs: number } | null { + const raw = this.readStorage(); + if (raw === null) return null; + const value = this.parseJson(raw); + if (typeof value !== 'object' || value === null) return null; + const { token, expiresAt } = value as { token?: unknown; expiresAt?: unknown }; + if (typeof token !== 'string') return null; + const expiresAtMs = typeof expiresAt === 'string' ? Date.parse(expiresAt) : Number.NaN; + return { token, expiresAtMs: Number.isNaN(expiresAtMs) ? 0 : expiresAtMs }; + } + + /** Removes the persisted record only when it still carries `token`. */ + private static removePersistedSessionIfOwned(token: string): void { + if (this.persistedBearer()?.token !== token) return; + this.removePersistedSession(); + } + + /** Removes the persisted record only when it is still exactly `raw`. */ + private static removePersistedRecordIfUnchanged(raw: string): void { + if (this.readStorage() !== raw) return; + this.removePersistedSession(); + } + private static removePersistedSession(): void { if (typeof window === 'undefined') return; try { diff --git a/src/core/services/marketplace/marketplace-transaction.test.ts b/src/core/services/marketplace/marketplace-transaction.test.ts index 222d195fa9..afd8550c81 100644 --- a/src/core/services/marketplace/marketplace-transaction.test.ts +++ b/src/core/services/marketplace/marketplace-transaction.test.ts @@ -11,7 +11,8 @@ import sellerDropCapture from '@/test/fixtures/commerce/live/seller-drop-v0621.j import { LIVE_ORDERS_WIRE_FIXTURE } from '@/test/fixtures/commerce/orders.wire'; import { asOpaque } from '@/test-utils/type-assertions'; import { MARKETPLACE_NOTIFICATION_TYPE_MAX_LENGTH, marketplaceNotificationSchema } from './marketplace-projections'; -import { MarketplaceSessionService } from './marketplace-session'; +import { MARKETPLACE_SESSION_STORAGE_KEY, MarketplaceSessionService } from './marketplace-session'; +import { MARKETPLACE_SESSION_GRANT } from './marketplace-session-grant'; import { MarketplaceTransactionService } from './marketplace-transaction'; const ACTOR = 'y'.repeat(52); @@ -194,6 +195,31 @@ describe('MarketplaceTransactionService.execute', () => { expect(MarketplaceSessionService.getActiveSession()).toBeNull(); }); + it('a 401 for the bearer a request carried keeps a newer session adopted while it was in flight', async () => { + await establishSession(); + const newer = 'B'.repeat(43); + vi.mocked(fetch).mockImplementationOnce(async () => { + MarketplaceSessionService.establishClaimedGrantSession( + { + token: newer, + pubky: ACTOR, + capabilities: MARKETPLACE_SESSION_GRANT, + expiresAt: new Date(Date.now() + 2 * 86_400_000).toISOString(), + }, + ACTOR, + ); + return jsonResponse(401, { error: { message: 'The session is invalid or expired.' } }); + }); + + await expect(MarketplaceTransactionService.execute(ACTOR, bidCommand())).rejects.toMatchObject({ + code: 'SESSION_EXPIRED', + }); + expect(MarketplaceSessionService.getActiveSession()?.token).toBe(newer); + expect(JSON.parse(window.localStorage.getItem(MARKETPLACE_SESSION_STORAGE_KEY) ?? '{}')).toMatchObject({ + token: newer, + }); + }); + it('refuses to act for a different pubky than the session was minted for', async () => { await establishSession(); diff --git a/src/core/services/marketplace/marketplace-transaction.ts b/src/core/services/marketplace/marketplace-transaction.ts index 85e8ccc173..2d98a861b1 100644 --- a/src/core/services/marketplace/marketplace-transaction.ts +++ b/src/core/services/marketplace/marketplace-transaction.ts @@ -213,7 +213,7 @@ export class MarketplaceTransactionService { ErrorService.Marketplace, 'execute', ); - this.throwIfSessionRejected(response.status, 'execute'); + this.throwIfSessionRejected(response.status, 'execute', session.token); const raw = await parseResponseOrThrow(response, ErrorService.Marketplace, 'execute', url); const parsed = marketplaceCommandResponseSchema.safeParse(toCamelCaseWire(raw)); if (!parsed.success) { @@ -541,7 +541,7 @@ export class MarketplaceTransactionService { ErrorService.Marketplace, operation, ); - this.throwIfSessionRejected(response.status, operation); + this.throwIfSessionRejected(response.status, operation, session.token); if (!response.ok) { throw await this.digitalReadRefusal(response, operation); } @@ -649,7 +649,7 @@ export class MarketplaceTransactionService { ErrorService.Marketplace, operation, ); - this.throwIfSessionRejected(response.status, operation); + this.throwIfSessionRejected(response.status, operation, session.token); if (!response.ok) { await this.throwPickupRefusal(response, operation); } @@ -814,7 +814,7 @@ export class MarketplaceTransactionService { ErrorService.Marketplace, operation, ); - this.throwIfSessionRejected(response.status, operation); + this.throwIfSessionRejected(response.status, operation, session.token); if (response.status === HttpStatusCode.FORBIDDEN || response.status === HttpStatusCode.SERVICE_UNAVAILABLE) { const code = await response .json() @@ -1247,7 +1247,7 @@ export class MarketplaceTransactionService { ErrorService.Marketplace, operation, ); - this.throwIfSessionRejected(response.status, operation); + this.throwIfSessionRejected(response.status, operation, session.token); if (operation === 'confirmBitcoinPayment' || operation === 'resolveBitcoinPayment') { await this.throwSellerPaymentReviewError(response, operation); } else { @@ -1368,7 +1368,7 @@ export class MarketplaceTransactionService { ErrorService.Marketplace, operation, ); - this.throwIfSessionRejected(response.status, operation); + this.throwIfSessionRejected(response.status, operation, session.token); if (options.nullOnNotFound && response.status === HttpStatusCode.NOT_FOUND) return null; if (options.nullOnForbidden && response.status === HttpStatusCode.FORBIDDEN) return null; const raw = await parseResponseOrThrow(response, ErrorService.Marketplace, operation, url); @@ -1402,7 +1402,7 @@ export class MarketplaceTransactionService { } if (session.pubky !== actor) { // A session minted for another key must never act for the current user. - MarketplaceSessionService.clearSession('rejected'); + MarketplaceSessionService.clearSessionIfBearer(session.token, 'rejected'); throw Err.auth(AuthErrorCode.FORBIDDEN, 'The marketplace session belongs to a different pubky.', { service: ErrorService.Marketplace, operation, @@ -1415,9 +1415,9 @@ export class MarketplaceTransactionService { * Only the auth middleware answers 401 (command failures map to 403/404/409/422), * so a 401 always means the session is gone server-side — drop the local copy. */ - private static throwIfSessionRejected(statusCode: number, operation: string): void { + private static throwIfSessionRejected(statusCode: number, operation: string, token: string): void { if (statusCode !== HttpStatusCode.UNAUTHORIZED) return; - MarketplaceSessionService.clearSession('rejected'); + MarketplaceSessionService.clearSessionIfBearer(token, 'rejected'); throw Err.auth( AuthErrorCode.SESSION_EXPIRED, 'The marketplace session expired. Approve the marketplace connection on your signer and try again.', diff --git a/src/server/marketplace-grant/bff.clear-session.test.ts b/src/server/marketplace-grant/bff.clear-session.test.ts new file mode 100644 index 0000000000..05a9878d2f --- /dev/null +++ b/src/server/marketplace-grant/bff.clear-session.test.ts @@ -0,0 +1,83 @@ +/** @vitest-environment node */ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { clearSession } from './bff'; +import { makeBoundCookie } from './crypto'; + +const ORIGIN = 'https://shop.example'; + +const bridges = vi.hoisted(() => ({ + rows: new Map(), + deleteBridge: vi.fn(), + deleteBridgeForSession: vi.fn(), +})); + +vi.mock('./config', () => ({ + getMarketplaceGrantConfig: () => ({ allowedOrigins: ['https://shop.example'] }), +})); + +vi.mock('./db', async (importOriginal) => ({ + ...(await importOriginal()), + deleteBridge: bridges.deleteBridge, + deleteBridgeForSession: bridges.deleteBridgeForSession, +})); + +const THIS_TAB_SESSION = '11111111-1111-4111-8111-111111111111'; +const OTHER_TAB_SESSION = '22222222-2222-4222-8222-222222222222'; +const BRIDGE_ID = '33333333-3333-4333-8333-333333333333'; + +function request(query = ''): Request { + return new Request(`${ORIGIN}/api/marketplace/session${query}`, { + method: 'DELETE', + headers: { origin: ORIGIN, 'sec-fetch-site': 'same-origin' }, + }); +} + +describe('BFF session unpair is scoped to the marketplace session a tab owns', () => { + const cookie = makeBoundCookie(BRIDGE_ID).value; + + beforeEach(() => { + bridges.rows = new Map([[BRIDGE_ID, OTHER_TAB_SESSION]]); + bridges.deleteBridge.mockReset().mockImplementation(async (_config: unknown, id: string) => { + bridges.rows.delete(id); + }); + bridges.deleteBridgeForSession + .mockReset() + .mockImplementation(async (_config: unknown, id: string, sessionId: string) => { + if (bridges.rows.get(id) === sessionId) { + bridges.rows.delete(id); + return true; + } + return !bridges.rows.has(id); + }); + }); + + it('keeps another tab’s newer pairing and its cookie when this tab’s session expires', async () => { + const cleared = await clearSession(request(`?session_id=${THIS_TAB_SESSION}`), cookie); + + expect(cleared).toBe(false); + expect(bridges.rows.get(BRIDGE_ID)).toBe(OTHER_TAB_SESSION); + expect(bridges.deleteBridge).not.toHaveBeenCalled(); + expect(bridges.deleteBridgeForSession).toHaveBeenCalledWith(expect.anything(), BRIDGE_ID, THIS_TAB_SESSION); + }); + + it('unpairs the bridge when it still holds this tab’s session', async () => { + bridges.rows.set(BRIDGE_ID, THIS_TAB_SESSION); + expect(await clearSession(request(`?session_id=${THIS_TAB_SESSION}`), cookie)).toBe(true); + expect(bridges.rows.has(BRIDGE_ID)).toBe(false); + }); + + it('sign-out (no session id) unpairs whatever the cookie names', async () => { + expect(await clearSession(request(), cookie)).toBe(true); + expect(bridges.deleteBridge).toHaveBeenCalledWith(expect.anything(), BRIDGE_ID); + expect(bridges.deleteBridgeForSession).not.toHaveBeenCalled(); + }); + + it('refuses a malformed session id before touching a bridge', async () => { + await expect(clearSession(request('?session_id=not%0Aan-id'), cookie)).rejects.toMatchObject({ + status: 400, + code: 'invalid_request', + }); + expect(bridges.deleteBridge).not.toHaveBeenCalled(); + expect(bridges.deleteBridgeForSession).not.toHaveBeenCalled(); + }); +}); diff --git a/src/server/marketplace-grant/bff.ts b/src/server/marketplace-grant/bff.ts index e7d2bd4717..2da2dfa827 100644 --- a/src/server/marketplace-grant/bff.ts +++ b/src/server/marketplace-grant/bff.ts @@ -22,6 +22,7 @@ import { bindFlow, completeClaim, deleteBridge, + deleteBridgeForSession, getBridge, getFlow, insertCreatingFlow, @@ -276,11 +277,26 @@ export async function cancelFlow( await terminalizeFlow(config, stateId, 'cancelled'); } -export async function clearSession(request: Request, sessionCookie: string | undefined): Promise { +/** + * Unpairs the session cookie's bridge. A `session_id` query scopes it to that + * marketplace session, so one tab's expired bearer cannot unpair a newer + * session another tab paired on the shared cookie. Returns whether the + * cookie should be dropped. + */ +export async function clearSession(request: Request, sessionCookie: string | undefined): Promise { const config = requiredConfig(); assertSameOrigin(request, config); + const ownedSessionId = new URL(request.url).searchParams.get('session_id'); + if (ownedSessionId !== null && !marketplaceSessionIdSchema.safeParse(ownedSessionId).success) { + throw new BffError(400, 'invalid_request'); + } const parsed = parseBoundCookie(sessionCookie); - if (parsed) await deleteBridge(config, parsed.id); + if (!parsed) return true; + if (ownedSessionId === null) { + await deleteBridge(config, parsed.id); + return true; + } + return await deleteBridgeForSession(config, parsed.id, ownedSessionId); } export function mapBffError(error: unknown): { status: number; code: string; retryAfterSeconds?: number } { diff --git a/src/server/marketplace-grant/db.ts b/src/server/marketplace-grant/db.ts index 808d350571..26cdae540d 100644 --- a/src/server/marketplace-grant/db.ts +++ b/src/server/marketplace-grant/db.ts @@ -112,6 +112,26 @@ export async function deleteBridge(config: MarketplaceGrantConfig, bridgeId: str await grantSql(config)`DELETE FROM shop_grant_bff.session_bridge WHERE bridge_id = ${bridgeId}`; } +/** + * Deletes the bridge only while it still pairs `marketplaceSessionId`. + * Returns whether the cookie naming this bridge may be dropped: the row was + * deleted, or no row is left for it. + */ +export async function deleteBridgeForSession( + config: MarketplaceGrantConfig, + bridgeId: string, + marketplaceSessionId: string, +): Promise { + const db = grantSql(config); + const deleted = await db` + DELETE FROM shop_grant_bff.session_bridge + WHERE bridge_id = ${bridgeId} AND marketplace_session_id = ${marketplaceSessionId} + `; + if (deleted.count === 1) return true; + const remaining = await db`SELECT 1 FROM shop_grant_bff.session_bridge WHERE bridge_id = ${bridgeId}`; + return remaining.length === 0; +} + export async function insertCreatingFlow( config: MarketplaceGrantConfig, row: {