From 854f3dbdba269f23d61c2f1b4bfb63eb9ead62f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Gallay?= Date: Mon, 16 Oct 2023 12:08:29 +0200 Subject: [PATCH 1/4] fix(payments-plugin): Stripe controller crashes server instance --- packages/payments-plugin/src/stripe/stripe.controller.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/payments-plugin/src/stripe/stripe.controller.ts b/packages/payments-plugin/src/stripe/stripe.controller.ts index 91fd4fde04..6f56a92ee7 100644 --- a/packages/payments-plugin/src/stripe/stripe.controller.ts +++ b/packages/payments-plugin/src/stripe/stripe.controller.ts @@ -121,10 +121,13 @@ export class StripeController { loggerCtx, ); } - }); - Logger.info(`Stripe payment intent id ${paymentIntent.id} added to order ${orderCode}`, loggerCtx); - response.status(HttpStatus.OK).send('Ok'); + Logger.info( + `Stripe payment intent id ${paymentIntent.id} added to order ${orderCode}`, + loggerCtx, + ); + response.status(HttpStatus.OK).send('Ok'); + }); } private async createContext(channelToken: string, req: RequestWithRawBody): Promise { From cd16726eb0a63360e56dad5545cbe2af2f0ad4da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Gallay?= Date: Mon, 16 Oct 2023 14:46:01 +0200 Subject: [PATCH 2/4] fix(payments-plugin): Fix stripe raw-body parsing issue --- packages/payments-plugin/src/stripe/raw-body.middleware.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/payments-plugin/src/stripe/raw-body.middleware.ts b/packages/payments-plugin/src/stripe/raw-body.middleware.ts index 03257b6748..0f02e7fe41 100644 --- a/packages/payments-plugin/src/stripe/raw-body.middleware.ts +++ b/packages/payments-plugin/src/stripe/raw-body.middleware.ts @@ -1,4 +1,4 @@ -import { raw } from 'body-parser'; +import { json } from 'body-parser'; import * as http from 'http'; import { RequestWithRawBody } from './types'; @@ -7,7 +7,8 @@ import { RequestWithRawBody } from './types'; * Middleware which adds the raw request body to the incoming message object. This is needed by * Stripe to properly verify webhook events. */ -export const rawBodyMiddleware = raw({ +export const rawBodyMiddleware = json({ + type: '*/*', verify(req: RequestWithRawBody, res: http.ServerResponse, buf: Buffer, encoding: string) { if (Buffer.isBuffer(buf)) { req.rawBody = Buffer.from(buf); From dcf6eaccc654640e5c3d3a8121dc4ae9bc62ecb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Gallay?= Date: Tue, 17 Oct 2023 09:10:44 +0200 Subject: [PATCH 3/4] fix(payments-plugin): Revert to use `raw-body-parser`, and update StripeController's return logic --- .../payments-plugin/src/stripe/raw-body.middleware.ts | 4 ++-- .../payments-plugin/src/stripe/stripe.controller.ts | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/payments-plugin/src/stripe/raw-body.middleware.ts b/packages/payments-plugin/src/stripe/raw-body.middleware.ts index 0f02e7fe41..71c21b55ac 100644 --- a/packages/payments-plugin/src/stripe/raw-body.middleware.ts +++ b/packages/payments-plugin/src/stripe/raw-body.middleware.ts @@ -1,4 +1,4 @@ -import { json } from 'body-parser'; +import { raw } from 'body-parser'; import * as http from 'http'; import { RequestWithRawBody } from './types'; @@ -7,7 +7,7 @@ import { RequestWithRawBody } from './types'; * Middleware which adds the raw request body to the incoming message object. This is needed by * Stripe to properly verify webhook events. */ -export const rawBodyMiddleware = json({ +export const rawBodyMiddleware = raw({ type: '*/*', verify(req: RequestWithRawBody, res: http.ServerResponse, buf: Buffer, encoding: string) { if (Buffer.isBuffer(buf)) { diff --git a/packages/payments-plugin/src/stripe/stripe.controller.ts b/packages/payments-plugin/src/stripe/stripe.controller.ts index 6f56a92ee7..0c80a5d512 100644 --- a/packages/payments-plugin/src/stripe/stripe.controller.ts +++ b/packages/payments-plugin/src/stripe/stripe.controller.ts @@ -45,7 +45,7 @@ export class StripeController { return; } - const event = request.body as Stripe.Event; + const event = JSON.parse(request.body.toString()) as Stripe.Event; const paymentIntent = event.data.object as Stripe.PaymentIntent; if (!paymentIntent) { @@ -120,14 +120,20 @@ export class StripeController { `Error adding payment to order ${orderCode}: ${addPaymentToOrderResult.message}`, loggerCtx, ); + return; } + // The payment intent ID is added to the order only if we can reach this point. Logger.info( `Stripe payment intent id ${paymentIntent.id} added to order ${orderCode}`, loggerCtx, ); - response.status(HttpStatus.OK).send('Ok'); }); + + // Send the response status only if we didn't sent anything yet. + if (!response.headersSent) { + response.status(HttpStatus.OK).send('Ok'); + } } private async createContext(channelToken: string, req: RequestWithRawBody): Promise { From a7104aa7b62435a3e8a67c272b1a8bb5e7295b37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Gallay?= Date: Tue, 17 Oct 2023 11:26:36 +0200 Subject: [PATCH 4/4] fix(payments-method): Added some e2e tests for webhook validation --- .../e2e/stripe-payment.e2e-spec.ts | 99 ++++++++++++++++++- 1 file changed, 97 insertions(+), 2 deletions(-) diff --git a/packages/payments-plugin/e2e/stripe-payment.e2e-spec.ts b/packages/payments-plugin/e2e/stripe-payment.e2e-spec.ts index 7bebbfcef7..a4bfe62d5d 100644 --- a/packages/payments-plugin/e2e/stripe-payment.e2e-spec.ts +++ b/packages/payments-plugin/e2e/stripe-payment.e2e-spec.ts @@ -10,7 +10,9 @@ import { CREATE_PRODUCT, CREATE_PRODUCT_VARIANTS } from '@vendure/core/e2e/graph import { createTestEnvironment, E2E_DEFAULT_CHANNEL_TOKEN } from '@vendure/testing'; import gql from 'graphql-tag'; import nock from 'nock'; +import fetch from 'node-fetch'; import path from 'path'; +import { Stripe } from 'stripe'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { initialData } from '../../../e2e-common/e2e-initial-data'; @@ -293,6 +295,101 @@ describe('Stripe payments', () => { }); }); + // https://github.com/vendure-ecommerce/vendure/issues/2450 + it('Should not crash on signature validation failure', async () => { + const MOCKED_WEBHOOK_PAYLOAD = { + id: 'evt_0', + object: 'event', + api_version: '2022-11-15', + data: { + object: { + id: 'pi_0', + currency: 'usd', + status: 'succeeded', + }, + }, + livemode: false, + pending_webhooks: 1, + request: { + id: 'req_0', + idempotency_key: '00000000-0000-0000-0000-000000000000', + }, + type: 'payment_intent.succeeded', + }; + + const payloadString = JSON.stringify(MOCKED_WEBHOOK_PAYLOAD, null, 2); + + const result = await fetch(`http://localhost:${serverPort}/payments/stripe`, { + method: 'post', + body: payloadString, + headers: { 'Content-Type': 'application/json' }, + }); + + // We didn't provided any signatures, it should result in a 400 - Bad request + expect(result.status).toEqual(400); + }); + + // TODO: Contribution welcome: test webhook handling and order settlement + // https://github.com/vendure-ecommerce/vendure/issues/2450 + it("Should validate the webhook's signature properly", async () => { + await shopClient.asUserWithCredentials(customers[0].emailAddress, 'test'); + + const { activeOrder } = await shopClient.query(GET_ACTIVE_ORDER); + order = activeOrder!; + + const MOCKED_WEBHOOK_PAYLOAD = { + id: 'evt_0', + object: 'event', + api_version: '2022-11-15', + data: { + object: { + id: 'pi_0', + currency: 'usd', + metadata: { + orderCode: order.code, + orderId: parseInt(order.id.replace('T_', ''), 10), + channelToken: E2E_DEFAULT_CHANNEL_TOKEN, + }, + amount_received: order.totalWithTax, + status: 'succeeded', + }, + }, + livemode: false, + pending_webhooks: 1, + request: { + id: 'req_0', + idempotency_key: '00000000-0000-0000-0000-000000000000', + }, + type: 'payment_intent.succeeded', + }; + + const payloadString = JSON.stringify(MOCKED_WEBHOOK_PAYLOAD, null, 2); + const stripeWebhooks = new Stripe('test-api-secret', { apiVersion: '2023-08-16' }).webhooks; + const header = stripeWebhooks.generateTestHeaderString({ + payload: payloadString, + secret: 'test-signing-secret', + }); + + const event = stripeWebhooks.constructEvent(payloadString, header, 'test-signing-secret'); + expect(event.id).to.equal(MOCKED_WEBHOOK_PAYLOAD.id); + await setShipping(shopClient); + // Due to the `this.orderService.transitionToState(...)` fails with the internal lookup by id, + // we need to put the order into `ArrangingPayment` state manually before calling the webhook handler. + // const transitionResult = await adminClient.query(TRANSITION_TO_ARRANGING_PAYMENT, { id: order.id }); + // expect(transitionResult.transitionOrderToState.__typename).toBe('Order') + + const result = await fetch(`http://localhost:${serverPort}/payments/stripe`, { + method: 'post', + body: payloadString, + headers: { 'Content-Type': 'application/json', 'Stripe-Signature': header }, + }); + + // I would expect to the status to be 200, but at the moment either the + // `orderService.transitionToState()` or the `orderService.addPaymentToOrder()` + // throws an error of 'error.entity-with-id-not-found' + expect(result.status).toEqual(200); + }); + // https://github.com/vendure-ecommerce/vendure/issues/1630 describe('currencies with no fractional units', () => { let japanProductId: string; @@ -401,6 +498,4 @@ describe('Stripe payments', () => { expect(createPaymentIntentPayload.currency).toBe('jpy'); }); }); - - // TODO: Contribution welcome: test webhook handling and order settlement });