Skip to content

Commit 0e6664d

Browse files
committed
fix(webauthn): give challenges their own store, with expiry and one-time use
Challenges lived in a single users.challenge column shared by registration, login and step-up, so the three flows clobbered each other and a second tab invalidated the first. The column also had no lifetime: the timeout in the credential options is only a hint to the browser, and nothing server side ever enforced it, so a challenge stayed valid until some later flow happened to overwrite it. Challenges now live in webauthn_challenges, keyed by user and flow, with a server enforced five minute life. That is comfortably longer than the sixty second client hint, so a user hunting for a security key is not cut off. They are also spent when verification reads them, before anything else can fail, so no outcome leaves a redeemable challenge behind. That closes the replay finding tracked privately alongside this issue, whose fix belonged in the same change. The per-flow context that used to sit in users.challengeContext travels with the challenge it was issued for. users.challenge and users.challengeContext are no longer read or written. They are left in place so this release can be rolled back, and should be dropped once it has run in production. Migration verified up and down against Postgres 17. Closes #164
1 parent 7ee36cf commit 0e6664d

14 files changed

Lines changed: 605 additions & 47 deletions

File tree

.changeset/brave-hoops-turn.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
'seamless-auth-api': minor
3+
---
4+
5+
Give WebAuthn challenges their own store, with an expiry and one-time use.
6+
7+
Challenges lived in a single `users.challenge` column shared by registration,
8+
login and step-up. Three consequences, all fixed here:
9+
10+
- **Flows clobbered each other.** Starting a login invalidated a registration
11+
already in flight for the same user, and a second tab invalidated the first.
12+
Challenges are now keyed by user and flow, so registration, login and step-up
13+
can be outstanding at once.
14+
- **Nothing expired.** The column had no lifetime, so a challenge stayed valid
15+
until some later flow happened to overwrite it. The `timeout` in the credential
16+
options is only a hint to the browser and was never enforced. Challenges now
17+
expire server side after five minutes, comfortably longer than that hint so no
18+
legitimate ceremony is cut short.
19+
- **A challenge could outlive its ceremony.** It is now spent when verification
20+
reads it, before anything else can fail, so an attempt that fails leaves
21+
nothing redeemable behind.
22+
23+
A magic link completing also spends any half-finished WebAuthn ceremony for that
24+
user, preserving what the old defensive clear did.
25+
26+
`users.challenge` and `users.challengeContext` are no longer read or written.
27+
They are left in place so this release can be rolled back, and should be dropped
28+
in a follow-up once it has run in production.

src/controllers/magicLinks.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { AuthEventService } from '../services/authEventService.js';
1616
import { getLoginPolicy, isLoginMethodEnabled } from '../services/loginPolicyService.js';
1717
import { sendMagicLinkEmail } from '../services/messagingService.js';
1818
import { issueSessionAndRespond } from '../services/sessionIssuance.js';
19+
import { invalidateChallengesForUser } from '../services/webauthnChallengeService.js';
1920
import { AuthenticatedRequest } from '../types/types.js';
2021
import getLogger from '../utils/logger.js';
2122
import { hashDeviceFingerprint, hashSha256 } from '../utils/utils.js';
@@ -253,11 +254,14 @@ export async function pollMagicLinkConfirmation(req: Request, res: Response) {
253254
req,
254255
});
255256

256-
user.challenge = '';
257257
user.verified = true;
258258

259259
await user.save();
260260

