mirror of
https://github.com/n8n-io/n8n.git
synced 2024-12-25 04:34:06 -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,
|
||||
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');
|
||||
|
||||
|
|
|
@ -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();
|
||||
|
|
Loading…
Reference in a new issue