bug(auth,event-broker): Don't allow invalid client ids through

Because:
- ClientIds with values 'none' were making their way to the auth server
- This was happening because 'none' is a valid value for service, and service was a fallback for clientId.

This Commit:
- Creates stricter checks on what a valid clientId is.

FXA-9802

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;
   }

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,

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()