Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Commit

Permalink
fix(metrics): Loosen /metrics-flow validation
Browse files Browse the repository at this point in the history
* Firefox 62's about:welcome page sends no query params, make all query params optional.
* Allow `context`, about:newinstall sends it unnecessarily
  • Loading branch information
Shane Tomlinson committed Mar 5, 2019
1 parent d14e116 commit 658f38d
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 33 deletions.
15 changes: 10 additions & 5 deletions server/lib/routes/get-metrics-flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const uuid = require('node-uuid');
const validation = require('../validation');

const {
CONTEXT: CONTEXT_PATTERN,
ENTRYPOINT: ENTRYPOINT_PATTERN,
FORM_TYPE: FORM_TYPE_PATTERN,
SERVICE: SERVICE_PATTERN,
Expand Down Expand Up @@ -44,18 +45,22 @@ module.exports = function (config) {
preflightContinue: false
};

// No query params are passed by Firefox 62's about:welcome page,
// so all are marked optional
const QUERY_SCHEMA = joi.object({
// Not passed by the Firefox Concert Series, otherwise should be required.
// Passed by about:newinstall unnecessarily, allow it.
context: STRING_TYPE.regex(CONTEXT_PATTERN).optional(),
// Not passed by the Firefox Concert Series.
// See https://github.com/mozilla/bedrock/issues/6839
entrypoint: STRING_TYPE.regex(ENTRYPOINT_PATTERN).optional(),
// Not passed by the Firefox Concert Series, otherwise should be required.
// Not passed by the Firefox Concert Series.
// See https://github.com/mozilla/bedrock/issues/6839
'form_type': STRING_TYPE.regex(FORM_TYPE_PATTERN).optional(),
'service': STRING_TYPE.regex(SERVICE_PATTERN).optional(),
'utm_campaign': UTM_CAMPAIGN_TYPE.required(),
'utm_campaign': UTM_CAMPAIGN_TYPE.optional(),
'utm_content': UTM_TYPE.optional(),
'utm_medium': UTM_TYPE.optional(),
'utm_source': UTM_TYPE.required(),
'utm_source': UTM_TYPE.optional(),
'utm_term': UTM_TYPE.optional()
});

Expand All @@ -76,7 +81,7 @@ module.exports = function (config) {
const paramValue = errorDetails && errorDetails.context && errorDetails.context.value;
logger.info('request.metrics-flow.invalid-param', {
param: paramName || 'unknown',
value: paramValue || 'unknown'
value: paramValue || 'unknown',
});
}

Expand Down
44 changes: 16 additions & 28 deletions tests/server/routes/get-metrics-flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ registerSuite('routes/get-metrics-flow', {
request = {
headers: {},
query: {
context: 'blee',
entrypoint: 'zoo',
'form_type': 'other',
'service': 'sync',
Expand Down Expand Up @@ -120,14 +121,26 @@ registerSuite('routes/get-metrics-flow', {
assert.ok(metricsData.deviceId);
},

'logs invalid context query parameter': function() {
request = {
headers: {},
query: {
context: 'con text'
}
};
instance.process(request, response);
assert.isTrue(mocks.log.info.calledOnceWith('request.metrics-flow.invalid-param', {
param: 'context',
value: 'con text',
}));
assert.isTrue(response.json.calledOnce);
},

'logs invalid entrypoint query parameter': function() {
request = {
headers: {},
query: {
entrypoint: 'foo bar',
'form_type': 'email',
'utm_campaign': 'biz',
'utm_source': 'baz',
}
};
instance.process(request, response);
Expand All @@ -142,10 +155,7 @@ registerSuite('routes/get-metrics-flow', {
request = {
headers: {},
query: {
entrypoint: 'bar',
'form_type': 'biz',
'utm_campaign': 'biz',
'utm_source': 'baz',
}
};
instance.process(request, response);
Expand All @@ -160,11 +170,7 @@ registerSuite('routes/get-metrics-flow', {
request = {
headers: {},
query: {
entrypoint: 'bar',
'form_type': 'email',
'service': 'zzzz',
'utm_campaign': 'biz',
'utm_source': 'baz',
}
};
instance.process(request, response);
Expand All @@ -179,10 +185,7 @@ registerSuite('routes/get-metrics-flow', {
request = {
headers: {},
query: {
entrypoint: 'bar',
'form_type': 'email',
'utm_campaign': 1,
'utm_source': 'baz',
}
};
instance.process(request, response);
Expand All @@ -197,11 +200,7 @@ registerSuite('routes/get-metrics-flow', {
request = {
headers: {},
query: {
entrypoint: 'bar',
'form_type': 'email',
'utm_campaign': 'biz',
'utm_content': 'qux qux',
'utm_source': 'baz',
}
};
instance.process(request, response);
Expand All @@ -216,11 +215,7 @@ registerSuite('routes/get-metrics-flow', {
request = {
headers: {},
query: {
entrypoint: 'bar',
'form_type': 'email',
'utm_campaign': 'biz',
'utm_medium': 'wimble!@$',
'utm_source': 'baz',
}
};
instance.process(request, response);
Expand All @@ -235,9 +230,6 @@ registerSuite('routes/get-metrics-flow', {
request = {
headers: {},
query: {
entrypoint: 'bar',
'form_type': 'email',
'utm_campaign': 'biz',
'utm_source': '%!@%womble'
}
};
Expand All @@ -253,10 +245,6 @@ registerSuite('routes/get-metrics-flow', {
request = {
headers: {},
query: {
entrypoint: 'bar',
'form_type': 'email',
'utm_campaign': 'biz',
'utm_source': 'baz',
'utm_term': 'jum!%^gle'
}
};
Expand Down

0 comments on commit 658f38d

Please sign in to comment.