Merge pull request #19493 from mozilla/FXA-12136

feat(auth): Add security events for login confirmation skip

Author: nshirley

packages/db-migrations/databases/fxa/patches/patch-176-177.sql

@@ -0,0 +1,5 @@
+CALL assertPatchLevel('176');
+
+INSERT INTO securityEventNames(name) VALUES ('account.signin_confirm_bypass_known_ip'), ('account.signin_confirm_bypass_new_account');
+
+UPDATE dbMetadata SET value = '177' WHERE name = 'schema-patch-level';

packages/db-migrations/databases/fxa/patches/patch-177-176.sql

@@ -0,0 +1,6 @@
+-- SET NAMES utf8mb4 COLLATE utf8mb4_bin;
+--
+-- DELETE FROM securityEventNames WHERE name = 'account.signin_confirm_bypass_known_ip';
+-- DELETE FROM securityEventNames WHERE name = 'account.signin_confirm_bypass_new_account';
+--
+-- UPDATE dbMetadata SET value = '176' WHERE name = 'schema-patch-level';

packages/db-migrations/databases/fxa/target-patch.json

@@ -1,3 +1,3 @@
 {
-  "level": 176
+  "level": 177
 }

packages/fxa-auth-server/lib/routes/account.ts

@@ -1090,6 +1090,11 @@ export class AccountHandler {
         this.log.info('Account.ipprofiling.seenAddress', {
           uid: account.uid,
         });
+        recordSecurityEvent('account.signin_confirm_bypass_known_ip', {
+          db: this.db,
+          request,
+          account,
+        });
         return true;
       }
 
@@ -1106,6 +1111,11 @@ export class AccountHandler {
           this.log.info('account.signin.confirm.bypass.age', {
             uid: account.uid,
           });
+          recordSecurityEvent('account.signin_confirm_bypass_new_account', {
+            db: this.db,
+            request,
+            account,
+          });
           return true;
         }
       }

packages/fxa-auth-server/test/local/routes/account.js

@@ -99,7 +99,10 @@ const makeRoutes = function (options = {}, requireMocks = {}) {
   Container.set(AuthLogger, log);
 
   Container.set(AppConfig, config);
-  Container.set(AccountEventsManager, new AccountEventsManager());
+  Container.set(
+    AccountEventsManager,
+    options.mockAccountEventsManager || new AccountEventsManager()
+  );
   Container.set(RelyingPartyConfigurationManager, rpConfigManager);
 
   const mailer = options.mailer || {};
@@ -3153,7 +3156,7 @@ describe('/account/login', () => {
     });
 
     describe('skip for new accounts', () => {
-      function setup(enabled, accountCreatedSince) {
+      function setup(enabled, accountCreatedSince, makeRoutesOptions = {}) {
         config.signinConfirmation.skipForNewAccounts = {
           enabled: enabled,
           maxAge: 5,
@@ -3184,6 +3187,7 @@ describe('/account/login', () => {
         };
 
         const accountRoutes = makeRoutes({
+          ...makeRoutesOptions,
           checkPassword: function () {
             return Promise.resolve(true);
           },
@@ -3373,14 +3377,30 @@ describe('/account/login', () => {
       it('logs metrics when accountAge is under maxAge config threshold', () => {
         glean.loginConfirmSkipFor.newAccount.reset();
 
-        setup(true, 0);
+        const mockAccountEventsManager = {
+          recordSecurityEvent: sinon.fake(),
+        };
+        setup(true, 0, { mockAccountEventsManager });
 
         return runTest(route, mockRequest, () => {
           sinon.assert.calledOnce(glean.loginConfirmSkipFor.newAccount);
           sinon.assert.calledWith(
             statsd.increment,
             'account.signin.confirm.bypass.newAccount'
           );
+          sinon.assert.calledWithMatch(
+            mockAccountEventsManager.recordSecurityEvent,
+            mockDB,
+            sinon.match({
+              name: 'account.signin_confirm_bypass_new_account',
+              uid,
+              ipAddr: mockRequest.app.clientAddress,
+              additionalInfo: {
+                userAgent: mockRequest.headers['user-agent'],
+                location: mockRequest.app.geo.location,
+              },
+            })
+          );
         });
       });
 
@@ -3396,6 +3416,11 @@ describe('/account/login', () => {
             },
           ]);
         });
+        const mockAccountEventsManager = {
+          recordSecurityEvent: sinon.fake(),
+        };
+        setup(true, 0, { mockAccountEventsManager });
+
         return runTest(route, mockRequest, (response) => {
           assert.equal(
             mockDB.verifiedLoginSecurityEvents.callCount,
@@ -3404,6 +3429,19 @@ describe('/account/login', () => {
           );
 
           sinon.assert.called(glean.loginConfirmSkipFor.knownIp);
+          sinon.assert.calledWithMatch(
+            mockAccountEventsManager.recordSecurityEvent,
+            mockDB,
+            sinon.match({
+              name: 'account.signin_confirm_bypass_known_ip',
+              uid,
+              ipAddr: mockRequest.app.clientAddress,
+              additionalInfo: {
+                userAgent: mockRequest.headers['user-agent'],
+                location: mockRequest.app.geo.location,
+              },
+            })
+          );
         });
       });
     });

packages/fxa-shared/db/models/auth/security-event.ts

@@ -59,6 +59,8 @@ export const EVENT_NAMES: Record<string, number> = {
   'account.mfa_send_otp_code': 45,
   'account.mfa_verify_otp_code_success': 46,
   'account.mfa_verify_otp_code_failed': 47,
+  'account.signin_confirm_bypass_known_ip': 48,
+  'account.signin_confirm_bypass_new_account': 49,
 } as const;
 
 export type SecurityEventNames = keyof typeof EVENT_NAMES;