Fix double subscriptions, second attempt (#14345)

* fix(subscriptions): reject subs that come in too fast

* fix(lint): remove unused import

* fix(groups): individual subs may come rapidly

* fix(subscriptions): bad paren, handle rapid testing

* fix(test): reset dateUpdated between subs

* fix(test): one more block for dateUpdated

Co-authored-by: SabreCat <sabe@habitica.com>

test/api/unit/libs/payments/apple.test.js

@@ -326,9 +326,12 @@ describe('Apple Payments', () => {
     it('errors when a user is already subscribed', async () => {
       payments.createSubscription.restore();
       user = new User();
+      user.purchased.plan.dateUpdated = moment().subtract(1, 'hours').toDate();
       await user.save();
 
       await applePayments.subscribe(sku, user, receipt, headers, nextPaymentProcessing);
+      user.purchased.plan.dateUpdated = moment().subtract(1, 'hours').toDate();
+      await user.save();
 
       await expect(applePayments.subscribe(sku, user, receipt, headers, nextPaymentProcessing))
         .to.eventually.be.rejected.and.to.eql({

test/api/unit/libs/payments/payments.test.js

@@ -350,6 +350,10 @@ describe('payments/index', () => {
     });
 
     context('Purchasing a subscription for self', () => {
+      beforeEach(() => {
+        data.user.purchased.plan.dateUpdated = moment().subtract(1, 'hours').toDate();
+      });
+
       it('creates a subscription', async () => {
         expect(user.purchased.plan.planId).to.not.exist;
 
@@ -376,6 +380,7 @@ describe('payments/index', () => {
         user.purchased.plan = plan;
         user.purchased.plan.dateTerminated = moment(new Date()).add(2, 'months');
         expect(user.purchased.plan.extraMonths).to.eql(0);
+        data.user.purchased.plan.dateUpdated = moment().subtract(1, 'hours').toDate();
 
         await api.createSubscription(data);
 
@@ -386,6 +391,7 @@ describe('payments/index', () => {
         user.purchased.plan = plan;
         user.purchased.plan.dateTerminated = moment(new Date()).subtract(2, 'months');
         expect(user.purchased.plan.extraMonths).to.eql(0);
+        data.user.purchased.plan.dateUpdated = moment().subtract(1, 'hours').toDate();
 
         await api.createSubscription(data);
 
@@ -395,6 +401,7 @@ describe('payments/index', () => {
       it('does not reset Gold-to-Gems cap on additional subscription', async () => {
         user.purchased.plan = plan;
         user.purchased.plan.gemsBought = 10;
+        data.user.purchased.plan.dateUpdated = moment().subtract(1, 'hours').toDate();
 
         await api.createSubscription(data);
 
@@ -448,6 +455,10 @@ describe('payments/index', () => {
     });
 
     context('Block subscription perks', () => {
+      beforeEach(() => {
+        data.user.purchased.plan.dateUpdated = moment().subtract(1, 'hours').toDate();
+      });
+
       it('adds block months to plan.consecutive.offset', async () => {
         await api.createSubscription(data);
 
@@ -486,6 +497,7 @@ describe('payments/index', () => {
         data.sub.key = 'basic_12mo';
 
         await api.createSubscription(data);
+        data.user.purchased.plan.dateUpdated = moment().subtract(1, 'hours').toDate();
         await api.createSubscription(data);
 
         expect(user.purchased.plan.consecutive.gemCapExtra).to.eql(25);
@@ -524,6 +536,7 @@ describe('payments/index', () => {
           now: mayMysteryItemTimeframe,
           toFake: ['Date'],
         });
+        data.user.purchased.plan.dateUpdated = moment().subtract(1, 'hours').toDate();
       });
 
       afterEach(() => {

website/server/libs/payments/subscriptions.js

@@ -11,10 +11,10 @@ import { // eslint-disable-line import/no-cycle
   model as Group,
   basicFields as basicGroupFields,
 } from '../../models/group';
-import { model as User } from '../../models/user'; // eslint-disable-line import/no-cycle
 import {
   NotAuthorized,
   NotFound,
+  TooManyRequests,
 } from '../errors';
 import shared from '../../../common';
 import { sendNotification as sendPushNotification } from '../pushNotifications'; // eslint-disable-line import/no-cycle
@@ -80,19 +80,9 @@ async function createSubscription (data) {
   let emailType = 'subscription-begins';
   let recipientIsSubscribed = recipient.isSubscribed();
 
-  if (data.user && !data.gift && !data.groupId) {
-    const unlockedUser = await User.findOneAndUpdate(
-      {
-        _id: data.user._id,
-        $or: [
-          { _subSignature: 'NOT_RUNNING' },
-          { _subSignature: { $exists: false } },
-        ],
-      },
-      { $set: { _subSignature: 'SUB_IN_PROGRESS' } },
-    );
-    if (!unlockedUser) {
-      throw new NotFound('User not found or subscription already processing.');
+  if (data.user && !data.gift && !data.groupId && data.customerId !== 'group-plan') {
+    if (moment().diff(data.user.purchased.plan.dateUpdated, 'minutes') < 3) {
+      throw new TooManyRequests('Subscription already processed, likely duplicate request');
     }
   }
 
@@ -299,6 +289,10 @@ async function createSubscription (data) {
     }
   }
 
+  if (group) await group.save();
+  if (data.user && data.user.isModified()) await data.user.save();
+  if (data.gift) await data.gift.member.save();
+
   slack.sendSubscriptionNotification({
     buyer: {
       id: data.user._id,
@@ -315,24 +309,6 @@ async function createSubscription (data) {
     groupId,
     autoRenews,
   });
-
-  if (group) {
-    await group.save();
-  }
-  if (data.user) {
-    if (data.user.isModified()) {
-      await data.user.save();
-    }
-    if (!data.gift && !data.groupId) {
-      await User.findOneAndUpdate(
-        { _id: data.user._id },
-        { $set: { _subSignature: 'NOT_RUNNING' } },
-      );
-    }
-  }
-  if (data.gift) {
-    await data.gift.member.save();
-  }
 }
 
 // Cancels a subscription or group plan, setting termination to happen later