fix: Allow disabling MFA with recovery codes (#12014)

Co-authored-by: Tomi Turtiainen <10324676+tomi@users.noreply.github.com>
This commit is contained in:
Ricardo Espinoza
2024-12-04 10:02:06 -05:00
committed by GitHub
parent 2b6a72f128
commit 95d56fee8d
12 changed files with 98 additions and 25 deletions

View File

@@ -86,13 +86,24 @@ export class MFAController {
@Post('/disable', { rateLimit: true })
async disableMFA(req: MFA.Disable) {
const { id: userId } = req.user;
const { mfaCode = null } = req.body;
if (typeof mfaCode !== 'string' || !mfaCode) {
throw new BadRequestError('Token is required to disable MFA feature');
const { mfaCode, mfaRecoveryCode } = req.body;
const mfaCodeDefined = mfaCode && typeof mfaCode === 'string';
const mfaRecoveryCodeDefined = mfaRecoveryCode && typeof mfaRecoveryCode === 'string';
if (!mfaCodeDefined === !mfaRecoveryCodeDefined) {
throw new BadRequestError(
'Either MFA code or recovery code is required to disable MFA feature',
);
}
await this.mfaService.disableMfa(userId, mfaCode);
if (mfaCodeDefined) {
await this.mfaService.disableMfaWithMfaCode(userId, mfaCode);
} else if (mfaRecoveryCodeDefined) {
await this.mfaService.disableMfaWithRecoveryCode(userId, mfaRecoveryCode);
}
}
@Post('/verify', { rateLimit: true })

View File

@@ -0,0 +1,7 @@
import { ForbiddenError } from './forbidden.error';
export class InvalidMfaRecoveryCodeError extends ForbiddenError {
constructor(hint?: string) {
super('Invalid MFA recovery code', hint);
}
}

View File

@@ -4,6 +4,7 @@ import { v4 as uuid } from 'uuid';
import { AuthUserRepository } from '@/databases/repositories/auth-user.repository';
import { InvalidMfaCodeError } from '@/errors/response-errors/invalid-mfa-code.error';
import { InvalidMfaRecoveryCodeError } from '@/errors/response-errors/invalid-mfa-recovery-code-error';
import { TOTPService } from './totp.service';
@@ -85,12 +86,27 @@ export class MfaService {
return await this.authUserRepository.save(user);
}
async disableMfa(userId: string, mfaCode: string) {
async disableMfaWithMfaCode(userId: string, mfaCode: string) {
const isValidToken = await this.validateMfa(userId, mfaCode, undefined);
if (!isValidToken) {
throw new InvalidMfaCodeError();
}
await this.disableMfaForUser(userId);
}
async disableMfaWithRecoveryCode(userId: string, recoveryCode: string) {
const isValidToken = await this.validateMfa(userId, undefined, recoveryCode);
if (!isValidToken) {
throw new InvalidMfaRecoveryCodeError();
}
await this.disableMfaForUser(userId);
}
private async disableMfaForUser(userId: string) {
await this.authUserRepository.update(userId, {
mfaEnabled: false,
mfaSecret: null,

View File

@@ -318,7 +318,7 @@ export type LoginRequest = AuthlessRequest<
export declare namespace MFA {
type Verify = AuthenticatedRequest<{}, {}, { mfaCode: string }, {}>;
type Activate = AuthenticatedRequest<{}, {}, { mfaCode: string }, {}>;
type Disable = AuthenticatedRequest<{}, {}, { mfaCode: string }, {}>;
type Disable = AuthenticatedRequest<{}, {}, { mfaCode?: string; mfaRecoveryCode?: string }, {}>;
type Config = AuthenticatedRequest<{}, {}, { login: { enabled: boolean } }, {}>;
type ValidateRecoveryCode = AuthenticatedRequest<
{},

View File

@@ -184,7 +184,19 @@ describe('Disable MFA setup', () => {
expect(dbUser.mfaRecoveryCodes.length).toBe(0);
});
test('POST /disable should fail if invalid mfaCode is given', async () => {
test('POST /disable should fail if invalid MFA recovery code is given', async () => {
const { user } = await createUserWithMfaEnabled();
await testServer
.authAgentFor(user)
.post('/mfa/disable')
.send({
mfaRecoveryCode: 'invalid token',
})
.expect(403);
});
test('POST /disable should fail if invalid MFA code is given', async () => {
const { user } = await createUserWithMfaEnabled();
await testServer
@@ -195,6 +207,12 @@ describe('Disable MFA setup', () => {
})
.expect(403);
});
test('POST /disable should fail if neither MFA code nor recovery code is sent', async () => {
const { user } = await createUserWithMfaEnabled();
await testServer.authAgentFor(user).post('/mfa/disable').send({ anotherParam: '' }).expect(400);
});
});
describe('Change password with MFA enabled', () => {