fix: Require mfa code to disable mfa

Continuation of #10341

Also enables rate limiting for the `/mfa/disable` endpoint.

For the FE:
- adds a typed event emitter
- a mechanism to use Modal event emitter to provide data back from the modal

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) {
-		const user = await this.authUserRepository.findOneByOrFail({ id: userId });
-		user.mfaEnabled = false;
-		user.mfaSecret = null;
-		user.mfaRecoveryCodes = [];
-		return await this.authUserRepository.save(user);
+	async disableMfa(userId: string, mfaToken: string) {
+		const isValidToken = await this.validateMfa(userId, mfaToken, undefined);
+		if (!isValidToken) {
+			throw new InvalidMfaCodeError();
+		}
+
+		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(
-					new ForbiddenError('Invalid two-factor code.'),
-				);
+				await expect(controller.updatePassword(req, mock())).rejects.toThrow(InvalidMfaCodeError);
 			});
 
 			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')
-	async disableMFA(req: AuthenticatedRequest) {
-		const { id } = req.user;
+	@Post('/disable', { rateLimit: true })
+	async disableMFA(req: MFA.Disable) {
+		const { id: userId } = req.user;
+		const { token = null } = req.body;
+
+		if (typeof token !== 'string' || !token) {
+			throw new BadRequestError('Token is required to disable MFA feature');
+		}
 
-		await this.mfaService.disableMfa(id);
+		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/design-system/src/utils/event-bus.ts

@@ -2,51 +2,17 @@
 export type CallbackFn = Function;
 export type UnregisterFn = () => void;
 
-export type Listener<Payload> = (payload: Payload) => void;
-
-export type Payloads<ListenerMap> = {
-	[E in keyof ListenerMap]: unknown;
-};
-
-// TODO: Fix all usages of `createEventBus` and convert `any` to `unknown`
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export interface EventBus<ListenerMap extends Payloads<ListenerMap> = Record<string, any>> {
-	on: <EventName extends keyof ListenerMap & string>(
-		eventName: EventName,
-		fn: Listener<ListenerMap[EventName]>,
-	) => UnregisterFn;
-
-	once: <EventName extends keyof ListenerMap & string>(
-		eventName: EventName,
-		fn: Listener<ListenerMap[EventName]>,
-	) => UnregisterFn;
-
-	off: <EventName extends keyof ListenerMap & string>(
-		eventName: EventName,
-		fn: Listener<ListenerMap[EventName]>,
-	) => void;
-
-	emit: <EventName extends keyof ListenerMap & string>(
-		eventName: EventName,
-		event?: ListenerMap[EventName],
-	) => void;
+export interface EventBus {
+	on: (eventName: string, fn: CallbackFn) => UnregisterFn;
+	once: (eventName: string, fn: CallbackFn) => UnregisterFn;
+	off: (eventName: string, fn: CallbackFn) => void;
+	emit: <T = Event>(eventName: string, event?: T) => void;
 }
 
 /**
- * Creates an event bus with the given listener map.
- *
- * @example
- * ```ts
- * const eventBus = createEventBus<{
- *   'user-logged-in': { username: string };
- *   'user-logged-out': never;
- * }>();
+ * @deprecated Use the typed version instead from `typed-event-bus.ts`
  */
-export function createEventBus<
-	// TODO: Fix all usages of `createEventBus` and convert `any` to `unknown`
-	// eslint-disable-next-line @typescript-eslint/no-explicit-any
-	ListenerMap extends Payloads<ListenerMap> = Record<string, any>,
->(): EventBus<ListenerMap> {
+export function createEventBus(): EventBus {
 	const handlers = new Map<string, CallbackFn[]>();
 
 	function off(eventName: string, fn: CallbackFn) {

packages/design-system/src/utils/typed-event-bus.ts

@@ -0,0 +1,36 @@
+import type { UnregisterFn } from './event-bus';
+import { createEventBus as createUntypedEventBus } from './event-bus';
+
+export type Listener<Payload> = (payload: Payload) => void;
+
+export type Payloads<ListenerMap> = {
+	[E in keyof ListenerMap]: unknown;
+};
+
+export interface TypedEventBus<ListenerMap extends Payloads<ListenerMap>> {
+	on: <EventName extends keyof ListenerMap & string>(
+		eventName: EventName,
+		fn: Listener<ListenerMap[EventName]>,
+	) => UnregisterFn;
+
+	once: <EventName extends keyof ListenerMap & string>(
+		eventName: EventName,
+		fn: Listener<ListenerMap[EventName]>,
+	) => UnregisterFn;
+
+	off: <EventName extends keyof ListenerMap & string>(
+		eventName: EventName,
+		fn: Listener<ListenerMap[EventName]>,
+	) => void;
+
+	emit: <EventName extends keyof ListenerMap & string>(
+		eventName: EventName,
+		event?: ListenerMap[EventName],
+	) => void;
+}
+
+export function createTypedEventBus<
+	ListenerMap extends Payloads<ListenerMap>,
+>(): TypedEventBus<ListenerMap> {
+	return createUntypedEventBus() as TypedEventBus<ListenerMap>;
+}

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> {
-	return await makeRestApiRequest(context, 'DELETE', '/mfa/disable');
+export type DisableMfaParams = {
+	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="() => closeDialog()"
 		:class="{
 			'dialog-wrapper': true,
 			scrollable: scrollable,
@@ -34,7 +34,7 @@
 			class="modal-content"
 			@keydown.stop
 			@keydown.enter="handleEnter"
-			@keydown.esc="closeDialog"
+			@keydown.esc="() => closeDialog()"
 		>
 			<slot v-if="!loading" name="content" />
 			<div v-else :class="$style.loader">
@@ -182,7 +182,7 @@ export default defineComponent({
 				this.$emit('enter');
 			}
 		},
-		async closeDialog() {
+		async closeDialog(returnData?: unknown) {
 			if (this.beforeClose) {
 				const shouldClose = await this.beforeClose();
 				if (shouldClose === false) {
@@ -191,6 +191,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" />