Server: Prevent duplicate Stripe subscriptions and improved Stripe workflow testing

packages/server/src/routes/index/stripe.test.ts

@@ -1,26 +1,49 @@
 import { findPrice, PricePeriod } from '@joplin/lib/utils/joplinCloud';
 import { AccountType } from '../../models/UserModel';
-import { betaUserTrialPeriodDays, initStripe, isBetaUser, stripeConfig } from '../../utils/stripe';
+import { betaUserTrialPeriodDays, isBetaUser, stripeConfig } from '../../utils/stripe';
 import { beforeAllDb, afterAllTests, beforeEachDb, models, koaAppContext, expectNotThrow } from '../../utils/testing/testUtils';
 import uuidgen from '../../utils/uuidgen';
 import { postHandlers } from './stripe';
 
-async function createUserViaSubscription(userEmail: string, eventId: string = '') {
+function mockStripe() {
-	eventId = eventId || uuidgen();
+	return {
+		customers: {
+			retrieve: jest.fn(),
+		},
+		subscriptions: {
+			del: jest.fn(),
+		},
+	};
+}
+
+interface SubscriptionOptions {
+	stripe?: any;
+	eventId?: string;
+	subscriptionId?: string;
+}
+
+async function createUserViaSubscription(userEmail: string, options: SubscriptionOptions = {}) {
+	options = {
+		stripe: mockStripe(),
+		eventId: uuidgen(),
+		subscriptionId: `sub_${uuidgen()}`,
+		...options,
+	};
+
 	const stripeSessionId = 'sess_123';
 	const stripePrice = findPrice(stripeConfig().prices, { accountType: 2, period: PricePeriod.Monthly });
 	await models().keyValue().setValue(`stripeSessionToPriceId::${stripeSessionId}`, stripePrice.id);
 
 	const ctx = await koaAppContext();
-	const stripe = initStripe();
+
-	await postHandlers.webhook(stripe, {}, ctx, {
+	await postHandlers.webhook(options.stripe, {}, ctx, {
-		id: eventId,
+		id: options.eventId,
 		type: 'checkout.session.completed',
 		data: {
 			object: {
 				id: stripeSessionId,
-				customer: 'cus_123',
+				customer: `cus_${uuidgen()}`,
-				subscription: 'sub_123',
+				subscription: options.subscriptionId,
 				customer_details: {
 					email: userEmail,
 				},
@@ -47,7 +70,7 @@ describe('index/stripe', function() {
 	test('should handle the checkout.session.completed event', async function() {
 		const startTime = Date.now();
 
-		await createUserViaSubscription('toto@example.com');
+		await createUserViaSubscription('toto@example.com', { subscriptionId: 'sub_123' });
 
 		const user = await models().user().loadByEmail('toto@example.com');
 		expect(user.account_type).toBe(AccountType.Pro);
@@ -60,11 +83,11 @@ describe('index/stripe', function() {
 	});
 
 	test('should not process the same event twice', async function() {
-		await createUserViaSubscription('toto@example.com', 'evt_1');
+		await createUserViaSubscription('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('toto@example.com', 'evt_1'));
+		await expectNotThrow(async () => createUserViaSubscription('toto@example.com', { eventId: 'evt_1' }));
 	});
 
 	test('should check if it is a beta user', async function() {
@@ -91,4 +114,34 @@ describe('index/stripe', function() {
 		expect(betaUserTrialPeriodDays(1614682158000, fromDateTime)).toBe(7); // Tue Mar 02 2021 10:49:18 GMT+0000
 	});
 
+	test('should setup subscription for an existing user', async function() {
+		// This is for example if a user has been manually added to the system,
+		// and then later they setup their subscription. Applies to beta users
+		// for instance.
+		const user = await models().user().save({ email: 'toto@example.com', password: uuidgen() });
+		expect(await models().subscription().byUserId(user.id)).toBeFalsy();
+		await createUserViaSubscription('toto@example.com', { subscriptionId: 'sub_123' });
+
+		const sub = await models().subscription().byUserId(user.id);
+		expect(sub).toBeTruthy();
+		expect(sub.stripe_subscription_id).toBe('sub_123');
+	});
+
+	test('should cancel duplicate subscriptions', async function() {
+		const stripe = mockStripe();
+
+		await createUserViaSubscription('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('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);
+
+		expect(subBefore.stripe_subscription_id).toBe(subAfter.stripe_subscription_id);
+	});
+
 });

packages/server/src/routes/index/stripe.ts

@@ -9,7 +9,7 @@ import { Stripe } from 'stripe';
 import Logger from '@joplin/lib/Logger';
 import getRawBody = require('raw-body');
 import { AccountType } from '../../models/UserModel';
-import { betaUserTrialPeriodDays, initStripe, isBetaUser, priceIdToAccountType, stripeConfig } from '../../utils/stripe';
+import { betaUserTrialPeriodDays, cancelSubscription, initStripe, isBetaUser, priceIdToAccountType, stripeConfig } from '../../utils/stripe';
 import { Subscription } from '../../db';
 import { findPrice, PricePeriod } from '@joplin/lib/utils/joplinCloud';
 
@@ -244,8 +244,10 @@ export const postHandlers: PostHandlers = {
 				const existingUser = await models.user().loadByEmail(userEmail);
 
 				if (existingUser) {
-					if (await isBetaUser(models, existingUser.id)) {
+					const sub = await models.subscription().byUserId(existingUser.id);
-						logger.info(`Setting up Beta user subscription: ${existingUser.email}`);
+
+					if (!sub) {
+						logger.info(`Setting up subscription for existing user: ${existingUser.email}`);
 
 						// First set the account type correctly (in case the
 						// user also upgraded or downgraded their account). Also
@@ -264,11 +266,15 @@ export const postHandlers: PostHandlers = {
 							last_payment_time: Date.now(),
 						});
 					} else {
-						// TODO: Some users accidentally subscribe multiple
+						// The user already has a subscription. Most likely
-						// times - in that case, cancel the subscription and
+						// they accidentally created a second one, so cancel
-						// don't do anything more.
+						// it.
+						logger.info(`User ${existingUser.email} already has a subscription: ${sub.stripe_subscription_id} - cancelling duplicate`);
+						await cancelSubscription(stripe, stripeSubscriptionId);
 					}
 				} else {
+					logger.info(`Creating subscription for new user: ${userEmail}`);
+
 					await models.subscription().saveUserAndSubscription(
 						userEmail,
 						customerName,

packages/server/src/utils/stripe.ts

@@ -50,13 +50,17 @@ export function stripePriceIdByStripeSub(stripeSub: Stripe.Subscription): string
 	return stripeSub.items.data[0].price.id;
 }
 
-export async function cancelSubscription(models: Models, userId: Uuid) {
+export async function cancelSubscriptionByUserId(models: Models, userId: Uuid) {
 	const sub = await models.subscription().byUserId(userId);
 	if (!sub) throw new Error(`No subscription for user: ${userId}`);
 	const stripe = initStripe();
 	await stripe.subscriptions.del(sub.stripe_subscription_id);
 }
 
+export async function cancelSubscription(stripe: Stripe, stripeSubId: string) {
+	await stripe.subscriptions.del(stripeSubId);
+}
+
 export async function updateSubscriptionType(models: Models, userId: Uuid, newAccountType: AccountType) {
 	const user = await models.user().load(userId);
 	if (user.account_type === newAccountType) throw new Error(`Account type is already: ${newAccountType}`);