fix: Require mfa code to disable mfa (#10345)

packages/cli/src/Mfa/mfa.service.ts

@@ -3,6 +3,7 @@ import { Service } from 'typedi';
 import { Cipher } from 'n8n-core';
 import { AuthUserRepository } from '@db/repositories/authUser.repository';
 import { TOTPService } from './totp.service';
+import { InvalidMfaCodeError } from '@/errors/response-errors/invalid-mfa-code.error';
 
 @Service()
 export class MfaService {
@@ -82,11 +83,16 @@ export class MfaService {
 		return await this.authUserRepository.save(user);
 	}
 
-	async disableMfa(userId: string) {
+	async disableMfa(userId: string, mfaToken: string) {
-		const user = await this.authUserRepository.findOneByOrFail({ id: userId });
+		const isValidToken = await this.validateMfa(userId, mfaToken, undefined);
-		user.mfaEnabled = false;
+		if (!isValidToken) {
-		user.mfaSecret = null;
+			throw new InvalidMfaCodeError();
-		user.mfaRecoveryCodes = [];
+		}
-		return await this.authUserRepository.save(user);
+
+		await this.authUserRepository.update(userId, {
+			mfaEnabled: false,
+			mfaSecret: null,
+			mfaRecoveryCodes: [],
+		});
 	}
 }

packages/cli/src/controllers/__tests__/me.controller.test.ts

@@ -17,7 +17,7 @@ import { badPasswords } from '@test/testData';
 import { mockInstance } from '@test/mocking';
 import { AuthUserRepository } from '@/databases/repositories/authUser.repository';
 import { MfaService } from '@/Mfa/mfa.service';
-import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
+import { InvalidMfaCodeError } from '@/errors/response-errors/invalid-mfa-code.error';
 
 const browserId = 'test-browser-id';
 
@@ -230,16 +230,14 @@ describe('MeController', () => {
 				);
 			});
 
-			it('should throw ForbiddenError if invalid mfa code is given', async () => {
+			it('should throw InvalidMfaCodeError if invalid mfa code is given', async () => {
 				const req = mock<MeRequest.Password>({
 					user: mock({ password: passwordHash, mfaEnabled: true }),
 					body: { currentPassword: 'old_password', newPassword: 'NewPassword123', mfaCode: '123' },
 				});
 				mockMfaService.validateMfa.mockResolvedValue(false);
 
-				await expect(controller.updatePassword(req, mock())).rejects.toThrowError(
+				await expect(controller.updatePassword(req, mock())).rejects.toThrow(InvalidMfaCodeError);
-					new ForbiddenError('Invalid two-factor code.'),
-				);
 			});
 
 			it('should succeed when mfa code is correct', async () => {

packages/cli/src/controllers/me.controller.ts

@@ -24,7 +24,7 @@ import { UserRepository } from '@/databases/repositories/user.repository';
 import { isApiEnabled } from '@/PublicApi';
 import { EventService } from '@/events/event.service';
 import { MfaService } from '@/Mfa/mfa.service';
-import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
+import { InvalidMfaCodeError } from '@/errors/response-errors/invalid-mfa-code.error';
 
 export const API_KEY_PREFIX = 'n8n_api_';
 
@@ -155,7 +155,7 @@ export class MeController {
 
 			const isMfaTokenValid = await this.mfaService.validateMfa(user.id, mfaCode, undefined);
 			if (!isMfaTokenValid) {
-				throw new ForbiddenError('Invalid two-factor code.');
+				throw new InvalidMfaCodeError();
 			}
 		}
 

packages/cli/src/controllers/mfa.controller.ts

@@ -1,4 +1,4 @@
-import { Delete, Get, Post, RestController } from '@/decorators';
+import { Get, Post, RestController } from '@/decorators';
 import { AuthenticatedRequest, MFA } from '@/requests';
 import { MfaService } from '@/Mfa/mfa.service';
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
@@ -71,11 +71,16 @@ export class MFAController {
 		await this.mfaService.enableMfa(id);
 	}
 
-	@Delete('/disable')
+	@Post('/disable', { rateLimit: true })
-	async disableMFA(req: AuthenticatedRequest) {
+	async disableMFA(req: MFA.Disable) {
-		const { id } = req.user;
+		const { id: userId } = req.user;
+		const { token = null } = req.body;
 
-		await this.mfaService.disableMfa(id);
+		if (typeof token !== 'string' || !token) {
+			throw new BadRequestError('Token is required to disable MFA feature');
+		}
+
+		await this.mfaService.disableMfa(userId, token);
 	}
 
 	@Post('/verify', { rateLimit: true })

packages/cli/src/errors/response-errors/invalid-mfa-code.error.ts

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

packages/cli/src/requests.ts

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

packages/cli/test/integration/mfa/mfa.api.test.ts

@@ -48,7 +48,8 @@ describe('Enable MFA setup', () => {
 				secondCall.body.data.recoveryCodes.join(''),
 			);
 
-			await testServer.authAgentFor(owner).delete('/mfa/disable').expect(200);
+			const token = new TOTPService().generateTOTP(firstCall.body.data.secret);
+			await testServer.authAgentFor(owner).post('/mfa/disable').send({ token }).expect(200);
 
 			const thirdCall = await testServer.authAgentFor(owner).get('/mfa/qr').expect(200);
 
@@ -135,9 +136,16 @@ describe('Enable MFA setup', () => {
 
 describe('Disable MFA setup', () => {
 	test('POST /disable should disable login with MFA', async () => {
-		const { user } = await createUserWithMfaEnabled();
+		const { user, rawSecret } = await createUserWithMfaEnabled();
+		const token = new TOTPService().generateTOTP(rawSecret);
 
-		await testServer.authAgentFor(user).delete('/mfa/disable').expect(200);
+		await testServer
+			.authAgentFor(user)
+			.post('/mfa/disable')
+			.send({
+				token,
+			})
+			.expect(200);
 
 		const dbUser = await Container.get(AuthUserRepository).findOneOrFail({
 			where: { id: user.id },
@@ -147,6 +155,18 @@ describe('Disable MFA setup', () => {
 		expect(dbUser.mfaSecret).toBe(null);
 		expect(dbUser.mfaRecoveryCodes.length).toBe(0);
 	});
+
+	test('POST /disable should fail if invalid token is given', async () => {
+		const { user } = await createUserWithMfaEnabled();
+
+		await testServer
+			.authAgentFor(user)
+			.post('/mfa/disable')
+			.send({
+				token: 'invalid token',
+			})
+			.expect(403);
+	});
 });
 
 describe('Change password with MFA enabled', () => {

packages/editor-ui/src/api/mfa.ts

@@ -18,6 +18,10 @@ export async function verifyMfaToken(
 	return await makeRestApiRequest(context, 'POST', '/mfa/verify', data);
 }
 
-export async function disableMfa(context: IRestApiContext): Promise<void> {
+export type DisableMfaParams = {
-	return await makeRestApiRequest(context, 'DELETE', '/mfa/disable');
+	token: string;
+};
+
+export async function disableMfa(context: IRestApiContext, data: DisableMfaParams): Promise<void> {
+	return await makeRestApiRequest(context, 'POST', '/mfa/disable', data);
 }

packages/editor-ui/src/components/Modal.vue

@@ -1,7 +1,7 @@
 <template>
 	<el-dialog
 		:model-value="uiStore.modalsById[name].open"
-		:before-close="closeDialog"
+		:before-close="onCloseDialog"
 		:class="{
 			'dialog-wrapper': true,
 			scrollable: scrollable,
@@ -34,7 +34,7 @@
 			class="modal-content"
 			@keydown.stop
 			@keydown.enter="handleEnter"
-			@keydown.esc="closeDialog"
+			@keydown.esc="onCloseDialog"
 		>
 			<slot v-if="!loading" name="content" />
 			<div v-else :class="$style.loader">
@@ -182,7 +182,10 @@ export default defineComponent({
 				this.$emit('enter');
 			}
 		},
-		async closeDialog() {
+		async onCloseDialog() {
+			await this.closeDialog();
+		},
+		async closeDialog(returnData?: unknown) {
 			if (this.beforeClose) {
 				const shouldClose = await this.beforeClose();
 				if (shouldClose === false) {
@@ -191,6 +194,7 @@ export default defineComponent({
 				}
 			}
 			this.uiStore.closeModal(this.name);
+			this.eventBus?.emit('closed', returnData);
 		},
 		getCustomClass() {
 			let classes = this.customClass || '';

packages/editor-ui/src/components/Modals.vue

@@ -30,6 +30,7 @@ import {
 	SETUP_CREDENTIALS_MODAL_KEY,
 	PROJECT_MOVE_RESOURCE_MODAL,
 	PROJECT_MOVE_RESOURCE_CONFIRM_MODAL,
+	PROMPT_MFA_CODE_MODAL_KEY,
 } from '@/constants';
 
 import AboutModal from '@/components/AboutModal.vue';
@@ -63,6 +64,7 @@ import WorkflowHistoryVersionRestoreModal from '@/components/WorkflowHistory/Wor
 import SetupWorkflowCredentialsModal from '@/components/SetupWorkflowCredentialsModal/SetupWorkflowCredentialsModal.vue';
 import ProjectMoveResourceModal from '@/components/Projects/ProjectMoveResourceModal.vue';
 import ProjectMoveResourceConfirmModal from '@/components/Projects/ProjectMoveResourceConfirmModal.vue';
+import PromptMfaCodeModal from './PromptMfaCodeModal/PromptMfaCodeModal.vue';
 </script>
 
 <template>
@@ -144,6 +146,10 @@ import ProjectMoveResourceConfirmModal from '@/components/Projects/ProjectMoveRe
 			<MfaSetupModal />
 		</ModalRoot>
 
+		<ModalRoot :name="PROMPT_MFA_CODE_MODAL_KEY">
+			<PromptMfaCodeModal />
+		</ModalRoot>
+
 		<ModalRoot :name="WORKFLOW_SHARE_MODAL_KEY">
 			<template #default="{ modalName, active, data }">
 				<WorkflowShareModal :data="data" :is-active="active" :modal-name="modalName" />

packages/editor-ui/src/components/PromptMfaCodeModal/PromptMfaCodeModal.vue

@@ -0,0 +1,84 @@
+<template>
+	<Modal
+		width="460px"
+		height="300px"
+		max-height="640px"
+		:title="i18n.baseText('mfa.prompt.code.modal.title')"
+		:event-bus="promptMfaCodeBus"
+		:name="PROMPT_MFA_CODE_MODAL_KEY"
+		:center="true"
+	>
+		<template #content>
+			<div :class="[$style.formContainer]">
+				<n8n-form-inputs
+					data-test-id="mfa-code-form"
+					:inputs="formFields"
+					:event-bus="formBus"
+					@submit="onSubmit"
+					@ready="onFormReady"
+				/>
+			</div>
+		</template>
+		<template #footer>
+			<div>
+				<n8n-button
+					float="right"
+					:disabled="!readyToSubmit"
+					:label="i18n.baseText('settings.personal.save')"
+					size="large"
+					data-test-id="mfa-save-button"
+					@click="onClickSave"
+				/>
+			</div>
+		</template>
+	</Modal>
+</template>
+
+<script setup lang="ts">
+import { ref } from 'vue';
+import Modal from '../Modal.vue';
+import { PROMPT_MFA_CODE_MODAL_KEY } from '@/constants';
+import { useI18n } from '@/composables/useI18n';
+import { promptMfaCodeBus } from '@/event-bus';
+import type { IFormInputs } from '@/Interface';
+import { createFormEventBus } from 'n8n-design-system';
+
+const i18n = useI18n();
+
+const formBus = ref(createFormEventBus());
+const readyToSubmit = ref(false);
+
+const formFields: IFormInputs = [
+	{
+		name: 'mfaCode',
+		initialValue: '',
+		properties: {
+			label: i18n.baseText('mfa.code.input.label'),
+			placeholder: i18n.baseText('mfa.code.input.placeholder'),
+			focusInitially: true,
+			capitalize: true,
+			required: true,
+		},
+	},
+];
+
+function onSubmit(values: { mfaCode: string }) {
+	promptMfaCodeBus.emit('close', {
+		mfaCode: values.mfaCode,
+	});
+}
+
+function onClickSave() {
+	formBus.value.emit('submit');
+}
+
+function onFormReady(isReady: boolean) {
+	readyToSubmit.value = isReady;
+}
+</script>
+
+<style lang="scss" module>
+.formContainer {
+	padding-bottom: var(--spacing-xl);
+}
+</style>

packages/editor-ui/src/constants.ts

@@ -60,6 +60,7 @@ export const SOURCE_CONTROL_PUSH_MODAL_KEY = 'sourceControlPush';
 export const SOURCE_CONTROL_PULL_MODAL_KEY = 'sourceControlPull';
 export const DEBUG_PAYWALL_MODAL_KEY = 'debugPaywall';
 export const MFA_SETUP_MODAL_KEY = 'mfaSetup';
+export const PROMPT_MFA_CODE_MODAL_KEY = 'promptMfaCode';
 export const WORKFLOW_HISTORY_VERSION_RESTORE = 'workflowHistoryVersionRestore';
 export const SETUP_CREDENTIALS_MODAL_KEY = 'setupCredentials';
 export const PROJECT_MOVE_RESOURCE_MODAL = 'projectMoveResourceModal';

packages/editor-ui/src/event-bus/mfa.ts

@@ -1,3 +1,18 @@
 import { createEventBus } from 'n8n-design-system/utils';
 
 export const mfaEventBus = createEventBus();
+
+export interface MfaModalClosedEventPayload {
+	mfaCode: string;
+}
+
+export interface MfaModalEvents {
+	close: MfaModalClosedEventPayload | undefined;
+
+	closed: MfaModalClosedEventPayload | undefined;
+}
+
+/**
+ * Event bus for transmitting the MFA code from a modal back to the view
+ */
+export const promptMfaCodeBus = createEventBus<MfaModalEvents>();

packages/editor-ui/src/plugins/i18n/locales/en.json

@@ -2521,6 +2521,7 @@
 	"mfa.setup.step2.toast.setupFinished.message": "Two-factor authentication enabled",
 	"mfa.setup.step2.toast.setupFinished.error.message": "Error enabling two-factor authentication",
 	"mfa.setup.step2.toast.tokenExpired.error.message": "MFA token expired. Close the modal and enable MFA again",
+	"mfa.prompt.code.modal.title": "Two-factor code required",
 	"settings.personal.mfa.section.title": "Two-factor authentication (2FA)",
 	"settings.personal.personalisation": "Personalisation",
 	"settings.personal.theme": "Theme",

packages/editor-ui/src/stores/ui.store.ts

@@ -35,6 +35,7 @@ import {
 	SETUP_CREDENTIALS_MODAL_KEY,
 	PROJECT_MOVE_RESOURCE_MODAL,
 	PROJECT_MOVE_RESOURCE_CONFIRM_MODAL,
+	PROMPT_MFA_CODE_MODAL_KEY,
 } from '@/constants';
 import type {
 	CloudUpdateLinkSourceType,
@@ -114,6 +115,7 @@ export const useUIStore = defineStore(STORES.UI, () => {
 				WORKFLOW_ACTIVE_MODAL_KEY,
 				COMMUNITY_PACKAGE_INSTALL_MODAL_KEY,
 				MFA_SETUP_MODAL_KEY,
+				PROMPT_MFA_CODE_MODAL_KEY,
 				SOURCE_CONTROL_PUSH_MODAL_KEY,
 				SOURCE_CONTROL_PULL_MODAL_KEY,
 				EXTERNAL_SECRETS_PROVIDER_MODAL_KEY,
@@ -696,7 +698,7 @@ export const useUIStore = defineStore(STORES.UI, () => {
 });
 
 /**
- * Helper function for listening to credential changes in the store
+ * Helper function for listening to model opening and closings in the store
  */
 export const listenForModalChanges = (opts: {
 	store: UiStore;

packages/editor-ui/src/stores/users.store.ts

@@ -324,15 +324,11 @@ export const useUsersStore = defineStore(STORES.USERS, () => {
 		}
 	};
 
-	const disableMfa = async () => {
+	const disableMfa = async (mfaCode: string) => {
-		await mfaApi.disableMfa(rootStore.restApiContext);
+		await mfaApi.disableMfa(rootStore.restApiContext, {
-		if (currentUser.value) {
+			token: mfaCode,
-			currentUser.value.mfaEnabled = false;
+		});
-		}
-	};
 
-	const disabledMfa = async () => {
-		await mfaApi.disableMfa(rootStore.restApiContext);
 		if (currentUser.value) {
 			currentUser.value.mfaEnabled = false;
 		}
@@ -410,7 +406,6 @@ export const useUsersStore = defineStore(STORES.USERS, () => {
 		fetchUserCloudAccount,
 		confirmEmail,
 		updateGlobalRole,
-		disabledMfa,
 		reset,
 	};
 });

packages/editor-ui/src/views/SettingsPersonalView.vue

@@ -1,22 +1,27 @@
 <script lang="ts" setup>
+import { ref, computed, onMounted, onBeforeUnmount } from 'vue';
 import { useI18n } from '@/composables/useI18n';
 import { useToast } from '@/composables/useToast';
 import type { IFormInputs, IUser, ThemeOption } from '@/Interface';
-import { CHANGE_PASSWORD_MODAL_KEY, MFA_DOCS_URL, MFA_SETUP_MODAL_KEY } from '@/constants';
+import {
+	CHANGE_PASSWORD_MODAL_KEY,
+	MFA_DOCS_URL,
+	MFA_SETUP_MODAL_KEY,
+	PROMPT_MFA_CODE_MODAL_KEY,
+} from '@/constants';
 import { useUIStore } from '@/stores/ui.store';
 import { useUsersStore } from '@/stores/users.store';
 import { useSettingsStore } from '@/stores/settings.store';
-import { createEventBus } from 'n8n-design-system/utils';
+import { createFormEventBus } from 'n8n-design-system/utils';
-import { ref } from 'vue';
+import type { MfaModalEvents } from '@/event-bus/mfa';
-import { computed } from 'vue';
+import { promptMfaCodeBus } from '@/event-bus/mfa';
-import { onMounted } from 'vue';
 
 const i18n = useI18n();
 const { showToast, showError } = useToast();
 
 const hasAnyBasicInfoChanges = ref<boolean>(false);
 const formInputs = ref<null | IFormInputs>(null);
-const formBus = ref(createEventBus());
+const formBus = ref(createFormEventBus());
 const readyToSubmit = ref(false);
 const currentSelectedTheme = ref(useUIStore().theme);
 const themeOptions = ref<Array<{ name: ThemeOption; label: string }>>([
@@ -154,9 +159,16 @@ function openPasswordModal() {
 function onMfaEnableClick() {
 	uiStore.openModal(MFA_SETUP_MODAL_KEY);
 }
-async function onMfaDisableClick() {
+
+async function disableMfa(payload: MfaModalEvents['closed']) {
+	if (!payload) {
+		// User closed the modal without submitting the form
+		return;
+	}
+
 	try {
-		await usersStore.disabledMfa();
+		await usersStore.disableMfa(payload.mfaCode);
+
 		showToast({
 			title: i18n.baseText('settings.personal.mfa.toast.disabledMfa.title'),
 			message: i18n.baseText('settings.personal.mfa.toast.disabledMfa.message'),
@@ -167,6 +179,16 @@ async function onMfaDisableClick() {
 		showError(e, i18n.baseText('settings.personal.mfa.toast.disabledMfa.error.message'));
 	}
 }
+
+async function onMfaDisableClick() {
+	uiStore.openModal(PROMPT_MFA_CODE_MODAL_KEY);
+
+	promptMfaCodeBus.once('closed', disableMfa);
+}
+
+onBeforeUnmount(() => {
+	promptMfaCodeBus.off('closed', disableMfa);
+});
 </script>
 
 <template>