261+
// A different route completed the sign-in, so any half-finished WebAuthn
262+
// ceremony for this user should not still be redeemable.
263+
await invalidateChallengesForUser(user.id);
264+
261265
await AuthEventService.log({
262266
userId: user.id,
263267
type: 'registration_success',
@@ -278,7 +282,6 @@ export async function pollMagicLinkConfirmation(req: Request, res: Response) {
278282

279283
await user.update({
280284
lastLogin: new Date(),
281-
challengeContext: null,
282285
});
283286

284287
return;

src/controllers/stepUp.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
recordStepUpVerification,
2323
serializeStepUpStatus,
2424
} from '../services/stepUpService.js';
25+
import { consumeChallenge, issueChallenge } from '../services/webauthnChallengeService.js';
2526
import { AuthenticatedRequest } from '../types/types.js';
2627
import getLogger from '../utils/logger.js';
2728

@@ -111,7 +112,9 @@ export const startWebAuthnStepUp = async (req: Request, res: Response) => {
111112
extensions: buildPrfAuthenticationExtensions(prf),
112113
});
113114

114-
await user.update({
115+
await issueChallenge({
116+
userId: user.id,
117+
purpose: 'step_up',
115118
challenge: options.challenge,
116119
});
117120

@@ -162,7 +165,11 @@ export const finishWebAuthnStepUp = async (req: Request, res: Response) => {
162165
return res.status(400).json({ error: 'prf_output_not_allowed' });
163166
}
164167

165-
if (!user.challenge || typeof assertionId !== 'string') {
168+
// Consumed before the credential lookup, so every exit below leaves the
169+
// challenge spent rather than live.
170+
const issued = await consumeChallenge({ userId: user.id, purpose: 'step_up' });
171+
172+
if (!issued || typeof assertionId !== 'string') {
166173
await AuthEventService.log({
167174
userId: user.id,
168175
type: 'step_up_failed',
@@ -186,8 +193,7 @@ export const finishWebAuthnStepUp = async (req: Request, res: Response) => {
186193
return res.status(401).json({ error: 'step_up_failed' });
187194
}
188195

189-
const expectedChallenge = user.challenge;
190-
await user.update({ challenge: null });
196+
const expectedChallenge = issued.challenge;
191197

192198
try {
193199
const { origins, rpid } = await getSystemConfig();

src/controllers/webauthn.ts

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,16 @@ import type { WebAuthnAuthenticatorAttachment } from '../schemas/webauthn.reques
2828
import { AuthEventService } from '../services/authEventService.js';
2929
import { rejectIfUserLocked } from '../services/lockoutPolicyService.js';
3030
import { issueSessionAndRespond } from '../services/sessionIssuance.js';
31+
import { consumeChallenge, issueChallenge } from '../services/webauthnChallengeService.js';
3132
import { AuthenticatedRequest } from '../types/types.js';
3233
import getLogger from '../utils/logger.js';
3334

3435
const logger = getLogger('webauthn');
35-
function getRegistrationChallengeContext(user: User) {
36-
const webauthnRegistration = user.challengeContext?.webauthnRegistration;
37-
38-
if (typeof webauthnRegistration !== 'object' || webauthnRegistration === null) {
36+
function getRegistrationChallengeContext(context: Record<string, unknown> | null | undefined) {
37+
if (!context) {
3938
return { prfRequested: false, requirePrf: false };
4039
}
4140

42-
const context = webauthnRegistration as Record<string, unknown>;
43-
4441
return {
4542
prfRequested: context.prfRequested === true,
4643
requirePrf: context.requirePrf === true,
@@ -152,15 +149,11 @@ const registerWebAuthn = async (req: Request, res: Response) => {
152149
extensions: buildPrfRegistrationExtensions(prfRequested),
153150
});
154151

155-
await verifiedUser.update({
152+
await issueChallenge({
153+
userId: verifiedUser.id,
154+
purpose: 'registration',
156155
challenge: options.challenge,
157-
challengeContext: {
158-
...(verifiedUser.challengeContext ?? {}),
159-
webauthnRegistration: {
160-
prfRequested,
161-
requirePrf,
162-
},
163-
},
156+
context: { prfRequested, requirePrf },
164157
});
165158

166159
logger.info('Generated registration options for user');
@@ -231,7 +224,11 @@ const verifyWebAuthnRegistration = async (req: Request, res: Response) => {
231224
return res.status(403).json({ error: 'Not allowed' });
232225
}
233226

234-
const expectedChallenge = user.challenge;
227+
// Consumed before verification, so the challenge is spent however this
228+
// attempt turns out and a failure cannot leave one live to replay against.
229+
const issued = await consumeChallenge({ userId: user.id, purpose: 'registration' });
230+
const expectedChallenge = issued?.challenge;
231+
235232
if (!expectedChallenge) {
236233
logger.error('Unexpected user challegnge supplied.');
237234
await AuthEventService.log({
@@ -278,7 +275,7 @@ const verifyWebAuthnRegistration = async (req: Request, res: Response) => {
278275
}
279276

280277
const { credential, credentialBackedUp, credentialDeviceType } = registrationInfo;
281-
const challengeContext = getRegistrationChallengeContext(user);
278+
const challengeContext = getRegistrationChallengeContext(issued?.context);
282279
const prfCapable =
283280
getRegistrationPrfCapable(attestationResponse) || metadata.prfCapable === true;
284281

@@ -312,8 +309,6 @@ const verifyWebAuthnRegistration = async (req: Request, res: Response) => {
312309
});
313310

314311
await user.update({
315-
challenge: null,
316-
challengeContext: null,
317312
lastLogin: new Date(),
318313
verified: true,
319314
});
@@ -403,7 +398,9 @@ const generateWebAuthn = async (req: Request, res: Response) => {
403398
extensions: buildPrfAuthenticationExtensions(prf),
404399
});
405400

406-
await user.update({
401+
await issueChallenge({
402+
userId: user.id,
403+
purpose: 'authentication',
407404
challenge: options.challenge,
408405
});
409406

@@ -466,7 +463,13 @@ const verifyWebAuthn = async (req: Request, res: Response) => {
466463
return res.status(403).json({ error: 'Not allowed' });
467464
}
468465

469-
if (!user || !user.challenge) {
466+
// Consumed before anything else can fail, so this path cannot leave a live
467+
// challenge behind for an assertion to be replayed against.
468+
const issued = user
469+
? await consumeChallenge({ userId: user.id, purpose: 'authentication' })
470+
: null;
471+
472+
if (!user || !issued) {
470473
logger.error('User or user challenge missing');
471474
await AuthEventService.log({
472475
userId: user.id,
@@ -495,7 +498,7 @@ const verifyWebAuthn = async (req: Request, res: Response) => {
495498
return res.status(401).json({ error: 'Authentication failed.' });
496499
}
497500

498-
const expectedChallenge = user.challenge;
501+
const expectedChallenge = issued.challenge;
499502
let verification;
500503

501504
try {
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
'use strict';
2+
3+
/**
4+
* Moves WebAuthn challenges off the single `users.challenge` column.
5+
*
6+
* That column was shared by registration, login and step-up, so two flows for
7+
* one user clobbered each other, and it had no expiry: a challenge stayed valid
8+
* until some later flow happened to overwrite it.
9+
*
10+
* @type {import('sequelize-cli').Migration}
11+
*/
12+
module.exports = {
13+
async up(queryInterface, Sequelize) {
14+
await queryInterface.createTable('webauthn_challenges', {
15+
id: {
16+
type: Sequelize.UUID,
17+
defaultValue: Sequelize.literal('gen_random_uuid()'),
18+
primaryKey: true,
19+
},
20+
user_id: {
21+
type: Sequelize.UUID,
22+
allowNull: false,
23+
references: { model: 'users', key: 'id' },
24+
onDelete: 'CASCADE',
25+
},
26+
purpose: {
27+
type: Sequelize.STRING,
28+
allowNull: false,
29+
},
30+
challenge: {
31+
type: Sequelize.STRING,
32+
allowNull: false,
33+
},
34+
// Flow state that used to live in users.challengeContext, for example
35+
// whether the registration asked for PRF.
36+
context: {
37+
type: Sequelize.JSONB,
38+
allowNull: true,
39+
},
40+
expires_at: {
41+
type: Sequelize.DATE,
42+
allowNull: false,
43+
},
44+
consumed_at: {
45+
type: Sequelize.DATE,
46+
allowNull: true,
47+
},
48+
created_at: {
49+
type: Sequelize.DATE,
50+
allowNull: false,
51+
defaultValue: Sequelize.fn('NOW'),
52+
},
53+
updated_at: {
54+
type: Sequelize.DATE,
55+
allowNull: false,
56+
defaultValue: Sequelize.fn('NOW'),
57+
},
58+
});
59+
60+
// Every lookup is "the live challenge for this user and this flow".
61+
await queryInterface.addIndex('webauthn_challenges', ['user_id', 'purpose'], {
62+
name: 'webauthn_challenges_user_purpose_idx',
63+
});
64+
65+
// Supports reaping expired rows without scanning the table.
66+
await queryInterface.addIndex('webauthn_challenges', ['expires_at'], {
67+
name: 'webauthn_challenges_expires_at_idx',
68+
});
69+
},
70+
71+
async down(queryInterface) {
72+
await queryInterface.dropTable('webauthn_challenges');
73+
},
74+
};

src/models/webauthnChallenges.ts

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* Copyright © 2026 Fells Code, LLC
3+
* Licensed under the GNU Affero General Public License v3.0
4+
* See LICENSE file in the project root for full license information
5+
*/
6+
7+
import { DataTypes, Model, Optional, Sequelize } from 'sequelize';
8+
9+
export type WebAuthnChallengePurpose = 'registration' | 'authentication' | 'step_up';
10+
11+
export interface WebAuthnChallengeAttributes {
12+
id: string;
13+
userId: string;
14+
purpose: WebAuthnChallengePurpose;
15+
challenge: string;
16+
context?: Record<string, unknown> | null;
17+
expiresAt: Date;
18+
consumedAt?: Date | null;
19+
createdAt?: Date;
20+
updatedAt?: Date;
21+
}
22+
23+
type WebAuthnChallengeCreationAttributes = Optional<
24+
WebAuthnChallengeAttributes,
25+
'id' | 'context' | 'consumedAt' | 'createdAt' | 'updatedAt'
26+
>;
27+
28+
export class WebAuthnChallenge
29+
extends Model<WebAuthnChallengeAttributes, WebAuthnChallengeCreationAttributes>
30+
implements WebAuthnChallengeAttributes
31+
{
32+
declare id: string;
33+
declare userId: string;
34+
declare purpose: WebAuthnChallengePurpose;
35+
declare challenge: string;
36+
declare context: Record<string, unknown> | null;
37+
declare expiresAt: Date;
38+
declare consumedAt: Date | null;
39+
declare readonly createdAt: Date;
40+
declare readonly updatedAt: Date;
41+
42+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
43+
static associate(models: any) {
44+
WebAuthnChallenge.belongsTo(models.User, {
45+
foreignKey: 'userId',
46+
onDelete: 'CASCADE',
47+
as: 'user',
48+
});
49+
}
50+
}
51+
52+
const initializeWebAuthnChallengeModel = (sequelize: Sequelize) => {
53+
WebAuthnChallenge.init(
54+
{
55+
id: {
56+
type: DataTypes.UUID,
57+
defaultValue: DataTypes.UUIDV4,
58+
primaryKey: true,
59+
},
60+
userId: {
61+
type: DataTypes.UUID,
62+
allowNull: false,
63+
},
64+
purpose: {
65+
type: DataTypes.STRING,
66+
allowNull: false,
67+
},
68+
challenge: {
69+
type: DataTypes.STRING,
70+
allowNull: false,
71+
},
72+
context: {
73+
type: DataTypes.JSONB,
74+
allowNull: true,
75+
},
76+
expiresAt: {
77+
type: DataTypes.DATE,
78+
allowNull: false,
79+
},
80+
consumedAt: {
81+
type: DataTypes.DATE,
82+
allowNull: true,
83+
},
84+
},
85+
{
86+
sequelize,
87+
modelName: 'WebAuthnChallenge',
88+
tableName: 'webauthn_challenges',
89+
underscored: true,
90+
timestamps: true,
91+
},
92+
);
93+
94+
return WebAuthnChallenge;
95+
};
96+
97+
export default initializeWebAuthnChallengeModel;

0 commit comments

Comments
 (0)