diff --git a/cypress/e2e/18-user-management.cy.ts b/cypress/e2e/18-user-management.cy.ts index d4eb5841cf..6f97597023 100644 --- a/cypress/e2e/18-user-management.cy.ts +++ b/cypress/e2e/18-user-management.cy.ts @@ -36,7 +36,7 @@ describe('User Management', { disableAutoLogin: true }, () => { it('should login and logout', () => { cy.visit('/'); - cy.get('input[name="email"]').type(INSTANCE_OWNER.email); + cy.get('input[name="emailOrLdapLoginId"]').type(INSTANCE_OWNER.email); cy.get('input[name="password"]').type(INSTANCE_OWNER.password); cy.getByTestId('form-submit-button').click(); mainSidebar.getters.logo().should('be.visible'); @@ -47,7 +47,7 @@ describe('User Management', { disableAutoLogin: true }, () => { mainSidebar.actions.openUserMenu(); cy.getByTestId('user-menu-item-logout').click(); - cy.get('input[name="email"]').type(INSTANCE_MEMBERS[0].email); + cy.get('input[name="emailOrLdapLoginId"]').type(INSTANCE_MEMBERS[0].email); cy.get('input[name="password"]').type(INSTANCE_MEMBERS[0].password); cy.getByTestId('form-submit-button').click(); mainSidebar.getters.logo().should('be.visible'); diff --git a/cypress/e2e/39-projects.cy.ts b/cypress/e2e/39-projects.cy.ts index d8c618539f..5a06e05f8c 100644 --- a/cypress/e2e/39-projects.cy.ts +++ b/cypress/e2e/39-projects.cy.ts @@ -506,7 +506,7 @@ describe('Projects', { disableAutoLogin: true }, () => { mainSidebar.actions.openUserMenu(); cy.getByTestId('user-menu-item-logout').click(); - cy.get('input[name="email"]').type(INSTANCE_MEMBERS[0].email); + cy.get('input[name="emailOrLdapLoginId"]').type(INSTANCE_MEMBERS[0].email); cy.get('input[name="password"]').type(INSTANCE_MEMBERS[0].password); cy.getByTestId('form-submit-button').click(); diff --git a/cypress/pages/signin.ts b/cypress/pages/signin.ts index bc0d7196a3..248ba49096 100644 --- a/cypress/pages/signin.ts +++ b/cypress/pages/signin.ts @@ -15,7 +15,7 @@ export class SigninPage extends BasePage { getters = { form: () => cy.getByTestId('auth-form'), - email: () => cy.getByTestId('email'), + email: () => cy.getByTestId('emailOrLdapLoginId'), password: () => cy.getByTestId('password'), submit: () => cy.get('button'), }; diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts index 158fe129ec..fea929e12c 100644 --- a/cypress/support/commands.ts +++ b/cypress/support/commands.ts @@ -69,7 +69,7 @@ Cypress.Commands.add('signin', ({ email, password }) => { .request({ method: 'POST', url: `${BACKEND_BASE_URL}/rest/login`, - body: { email, password }, + body: { emailOrLdapLoginId: email, password }, failOnStatusCode: false, }) .then((response) => { diff --git a/packages/@n8n/api-types/src/dto/auth/__tests__/login-request.dto.test.ts b/packages/@n8n/api-types/src/dto/auth/__tests__/login-request.dto.test.ts index f222f1d93e..8c9e888007 100644 --- a/packages/@n8n/api-types/src/dto/auth/__tests__/login-request.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/auth/__tests__/login-request.dto.test.ts @@ -6,7 +6,7 @@ describe('LoginRequestDto', () => { { name: 'complete valid login request', request: { - email: 'test@example.com', + emailOrLdapLoginId: 'test@example.com', password: 'securePassword123', mfaCode: '123456', }, @@ -14,14 +14,14 @@ describe('LoginRequestDto', () => { { name: 'login request without optional MFA', request: { - email: 'test@example.com', + emailOrLdapLoginId: 'test@example.com', password: 'securePassword123', }, }, { name: 'login request with both mfaCode and mfaRecoveryCode', request: { - email: 'test@example.com', + emailOrLdapLoginId: 'test@example.com', password: 'securePassword123', mfaCode: '123456', mfaRecoveryCode: 'recovery-code-123', @@ -30,7 +30,7 @@ describe('LoginRequestDto', () => { { name: 'login request with only mfaRecoveryCode', request: { - email: 'test@example.com', + emailOrLdapLoginId: 'test@example.com', password: 'securePassword123', mfaRecoveryCode: 'recovery-code-123', }, @@ -44,43 +44,35 @@ describe('LoginRequestDto', () => { describe('Invalid requests', () => { test.each([ { - name: 'invalid email', + name: 'invalid emailOrLdapLoginId', request: { - email: 'invalid-email', + emailOrLdapLoginId: 0, password: 'securePassword123', }, - expectedErrorPath: ['email'], + expectedErrorPath: ['emailOrLdapLoginId'], }, { name: 'empty password', request: { - email: 'test@example.com', + emailOrLdapLoginId: 'test@example.com', password: '', }, expectedErrorPath: ['password'], }, { - name: 'missing email', + name: 'missing emailOrLdapLoginId', request: { password: 'securePassword123', }, - expectedErrorPath: ['email'], + expectedErrorPath: ['emailOrLdapLoginId'], }, { name: 'missing password', request: { - email: 'test@example.com', + emailOrLdapLoginId: 'test@example.com', }, expectedErrorPath: ['password'], }, - { - name: 'whitespace in email and password', - request: { - email: ' test@example.com ', - password: ' securePassword123 ', - }, - expectedErrorPath: ['email'], - }, ])('should fail validation for $name', ({ request, expectedErrorPath }) => { const result = LoginRequestDto.safeParse(request); expect(result.success).toBe(false); diff --git a/packages/@n8n/api-types/src/dto/auth/login-request.dto.ts b/packages/@n8n/api-types/src/dto/auth/login-request.dto.ts index 894263992c..d1f6771b9c 100644 --- a/packages/@n8n/api-types/src/dto/auth/login-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/auth/login-request.dto.ts @@ -2,7 +2,12 @@ import { z } from 'zod'; import { Z } from 'zod-class'; export class LoginRequestDto extends Z.class({ - email: z.string().email(), + /* + * The LDAP username does not need to be an email, so email validation + * is not enforced here. The controller determines whether this is an + * email and validates when LDAP is disabled + */ + emailOrLdapLoginId: z.string().trim(), password: z.string().min(1), mfaCode: z.string().optional(), mfaRecoveryCode: z.string().optional(), diff --git a/packages/cli/src/controllers/__tests__/auth.controller.test.ts b/packages/cli/src/controllers/__tests__/auth.controller.test.ts new file mode 100644 index 0000000000..0abebb5ecf --- /dev/null +++ b/packages/cli/src/controllers/__tests__/auth.controller.test.ts @@ -0,0 +1,99 @@ +import type { LoginRequestDto } from '@n8n/api-types'; +import { Container } from '@n8n/di'; +import type { Response } from 'express'; +import { mock } from 'jest-mock-extended'; +import { Logger } from 'n8n-core'; + +import * as auth from '@/auth'; +import { AuthService } from '@/auth/auth.service'; +import config from '@/config'; +import type { User } from '@/databases/entities/user'; +import { UserRepository } from '@/databases/repositories/user.repository'; +import { EventService } from '@/events/event.service'; +import { License } from '@/license'; +import { MfaService } from '@/mfa/mfa.service'; +import { PostHogClient } from '@/posthog'; +import type { AuthenticatedRequest } from '@/requests'; +import { UserService } from '@/services/user.service'; +import { mockInstance } from '@test/mocking'; + +import { AuthController } from '../auth.controller'; + +jest.mock('@/auth'); + +const mockedAuth = auth as jest.Mocked; + +describe('AuthController', () => { + mockInstance(Logger); + mockInstance(EventService); + mockInstance(AuthService); + mockInstance(MfaService); + mockInstance(UserService); + mockInstance(UserRepository); + mockInstance(PostHogClient); + mockInstance(License); + const controller = Container.get(AuthController); + const userService = Container.get(UserService); + const authService = Container.get(AuthService); + const eventsService = Container.get(EventService); + const postHog = Container.get(PostHogClient); + + describe('login', () => { + it('should not validate email in "emailOrLdapLoginId" if LDAP is enabled', async () => { + // Arrange + + const browserId = '1'; + + const member = mock({ + id: '123', + role: 'global:member', + mfaEnabled: false, + }); + + const body = mock({ + emailOrLdapLoginId: 'non email', + password: 'password', + }); + + const req = mock({ + user: member, + body, + browserId, + }); + + const res = mock(); + + mockedAuth.handleEmailLogin.mockResolvedValue(member); + + mockedAuth.handleLdapLogin.mockResolvedValue(member); + + config.set('userManagement.authenticationMethod', 'ldap'); + + // Act + + await controller.login(req, res, body); + + // Assert + + expect(mockedAuth.handleEmailLogin).toHaveBeenCalledWith( + body.emailOrLdapLoginId, + body.password, + ); + expect(mockedAuth.handleLdapLogin).toHaveBeenCalledWith( + body.emailOrLdapLoginId, + body.password, + ); + + expect(authService.issueCookie).toHaveBeenCalledWith(res, member, browserId); + expect(eventsService.emit).toHaveBeenCalledWith('user-logged-in', { + user: member, + authenticationMethod: 'ldap', + }); + + expect(userService.toPublic).toHaveBeenCalledWith(member, { + posthog: postHog, + withScopes: true, + }); + }); + }); +}); diff --git a/packages/cli/src/controllers/auth.controller.ts b/packages/cli/src/controllers/auth.controller.ts index fb06c1a80b..dac65903fe 100644 --- a/packages/cli/src/controllers/auth.controller.ts +++ b/packages/cli/src/controllers/auth.controller.ts @@ -1,4 +1,5 @@ import { LoginRequestDto, ResolveSignupTokenQueryDto } from '@n8n/api-types'; +import { isEmail } from 'class-validator'; import { Response } from 'express'; import { Logger } from 'n8n-core'; @@ -44,14 +45,19 @@ export class AuthController { res: Response, @Body payload: LoginRequestDto, ): Promise { - const { email, password, mfaCode, mfaRecoveryCode } = payload; + const { emailOrLdapLoginId, password, mfaCode, mfaRecoveryCode } = payload; let user: User | undefined; let usedAuthenticationMethod = getCurrentAuthenticationMethod(); + + if (usedAuthenticationMethod === 'email' && !isEmail(emailOrLdapLoginId)) { + throw new BadRequestError('Invalid email address'); + } + if (isSamlCurrentAuthenticationMethod()) { // attempt to fetch user data with the credentials, but don't log in yet - const preliminaryUser = await handleEmailLogin(email, password); + const preliminaryUser = await handleEmailLogin(emailOrLdapLoginId, password); // if the user is an owner, continue with the login if ( preliminaryUser?.role === 'global:owner' || @@ -63,15 +69,15 @@ export class AuthController { throw new AuthError('SSO is enabled, please log in with SSO'); } } else if (isLdapCurrentAuthenticationMethod()) { - const preliminaryUser = await handleEmailLogin(email, password); + const preliminaryUser = await handleEmailLogin(emailOrLdapLoginId, password); if (preliminaryUser?.role === 'global:owner') { user = preliminaryUser; usedAuthenticationMethod = 'email'; } else { - user = await handleLdapLogin(email, password); + user = await handleLdapLogin(emailOrLdapLoginId, password); } } else { - user = await handleEmailLogin(email, password); + user = await handleEmailLogin(emailOrLdapLoginId, password); } if (user) { @@ -101,7 +107,7 @@ export class AuthController { } this.eventService.emit('user-login-failed', { authenticationMethod: usedAuthenticationMethod, - userEmail: email, + userEmail: emailOrLdapLoginId, reason: 'wrong credentials', }); throw new AuthError('Wrong username or password. Do you have caps lock on?'); diff --git a/packages/cli/test/integration/auth.api.test.ts b/packages/cli/test/integration/auth.api.test.ts index 7fd3f50eac..92422c27b9 100644 --- a/packages/cli/test/integration/auth.api.test.ts +++ b/packages/cli/test/integration/auth.api.test.ts @@ -43,7 +43,7 @@ describe('POST /login', () => { test('should log user in', async () => { const response = await testServer.authlessAgent.post('/login').send({ - email: owner.email, + emailOrLdapLoginId: owner.email, password: ownerPassword, }); @@ -87,7 +87,7 @@ describe('POST /login', () => { await mfaService.enableMfa(owner.id); const response = await testServer.authlessAgent.post('/login').send({ - email: owner.email, + emailOrLdapLoginId: owner.email, password: ownerPassword, mfaCode: mfaService.totp.generateTOTP(secret), }); @@ -131,7 +131,7 @@ describe('POST /login', () => { }); const response = await testServer.authlessAgent.post('/login').send({ - email: member.email, + emailOrLdapLoginId: member.email, password, }); expect(response.statusCode).toBe(403); @@ -148,19 +148,16 @@ describe('POST /login', () => { expect(response.statusCode).toBe(200); }); - test('should fail on invalid email in the payload', async () => { + test('should fail with invalid email in the payload is the current authentication method is "email"', async () => { + config.set('userManagement.authenticationMethod', 'email'); + const response = await testServer.authlessAgent.post('/login').send({ - email: 'invalid-email', + emailOrLdapLoginId: 'invalid-email', password: ownerPassword, }); expect(response.statusCode).toBe(400); - expect(response.body).toEqual({ - validation: 'email', - code: 'invalid_string', - message: 'Invalid email', - path: ['email'], - }); + expect(response.body.message).toBe('Invalid email address'); }); }); diff --git a/packages/cli/test/integration/ldap/ldap.api.test.ts b/packages/cli/test/integration/ldap/ldap.api.test.ts index 9a4c214f10..1f4421e908 100644 --- a/packages/cli/test/integration/ldap/ldap.api.test.ts +++ b/packages/cli/test/integration/ldap/ldap.api.test.ts @@ -470,7 +470,7 @@ describe('POST /login', () => { const response = await testServer.authlessAgent .post('/login') - .send({ email: ldapUser.mail, password: 'password' }); + .send({ emailOrLdapLoginId: ldapUser.mail, password: 'password' }); expect(response.statusCode).toBe(200); expect(response.headers['set-cookie']).toBeDefined(); @@ -529,7 +529,7 @@ describe('POST /login', () => { const response = await testServer.authlessAgent .post('/login') - .send({ email: owner.email, password: 'password' }); + .send({ emailOrLdapLoginId: owner.email, password: 'password' }); expect(response.status).toBe(200); expect(response.body.data?.signInType).toBeDefined(); diff --git a/packages/cli/test/integration/mfa/mfa.api.test.ts b/packages/cli/test/integration/mfa/mfa.api.test.ts index 935ede4c17..3a10e6b39b 100644 --- a/packages/cli/test/integration/mfa/mfa.api.test.ts +++ b/packages/cli/test/integration/mfa/mfa.api.test.ts @@ -268,7 +268,7 @@ describe('Change password with MFA enabled', () => { .authAgentFor(user) .post('/login') .send({ - email: user.email, + emailOrLdapLoginId: user.email, password: newPassword, mfaCode: new TOTPService().generateTOTP(rawSecret), }) @@ -306,7 +306,10 @@ describe('Login', () => { const user = await createUser({ password }); - await testServer.authlessAgent.post('/login').send({ email: user.email, password }).expect(200); + await testServer.authlessAgent + .post('/login') + .send({ emailOrLdapLoginId: user.email, password }) + .expect(200); }); test('GET /login should not include mfaSecret and mfaRecoveryCodes property in response', async () => { @@ -323,7 +326,7 @@ describe('Login', () => { await testServer.authlessAgent .post('/login') - .send({ email: user.email, password: rawPassword }) + .send({ emailOrLdapLoginId: user.email, password: rawPassword }) .expect(401); }); @@ -333,7 +336,7 @@ describe('Login', () => { await testServer.authlessAgent .post('/login') - .send({ email: user.email, password: rawPassword, mfaCode: 'wrongvalue' }) + .send({ emailOrLdapLoginId: user.email, password: rawPassword, mfaCode: 'wrongvalue' }) .expect(401); }); @@ -342,7 +345,7 @@ describe('Login', () => { const response = await testServer.authlessAgent .post('/login') - .send({ email: user.email, password: rawPassword }) + .send({ emailOrLdapLoginId: user.email, password: rawPassword }) .expect(401); expect(response.body.code).toBe(998); @@ -355,7 +358,7 @@ describe('Login', () => { const response = await testServer.authlessAgent .post('/login') - .send({ email: user.email, password: rawPassword, mfaCode: token }) + .send({ emailOrLdapLoginId: user.email, password: rawPassword, mfaCode: token }) .expect(200); const data = response.body.data; @@ -370,7 +373,11 @@ describe('Login', () => { await testServer.authlessAgent .post('/login') - .send({ email: user.email, password: rawPassword, mfaRecoveryCode: 'wrongvalue' }) + .send({ + emailOrLdapLoginId: user.email, + password: rawPassword, + mfaRecoveryCode: 'wrongvalue', + }) .expect(401); }); @@ -379,7 +386,11 @@ describe('Login', () => { const response = await testServer.authlessAgent .post('/login') - .send({ email: user.email, password: rawPassword, mfaRecoveryCode: rawRecoveryCodes[0] }) + .send({ + emailOrLdapLoginId: user.email, + password: rawPassword, + mfaRecoveryCode: rawRecoveryCodes[0], + }) .expect(200); const data = response.body.data; diff --git a/packages/frontend/editor-ui/src/api/users.ts b/packages/frontend/editor-ui/src/api/users.ts index bff4f65fac..dff84242b5 100644 --- a/packages/frontend/editor-ui/src/api/users.ts +++ b/packages/frontend/editor-ui/src/api/users.ts @@ -1,4 +1,5 @@ import type { + LoginRequestDto, PasswordUpdateRequestDto, SettingsUpdateRequestDto, UserUpdateRequestDto, @@ -21,7 +22,7 @@ export async function loginCurrentUser( export async function login( context: IRestApiContext, - params: { email: string; password: string; mfaCode?: string; mfaRecoveryToken?: string }, + params: LoginRequestDto, ): Promise { return await makeRestApiRequest(context, 'POST', '/login', params); } diff --git a/packages/frontend/editor-ui/src/stores/users.store.ts b/packages/frontend/editor-ui/src/stores/users.store.ts index b349bcb37f..2bb85113d6 100644 --- a/packages/frontend/editor-ui/src/stores/users.store.ts +++ b/packages/frontend/editor-ui/src/stores/users.store.ts @@ -1,4 +1,5 @@ import type { + LoginRequestDto, PasswordUpdateRequestDto, SettingsUpdateRequestDto, UserUpdateRequestDto, @@ -181,12 +182,7 @@ export const useUsersStore = defineStore(STORES.USERS, () => { }; }; - const loginWithCreds = async (params: { - email: string; - password: string; - mfaCode?: string; - mfaRecoveryCode?: string; - }) => { + const loginWithCreds = async (params: LoginRequestDto) => { const user = await usersApi.login(rootStore.restApiContext, params); if (!user) { return; diff --git a/packages/frontend/editor-ui/src/views/AuthView.vue b/packages/frontend/editor-ui/src/views/AuthView.vue index 426fac0345..5dc46aac4f 100644 --- a/packages/frontend/editor-ui/src/views/AuthView.vue +++ b/packages/frontend/editor-ui/src/views/AuthView.vue @@ -3,6 +3,7 @@ import Logo from '@/components/Logo/Logo.vue'; import SSOLogin from '@/components/SSOLogin.vue'; import type { IFormBoxConfig } from '@/Interface'; import { useSettingsStore } from '@/stores/settings.store'; +import type { EmailOrLdapLoginIdAndPassword } from './SigninView.vue'; withDefaults( defineProps<{ @@ -19,7 +20,7 @@ withDefaults( const emit = defineEmits<{ update: [{ name: string; value: string }]; - submit: [values: { [key: string]: string }]; + submit: [values: EmailOrLdapLoginIdAndPassword]; secondaryClick: []; }>(); @@ -27,7 +28,7 @@ const onUpdate = (e: { name: string; value: string }) => { emit('update', e); }; -const onSubmit = (values: { [key: string]: string }) => { +const onSubmit = (values: EmailOrLdapLoginIdAndPassword) => { emit('submit', values); }; diff --git a/packages/frontend/editor-ui/src/views/SigninView.test.ts b/packages/frontend/editor-ui/src/views/SigninView.test.ts index 71c578d595..5652122b13 100644 --- a/packages/frontend/editor-ui/src/views/SigninView.test.ts +++ b/packages/frontend/editor-ui/src/views/SigninView.test.ts @@ -85,7 +85,7 @@ describe('SigninView', () => { await userEvent.click(submitButton); expect(usersStore.loginWithCreds).toHaveBeenCalledWith({ - email: 'test@n8n.io', + emailOrLdapLoginId: 'test@n8n.io', password: 'password', mfaCode: undefined, mfaRecoveryCode: undefined, diff --git a/packages/frontend/editor-ui/src/views/SigninView.vue b/packages/frontend/editor-ui/src/views/SigninView.vue index 0257870c7d..e2b9f6ccad 100644 --- a/packages/frontend/editor-ui/src/views/SigninView.vue +++ b/packages/frontend/editor-ui/src/views/SigninView.vue @@ -15,6 +15,14 @@ import { useCloudPlanStore } from '@/stores/cloudPlan.store'; import type { IFormBoxConfig } from '@/Interface'; import { MFA_AUTHENTICATION_REQUIRED_ERROR_CODE, VIEWS, MFA_FORM } from '@/constants'; +import type { LoginRequestDto } from '@n8n/api-types'; + +export type EmailOrLdapLoginIdAndPassword = Pick< + LoginRequestDto, + 'emailOrLdapLoginId' | 'password' +>; + +export type MfaCodeOrMfaRecoveryCode = Pick; const usersStore = useUsersStore(); const settingsStore = useSettingsStore(); @@ -29,7 +37,7 @@ const telemetry = useTelemetry(); const loading = ref(false); const showMfaView = ref(false); -const email = ref(''); +const emailOrLdapLoginId = ref(''); const password = ref(''); const reportError = ref(false); @@ -50,7 +58,7 @@ const formConfig: IFormBoxConfig = reactive({ redirectLink: '/forgot-password', inputs: [ { - name: 'email', + name: 'emailOrLdapLoginId', properties: { label: emailLabel.value, type: 'email', @@ -78,23 +86,16 @@ const formConfig: IFormBoxConfig = reactive({ ], }); -const onMFASubmitted = async (form: { mfaCode?: string; mfaRecoveryCode?: string }) => { +const onMFASubmitted = async (form: MfaCodeOrMfaRecoveryCode) => { await login({ - email: email.value, + emailOrLdapLoginId: emailOrLdapLoginId.value, password: password.value, mfaCode: form.mfaCode, mfaRecoveryCode: form.mfaRecoveryCode, }); }; -const isFormWithEmailAndPassword = (values: { - [key: string]: string; -}): values is { email: string; password: string } => { - return 'email' in values && 'password' in values; -}; - -const onEmailPasswordSubmitted = async (form: { [key: string]: string }) => { - if (!isFormWithEmailAndPassword(form)) return; +const onEmailPasswordSubmitted = async (form: EmailOrLdapLoginIdAndPassword) => { await login(form); }; @@ -111,16 +112,11 @@ const getRedirectQueryParameter = () => { return redirect; }; -const login = async (form: { - email: string; - password: string; - mfaCode?: string; - mfaRecoveryCode?: string; -}) => { +const login = async (form: LoginRequestDto) => { try { loading.value = true; await usersStore.loginWithCreds({ - email: form.email, + emailOrLdapLoginId: form.emailOrLdapLoginId, password: form.password, mfaCode: form.mfaCode, mfaRecoveryCode: form.mfaRecoveryCode, @@ -185,8 +181,8 @@ const onFormChanged = (toForm: string) => { reportError.value = false; } }; -const cacheCredentials = (form: { email: string; password: string }) => { - email.value = form.email; +const cacheCredentials = (form: EmailOrLdapLoginIdAndPassword) => { + emailOrLdapLoginId.value = form.emailOrLdapLoginId; password.value = form.password; };