Merge train-131 into master

https://github.com/mozilla/fxa-auth-server/pull/2930
r=vladikoff

CHANGELOG.md

@@ -1,3 +1,14 @@
+<a name="1.131.1"></a>
+## [1.131.1](https://github.com/mozilla/fxa-auth-server/compare/v1.131.0...v1.131.1) (2019-02-21)
+
+
+### Bug Fixes
+
+* **api:** add validation for utm params and entrypoint ([9732cf2](https://github.com/mozilla/fxa-auth-server/commit/9732cf2))
+* **metrics:** reinstate entrypoint to the metrics context schema ([551467e](https://github.com/mozilla/fxa-auth-server/commit/551467e))
+
+
+
 <a name="1.131.0"></a>
 # [1.131.0](https://github.com/mozilla/fxa-auth-server/compare/v1.130.1...v1.131.0) (2019-02-20)
 

docs/api.md

@@ -397,13 +397,14 @@ those common validations are defined here.
 
 * `SCHEMA`: object({
     * `deviceId`: string, length(32), regex(HEX_STRING), optional
+    * `entrypoint`: ENTRYPOINT_SCHEMA.optional
     * `flowId`: string, length(64), regex(HEX_STRING), optional
     * `flowBeginTime`: number, integer, positive, optional
-    * `utmCampaign`: string, optional
-    * `utmContent`: string, optional
-    * `utmMedium`: string, optional
-    * `utmSource`: string, optional
-    * `utmTerm`: string, optional
+    * `utmCampaign`: UTM_CAMPAIGN_SCHEMA.optional
+    * `utmContent`: UTM_SCHEMA.optional
+    * `utmMedium`: UTM_SCHEMA.optional
+    * `utmSource`: UTM_SCHEMA.optional
+    * `utmTerm`: UTM_SCHEMA.optional
 
   }), unknown(false), and('flowId', 'flowBeginTime')
 * `schema`: SCHEMA.optional

lib/metrics/context.js

@@ -12,19 +12,26 @@ const P = require('../promise')
 
 const FLOW_ID_LENGTH = 64
 
+// These match validation in the content server backend.
+// We should probably refactor them to fxa-shared.
+const ENTRYPOINT_SCHEMA = isA.string().max(128).regex(/^[\w.:-]+$/)
+const UTM_SCHEMA = isA.string().max(128).regex(/^[\w\/.%-]+$/)
+const UTM_CAMPAIGN_SCHEMA = UTM_SCHEMA.allow('page+referral+-+not+part+of+a+campaign')
+
 const SCHEMA = isA.object({
   // The metrics context device id is a client-generated property
   // that is entirely separate to the devices table in our db.
   // All clients can generate a metrics context device id, whereas
   // only Sync creates records in the devices table.
   deviceId: isA.string().length(32).regex(HEX_STRING).optional(),
+  entrypoint: ENTRYPOINT_SCHEMA.optional(),
   flowId: isA.string().length(64).regex(HEX_STRING).optional(),
   flowBeginTime: isA.number().integer().positive().optional(),
-  utmCampaign: isA.string().optional(),
-  utmContent: isA.string().optional(),
-  utmMedium: isA.string().optional(),
-  utmSource: isA.string().optional(),
-  utmTerm: isA.string().optional()
+  utmCampaign: UTM_CAMPAIGN_SCHEMA.optional(),
+  utmContent: UTM_SCHEMA.optional(),
+  utmMedium: UTM_SCHEMA.optional(),
+  utmSource: UTM_SCHEMA.optional(),
+  utmTerm: UTM_SCHEMA.optional()
 })
   .unknown(false)
   .and('flowId', 'flowBeginTime')
@@ -144,6 +151,7 @@ module.exports = function (log, config) {
 
       const doNotTrack = this.headers && this.headers.dnt === '1'
       if (! doNotTrack) {
+        data.entrypoint = metadata.entrypoint
         data.utm_campaign = metadata.utmCampaign
         data.utm_content = metadata.utmContent
         data.utm_medium = metadata.utmMedium

npm-shrinkwrap.json

@@ -1,6 +1,6 @@
 {
   "name": "fxa-auth-server",
-  "version": "1.131.0",
+  "version": "1.131.1",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "fxa-auth-server",
-  "version": "1.131.0",
+  "version": "1.131.1",
   "description": "Firefox Accounts, an identity provider for Mozilla cloud services",
   "bin": {
     "fxa-auth": "./bin/key_server.js"

test/local/log.js

@@ -601,6 +601,7 @@ describe('log', () => {
       payload: {
         service: 'testservice',
         metricsContext: {
+          entrypoint: 'wibble',
           flowBeginTime: now - 23,
           flowId: 'F1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF103',
           utmCampaign: 'utm campaign',
@@ -622,6 +623,7 @@ describe('log', () => {
           ts: now,
           metricsContext: {
             time: now,
+            entrypoint: 'wibble',
             flow_id: request.payload.metricsContext.flowId,
             flow_time: now - request.payload.metricsContext.flowBeginTime,
             flowBeginTime: request.payload.metricsContext.flowBeginTime,

test/local/metrics/context.js

@@ -438,9 +438,10 @@ describe('metricsContext', () => {
       }, {}).then(function (result) {
         assert.equal(typeof result, 'object', 'result is object')
         assert.notEqual(result, null, 'result is not null')
-        assert.equal(Object.keys(result).length, 13, 'result has 13 properties')
+        assert.equal(Object.keys(result).length, 14, 'result has 14 properties')
         assert.ok(result.time > time, 'result.time seems correct')
         assert.equal(result.device_id, 'mock device id', 'result.device_id is correct')
+        assert.equal(result.entrypoint, 'mock entry point')
         assert.equal(result.flow_id, 'mock flow id', 'result.flow_id is correct')
         assert.ok(result.flow_time > 0, 'result.flow_time is greater than zero')
         assert.ok(result.flow_time < time, 'result.flow_time is less than the current time')
@@ -488,6 +489,7 @@ describe('metricsContext', () => {
         }
       }, {}).then(function (result) {
         assert.equal(Object.keys(result).length, 8, 'result has 8 properties')
+        assert.isUndefined(result.entrypoint)
         assert.equal(result.utm_campaign, undefined, 'result.utm_campaign is undefined')
         assert.equal(result.utm_content, undefined, 'result.utm_content is undefined')
         assert.equal(result.utm_medium, undefined, 'result.utm_medium is undefined')

test/local/metrics/events.js

@@ -170,6 +170,7 @@ describe('metrics/events', () => {
       metricsContext,
       payload: {
         metricsContext: {
+          entrypoint: 'wibble',
           flowId: 'bar',
           flowBeginTime: time - 1000,
           flowCompleteSignal: 'account.signed',
@@ -197,6 +198,7 @@ describe('metrics/events', () => {
         assert.deepEqual(args[0], {
           country: 'United States',
           event: 'email.verification.sent',
+          entrypoint: 'wibble',
           flow_id: 'bar',
           flow_time: 1000,
           flowBeginTime: time - 1000,

test/local/routes/account.js

@@ -341,6 +341,7 @@ describe('/account/create', () => {
         service: 'sync',
         metricsContext: {
           deviceId: 'wibble',
+          entrypoint: 'blee',
           flowBeginTime: Date.now(),
           flowId: 'F1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF103',
           utmCampaign: 'utm campaign',
@@ -458,6 +459,7 @@ describe('/account/create', () => {
       assert.equal(eventData.data.email, TEST_EMAIL, 'it was for the correct email')
       assert.ok(eventData.data.ts, 'timestamp of event set')
       assert.deepEqual(eventData.data.metricsContext, {
+        entrypoint: 'blee',
         flowBeginTime: mockRequest.payload.metricsContext.flowBeginTime,
         flowCompleteSignal: 'account.signed',
         flowType: undefined,
@@ -488,6 +490,7 @@ describe('/account/create', () => {
       assert.equal(args.length, 1, 'log.flowEvent was passed one argument')
       assert.deepEqual(args[0], {
         country: 'United States',
+        entrypoint: 'blee',
         event: 'account.created',
         flowBeginTime: mockRequest.payload.metricsContext.flowBeginTime,
         flowCompleteSignal: 'account.signed',
@@ -632,6 +635,7 @@ describe('/account/login', function () {
       reason: 'signin',
       metricsContext: {
         deviceId: 'blee',
+        entrypoint: 'flub',
         flowBeginTime: Date.now(),
         flowId: 'F1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF103',
         utmCampaign: 'utm campaign',

test/mocks.js

@@ -484,6 +484,7 @@ function mockMetricsContext (methods) {
               flowCompleteSignal: this.payload.metricsContext.flowCompleteSignal,
               flowType: this.payload.metricsContext.flowType
             }, this.headers && this.headers.dnt === '1' ? {} : {
+              entrypoint: this.payload.metricsContext.entrypoint,
               utm_campaign: this.payload.metricsContext.utmCampaign,
               utm_content: this.payload.metricsContext.utmContent,
               utm_medium: this.payload.metricsContext.utmMedium,

test/remote/account_create_tests.js

@@ -374,6 +374,131 @@ describe('remote account create', function() {
     }
   )
 
+  it('valid metricsContext', () => {
+    const api = new Client.Api(config.publicUrl)
+    const email = server.uniqueEmail()
+    const authPW = '0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF'
+    const options = {
+      metricsContext: {
+        entrypoint: 'foo',
+        utmCampaign: 'bar',
+        utmContent: 'baz',
+        utmMedium: 'qux',
+        utmSource: 'wibble',
+        utmTerm: 'blee',
+      }
+    }
+    return api.accountCreate(email, authPW, options)
+  })
+
+  it('invalid entrypoint', () => {
+    const api = new Client.Api(config.publicUrl)
+    const email = server.uniqueEmail()
+    const authPW = '0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF'
+    const options = {
+      metricsContext: {
+        entrypoint: ';',
+        utmCampaign: 'foo',
+        utmContent: 'bar',
+        utmMedium: 'baz',
+        utmSource: 'qux',
+        utmTerm: 'wibble',
+      }
+    }
+    return api.accountCreate(email, authPW, options)
+      .then(assert.fail, err => assert.equal(err.errno, 107))
+  })
+
+  it('invalid utmCampaign', () => {
+    const api = new Client.Api(config.publicUrl)
+    const email = server.uniqueEmail()
+    const authPW = '0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF'
+    const options = {
+      metricsContext: {
+        entrypoint: 'foo',
+        utmCampaign: ';',
+        utmContent: 'bar',
+        utmMedium: 'baz',
+        utmSource: 'qux',
+        utmTerm: 'wibble',
+      }
+    }
+    return api.accountCreate(email, authPW, options)
+      .then(assert.fail, err => assert.equal(err.errno, 107))
+  })
+
+  it('invalid utmContent', () => {
+    const api = new Client.Api(config.publicUrl)
+    const email = server.uniqueEmail()
+    const authPW = '0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF'
+    const options = {
+      metricsContext: {
+        entrypoint: 'foo',
+        utmCampaign: 'bar',
+        utmContent: ';',
+        utmMedium: 'baz',
+        utmSource: 'qux',
+        utmTerm: 'wibble',
+      }
+    }
+    return api.accountCreate(email, authPW, options)
+      .then(assert.fail, err => assert.equal(err.errno, 107))
+  })
+
+  it('invalid utmMedium', () => {
+    const api = new Client.Api(config.publicUrl)
+    const email = server.uniqueEmail()
+    const authPW = '0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF'
+    const options = {
+      metricsContext: {
+        entrypoint: 'foo',
+        utmCampaign: 'bar',
+        utmContent: 'baz',
+        utmMedium: ';',
+        utmSource: 'qux',
+        utmTerm: 'wibble',
+      }
+    }
+    return api.accountCreate(email, authPW, options)
+      .then(assert.fail, err => assert.equal(err.errno, 107))
+  })
+
+  it('invalid utmSource', () => {
+    const api = new Client.Api(config.publicUrl)
+    const email = server.uniqueEmail()
+    const authPW = '0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF'
+    const options = {
+      metricsContext: {
+        entrypoint: 'foo',
+        utmCampaign: 'bar',
+        utmContent: 'baz',
+        utmMedium: 'qux',
+        utmSource: ';',
+        utmTerm: 'wibble',
+      }
+    }
+    return api.accountCreate(email, authPW, options)
+      .then(assert.fail, err => assert.equal(err.errno, 107))
+  })
+
+  it('invalid utmTerm', () => {
+    const api = new Client.Api(config.publicUrl)
+    const email = server.uniqueEmail()
+    const authPW = '0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF'
+    const options = {
+      metricsContext: {
+        entrypoint: 'foo',
+        utmCampaign: 'bar',
+        utmContent: 'baz',
+        utmMedium: 'qux',
+        utmSource: 'wibble',
+        utmTerm: ';',
+      }
+    }
+    return api.accountCreate(email, authPW, options)
+      .then(assert.fail, err => assert.equal(err.errno, 107))
+  })
+
   it(
     'create account with service query parameter',
     () => {