Merge pull request #13428 from mozilla/fxa-3621-handle-merch-payment-no-pay-invoice

fix(auth): validate invoice for paypal before marking paid

packages/fxa-auth-server/lib/routes/subscriptions/paypal-notifications.ts

@@ -13,7 +13,7 @@ import Stripe from 'stripe';
 import { ConfigType } from '../../../config';
 import error from '../../error';
 import { IpnMerchPmtType, isIpnMerchPmt } from '../../payments/paypal/client';
-import { StripeHelper, STRIPE_CUSTOMER_METADATA } from '../../payments/stripe';
+import { StripeHelper, SUBSCRIPTIONS_RESOURCE } from '../../payments/stripe';
 import { reportSentryError } from '../../sentry';
 import { AuthLogger, AuthRequest } from '../../types';
 import { PayPalHandler } from './paypal';
@@ -21,6 +21,51 @@ import { PayPalHandler } from './paypal';
 const IPN_EXCLUDED = ['mp_signup'];
 
 export class PayPalNotificationHandler extends PayPalHandler {
+  /**
+   * Handle a successful payment notification from PayPal
+   * Perform some validation on and update the Stripe invoice accordingly.
+   */
+  private async handleSuccessfulPayment(
+    invoice: Stripe.Invoice,
+    message: IpnMerchPmtType
+  ) {
+    const paypalTransactionId =
+      this.stripeHelper.getInvoicePaypalTransactionId(invoice);
+    if (!paypalTransactionId) {
+      await this.stripeHelper.updateInvoiceWithPaypalTransactionId(
+        invoice,
+        message.txn_id
+      );
+    } else if (paypalTransactionId !== message.txn_id) {
+      this.log.error('handleSuccessfulPayment', {
+        message: 'Invoice paypalTransactionId does not match Paypal IPN txn_id',
+        invoiceId: invoice.id,
+        paypalIPNTxnId: message.txn_id,
+      });
+      throw error.internalValidationError('handleSuccessfulPayment', {
+        message: 'Invoice paypalTransactionId does not match Paypal IPN txn_id',
+        invoiceId: invoice.id,
+        paypalIPNTxnId: message.txn_id,
+      });
+    }
+
+    if (invoice.subscription) {
+      const subscription =
+        typeof invoice.subscription !== 'string'
+          ? invoice.subscription
+          : await this.stripeHelper.expandResource<Stripe.Subscription>(
+              invoice.subscription,
+              SUBSCRIPTIONS_RESOURCE
+            );
+
+      if (subscription?.status === 'canceled') {
+        return this.paypalHelper.issueRefund(invoice, message.txn_id);
+      }
+    }
+
+    return this.stripeHelper.payInvoiceOutOfBand(invoice);
+  }
+
   /**
    * Handle merchant payment notification from PayPal
    * and update Stripe invoice according to the payment_status
@@ -55,7 +100,7 @@ export class PayPalNotificationHandler extends PayPalHandler {
     switch (message.payment_status) {
       case 'Completed':
       case 'Processed':
-        return this.stripeHelper.payInvoiceOutOfBand(invoice);
+        return this.handleSuccessfulPayment(invoice, message);
       case 'Pending':
       case 'In-Progress':
         return;
@@ -205,9 +250,11 @@ export class PayPalNotificationHandler extends PayPalHandler {
       if (isIpnMerchPmt(payload)) {
         this.log.debug('Handling Ipn message', { payload });
         if (payload.txn_type === 'merch_pmt') {
-          return this.handleMerchPayment(payload);
+          // Added await, before returning, so that errors thrown by
+          // the functions are caught and handled by this try/catch.
+          return await this.handleMerchPayment(payload);
         } else {
-          return this.handleMpCancel(payload);
+          return await this.handleMpCancel(payload);
         }
       }
       if (!IPN_EXCLUDED.includes(payload.txn_type)) {

packages/fxa-auth-server/test/local/routes/subscriptions/paypal-notifications.js

@@ -32,6 +32,8 @@ const { PayPalNotificationHandler } = proxyquire(
 const { PayPalHelper } = require('../../../../lib/payments/paypal/helper');
 const { CapabilityService } = require('../../../../lib/payments/capability');
 
+import { SUBSCRIPTIONS_RESOURCE } from '../../../../lib/payments/stripe';
+
 const ACCOUNT_LOCALE = 'en-US';
 const TEST_EMAIL = 'test@email.com';
 const UID = uuid.v4({}, Buffer.alloc(16)).toString('hex');
@@ -197,6 +199,145 @@ describe('PayPalNotificationHandler', () => {
     });
   });
 
+  describe('handleSuccessfulPayment', () => {
+    const ipnTransactionId = 'ipn_id_123';
+    const validMessage = {
+      txn_id: ipnTransactionId,
+    };
+    const validInvoice = {
+      metadata: {
+        paypalTransactionId: ipnTransactionId,
+      },
+      subscription: {
+        status: 'active',
+      },
+    };
+    const paidInvoice = { status: 'paid' };
+    const refundReturn = undefined;
+
+    beforeEach(() => {
+      stripeHelper.getInvoicePaypalTransactionId =
+        sinon.fake.returns(ipnTransactionId);
+      stripeHelper.payInvoiceOutOfBand = sinon.fake.resolves(paidInvoice);
+      paypalHelper.issueRefund = sinon.fake.resolves(refundReturn);
+    });
+
+    it('should update invoice to paid', async () => {
+      const invoice = validInvoice;
+      const message = validMessage;
+
+      const result = await handler.handleSuccessfulPayment(invoice, message);
+
+      assert.deepEqual(result, paidInvoice);
+      sinon.assert.calledOnceWithExactly(
+        stripeHelper.payInvoiceOutOfBand,
+        invoice
+      );
+      sinon.assert.notCalled(paypalHelper.issueRefund);
+    });
+
+    it('should update Invoice with paypalTransactionId if not already there', async () => {
+      const invoice = {
+        subscription: {
+          status: 'active',
+        },
+      };
+      const message = validMessage;
+      stripeHelper.getInvoicePaypalTransactionId =
+        sinon.fake.returns(undefined);
+      stripeHelper.updateInvoiceWithPaypalTransactionId =
+        sinon.fake.resolves(validInvoice);
+
+      const result = await handler.handleSuccessfulPayment(invoice, message);
+
+      assert.deepEqual(result, paidInvoice);
+      sinon.assert.calledOnceWithExactly(
+        stripeHelper.updateInvoiceWithPaypalTransactionId,
+        invoice,
+        ipnTransactionId
+      );
+      sinon.assert.calledOnceWithExactly(
+        stripeHelper.payInvoiceOutOfBand,
+        invoice
+      );
+      sinon.assert.notCalled(paypalHelper.issueRefund);
+    });
+
+    it('should throw an error when paypalTransactionId and IPN txn_id dont match', async () => {
+      const invoice = validInvoice;
+      const message = {
+        txn_id: 'notcorrect',
+      };
+
+      try {
+        await handler.handleSuccessfulPayment(invoice, message);
+        assert.fail('Error should throw error with transactionId not matching');
+      } catch (err) {
+        assert.deepEqual(
+          err,
+          error.internalValidationError('handleSuccessfulPayment', {
+            message:
+              'Invoice paypalTransactionId does not match Paypal IPN txn_id',
+            invoiceId: invoice.id,
+            paypalIPNTxnId: message.txn_id,
+          })
+        );
+        sinon.assert.notCalled(stripeHelper.payInvoiceOutOfBand);
+        sinon.assert.notCalled(paypalHelper.issueRefund);
+      }
+    });
+
+    it('should not expand subscription and refund the invoice if the subscription has status canceled', async () => {
+      const invoice = {
+        metadata: {
+          paypalTransactionId: ipnTransactionId,
+        },
+        subscription: {
+          status: 'canceled',
+        },
+      };
+      const message = validMessage;
+      stripeHelper.expandResource = sinon.spy();
+
+      const result = await handler.handleSuccessfulPayment(invoice, message);
+
+      assert.deepEqual(result, refundReturn);
+      sinon.assert.calledOnceWithExactly(
+        paypalHelper.issueRefund,
+        invoice,
+        validMessage.txn_id
+      );
+      sinon.assert.notCalled(stripeHelper.expandResource);
+      sinon.assert.notCalled(stripeHelper.payInvoiceOutOfBand);
+    });
+
+    it('should expand subscription and refund the invoice if the subscription has status canceled', async () => {
+      const invoice = {
+        metadata: {
+          paypalTransactionId: ipnTransactionId,
+        },
+        subscription: 'sub_id',
+      };
+      const message = validMessage;
+      stripeHelper.expandResource = sinon.fake.resolves({ status: 'canceled' });
+
+      const result = await handler.handleSuccessfulPayment(invoice, message);
+
+      assert.deepEqual(result, refundReturn);
+      sinon.assert.calledOnceWithExactly(
+        paypalHelper.issueRefund,
+        invoice,
+        validMessage.txn_id
+      );
+      sinon.assert.calledOnceWithExactly(
+        stripeHelper.expandResource,
+        invoice.subscription,
+        SUBSCRIPTIONS_RESOURCE
+      );
+      sinon.assert.notCalled(stripeHelper.payInvoiceOutOfBand);
+    });
+  });
+
   describe('handleMerchPayment', () => {
     const message = {
       txn_type: 'merch_pmt',
@@ -206,7 +347,8 @@ describe('PayPalNotificationHandler', () => {
       const invoice = { status: 'open' };
       const paidInvoice = { status: 'paid' };
       stripeHelper.getInvoice = sinon.fake.resolves(invoice);
-      stripeHelper.payInvoiceOutOfBand = sinon.fake.resolves(paidInvoice);
+      handler.handleSuccessfulPayment = sinon.fake.resolves(paidInvoice);
+
       const result = await handler.handleMerchPayment(
         completedMerchantPaymentNotification
       );
@@ -216,8 +358,9 @@ describe('PayPalNotificationHandler', () => {
         completedMerchantPaymentNotification.invoice
       );
       sinon.assert.calledOnceWithExactly(
-        stripeHelper.payInvoiceOutOfBand,
-        invoice
+        handler.handleSuccessfulPayment,
+        invoice,
+        completedMerchantPaymentNotification
       );
     });