diff --git a/CHANGELOG.md b/CHANGELOG.md index 8596b072..6a974cfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ + +## [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)) + + + # [1.131.0](https://github.com/mozilla/fxa-auth-server/compare/v1.130.1...v1.131.0) (2019-02-20) diff --git a/docs/api.md b/docs/api.md index 03270ab1..9c2a6cb8 100644 --- a/docs/api.md +++ b/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 diff --git a/lib/metrics/context.js b/lib/metrics/context.js index 8d213710..c03daa12 100644 --- a/lib/metrics/context.js +++ b/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 diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 8392f144..9e8ad090 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "fxa-auth-server", - "version": "1.131.0", + "version": "1.131.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index d8d659a2..9956ee67 100644 --- a/package.json +++ b/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" diff --git a/test/local/log.js b/test/local/log.js index 46b84df5..4c70644f 100644 --- a/test/local/log.js +++ b/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, diff --git a/test/local/metrics/context.js b/test/local/metrics/context.js index 1697c4c9..bae2e343 100644 --- a/test/local/metrics/context.js +++ b/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') diff --git a/test/local/metrics/events.js b/test/local/metrics/events.js index 349e139b..ee3b0b00 100644 --- a/test/local/metrics/events.js +++ b/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, diff --git a/test/local/routes/account.js b/test/local/routes/account.js index abc9c5f3..dfb25797 100644 --- a/test/local/routes/account.js +++ b/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', diff --git a/test/mocks.js b/test/mocks.js index 6954210b..206fee07 100644 --- a/test/mocks.js +++ b/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, diff --git a/test/remote/account_create_tests.js b/test/remote/account_create_tests.js index aa3b7119..783db0e4 100644 --- a/test/remote/account_create_tests.js +++ b/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', () => {