diff --git a/packages/fxa-auth-server/lib/log.js b/packages/fxa-auth-server/lib/log.js index f9ef0c9138..850856d83a 100644 --- a/packages/fxa-auth-server/lib/log.js +++ b/packages/fxa-auth-server/lib/log.js @@ -14,6 +14,7 @@ const validateAmplitudeEvent = require('fxa-shared').metrics.amplitude.validate; let statsd; const Sentry = require('@sentry/node'); const notifier = require('./notifier'); +const validators = require('./oauth/validators'); const ISSUER = config.get('domain') || ''; const CLIENT_ID_TO_SERVICE_NAMES = config.get('oauth.clientIds') || {}; @@ -214,8 +215,9 @@ Lug.prototype.notifyAttachedServices = async function (name, request, data) { data.iss = ISSUER; // convert an oauth client-id to a human readable format, if a name is available. - // If no name is available, continue to use the client_id. - if (data.service && data.service !== 'sync') { + // If no name is available, continue to use the client_id. Note that oauth client + // IDs are always hex strings. So values like sync or none aren't valid... + if (data.service && validators.HEX_STRING.test(data.service)) { data.clientId = data.service; data.service = CLIENT_ID_TO_SERVICE_NAMES[data.service] || data.service; } diff --git a/packages/fxa-auth-server/test/local/log.js b/packages/fxa-auth-server/test/local/log.js index efe55611a3..db9b4009fb 100644 --- a/packages/fxa-auth-server/test/local/log.js +++ b/packages/fxa-auth-server/test/local/log.js @@ -927,7 +927,7 @@ describe('log', () => { log, metricsContext, payload: { - service: 'clientid', + service: '0123456789abcdef', metricsContext: { entrypoint: 'wibble', entrypointExperiment: 'blee-experiment', @@ -946,7 +946,7 @@ describe('log', () => { sinon.stub(Date, 'now').callsFake(() => now); return log .notifyAttachedServices('login', request, { - service: 'clientid', + service: '0123456789abcdef', ts: now, }) .then(() => { @@ -956,8 +956,8 @@ describe('log', () => { assert.deepEqual(log.notifier.send.args[0][0], { event: 'login', data: { - clientId: 'clientid', - service: 'human readable name', + clientId: '0123456789abcdef', + service: '0123456789abcdef', timestamp: now, ts: now, iss: 'example.com', @@ -1021,7 +1021,6 @@ describe('log', () => { assert.deepEqual(log.notifier.send.args[0][0], { event: 'login', data: { - clientId: 'unknown-clientid', service: 'unknown-clientid', timestamp: now, ts: now, diff --git a/packages/fxa-event-broker/src/queueworker/sqs.dto.ts b/packages/fxa-event-broker/src/queueworker/sqs.dto.ts index 15ac230707..ff97834e14 100644 --- a/packages/fxa-event-broker/src/queueworker/sqs.dto.ts +++ b/packages/fxa-event-broker/src/queueworker/sqs.dto.ts @@ -14,7 +14,8 @@ export const SUBSCRIPTION_UPDATE_EVENT = 'subscription:update'; export const APPLE_USER_MIGRATION_EVENT = 'appleUserMigration'; // Message schemas -export const CLIENT_ID = joi.string().regex(/[a-z0-9]{16}/); +const HEX_STRING = /^(?:[0-9a-f]{2})+$/; +export const CLIENT_ID = joi.string().regex(HEX_STRING); export const BASE_MESSAGE_SCHEMA = joi .object()