mirror of
https://github.com/n8n-io/n8n.git
synced 2024-12-26 05:04:05 -08:00
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:
parent
46dd4d3105
commit
f5502cc628
|
@ -125,7 +125,8 @@ export class UserService {
|
||||||
user: User,
|
user: User,
|
||||||
options?: { withInviteUrl?: boolean; posthog?: PostHogClient; withScopes?: boolean },
|
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');
|
const ldapIdentity = authIdentities?.find((i) => i.providerType === 'ldap');
|
||||||
|
|
||||||
|
|
|
@ -12,6 +12,7 @@ import * as utils from './shared/utils/';
|
||||||
import { getGlobalMemberRole, getGlobalOwnerRole } from './shared/db/roles';
|
import { getGlobalMemberRole, getGlobalOwnerRole } from './shared/db/roles';
|
||||||
import { createUser, createUserShell } from './shared/db/users';
|
import { createUser, createUserShell } from './shared/db/users';
|
||||||
import { UserRepository } from '@db/repositories/user.repository';
|
import { UserRepository } from '@db/repositories/user.repository';
|
||||||
|
import { MfaService } from '@/Mfa/mfa.service';
|
||||||
|
|
||||||
let globalOwnerRole: Role;
|
let globalOwnerRole: Role;
|
||||||
let globalMemberRole: Role;
|
let globalMemberRole: Role;
|
||||||
|
@ -22,9 +23,12 @@ const ownerPassword = randomValidPassword();
|
||||||
const testServer = utils.setupTestServer({ endpointGroups: ['auth'] });
|
const testServer = utils.setupTestServer({ endpointGroups: ['auth'] });
|
||||||
const license = testServer.license;
|
const license = testServer.license;
|
||||||
|
|
||||||
|
let mfaService: MfaService;
|
||||||
|
|
||||||
beforeAll(async () => {
|
beforeAll(async () => {
|
||||||
globalOwnerRole = await getGlobalOwnerRole();
|
globalOwnerRole = await getGlobalOwnerRole();
|
||||||
globalMemberRole = await getGlobalMemberRole();
|
globalMemberRole = await getGlobalMemberRole();
|
||||||
|
mfaService = Container.get(MfaService);
|
||||||
});
|
});
|
||||||
|
|
||||||
beforeEach(async () => {
|
beforeEach(async () => {
|
||||||
|
@ -59,6 +63,8 @@ describe('POST /login', () => {
|
||||||
globalRole,
|
globalRole,
|
||||||
apiKey,
|
apiKey,
|
||||||
globalScopes,
|
globalScopes,
|
||||||
|
mfaSecret,
|
||||||
|
mfaRecoveryCodes,
|
||||||
} = response.body.data;
|
} = response.body.data;
|
||||||
|
|
||||||
expect(validator.isUUID(id)).toBe(true);
|
expect(validator.isUUID(id)).toBe(true);
|
||||||
|
@ -73,6 +79,53 @@ describe('POST /login', () => {
|
||||||
expect(globalRole.scope).toBe('global');
|
expect(globalRole.scope).toBe('global');
|
||||||
expect(apiKey).toBeUndefined();
|
expect(apiKey).toBeUndefined();
|
||||||
expect(globalScopes).toBeDefined();
|
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);
|
const authToken = utils.getAuthToken(response);
|
||||||
expect(authToken).toBeDefined();
|
expect(authToken).toBeDefined();
|
||||||
|
|
Loading…
Reference in a new issue