Merge pull request #19517 from mozilla/fxa-12422

vbudhram · web-flow · commit b1a42e5617c7 · 2025-10-02T15:50:40.000-04:00
feat(skip): Skip confirmation code for a seen device user agent
diff --git a/packages/fxa-auth-server/config/dev.json b/packages/fxa-auth-server/config/dev.json
@@ -31,6 +31,13 @@
       "allowedRecency": 0
     }
   },
+  "signinConfirmation": {
+    "deviceFingerprinting": {
+      "enabled": true,
+      "reportOnlyMode": true,
+      "duration": "7 days"
+    }
+  },
   "lastAccessTimeUpdates": {
     "enabled": true,
     "sampleRate": 1
diff --git a/packages/fxa-auth-server/config/index.ts b/packages/fxa-auth-server/config/index.ts
@@ -1617,6 +1617,26 @@ const convictConf = convict({
       default: false,
       env: 'SIGNIN_CONFIRMATION_FORCE_GLOBALLY',
     },
+    deviceFingerprinting: {
+      enabled: {
+        doc: 'Enable device fingerprinting based verification skip',
+        format: Boolean,
+        default: false,
+        env: 'SIGNIN_CONFIRMATION_DEVICE_FINGERPRINTING_ENABLED',
+      },
+      reportOnlyMode: {
+        doc: 'When true, logs device matches but does not skip verification',
+        format: Boolean,
+        default: true,
+        env: 'SIGNIN_CONFIRMATION_DEVICE_FINGERPRINTING_REPORT_ONLY',
+      },
+      duration: {
+        doc: 'How long a recognized device can skip verification',
+        default: '7 days',
+        format: 'duration',
+        env: 'SIGNIN_CONFIRMATION_DEVICE_FINGERPRINTING_DURATION',
+      },
+    },
     tokenVerification: {
       doc: 'If set to false, force sign-in confirmation for logins that do not request scoped keys. Sets "mustVerify: 0" on created session tokens but creates an entry in unverifiedTokens, simulating an unverified session state',
       format: Boolean,
diff --git a/packages/fxa-auth-server/lib/db.ts b/packages/fxa-auth-server/lib/db.ts
@@ -1064,6 +1064,17 @@ export const createDB = (
       );
     }
 
+    async verifiedLoginSecurityEventsByUid(params: { uid: string; skipTimeframeMs: number}) {
+      log.trace('DB.verifiedLoginSecurityEventsByUid', {
+        params: params,
+      });
+      const { uid, skipTimeframeMs } = params;
+      return SecurityEvent.findByUidAndVerifiedLogin(
+        uid,
+        skipTimeframeMs
+      );
+    }
+
     async securityEventsByUid(params: { uid: string }) {
       log.trace('DB.securityEventsByUid', {
         params: params,
diff --git a/packages/fxa-auth-server/lib/routes/account.ts b/packages/fxa-auth-server/lib/routes/account.ts
@@ -47,7 +47,7 @@ import { DeleteAccountTasks, ReasonForDeletion } from '@fxa/shared/cloud-tasks';
 import { ProfileClient } from '@fxa/profile/client';
 import { DB } from '../db';
 import { StatsD } from 'hot-shots';
-import { recordSecurityEvent } from './utils/security-event';
+import { recordSecurityEvent, isRecognizedDevice } from './utils/security-event';
 import { RelyingPartyConfigurationManager } from '@fxa/shared/cms';
 import { OtpUtils } from './utils/otp';
 
@@ -1074,11 +1074,49 @@ export class AccountHandler {
       return false;
     };
 
-    const skipTokenVerification = (request: AuthRequest, account: any) => {
+    const skipTokenVerification = async (request: AuthRequest, account: any) => {
       // Skip all checks to simulate an unverified session token state
       if (this.config.signinConfirmation.tokenVerification === false) {
         return false;
       }
+
+      // Check to see if there has been a recent, successfully-verified login from
+      // the same device (as indicated by User-Agent string)
+      if (this.config.signinConfirmation.deviceFingerprinting?.enabled) {
+        try {
+          const currentUserAgent = request.headers['user-agent'] || '';
+          const skipTimeframeMs = this.config.signinConfirmation.deviceFingerprinting.duration as unknown as number;
+
+          const isRecognized = await isRecognizedDevice(this.db, account.uid, currentUserAgent, skipTimeframeMs);
+
+          if (isRecognized) {
+            if (this.config.signinConfirmation.deviceFingerprinting.reportOnlyMode) {
+              this.statsd.increment('account.signin.confirm.device.match.reportOnly');
+              this.log.info('account.signin.confirm.device.match.reportOnly', {
+                uid: account.uid,
+              });
+              // Continue to existing logic instead of returning
+            } else {
+              this.statsd.increment('account.signin.confirm.device.skip');
+              this.log.info('account.signin.confirm.device.skip', {
+                uid: account.uid,
+              });
+              return true;
+            }
+          } else {
+            this.statsd.increment('account.signin.confirm.device.notfound');
+            this.log.info('account.signin.confirm.device.notfound', {
+              uid: account.uid,
+            });
+          }
+        } catch (error) {
+          this.log.error('account.signin.confirm.device.error', {
+            uid: account.uid,
+            error: error.message
+          });
+          // Fall through to existing logic on error
+        }
+      }
       // If they're logging in from an IP address on which they recently did
       // another, successfully-verified login, then we can consider this one
       // verified as well without going through the loop again.
@@ -1165,7 +1203,7 @@ export class AccountHandler {
       // to know for sure what flow they're going to see.
       const verificationForced = forceTokenVerification(request, accountRecord);
       if (!verificationForced) {
-        if (skipTokenVerification(request, accountRecord)) {
+        if (await skipTokenVerification(request, accountRecord)) {
           needsVerificationId = false;
         }
       }
diff --git a/packages/fxa-auth-server/lib/routes/utils/security-event.ts b/packages/fxa-auth-server/lib/routes/utils/security-event.ts
@@ -23,3 +23,35 @@ export async function recordSecurityEvent(name: SecurityEventNames, opts: any) {
     },
   });
 }
+
+export async function isRecognizedDevice(
+  db: any,
+  uid: string,
+  userAgent: string,
+  skipTimeframeMs: number
+): Promise<boolean> {
+  const verifiedLoginEvents = await db.verifiedLoginSecurityEventsByUid({
+    uid,
+    skipTimeframeMs
+  });
+
+  if (!verifiedLoginEvents || verifiedLoginEvents.length === 0) {
+    return false;
+  }
+
+  // Search through the results for matching user agent
+  for (const event of verifiedLoginEvents) {
+    if (event.additionalInfo) {
+      try {
+        const additionalInfo = JSON.parse(event.additionalInfo);
+        if (additionalInfo.userAgent === userAgent) {
+          return true;
+        }
+      } catch (e) {
+        // Skip events with invalid JSON
+      }
+    }
+  }
+
+  return false;
+}
diff --git a/packages/fxa-auth-server/test/local/routes/account.js b/packages/fxa-auth-server/test/local/routes/account.js
@@ -3573,6 +3573,219 @@ describe('/account/login', () => {
         }
       );
     });
+
+    describe('skip for seen user agent', () => {
+      beforeEach(() => {
+        config.securityHistory.ipProfiling = {};
+        config.signinConfirmation.skipForNewAccounts = { enabled: false };
+        config.signinConfirmation.deviceFingerprinting = {
+          enabled: true,
+          reportOnlyMode: false,
+          duration: 604800000 // 7 days
+        };
+
+        const email = mockRequest.payload.email;
+
+        mockDB.accountRecord = function () {
+          return Promise.resolve({
+            authSalt: hexString(32),
+            createdAt: Date.now(),
+            data: hexString(32),
+            email: email,
+            emailVerified: true,
+            primaryEmail: {
+              normalizedEmail: normalizeEmail(email),
+              email: email,
+              isVerified: true,
+              isPrimary: true,
+            },
+            kA: hexString(32),
+            lastAuthAt: function () {
+              return Date.now();
+            },
+            uid: uid,
+            wrapWrapKb: hexString(32),
+          });
+        };
+
+        const accountRoutes = makeRoutes({
+          checkPassword: function () {
+            return Promise.resolve(true);
+          },
+          config: config,
+          customs: mockCustoms,
+          db: mockDB,
+          log: mockLog,
+          mailer: mockMailer,
+          push: mockPush,
+          cadReminders: mockCadReminders,
+        });
+
+        route = getRoute(accountRoutes, '/account/login');
+      })
+
+      it('should skip verification when device is recognized and not in report-only mode', () => {
+        mockDB.verifiedLoginSecurityEventsByUid = sinon.spy(() =>
+          Promise.resolve([
+            {
+              name: 'account.login',
+              verified: true,
+              createdAt: Date.now() - 3600000, // 1 hour ago
+              additionalInfo: JSON.stringify({
+                userAgent: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36',
+                location: { country: 'US', state: 'CA' }
+              })
+            }
+          ])
+        );
+
+        const requestWithUserAgent = {
+          ...mockRequest,
+          headers: {
+            'user-agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36'
+          }
+        };
+
+        return runTest(route, requestWithUserAgent, (response) => {
+          assert.equal(
+            mockDB.createSessionToken.callCount,
+            1,
+            'db.createSessionToken was called'
+          );
+          const tokenData = mockDB.createSessionToken.getCall(0).args[0];
+          assert.ok(
+            !tokenData.mustVerify,
+            'sessionToken does not require verification'
+          );
+          assert.ok(
+            response.verified,
+            'response indicates session is verified'
+          );
+
+          assert.calledWith(
+            statsd.increment,
+            'account.signin.confirm.device.skip'
+          );
+        });
+      });
+
+      it('should not skip verification when device is not recognized', () => {
+        mockDB.verifiedLoginSecurityEventsByUid = sinon.spy(() =>
+          Promise.resolve([])
+        );
+
+        const requestWithDifferentUserAgent = {
+          ...mockRequest,
+          headers: {
+            'user-agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36'
+          }
+        };
+
+        return runTest(route, requestWithDifferentUserAgent, (response) => {
+          assert.equal(
+            mockDB.createSessionToken.callCount,
+            1,
+            'db.createSessionToken was called'
+          );
+          const tokenData = mockDB.createSessionToken.getCall(0).args[0];
+          assert.ok(
+            tokenData.mustVerify,
+            'sessionToken requires verification'
+          );
+          assert.ok(
+            !response.verified,
+            'response indicates session is not verified'
+          );
+
+          assert.calledWith(
+            statsd.increment,
+            'account.signin.confirm.device.notfound'
+          );
+        });
+      });
+
+      it('should not skip verification when in report-only mode', () => {
+        config.signinConfirmation.deviceFingerprinting.reportOnlyMode = true;
+
+        mockDB.verifiedLoginSecurityEventsByUid = sinon.spy(() =>
+          Promise.resolve([
+            {
+              name: 'account.login',
+              verified: true,
+              createdAt: Date.now() - 3600000, // 1 hour ago
+              additionalInfo: JSON.stringify({
+                userAgent: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36',
+                location: { country: 'US', state: 'CA' }
+              })
+            }
+          ])
+        );
+
+        const requestWithUserAgent = {
+          ...mockRequest,
+          headers: {
+            'user-agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36'
+          }
+        };
+
+        return runTest(route, requestWithUserAgent, (response) => {
+          assert.equal(
+            mockDB.createSessionToken.callCount,
+            1,
+            'db.createSessionToken was called'
+          );
+          const tokenData = mockDB.createSessionToken.getCall(0).args[0];
+          assert.ok(
+            tokenData.mustVerify,
+            'sessionToken requires verification in report-only mode'
+          );
+          assert.ok(
+            !response.verified,
+            'response indicates session is not verified'
+          );
+
+          // Assert StatsD metric is emitted for report-only mode
+          sinon.assert.calledWith(
+            statsd.increment,
+            'account.signin.confirm.device.match.reportOnly'
+          );
+        });
+      });
+
+      it('should handle errors gracefully and continue to existing logic', () => {
+        mockDB.verifiedLoginSecurityEventsByUid = sinon.spy(() =>
+          Promise.reject(new Error('Database connection failed'))
+        );
+
+        return runTest(route, mockRequest, (response) => {
+          assert.equal(
+            mockDB.createSessionToken.callCount,
+            1,
+            'db.createSessionToken was called'
+          );
+          // Should continue to existing verification logic
+          const tokenData = mockDB.createSessionToken.getCall(0).args[0];
+          assert.ok(
+            tokenData.mustVerify,
+            'sessionToken requires verification when error occurs'
+          );
+        });
+      });
+
+      it('should not call device fingerprinting when disabled', () => {
+        config.signinConfirmation.deviceFingerprinting.enabled = false;
+
+        const originalSpy = mockDB.verifiedLoginSecurityEventsByUid;
+        mockDB.verifiedLoginSecurityEventsByUid = sinon.spy(() => Promise.resolve([]));
+
+        return runTest(route, mockRequest, (response) => {
+          // Should not call the device fingerprinting database method
+          assert.equal(mockDB.verifiedLoginSecurityEventsByUid.callCount, 0, 'device fingerprinting was not called');
+          // Restore original spy
+          mockDB.verifiedLoginSecurityEventsByUid = originalSpy;
+        });
+      });
+    });
   });
 
   it('#integration - creating too many sessions causes an error to be logged', () => {
diff --git a/packages/fxa-auth-server/test/local/routes/utils/security-event.ts b/packages/fxa-auth-server/test/local/routes/utils/security-event.ts
diff --git a/packages/fxa-auth-server/test/mocks.js b/packages/fxa-auth-server/test/mocks.js
diff --git a/packages/fxa-shared/db/models/auth/security-event.ts b/packages/fxa-shared/db/models/auth/security-event.ts
