diff --git a/packages/@n8n/api-types/src/dto/user/__tests__/user-update-request.dto.test.ts b/packages/@n8n/api-types/src/dto/user/__tests__/user-update-request.dto.test.ts index 0869c4a0f14..9da2c15933d 100644 --- a/packages/@n8n/api-types/src/dto/user/__tests__/user-update-request.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/user/__tests__/user-update-request.dto.test.ts @@ -1,26 +1,10 @@ import { UserUpdateRequestDto } from '../user-update-request.dto'; describe('UserUpdateRequestDto', () => { - it('should fail validation for an invalid email', () => { - const invalidRequest = { - email: 'invalid-email', - firstName: 'John', - lastName: 'Doe', - mfaCode: '123456', - }; - - const result = UserUpdateRequestDto.safeParse(invalidRequest); - - expect(result.success).toBe(false); - expect(result.error?.issues[0].path).toEqual(['email']); - }); - it('should fail validation for a firstName with potential XSS attack', () => { const invalidRequest = { - email: 'test@example.com', firstName: '', lastName: 'Doe', - mfaCode: '123456', }; const result = UserUpdateRequestDto.safeParse(invalidRequest); @@ -31,10 +15,8 @@ describe('UserUpdateRequestDto', () => { it('should fail validation for a firstName with a URL', () => { const invalidRequest = { - email: 'test@example.com', firstName: 'test http://malicious.com', lastName: 'Doe', - mfaCode: '123456', }; const result = UserUpdateRequestDto.safeParse(invalidRequest); @@ -45,10 +27,8 @@ describe('UserUpdateRequestDto', () => { it('should fail validation for a lastName with potential XSS attack', () => { const invalidRequest = { - email: 'test@example.com', firstName: 'John', lastName: '', - mfaCode: '123456', }; const result = UserUpdateRequestDto.safeParse(invalidRequest); @@ -59,10 +39,8 @@ describe('UserUpdateRequestDto', () => { it('should fail validation for a lastName with a URL', () => { const invalidRequest = { - email: 'test@example.com', firstName: 'John', lastName: 'testing http://malicious.com', - mfaCode: '123456', }; const result = UserUpdateRequestDto.safeParse(invalidRequest); @@ -73,14 +51,25 @@ describe('UserUpdateRequestDto', () => { it('should validate a valid user update request', () => { const validRequest = { - email: 'test@example.com', firstName: 'John', lastName: 'Doe', - mfaCode: '123456', }; const result = UserUpdateRequestDto.safeParse(validRequest); expect(result.success).toBe(true); }); + + it('should strip an email field from the request', () => { + const requestWithEmail = { + firstName: 'John', + lastName: 'Doe', + email: 'test@example.com', + }; + + const result = UserUpdateRequestDto.safeParse(requestWithEmail); + + expect(result.success).toBe(true); + expect(result.data).not.toHaveProperty('email'); + }); }); diff --git a/packages/@n8n/api-types/src/dto/user/user-update-request.dto.ts b/packages/@n8n/api-types/src/dto/user/user-update-request.dto.ts index 10393863543..6afc54406e7 100644 --- a/packages/@n8n/api-types/src/dto/user/user-update-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/user/user-update-request.dto.ts @@ -25,12 +25,6 @@ const nameSchema = () => }); export class UserUpdateRequestDto extends Z.class({ - email: z.string().email(), firstName: nameSchema().optional(), lastName: nameSchema().optional(), - mfaCode: z.string().optional(), - /** - * The current password is required when changing the email address and MFA is disabled. - */ - currentPassword: z.string().optional(), }) {} diff --git a/packages/cli/src/controllers/__tests__/me.controller.test.ts b/packages/cli/src/controllers/__tests__/me.controller.test.ts index bdc2c6a6e50..fb4e7f38a32 100644 --- a/packages/cli/src/controllers/__tests__/me.controller.test.ts +++ b/packages/cli/src/controllers/__tests__/me.controller.test.ts @@ -48,7 +48,7 @@ describe('MeController', () => { }); describe('updateCurrentUser', () => { - it('should update the user in the DB, and issue a new cookie', async () => { + it('should update the user name in the DB', async () => { const user = { id: '123', email: 'valid@email.com', @@ -58,7 +58,6 @@ describe('MeController', () => { mfaEnabled: false, } as unknown as User; const payload = new UserUpdateRequestDto({ - email: 'valid@email.com', firstName: 'John', lastName: 'Potato', }); @@ -66,7 +65,6 @@ describe('MeController', () => { const res = mock(); userRepository.findOneByOrFail.mockResolvedValue(user); userService.findUserWithAuthIdentities.mockResolvedValue(user); - vi.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); userService.toPublic.mockResolvedValue({} as unknown as PublicUser); await controller.updateCurrentUser(req, res, payload); @@ -80,18 +78,8 @@ describe('MeController', () => { expect(userService.update).toHaveBeenCalled(); expect(eventService.emit).toHaveBeenCalledWith('user-updated', { user, - fieldsChanged: ['firstName', 'lastName'], // email did not change + fieldsChanged: ['firstName', 'lastName'], }); - expect(res.cookie).toHaveBeenCalledWith( - AUTH_COOKIE_NAME, - 'signed-token', - expect.objectContaining({ - maxAge: expect.any(Number), - httpOnly: true, - sameSite: 'lax', - secure: false, - }), - ); expect(externalHooks.run).toHaveBeenCalledWith('user.profile.update', [ user.email, @@ -99,11 +87,49 @@ describe('MeController', () => { ]); }); + it('should not change the email when an email field is sent', async () => { + const user = { + id: '123', + email: 'valid@email.com', + firstName: 'John', + lastName: 'Doe', + password: 'password', + authIdentities: [], + role: GLOBAL_OWNER_ROLE, + mfaEnabled: false, + } as unknown as User; + const req = { user, browserId } as unknown as AuthenticatedRequest; + const res = mock(); + userRepository.findOneByOrFail.mockResolvedValue(user); + userService.findUserWithAuthIdentities.mockResolvedValue(user); + userService.toPublic.mockResolvedValue({} as unknown as PublicUser); + + // The DTO strips unknown keys, so a stray email never reaches the service. + const payload = new UserUpdateRequestDto({ + firstName: 'Jane', + lastName: 'Doe', + email: 'new@email.com', + } as unknown as UserUpdateRequestDto); + + await controller.updateCurrentUser(req, res, payload); + + expect(userService.update).toHaveBeenCalledWith(user.id, { + firstName: 'Jane', + lastName: 'Doe', + }); + expect(eventService.emit).toHaveBeenCalledWith('user-updated', { + user, + fieldsChanged: ['firstName'], + }); + }); + it('should throw BadRequestError if beforeUpdate hook throws BadRequestError', async () => { const user = { id: '123', password: 'password', email: 'current@email.com', + firstName: 'John', + lastName: 'Doe', authIdentities: [], role: GLOBAL_OWNER_ROLE, mfaEnabled: false, @@ -112,17 +138,17 @@ describe('MeController', () => { externalHooks.run.mockImplementationOnce(async (hookName) => { if (hookName === 'user.profile.beforeUpdate') { - throw new BadRequestError('Invalid email address'); + throw new BadRequestError('Invalid name'); } }); const execution = controller.updateCurrentUser( req, mock(), - mock({ email: user.email, firstName: 'John', lastName: 'Potato' }), + mock({ firstName: 'Jane', lastName: 'Doe' }), ); await expect(execution).rejects.toThrow(BadRequestError); - await expect(execution).rejects.toThrow('Invalid email address'); + await expect(execution).rejects.toThrow('Invalid name'); }); describe('when user is authenticated via LDAP or OIDC', () => { @@ -148,7 +174,6 @@ describe('MeController', () => { req, mock(), new UserUpdateRequestDto({ - email: 'ldap@email.com', firstName: 'Jane', lastName: 'Doe', }), @@ -182,8 +207,7 @@ describe('MeController', () => { req, mock(), new UserUpdateRequestDto({ - email: 'new-oidc@email.com', - firstName: 'John', + firstName: 'Jane', lastName: 'Doe', }), ); @@ -198,14 +222,15 @@ describe('MeController', () => { const user = { id: '123', email: 'valid@email.com', + firstName: 'John', + lastName: 'Doe', password: 'password', authIdentities: [], role: GLOBAL_OWNER_ROLE, mfaEnabled: false, } as unknown as User; const payload = new UserUpdateRequestDto({ - email: 'valid@email.com', - firstName: 'John', + firstName: 'Jane', lastName: 'Potato', }); const req = { user, browserId } as unknown as AuthenticatedRequest; @@ -213,7 +238,6 @@ describe('MeController', () => { userRepository.findOneByOrFail.mockResolvedValue(user); userService.findUserWithAuthIdentities.mockResolvedValue(user); - vi.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); userService.toPublic.mockResolvedValue({} as unknown as PublicUser); await controller.updateCurrentUser(req, res, payload); @@ -244,7 +268,6 @@ describe('MeController', () => { req, mock(), new UserUpdateRequestDto({ - email: 'multi@email.com', firstName: 'Jane', lastName: 'Doe', }), @@ -258,12 +281,9 @@ describe('MeController', () => { }); describe('when an auth_identity exists but the SSO provider is no longer active', () => { - const passwordHash = '$2a$10$ffitcKrHT.Ls.m9FfWrMrOod76aaI0ogKbc3S96Q320impWpCbgj6'; // Hashed 'old_password' - const setUpdateMocks = (user: User) => { userRepository.findOneByOrFail.mockResolvedValue(user); userService.findUserWithAuthIdentities.mockResolvedValue(user); - vi.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); userService.toPublic.mockResolvedValue({} as unknown as PublicUser); }; @@ -289,7 +309,6 @@ describe('MeController', () => { req, mock(), new UserUpdateRequestDto({ - email: 'saml@email.com', firstName: 'Jane', lastName: 'Doe', }), @@ -313,7 +332,6 @@ describe('MeController', () => { mfaEnabled: false, } as unknown as User; const payload = new UserUpdateRequestDto({ - email: 'saml@email.com', firstName: 'NewFirst', lastName: 'NewLast', }); @@ -328,7 +346,6 @@ describe('MeController', () => { await controller.updateCurrentUser(req, res, payload); expect(userService.update).toHaveBeenCalledWith(user.id, { - email: 'saml@email.com', firstName: 'NewFirst', lastName: 'NewLast', }); @@ -350,7 +367,6 @@ describe('MeController', () => { mfaEnabled: false, } as unknown as User; const payload = new UserUpdateRequestDto({ - email: 'ldap@email.com', firstName: 'NewFirst', lastName: 'NewLast', }); @@ -379,7 +395,6 @@ describe('MeController', () => { mfaEnabled: false, } as unknown as User; const payload = new UserUpdateRequestDto({ - email: 'oidc@email.com', firstName: 'NewFirst', lastName: 'NewLast', }); @@ -396,39 +411,6 @@ describe('MeController', () => { expect(userService.update).toHaveBeenCalled(); }); - it('should allow email change for previously-SAML user once SAML is disabled', async () => { - const user = { - id: '123', - email: 'saml-old@email.com', - firstName: 'John', - lastName: 'Doe', - password: passwordHash, - authIdentities: [], - role: GLOBAL_OWNER_ROLE, - mfaEnabled: false, - } as unknown as User; - const req = { user, browserId } as unknown as AuthenticatedRequest; - const res = mock(); - - userService.findSsoIdentity.mockResolvedValue({ - providerType: 'saml', - } as unknown as AuthIdentity); - setUpdateMocks(user); - - await controller.updateCurrentUser( - req, - res, - new UserUpdateRequestDto({ - email: 'saml-new@email.com', - firstName: 'John', - lastName: 'Doe', - currentPassword: 'old_password', - }), - ); - - expect(userService.update).toHaveBeenCalled(); - }); - it('should allow profile update when providerType is token-exchange', async () => { const user = { id: '123', @@ -452,7 +434,6 @@ describe('MeController', () => { req, res, new UserUpdateRequestDto({ - email: 'token@email.com', firstName: 'NewFirst', lastName: 'NewLast', }), @@ -486,7 +467,6 @@ describe('MeController', () => { req, res, new UserUpdateRequestDto({ - email: 'unchanged@email.com', firstName: 'Same', lastName: 'Name', }), @@ -496,221 +476,6 @@ describe('MeController', () => { expect(userService.update).toHaveBeenCalled(); }); }); - - describe('when mfa is enabled', () => { - it('should throw BadRequestError if mfa code is missing', async () => { - const user = { - id: '123', - email: 'valid@email.com', - password: 'password', - authIdentities: [], - role: GLOBAL_OWNER_ROLE, - mfaEnabled: true, - } as unknown as User; - const req = { user, browserId } as unknown as AuthenticatedRequest; - - const execution = controller.updateCurrentUser( - req, - mock(), - new UserUpdateRequestDto({ - email: 'new@email.com', - firstName: 'John', - lastName: 'Potato', - }), - ); - - await expect(execution).rejects.toThrow(BadRequestError); - await expect(execution).rejects.toThrow('Two-factor code is required to change email'); - }); - - it('should throw InvalidMfaCodeError if mfa code is invalid', async () => { - const user = { - id: '123', - email: 'valid@email.com', - password: 'password', - authIdentities: [], - role: GLOBAL_OWNER_ROLE, - mfaEnabled: true, - } as unknown as User; - const req = { user, browserId } as unknown as AuthenticatedRequest; - mockMfaService.validateMfa.mockResolvedValue(false); - - await expect( - controller.updateCurrentUser( - req, - mock(), - mock({ - email: 'new@email.com', - firstName: 'John', - lastName: 'Potato', - mfaCode: 'invalid', - }), - ), - ).rejects.toThrow(InvalidMfaCodeError); - }); - - it("should update the user's email if mfa code is valid", async () => { - const user = { - id: '123', - email: 'valid@email.com', - password: 'password', - authIdentities: [], - role: GLOBAL_OWNER_ROLE, - mfaEnabled: true, - mfaSecret: 'secret', - } as unknown as User; - const req = { user, browserId } as unknown as AuthenticatedRequest; - const res = mock(); - userRepository.findOneByOrFail.mockResolvedValue(user); - userService.findUserWithAuthIdentities.mockResolvedValue(user); - vi.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); - userService.toPublic.mockResolvedValue({} as unknown as PublicUser); - mockMfaService.validateMfa.mockResolvedValue(true); - - const result = await controller.updateCurrentUser( - req, - res, - mock({ - email: 'new@email.com', - firstName: 'John', - lastName: 'Potato', - mfaCode: '123456', - }), - ); - - expect(result).toEqual({}); - }); - }); - - describe('when mfa is disabled and email is being changed', () => { - const oldPasswordPlain = 'old_password'; - const passwordHash = '$2a$10$ffitcKrHT.Ls.m9FfWrMrOod76aaI0ogKbc3S96Q320impWpCbgj6'; // Hashed 'old_password' - - it('should throw BadRequestError if currentPassword is missing', async () => { - const user = { - id: '123', - email: 'michel-old@email.com', - password: passwordHash, - mfaEnabled: false, - } as unknown as User; - const req = { user, browserId } as unknown as AuthenticatedRequest; - - const execution = controller.updateCurrentUser( - req, - mock(), - new UserUpdateRequestDto({ - email: 'michel-new@email.com', - firstName: 'Michel', - lastName: 'n8n', - }), - ); - - await expect(execution).rejects.toThrow(BadRequestError); - await expect(execution).rejects.toThrow('Current password is required to change email'); - }); - - it('should throw BadRequestError if currentPassword is not a string', async () => { - const user = { - id: '123', - email: 'michel-old@email.com', - password: passwordHash, - mfaEnabled: false, - } as unknown as User; - const req = { user, browserId } as unknown as AuthenticatedRequest; - - const execution = controller.updateCurrentUser(req, mock(), { - email: 'michel-new@email.com', - firstName: 'Michel', - lastName: 'n8n', - currentPassword: 123 as any, - } as any); - - await expect(execution).rejects.toThrow(BadRequestError); - await expect(execution).rejects.toThrow('Current password is required to change email'); - }); - - it('should throw BadRequestError if currentPassword is incorrect', async () => { - const user = { - email: 'michel-old@email.com', - password: passwordHash, - mfaEnabled: false, - } as unknown as User; - const req = { user, browserId } as unknown as AuthenticatedRequest; - - const execution = controller.updateCurrentUser( - req, - mock(), - mock({ - email: 'michel-new@email.com', - firstName: 'Michel', - lastName: 'n8n', - currentPassword: 'wrong-password', - }), - ); - - await expect(execution).rejects.toThrow(BadRequestError); - await expect(execution).rejects.toThrow( - 'Unable to update profile. Please check your credentials and try again.', - ); - }); - - it('should update the user email if currentPassword is correct', async () => { - const user = { - email: 'michel-old@email.com', - password: passwordHash, - mfaEnabled: false, - role: GLOBAL_OWNER_ROLE, - } as unknown as User; - const req = { user, browserId } as unknown as AuthenticatedRequest; - const res = mock(); - userRepository.findOneByOrFail.mockResolvedValue(user); - userService.findUserWithAuthIdentities.mockResolvedValue(user); - vi.spyOn(jwt, 'sign').mockImplementation(() => 'new-signed-token'); - userService.toPublic.mockResolvedValue({} as unknown as PublicUser); - - const result = await controller.updateCurrentUser( - req, - res, - mock({ - email: 'michel-new@email.com', - firstName: 'Michel', - lastName: 'n8n', - currentPassword: oldPasswordPlain, - }), - ); - - expect(userService.update).toHaveBeenCalled(); - expect(result).toEqual({}); - }); - - it('should not require currentPassword when email is not being changed', async () => { - const user = { - email: 'michel@email.com', - password: passwordHash, - mfaEnabled: false, - role: GLOBAL_OWNER_ROLE, - } as unknown as User; - const req = { user, browserId } as unknown as AuthenticatedRequest; - const res = mock(); - userRepository.findOneByOrFail.mockResolvedValue(user); - userService.findUserWithAuthIdentities.mockResolvedValue(user); - vi.spyOn(jwt, 'sign').mockImplementation(() => 'new-signed-token'); - userService.toPublic.mockResolvedValue({} as unknown as PublicUser); - - const result = await controller.updateCurrentUser( - req, - res, - new UserUpdateRequestDto({ - email: 'michel@email.com', - firstName: 'Michel', - lastName: 'n8n', - }), - ); - - expect(userService.update).toHaveBeenCalled(); - expect(result).toEqual({}); - }); - }); }); describe('when user is managed by env', () => { diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index 573195fb48b..46cd1bc5c83 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -21,7 +21,6 @@ import { ExternalHooks } from '@/external-hooks'; import { validateEntity } from '@/generic-helpers'; import { MfaService } from '@/mfa/mfa.service'; import { MeRequest } from '@/requests'; -import { EmailChangeService } from '@/services/email-change.service'; import { PasswordUtility } from '@/services/password.utility'; import { UserService } from '@/services/user.service'; import { getCurrentAuthenticationMethod, isSamlLicensedAndEnabled } from '@/sso.ee/sso-helpers'; @@ -40,7 +39,6 @@ export class MeController { private readonly eventService: EventService, private readonly mfaService: MfaService, private readonly globalConfig: GlobalConfig, - private readonly emailChangeService: EmailChangeService, ) {} /** @@ -49,7 +47,7 @@ export class MeController { @Patch('/') async updateCurrentUser( req: AuthenticatedRequest, - res: Response, + _: Response, @Body payload: UserUpdateRequestDto, ): Promise { const { @@ -65,14 +63,12 @@ export class MeController { ); } - const { currentPassword, ...payloadWithoutPassword } = payload; - const { email, firstName, lastName } = payload; - const isEmailBeingChanged = email !== currentEmail; + const { firstName, lastName } = payload; const isFirstNameChanged = firstName !== currentFirstName; const isLastNameChanged = lastName !== currentLastName; // Check if the user is authenticated via SSO - they cannot change their profile info - if (isEmailBeingChanged || isFirstNameChanged || isLastNameChanged) { + if (isFirstNameChanged || isLastNameChanged) { const ssoIdentity = await this.userService.findSsoIdentity(userId); if (ssoIdentity && this.isAuthIdentityActive(ssoIdentity)) { @@ -80,7 +76,7 @@ export class MeController { `Request to update user failed because ${ssoIdentity.providerType} user may not change their profile information`, { userId, - payload: payloadWithoutPassword, + payload, }, ); throw new BadRequestError( @@ -89,28 +85,15 @@ export class MeController { } } - if (isEmailBeingChanged) { - await this.emailChangeService.assertMayRequestEmailChange(req.user, { - currentPassword, - mfaCode: payload.mfaCode, - }); - } - - await this.externalHooks.run('user.profile.beforeUpdate', [ - userId, - currentEmail, - payloadWithoutPassword, - ]); + await this.externalHooks.run('user.profile.beforeUpdate', [userId, currentEmail, payload]); const preUpdateUser = await this.userRepository.findOneByOrFail({ id: userId }); - await this.userService.update(userId, payloadWithoutPassword); + await this.userService.update(userId, payload); const user = await this.userService.findUserWithAuthIdentities(userId); this.logger.info('User updated successfully', { userId }); - this.authService.issueCookie(res, user, req.authInfo?.usedMfa ?? false, req.browserId); - - const changeableFields = ['email', 'firstName', 'lastName'] as const; + const changeableFields = ['firstName', 'lastName'] as const; const fieldsChanged = changeableFields.filter( (key) => key in payload && payload[key] !== preUpdateUser[key], ); diff --git a/packages/cli/src/external-hooks.ts b/packages/cli/src/external-hooks.ts index 879bcca37d9..e9b385bef22 100644 --- a/packages/cli/src/external-hooks.ts +++ b/packages/cli/src/external-hooks.ts @@ -1,4 +1,4 @@ -import type { FrontendSettings, UserUpdateRequestDto } from '@n8n/api-types'; +import type { FrontendSettings } from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; import type { ClientOAuth2Options } from '@n8n/client-oauth2'; import { GlobalConfig } from '@n8n/config'; @@ -104,7 +104,7 @@ type ExternalHooksMap = { 'user.profile.beforeUpdate': [ userId: string, currentEmail: string, - payload: UserUpdateRequestDto, + payload: { email?: string; firstName?: string; lastName?: string }, ]; 'user.profile.update': [currentEmail: string, publicUser: PublicUser]; 'user.password.update': [updatedEmail: string, updatedPassword: string | null]; diff --git a/packages/cli/test/integration/me.api.test.ts b/packages/cli/test/integration/me.api.test.ts index fb6b139f918..32db673a779 100644 --- a/packages/cli/test/integration/me.api.test.ts +++ b/packages/cli/test/integration/me.api.test.ts @@ -49,9 +49,9 @@ describe('Owner shell', () => { response.body.data; expect(validator.isUUID(id)).toBe(true); - expect(email).toBe(validPayload.email.toLowerCase()); - expect(firstName).toBe(validPayload.firstName); - expect(lastName).toBe(validPayload.lastName); + expect(email).toBeNull(); + if (validPayload.firstName !== undefined) expect(firstName).toBe(validPayload.firstName); + if (validPayload.lastName !== undefined) expect(lastName).toBe(validPayload.lastName); expect(personalizationAnswers).toBeNull(); expect(password).toBeUndefined(); expect(isPending).toBe(false); @@ -59,15 +59,13 @@ describe('Owner shell', () => { const storedOwnerShell = await Container.get(UserRepository).findOneByOrFail({ id }); - expect(storedOwnerShell.email).toBe(validPayload.email.toLowerCase()); - expect(storedOwnerShell.firstName).toBe(validPayload.firstName); - expect(storedOwnerShell.lastName).toBe(validPayload.lastName); - - const storedPersonalProject = await Container.get( - ProjectRepository, - ).getPersonalProjectForUserOrFail(storedOwnerShell.id); - - expect(storedPersonalProject.name).toBe(storedOwnerShell.createPersonalProjectName()); + expect(storedOwnerShell.email).toBeNull(); + if (validPayload.firstName !== undefined) { + expect(storedOwnerShell.firstName).toBe(validPayload.firstName); + } + if (validPayload.lastName !== undefined) { + expect(storedOwnerShell.lastName).toBe(validPayload.lastName); + } } }); @@ -147,16 +145,16 @@ describe('Member', () => { }); test('PATCH /me should succeed with valid inputs', async () => { - for (const validPayload of getValidPatchMePayloads('member')) { + for (const validPayload of VALID_PATCH_ME_PAYLOADS) { const response = await authMemberAgent.patch('/me').send(validPayload).expect(200); const { id, email, firstName, lastName, personalizationAnswers, role, password, isPending } = response.body.data; expect(validator.isUUID(id)).toBe(true); - expect(email).toBe(validPayload.email.toLowerCase()); - expect(firstName).toBe(validPayload.firstName); - expect(lastName).toBe(validPayload.lastName); + expect(email).toBe(member.email); + if (validPayload.firstName !== undefined) expect(firstName).toBe(validPayload.firstName); + if (validPayload.lastName !== undefined) expect(lastName).toBe(validPayload.lastName); expect(personalizationAnswers).toBeNull(); expect(password).toBeUndefined(); expect(isPending).toBe(false); @@ -164,19 +162,30 @@ describe('Member', () => { const storedMember = await Container.get(UserRepository).findOneByOrFail({ id }); - expect(storedMember.email).toBe(validPayload.email.toLowerCase()); - expect(storedMember.firstName).toBe(validPayload.firstName); - expect(storedMember.lastName).toBe(validPayload.lastName); - - const storedPersonalProject = - await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(id); - - expect(storedPersonalProject.name).toBe(storedMember.createPersonalProjectName()); + expect(storedMember.email).toBe(member.email); + if (validPayload.firstName !== undefined) { + expect(storedMember.firstName).toBe(validPayload.firstName); + } + if (validPayload.lastName !== undefined) { + expect(storedMember.lastName).toBe(validPayload.lastName); + } } }); + test('PATCH /me should not change the email when an email field is sent', async () => { + const response = await authMemberAgent + .patch('/me') + .send({ email: randomEmail(), firstName: randomName(), lastName: randomName() }); + + expect(response.statusCode).toBe(200); + expect(response.body.data.email).toBe(member.email); + + const storedMember = await Container.get(UserRepository).findOneByOrFail({ id: member.id }); + expect(storedMember.email).toBe(member.email); + }); + test('PATCH /me should fail with invalid inputs', async () => { - for (const invalidPayload of getInvalidPatchMePayloads('member')) { + for (const invalidPayload of INVALID_PATCH_ME_PAYLOADS) { const response = await authMemberAgent.patch('/me').send(invalidPayload); expect(response.statusCode).toBe(400); @@ -193,39 +202,6 @@ describe('Member', () => { } }); - test('PATCH /me should fail when changing email without currentPassword', async () => { - const payloadWithoutPassword = { - email: randomEmail(), - firstName: randomName(), - lastName: randomName(), - }; - - const response = await authMemberAgent.patch('/me').send(payloadWithoutPassword); - expect(response.statusCode).toBe(400); - expect(response.body.message).toContain('Current password is required to change email'); - - const storedMember = await Container.get(UserRepository).findOneByOrFail({}); - expect(storedMember.email).toBe(member.email); - }); - - test('PATCH /me should fail when changing email with wrong currentPassword', async () => { - const payloadWithWrongPassword = { - email: randomEmail(), - firstName: randomName(), - lastName: randomName(), - currentPassword: 'WrongPassword123', - }; - - const response = await authMemberAgent.patch('/me').send(payloadWithWrongPassword); - expect(response.statusCode).toBe(400); - expect(response.body.message).toContain( - 'Unable to update profile. Please check your credentials and try again.', - ); - - const storedMember = await Container.get(UserRepository).findOneByOrFail({}); - expect(storedMember.email).toBe(member.email); - }); - test('PATCH /me/password should succeed with valid inputs', async () => { const validPayload = { currentPassword: memberPassword, @@ -343,16 +319,16 @@ describe('Chat User', () => { }); test('PATCH /me should succeed with valid inputs', async () => { - for (const validPayload of getValidPatchMePayloads('chatUser')) { + for (const validPayload of VALID_PATCH_ME_PAYLOADS) { const response = await authMemberAgent.patch('/me').send(validPayload).expect(200); const { id, email, firstName, lastName, personalizationAnswers, role, password, isPending } = response.body.data; expect(validator.isUUID(id)).toBe(true); - expect(email).toBe(validPayload.email.toLowerCase()); - expect(firstName).toBe(validPayload.firstName); - expect(lastName).toBe(validPayload.lastName); + expect(email).toBe(member.email); + if (validPayload.firstName !== undefined) expect(firstName).toBe(validPayload.firstName); + if (validPayload.lastName !== undefined) expect(lastName).toBe(validPayload.lastName); expect(personalizationAnswers).toBeNull(); expect(password).toBeUndefined(); expect(isPending).toBe(false); @@ -360,19 +336,18 @@ describe('Chat User', () => { const storedMember = await Container.get(UserRepository).findOneByOrFail({ id }); - expect(storedMember.email).toBe(validPayload.email.toLowerCase()); - expect(storedMember.firstName).toBe(validPayload.firstName); - expect(storedMember.lastName).toBe(validPayload.lastName); - - const storedPersonalProject = - await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(id); - - expect(storedPersonalProject.name).toBe(storedMember.createPersonalProjectName()); + expect(storedMember.email).toBe(member.email); + if (validPayload.firstName !== undefined) { + expect(storedMember.firstName).toBe(validPayload.firstName); + } + if (validPayload.lastName !== undefined) { + expect(storedMember.lastName).toBe(validPayload.lastName); + } } }); test('PATCH /me should fail with invalid inputs', async () => { - for (const invalidPayload of getInvalidPatchMePayloads('chatUser')) { + for (const invalidPayload of INVALID_PATCH_ME_PAYLOADS) { const response = await authMemberAgent.patch('/me').send(invalidPayload); expect(response.statusCode).toBe(400); @@ -389,39 +364,6 @@ describe('Chat User', () => { } }); - test('PATCH /me should fail when changing email without currentPassword', async () => { - const payloadWithoutPassword = { - email: randomEmail(), - firstName: randomName(), - lastName: randomName(), - }; - - const response = await authMemberAgent.patch('/me').send(payloadWithoutPassword); - expect(response.statusCode).toBe(400); - expect(response.body.message).toContain('Current password is required to change email'); - - const storedMember = await Container.get(UserRepository).findOneByOrFail({}); - expect(storedMember.email).toBe(member.email); - }); - - test('PATCH /me should fail when changing email with wrong currentPassword', async () => { - const payloadWithWrongPassword = { - email: randomEmail(), - firstName: randomName(), - lastName: randomName(), - currentPassword: 'WrongPassword123', - }; - - const response = await authMemberAgent.patch('/me').send(payloadWithWrongPassword); - expect(response.statusCode).toBe(400); - expect(response.body.message).toContain( - 'Unable to update profile. Please check your credentials and try again.', - ); - - const storedMember = await Container.get(UserRepository).findOneByOrFail({}); - expect(storedMember.email).toBe(member.email); - }); - test('PATCH /me/password should succeed with valid inputs', async () => { const validPayload = { currentPassword: memberPassword, @@ -479,7 +421,7 @@ describe('Owner', () => { }); const authOwnerAgent = testServer.authAgentFor(owner); - for (const validPayload of getValidPatchMePayloads('owner')) { + for (const validPayload of VALID_PATCH_ME_PAYLOADS) { const response = await authOwnerAgent.patch('/me').send(validPayload); expect(response.statusCode).toBe(200); @@ -497,9 +439,9 @@ describe('Owner', () => { } = response.body.data; expect(validator.isUUID(id)).toBe(true); - expect(email).toBe(validPayload.email.toLowerCase()); - expect(firstName).toBe(validPayload.firstName); - expect(lastName).toBe(validPayload.lastName); + expect(email).toBe(owner.email); + if (validPayload.firstName !== undefined) expect(firstName).toBe(validPayload.firstName); + if (validPayload.lastName !== undefined) expect(lastName).toBe(validPayload.lastName); expect(personalizationAnswers).toBeNull(); expect(password).toBeUndefined(); expect(isPending).toBe(false); @@ -508,15 +450,13 @@ describe('Owner', () => { const storedOwner = await Container.get(UserRepository).findOneByOrFail({ id }); - expect(storedOwner.email).toBe(validPayload.email.toLowerCase()); - expect(storedOwner.firstName).toBe(validPayload.firstName); - expect(storedOwner.lastName).toBe(validPayload.lastName); - - const storedPersonalProject = await Container.get( - ProjectRepository, - ).getPersonalProjectForUserOrFail(storedOwner.id); - - expect(storedPersonalProject.name).toBe(storedOwner.createPersonalProjectName()); + expect(storedOwner.email).toBe(owner.email); + if (validPayload.firstName !== undefined) { + expect(storedOwner.firstName).toBe(validPayload.firstName); + } + if (validPayload.lastName !== undefined) { + expect(storedOwner.lastName).toBe(validPayload.lastName); + } } }); }); @@ -547,75 +487,37 @@ const EMPTY_SURVEY: IPersonalizationSurveyAnswersV4 = { personalization_survey_n8n_version: '1.0.0', }; -function getValidPatchMePayloads(userType: 'owner' | 'member' | 'chatUser') { - return VALID_PATCH_ME_PAYLOADS.map((payload) => { - if (userType === 'owner') { - return { ...payload, currentPassword: ownerPassword }; - } - return { ...payload, currentPassword: memberPassword }; - }); -} - -function getInvalidPatchMePayloads(userType: 'owner' | 'member' | 'chatUser') { - return INVALID_PATCH_ME_PAYLOADS.map((payload) => { - if (userType === 'owner') { - return { ...payload, currentPassword: ownerPassword }; - } - return { ...payload, currentPassword: memberPassword }; - }); -} - const VALID_PATCH_ME_PAYLOADS = [ { - email: randomEmail(), firstName: randomName(), lastName: randomName(), }, - // { - // email: randomEmail().toUpperCase(), - // firstName: randomName(), - // lastName: randomName(), - // }, + { + firstName: randomName(), + }, + { + lastName: randomName(), + }, ]; const INVALID_PATCH_ME_PAYLOADS = [ { - email: 'invalid', - firstName: randomName(), - lastName: randomName(), - }, - { - email: randomEmail(), firstName: '', lastName: randomName(), }, { - email: randomEmail(), firstName: randomName(), lastName: '', }, { - email: randomEmail(), firstName: 123, lastName: randomName(), }, { - firstName: randomName(), - lastName: randomName(), - }, - { - firstName: randomName(), - }, - { - lastName: randomName(), - }, - { - email: randomEmail(), firstName: 'John { }) .expect(200); }); + }); + describe('POST /change-email', () => { test('should throw BadRequestError if email is changed when SAML is enabled', async () => { await enableSaml(true); await authOwnerAgent - .patch('/me') - .send({ - email: randomEmail(), - firstName: randomName(), - lastName: randomName(), - }) + .post('/change-email') + .send({ email: randomEmail() }) .expect(400, { code: 400, message: 'SAML user may not change their email' }); }); + + test('should allow a user with a SAML auth_identity to change email once SAML is disabled', async () => { + // The SAML identity is still attached but inactive, so the SSO guard must pass. + await enableSaml(false); + const newEmail = randomEmail(); + + await authSamlUserAgent + .post('/change-email') + .send({ email: newEmail, currentPassword: samlUserPassword }) + .expect(200); + + const refreshed = await Container.get(UserRepository).findOneByOrFail({ id: samlUser.id }); + expect(refreshed.email).toBe(newEmail); + samlUser.email = newEmail; + }); }); describe('PATCH /me for a user with a SAML auth_identity', () => { @@ -148,25 +161,6 @@ describe('Instance owner', () => { samlUser.firstName = newFirstName; samlUser.lastName = newLastName; }); - - test('should allow email change once SAML is disabled', async () => { - await enableSaml(false); - const newEmail = randomEmail(); - - await authSamlUserAgent - .patch('/me') - .send({ - email: newEmail, - firstName: samlUser.firstName, - lastName: samlUser.lastName, - currentPassword: samlUserPassword, - }) - .expect(200); - - const refreshed = await Container.get(UserRepository).findOneByOrFail({ id: samlUser.id }); - expect(refreshed.email).toBe(newEmail); - samlUser.email = newEmail; - }); }); describe('PATCH /password', () => { diff --git a/packages/frontend/@n8n/frontend-constants/src/views.ts b/packages/frontend/@n8n/frontend-constants/src/views.ts index 65a5d51fc73..df1110217b1 100644 --- a/packages/frontend/@n8n/frontend-constants/src/views.ts +++ b/packages/frontend/@n8n/frontend-constants/src/views.ts @@ -40,6 +40,7 @@ export enum VIEWS { SETUP = 'SetupView', FORGOT_PASSWORD = 'ForgotMyPasswordView', CHANGE_PASSWORD = 'ChangePasswordView', + CONFIRM_EMAIL_CHANGE = 'ConfirmEmailChangeView', SETTINGS = 'Settings', USERS_SETTINGS = 'UsersSettings', LDAP_SETTINGS = 'LdapSettings', diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index dd4a303a120..f3a2fe9747d 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -219,6 +219,14 @@ "auth.changePassword.passwordsMustMatchError": "Passwords must match", "auth.changePassword.reenterNewPassword": "Re-enter new password", "auth.changePassword.tokenValidationError": "Invalid password-reset token", + "auth.confirmEmailChange.title": "Confirm email change", + "auth.confirmEmailChange.message": "Confirm the change of your email address to {newEmail}.", + "auth.confirmEmailChange.button": "Confirm email change", + "auth.confirmEmailChange.success.title": "Email changed", + "auth.confirmEmailChange.success.message": "You can now sign in with your new email address", + "auth.confirmEmailChange.error": "Problem confirming the email change", + "auth.confirmEmailChange.tokenValidationError": "Invalid or expired email-change link", + "auth.confirmEmailChange.missingTokenError": "Missing token", "auth.confirmPassword": "Confirm password", "auth.confirmPassword.currentPassword": "Current password", "auth.confirmPassword.confirmPasswordToChangeEmail": "Please confirm your password in order to change your email address.", @@ -3802,6 +3810,8 @@ "settings.personal.personalSettings": "Personal Settings", "settings.personal.personalSettingsUpdated": "Personal details updated", "settings.personal.personalSettingsUpdatedError": "Problem updating your details", + "settings.personal.emailChange.confirmationSent.title": "Confirm your email change", + "settings.personal.emailChange.confirmationSent.message": "Check your current inbox for a link to confirm the change.", "settings.personal.role.tooltip.default": "Default role for new users", "settings.personal.role.tooltip.member": "Create and manage own workflows and credentials", "settings.personal.role.tooltip.chatUser": "Access to Chat feature only", diff --git a/packages/frontend/@n8n/rest-api-client/src/api/users.ts b/packages/frontend/@n8n/rest-api-client/src/api/users.ts index 4df3607e7b5..8bc857575cd 100644 --- a/packages/frontend/@n8n/rest-api-client/src/api/users.ts +++ b/packages/frontend/@n8n/rest-api-client/src/api/users.ts @@ -1,4 +1,7 @@ import type { + ChangeEmailRequestDto, + ChangeEmailResponse, + ConfirmEmailChangeRequestDto, LoginRequestDto, PasswordUpdateRequestDto, SettingsUpdateRequestDto, @@ -150,6 +153,27 @@ export async function updateCurrentUser( return await makeRestApiRequest(context, 'PATCH', '/me', params); } +export async function requestEmailChange( + context: IRestApiContext, + params: ChangeEmailRequestDto, +): Promise { + return await makeRestApiRequest(context, 'POST', '/change-email', params); +} + +export async function resolveEmailChangeToken( + context: IRestApiContext, + params: { token: string }, +): Promise<{ email: string }> { + return await makeRestApiRequest(context, 'GET', '/resolve-change-email-token', params); +} + +export async function confirmEmailChange( + context: IRestApiContext, + params: ConfirmEmailChangeRequestDto, +): Promise<{ success: true }> { + return await makeRestApiRequest(context, 'POST', '/confirm-email-change', params); +} + export async function updateCurrentUserSettings( context: IRestApiContext, settings: UserSelfSettingsUpdateRequestDto, diff --git a/packages/frontend/@n8n/stores/src/users.store.test.ts b/packages/frontend/@n8n/stores/src/users.store.test.ts index f5075e1f0b5..ed583027ec7 100644 --- a/packages/frontend/@n8n/stores/src/users.store.test.ts +++ b/packages/frontend/@n8n/stores/src/users.store.test.ts @@ -6,7 +6,18 @@ import { createPinia, setActivePinia } from 'pinia'; import { useSettingsStore } from './settings.store'; import { useUsersStore } from './users.store'; -const { loginCurrentUser, inviteUsers, login, logout, getUsers, oidcLogout } = vi.hoisted(() => { +const { + loginCurrentUser, + inviteUsers, + login, + logout, + getUsers, + oidcLogout, + updateCurrentUser, + requestEmailChange, + resolveEmailChangeToken, + confirmEmailChange, +} = vi.hoisted(() => { return { loginCurrentUser: vi.fn(), identify: vi.fn(), @@ -15,6 +26,10 @@ const { loginCurrentUser, inviteUsers, login, logout, getUsers, oidcLogout } = v logout: vi.fn(), getUsers: vi.fn(), oidcLogout: vi.fn(), + updateCurrentUser: vi.fn(), + requestEmailChange: vi.fn(), + resolveEmailChangeToken: vi.fn(), + confirmEmailChange: vi.fn(), }; }); @@ -23,6 +38,10 @@ vi.mock('@n8n/rest-api-client/api/users', () => ({ login, logout, getUsers, + updateCurrentUser, + requestEmailChange, + resolveEmailChangeToken, + confirmEmailChange, })); vi.mock('@n8n/rest-api-client/api/sso', () => ({ @@ -36,6 +55,7 @@ vi.mock('./invitation.api', () => ({ vi.mock('./useRootStore', () => ({ useRootStore: vi.fn(() => ({ instanceId: 'test-instance-id', + restApiContext: { baseUrl: 'http://localhost', pushRef: '' }, })), })); @@ -433,6 +453,71 @@ describe('users.store', () => { }); }); + describe('updateUserName', () => { + it('sends only the name to PATCH /me', async () => { + const usersStore = useUsersStore(); + loginCurrentUser.mockResolvedValueOnce(mockUser); + await usersStore.loginWithCookie(); + updateCurrentUser.mockResolvedValueOnce({ ...mockUser, firstName: 'Jane' }); + + await usersStore.updateUserName({ firstName: 'Jane', lastName: 'Doe' }); + + expect(updateCurrentUser).toHaveBeenCalledWith(expect.anything(), { + firstName: 'Jane', + lastName: 'Doe', + }); + }); + }); + + describe('requestEmailChange', () => { + it('passes the params through and returns the response', async () => { + const usersStore = useUsersStore(); + requestEmailChange.mockResolvedValueOnce({ status: 'confirmation-sent' }); + + const result = await usersStore.requestEmailChange({ + email: 'new@email.com', + currentPassword: 'secret', + }); + + expect(requestEmailChange).toHaveBeenCalledWith(expect.anything(), { + email: 'new@email.com', + currentPassword: 'secret', + }); + expect(result).toEqual({ status: 'confirmation-sent' }); + }); + + it('adds the user to the store when the change applies immediately', async () => { + const usersStore = useUsersStore(); + const changedUser = { id: '2', firstName: 'Jane', role: 'global:member', email: 'x@y.com' }; + requestEmailChange.mockResolvedValueOnce({ status: 'changed', user: changedUser }); + + await usersStore.requestEmailChange({ email: 'x@y.com', currentPassword: 'secret' }); + + expect(usersStore.usersById['2']).toMatchObject({ id: '2', email: 'x@y.com' }); + }); + }); + + describe('resolveEmailChangeToken / confirmEmailChange', () => { + it('resolves the token via the API', async () => { + const usersStore = useUsersStore(); + resolveEmailChangeToken.mockResolvedValueOnce({ email: 'new@email.com' }); + + const result = await usersStore.resolveEmailChangeToken({ token: 'abc' }); + + expect(resolveEmailChangeToken).toHaveBeenCalledWith(expect.anything(), { token: 'abc' }); + expect(result).toEqual({ email: 'new@email.com' }); + }); + + it('confirms the change via the API', async () => { + const usersStore = useUsersStore(); + confirmEmailChange.mockResolvedValueOnce({ success: true }); + + await usersStore.confirmEmailChange({ token: 'abc' }); + + expect(confirmEmailChange).toHaveBeenCalledWith(expect.anything(), { token: 'abc' }); + }); + }); + describe('fetchUsers', () => { it('does not fetch when the injected permission check denies listing (default)', async () => { const usersStore = useUsersStore(); diff --git a/packages/frontend/@n8n/stores/src/users.store.ts b/packages/frontend/@n8n/stores/src/users.store.ts index fe667cfa781..91df3e38bb5 100644 --- a/packages/frontend/@n8n/stores/src/users.store.ts +++ b/packages/frontend/@n8n/stores/src/users.store.ts @@ -1,4 +1,7 @@ import { + type ChangeEmailRequestDto, + type ChangeEmailResponse, + type ConfirmEmailChangeRequestDto, type LoginRequestDto, type PasswordUpdateRequestDto, type SettingsUpdateRequestDto, @@ -336,10 +339,26 @@ export const useUsersStore = defineStore(STORES.USERS, () => { return; } - return await updateUser({ - email: currentUser.value.email as string, - ...params, - }); + return await updateUser(params); + }; + + const requestEmailChange = async ( + params: ChangeEmailRequestDto, + ): Promise => { + const result = await usersApi.requestEmailChange(rootStore.restApiContext, params); + // Without email delivery the backend applies the change at once, so sync the store. + if (result.status === 'changed') { + addUsers([result.user]); + } + return result; + }; + + const resolveEmailChangeToken = async (params: { token: string }) => { + return await usersApi.resolveEmailChangeToken(rootStore.restApiContext, params); + }; + + const confirmEmailChange = async (params: ConfirmEmailChangeRequestDto) => { + await usersApi.confirmEmailChange(rootStore.restApiContext, params); }; const updateUserSettings = async (settings: UserSelfSettingsUpdateRequestDto) => { @@ -529,6 +548,9 @@ export const useUsersStore = defineStore(STORES.USERS, () => { changePassword, updateUser, updateUserName, + requestEmailChange, + resolveEmailChangeToken, + confirmEmailChange, updateUserSettings, updateOtherUserSettings, updateCurrentUserPassword, diff --git a/packages/frontend/editor-ui/src/app/router.ts b/packages/frontend/editor-ui/src/app/router.ts index 017815f14c1..707edf8aedc 100644 --- a/packages/frontend/editor-ui/src/app/router.ts +++ b/packages/frontend/editor-ui/src/app/router.ts @@ -34,6 +34,8 @@ import { const ChangePasswordView = async () => await import('@/features/core/auth/views/ChangePasswordView.vue'); +const ConfirmEmailChangeView = async () => + await import('@/features/core/auth/views/ConfirmEmailChangeView.vue'); const ErrorView = async () => await import('@/app/views/ErrorView.vue'); const EntityNotFound = async () => await import('@/app/views/EntityNotFound.vue'); const EntityUnAuthorised = async () => await import('@/app/views/EntityUnAuthorised.vue'); @@ -600,6 +602,19 @@ export const routes: RouteRecordRaw[] = [ }, }, }, + { + path: '/confirm-email-change', + name: VIEWS.CONFIRM_EMAIL_CHANGE, + component: ConfirmEmailChangeView, + // No auth middleware: the token authorizes the change, so the link works + // whether the user is signed in or not (the confirm endpoint is skipAuth). + meta: { + layout: 'auth', + telemetry: { + pageCategory: 'auth', + }, + }, + }, { path: '/settings', name: VIEWS.SETTINGS, diff --git a/packages/frontend/editor-ui/src/features/core/auth/views/ConfirmEmailChangeView.test.ts b/packages/frontend/editor-ui/src/features/core/auth/views/ConfirmEmailChangeView.test.ts new file mode 100644 index 00000000000..bf336ee529e --- /dev/null +++ b/packages/frontend/editor-ui/src/features/core/auth/views/ConfirmEmailChangeView.test.ts @@ -0,0 +1,95 @@ +import { createComponentRenderer } from '@/__tests__/render'; +import { mockedStore } from '@/__tests__/utils'; +import { createTestingPinia } from '@pinia/testing'; +import { fireEvent, waitFor } from '@testing-library/vue'; +import { VIEWS } from '@/app/constants'; +import ConfirmEmailChangeView from './ConfirmEmailChangeView.vue'; +import { useUsersStore } from '@n8n/stores/users.store'; + +const { push, replace, routerState } = vi.hoisted(() => ({ + push: vi.fn(), + replace: vi.fn(), + routerState: { token: 'test-token' as string | undefined }, +})); + +vi.mock('vue-router', () => ({ + useRouter: () => ({ + push, + replace, + currentRoute: { value: { query: { token: routerState.token } } }, + }), + RouterLink: { template: '' }, +})); + +const { showError, showMessage } = vi.hoisted(() => ({ + showError: vi.fn(), + showMessage: vi.fn(), +})); + +vi.mock('@n8n/composables/useToast', () => ({ + useToast: () => ({ showError, showMessage }), +})); + +const renderComponent = createComponentRenderer(ConfirmEmailChangeView); + +describe('ConfirmEmailChangeView', () => { + beforeEach(() => { + vi.clearAllMocks(); + routerState.token = 'test-token'; + }); + + it('should resolve the token and render the confirm card', async () => { + const pinia = createTestingPinia(); + const usersStore = mockedStore(useUsersStore); + usersStore.resolveEmailChangeToken.mockResolvedValue({ email: 'new@email.com' }); + + const { findByTestId } = renderComponent({ pinia }); + + expect(await findByTestId('confirm-email-change')).toHaveTextContent('new@email.com'); + expect(usersStore.resolveEmailChangeToken).toHaveBeenCalledWith({ token: 'test-token' }); + }); + + it('should confirm the change and redirect to sign-in on submit', async () => { + const pinia = createTestingPinia(); + const usersStore = mockedStore(useUsersStore); + usersStore.resolveEmailChangeToken.mockResolvedValue({ email: 'new@email.com' }); + usersStore.confirmEmailChange.mockResolvedValue(undefined); + usersStore.logout.mockResolvedValue({ redirectUrl: null }); + + const { findByTestId } = renderComponent({ pinia }); + + const button = await findByTestId('confirm-email-change-button'); + await fireEvent.click(button); + + await waitFor(() => { + expect(usersStore.confirmEmailChange).toHaveBeenCalledWith({ token: 'test-token' }); + expect(usersStore.logout).toHaveBeenCalled(); + expect(push).toHaveBeenCalledWith({ name: VIEWS.SIGNIN }); + }); + }); + + it('should redirect to sign-in when the token is invalid', async () => { + const pinia = createTestingPinia(); + const usersStore = mockedStore(useUsersStore); + usersStore.resolveEmailChangeToken.mockRejectedValue(new Error('invalid')); + + renderComponent({ pinia }); + + await waitFor(() => { + expect(replace).toHaveBeenCalledWith({ name: VIEWS.SIGNIN }); + }); + }); + + it('should redirect to sign-in when the token is missing', async () => { + routerState.token = undefined; + const pinia = createTestingPinia(); + const usersStore = mockedStore(useUsersStore); + + renderComponent({ pinia }); + + await waitFor(() => { + expect(replace).toHaveBeenCalledWith({ name: VIEWS.SIGNIN }); + }); + expect(usersStore.resolveEmailChangeToken).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/core/auth/views/ConfirmEmailChangeView.vue b/packages/frontend/editor-ui/src/features/core/auth/views/ConfirmEmailChangeView.vue new file mode 100644 index 00000000000..b9feec8d885 --- /dev/null +++ b/packages/frontend/editor-ui/src/features/core/auth/views/ConfirmEmailChangeView.vue @@ -0,0 +1,121 @@ + + + + + diff --git a/packages/frontend/editor-ui/src/features/core/auth/views/SettingsPersonalView.test.ts b/packages/frontend/editor-ui/src/features/core/auth/views/SettingsPersonalView.test.ts index c68ffd0e01b..17d51f929f0 100644 --- a/packages/frontend/editor-ui/src/features/core/auth/views/SettingsPersonalView.test.ts +++ b/packages/frontend/editor-ui/src/features/core/auth/views/SettingsPersonalView.test.ts @@ -1,7 +1,8 @@ import { createPinia } from 'pinia'; -import { waitFor } from '@testing-library/vue'; +import { fireEvent, waitFor } from '@testing-library/vue'; import { waitAllPromises, getTooltip, hoverTooltipTrigger } from '@/__tests__/utils'; import SettingsPersonalView from './SettingsPersonalView.vue'; +import { confirmPasswordEventBus } from '../auth.eventBus'; import { useSettingsStore } from '@n8n/stores/settings.store'; import { useUsersStore } from '@n8n/stores/users.store'; import { createComponentRenderer } from '@/__tests__/render'; @@ -75,6 +76,55 @@ describe('SettingsPersonalView', () => { expect(getByTestId('change-password-link')).toBeInTheDocument(); }); + describe('when saving basic info', () => { + it('should save a name-only change through updateUserName', async () => { + const updateUserNameSpy = vi + .spyOn(usersStore, 'updateUserName') + .mockResolvedValue({ id: '1', isPending: false }); + const requestEmailChangeSpy = vi.spyOn(usersStore, 'requestEmailChange'); + + const { getByTestId, getAllByRole } = renderComponent({ pinia }); + await waitAllPromises(); + + const firstNameInput = getAllByRole('textbox')[0]; + await fireEvent.update(firstNameInput, 'Jane'); + await waitAllPromises(); + + getByTestId('save-settings-button').click(); + await waitAllPromises(); + + expect(updateUserNameSpy).toHaveBeenCalledWith({ firstName: 'Jane', lastName: 'Doe' }); + expect(requestEmailChangeSpy).not.toHaveBeenCalled(); + }); + + it('should route an email change through requestEmailChange, not updateUser', async () => { + const requestEmailChangeSpy = vi + .spyOn(usersStore, 'requestEmailChange') + .mockResolvedValue({ status: 'confirmation-sent' }); + const updateUserSpy = vi.spyOn(usersStore, 'updateUser'); + + const { getByTestId, getAllByRole } = renderComponent({ pinia }); + await waitAllPromises(); + + const emailInput = getAllByRole('textbox').find((el) => el.getAttribute('type') === 'email')!; + await fireEvent.update(emailInput, 'new@example.com'); + await waitAllPromises(); + + getByTestId('save-settings-button').click(); + await waitAllPromises(); + + // The password modal collects the current password; simulate confirming it. + confirmPasswordEventBus.emit('close', { currentPassword: 'secret' }); + await waitAllPromises(); + + expect(requestEmailChangeSpy).toHaveBeenCalledWith({ + email: 'new@example.com', + currentPassword: 'secret', + }); + expect(updateUserSpy).not.toHaveBeenCalled(); + }); + }); + describe('when changing theme', () => { it('should disable save button when theme has not been changed', async () => { const { getByTestId } = renderComponent({ pinia }); diff --git a/packages/frontend/editor-ui/src/features/core/auth/views/SettingsPersonalView.vue b/packages/frontend/editor-ui/src/features/core/auth/views/SettingsPersonalView.vue index f98420b723b..2d879758b86 100644 --- a/packages/frontend/editor-ui/src/features/core/auth/views/SettingsPersonalView.vue +++ b/packages/frontend/editor-ui/src/features/core/auth/views/SettingsPersonalView.vue @@ -1,6 +1,6 @@