From 5b923067e71a674026d33db65034ae4e1f0a41b9 Mon Sep 17 00:00:00 2001 From: Aleksandar Petkov Date: Mon, 9 Dec 2024 10:19:42 +0200 Subject: [PATCH 01/11] fix: Generate Stripe idempotency keys server-side --- apps/api/src/stripe/stripe.controller.ts | 14 ++++-------- apps/api/src/stripe/stripe.service.ts | 28 +++++++++++++----------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/apps/api/src/stripe/stripe.controller.ts b/apps/api/src/stripe/stripe.controller.ts index 4e3629f5..21c37ddd 100644 --- a/apps/api/src/stripe/stripe.controller.ts +++ b/apps/api/src/stripe/stripe.controller.ts @@ -7,18 +7,15 @@ import { Param, Patch, Post, - Query, UnauthorizedException, } from '@nestjs/common' import { ApiBody, ApiTags } from '@nestjs/swagger' -import { AuthenticatedUser, Public, RoleMatchingMode, Roles } from 'nest-keycloak-connect' +import { Public, RoleMatchingMode, Roles } from 'nest-keycloak-connect' import { CancelPaymentIntentDto } from './dto/cancel-payment-intent.dto' import { CreatePaymentIntentDto } from './dto/create-payment-intent.dto' import { UpdatePaymentIntentDto } from './dto/update-payment-intent.dto' import { UpdateSetupIntentDto } from './dto/update-setup-intent.dto' import { StripeService } from './stripe.service' -import { KeycloakTokenParsed } from '../auth/keycloak' -import { CreateSubscriptionPaymentDto } from './dto/create-subscription-payment.dto' import { EditFinancialsRequests } from '@podkrepi-bg/podkrepi-types' import { CreateSessionDto } from '../donations/dto/create-session.dto' import { PersonService } from '../person/person.service' @@ -80,10 +77,9 @@ export class StripeController { @Public() updateSetupIntent( @Param('id') id: string, - @Query('idempotency-key') idempotencyKey: string, @Body() updateSetupIntentDto: UpdateSetupIntentDto, ) { - return this.stripeService.updateSetupIntent(id, idempotencyKey, updateSetupIntentDto) + return this.stripeService.updateSetupIntent(id, updateSetupIntentDto) } @Patch('setup-intent/:id/cancel') @@ -99,9 +95,8 @@ export class StripeController { @Public() setupIntentToPaymentIntent( @Param('id') id: string, - @Query('idempotency-key') idempotencyKey: string, ) { - return this.stripeService.setupIntentToPaymentIntent(id, idempotencyKey) + return this.stripeService.setupIntentToPaymentIntent(id) } @Post('setup-intent/:id/subscription') @@ -110,9 +105,8 @@ export class StripeController { }) setupIntentToSubscription( @Param('id') id: string, - @Query('idempotency-key') idempotencyKey: string, ) { - return this.stripeService.setupIntentToSubscription(id, idempotencyKey) + return this.stripeService.setupIntentToSubscription(id) } @Post('payment-intent') diff --git a/apps/api/src/stripe/stripe.service.ts b/apps/api/src/stripe/stripe.service.ts index ee76a707..fe2e16b2 100644 --- a/apps/api/src/stripe/stripe.service.ts +++ b/apps/api/src/stripe/stripe.service.ts @@ -32,7 +32,6 @@ export class StripeService { */ async updateSetupIntent( id: string, - idempotencyKey: string, inputDto: UpdateSetupIntentDto, ): Promise> { if (!inputDto.metadata.campaignId) @@ -40,6 +39,7 @@ export class StripeService { await this.campaignService.validateCampaignId( inputDto.metadata.campaignId as string, ) + const idempotencyKey = crypto.randomUUID() return await this.stripeClient.setupIntents.update(id, inputDto, { idempotencyKey }) } /** @@ -74,8 +74,9 @@ export class StripeService { async attachPaymentMethodToCustomer( paymentMethod: Stripe.PaymentMethod, customer: Stripe.Customer, - idempotencyKey: string, ) { + const idempotencyKey = crypto.randomUUID() + return await this.stripeClient.paymentMethods.attach( paymentMethod.id, { @@ -86,7 +87,6 @@ export class StripeService { } async setupIntentToPaymentIntent( setupIntentId: string, - idempotencyKey: string, ): Promise { const setupIntent = await this.findSetupIntentById(setupIntentId) @@ -96,9 +96,10 @@ export class StripeService { const name = paymentMethod.billing_details.name as string const metadata = setupIntent.metadata as Stripe.Metadata - const customer = await this.createCustomer(email, name, paymentMethod, idempotencyKey) + const customer = await this.createCustomer(email, name, paymentMethod) - await this.attachPaymentMethodToCustomer(paymentMethod, customer, idempotencyKey) + await this.attachPaymentMethodToCustomer(paymentMethod, customer) + const idempotencyKey = crypto.randomUUID() const paymentIntent = await this.stripeClient.paymentIntents.create( { @@ -130,7 +131,6 @@ export class StripeService { async setupIntentToSubscription( setupIntentId: string, - idempotencyKey: string, ): Promise { const setupIntent = await this.findSetupIntentById(setupIntentId) if (setupIntent instanceof Error) throw new BadRequestException(setupIntent.message) @@ -139,11 +139,11 @@ export class StripeService { const name = paymentMethod.billing_details.name as string const metadata = setupIntent.metadata as Stripe.Metadata - const customer = await this.createCustomer(email, name, paymentMethod, idempotencyKey) - await this.attachPaymentMethodToCustomer(paymentMethod, customer, idempotencyKey) + const customer = await this.createCustomer(email, name, paymentMethod) + await this.attachPaymentMethodToCustomer(paymentMethod, customer) - const product = await this.createProduct(metadata.campaignId, idempotencyKey) - return await this.createSubscription(metadata, customer, product, paymentMethod, idempotencyKey) + const product = await this.createProduct(metadata.campaignId) + return await this.createSubscription(metadata, customer, product, paymentMethod) } /** @@ -200,11 +200,11 @@ export class StripeService { email: string, name: string, paymentMethod: Stripe.PaymentMethod, - idempotencyKey: string, ) { const customerLookup = await this.stripeClient.customers.list({ email, }) + const idempotencyKey = crypto.randomUUID() const customer = customerLookup.data[0] //Customer not found. Create new onw if (!customer) @@ -220,8 +220,9 @@ export class StripeService { return customer } - async createProduct(campaignId: string, idempotencyKey: string): Promise { + async createProduct(campaignId: string): Promise { const campaign = await this.campaignService.getCampaignById(campaignId) + const idempotencyKey = crypto.randomUUID() if (!campaign) throw new Error(`Campaign with id ${campaignId} not found`) const productLookup = await this.stripeClient.products.search({ @@ -242,8 +243,9 @@ export class StripeService { customer: Stripe.Customer, product: Stripe.Product, paymentMethod: Stripe.PaymentMethod, - idempotencyKey: string, ) { + const idempotencyKey = crypto.randomUUID() + const subscription = await this.stripeClient.subscriptions.create( { customer: customer.id, From 4e5f115a25db408b5a34dc1a118531507bd532cb Mon Sep 17 00:00:00 2001 From: Aleksandar Petkov Date: Mon, 9 Dec 2024 12:32:05 +0200 Subject: [PATCH 02/11] chore: Fix tests --- apps/api/src/stripe/stripe.controller.spec.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/api/src/stripe/stripe.controller.spec.ts b/apps/api/src/stripe/stripe.controller.spec.ts index e6690324..949b7786 100644 --- a/apps/api/src/stripe/stripe.controller.spec.ts +++ b/apps/api/src/stripe/stripe.controller.spec.ts @@ -29,7 +29,6 @@ import { NotificationModule } from '../sockets/notifications/notification.module import { KeycloakTokenParsed } from '../auth/keycloak' describe('StripeController', () => { let controller: StripeController - const idempotencyKey = 'test_123' const stripeMock = { checkout: { sessions: { create: jest.fn() } }, paymentIntents: { retrieve: jest.fn() }, @@ -212,7 +211,7 @@ describe('StripeController', () => { }, } - await expect(controller.updateSetupIntent('123', idempotencyKey, payload)).rejects.toThrow( + await expect(controller.updateSetupIntent('123', payload)).rejects.toThrow( new NotAcceptableException('Campaign cannot accept donations in state: complete'), ) }) @@ -228,7 +227,7 @@ describe('StripeController', () => { state: CampaignState.complete, title: 'active-campaign', } as Campaign) - await expect(controller.setupIntentToSubscription('123', idempotencyKey)).toResolve() + await expect(controller.setupIntentToSubscription('123')).toResolve() expect(stripeMock.setupIntents.retrieve).toHaveBeenCalledWith('123', { expand: ['payment_method'], }) From d34dd63d19a7d65c928a107f42afedea932e5362 Mon Sep 17 00:00:00 2001 From: Aleksandar Petkov Date: Mon, 9 Dec 2024 20:31:29 +0200 Subject: [PATCH 03/11] fix: Tests --- apps/api/src/stripe/stripe.controller.ts | 15 ++++----------- apps/api/src/stripe/stripe.service.ts | 18 ++++-------------- 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/apps/api/src/stripe/stripe.controller.ts b/apps/api/src/stripe/stripe.controller.ts index 21c37ddd..a20abf84 100644 --- a/apps/api/src/stripe/stripe.controller.ts +++ b/apps/api/src/stripe/stripe.controller.ts @@ -10,7 +10,7 @@ import { UnauthorizedException, } from '@nestjs/common' import { ApiBody, ApiTags } from '@nestjs/swagger' -import { Public, RoleMatchingMode, Roles } from 'nest-keycloak-connect' +import { Public, RoleMatchingMode, Roles } from 'nest-keycloak-connect' import { CancelPaymentIntentDto } from './dto/cancel-payment-intent.dto' import { CreatePaymentIntentDto } from './dto/create-payment-intent.dto' import { UpdatePaymentIntentDto } from './dto/update-payment-intent.dto' @@ -75,10 +75,7 @@ export class StripeController { @Post('setup-intent/:id') @Public() - updateSetupIntent( - @Param('id') id: string, - @Body() updateSetupIntentDto: UpdateSetupIntentDto, - ) { + updateSetupIntent(@Param('id') id: string, @Body() updateSetupIntentDto: UpdateSetupIntentDto) { return this.stripeService.updateSetupIntent(id, updateSetupIntentDto) } @@ -93,9 +90,7 @@ export class StripeController { description: 'Create payment intent from setup intent', }) @Public() - setupIntentToPaymentIntent( - @Param('id') id: string, - ) { + setupIntentToPaymentIntent(@Param('id') id: string) { return this.stripeService.setupIntentToPaymentIntent(id) } @@ -103,9 +98,7 @@ export class StripeController { @ApiBody({ description: 'Create payment intent from setup intent', }) - setupIntentToSubscription( - @Param('id') id: string, - ) { + setupIntentToSubscription(@Param('id') id: string) { return this.stripeService.setupIntentToSubscription(id) } diff --git a/apps/api/src/stripe/stripe.service.ts b/apps/api/src/stripe/stripe.service.ts index fe2e16b2..57f47b2e 100644 --- a/apps/api/src/stripe/stripe.service.ts +++ b/apps/api/src/stripe/stripe.service.ts @@ -36,9 +36,7 @@ export class StripeService { ): Promise> { if (!inputDto.metadata.campaignId) throw new BadRequestException('campaignId is missing from metadata') - await this.campaignService.validateCampaignId( - inputDto.metadata.campaignId as string, - ) + await this.campaignService.validateCampaignId(inputDto.metadata.campaignId as string) const idempotencyKey = crypto.randomUUID() return await this.stripeClient.setupIntents.update(id, inputDto, { idempotencyKey }) } @@ -85,9 +83,7 @@ export class StripeService { { idempotencyKey: `${idempotencyKey}--pm` }, ) } - async setupIntentToPaymentIntent( - setupIntentId: string, - ): Promise { + async setupIntentToPaymentIntent(setupIntentId: string): Promise { const setupIntent = await this.findSetupIntentById(setupIntentId) if (setupIntent instanceof Error) throw new BadRequestException(setupIntent.message) @@ -129,9 +125,7 @@ export class StripeService { return await this.stripeClient.setupIntents.create({}, { idempotencyKey }) } - async setupIntentToSubscription( - setupIntentId: string, - ): Promise { + async setupIntentToSubscription(setupIntentId: string): Promise { const setupIntent = await this.findSetupIntentById(setupIntentId) if (setupIntent instanceof Error) throw new BadRequestException(setupIntent.message) const paymentMethod = setupIntent.payment_method as Stripe.PaymentMethod @@ -196,11 +190,7 @@ export class StripeService { } else return new Array() } - async createCustomer( - email: string, - name: string, - paymentMethod: Stripe.PaymentMethod, - ) { + async createCustomer(email: string, name: string, paymentMethod: Stripe.PaymentMethod) { const customerLookup = await this.stripeClient.customers.list({ email, }) From 036bee59e3361c11ad54738638f1002c1c1096f6 Mon Sep 17 00:00:00 2001 From: Aleksandar Petkov Date: Mon, 9 Dec 2024 21:13:09 +0200 Subject: [PATCH 04/11] fix: Subscription returning bad request --- apps/api/src/stripe/stripe.service.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/api/src/stripe/stripe.service.ts b/apps/api/src/stripe/stripe.service.ts index 57f47b2e..18102da2 100644 --- a/apps/api/src/stripe/stripe.service.ts +++ b/apps/api/src/stripe/stripe.service.ts @@ -216,14 +216,17 @@ export class StripeService { if (!campaign) throw new Error(`Campaign with id ${campaignId} not found`) const productLookup = await this.stripeClient.products.search({ - query: `-name:'${campaign.title}'`, + query: `metadata["campaignId"]:"${campaign.id}"`, }) - if (productLookup) return productLookup.data[0] + if (productLookup.data.length) return productLookup.data[0] return await this.stripeClient.products.create( { name: campaign.title, description: `Donate to ${campaign.title}`, + metadata: { + campaignId: campaign.id, + }, }, { idempotencyKey: `${idempotencyKey}--product` }, ) From febd7dadf7c6626278ae8adb0ba5499561d37e4c Mon Sep 17 00:00:00 2001 From: Aleksandar Petkov Date: Mon, 9 Dec 2024 21:29:26 +0200 Subject: [PATCH 05/11] chore: Attempt to log the error --- apps/api/src/stripe/stripe.controller.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/api/src/stripe/stripe.controller.spec.ts b/apps/api/src/stripe/stripe.controller.spec.ts index 949b7786..f586f5d0 100644 --- a/apps/api/src/stripe/stripe.controller.spec.ts +++ b/apps/api/src/stripe/stripe.controller.spec.ts @@ -227,7 +227,7 @@ describe('StripeController', () => { state: CampaignState.complete, title: 'active-campaign', } as Campaign) - await expect(controller.setupIntentToSubscription('123')).toResolve() + await expect(controller.setupIntentToSubscription('123').catch((err) => console.log(err))).toResolve() expect(stripeMock.setupIntents.retrieve).toHaveBeenCalledWith('123', { expand: ['payment_method'], }) From 0a5f9ef1d71b7ab03156262e76d29a4c898aef82 Mon Sep 17 00:00:00 2001 From: Aleksandar Petkov Date: Mon, 9 Dec 2024 21:34:58 +0200 Subject: [PATCH 06/11] chore: Try to log in try catch clause --- apps/api/src/stripe/stripe.controller.spec.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/apps/api/src/stripe/stripe.controller.spec.ts b/apps/api/src/stripe/stripe.controller.spec.ts index f586f5d0..7338dc41 100644 --- a/apps/api/src/stripe/stripe.controller.spec.ts +++ b/apps/api/src/stripe/stripe.controller.spec.ts @@ -227,11 +227,16 @@ describe('StripeController', () => { state: CampaignState.complete, title: 'active-campaign', } as Campaign) - await expect(controller.setupIntentToSubscription('123').catch((err) => console.log(err))).toResolve() - expect(stripeMock.setupIntents.retrieve).toHaveBeenCalledWith('123', { - expand: ['payment_method'], - }) - expect(stripeMock.customers.create).not.toHaveBeenCalled() - expect(stripeMock.products.create).not.toHaveBeenCalled() + try { + + await expect(controller.setupIntentToSubscription('123')).toResolve() + expect(stripeMock.setupIntents.retrieve).toHaveBeenCalledWith('123', { + expand: ['payment_method'], + }) + expect(stripeMock.customers.create).not.toHaveBeenCalled() + expect(stripeMock.products.create).not.toHaveBeenCalled() + } catch (err) { + throw new Error(JSON.stringify(err)) + } }) }) From 4675434c2cf2fc18289a5bacd2d40977b05993da Mon Sep 17 00:00:00 2001 From: Aleksandar Petkov Date: Mon, 9 Dec 2024 21:38:56 +0200 Subject: [PATCH 07/11] chore: Try to debug by the thrown error message --- apps/api/src/stripe/stripe.controller.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/api/src/stripe/stripe.controller.spec.ts b/apps/api/src/stripe/stripe.controller.spec.ts index 7338dc41..e8695ecf 100644 --- a/apps/api/src/stripe/stripe.controller.spec.ts +++ b/apps/api/src/stripe/stripe.controller.spec.ts @@ -229,7 +229,7 @@ describe('StripeController', () => { } as Campaign) try { - await expect(controller.setupIntentToSubscription('123')).toResolve() + await expect(controller.setupIntentToSubscription('123')).rejects.toThrow(new NotAcceptableException('Campaign cannot accept donations in state: complete'),) expect(stripeMock.setupIntents.retrieve).toHaveBeenCalledWith('123', { expand: ['payment_method'], }) From 0c5cb330f92aa9e7b1ea4b5d7987dda55c230b29 Mon Sep 17 00:00:00 2001 From: Aleksandar Petkov Date: Mon, 9 Dec 2024 21:43:32 +0200 Subject: [PATCH 08/11] chore: Add crypto import --- apps/api/src/stripe/stripe.service.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/api/src/stripe/stripe.service.ts b/apps/api/src/stripe/stripe.service.ts index 18102da2..d3e62b11 100644 --- a/apps/api/src/stripe/stripe.service.ts +++ b/apps/api/src/stripe/stripe.service.ts @@ -13,6 +13,7 @@ import { ConfigService } from '@nestjs/config' import { StripeMetadata } from './stripe-metadata.interface' import { CreateStripePaymentDto } from '../donations/dto/create-stripe-payment.dto' import { RecurringDonationService } from '../recurring-donation/recurring-donation.service' +import * as crypto from 'crypto' @Injectable() export class StripeService { From 50c5831b03ebfcd58b50c06c28d5db86e6ee0907 Mon Sep 17 00:00:00 2001 From: Aleksandar Petkov Date: Mon, 9 Dec 2024 21:46:45 +0200 Subject: [PATCH 09/11] chore: Bring back original test behavior --- apps/api/src/stripe/stripe.controller.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/api/src/stripe/stripe.controller.spec.ts b/apps/api/src/stripe/stripe.controller.spec.ts index e8695ecf..7338dc41 100644 --- a/apps/api/src/stripe/stripe.controller.spec.ts +++ b/apps/api/src/stripe/stripe.controller.spec.ts @@ -229,7 +229,7 @@ describe('StripeController', () => { } as Campaign) try { - await expect(controller.setupIntentToSubscription('123')).rejects.toThrow(new NotAcceptableException('Campaign cannot accept donations in state: complete'),) + await expect(controller.setupIntentToSubscription('123')).toResolve() expect(stripeMock.setupIntents.retrieve).toHaveBeenCalledWith('123', { expand: ['payment_method'], }) From ef268fdee3271ee167cfd716f27f9efbd9af24ef Mon Sep 17 00:00:00 2001 From: Aleksandar Petkov Date: Mon, 9 Dec 2024 23:51:56 +0200 Subject: [PATCH 10/11] chore: Send forgotten idempotency key --- apps/api/src/stripe/stripe.service.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/apps/api/src/stripe/stripe.service.ts b/apps/api/src/stripe/stripe.service.ts index d3e62b11..c2bfc611 100644 --- a/apps/api/src/stripe/stripe.service.ts +++ b/apps/api/src/stripe/stripe.service.ts @@ -118,11 +118,8 @@ export class StripeService { * @param inputDto Payment intent create params * @returns {Promise>} */ - async createSetupIntent({ - idempotencyKey, - }: { - idempotencyKey: string - }): Promise> { + async createSetupIntent(): Promise> { + const idempotencyKey = crypto.randomUUID() return await this.stripeClient.setupIntents.create({}, { idempotencyKey }) } From c4cf9a6c3d7ce736612586e787c7ac93cce2f00b Mon Sep 17 00:00:00 2001 From: Aleksandar Petkov Date: Tue, 10 Dec 2024 09:12:29 +0200 Subject: [PATCH 11/11] chore: Remove idempotency from bodyDto --- apps/api/src/stripe/stripe.controller.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/api/src/stripe/stripe.controller.ts b/apps/api/src/stripe/stripe.controller.ts index a20abf84..2a6d4436 100644 --- a/apps/api/src/stripe/stripe.controller.ts +++ b/apps/api/src/stripe/stripe.controller.ts @@ -30,8 +30,8 @@ export class StripeController { @Post('setup-intent') @Public() - createSetupIntent(@Body() body: { idempotencyKey: string }) { - return this.stripeService.createSetupIntent(body) + createSetupIntent() { + return this.stripeService.createSetupIntent() } @Post('create-checkout-session')