fix(params): Gracefully handle invalid utm params

packages/fxa-content-server/app/scripts/lib/metrics.js

@@ -16,6 +16,7 @@
 
 import $ from 'jquery';
 import _ from 'underscore';
+import AuthErrors from '../lib/auth-errors';
 import Cocktail from 'cocktail';
 import Constants from './constants';
 import Backbone from 'backbone';
@@ -27,6 +28,7 @@ import speedTrap from 'speed-trap';
 import Strings from './strings';
 import SubscriptionModel from 'models/subscription';
 import xhr from './xhr';
+import Validate from '../lib/validate';
 
 // Speed trap is a singleton, convert it
 // to an instantiable function.
@@ -83,6 +85,7 @@ const ALLOWED_FIELDS = [
 var DEFAULT_INACTIVITY_TIMEOUT_MS = new Duration('10s').milliseconds();
 var NOT_REPORTED_VALUE = 'none';
 var UNKNOWN_CAMPAIGN_ID = 'unknown';
+const INVALID_UTM = 'invalid';
 
 // convert a hash of metrics impressions into an array of objects.
 function flattenHashIntoArrayOfObjects(hashTable) {
@@ -115,6 +118,15 @@ function marshallProperty(property) {
   }
 }
 
+function marshallUtmProperty(property) {
+  if (property && property !== NOT_REPORTED_VALUE) {
+    if (Validate.isUtmValid(property)) {
+      return property;
+    }
+    return INVALID_UTM;
+  }
+}
+
 function marshallEmailDomain(email) {
   if (!email) {
     return;
@@ -330,6 +342,8 @@ _.extend(Metrics.prototype, Backbone.Events, {
     // reported again.
     this._numStoredAccounts = '';
 
+    this._validateSanitizeUtmParams(filteredData);
+
     const send = () => this._send(filteredData, isPageUnloading);
     return (
       send()
@@ -383,6 +397,30 @@ _.extend(Metrics.prototype, Backbone.Events, {
     }, this._inactivityFlushMs);
   },
 
+  // Since RPs and client do not have full control over the UTM
+  // params that could be passed to them, we need to sanitize those values
+  // the best we can. This replaces and reports any invalid utm params which
+  // allows us to still submit the metrics.
+  _validateSanitizeUtmParams(filteredData) {
+    const utmKeyReg = /^utm_/;
+    return Object.keys(filteredData)
+      .filter((key) => {
+        return utmKeyReg.test(key);
+      })
+      .forEach((utmKey) => {
+        const valid = Validate.isUtmValid(filteredData[utmKey]);
+        if (!valid && this._sentryMetrics) {
+          this._sentryMetrics.captureException(
+            AuthErrors.toInvalidParameterError(utmKey)
+          );
+
+          // Override original UTM param with `invalid` value. This will allow us
+          // to submit the metrics
+          filteredData[utmKey] = INVALID_UTM;
+        }
+      });
+  },
+
   /**
    * Get all the data, whether it's allowed to be sent or not.
    *
@@ -767,11 +805,11 @@ _.extend(Metrics.prototype, Backbone.Events, {
       entrypointVariation: marshallProperty(this._entrypointVariation),
       flowBeginTime: metadata.flowBegin,
       flowId: metadata.flowId,
-      utmCampaign: marshallProperty(this._utmCampaign),
-      utmContent: marshallProperty(this._utmContent),
-      utmMedium: marshallProperty(this._utmMedium),
-      utmSource: marshallProperty(this._utmSource),
-      utmTerm: marshallProperty(this._utmTerm),
+      utmCampaign: marshallUtmProperty(this._utmCampaign),
+      utmContent: marshallUtmProperty(this._utmContent),
+      utmMedium: marshallUtmProperty(this._utmMedium),
+      utmSource: marshallUtmProperty(this._utmSource),
+      utmTerm: marshallUtmProperty(this._utmTerm),
       productId: subscriptionMetadata.productId || undefined,
       planId: subscriptionMetadata.planId || undefined,
     };

packages/fxa-content-server/app/scripts/lib/validate.js

@@ -49,6 +49,8 @@ const TOTP_CODE = /^[0-9]{6}$/;
 // Recovery codes can be 8-10 alpha numeric characters
 const RECOVERY_CODE = /^([a-zA-Z0-9]{8,10})$/;
 
+const utmRegex = /^[\w\/.%-]{1,128}$/;
+
 var Validate = {
   /**
    * Check if an email address is valid
@@ -273,6 +275,16 @@ var Validate = {
 
     return areAllValid;
   },
+
+  /**
+   * Check if the utm param is valid.
+   *
+   * @param {String} value
+   * @returns {Boolean}
+   */
+  isUtmValid(value) {
+    return utmRegex.test(value);
+  },
 };
 
 export default Validate;

packages/fxa-content-server/app/tests/spec/lib/metrics.js

@@ -26,6 +26,7 @@ describe('lib/metrics', () => {
   let environment;
   let metrics;
   let notifier;
+  let sentryMock;
   let windowMock;
   let xhr;
 
@@ -37,6 +38,10 @@ describe('lib/metrics', () => {
 
     xhr = { ajax() {} };
 
+    sentryMock = {
+      captureException: sinon.spy(),
+    };
+
     metrics = new Metrics(
       _.defaults(options, {
         brokerType: 'fx-desktop-v3',
@@ -54,6 +59,7 @@ describe('lib/metrics', () => {
         notifier,
         screenHeight: 1200,
         screenWidth: 1600,
+        sentryMetrics: sentryMock,
         service: 'sync',
         startTime: 1439233336187,
         uid: '0ae7fe2b244f4a789857dff3ae263927',
@@ -975,6 +981,47 @@ describe('lib/metrics', () => {
     });
   });
 
+  describe('sanitizes and reports invalid utm', () => {
+    it('flushes as expected', (done) => {
+      createMetrics({
+        utmCampaign: '(not valid)',
+        utmContent: 'utm_content',
+        utmMedium: 'utm_medium',
+        utmSource: 'none',
+        utmTerm: '',
+      });
+
+      sinon.stub(metrics, '_send').resolves(true);
+
+      metrics.logEvent('signin.success');
+
+      setTimeout(() => {
+        try {
+          assert.equal(metrics._send.callCount, 1);
+
+          const firstPayload = metrics._send.args[0][0];
+          assert.equal(firstPayload.events[0].type, 'signin.success');
+          assert.equal(firstPayload.utm_campaign, 'invalid');
+          assert.equal(firstPayload.utm_content, 'utm_content');
+          assert.equal(firstPayload.utm_medium, 'utm_medium');
+          assert.equal(firstPayload.utm_source, 'none');
+          assert.equal(firstPayload.utm_term, 'none');
+
+          assert.equal(sentryMock.captureException.callCount, 1);
+          assert.equal(
+            sentryMock.captureException.args[0][0].errno,
+            107,
+            'invalid param'
+          );
+        } catch (err) {
+          return done(err);
+        }
+
+        done();
+      }, 50);
+    });
+  });
+
   describe('all together now', () => {
     it('flushes as expected', (done) => {
       sinon.stub(metrics, '_send').resolves(true);

packages/fxa-content-server/app/tests/spec/lib/validate.js

@@ -359,4 +359,22 @@ describe('lib/validate', function () {
       );
     });
   });
+
+  describe('isUtmValid', () => {
+    it('returns false if more than 128 chars', () => {
+      assert.isFalse(Validate.isUtmValid(createRandomHexString(129)));
+    });
+
+    it('returns false for empty string', () => {
+      assert.isFalse(Validate.isUtmValid(''));
+    });
+
+    it('returns false for invalid chars', () => {
+      assert.isFalse(Validate.isUtmValid('(not valid)'));
+    });
+
+    it('returns true for valid chars', () => {
+      assert.isTrue(Validate.isUtmValid('marketing-snippet'));
+    });
+  });
 });