Merge pull request #6569 from overleaf/ab-recurly-webhook-filter

Add a failfast for invalid userId on recurly webhook

GitOrigin-RevId: 94e7c5fd756f5fbc592a9c346fca157883625396

services/web/app/src/Features/Subscription/RecurlyEventHandler.js

@@ -1,33 +1,39 @@
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const SplitTestHandler = require('../SplitTests/SplitTestHandler')
 const SubscriptionEmailHandler = require('./SubscriptionEmailHandler')
+const { ObjectID } = require('mongodb')
+
+async function sendRecurlyAnalyticsEvent(event, eventData) {
+  const userId = _getUserId(eventData)
+  if (!ObjectID.isValid(userId)) {
+    return
+  }
 
-function sendRecurlyAnalyticsEvent(event, eventData) {
   switch (event) {
     case 'new_subscription_notification':
-      sendSubscriptionStartedEvent(eventData)
+      await _sendSubscriptionStartedEvent(userId, eventData)
       break
     case 'updated_subscription_notification':
-      _sendSubscriptionUpdatedEvent(eventData)
+      await _sendSubscriptionUpdatedEvent(userId, eventData)
       break
     case 'canceled_subscription_notification':
-      _sendSubscriptionCancelledEvent(eventData)
+      await _sendSubscriptionCancelledEvent(userId, eventData)
       break
     case 'expired_subscription_notification':
-      _sendSubscriptionExpiredEvent(eventData)
+      await _sendSubscriptionExpiredEvent(userId, eventData)
       break
     case 'renewed_subscription_notification':
-      _sendSubscriptionRenewedEvent(eventData)
+      await _sendSubscriptionRenewedEvent(userId, eventData)
       break
     case 'reactivated_account_notification':
-      _sendSubscriptionReactivatedEvent(eventData)
+      await _sendSubscriptionReactivatedEvent(userId, eventData)
       break
     case 'paid_charge_invoice_notification':
       if (
         eventData.invoice.state === 'paid' &&
         eventData.invoice.total_in_cents > 0
       ) {
-        _sendInvoicePaidEvent(eventData)
+        await _sendInvoicePaidEvent(userId, eventData)
       }
       break
     case 'closed_invoice_notification':
@@ -35,14 +41,13 @@ function sendRecurlyAnalyticsEvent(event, eventData) {
         eventData.invoice.state === 'collected' &&
         eventData.invoice.total_in_cents > 0
       ) {
-        _sendInvoicePaidEvent(eventData)
+        await _sendInvoicePaidEvent(userId, eventData)
       }
       break
   }
 }
 
-async function sendSubscriptionStartedEvent(eventData) {
-  const userId = _getUserId(eventData)
+async function _sendSubscriptionStartedEvent(userId, eventData) {
   const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData)
   AnalyticsManager.recordEventForUser(userId, 'subscription-started', {
     plan_code: planCode,
@@ -69,13 +74,12 @@ async function sendSubscriptionStartedEvent(eventData) {
     )
 
     if (assignment.variant === 'send-email') {
-      SubscriptionEmailHandler.sendTrialOnboardingEmail(userId)
+      await SubscriptionEmailHandler.sendTrialOnboardingEmail(userId)
     }
   }
 }
 
-async function _sendSubscriptionUpdatedEvent(eventData) {
-  const userId = _getUserId(eventData)
+async function _sendSubscriptionUpdatedEvent(userId, eventData) {
   const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData)
   AnalyticsManager.recordEventForUser(userId, 'subscription-updated', {
     plan_code: planCode,
@@ -95,8 +99,7 @@ async function _sendSubscriptionUpdatedEvent(eventData) {
   )
 }
 
-async function _sendSubscriptionCancelledEvent(eventData) {
-  const userId = _getUserId(eventData)
+async function _sendSubscriptionCancelledEvent(userId, eventData) {
   const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData)
   AnalyticsManager.recordEventForUser(userId, 'subscription-cancelled', {
     plan_code: planCode,
@@ -111,8 +114,7 @@ async function _sendSubscriptionCancelledEvent(eventData) {
   )
 }
 
-async function _sendSubscriptionExpiredEvent(eventData) {
-  const userId = _getUserId(eventData)
+async function _sendSubscriptionExpiredEvent(userId, eventData) {
   const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData)
   AnalyticsManager.recordEventForUser(userId, 'subscription-expired', {
     plan_code: planCode,
@@ -132,8 +134,7 @@ async function _sendSubscriptionExpiredEvent(eventData) {
   )
 }
 
-async function _sendSubscriptionRenewedEvent(eventData) {
-  const userId = _getUserId(eventData)
+async function _sendSubscriptionRenewedEvent(userId, eventData) {
   const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData)
   AnalyticsManager.recordEventForUser(userId, 'subscription-renewed', {
     plan_code: planCode,
@@ -153,8 +154,7 @@ async function _sendSubscriptionRenewedEvent(eventData) {
   )
 }
 
-async function _sendSubscriptionReactivatedEvent(eventData) {
-  const userId = _getUserId(eventData)
+async function _sendSubscriptionReactivatedEvent(userId, eventData) {
   const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData)
   AnalyticsManager.recordEventForUser(userId, 'subscription-reactivated', {
     plan_code: planCode,
@@ -173,8 +173,7 @@ async function _sendSubscriptionReactivatedEvent(eventData) {
   )
 }
 
-async function _sendInvoicePaidEvent(eventData) {
-  const userId = _getUserId(eventData)
+async function _sendInvoicePaidEvent(userId, eventData) {
   AnalyticsManager.recordEventForUser(userId, 'subscription-invoice-collected')
   AnalyticsManager.setUserPropertyForUser(
     userId,
@@ -211,5 +210,4 @@ function _getSubscriptionData(eventData) {
 
 module.exports = {
   sendRecurlyAnalyticsEvent,
-  sendSubscriptionStartedEvent,
 }

services/web/app/src/Features/Subscription/SubscriptionController.js

@@ -357,7 +357,12 @@ function recurlyCallback(req, res, next) {
   const event = Object.keys(req.body)[0]
   const eventData = req.body[event]
 
-  RecurlyEventHandler.sendRecurlyAnalyticsEvent(event, eventData)
+  RecurlyEventHandler.sendRecurlyAnalyticsEvent(event, eventData).catch(error =>
+    logger.error(
+      { err: error },
+      'Failed to process analytics event on Recurly webhook'
+    )
+  )
 
   if (
     [

services/web/test/unit/src/Subscription/RecurlyEventHandlerTests.js

@@ -5,7 +5,7 @@ const modulePath =
 
 describe('RecurlyEventHandler', function () {
   beforeEach(function () {
-    this.userId = '123456789abcde'
+    this.userId = '123abc234bcd456cde567def'
     this.planCode = 'collaborator-annual'
     this.eventData = {
       account: {
@@ -86,12 +86,11 @@ describe('RecurlyEventHandler', function () {
     this.SplitTestHandler.promises.getAssignmentForUser = sinon
       .stub()
       .resolves({ variant: 'send-email' })
-    this.userId = '123456789trial'
-    this.eventData.account.account_code = this.userId
 
-    // testing directly on the send subscription started event to ensure the split handler
-    // promise is resolved before checking calls
-    await this.RecurlyEventHandler.sendSubscriptionStartedEvent(this.eventData)
+    await this.RecurlyEventHandler.sendRecurlyAnalyticsEvent(
+      'new_subscription_notification',
+      this.eventData
+    )
 
     sinon.assert.calledWith(
       this.SplitTestHandler.promises.getAssignmentForUser,
@@ -345,4 +344,18 @@ describe('RecurlyEventHandler', function () {
     )
     sinon.assert.notCalled(this.AnalyticsManager.recordEventForUser)
   })
+
+  it('nothing is called with invalid account code', function () {
+    this.eventData.account.account_code = 'foo_bar'
+
+    this.RecurlyEventHandler.sendRecurlyAnalyticsEvent(
+      'new_subscription_notification',
+      this.eventData
+    )
+    sinon.assert.notCalled(this.AnalyticsManager.recordEventForUser)
+    sinon.assert.notCalled(this.AnalyticsManager.setUserPropertyForUser)
+    sinon.assert.notCalled(this.AnalyticsManager.setUserPropertyForUser)
+    sinon.assert.notCalled(this.AnalyticsManager.setUserPropertyForUser)
+    sinon.assert.notCalled(this.SplitTestHandler.promises.getAssignmentForUser)
+  })
 })

services/web/test/unit/src/Subscription/SubscriptionControllerTests.js

@@ -149,7 +149,9 @@ describe('SubscriptionController', function () {
         './RecurlyWrapper': (this.RecurlyWrapper = {
           updateAccountEmailAddress: sinon.stub().yields(),
         }),
-        './RecurlyEventHandler': { sendRecurlyAnalyticsEvent: sinon.stub() },
+        './RecurlyEventHandler': {
+          sendRecurlyAnalyticsEvent: sinon.stub().resolves(),
+        },
         './FeaturesUpdater': (this.FeaturesUpdater = {}),
         './GroupPlansData': (this.GroupPlansData = {}),
         './V1SubscriptionManager': (this.V1SubscriptionManager = {}),