diff --git a/packages/atxp-express/src/atxpExpress.ts b/packages/atxp-express/src/atxpExpress.ts index ba5ed7b..d8252d5 100644 --- a/packages/atxp-express/src/atxpExpress.ts +++ b/packages/atxp-express/src/atxpExpress.ts @@ -12,7 +12,6 @@ import { sendProtectedResourceMetadataNode, sendOAuthMetadataNode, detectProtocol, - setDetectedCredential, getPendingPaymentChallenge, type PaymentProtocol, type ATXPConfig, @@ -20,6 +19,7 @@ import { type PendingPaymentChallenge, verifyOpaqueIdentity, parseCredentialBase64, + ProtocolSettlement, } from "@atxp/server"; export function atxpExpress(args: ATXPArgs): Router { @@ -105,36 +105,62 @@ export function atxpExpress(args: ATXPArgs): Router { return; } - // Set up ATXP context, then store detected credential if present. - // requirePayment() will find it via getDetectedCredential() and settle - // before charging, using the pricing context it has (amount, options). - return withATXPContext(config, resource, tokenCheck, () => { + // Set up ATXP context, settle any payment credential, then run route. + // Settlement happens HERE (in middleware) rather than in requirePayment() + // so the ledger is credited before any route code runs. This avoids + // footguns where tool handlers call requirePayment() multiple times + // (e.g., pre-flight balance check + post-generation charge) and the + // first call consumes the credential, leaving nothing for the second. + return withATXPContext(config, resource, tokenCheck, async () => { if (detected) { // Resolve identity for the settlement ledger credit. - // The settle must use the same sourceAccountId as the charge - // (atxpAccountId() = OAuth sub) so the ledger entries match. - // For MPP: prefer the OAuth user (recovered from opaque) over the - // wallet address from the credential's `source` field — the ledger - // is keyed by OAuth identity, not wallet address. - // For ATXP: use the sourceAccountId embedded in the credential. - // For X402: falls back to OAuth sub (credential has no identity). const sourceAccountId = (detected.protocol === 'mpp' && user) ? user : resolveIdentitySync(config, req, detected.protocol, detected.credential) || user || undefined; - setDetectedCredential({ - protocol: detected.protocol, - credential: detected.credential, - sourceAccountId, - }); - logger.info(`Stored ${detected.protocol} credential in context for requirePayment (sourceAccountId=${sourceAccountId})`); + + // Settle the credential immediately — credits the auth server's + // ledger so subsequent charge() calls in requirePayment() succeed. + const destinationAccountId = await config.destination.getAccountId(); + const settlement = new ProtocolSettlement( + config.server, + logger, + fetch.bind(globalThis), + destinationAccountId, + ); + + // For X402: the credential's parsed payload contains `accepted` — the + // exact payment requirement the client signed off on. Pass it directly + // as paymentRequirements instead of regenerating from server config. + // For MPP/ATXP: credentials are self-contained, no extra context needed. + const context: Record = { + ...(sourceAccountId && { sourceAccountId }), + destinationAccountId, + }; + + if (detected.protocol === 'x402') { + const parsed = parseCredentialBase64(detected.credential); + if (parsed?.accepted) { + context.paymentRequirements = parsed.accepted; + } + } + + try { + const result = await settlement.settle( + detected.protocol, + detected.credential, + context as Parameters[2], + ); + logger.info(`Settled ${detected.protocol} in middleware: txHash=${result.txHash}, amount=${result.settledAmount}`); + } catch (error) { + logger.error(`Middleware settlement failed for ${detected.protocol}: ${error instanceof Error ? error.message : String(error)}`); + // Don't store the credential — it's already consumed/invalid. + // requirePayment() will see no credential, charge will fail, + // and a fresh payment challenge will be issued. + } } // Intercept the response to rewrite McpServer's wrapped payment errors // back into proper JSON-RPC errors with full challenge data. - // McpServer catches McpError(-30402) and wraps it into a CallToolResult, - // discarding error.data (which contains x402/mpp challenge data). - // We detect the wrapped error and reconstruct the JSON-RPC error using - // challenge data stored in AsyncLocalStorage by buildOmniError. installPaymentResponseRewriter(res, logger); return next(); diff --git a/packages/atxp-express/src/omniChallenge.test.ts b/packages/atxp-express/src/omniChallenge.test.ts index 349a611..a247f71 100644 --- a/packages/atxp-express/src/omniChallenge.test.ts +++ b/packages/atxp-express/src/omniChallenge.test.ts @@ -268,10 +268,12 @@ describe('credential detection Express middleware', () => { expect(storedCredential).toBeNull(); }); - it('should store ATXP credential with sourceAccountId from raw JSON', async () => { + // Settlement now happens in the middleware, not in requirePayment(). + // The credential is NOT stored — it's settled immediately, so + // getDetectedCredential() returns null in route handlers. + it('should settle ATXP credential in middleware (not store for later)', async () => { let storedCredential: DetectedCredential | null = null; - // Raw JSON (not base64-encoded) const atxpCredential = JSON.stringify({ sourceAccountId: 'atxp_acct_raw123', sourceAccountToken: 'tok_raw', @@ -297,12 +299,11 @@ describe('credential detection Express middleware', () => { .send(TH.mcpToolRequest()); expect(response.status).toBe(200); - expect(storedCredential).not.toBeNull(); - expect(storedCredential!.protocol).toBe('atxp'); - expect(storedCredential!.sourceAccountId).toBe('atxp_acct_raw123'); + // Credential was settled in middleware, not stored for requirePayment() + expect(storedCredential).toBeNull(); }); - it('should store X402 credential with sourceAccountId from OAuth sub (fallback)', async () => { + it('should settle X402 credential in middleware (not store for later)', async () => { let storedCredential: DetectedCredential | null = null; const router = atxpExpress(TH.config({ @@ -325,18 +326,12 @@ describe('credential detection Express middleware', () => { .send(TH.mcpToolRequest()); expect(response.status).toBe(200); - expect(storedCredential).not.toBeNull(); - expect(storedCredential!.protocol).toBe('x402'); - // X402 credentials don't contain identity, so sourceAccountId falls back - // to the OAuth sub. This ensures the settle credits the same account that - // the charge deducts from (atxpAccountId() = OAuth sub). - expect(storedCredential!.sourceAccountId).toBe('atxp:atxp_acct_x402user'); + expect(storedCredential).toBeNull(); }); - it('should store ATXP credential with sourceAccountId from base64-encoded JSON', async () => { + it('should settle base64-encoded ATXP credential in middleware (not store for later)', async () => { let storedCredential: DetectedCredential | null = null; - // Base64-encoded JSON const atxpCredential = Buffer.from(JSON.stringify({ sourceAccountId: 'atxp_acct_b64_456', sourceAccountToken: 'tok_b64', @@ -362,9 +357,7 @@ describe('credential detection Express middleware', () => { .send(TH.mcpToolRequest()); expect(response.status).toBe(200); - expect(storedCredential).not.toBeNull(); - expect(storedCredential!.protocol).toBe('atxp'); - expect(storedCredential!.sourceAccountId).toBe('atxp_acct_b64_456'); + expect(storedCredential).toBeNull(); }); }); }); diff --git a/packages/atxp-server/src/requirePayment.test.ts b/packages/atxp-server/src/requirePayment.test.ts index 8728639..f60f6fd 100644 --- a/packages/atxp-server/src/requirePayment.test.ts +++ b/packages/atxp-server/src/requirePayment.test.ts @@ -2,9 +2,8 @@ import { describe, it, expect, vi } from 'vitest'; import { requirePayment } from './index.js'; import * as TH from './serverTestHelpers.js'; import { BigNumber } from 'bignumber.js'; -import { withATXPContext, setDetectedCredential } from './atxpContext.js'; +import { withATXPContext } from './atxpContext.js'; import { PAYMENT_REQUIRED_ERROR_CODE } from '@atxp/common'; -import { McpError } from '@modelcontextprotocol/sdk/types.js'; import { ProtocolSettlement } from './protocol.js'; describe('requirePayment', () => { @@ -341,54 +340,10 @@ describe('requirePayment', () => { }); }); - describe('settlement of detected credentials', () => { - it('should settle credential and succeed when charge passes after settlement', async () => { - const mockSettle = vi.fn().mockResolvedValue({ txHash: '0xabc', settledAmount: '10000' }); - vi.spyOn(ProtocolSettlement.prototype, 'settle').mockImplementation(mockSettle); - - const paymentServer = TH.paymentServer({ charge: vi.fn().mockResolvedValue(true) }); - const config = TH.config({ paymentServer }); - - await withATXPContext(config, new URL('https://example.com'), TH.tokenCheck(), async () => { - setDetectedCredential({ protocol: 'x402', credential: 'dGVzdA==', sourceAccountId: 'base:0x123' }); - await expect(requirePayment({ price: BigNumber(0.01) })).resolves.not.toThrow(); - expect(mockSettle).toHaveBeenCalledWith('x402', 'dGVzdA==', expect.objectContaining({ - destinationAccountId: `base:${TH.DESTINATION}`, - })); - expect(paymentServer.charge).toHaveBeenCalled(); - }); - - vi.restoreAllMocks(); - }); - - it('should throw McpError when settlement fails instead of falling through silently', async () => { - const mockSettle = vi.fn().mockRejectedValue(new Error('on-chain tx reverted')); - vi.spyOn(ProtocolSettlement.prototype, 'settle').mockImplementation(mockSettle); - - const paymentServer = TH.paymentServer({ charge: vi.fn().mockResolvedValue(false) }); - const config = TH.config({ paymentServer }); - - await withATXPContext(config, new URL('https://example.com'), TH.tokenCheck(), async () => { - setDetectedCredential({ protocol: 'x402', credential: 'dGVzdA==', sourceAccountId: 'base:0x123' }); - try { - await requirePayment({ price: BigNumber(0.01) }); - expect.fail('should have thrown'); - } catch (err: unknown) { - // Should be an McpError with settlement failure details, NOT an omni-challenge - expect(err).toBeInstanceOf(McpError); - const mcpErr = err as McpError; - expect(mcpErr.code).toBe(-32000); - expect(mcpErr.message).toContain('Payment settlement failed for x402'); - expect((mcpErr.data as Record)?.reason).toBe('on-chain tx reverted'); - // charge() should NOT have been called — we threw before reaching it - expect(paymentServer.charge).not.toHaveBeenCalled(); - } - }); - - vi.restoreAllMocks(); - }); - - it('should skip settlement and proceed to normal charge when no credential detected', async () => { + // Settlement is now handled by the middleware (atxpExpress), not requirePayment(). + // See atxpExpress.test.ts for settlement tests. + describe('requirePayment does not settle (settlement moved to middleware)', () => { + it('should charge directly without settling — middleware handles settlement before route code runs', async () => { const mockSettle = vi.fn(); vi.spyOn(ProtocolSettlement.prototype, 'settle').mockImplementation(mockSettle); @@ -396,7 +351,6 @@ describe('requirePayment', () => { const config = TH.config({ paymentServer }); await withATXPContext(config, new URL('https://example.com'), TH.tokenCheck(), async () => { - // No setDetectedCredential call — no credential on this request await expect(requirePayment({ price: BigNumber(0.01) })).resolves.not.toThrow(); expect(mockSettle).not.toHaveBeenCalled(); expect(paymentServer.charge).toHaveBeenCalled(); diff --git a/packages/atxp-server/src/requirePayment.ts b/packages/atxp-server/src/requirePayment.ts index b368711..93809fc 100644 --- a/packages/atxp-server/src/requirePayment.ts +++ b/packages/atxp-server/src/requirePayment.ts @@ -1,10 +1,8 @@ import { RequirePaymentConfig, extractNetworkFromAccountId, extractAddressFromAccountId, Network, AuthorizationServerUrl } from "@atxp/common"; -import { McpError } from "@modelcontextprotocol/sdk/types.js"; import { BigNumber } from "bignumber.js"; -import { getATXPConfig, atxpAccountId, atxpToken, getDetectedCredential, setPendingPaymentChallenge } from "./atxpContext.js"; +import { getATXPConfig, atxpAccountId, atxpToken, setPendingPaymentChallenge } from "./atxpContext.js"; import { buildPaymentOptions, omniChallengeMcpError } from "./omniChallenge.js"; import { getATXPResource } from "./atxpContext.js"; -import { ProtocolSettlement, type SettlementContext } from "./protocol.js"; import { signOpaqueIdentity } from "./opaqueIdentity.js"; export async function requirePayment(paymentConfig: RequirePaymentConfig): Promise { @@ -45,14 +43,8 @@ export async function requirePayment(paymentConfig: RequirePaymentConfig): Promi ...(token && { sourceAccountToken: token }), }; - // If a payment credential was detected on this request (retry after challenge), - // settle it now. We have the full pricing context to generate requirements. - const detectedCredential = getDetectedCredential(); - if (detectedCredential) { - await settleDetectedCredential(config, detectedCredential, charge, destinationAccountId, paymentAmount); - // After settlement, the ledger should be credited. Fall through to charge below. - } - + // Settlement is handled by the middleware (atxpExpress) before route code runs. + // The ledger is already credited by the time we get here on a retry request. config.logger.debug(`Charging ${paymentConfig.price} to ${charge.options.length} options for source ${user}`); const chargeSucceeded = await config.paymentServer.charge(charge); @@ -121,78 +113,6 @@ async function fetchAllSources( return sources; } -/** - * Settle a payment credential that was detected on this retry request. - * - * This runs inside requirePayment because it has the pricing context needed - * to generate protocol-specific settlement data: - * - X402: regenerates paymentRequirements from charge options (same as the challenge) - * - ATXP: passes sourceAccountToken and payment options - * - MPP: passes credential directly (self-contained) - * - * After settlement, the auth service credits the local ledger, so the - * subsequent charge() call will succeed. - */ -async function settleDetectedCredential( - config: NonNullable>, - detected: NonNullable>, - charge: { options: Array<{ network: string; currency: string; address: string; amount: BigNumber }>; sourceAccountId: string; destinationAccountId: string }, - destinationAccountId: string, - paymentAmount: BigNumber, -): Promise { - const { protocol, credential, sourceAccountId } = detected; - config.logger.info(`Settling ${protocol} credential in requirePayment (has pricing context)`); - - // ProtocolSettlement is instantiated per-request. This is intentional — the class - // is lightweight (stores config references only, no connections or heavy init). - // Caching would require threading persistent state through requirePayment's - // stateless call chain, for negligible benefit. - const settlement = new ProtocolSettlement( - config.server, - config.logger, - fetch.bind(globalThis), - destinationAccountId, - ); - - // Build settlement context with identity and protocol-specific data - const context: SettlementContext = { - ...(sourceAccountId && { sourceAccountId }), - destinationAccountId, - options: charge.options, - }; - - // For X402, regenerate the paymentRequirements from the destination's - // real chain addresses (not the ATXP account ID). This is the standard X402 - // pattern — the server generates requirements from its own config. - if (protocol === 'x402') { - const resource = getATXPResource()?.toString() ?? ''; - let sources: Array<{ chain: string; address: string }> = charge.options.map(o => ({ chain: o.network, address: o.address })); - try { - const fetchedSources = await config.destination.getSources(); - sources = fetchedSources.map(s => ({ chain: s.chain, address: s.address })); - } catch (err) { - config.logger.warn(`Failed to fetch destination sources for X402 settle: ${err}`); - } - const payment = buildPaymentOptions({ amount: paymentAmount, sources, resource, payeeName: config.payeeName }); - if (payment.x402.accepts.length === 0) { - config.logger.warn('X402 settle: no compatible payment options after filtering'); - } - context.paymentRequirements = payment.x402; - } - - try { - const result = await settlement.settle(protocol, credential, context); - config.logger.info(`${protocol} settlement succeeded: txHash=${result.txHash}, amount=${result.settledAmount}`); - } catch (error) { - // Settlement failed — the credential was invalid or the on-chain tx failed. - // Throw an explicit error so the client knows its credential was rejected, - // rather than silently falling through to charge (which would fail with a - // confusing insufficient_balance + new challenge). - const reason = error instanceof Error ? error.message : String(error); - config.logger.error(`${protocol} settlement failed: ${reason}`); - throw new McpError(-32000, `Payment settlement failed for ${protocol}`, { protocol, reason }); - } -} /** * Build an omni-challenge MCP error that includes ATXP-MCP + X402 + MPP data.