From f5502cc628f6b348f7fe3325b96ec9dc3360beaf Mon Sep 17 00:00:00 2001
From: Ricardo Espinoza <ricardo@n8n.io>
Date: Wed, 6 Dec 2023 04:00:13 -0500
Subject: [PATCH] fix(core): Make sure mfa secret and recovery codes are not
 returned on login (#7936)

## Summary

What: Fix issue of login endpoint returning secret and recovery codes
when MFA is enabled. Bug was introduced in this
[PR](https://github.com/n8n-io/n8n/pull/6994), specifically in this
[line](https://github.com/n8n-io/n8n/pull/6994/files#diff-95a87cb029a3d26e6722df2e68132453fc254fc1f4540cbdaa95cfdbda1893deL91).

Why: We should not be filtering the secret and recovery codes

Same PR caused the issues on ticket ->
https://linear.app/n8n/issue/ADO-1494/on-user-list-copy-password-reset-link-and-copy-invite-link-are-broken

## Review / Merge checklist
- [x] PR title and summary are descriptive. **Remember, the title
automatically goes into the changelog. Use `(no-changelog)` otherwise.**
([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md))
- [x] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up
ticket created.
- [x] Tests included.
> A bug is not considered fixed, unless a test is added to prevent it
from happening again. A feature is not complete without tests.
  >
> *(internal)* You can use Slack commands to trigger [e2e
tests](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#a39f9e5ba64a48b58a71d81c837e8227)
or [deploy test
instance](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#f6a177d32bde4b57ae2da0b8e454bfce)
or [deploy early access version on
Cloud](https://www.notion.so/n8n/Cloudbot-3dbe779836004972b7057bc989526998?pvs=4#fef2d36ab02247e1a0f65a74f6fb534e).
---
 packages/cli/src/services/user.service.ts     |  3 +-
 .../cli/test/integration/auth.api.test.ts     | 53 +++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/packages/cli/src/services/user.service.ts b/packages/cli/src/services/user.service.ts
index 2500d9cfe5cf9..c19e98eb0c31a 100644
--- a/packages/cli/src/services/user.service.ts
+++ b/packages/cli/src/services/user.service.ts
@@ -125,7 +125,8 @@ export class UserService {
 		user: User,
 		options?: { withInviteUrl?: boolean; posthog?: PostHogClient; withScopes?: boolean },
 	) {
-		const { password, updatedAt, apiKey, authIdentities, ...rest } = user;
+		const { password, updatedAt, apiKey, authIdentities, mfaRecoveryCodes, mfaSecret, ...rest } =
+			user;
 
 		const ldapIdentity = authIdentities?.find((i) => i.providerType === 'ldap');
 
diff --git a/packages/cli/test/integration/auth.api.test.ts b/packages/cli/test/integration/auth.api.test.ts
index 4044482be695d..ab3f3a8dd3c42 100644
--- a/packages/cli/test/integration/auth.api.test.ts
+++ b/packages/cli/test/integration/auth.api.test.ts
@@ -12,6 +12,7 @@ import * as utils from './shared/utils/';
 import { getGlobalMemberRole, getGlobalOwnerRole } from './shared/db/roles';
 import { createUser, createUserShell } from './shared/db/users';
 import { UserRepository } from '@db/repositories/user.repository';
+import { MfaService } from '@/Mfa/mfa.service';
 
 let globalOwnerRole: Role;
 let globalMemberRole: Role;
@@ -22,9 +23,12 @@ const ownerPassword = randomValidPassword();
 const testServer = utils.setupTestServer({ endpointGroups: ['auth'] });
 const license = testServer.license;
 
+let mfaService: MfaService;
+
 beforeAll(async () => {
 	globalOwnerRole = await getGlobalOwnerRole();
 	globalMemberRole = await getGlobalMemberRole();
+	mfaService = Container.get(MfaService);
 });
 
 beforeEach(async () => {
@@ -59,6 +63,8 @@ describe('POST /login', () => {
 			globalRole,
 			apiKey,
 			globalScopes,
+			mfaSecret,
+			mfaRecoveryCodes,
 		} = response.body.data;
 
 		expect(validator.isUUID(id)).toBe(true);
@@ -73,6 +79,53 @@ describe('POST /login', () => {
 		expect(globalRole.scope).toBe('global');
 		expect(apiKey).toBeUndefined();
 		expect(globalScopes).toBeDefined();
+		expect(mfaRecoveryCodes).toBeUndefined();
+		expect(mfaSecret).toBeUndefined();
+
+		const authToken = utils.getAuthToken(response);
+		expect(authToken).toBeDefined();
+	});
+
+	test('should log user with MFA enabled', async () => {
+		const secret = 'test';
+		const recoveryCodes = ['1'];
+		await mfaService.saveSecretAndRecoveryCodes(owner.id, secret, recoveryCodes);
+		await mfaService.enableMfa(owner.id);
+
+		const response = await testServer.authlessAgent.post('/login').send({
+			email: owner.email,
+			password: ownerPassword,
+			mfaToken: mfaService.totp.generateTOTP(secret),
+		});
+
+		expect(response.statusCode).toBe(200);
+
+		const {
+			id,
+			email,
+			firstName,
+			lastName,
+			password,
+			personalizationAnswers,
+			globalRole,
+			apiKey,
+			mfaRecoveryCodes,
+			mfaSecret,
+		} = response.body.data;
+
+		expect(validator.isUUID(id)).toBe(true);
+		expect(email).toBe(owner.email);
+		expect(firstName).toBe(owner.firstName);
+		expect(lastName).toBe(owner.lastName);
+		expect(password).toBeUndefined();
+		expect(personalizationAnswers).toBeNull();
+		expect(password).toBeUndefined();
+		expect(globalRole).toBeDefined();
+		expect(globalRole.name).toBe('owner');
+		expect(globalRole.scope).toBe('global');
+		expect(apiKey).toBeUndefined();
+		expect(mfaRecoveryCodes).toBeUndefined();
+		expect(mfaSecret).toBeUndefined();
 
 		const authToken = utils.getAuthToken(response);
 		expect(authToken).toBeDefined();