From cdde937d7fce130d67e331bd968cb2c4734902d4 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Fri, 11 Sep 2026 10:40:27 +0200 Subject: [PATCH] fix(backend): reject a replayed TOTP code within its validity window (#1389) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(backend): reject a replayed TOTP code within its validity window verifyTotp() was stateless — otplib's window:1 tolerance meant the same 6-digit code could complete two independent logins inside its ~90s validity window. Track each admin's last-consumed step and reject a code that doesn't advance past it. * fix(backend): make the TOTP replay-tracking persist atomic verifyTotpEncryptedStep() read two_factor_last_used_step, then a plain UPDATE wrote the new step with no conditional guard — two concurrent requests carrying the same captured code could both pass the check before either UPDATE landed. The persist is now a conditional UPDATE (only advances the step, checked via affected-row count), so a losing concurrent request is correctly treated as a replay. --------- Co-authored-by: Paul Nothaft --- backend/__tests__/routes/adminMfa.test.js | 213 ++++++++++++++++++ backend/__tests__/services/mfaService.test.js | 46 ++++ .../core/213_admin_mfa_last_used_step.js | 29 +++ backend/scripts/reset-admin-mfa.js | 1 + backend/src/routes/adminAuth.js | 29 ++- backend/src/routes/auth.js | 19 +- backend/src/services/mfaService.js | 102 ++++++++- 7 files changed, 426 insertions(+), 13 deletions(-) create mode 100644 backend/migrations/core/213_admin_mfa_last_used_step.js diff --git a/backend/__tests__/routes/adminMfa.test.js b/backend/__tests__/routes/adminMfa.test.js index 0cfaabb3..3ab8871c 100644 --- a/backend/__tests__/routes/adminMfa.test.js +++ b/backend/__tests__/routes/adminMfa.test.js @@ -38,6 +38,7 @@ const { authenticator } = require('otplib'); const { bootCrmDb, mintAdminToken, buildRouteApp, } = require('../integration/helpers/crmDb'); +const mfaService = require('../../src/services/mfaService'); jest.setTimeout(120000); @@ -235,6 +236,138 @@ describe('MFA disable — /api/admin/auth/mfa/disable', () => { expect(row.two_factor_secret).toBeNull(); expect(row.two_factor_recovery_codes).toBeNull(); }); + + // Concurrency regression: a plain UPDATE with no conditional guard let two + // requests carrying the same captured code both read the same + // two_factor_last_used_step and both persist, defeating replay protection. + // The guarded UPDATE (mfaService.persistTotpStep) makes only the first + // writer's affected-row count > 0; the loser must be rejected. + it('two concurrent disable requests with the SAME captured code: only one succeeds', async () => { + const admin = await seedAdmin(); + const { secret, token } = await enroll(admin.id); + const code = authenticator.generate(secret); + + const [r1, r2] = await Promise.all([ + request(adminApp) + .post('/api/admin/auth/mfa/disable') + .set('Authorization', `Bearer ${token}`) + .send({ code }), + request(adminApp) + .post('/api/admin/auth/mfa/disable') + .set('Authorization', `Bearer ${token}`) + .send({ code }), + ]); + + expect([r1.status, r2.status].sort()).toEqual([200, 400]); + + const status = await request(adminApp) + .get('/api/admin/auth/mfa/status') + .set('Authorization', `Bearer ${token}`); + expect(status.body.enabled).toBe(false); + }); +}); + +describe('MFA regenerate recovery codes — /api/admin/auth/mfa/recovery-codes', () => { + it('a valid TOTP regenerates the recovery codes and persists the step', async () => { + const admin = await seedAdmin(); + const { secret, token } = await enroll(admin.id); + + const res = await request(adminApp) + .post('/api/admin/auth/mfa/recovery-codes') + .set('Authorization', `Bearer ${token}`) + .send({ code: authenticator.generate(secret) }); + + expect(res.status).toBe(200); + expect(res.body.recoveryCodes).toHaveLength(10); + }); + + it('a wrong code is rejected (400)', async () => { + const admin = await seedAdmin(); + const { secret, token } = await enroll(admin.id); + const valid = authenticator.generate(secret); + const wrong = valid === '000000' ? '111111' : '000000'; + + const res = await request(adminApp) + .post('/api/admin/auth/mfa/recovery-codes') + .set('Authorization', `Bearer ${token}`) + .send({ code: wrong }); + expect(res.status).toBe(400); + }); + + // Concurrency regression (see the disable test above for the mechanism): + // this is the endpoint called out as the worst lost-update case, since it + // both rotates the recovery codes and (previously) persisted the step in + // one unconditional UPDATE. + it('two concurrent regenerations with the SAME captured code: only one succeeds', async () => { + const admin = await seedAdmin(); + const { secret, token } = await enroll(admin.id); + const code = authenticator.generate(secret); + + const [r1, r2] = await Promise.all([ + request(adminApp) + .post('/api/admin/auth/mfa/recovery-codes') + .set('Authorization', `Bearer ${token}`) + .send({ code }), + request(adminApp) + .post('/api/admin/auth/mfa/recovery-codes') + .set('Authorization', `Bearer ${token}`) + .send({ code }), + ]); + + expect([r1.status, r2.status].sort()).toEqual([200, 400]); + const winner = r1.status === 200 ? r1 : r2; + expect(winner.body.recoveryCodes).toHaveLength(10); + + const row = await db('admin_users').where({ id: admin.id }).first(); + expect(row.two_factor_last_used_step).not.toBeNull(); + }); +}); + +describe('mfaService.persistTotpStep — atomic replay-tracking persist', () => { + // Deterministic simulation of the race: two "concurrent" requests that + // read the SAME two_factor_last_used_step and computed the SAME totpStep + // from the same captured code. Calling persistTotpStep twice in a row with + // that identical totpStep reproduces exactly the DB-level outcome of a + // true race, without relying on event-loop timing. + it('the second writer with the same totpStep affects 0 rows and is rejected', async () => { + const admin = await seedAdmin(); + const { secret } = await enroll(admin.id); + const row = await db('admin_users').where({ id: admin.id }).first(); + const code = authenticator.generate(secret); + const totpStep = mfaService.verifyTotpEncryptedStep(code, row.two_factor_secret, null); + expect(totpStep).toEqual(expect.any(Number)); + + const first = await mfaService.persistTotpStep(db, admin.id, totpStep, { updated_at: new Date() }); + expect(first).toBe(true); + + // The row's two_factor_last_used_step has now already advanced to + // totpStep by the time this "losing" write runs — the guard condition + // (whereNull OR < totpStep) is false, so 0 rows are affected. + const second = await mfaService.persistTotpStep(db, admin.id, totpStep, { updated_at: new Date() }); + expect(second).toBe(false); + + const after = await db('admin_users').where({ id: admin.id }).first(); + expect(Number(after.two_factor_last_used_step)).toBe(totpStep); + }); + + it('succeeds when the new step advances past the current one', async () => { + const admin = await seedAdmin(); + const { secret } = await enroll(admin.id); + const row = await db('admin_users').where({ id: admin.id }).first(); + const code = authenticator.generate(secret); + const totpStep = mfaService.verifyTotpEncryptedStep(code, row.two_factor_secret, null); + + const ok = await mfaService.persistTotpStep(db, admin.id, totpStep, {}); + expect(ok).toBe(true); + + const nextStepAuthenticator = authenticator.clone({ epoch: Date.now() + 30000 }); + const nextCode = nextStepAuthenticator.generate(secret); + const nextStep = mfaService.verifyTotpEncryptedStep(nextCode, row.two_factor_secret, totpStep); + expect(nextStep).toBeGreaterThan(totpStep); + + const advanced = await mfaService.persistTotpStep(db, admin.id, nextStep, {}); + expect(advanced).toBe(true); + }); }); describe('Admin login challenge — /api/auth/admin/login[/mfa]', () => { @@ -284,6 +417,86 @@ describe('Admin login challenge — /api/auth/admin/login[/mfa]', () => { expect(res.body.user.id).toBe(admin.id); }); + // GHSA-qcwx-r25m-j869: verifyTotp() was stateless, so otplib's window:1 + // tolerance let the same 6-digit code complete two independent logins + // within its ~90s validity window. mfaService now tracks each admin's + // last-consumed TOTP step and rejects a code that doesn't advance past it. + it('#GHSA-qcwx-r25m-j869 — a TOTP code cannot be replayed into a second login', async () => { + const admin = await seedAdmin(); + const { secret } = await enroll(admin.id); + const code = authenticator.generate(secret); + + // First use of the code completes a login. + const c1 = await request(authApp) + .post('/api/auth/admin/login') + .send({ username: admin.username, password: admin.password }); + const first = await request(authApp) + .post('/api/auth/admin/login/mfa') + .send({ mfaToken: c1.body.mfaToken, code }); + expect(first.status).toBe(200); + expect(first.body.user).toBeDefined(); + + // Replaying the SAME code for an independent second login must fail, + // even though otplib's window:1 tolerance still considers it valid. + const c2 = await request(authApp) + .post('/api/auth/admin/login') + .send({ username: admin.username, password: admin.password }); + const replay = await request(authApp) + .post('/api/auth/admin/login/mfa') + .send({ mfaToken: c2.body.mfaToken, code }); + expect(replay.status).toBe(401); + expect(replay.body.code).toBe('MFA_INVALID'); + expect(replay.body.user).toBeUndefined(); + + // A freshly generated code for the NEXT TOTP step is not a replay and + // succeeds. Generated via a cloned authenticator with a future epoch + // rather than mocking Date.now(), so mfaService's own step computation + // (real Date.now()) still lands the match one step ahead. + const nextStepAuthenticator = authenticator.clone({ epoch: Date.now() + 30000 }); + const nextCode = nextStepAuthenticator.generate(secret); + const c3 = await request(authApp) + .post('/api/auth/admin/login') + .send({ username: admin.username, password: admin.password }); + const third = await request(authApp) + .post('/api/auth/admin/login/mfa') + .send({ mfaToken: c3.body.mfaToken, code: nextCode }); + expect(third.status).toBe(200); + expect(third.body.user).toBeDefined(); + expect(third.body.user.id).toBe(admin.id); + }); + + // Concurrency regression: verifyTotpEncryptedStep()'s "does this advance" + // check was read against a snapshot taken earlier in the request, then a + // PLAIN update persisted the step — two concurrent requests carrying the + // SAME captured code could both pass the check and both complete a login + // before either write landed. The persist is now a conditional UPDATE + // (mfaService.persistTotpStep), so only the first writer's affected-row + // count is > 0 and the other is correctly treated as a replay. + it('two concurrent login/mfa requests with the SAME captured code: only one completes', async () => { + const admin = await seedAdmin(); + const { secret } = await enroll(admin.id); + const code = authenticator.generate(secret); + + const c1 = await request(authApp) + .post('/api/auth/admin/login') + .send({ username: admin.username, password: admin.password }); + const c2 = await request(authApp) + .post('/api/auth/admin/login') + .send({ username: admin.username, password: admin.password }); + + const [r1, r2] = await Promise.all([ + request(authApp).post('/api/auth/admin/login/mfa').send({ mfaToken: c1.body.mfaToken, code }), + request(authApp).post('/api/auth/admin/login/mfa').send({ mfaToken: c2.body.mfaToken, code }), + ]); + + expect([r1.status, r2.status].sort()).toEqual([200, 401]); + const winner = r1.status === 200 ? r1 : r2; + const loser = r1.status === 200 ? r2 : r1; + expect(winner.body.user).toBeDefined(); + expect(loser.body.user).toBeUndefined(); + expect(loser.body.code).toBe('MFA_INVALID'); + }); + it('login/mfa with a wrong code is 401 MFA_INVALID', async () => { const admin = await seedAdmin(); const { secret } = await enroll(admin.id); diff --git a/backend/__tests__/services/mfaService.test.js b/backend/__tests__/services/mfaService.test.js index a6e5b36b..883f33ac 100644 --- a/backend/__tests__/services/mfaService.test.js +++ b/backend/__tests__/services/mfaService.test.js @@ -92,6 +92,52 @@ describe('mfaService — TOTP verification', () => { }); }); +describe('mfaService — replay protection (GHSA-qcwx-r25m-j869)', () => { + it('verifyTotp accepts a code once and rejects the same code as a replay', () => { + const secret = mfaService.generateSecret(); + const code = authenticator.generate(secret); + + // First use: no lastUsedStep yet, so it's accepted. + expect(mfaService.verifyTotp(code, secret)).toBe(true); + + // Simulate persisting the matched step and replaying the same code: the + // matched step must strictly advance past lastUsedStep, so this fails. + const step = mfaService.currentTotpStep(); + expect(mfaService.verifyTotp(code, secret, step)).toBe(false); + // A lastUsedStep the code hasn't caught up to yet also rejects it. + expect(mfaService.verifyTotp(code, secret, step + 1)).toBe(false); + }); + + it('verifyTotpEncryptedStep returns the matched step on success and null on replay', () => { + const secret = mfaService.generateSecret(); + const stored = mfaService.encryptSecret(secret); + const code = authenticator.generate(secret); + + const step = mfaService.verifyTotpEncryptedStep(code, stored, null); + expect(step).toEqual(expect.any(Number)); + expect(step).toBeGreaterThan(0); + + // Replaying the same code against the just-persisted step is rejected. + expect(mfaService.verifyTotpEncryptedStep(code, stored, step)).toBeNull(); + }); + + it('a freshly generated code for the next TOTP step is accepted after a replay is rejected', () => { + const secret = mfaService.generateSecret(); + const code = authenticator.generate(secret); + const step = mfaService.verifyTotpEncryptedStep(code, mfaService.encryptSecret(secret), null) + || mfaService.currentTotpStep(); + + // Same-step replay: rejected. + expect(mfaService.verifyTotp(code, secret, step)).toBe(false); + + // A code minted for the next step (via a cloned authenticator with a + // future epoch, not by mocking Date.now()) advances past last_used_step. + const nextStepAuthenticator = authenticator.clone({ epoch: Date.now() + 30000 }); + const nextCode = nextStepAuthenticator.generate(secret); + expect(mfaService.verifyTotp(nextCode, secret, step)).toBe(true); + }); +}); + describe('mfaService — otpauth URI / QR', () => { it('builds an otpauth:// URI containing issuer, account and secret', () => { const secret = mfaService.generateSecret(); diff --git a/backend/migrations/core/213_admin_mfa_last_used_step.js b/backend/migrations/core/213_admin_mfa_last_used_step.js new file mode 100644 index 00000000..f213c521 --- /dev/null +++ b/backend/migrations/core/213_admin_mfa_last_used_step.js @@ -0,0 +1,29 @@ +/** + * Migration 213: TOTP replay protection for admin MFA (GHSA-qcwx-r25m-j869). + * + * verifyTotp()/verifyTotpEncrypted() were stateless: otplib's window:1 + * tolerance means a captured 6-digit code stays valid across several real + * time-steps (~90s), so the same code could complete two independent admin + * logins. `two_factor_last_used_step` tracks, per admin, the absolute TOTP + * time-step (Math.floor(Date.now() / 30000)) that their last successfully + * consumed code matched; mfaService now rejects a code whose matched step + * doesn't advance past it. + * + * Additive and idempotent: only adds a column, guarded by hasColumn, so it + * is safe to re-run and touches no existing data. + */ +exports.up = async function (knex) { + if (!(await knex.schema.hasColumn('admin_users', 'two_factor_last_used_step'))) { + await knex.schema.alterTable('admin_users', (t) => { + t.integer('two_factor_last_used_step').nullable(); + }); + } +}; + +exports.down = async function (knex) { + if (await knex.schema.hasColumn('admin_users', 'two_factor_last_used_step')) { + await knex.schema.alterTable('admin_users', (t) => { + t.dropColumn('two_factor_last_used_step'); + }); + } +}; diff --git a/backend/scripts/reset-admin-mfa.js b/backend/scripts/reset-admin-mfa.js index ce4985cb..8f3d115e 100644 --- a/backend/scripts/reset-admin-mfa.js +++ b/backend/scripts/reset-admin-mfa.js @@ -36,6 +36,7 @@ const MFA_CLEAR = { two_factor_secret: null, two_factor_recovery_codes: null, two_factor_enrolled_at: null, + two_factor_last_used_step: null, updated_at: new Date(), }; diff --git a/backend/src/routes/adminAuth.js b/backend/src/routes/adminAuth.js index b2abc2c5..b611ae4f 100644 --- a/backend/src/routes/adminAuth.js +++ b/backend/src/routes/adminAuth.js @@ -264,6 +264,11 @@ router.post('/mfa/setup', adminAuth, handleAsync(async (req, res) => { // Complete enrollment: verify a code against the provisional secret, enable // MFA, and return one-time recovery codes (shown exactly once). +// +// No replay tracking here: this confirms an already-authenticated session +// still holds the authenticator (no new session is granted), and starting +// the last-used-step counter here would reject the very next login if it +// lands in the same 30s TOTP step as this call. router.post('/mfa/enable', [ adminAuth, body('code').notEmpty().withMessage('Verification code is required') @@ -314,7 +319,17 @@ router.post('/mfa/disable', [ throw new ValidationError('Two-factor authentication is not enabled'); } - const totpOk = mfaService.verifyTotpEncrypted(req.body.code, admin.two_factor_secret); + // Persist the matched step atomically right here (see mfaService.persistTotpStep): + // two concurrent requests carrying the same captured code can't both read the + // same last-used step and both win — only the first writer's UPDATE affects a + // row, so a losing concurrent request is correctly treated as invalid below. + const totpStep = mfaService.verifyTotpEncryptedStep( + req.body.code, admin.two_factor_secret, admin.two_factor_last_used_step + ); + let totpOk = false; + if (totpStep !== null) { + totpOk = await mfaService.persistTotpStep(db, admin.id, totpStep, { updated_at: new Date() }); + } let recoveryOk = false; if (!totpOk) { const stored = mfaService.parseRecoveryCodes(admin.two_factor_recovery_codes); @@ -329,6 +344,7 @@ router.post('/mfa/disable', [ two_factor_secret: null, two_factor_recovery_codes: null, two_factor_enrolled_at: null, + two_factor_last_used_step: null, updated_at: new Date() }); @@ -353,7 +369,16 @@ router.post('/mfa/recovery-codes', [ if (!isMfaEnabled(admin)) { throw new ValidationError('Two-factor authentication is not enabled'); } - if (!mfaService.verifyTotpEncrypted(req.body.code, admin.two_factor_secret)) { + // Persist the matched step atomically right here (see mfaService.persistTotpStep): + // two concurrent requests carrying the same captured code can't both read the + // same last-used step and both win — only the first writer's UPDATE affects a + // row, so a losing concurrent request is correctly treated as invalid below. + const totpStep = mfaService.verifyTotpEncryptedStep( + req.body.code, admin.two_factor_secret, admin.two_factor_last_used_step + ); + const totpOk = totpStep !== null + && await mfaService.persistTotpStep(db, admin.id, totpStep, { updated_at: new Date() }); + if (!totpOk) { throw new ValidationError('Invalid verification code'); } diff --git a/backend/src/routes/auth.js b/backend/src/routes/auth.js index 6e5c4e17..d140f7d5 100644 --- a/backend/src/routes/auth.js +++ b/backend/src/routes/auth.js @@ -298,8 +298,22 @@ router.post('/admin/login/mfa', [ return res.status(401).json({ error: getGenericAuthError() }); } - // TOTP first, then a one-time recovery code. - let ok = mfaService.verifyTotpEncrypted(code, admin.two_factor_secret); + // TOTP first, then a one-time recovery code. verifyTotpEncryptedStep also + // enforces replay protection (GHSA-qcwx-r25m-j869): a code whose matched + // step doesn't advance past this admin's two_factor_last_used_step is + // rejected, so the same code can't complete two logins. The step is + // persisted atomically (persistTotpStep) right here, immediately after a + // match, so two concurrent requests carrying the same captured code + // can't both read the same last-used step and both win — only the first + // writer's UPDATE affects a row; the loser falls through and is treated + // as a replay below. + const totpStep = mfaService.verifyTotpEncryptedStep( + code, admin.two_factor_secret, admin.two_factor_last_used_step + ); + let ok = false; + if (totpStep !== null) { + ok = await mfaService.persistTotpStep(db, admin.id, totpStep, { updated_at: new Date() }); + } let usedRecovery = false; let remainingHashes = null; if (!ok) { @@ -328,6 +342,7 @@ router.post('/admin/login/mfa', [ { type: 'admin', id: admin.id, name: admin.username } ); } + // else: the TOTP step was already persisted atomically above. await logActivity('admin_mfa_login', { admin_id: admin.id, method: usedRecovery ? 'recovery_code' : 'totp' }, diff --git a/backend/src/services/mfaService.js b/backend/src/services/mfaService.js index 422a91a5..53a7e229 100644 --- a/backend/src/services/mfaService.js +++ b/backend/src/services/mfaService.js @@ -3,7 +3,11 @@ * * Responsibilities: * - generate/verify TOTP secrets (otplib, standard SHA1/6-digit/30s so - * Google Authenticator / Authy / 1Password all work); + * Google Authenticator / Authy / 1Password all work), with replay + * protection: verifyTotpEncryptedStep() rejects a code whose matched + * time-step doesn't advance past the admin's last consumed one + * (GHSA-qcwx-r25m-j869 — otplib's window:1 tolerance alone lets a + * captured code stay valid across several time-steps, ~90s); * - encrypt the secret at rest (AES-256-GCM) so a DB leak alone doesn't * yield working authenticator seeds; * - generate/verify one-time recovery codes, hashed (bcrypt) and single-use; @@ -67,25 +71,102 @@ function decryptSecret(stored) { return pt.toString('utf8'); } -/** Verify a 6-digit TOTP code against the (plaintext) secret. */ -function verifyTotp(code, plainSecret) { - if (!code || !plainSecret) return false; +/** Absolute TOTP time-step for "now" (Math.floor(Date.now() / 30000)). */ +function currentTotpStep() { + return Math.floor(Date.now() / 30000); +} + +/** + * Core TOTP check. Returns the matched absolute time-step (always a + * positive, truthy integer) when `code` is valid for `plainSecret`; + * otherwise `null`. + * + * When `lastUsedStep` is given, a code whose matched step doesn't advance + * past it is treated as invalid — replay protection. Without this, otplib's + * window:1 tolerance lets a captured code stay valid across several real + * time-steps (~90s), so the same code could complete two independent admin + * logins (GHSA-qcwx-r25m-j869). + */ +function matchTotpStep(code, plainSecret, lastUsedStep) { + if (!code || !plainSecret) return null; try { - return authenticator.verify({ token: String(code).replace(/\s+/g, ''), secret: plainSecret }); + const token = String(code).replace(/\s+/g, ''); + const delta = authenticator.checkDelta(token, plainSecret); + if (typeof delta !== 'number') return null; + const step = currentTotpStep() + delta; + if (typeof lastUsedStep === 'number' && step <= lastUsedStep) return null; + return step; + } catch { + return null; + } +} + +/** + * Verify a 6-digit TOTP code against the (plaintext) secret. Pass + * `lastUsedStep` (the admin's previously-consumed step) to also enforce + * replay protection — see matchTotpStep(). + */ +function verifyTotp(code, plainSecret, lastUsedStep) { + return matchTotpStep(code, plainSecret, lastUsedStep) !== null; +} + +/** Verify a code against a STORED (encrypted) secret. */ +function verifyTotpEncrypted(code, storedSecret, lastUsedStep) { + try { + return verifyTotp(code, decryptSecret(storedSecret), lastUsedStep); } catch { return false; } } -/** Verify a code against a STORED (encrypted) secret. */ -function verifyTotpEncrypted(code, storedSecret) { +/** + * Like verifyTotpEncrypted(), but returns the matched step (or `null` when + * the code is invalid/replayed) instead of a boolean, so a caller that + * grants a session or a sensitive action can persist it as the admin's new + * `two_factor_last_used_step`. + */ +function verifyTotpEncryptedStep(code, storedSecret, lastUsedStep) { try { - return verifyTotp(code, decryptSecret(storedSecret)); + return matchTotpStep(code, decryptSecret(storedSecret), lastUsedStep); } catch { - return false; + return null; } } +/** + * Persist a newly-matched TOTP step, but only if it still advances + * `two_factor_last_used_step` at write time (`db('admin_users').where('id', + * adminId).whereNull(...).orWhere(...).update(...)`). + * + * matchTotpStep()'s "does this advance past lastUsedStep" check is read + * against a snapshot taken earlier in the request. Two concurrent requests + * carrying the same captured code can both read the same lastUsedStep and + * both pass that check before either write lands — a plain, unconditional + * UPDATE would let both persist, defeating replay protection. Guarding the + * UPDATE with the same condition and checking the affected-row count makes + * only the first writer succeed; a losing concurrent request gets 0 affected + * rows and must be treated as a replay by the caller. + * + * @param {object} db - knex instance + * @param {number} adminId + * @param {number} totpStep - matched step from verifyTotpEncryptedStep() + * @param {object} [extraFields] - additional columns to set in the same UPDATE + * @returns {Promise} true if this call won the race and persisted + */ +async function persistTotpStep(db, adminId, totpStep, extraFields = {}) { + const affected = await db('admin_users') + .where('id', adminId) + .where(function () { + this.whereNull('two_factor_last_used_step') + .orWhere('two_factor_last_used_step', '<', totpStep); + }) + .update({ + two_factor_last_used_step: totpStep, + ...extraFields + }); + return affected > 0; +} + /** otpauth:// URI for an authenticator app. */ function buildOtpauthUri(accountName, plainSecret) { return authenticator.keyuri(accountName, ISSUER, plainSecret); @@ -169,8 +250,11 @@ module.exports = { generateSecret, encryptSecret, decryptSecret, + currentTotpStep, verifyTotp, verifyTotpEncrypted, + verifyTotpEncryptedStep, + persistTotpStep, buildOtpauthUri, buildQrDataUrl, generateRecoveryCodes,