Skip to content

Commit 2f9775b

Browse files
committed
feat(passkeys): atomic passkey session token creation with verificationMethod pre-stamped
Because: * Passkey authentication is a single-gesture ceremony that inherently proves both possession and user presence, so the resulting session token should be fully verified (AAL2) from the moment of creation — unlike email/TOTP flows that require a separate verification step after the session exists. This commit: * Adds DB migration patch introducing createVerifiedSessionToken_1, a stored procedure that INSERTs the session token with verificationMethod and verifiedAt already set in one operation, eliminating any AAL1 window * Adds revert patch * Wires up RawSessionToken.createVerified (fxa-shared) and db.createPasskeyVerifiedSessionToken (fxa-auth-server) backed by the new procedure; the auth-server method name makes the passkey-specific use explicit, while RawSessionToken.createVerified stays generic for future pre-verified flows * Replaces the two-step createSessionToken + verifyTokensWithMethod call in PasskeyHandler with a single db.createPasskeyVerifiedSessionToken call, removing the orphan-cleanup try/catch * Updates passkey session token tests to reflect the atomic operation Closes #FXA-13444
1 parent ac46003 commit 2f9775b

9 files changed

Lines changed: 210 additions & 94 deletions

File tree

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
SET NAMES utf8mb4 COLLATE utf8mb4_bin;
2+
3+
CALL assertPatchLevel('190');
4+
5+
CREATE PROCEDURE `createVerifiedSessionToken_1` (
6+
IN `tokenId` BINARY(32),
7+
IN `tokenData` BINARY(32),
8+
IN `uid` BINARY(16),
9+
IN `createdAt` BIGINT UNSIGNED,
10+
IN `uaBrowser` VARCHAR(255),
11+
IN `uaBrowserVersion` VARCHAR(255),
12+
IN `uaOS` VARCHAR(255),
13+
IN `uaOSVersion` VARCHAR(255),
14+
IN `uaDeviceType` VARCHAR(255),
15+
IN `uaFormFactor` VARCHAR(255),
16+
IN `mustVerify` BOOLEAN,
17+
IN `providerId` TINYINT,
18+
IN `verificationMethod` INT,
19+
IN `verifiedAt` BIGINT UNSIGNED
20+
)
21+
BEGIN
22+
DECLARE EXIT HANDLER FOR SQLEXCEPTION
23+
BEGIN
24+
ROLLBACK;
25+
RESIGNAL;
26+
END;
27+
28+
START TRANSACTION;
29+
30+
INSERT INTO sessionTokens(
31+
tokenId,
32+
tokenData,
33+
uid,
34+
createdAt,
35+
uaBrowser,
36+
uaBrowserVersion,
37+
uaOS,
38+
uaOSVersion,
39+
uaDeviceType,
40+
uaFormFactor,
41+
lastAccessTime,
42+
mustVerify,
43+
providerId,
44+
verificationMethod,
45+
verifiedAt
46+
)
47+
VALUES(
48+
tokenId,
49+
tokenData,
50+
uid,
51+
createdAt,
52+
uaBrowser,
53+
uaBrowserVersion,
54+
uaOS,
55+
uaOSVersion,
56+
uaDeviceType,
57+
uaFormFactor,
58+
createdAt,
59+
mustVerify,
60+
providerId,
61+
verificationMethod,
62+
verifiedAt
63+
);
64+
65+
COMMIT;
66+
END;
67+
68+
UPDATE dbMetadata SET value = '191' WHERE name = 'schema-patch-level';
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-- SET NAMES utf8mb4 COLLATE utf8mb4_bin;
2+
3+
-- DROP PROCEDURE `createVerifiedSessionToken_1`;
4+
5+
-- UPDATE dbMetadata SET value = '190' WHERE name = 'schema-patch-level';
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
{
2-
"level": 190
2+
"level": 191
33
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ jest.mock('fxa-shared/db/models/auth', () => ({
1919
},
2020
SessionToken: {
2121
create: jest.fn().mockResolvedValue(null),
22+
createVerified: jest.fn().mockResolvedValue(null),
2223
delete: jest.fn().mockResolvedValue(null),
2324
findByUid: jest.fn().mockResolvedValue([]),
2425
},
@@ -385,6 +386,35 @@ describe('redis enabled, token-pruning enabled:', () => {
385386
);
386387
});
387388

389+
it('should call RawSessionToken.createVerified (not create) and set verificationMethod/verifiedAt in db.createPasskeyVerifiedSessionToken', async () => {
390+
const metrics = {
391+
increment: jest.fn(),
392+
};
393+
db.metrics = metrics;
394+
395+
const result = await db.createPasskeyVerifiedSessionToken({
396+
uid: 'wibble',
397+
});
398+
expect(redis.pruneSessionTokens).toHaveBeenCalledTimes(1);
399+
expect(redis.pruneSessionTokens).toHaveBeenCalledWith(
400+
'wibble',
401+
expect.anything()
402+
);
403+
// Uses createVerified (the passkey proc), not the plain create
404+
expect(models.SessionToken.createVerified.mock.calls.length).toBe(1);
405+
// Proc receives verificationMethod=5 (passkey) and a numeric verifiedAt
406+
const procArg = models.SessionToken.createVerified.mock.calls[0][0];
407+
expect(procArg.verificationMethod).toBe(5);
408+
expect(typeof procArg.verifiedAt).toBe('number');
409+
// Returned token reflects the same values
410+
expect(result.verificationMethod).toBe(5);
411+
expect(typeof result.verifiedAt).toBe('number');
412+
// Metrics tagged with method: passkey
413+
expect(metrics.increment).toHaveBeenCalledWith('db.sessionToken.created', {
414+
method: 'passkey',
415+
});
416+
});
417+
388418
describe('mock db.pruneSessionTokens:', () => {
389419
beforeEach(() => {
390420
db.pruneSessionTokens = jest.fn(() => Promise.resolve());

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { Container } from 'typedi';
3333
import random, { base32 } from './crypto/random';
3434
import { AppError as error } from '@fxa/accounts/errors';
3535
import {
36+
verificationMethodToNumber,
3637
verificationMethodToString,
3738
VerificationMethod,
3839
} from 'fxa-shared/db/models/auth/session-token';
@@ -180,6 +181,34 @@ export const createDB = (
180181
return sessionToken;
181182
}
182183

184+
async createPasskeyVerifiedSessionToken(authToken: any) {
185+
const { uid } = authToken;
186+
187+
log.trace('DB.createPasskeyVerifiedSessionToken', { uid });
188+
189+
const verifiedAt = Date.now();
190+
const sessionToken = await SessionToken.create({
191+
...authToken,
192+
mustVerify: false,
193+
tokenVerificationId: null,
194+
verificationMethod: verificationMethodToNumber('passkey'),
195+
verifiedAt,
196+
});
197+
198+
const { id } = sessionToken;
199+
200+
// Ensure there are no clashes with zombie tokens left behind in Redis
201+
try {
202+
await this.deleteSessionTokenFromRedis(uid, id);
203+
} catch (unusedErr) {
204+
// Ignore errors deleting the token.
205+
}
206+
await RawSessionToken.createVerified(sessionToken);
207+
208+
this.metrics?.increment('db.sessionToken.created', { method: 'passkey' });
209+
return sessionToken;
210+
}
211+
183212
async createKeyFetchToken(authToken: any) {
184213
log.trace('DB.createKeyFetchToken', { uid: authToken && authToken.uid });
185214
const keyFetchToken = await KeyFetchToken.create(authToken);

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

Lines changed: 17 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,9 @@ describe('passkeys routes', () => {
110110
emailVerified: true,
111111
verifierSetAt: 1234567890,
112112
}),
113-
createSessionToken: jest
113+
createPasskeyVerifiedSessionToken: jest
114114
.fn()
115115
.mockResolvedValue({ id: 'new-session-token-id' }),
116-
verifyTokensWithMethod: jest.fn().mockResolvedValue(undefined),
117-
deleteSessionToken: jest.fn().mockResolvedValue(undefined),
118116
securityEvent: jest.fn().mockResolvedValue(undefined),
119117
};
120118

@@ -833,35 +831,22 @@ describe('passkeys routes', () => {
833831
);
834832
});
835833

836-
it('creates a pre-verified session token with correct options', async () => {
834+
it('creates a verified session token with correct options', async () => {
837835
await handler.createPasskeySessionToken(mockAccount, mockRequest as any);
838836

839-
expect(db.createSessionToken).toHaveBeenCalledWith(
840-
expect.objectContaining({
841-
uid: UID,
842-
email: TEST_EMAIL,
843-
emailCode: 'emailcode123',
844-
emailVerified: true,
845-
verifierSetAt: 1234567890,
846-
mustVerify: false,
847-
tokenVerificationId: null,
848-
uaBrowser: 'Firefox',
849-
uaBrowserVersion: '124.0',
850-
uaOS: 'macOS',
851-
uaOSVersion: '14.0',
852-
})
853-
);
854-
});
855-
856-
// TODO(FXA-13444): once the atomic stored procedure lands, this test
857-
// should be updated to assert the single new DB call instead.
858-
it('stamps the token with the passkey verification method', async () => {
859-
await handler.createPasskeySessionToken(mockAccount, mockRequest as any);
860-
861-
expect(db.verifyTokensWithMethod).toHaveBeenCalledWith(
862-
'new-session-token-id',
863-
'passkey'
864-
);
837+
expect(db.createPasskeyVerifiedSessionToken).toHaveBeenCalledWith({
838+
uid: UID,
839+
email: TEST_EMAIL,
840+
emailCode: 'emailcode123',
841+
emailVerified: true,
842+
verifierSetAt: 1234567890,
843+
uaBrowser: 'Firefox',
844+
uaBrowserVersion: '124.0',
845+
uaOS: 'macOS',
846+
uaOSVersion: '14.0',
847+
uaDeviceType: null,
848+
uaFormFactor: null,
849+
});
865850
});
866851

867852
it('returns the created session token and emits success metric', async () => {
@@ -876,31 +861,13 @@ describe('passkeys routes', () => {
876861
);
877862
});
878863

879-
it('propagates errors from createSessionToken', async () => {
864+
it('propagates errors from createPasskeyVerifiedSessionToken', async () => {
880865
const dbError = new Error('DB unavailable');
881-
db.createSessionToken.mockRejectedValue(dbError);
866+
db.createPasskeyVerifiedSessionToken.mockRejectedValue(dbError);
882867

883868
await expect(
884869
handler.createPasskeySessionToken(mockAccount, mockRequest as any)
885870
).rejects.toThrow('DB unavailable');
886871
});
887-
888-
// TODO(FXA-13444): remove this test once the atomic stored procedure lands.
889-
it('deletes the token, emits failure metric, and rethrows if verifyTokensWithMethod fails', async () => {
890-
const dbError = new Error('DB unavailable');
891-
db.verifyTokensWithMethod.mockRejectedValue(dbError);
892-
893-
await expect(
894-
handler.createPasskeySessionToken(mockAccount, mockRequest as any)
895-
).rejects.toThrow('DB unavailable');
896-
897-
expect(db.deleteSessionToken).toHaveBeenCalledWith({
898-
id: 'new-session-token-id',
899-
uid: UID,
900-
});
901-
expect(statsd.increment).toHaveBeenCalledWith(
902-
'passkeys.createSessionToken.failure'
903-
);
904-
});
905872
});
906873
});

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

Lines changed: 7 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -42,29 +42,20 @@ interface DB {
4242
emailVerified: boolean;
4343
verifierSetAt: number;
4444
}>;
45-
/** Creates a new session token and persists it. */
46-
createSessionToken(options: {
45+
/** Creates a passkey session token, pre-verified as AAL2 at creation time. */
46+
createPasskeyVerifiedSessionToken(options: {
4747
uid: string;
4848
email: string;
4949
emailCode: string;
5050
emailVerified: boolean;
5151
verifierSetAt: number;
52-
mustVerify: boolean;
53-
tokenVerificationId: string | null;
5452
uaBrowser?: string;
5553
uaBrowserVersion?: string;
5654
uaOS?: string;
5755
uaOSVersion?: string;
5856
uaDeviceType?: string;
5957
uaFormFactor?: string;
6058
}): Promise<{ id: string }>;
61-
/** Sets the verification method on a session token. */
62-
verifyTokensWithMethod(
63-
tokenId: string,
64-
method: string | number
65-
): Promise<void>;
66-
/** Deletes a session token. Used for cleanup on partial failure. */
67-
deleteSessionToken(token: { id: string; uid: string }): Promise<void>;
6859
/** Records a security event in the audit log. */
6960
securityEvent: (arg: any) => Promise<void>;
7061
}
@@ -373,12 +364,10 @@ export class PasskeyHandler {
373364
* Creates a passkey-verified session token for the authenticated account.
374365
*
375366
* No `tokenVerificationId` is set — the passkey assertion is itself AAL2, so
376-
* no follow-up email challenge is needed. After creation,
377-
* `verifyTokensWithMethod` stamps `verificationMethod = 5` (passkey) on the
378-
* row; the token's AMR becomes `{pwd, webauthn}` → AAL2.
379-
*
380-
* TODO(FXA-13444): this is a temporary two-step implementation. The create
381-
* and stamp will be replaced by a single atomic stored procedure.
367+
* no follow-up email challenge is needed. The token is written in a single
368+
* MySQL INSERT with `verificationMethod = 5` (passkey) and `verifiedAt`
369+
* pre-stamped, so no AAL1 window exists. The token's AMR becomes
370+
* `{pwd, webauthn}` → AAL2.
382371
*/
383372
async createPasskeySessionToken(
384373
account: {
@@ -390,14 +379,12 @@ export class PasskeyHandler {
390379
},
391380
request: AuthRequest
392381
) {
393-
const sessionToken = await this.db.createSessionToken({
382+
const sessionToken = await this.db.createPasskeyVerifiedSessionToken({
394383
uid: account.uid,
395384
email: account.email,
396385
emailCode: account.emailCode,
397386
emailVerified: account.emailVerified,
398387
verifierSetAt: account.verifierSetAt,
399-
mustVerify: false,
400-
tokenVerificationId: null,
401388
uaBrowser: request.app.ua.browser,
402389
uaBrowserVersion: request.app.ua.browserVersion,
403390
uaOS: request.app.ua.os,
@@ -406,29 +393,6 @@ export class PasskeyHandler {
406393
uaFormFactor: request.app.ua.formFactor,
407394
});
408395

409-
try {
410-
await this.db.verifyTokensWithMethod(sessionToken.id, 'passkey');
411-
} catch (err) {
412-
// If stamping the verification method fails, delete the token rather than
413-
// leaving an orphan session at AAL1. If cleanup also fails, log and report
414-
// it but always re-throw the original error.
415-
// TODO(FXA-13444): remove this entire catch block once the atomic procedure lands.
416-
try {
417-
await this.db.deleteSessionToken({
418-
id: sessionToken.id,
419-
uid: account.uid,
420-
});
421-
} catch (cleanupErr) {
422-
this.log.error('passkeys.createPasskeySessionToken.deleteOrphan', {
423-
err: cleanupErr,
424-
tokenId: sessionToken.id,
425-
});
426-
reportSentryError(cleanupErr, request);
427-
}
428-
this.statsd.increment('passkeys.createSessionToken.failure');
429-
throw err;
430-
}
431-
432396
this.statsd.increment('passkeys.createSessionToken.success');
433397
return sessionToken;
434398
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export enum Proc {
2424
CreateRecoveryKey = 'createRecoveryKey_4',
2525
CreateSecurityEvent = 'createSecurityEvent_5',
2626
CreateSessionToken = 'createSessionToken_10',
27+
CreateVerifiedSessionToken = 'createVerifiedSessionToken_1',
2728
CreateSigninCode = 'createSigninCode_2',
2829
CreateTotpToken = 'createTotpToken_1',
2930
CreateUnblockCode = 'createUnblockCode_1',

0 commit comments

Comments
 (0)