diff --git a/packages/cli/src/controllers/mfa.controller.ts b/packages/cli/src/controllers/mfa.controller.ts index f0af103265..694765761c 100644 --- a/packages/cli/src/controllers/mfa.controller.ts +++ b/packages/cli/src/controllers/mfa.controller.ts @@ -1,11 +1,21 @@ import { Get, Post, RestController } from '@/decorators'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { ExternalHooks } from '@/external-hooks'; import { MfaService } from '@/mfa/mfa.service'; import { AuthenticatedRequest, MFA } from '@/requests'; @RestController('/mfa') export class MFAController { - constructor(private mfaService: MfaService) {} + constructor( + private mfaService: MfaService, + private externalHooks: ExternalHooks, + ) {} + + @Post('/can-enable') + async canEnableMFA(req: AuthenticatedRequest) { + await this.externalHooks.run('mfa.beforeSetup', [req.user]); + return; + } @Get('/qr') async getQRCode(req: AuthenticatedRequest) { @@ -52,6 +62,8 @@ export class MFAController { const { token = null } = req.body; const { id, mfaEnabled } = req.user; + await this.externalHooks.run('mfa.beforeSetup', [req.user]); + const { decryptedSecret: secret, decryptedRecoveryCodes: recoveryCodes } = await this.mfaService.getSecretAndRecoveryCodes(id); diff --git a/packages/cli/test/integration/mfa/mfa.api.test.ts b/packages/cli/test/integration/mfa/mfa.api.test.ts index 0062f87e89..3f19632506 100644 --- a/packages/cli/test/integration/mfa/mfa.api.test.ts +++ b/packages/cli/test/integration/mfa/mfa.api.test.ts @@ -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); diff --git a/packages/editor-ui/src/api/cloudPlans.ts b/packages/editor-ui/src/api/cloudPlans.ts index f7daa4b0a1..821c1ce1a3 100644 --- a/packages/editor-ui/src/api/cloudPlans.ts +++ b/packages/editor-ui/src/api/cloudPlans.ts @@ -13,7 +13,7 @@ export async function getCloudUserInfo(context: IRestApiContext): Promise { +export async function sendConfirmationEmail(context: IRestApiContext): Promise { return await post(context.baseUrl, '/cloud/proxy/user/resend-confirmation-email'); } diff --git a/packages/editor-ui/src/api/mfa.ts b/packages/editor-ui/src/api/mfa.ts index 09cfb84df4..0cce31c96d 100644 --- a/packages/editor-ui/src/api/mfa.ts +++ b/packages/editor-ui/src/api/mfa.ts @@ -1,6 +1,10 @@ import type { IRestApiContext } from '@/Interface'; import { makeRestApiRequest } from '@/utils/apiUtils'; +export async function canEnableMFA(context: IRestApiContext) { + return await makeRestApiRequest(context, 'POST', '/mfa/can-enable'); +} + export async function getMfaQR( context: IRestApiContext, ): Promise<{ qrCode: string; secret: string; recoveryCodes: string[] }> { diff --git a/packages/editor-ui/src/components/__tests__/BannersStack.test.ts b/packages/editor-ui/src/components/__tests__/BannersStack.test.ts index 71955e8739..468a20232d 100644 --- a/packages/editor-ui/src/components/__tests__/BannersStack.test.ts +++ b/packages/editor-ui/src/components/__tests__/BannersStack.test.ts @@ -105,7 +105,7 @@ describe('BannerStack', () => { }, }), }); - const confirmEmailSpy = vi.spyOn(useUsersStore(), 'confirmEmail'); + const confirmEmailSpy = vi.spyOn(useUsersStore(), 'sendConfirmationEmail'); getByTestId('confirm-email-button').click(); await waitFor(() => expect(confirmEmailSpy).toHaveBeenCalled()); await waitFor(() => { @@ -125,9 +125,11 @@ describe('BannerStack', () => { }, }), }); - const confirmEmailSpy = vi.spyOn(useUsersStore(), 'confirmEmail').mockImplementation(() => { - throw new Error(ERROR_MESSAGE); - }); + const confirmEmailSpy = vi + .spyOn(useUsersStore(), 'sendConfirmationEmail') + .mockImplementation(() => { + throw new Error(ERROR_MESSAGE); + }); getByTestId('confirm-email-button').click(); await waitFor(() => expect(confirmEmailSpy).toHaveBeenCalled()); await waitFor(() => { diff --git a/packages/editor-ui/src/components/banners/EmailConfirmationBanner.vue b/packages/editor-ui/src/components/banners/EmailConfirmationBanner.vue index 45b3d8c425..72d3f6e923 100644 --- a/packages/editor-ui/src/components/banners/EmailConfirmationBanner.vue +++ b/packages/editor-ui/src/components/banners/EmailConfirmationBanner.vue @@ -14,7 +14,7 @@ const userEmail = computed(() => { async function onConfirmEmailClick() { try { - await useUsersStore().confirmEmail(); + await useUsersStore().sendConfirmationEmail(); toast.showMessage({ type: 'success', title: locale.baseText('banners.confirmEmail.toast.success.heading'), diff --git a/packages/editor-ui/src/plugins/i18n/locales/en.json b/packages/editor-ui/src/plugins/i18n/locales/en.json index 35bc02c799..e975ebb12c 100644 --- a/packages/editor-ui/src/plugins/i18n/locales/en.json +++ b/packages/editor-ui/src/plugins/i18n/locales/en.json @@ -2592,6 +2592,7 @@ "settings.personal.mfa.toast.disabledMfa.title": "Two-factor authentication disabled", "settings.personal.mfa.toast.disabledMfa.message": "You will no longer need your authenticator app when signing in", "settings.personal.mfa.toast.disabledMfa.error.message": "Error disabling two-factor authentication", + "settings.personal.mfa.toast.canEnableMfa.title": "MFA pre-requisite failed", "settings.mfa.toast.noRecoveryCodeLeft.title": "No 2FA recovery codes remaining", "settings.mfa.toast.noRecoveryCodeLeft.message": "You have used all of your recovery codes. Disable then re-enable two-factor authentication to generate new codes. Open settings", "sso.login.divider": "or", diff --git a/packages/editor-ui/src/stores/users.store.ts b/packages/editor-ui/src/stores/users.store.ts index 53d538012c..2091048fd6 100644 --- a/packages/editor-ui/src/stores/users.store.ts +++ b/packages/editor-ui/src/stores/users.store.ts @@ -320,6 +320,10 @@ export const useUsersStore = defineStore(STORES.USERS, () => { return await mfaApi.verifyMfaToken(rootStore.restApiContext, data); }; + const canEnableMFA = async () => { + return await mfaApi.canEnableMFA(rootStore.restApiContext); + }; + const enableMfa = async (data: { token: string }) => { await mfaApi.enableMfa(rootStore.restApiContext, data); if (currentUser.value) { @@ -347,8 +351,8 @@ export const useUsersStore = defineStore(STORES.USERS, () => { } }; - const confirmEmail = async () => { - await cloudApi.confirmEmail(rootStore.restApiContext); + const sendConfirmationEmail = async () => { + await cloudApi.sendConfirmationEmail(rootStore.restApiContext); }; const updateGlobalRole = async ({ id, newRoleName }: UpdateGlobalRolePayload) => { @@ -403,8 +407,9 @@ export const useUsersStore = defineStore(STORES.USERS, () => { verifyMfaToken, enableMfa, disableMfa, + canEnableMFA, fetchUserCloudAccount, - confirmEmail, + sendConfirmationEmail, updateGlobalRole, reset, }; diff --git a/packages/editor-ui/src/views/SettingsPersonalView.vue b/packages/editor-ui/src/views/SettingsPersonalView.vue index af1c92519d..64effcf52f 100644 --- a/packages/editor-ui/src/views/SettingsPersonalView.vue +++ b/packages/editor-ui/src/views/SettingsPersonalView.vue @@ -200,8 +200,23 @@ function openPasswordModal() { uiStore.openModal(CHANGE_PASSWORD_MODAL_KEY); } -function onMfaEnableClick() { - uiStore.openModal(MFA_SETUP_MODAL_KEY); +async function onMfaEnableClick() { + if (!settingsStore.isCloudDeployment || !usersStore.isInstanceOwner) { + uiStore.openModal(MFA_SETUP_MODAL_KEY); + return; + } + + try { + await usersStore.canEnableMFA(); + uiStore.openModal(MFA_SETUP_MODAL_KEY); + } catch (e) { + showToast({ + title: i18n.baseText('settings.personal.mfa.toast.canEnableMfa.title'), + message: e.message, + type: 'error', + }); + await usersStore.sendConfirmationEmail(); + } } async function disableMfa(payload: MfaModalEvents['closed']) {