From 31fcd0ed1d8e3ae9dc3b780a981ad852e676a1b2 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Thu, 3 Feb 2022 19:15:19 +0000 Subject: [PATCH] Server: Only use Stripe "customer.subscription.created" event to provision subscriptions --- .../server/src/routes/index/stripe.test.ts | 85 ++++++------- packages/server/src/routes/index/stripe.ts | 113 ++++++++++-------- 2 files changed, 109 insertions(+), 89 deletions(-) diff --git a/packages/server/src/routes/index/stripe.test.ts b/packages/server/src/routes/index/stripe.test.ts index 7caf14c62a..ccbe34fdd9 100644 --- a/packages/server/src/routes/index/stripe.test.ts +++ b/packages/server/src/routes/index/stripe.test.ts @@ -7,15 +7,28 @@ import { AppContext } from '../../utils/types'; import uuidgen from '../../utils/uuidgen'; import { postHandlers } from './stripe'; -function mockStripe(overrides: any = null) { +interface StripeOptions { + userEmail?: string; +} + +function mockStripe(options: StripeOptions = null) { + options = { + userEmail: 'toto@example.com', + ...options, + }; + return { customers: { - retrieve: jest.fn(), + retrieve: async () => { + return { + name: 'Toto', + email: options.userEmail, + }; + }, }, subscriptions: { del: jest.fn(), }, - ...overrides, }; } @@ -25,11 +38,12 @@ interface WebhookOptions { subscriptionId?: string; customerId?: string; sessionId?: string; + userEmail?: string; } async function simulateWebhook(ctx: AppContext, type: string, object: any, options: WebhookOptions = {}) { options = { - stripe: mockStripe(), + stripe: mockStripe({ userEmail: options.userEmail }), eventId: uuidgen(), ...options, }; @@ -43,7 +57,7 @@ async function simulateWebhook(ctx: AppContext, type: string, object: any, optio }, false); } -async function createUserViaSubscription(ctx: AppContext, userEmail: string, options: WebhookOptions = {}) { +async function createUserViaSubscription(ctx: AppContext, options: WebhookOptions = {}) { options = { subscriptionId: `sub_${uuidgen()}`, customerId: `cus_${uuidgen()}`, @@ -54,12 +68,15 @@ async function createUserViaSubscription(ctx: AppContext, userEmail: string, opt const stripePrice = findPrice(stripeConfig().prices, { accountType: 2, period: PricePeriod.Monthly }); await models().keyValue().setValue(`stripeSessionToPriceId::${stripeSessionId}`, stripePrice.id); - await simulateWebhook(ctx, 'checkout.session.completed', { - id: stripeSessionId, + await simulateWebhook(ctx, 'customer.subscription.created', { + id: options.subscriptionId, customer: options.customerId, - subscription: options.subscriptionId, - customer_details: { - email: userEmail, + items: { + data: [ + { + price: stripePrice, + }, + ], }, }, options); } @@ -83,7 +100,7 @@ describe('index/stripe', function() { const startTime = Date.now(); const ctx = await koaAppContext(); - await createUserViaSubscription(ctx, 'toto@example.com', { subscriptionId: 'sub_123' }); + await createUserViaSubscription(ctx, { userEmail: 'toto@example.com', subscriptionId: 'sub_123' }); const user = await models().user().loadByEmail('toto@example.com'); expect(user.account_type).toBe(AccountType.Pro); @@ -98,11 +115,11 @@ describe('index/stripe', function() { test('should not process the same event twice', async function() { const ctx = await koaAppContext(); - await createUserViaSubscription(ctx, 'toto@example.com', { eventId: 'evt_1' }); + await createUserViaSubscription(ctx, { userEmail: 'toto@example.com', eventId: 'evt_1' }); const v = await models().keyValue().value('stripeEventDone::evt_1'); expect(v).toBe(1); // This event should simply be skipped - await expectNotThrow(async () => createUserViaSubscription(ctx, 'toto@example.com', { eventId: 'evt_1' })); + await expectNotThrow(async () => createUserViaSubscription(ctx, { userEmail: 'toto@example.com', eventId: 'evt_1' })); }); test('should check if it is a beta user', async function() { @@ -136,7 +153,7 @@ describe('index/stripe', function() { const ctx = await koaAppContext(); const user = await models().user().save({ email: 'toto@example.com', password: uuidgen() }); expect(await models().subscription().byUserId(user.id)).toBeFalsy(); - await createUserViaSubscription(ctx, 'toto@example.com', { subscriptionId: 'sub_123' }); + await createUserViaSubscription(ctx, { userEmail: 'toto@example.com', subscriptionId: 'sub_123' }); const sub = await models().subscription().byUserId(user.id); expect(sub).toBeTruthy(); @@ -147,13 +164,13 @@ describe('index/stripe', function() { const stripe = mockStripe(); const ctx = await koaAppContext(); - await createUserViaSubscription(ctx, 'toto@example.com', { stripe }); + await createUserViaSubscription(ctx, { userEmail: 'toto@example.com', stripe }); expect((await models().user().all()).length).toBe(1); const user = (await models().user().all())[0]; const subBefore = await models().subscription().byUserId(user.id); expect(stripe.subscriptions.del).toHaveBeenCalledTimes(0); - await createUserViaSubscription(ctx, 'toto@example.com', { stripe }); + await createUserViaSubscription(ctx, { userEmail: 'toto@example.com', stripe }); expect((await models().user().all()).length).toBe(1); const subAfter = await models().subscription().byUserId(user.id); expect(stripe.subscriptions.del).toHaveBeenCalledTimes(1); @@ -165,7 +182,7 @@ describe('index/stripe', function() { const stripe = mockStripe(); const ctx = await koaAppContext(); - await createUserViaSubscription(ctx, 'toto@example.com', { stripe, subscriptionId: 'sub_init' }); + await createUserViaSubscription(ctx, { userEmail: 'toto@example.com', stripe, subscriptionId: 'sub_init' }); await simulateWebhook(ctx, 'customer.subscription.deleted', { id: 'sub_init' }); const user = (await models().user().all())[0]; @@ -178,14 +195,14 @@ describe('index/stripe', function() { const stripe = mockStripe(); const ctx = await koaAppContext(); - await createUserViaSubscription(ctx, 'toto@example.com', { stripe, subscriptionId: 'sub_init' }); + await createUserViaSubscription(ctx, { userEmail: 'toto@example.com', stripe, subscriptionId: 'sub_init' }); const user = (await models().user().all())[0]; const sub = await models().subscription().byUserId(user.id); expect(sub.stripe_subscription_id).toBe('sub_init'); await simulateWebhook(ctx, 'customer.subscription.deleted', { id: 'sub_init' }); - await createUserViaSubscription(ctx, 'toto@example.com', { stripe, subscriptionId: 'cus_recreate' }); + await createUserViaSubscription(ctx, { userEmail: 'toto@example.com', stripe, subscriptionId: 'cus_recreate' }); { const user = (await models().user().all())[0]; @@ -200,7 +217,7 @@ describe('index/stripe', function() { const stripe = mockStripe(); const ctx = await koaAppContext(); - await createUserViaSubscription(ctx, 'toto@example.com', { stripe, subscriptionId: 'sub_init' }); + await createUserViaSubscription(ctx, { userEmail: 'toto@example.com', stripe, subscriptionId: 'sub_init' }); let user = (await models().user().all())[0]; await models().user().save({ id: user.id, @@ -225,23 +242,15 @@ describe('index/stripe', function() { // - Then a new subscription is attached to the user on Stripe // => In that case, the sub should be attached to the user on Joplin Server - const stripe = mockStripe({ - customers: { - retrieve: async () => { - return { - name: 'Toto', - email: 'toto@example.com', - }; - }, - }, - }); + const stripe = mockStripe({ userEmail: 'toto@example.com' }); const ctx = await koaAppContext(); - await createUserViaSubscription(ctx, 'toto@example.com', { + await createUserViaSubscription(ctx, { stripe, subscriptionId: 'sub_1', customerId: 'cus_toto', + userEmail: 'toto@example.com', }); await simulateWebhook(ctx, 'customer.subscription.deleted', { id: 'sub_1' }); @@ -275,23 +284,15 @@ describe('index/stripe', function() { // (when a user accidentally subscribe multiple times), and we don't // want that newly, valid, subscription to be cancelled as a duplicate. - const stripe = mockStripe({ - customers: { - retrieve: async () => { - return { - name: 'Toto', - email: 'toto@example.com', - }; - }, - }, - }); + const stripe = mockStripe({ userEmail: 'toto@example.com' }); const ctx = await koaAppContext(); - await createUserViaSubscription(ctx, 'toto@example.com', { + await createUserViaSubscription(ctx, { stripe, subscriptionId: 'sub_1', customerId: 'cus_toto', + userEmail: 'toto@example.com', }); const stripePrice = findPrice(stripeConfig().prices, { accountType: 1, period: PricePeriod.Monthly }); diff --git a/packages/server/src/routes/index/stripe.ts b/packages/server/src/routes/index/stripe.ts index 1f91e77201..df58bc2603 100644 --- a/packages/server/src/routes/index/stripe.ts +++ b/packages/server/src/routes/index/stripe.ts @@ -106,7 +106,7 @@ export const handleSubscriptionCreated = async (stripe: Stripe, models: Models, } } } else { - logger.info(`Creating subscription for new user: ${userEmail}`); + logger.info(`Creating subscription for new user: ${customerName} (${userEmail})`); await models.subscription().saveUserAndSubscription( userEmail, @@ -233,60 +233,79 @@ export const postHandlers: PostHandlers = { } await models.keyValue().setValue(eventDoneKey, 1); - const hooks: any = { + type HookFunction = ()=> Promise; + + const hooks: Record = { + + // Stripe says that handling this event is required, and to + // provision the subscription at that point: + // + // https://stripe.com/docs/billing/subscriptions/build-subscription?ui=checkout#provision-and-monitor + // + // But it's strange because it doesn't contain any info about the + // subscription. In fact we don't need this event at all, we only + // need "customer.subscription.created", which is sent at the same + // time and actually contains the subscription info. 'checkout.session.completed': async () => { - // Payment is successful and the subscription is created. - // - // For testing: `stripe trigger checkout.session.completed` - // Or use /checkoutTest URL. - const checkoutSession: Stripe.Checkout.Session = event.data.object as Stripe.Checkout.Session; const userEmail = checkoutSession.customer_details.email || checkoutSession.customer_email; - - let customerName = ''; - try { - const customer = await stripe.customers.retrieve(checkoutSession.customer as string) as Stripe.Customer; - customerName = customer.name; - } catch (error) { - logger.error('Could not fetch customer information:', error); - } - logger.info('Checkout session completed:', checkoutSession.id); logger.info('User email:', userEmail); - logger.info('User name:', customerName); - - let accountType = AccountType.Basic; - try { - const priceId: string = await models.keyValue().value(`stripeSessionToPriceId::${checkoutSession.id}`); - accountType = priceIdToAccountType(priceId); - logger.info('Price ID:', priceId); - } catch (error) { - // We don't want this part to fail since the user has - // already paid at that point, so we just default to Basic - // in that case. Normally it shoud not happen anyway. - logger.error('Could not determine account type from price ID - defaulting to "Basic"', error); - } - - logger.info('Account type:', accountType); - - // The Stripe TypeScript object defines "customer" and - // "subscription" as various types but they are actually - // string according to the documentation. - const stripeUserId = checkoutSession.customer as string; - const stripeSubscriptionId = checkoutSession.subscription as string; - - await handleSubscriptionCreated( - stripe, - models, - customerName, - userEmail, - accountType, - stripeUserId, - stripeSubscriptionId - ); }, + // 'checkout.session.completed': async () => { + // // Payment is successful and the subscription is created. + // // + // // For testing: `stripe trigger checkout.session.completed` + // // Or use /checkoutTest URL. + + // const checkoutSession: Stripe.Checkout.Session = event.data.object as Stripe.Checkout.Session; + // const userEmail = checkoutSession.customer_details.email || checkoutSession.customer_email; + + // let customerName = ''; + // try { + // const customer = await stripe.customers.retrieve(checkoutSession.customer as string) as Stripe.Customer; + // customerName = customer.name; + // } catch (error) { + // logger.error('Could not fetch customer information:', error); + // } + + // logger.info('Checkout session completed:', checkoutSession.id); + // logger.info('User email:', userEmail); + // logger.info('User name:', customerName); + + // let accountType = AccountType.Basic; + // try { + // const priceId: string = await models.keyValue().value(`stripeSessionToPriceId::${checkoutSession.id}`); + // accountType = priceIdToAccountType(priceId); + // logger.info('Price ID:', priceId); + // } catch (error) { + // // We don't want this part to fail since the user has + // // already paid at that point, so we just default to Basic + // // in that case. Normally it shoud not happen anyway. + // logger.error('Could not determine account type from price ID - defaulting to "Basic"', error); + // } + + // logger.info('Account type:', accountType); + + // // The Stripe TypeScript object defines "customer" and + // // "subscription" as various types but they are actually + // // string according to the documentation. + // const stripeUserId = checkoutSession.customer as string; + // const stripeSubscriptionId = checkoutSession.subscription as string; + + // await handleSubscriptionCreated( + // stripe, + // models, + // customerName, + // userEmail, + // accountType, + // stripeUserId, + // stripeSubscriptionId + // ); + // }, + 'customer.subscription.created': async () => { const stripeSub: Stripe.Subscription = event.data.object as Stripe.Subscription; const stripeUserId = stripeSub.customer as string;