Updated invoice webhook handling for payment events

no-issue

1. We do not want to store payment events for payments of 0 value
2. Stripe webhooks can arrive and be processed "out of order", which can
   result in us attempting to add a payment event for a member which
   does not yet exist. The change here will 404 in such (edge) cases, so
   that Stripe will retry the webhook at a later point, when the Member
   has been created, allowing us to store the payment event.

ghost/members-api/index.js

@@ -90,6 +90,7 @@ module.exports = function MembersApi({
     const stripeWebhookService = new StripeWebhookService({
         StripeWebhook,
         stripeAPIService,
+        stripePlansService,
         memberRepository,
         eventRepository,
         sendEmailWithMagicLink
@@ -312,7 +313,7 @@ module.exports = function MembersApi({
             res.end();
         } catch (err) {
             common.logging.error(`Error handling webhook ${event.type}`, err);
-            res.writeHead(400);
+            res.writeHead(err.statusCode || 500);
             res.end();
         }
     });

ghost/members-api/lib/services/stripe-webhook/index.js

@@ -1,10 +1,12 @@
 const _ = require('lodash');
+const errors = require('@tryghost/errors');
 
 module.exports = class StripeWebhookService {
     /**
      * @param {object} deps
      * @param {any} deps.StripeWebhook
      * @param {import('../stripe-api')} deps.stripeAPIService
+     * @param {import('../stripe-plans')} deps.stripePlansService
      * @param {import('../../repositories/member')} deps.memberRepository
      * @param {import('../../repositories/event')} deps.eventRepository
      * @param {any} deps.sendEmailWithMagicLink
@@ -12,12 +14,14 @@ module.exports = class StripeWebhookService {
     constructor({
         StripeWebhook,
         stripeAPIService,
+        stripePlansService,
         memberRepository,
         eventRepository,
         sendEmailWithMagicLink
     }) {
         this._StripeWebhook = StripeWebhook;
         this._stripeAPIService = stripeAPIService;
+        this._stripePlansService = stripePlansService;
         this._memberRepository = memberRepository;
         this._eventRepository = eventRepository;
         this._sendEmailWithMagicLink = sendEmailWithMagicLink;
@@ -152,13 +156,26 @@ module.exports = class StripeWebhookService {
         });
 
         if (member) {
-            if (invoice.paid) {
+            if (invoice.paid && invoice.amount_paid !== 0) {
                 await this._eventRepository.registerPayment({
                     member_id: member.id,
                     currency: invoice.currency,
                     amount: invoice.amount_paid
                 });
             }
+        } else {
+            // Subscription has more than one plan - meaning it is not one created by us - ignore.
+            if (!subscription.plan) {
+                return;
+            }
+            // Subscription is for a different product - ignore.
+            if (this._stripePlansService.getProduct().id !== subscription.plan.product) {
+                return;
+            }
+            // Could not find the member, which we need in order to insert an payment event.
+            throw new errors.NotFoundError({
+                message: `No member found for customer ${subscription.customer}`
+            });
         }
     }
 

ghost/members-api/package.json

@@ -25,6 +25,7 @@
     "sinon": "7.5.0"
   },
   "dependencies": {
+    "@tryghost/errors": "^0.2.9",
     "@tryghost/magic-link": "^0.6.7",
     "bluebird": "^3.5.4",
     "body-parser": "^1.19.0",

ghost/members-api/test/unit/lib/services/stripe-webhook.test.js

@@ -0,0 +1,45 @@
+const {describe, it} = require('mocha');
+const should = require('should');
+const sinon = require('sinon');
+const StripeAPIService = require('../../../../lib/services/stripe-api');
+const StripePlansService = require('../../../../lib/services/stripe-plans');
+const StripeWebhookService = require('../../../../lib/services/stripe-webhook');
+const MemberRepository = require('../../../../lib/repositories/member');
+
+function mock(Class) {
+    return sinon.stub(Object.create(Class.prototype));
+}
+
+describe('StripeWebhookService', function () {
+    describe('invoice.payment_succeeded webhooks', function () {
+        it('Should throw a 404 error when a member is not found for a valid Ghost Members invoice', async function () {
+            const stripeWebhookService = new StripeWebhookService({
+                stripeAPIService: mock(StripeAPIService),
+                stripePlansService: mock(StripePlansService),
+                memberRepository: mock(MemberRepository)
+            });
+
+            stripeWebhookService._stripeAPIService.getSubscription.resolves({
+                customer: 'customer_id',
+                plan: {
+                    product: 'product_id'
+                }
+            });
+
+            stripeWebhookService._memberRepository.get.resolves(null);
+
+            stripeWebhookService._stripePlansService.getProduct.returns({
+                id: 'product_id'
+            });
+
+            try {
+                await stripeWebhookService.invoiceEvent({
+                    subscription: 'sub_id'
+                });
+                should.fail();
+            } catch (err) {
+                should.equal(err.statusCode, 404);
+            }
+        });
+    });
+});