diff --git a/.changeset/upset-waves-rescue.md b/.changeset/upset-waves-rescue.md new file mode 100644 index 00000000..a15bc296 --- /dev/null +++ b/.changeset/upset-waves-rescue.md @@ -0,0 +1,5 @@ +--- +"@godaddy/app-connect": patch +--- + +Verification middlewares for tanstack now pass raw text of request body through to verification function. diff --git a/packages/app-connect/src/tanstack/index.test.ts b/packages/app-connect/src/tanstack/index.test.ts new file mode 100644 index 00000000..19be216d --- /dev/null +++ b/packages/app-connect/src/tanstack/index.test.ts @@ -0,0 +1,383 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// Capture the server callback passed to createMiddleware().server() +let capturedServerFn: (args: { + next: (opts?: unknown) => unknown; + request: Request; +}) => Promise; + +vi.mock('@tanstack/react-start', () => ({ + createMiddleware: () => ({ + server: (fn: typeof capturedServerFn) => { + capturedServerFn = fn; + return 'middleware-instance'; + }, + }), +})); + +vi.mock('../utils/verification', () => ({ + verifyAction: vi.fn(), +})); + +vi.mock('../utils/webhook', () => ({ + verifyWebhookSubscription: vi.fn(), +})); + +import { verifyAction } from '../utils/verification'; +import { verifyWebhookSubscription } from '../utils/webhook'; +import { createActionMiddleware, createWebhookMiddleware } from './index'; + +const mockVerifyAction = vi.mocked(verifyAction); +const mockVerifyWebhookSubscription = vi.mocked(verifyWebhookSubscription); + +function createMockRequest( + body?: Record, + headers?: Record, +): Request { + const requestInit: RequestInit = { + method: 'POST', + headers: { + 'content-type': 'application/json', + ...(headers ?? {}), + }, + }; + + if (body !== undefined) { + requestInit.body = JSON.stringify(body); + } + + return new Request('https://example.com/api/test?foo=bar', requestInit); +} + +describe('createActionMiddleware', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should call next without arguments on success', async () => { + const body = { action: 'test', data: { key: 'value' } }; + const request = createMockRequest(body); + const next = vi.fn().mockResolvedValue('next-result'); + + mockVerifyAction.mockResolvedValue({ success: true }); + + createActionMiddleware(); + const result = await capturedServerFn({ next, request }); + + expect(next).toHaveBeenCalledWith(); + expect(result).toBe('next-result'); + }); + + it('should read body as text from cloned request (not json)', async () => { + const body = { action: 'test' }; + const request = createMockRequest(body); + const jsonSpy = vi.spyOn(request, 'json'); + const textSpy = vi.spyOn(Request.prototype, 'text'); + const next = vi.fn().mockResolvedValue('next-result'); + + mockVerifyAction.mockResolvedValue({ success: true }); + + createActionMiddleware(); + await capturedServerFn({ next, request }); + + // Should read as text (from clone), not call json() on original + expect(jsonSpy).not.toHaveBeenCalled(); + expect(textSpy).toHaveBeenCalledTimes(1); + + textSpy.mockRestore(); + }); + + it('should build a correct VerifiableRequest and pass it to verifyAction', async () => { + const body = { action: 'test' }; + const request = createMockRequest(body, { + 'x-store-id': 'store-123', + }); + const next = vi.fn().mockResolvedValue('next-result'); + + mockVerifyAction.mockResolvedValue({ success: true }); + + const options = { maxTimestampAgeSeconds: 300 }; + createActionMiddleware(options); + await capturedServerFn({ next, request }); + + // Verify that body is passed as RAW STRING (not parsed object) for signature verification + expect(mockVerifyAction).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'POST', + url: 'https://example.com/api/test?foo=bar', + path: '/api/test', + body: JSON.stringify(body), // Raw string, not parsed object + headers: expect.objectContaining({ + 'content-type': 'application/json', + 'x-store-id': 'store-123', + }), + query: { foo: 'bar' }, + }), + options, + ); + }); + + it('should throw a 401 Response when verification fails', async () => { + const errorPayload = { + name: 'MISSING_HEADER', + message: 'Missing required header', + }; + const body = { action: 'test' }; + const request = createMockRequest(body); + const next = vi.fn(); + + mockVerifyAction.mockResolvedValue({ success: false, error: errorPayload }); + + createActionMiddleware(); + + try { + await capturedServerFn({ next, request }); + expect.unreachable('Should have thrown'); + } catch (thrown) { + expect(thrown).toBeInstanceOf(Response); + const response = thrown as Response; + expect(response.status).toBe(401); + expect(response.headers.get('Content-Type')).toBe('application/json'); + const responseBody = await response.json(); + expect(responseBody).toEqual(errorPayload); + } + + expect(next).not.toHaveBeenCalled(); + }); + + it('should not call next when verification fails', async () => { + const body = { action: 'test' }; + const request = createMockRequest(body); + const next = vi.fn(); + + mockVerifyAction.mockResolvedValue({ + success: false, + error: { name: 'ERROR', message: 'fail' }, + }); + + createActionMiddleware(); + + try { + await capturedServerFn({ next, request }); + } catch { + // expected + } + + expect(next).not.toHaveBeenCalled(); + }); + + it('should preserve exact raw body string including whitespace', async () => { + // Test that formatting/whitespace is preserved exactly + const rawBodyString = '{\n "action": "test",\n "data": {\n "key": "value"\n }\n}'; + const request = new Request('https://example.com/api/test', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: rawBodyString, + }); + const next = vi.fn().mockResolvedValue('next-result'); + + mockVerifyAction.mockResolvedValue({ success: true }); + + createActionMiddleware(); + await capturedServerFn({ next, request }); + + // Verify raw string is passed exactly as-is to verification + expect(mockVerifyAction).toHaveBeenCalledWith( + expect.objectContaining({ + body: rawBodyString, // Exact match including whitespace + }), + undefined, + ); + }); + + it('should preserve unicode and special characters in raw body', async () => { + const rawBodyString = '{"emoji":"🎉","unicode":"\\u00e9","text":"café"}'; + const request = new Request('https://example.com/api/test', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: rawBodyString, + }); + const next = vi.fn().mockResolvedValue('next-result'); + + mockVerifyAction.mockResolvedValue({ success: true }); + + createActionMiddleware(); + await capturedServerFn({ next, request }); + + // Verify exact raw string preservation + expect(mockVerifyAction).toHaveBeenCalledWith( + expect.objectContaining({ + body: rawBodyString, + }), + undefined, + ); + }); +}); + +describe('createWebhookMiddleware', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should call next without arguments on success', async () => { + const body = { event: 'order.created', payload: { id: '123' } }; + const request = createMockRequest(body); + const next = vi.fn().mockResolvedValue('next-result'); + + mockVerifyWebhookSubscription.mockReturnValue({ success: true }); + + createWebhookMiddleware(); + const result = await capturedServerFn({ next, request }); + + expect(next).toHaveBeenCalledWith(); + expect(result).toBe('next-result'); + }); + + it('should read body as text from cloned request (not json)', async () => { + const body = { event: 'order.created' }; + const request = createMockRequest(body); + const jsonSpy = vi.spyOn(request, 'json'); + const textSpy = vi.spyOn(Request.prototype, 'text'); + const next = vi.fn().mockResolvedValue('next-result'); + + mockVerifyWebhookSubscription.mockReturnValue({ success: true }); + + createWebhookMiddleware(); + await capturedServerFn({ next, request }); + + // Should read as text (from clone), not call json() on original + expect(jsonSpy).not.toHaveBeenCalled(); + expect(textSpy).toHaveBeenCalledTimes(1); + + textSpy.mockRestore(); + }); + + it('should build a correct VerifiableRequest and pass it to verifyWebhookSubscription', async () => { + const body = { event: 'order.created' }; + const request = createMockRequest(body, { + 'webhook-signature': 'sig-value', + }); + const next = vi.fn().mockResolvedValue('next-result'); + + mockVerifyWebhookSubscription.mockReturnValue({ success: true }); + + const options = { secret: 'test-secret' }; + createWebhookMiddleware(options); + await capturedServerFn({ next, request }); + + // Verify that body is passed as RAW STRING (not parsed object) for signature verification + expect(mockVerifyWebhookSubscription).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'POST', + url: 'https://example.com/api/test?foo=bar', + path: '/api/test', + body: JSON.stringify(body), // Raw string, not parsed object + headers: expect.objectContaining({ + 'content-type': 'application/json', + 'webhook-signature': 'sig-value', + }), + query: { foo: 'bar' }, + }), + options, + ); + }); + + it('should throw a 401 Response when verification fails', async () => { + const errorPayload = { + name: 'INVALID_WEBHOOK_SIGNATURE', + message: 'The webhook signature is invalid', + }; + const body = { event: 'order.created' }; + const request = createMockRequest(body); + const next = vi.fn(); + + mockVerifyWebhookSubscription.mockReturnValue({ + success: false, + error: errorPayload, + }); + + createWebhookMiddleware(); + + try { + await capturedServerFn({ next, request }); + expect.unreachable('Should have thrown'); + } catch (thrown) { + expect(thrown).toBeInstanceOf(Response); + const response = thrown as Response; + expect(response.status).toBe(401); + expect(response.headers.get('Content-Type')).toBe('application/json'); + const responseBody = await response.json(); + expect(responseBody).toEqual(errorPayload); + } + + expect(next).not.toHaveBeenCalled(); + }); + + it('should not call next when verification fails', async () => { + const body = { event: 'order.created' }; + const request = createMockRequest(body); + const next = vi.fn(); + + mockVerifyWebhookSubscription.mockReturnValue({ + success: false, + error: { name: 'ERROR', message: 'fail' }, + }); + + createWebhookMiddleware(); + + try { + await capturedServerFn({ next, request }); + } catch { + // expected + } + + expect(next).not.toHaveBeenCalled(); + }); + + it('should preserve exact raw body string including whitespace', async () => { + // Test that formatting/whitespace is preserved exactly + const rawBodyString = '{\n "event": "order.created",\n "payload": {\n "id": "123"\n }\n}'; + const request = new Request('https://example.com/api/test', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: rawBodyString, + }); + const next = vi.fn().mockResolvedValue('next-result'); + + mockVerifyWebhookSubscription.mockReturnValue({ success: true }); + + createWebhookMiddleware(); + await capturedServerFn({ next, request }); + + // Verify raw string is passed exactly as-is to verification + expect(mockVerifyWebhookSubscription).toHaveBeenCalledWith( + expect.objectContaining({ + body: rawBodyString, // Exact match including whitespace + }), + undefined, + ); + }); + + it('should preserve unicode and special characters in raw body', async () => { + const rawBodyString = '{"event":"user.created","data":"café","emoji":"🎉"}'; + const request = new Request('https://example.com/api/test', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: rawBodyString, + }); + const next = vi.fn().mockResolvedValue('next-result'); + + mockVerifyWebhookSubscription.mockReturnValue({ success: true }); + + createWebhookMiddleware(); + await capturedServerFn({ next, request }); + + // Verify exact raw string preservation + expect(mockVerifyWebhookSubscription).toHaveBeenCalledWith( + expect.objectContaining({ + body: rawBodyString, + }), + undefined, + ); + }); +}); \ No newline at end of file diff --git a/packages/app-connect/src/tanstack/index.ts b/packages/app-connect/src/tanstack/index.ts index 240d659a..3aacd12c 100644 --- a/packages/app-connect/src/tanstack/index.ts +++ b/packages/app-connect/src/tanstack/index.ts @@ -15,6 +15,16 @@ export function createActionMiddleware(options?: VerificationOptions) { async ({ next, request }) => { const url = new URL(request.url); + // Read raw body as text from cloned request + // This preserves the original request stream for downstream handlers + let rawBody: string | undefined; + + if (request.body) { + // Clone the request to read body without consuming original + const clonedRequest = request.clone(); + rawBody = await clonedRequest.text(); + } + const verifiableRequest: VerifiableRequest = { method: request.method, url: request.url, @@ -22,7 +32,8 @@ export function createActionMiddleware(options?: VerificationOptions) { query: Object.fromEntries(url.searchParams), // biome-ignore lint/suspicious/noExplicitAny: Headers type compatibility issue headers: Object.fromEntries(request.headers as any), - body: request.body ? await request.json() : undefined, + // Pass raw body string directly for signature verification + body: rawBody, }; const result = await verifyAction(verifiableRequest, options); @@ -50,6 +61,16 @@ export function createWebhookMiddleware(options?: WebhookVerificationOptions) { async ({ next, request }) => { const url = new URL(request.url); + // Read raw body as text from cloned request + // This preserves the original request stream for downstream handlers + let rawBody: string | undefined; + + if (request.body) { + // Clone the request to read body without consuming original + const clonedRequest = request.clone(); + rawBody = await clonedRequest.text(); + } + const verifiableRequest: VerifiableRequest = { method: request.method, url: request.url, @@ -57,7 +78,8 @@ export function createWebhookMiddleware(options?: WebhookVerificationOptions) { query: Object.fromEntries(url.searchParams), // biome-ignore lint/suspicious/noExplicitAny: Headers type compatibility issue headers: Object.fromEntries(request.headers as any), - body: request.body ? await request.json() : undefined, + // Pass raw body string directly for signature verification + body: rawBody, }; const result = verifyWebhookSubscription(verifiableRequest, options); diff --git a/packages/app-connect/src/types/verifiable-request.ts b/packages/app-connect/src/types/verifiable-request.ts index f62ce0ca..1c9d8146 100644 --- a/packages/app-connect/src/types/verifiable-request.ts +++ b/packages/app-connect/src/types/verifiable-request.ts @@ -18,6 +18,11 @@ export interface VerifiableRequest { /** The request headers as a record */ headers: Record; - /** The request body, if any */ + /** + * The request body, if any. + * - For Express middleware: pre-parsed object from express.json() + * - For TanStack middleware: raw string from request.text() + * - For Next.js: caller-provided (typically pre-parsed with request.json()) + */ body?: unknown; } diff --git a/packages/app-connect/src/utils/verification.ts b/packages/app-connect/src/utils/verification.ts index f2b65de2..af337d07 100644 --- a/packages/app-connect/src/utils/verification.ts +++ b/packages/app-connect/src/utils/verification.ts @@ -102,7 +102,10 @@ export function canonicalizeRequest(req: VerifiableRequest): string { canonicalString += '\n'; // Add the body to the canonical string - // Use original body exactly as received to match Go middleware + // Use raw body string if available (TanStack), otherwise stringify parsed body (Express) + // IMPORTANT: For signature verification to work, we need the exact bytes that were signed. + // TanStack middleware provides the raw body string directly for accurate verification. + // Express middleware provides parsed object from express.json(), which we re-stringify. const bodyContent = typeof req.body === 'string' ? req.body : JSON.stringify(req.body); canonicalString += bodyContent; @@ -450,7 +453,7 @@ export function verifyWebhookSubscription( } return ok(); - } catch (err) { + } catch (_err) { return error(new InvalidWebhookSignatureError().toJSON()); } }