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).
This commit is contained in:
Ricardo Espinoza 2023-12-06 04:00:13 -05:00 committed by GitHub
parent 46dd4d3105
commit f5502cc628
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 55 additions and 1 deletions

View file

@ -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');

View file

@ -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();