From 8f64431ff1199660825be3c64eb43566d4f04b9c Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 18 Oct 2026 12:35:37 +0200 Subject: [PATCH] refactor(auth): one helper finds a backup code for login, reset and MFA disable Keeps auth.service.ts inside its size baseline: the recovery-code check the disable path gained is the lookup the password reset already spelled out. --- server/src/nest/auth/auth.helpers.ts | 5 +++++ server/src/nest/auth/auth.service.ts | 17 +++++------------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/server/src/nest/auth/auth.helpers.ts b/server/src/nest/auth/auth.helpers.ts index 0f626280f..43d285cc7 100644 --- a/server/src/nest/auth/auth.helpers.ts +++ b/server/src/nest/auth/auth.helpers.ts @@ -135,3 +135,8 @@ export function generateBackupCodes(count = MFA_BACKUP_CODE_COUNT): string[] { export function parseBackupCodeHashes(raw: string | null | undefined): string[] { return decodeJson(USER_MFA_BACKUP_CODES, raw).filter((v): v is string => typeof v === 'string'); } + +/** Where `plaintext` sits among a user's stored backup-code hashes, or -1 when it is none of them. */ +export function backupCodeIndex(plaintext: string, raw: string | null | undefined): number { + return parseBackupCodeHashes(raw).findIndex((hash) => matchBackupCode(plaintext, hash)); +} diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index dfa0ef570..25dea4449 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -64,6 +64,7 @@ import { generateBackupCodes, hashBackupCodeBcrypt, mask_stored_api_key, + backupCodeIndex, matchBackupCode, parseBackupCodeHashes, stripUserForClient, @@ -873,16 +874,10 @@ export class AuthService { const secret = decryptMfaSecret(user.mfa_secret); const tokenStr = String(code).trim(); const okTotp = authenticator.verify({ token: tokenStr.replace(/\s/g, ''), secret }); - // Recovery codes already work at login. Accept them here too, but only - // after the account password has been verified and while the existing - // rate limit / instance MFA policy allow this operation. - const okRecovery = !okTotp && parseBackupCodeHashes(user.mfa_backup_codes) - .some((hash) => matchBackupCode(tokenStr, hash)); - if (!okTotp && !okRecovery) { + // A backup code proves the second factor as at login; disabling drops them all (#2665). + if (!okTotp && backupCodeIndex(tokenStr, user.mfa_backup_codes) === -1) { throw new DomainError(401, 'Invalid verification code'); } - // Disabling MFA clears the old secret and *all* backup codes atomically. - // A recovery code used here cannot be replayed after re-enrolment. // Every other session ends with the second factor; this one just proved both. await this.uow.transactional(async () => { await this.usersRepo.disableMfa(userId); @@ -1060,10 +1055,8 @@ export class AuthService { const secret = decryptMfaSecret(user.mfa_secret); const okTotp = authenticator.verify({ token: supplied.replace(/\s/g, ''), secret }); if (!okTotp) { - const hashes = parseBackupCodeHashes(user.mfa_backup_codes); - const idx = hashes.findIndex((h) => matchBackupCode(supplied, h)); - if (idx === -1) throw new DomainError(401, 'Invalid MFA code'); - backupCodeConsumedIndex = idx; + backupCodeConsumedIndex = backupCodeIndex(supplied, user.mfa_backup_codes); + if (backupCodeConsumedIndex === -1) throw new DomainError(401, 'Invalid MFA code'); } }