Merge pull request #20396 from mozilla/FXA-13444

feat(passkeys): atomic passkey session token creation with verificationMethod pre-stamped

Author: vpomerleau

packages/db-migrations/databases/fxa/patches/patch-190-191.sql

@@ -0,0 +1,68 @@
+SET NAMES utf8mb4 COLLATE utf8mb4_bin;
+
+CALL assertPatchLevel('190');
+
+CREATE PROCEDURE `createVerifiedSessionToken_1` (
+  IN `tokenId` BINARY(32),
+  IN `tokenData` BINARY(32),
+  IN `uid` BINARY(16),
+  IN `createdAt` BIGINT UNSIGNED,
+  IN `uaBrowser` VARCHAR(255),
+  IN `uaBrowserVersion` VARCHAR(255),
+  IN `uaOS` VARCHAR(255),
+  IN `uaOSVersion` VARCHAR(255),
+  IN `uaDeviceType` VARCHAR(255),
+  IN `uaFormFactor` VARCHAR(255),
+  IN `mustVerify` BOOLEAN,
+  IN `providerId` TINYINT,
+  IN `verificationMethod` INT,
+  IN `verifiedAt` BIGINT UNSIGNED
+)
+BEGIN
+  DECLARE EXIT HANDLER FOR SQLEXCEPTION
+  BEGIN
+    ROLLBACK;
+    RESIGNAL;
+  END;
+
+  START TRANSACTION;
+
+  INSERT INTO sessionTokens(
+    tokenId,
+    tokenData,
+    uid,
+    createdAt,
+    uaBrowser,
+    uaBrowserVersion,
+    uaOS,
+    uaOSVersion,
+    uaDeviceType,
+    uaFormFactor,
+    lastAccessTime,
+    mustVerify,
+    providerId,
+    verificationMethod,
+    verifiedAt
+  )
+  VALUES(
+    tokenId,
+    tokenData,
+    uid,
+    createdAt,
+    uaBrowser,
+    uaBrowserVersion,
+    uaOS,
+    uaOSVersion,
+    uaDeviceType,
+    uaFormFactor,
+    createdAt,
+    mustVerify,
+    providerId,
+    verificationMethod,
+    verifiedAt
+  );
+
+  COMMIT;
+END;
+
+UPDATE dbMetadata SET value = '191' WHERE name = 'schema-patch-level';

packages/db-migrations/databases/fxa/patches/patch-191-190.sql

@@ -0,0 +1,5 @@
+-- SET NAMES utf8mb4 COLLATE utf8mb4_bin;
+
+-- DROP PROCEDURE `createVerifiedSessionToken_1`;
+
+-- UPDATE dbMetadata SET value = '190' WHERE name = 'schema-patch-level';

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

@@ -1,3 +1,3 @@
 {
-  "level": 190
+  "level": 191
 }

packages/fxa-auth-server/lib/db.spec.ts

