feat: Run mfa.beforeSetup hook before enabling MFA (#11116)

This commit is contained in:
Ricardo Espinoza
2024-10-17 19:47:10 +02:00
committed by GitHub
parent 44f95160fb
commit 25c1c3218c
9 changed files with 103 additions and 14 deletions

View File

@@ -5,9 +5,12 @@ import { AuthService } from '@/auth/auth.service';
import config from '@/config';
import type { User } from '@/databases/entities/user';
import { AuthUserRepository } from '@/databases/repositories/auth-user.repository';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { ExternalHooks } from '@/external-hooks';
import { TOTPService } from '@/mfa/totp.service';
import { mockInstance } from '@test/mocking';
import { createUser, createUserWithMfaEnabled } from '../shared/db/users';
import { createOwner, createUser, createUserWithMfaEnabled } from '../shared/db/users';
import { randomValidPassword, uniqueId } from '../shared/random';
import * as testDb from '../shared/test-db';
import * as utils from '../shared/utils';
@@ -16,6 +19,8 @@ jest.mock('@/telemetry');
let owner: User;
const externalHooks = mockInstance(ExternalHooks);
const testServer = utils.setupTestServer({
endpointGroups: ['mfa', 'auth', 'me', 'passwordReset'],
});
@@ -23,7 +28,9 @@ const testServer = utils.setupTestServer({
beforeEach(async () => {
await testDb.truncate(['User']);
owner = await createUser({ role: 'global:owner' });
owner = await createOwner();
externalHooks.run.mockReset();
config.set('userManagement.disabled', false);
});
@@ -131,6 +138,27 @@ describe('Enable MFA setup', () => {
expect(user.mfaRecoveryCodes).toBeDefined();
expect(user.mfaSecret).toBeDefined();
});
test('POST /enable should not enable MFA if pre check fails', async () => {
// This test is to make sure owners verify their email before enabling MFA in cloud
const response = await testServer.authAgentFor(owner).get('/mfa/qr').expect(200);
const { secret } = response.body.data;
const token = new TOTPService().generateTOTP(secret);
await testServer.authAgentFor(owner).post('/mfa/verify').send({ token }).expect(200);
externalHooks.run.mockRejectedValue(new BadRequestError('Error message'));
await testServer.authAgentFor(owner).post('/mfa/enable').send({ token }).expect(400);
const user = await Container.get(AuthUserRepository).findOneOrFail({
where: {},
});
expect(user.mfaEnabled).toBe(false);
});
});
});
@@ -232,6 +260,28 @@ describe('Change password with MFA enabled', () => {
});
});
describe('MFA before enable checks', () => {
test('POST /can-enable should throw error if mfa.beforeSetup returns error', async () => {
externalHooks.run.mockRejectedValue(new BadRequestError('Error message'));
await testServer.authAgentFor(owner).post('/mfa/can-enable').expect(400);
expect(externalHooks.run).toHaveBeenCalledWith('mfa.beforeSetup', [
expect.objectContaining(owner),
]);
});
test('POST /can-enable should not throw error if mfa.beforeSetup does not exist', async () => {
externalHooks.run.mockResolvedValue(undefined);
await testServer.authAgentFor(owner).post('/mfa/can-enable').expect(200);
expect(externalHooks.run).toHaveBeenCalledWith('mfa.beforeSetup', [
expect.objectContaining(owner),
]);
});
});
describe('Login', () => {
test('POST /login with email/password should succeed when mfa is disabled', async () => {
const password = randomString(8);