@@ -19,6 +19,7 @@ jest.mock('fxa-shared/db/models/auth', () => ({
   },
   SessionToken: {
     create: jest.fn().mockResolvedValue(null),
+    createVerified: jest.fn().mockResolvedValue(null),
     delete: jest.fn().mockResolvedValue(null),
     findByUid: jest.fn().mockResolvedValue([]),
   },
@@ -385,6 +386,35 @@ describe('redis enabled, token-pruning enabled:', () => {
     );
   });
 
+  it('should call RawSessionToken.createVerified (not create) and set verificationMethod/verifiedAt in db.createPasskeyVerifiedSessionToken', async () => {
+    const metrics = {
+      increment: jest.fn(),
+    };
+    db.metrics = metrics;
+
+    const result = await db.createPasskeyVerifiedSessionToken({
+      uid: 'wibble',
+    });
+    expect(redis.pruneSessionTokens).toHaveBeenCalledTimes(1);
+    expect(redis.pruneSessionTokens).toHaveBeenCalledWith(
+      'wibble',
+      expect.anything()
+    );
+    // Uses createVerified (the passkey proc), not the plain create
+    expect(models.SessionToken.createVerified.mock.calls.length).toBe(1);
+    // Proc receives verificationMethod=5 (passkey) and a numeric verifiedAt
+    const procArg = models.SessionToken.createVerified.mock.calls[0][0];
+    expect(procArg.verificationMethod).toBe(5);
+    expect(typeof procArg.verifiedAt).toBe('number');
+    // Returned token reflects the same values
+    expect(result.verificationMethod).toBe(5);
+    expect(typeof result.verifiedAt).toBe('number');
+    // Metrics tagged with method: passkey
+    expect(metrics.increment).toHaveBeenCalledWith('db.sessionToken.created', {
+      method: 'passkey',
+    });
+  });
+
   describe('mock db.pruneSessionTokens:', () => {
     beforeEach(() => {
       db.pruneSessionTokens = jest.fn(() => Promise.resolve());

packages/fxa-auth-server/lib/db.ts

@@ -33,6 +33,7 @@ import { Container } from 'typedi';
 import random, { base32 } from './crypto/random';
 import { AppError as error } from '@fxa/accounts/errors';
 import {
+  verificationMethodToNumber,
   verificationMethodToString,
   VerificationMethod,
 } from 'fxa-shared/db/models/auth/session-token';
@@ -180,6 +181,34 @@ export const createDB = (
       return sessionToken;
     }
 
+    async createPasskeyVerifiedSessionToken(authToken: any) {
+      const { uid } = authToken;
+
+      log.trace('DB.createPasskeyVerifiedSessionToken', { uid });
+
+      const verifiedAt = Date.now();
+      const sessionToken = await SessionToken.create({
+        ...authToken,
+        mustVerify: false,
+        tokenVerificationId: null,
+        verificationMethod: verificationMethodToNumber('passkey'),
+        verifiedAt,
+      });
+
+      const { id } = sessionToken;
+
+      // Ensure there are no clashes with zombie tokens left behind in Redis
+      try {
+        await this.deleteSessionTokenFromRedis(uid, id);
+      } catch (unusedErr) {
+        // Ignore errors deleting the token.
+      }
+      await RawSessionToken.createVerified(sessionToken);
+
+      this.metrics?.increment('db.sessionToken.created', { method: 'passkey' });
+      return sessionToken;
+    }
+
     async createKeyFetchToken(authToken: any) {
       log.trace('DB.createKeyFetchToken', { uid: authToken && authToken.uid });
       const keyFetchToken = await KeyFetchToken.create(authToken);

packages/fxa-auth-server/lib/routes/passkeys.spec.ts

@@ -110,11 +110,9 @@ describe('passkeys routes', () => {
         emailVerified: true,
         verifierSetAt: 1234567890,
       }),
-      createSessionToken: jest
+      createPasskeyVerifiedSessionToken: jest
         .fn()
         .mockResolvedValue({ id: 'new-session-token-id' }),
-      verifyTokensWithMethod: jest.fn().mockResolvedValue(undefined),
-      deleteSessionToken: jest.fn().mockResolvedValue(undefined),
       securityEvent: jest.fn().mockResolvedValue(undefined),
     };
 
@@ -833,35 +831,22 @@ describe('passkeys routes', () => {
       );
     });
 
-    it('creates a pre-verified session token with correct options', async () => {
+    it('creates a verified session token with correct options', async () => {
       await handler.createPasskeySessionToken(mockAccount, mockRequest as any);
 
-      expect(db.createSessionToken).toHaveBeenCalledWith(
-        expect.objectContaining({
-          uid: UID,
-          email: TEST_EMAIL,
-          emailCode: 'emailcode123',
-          emailVerified: true,
-          verifierSetAt: 1234567890,
-          mustVerify: false,
-          tokenVerificationId: null,
-          uaBrowser: 'Firefox',
-          uaBrowserVersion: '124.0',
-          uaOS: 'macOS',
-          uaOSVersion: '14.0',
-        })
-      );
-    });
-
-    // TODO(FXA-13444): once the atomic stored procedure lands, this test
-    // should be updated to assert the single new DB call instead.
-    it('stamps the token with the passkey verification method', async () => {
-      await handler.createPasskeySessionToken(mockAccount, mockRequest as any);
-
-      expect(db.verifyTokensWithMethod).toHaveBeenCalledWith(
-        'new-session-token-id',
-        'passkey'
-      );
+      expect(db.createPasskeyVerifiedSessionToken).toHaveBeenCalledWith({
+        uid: UID,
+        email: TEST_EMAIL,
+        emailCode: 'emailcode123',
+        emailVerified: true,
+        verifierSetAt: 1234567890,
+        uaBrowser: 'Firefox',
+        uaBrowserVersion: '124.0',
+        uaOS: 'macOS',
+        uaOSVersion: '14.0',
+        uaDeviceType: null,
+        uaFormFactor: null,
+      });
     });
 
     it('returns the created session token and emits success metric', async () => {
@@ -876,31 +861,13 @@ describe('passkeys routes', () => {
       );
     });
 
-    it('propagates errors from createSessionToken', async () => {
+    it('propagates errors from createPasskeyVerifiedSessionToken', async () => {
       const dbError = new Error('DB unavailable');
-      db.createSessionToken.mockRejectedValue(dbError);
+      db.createPasskeyVerifiedSessionToken.mockRejectedValue(dbError);
 
       await expect(
         handler.createPasskeySessionToken(mockAccount, mockRequest as any)
       ).rejects.toThrow('DB unavailable');
     });
-
-    // TODO(FXA-13444): remove this test once the atomic stored procedure lands.
-    it('deletes the token, emits failure metric, and rethrows if verifyTokensWithMethod fails', async () => {
-      const dbError = new Error('DB unavailable');
-      db.verifyTokensWithMethod.mockRejectedValue(dbError);
-
-      await expect(
-        handler.createPasskeySessionToken(mockAccount, mockRequest as any)
-      ).rejects.toThrow('DB unavailable');
-
-      expect(db.deleteSessionToken).toHaveBeenCalledWith({
-        id: 'new-session-token-id',
-        uid: UID,
-      });
-      expect(statsd.increment).toHaveBeenCalledWith(
-        'passkeys.createSessionToken.failure'
-      );
-    });
   });
 });

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

@@ -42,29 +42,20 @@ interface DB {
     emailVerified: boolean;
     verifierSetAt: number;
   }>;
-  /** Creates a new session token and persists it. */
-  createSessionToken(options: {
+  /** Creates a passkey session token, pre-verified as AAL2 at creation time. */
+  createPasskeyVerifiedSessionToken(options: {
     uid: string;
     email: string;
     emailCode: string;
     emailVerified: boolean;
     verifierSetAt: number;
-    mustVerify: boolean;
-    tokenVerificationId: string | null;
     uaBrowser?: string;
     uaBrowserVersion?: string;
     uaOS?: string;
     uaOSVersion?: string;
     uaDeviceType?: string;
     uaFormFactor?: string;
   }): Promise<{ id: string }>;
-  /** Sets the verification method on a session token. */
-  verifyTokensWithMethod(
-    tokenId: string,
-    method: string | number
-  ): Promise<void>;
-  /** Deletes a session token. Used for cleanup on partial failure. */
-  deleteSessionToken(token: { id: string; uid: string }): Promise<void>;
   /** Records a security event in the audit log. */
   securityEvent: (arg: any) => Promise<void>;
 }
@@ -373,12 +364,10 @@ export class PasskeyHandler {
    * Creates a passkey-verified session token for the authenticated account.
    *
    * No `tokenVerificationId` is set — the passkey assertion is itself AAL2, so
-   * no follow-up email challenge is needed. After creation,
-   * `verifyTokensWithMethod` stamps `verificationMethod = 5` (passkey) on the
-   * row; the token's AMR becomes `{pwd, webauthn}` → AAL2.
-   *
-   * TODO(FXA-13444): this is a temporary two-step implementation. The create
-   * and stamp will be replaced by a single atomic stored procedure.
+   * no follow-up email challenge is needed. The token is written in a single
+   * MySQL INSERT with `verificationMethod = 5` (passkey) and `verifiedAt`
+   * pre-stamped, so no AAL1 window exists. The token's AMR becomes
+   * `{pwd, webauthn}` → AAL2.
    */
   async createPasskeySessionToken(
     account: {
@@ -390,14 +379,12 @@ export class PasskeyHandler {
     },
     request: AuthRequest
   ) {
-    const sessionToken = await this.db.createSessionToken({
+    const sessionToken = await this.db.createPasskeyVerifiedSessionToken({
       uid: account.uid,
       email: account.email,
       emailCode: account.emailCode,
       emailVerified: account.emailVerified,
       verifierSetAt: account.verifierSetAt,
-      mustVerify: false,
-      tokenVerificationId: null,
       uaBrowser: request.app.ua.browser,
       uaBrowserVersion: request.app.ua.browserVersion,
       uaOS: request.app.ua.os,
@@ -406,29 +393,6 @@ export class PasskeyHandler {
       uaFormFactor: request.app.ua.formFactor,
     });
 
-    try {
-      await this.db.verifyTokensWithMethod(sessionToken.id, 'passkey');
-    } catch (err) {
-      // If stamping the verification method fails, delete the token rather than
-      // leaving an orphan session at AAL1. If cleanup also fails, log and report
-      // it but always re-throw the original error.
-      // TODO(FXA-13444): remove this entire catch block once the atomic procedure lands.
-      try {
-        await this.db.deleteSessionToken({
-          id: sessionToken.id,
-          uid: account.uid,
-        });
-      } catch (cleanupErr) {
-        this.log.error('passkeys.createPasskeySessionToken.deleteOrphan', {
-          err: cleanupErr,
-          tokenId: sessionToken.id,
-        });
-        reportSentryError(cleanupErr, request);
-      }
-      this.statsd.increment('passkeys.createSessionToken.failure');
-      throw err;
-    }
-
     this.statsd.increment('passkeys.createSessionToken.success');
     return sessionToken;
   }

packages/fxa-shared/db/models/auth/base-auth.ts

@@ -24,6 +24,7 @@ export enum Proc {
   CreateRecoveryKey = 'createRecoveryKey_4',
   CreateSecurityEvent = 'createSecurityEvent_5',
   CreateSessionToken = 'createSessionToken_10',
+  CreateVerifiedSessionToken = 'createVerifiedSessionToken_1',
   CreateSigninCode = 'createSigninCode_2',
   CreateTotpToken = 'createTotpToken_1',
   CreateUnblockCode = 'createUnblockCode_1',