diff --git a/biome.jsonc b/biome.jsonc index 356c964cd9..d11acefc5a 100644 --- a/biome.jsonc +++ b/biome.jsonc @@ -33,6 +33,9 @@ "enabled": false }, "javascript": { + "parser": { + "unsafeParameterDecoratorsEnabled": true + }, "formatter": { "jsxQuoteStyle": "double", "quoteProperties": "asNeeded", diff --git a/cypress/e2e/33-settings-personal.cy.ts b/cypress/e2e/33-settings-personal.cy.ts index e3cc572c9e..6b5cc94687 100644 --- a/cypress/e2e/33-settings-personal.cy.ts +++ b/cypress/e2e/33-settings-personal.cy.ts @@ -42,7 +42,7 @@ describe('Personal Settings', () => { cy.getByTestId('personal-data-form').find('input[name="firstName"]').clear().type(name); cy.getByTestId('personal-data-form').find('input[name="lastName"]').clear().type(name); cy.getByTestId('save-settings-button').click(); - errorToast().should('contain', 'Potentially malicious string | Potentially malicious string'); + errorToast().should('contain', 'Potentially malicious string'); errorToast().find('.el-notification__closeBtn').click(); }); }); diff --git a/packages/@n8n/api-types/package.json b/packages/@n8n/api-types/package.json index 294a169906..726b0f908e 100644 --- a/packages/@n8n/api-types/package.json +++ b/packages/@n8n/api-types/package.json @@ -11,7 +11,8 @@ "lint": "eslint .", "lintfix": "eslint . --fix", "watch": "tsc -p tsconfig.build.json --watch", - "test": "echo \"No tests yet\" && exit 0" + "test": "jest", + "test:dev": "jest --watch" }, "main": "dist/index.js", "module": "src/index.ts", @@ -21,5 +22,10 @@ ], "devDependencies": { "n8n-workflow": "workspace:*" + }, + "dependencies": { + "xss": "catalog:", + "zod": "catalog:", + "zod-class": "0.0.15" } } diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts new file mode 100644 index 0000000000..68c1972a46 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -0,0 +1,4 @@ +export { PasswordUpdateRequestDto } from './user/password-update-request.dto'; +export { RoleChangeRequestDto } from './user/role-change-request.dto'; +export { SettingsUpdateRequestDto } from './user/settings-update-request.dto'; +export { UserUpdateRequestDto } from './user/user-update-request.dto'; diff --git a/packages/@n8n/api-types/src/dto/user/__tests__/password-update-request.dto.test.ts b/packages/@n8n/api-types/src/dto/user/__tests__/password-update-request.dto.test.ts new file mode 100644 index 0000000000..608f0f4bbd --- /dev/null +++ b/packages/@n8n/api-types/src/dto/user/__tests__/password-update-request.dto.test.ts @@ -0,0 +1,50 @@ +import { PasswordUpdateRequestDto } from '../password-update-request.dto'; + +describe('PasswordUpdateRequestDto', () => { + it('should fail validation with missing currentPassword', () => { + const data = { + newPassword: 'newPassword123', + mfaCode: '123456', + }; + + const result = PasswordUpdateRequestDto.safeParse(data); + + expect(result.success).toBe(false); + expect(result.error?.issues[0].path[0]).toBe('currentPassword'); + }); + + it('should fail validation with missing newPassword', () => { + const data = { + currentPassword: 'oldPassword123', + mfaCode: '123456', + }; + + const result = PasswordUpdateRequestDto.safeParse(data); + + expect(result.success).toBe(false); + expect(result.error?.issues[0].path[0]).toBe('newPassword'); + }); + + it('should pass validation with missing mfaCode', () => { + const data = { + currentPassword: 'oldPassword123', + newPassword: 'newPassword123', + }; + + const result = PasswordUpdateRequestDto.safeParse(data); + + expect(result.success).toBe(true); + }); + + it('should pass validation with valid data', () => { + const data = { + currentPassword: 'oldPassword123', + newPassword: 'newPassword123', + mfaCode: '123456', + }; + + const result = PasswordUpdateRequestDto.safeParse(data); + + expect(result.success).toBe(true); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/user/__tests__/role-change-request.dto.test.ts b/packages/@n8n/api-types/src/dto/user/__tests__/role-change-request.dto.test.ts new file mode 100644 index 0000000000..87ed9799d1 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/user/__tests__/role-change-request.dto.test.ts @@ -0,0 +1,37 @@ +import { RoleChangeRequestDto } from '../role-change-request.dto'; + +describe('RoleChangeRequestDto', () => { + it('should fail validation with missing newRoleName', () => { + const data = {}; + + const result = RoleChangeRequestDto.safeParse(data); + + expect(result.success).toBe(false); + expect(result.error?.issues[0].path[0]).toBe('newRoleName'); + expect(result.error?.issues[0].message).toBe('New role is required'); + }); + + it('should fail validation with invalid newRoleName', () => { + const data = { + newRoleName: 'invalidRole', + }; + + const result = RoleChangeRequestDto.safeParse(data); + + expect(result.success).toBe(false); + expect(result.error?.issues[0].path[0]).toBe('newRoleName'); + expect(result.error?.issues[0].message).toBe( + "Invalid enum value. Expected 'global:admin' | 'global:member', received 'invalidRole'", + ); + }); + + it('should pass validation with valid data', () => { + const data = { + newRoleName: 'global:admin', + }; + + const result = RoleChangeRequestDto.safeParse(data); + + expect(result.success).toBe(true); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/user/__tests__/settings-update-request.dto.test.ts b/packages/@n8n/api-types/src/dto/user/__tests__/settings-update-request.dto.test.ts new file mode 100644 index 0000000000..f31038c070 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/user/__tests__/settings-update-request.dto.test.ts @@ -0,0 +1,68 @@ +import { SettingsUpdateRequestDto } from '../settings-update-request.dto'; + +describe('SettingsUpdateRequestDto', () => { + it('should pass validation with missing userActivated', () => { + const data = { + allowSSOManualLogin: false, + }; + + const result = SettingsUpdateRequestDto.safeParse(data); + + expect(result.success).toBe(true); + }); + + it('should pass validation with missing allowSSOManualLogin', () => { + const data = { + userActivated: true, + }; + + const result = SettingsUpdateRequestDto.safeParse(data); + + expect(result.success).toBe(true); + }); + + it('should pass validation with missing userActivated and allowSSOManualLogin', () => { + const data = {}; + + const result = SettingsUpdateRequestDto.safeParse(data); + + expect(result.success).toBe(true); + }); + + it('should fail validation with invalid userActivated', () => { + const data = { + userActivated: 'invalid', + allowSSOManualLogin: false, + }; + + const result = SettingsUpdateRequestDto.safeParse(data); + + expect(result.success).toBe(false); + expect(result.error?.issues[0].path[0]).toBe('userActivated'); + expect(result.error?.issues[0].message).toBe('Expected boolean, received string'); + }); + + it('should fail validation with invalid allowSSOManualLogin', () => { + const data = { + userActivated: true, + allowSSOManualLogin: 'invalid', + }; + + const result = SettingsUpdateRequestDto.safeParse(data); + + expect(result.success).toBe(false); + expect(result.error?.issues[0].path[0]).toBe('allowSSOManualLogin'); + expect(result.error?.issues[0].message).toBe('Expected boolean, received string'); + }); + + it('should pass validation with valid data', () => { + const data = { + userActivated: true, + allowSSOManualLogin: false, + }; + + const result = SettingsUpdateRequestDto.safeParse(data); + + expect(result.success).toBe(true); + }); +}); 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 new file mode 100644 index 0000000000..0869c4a0f1 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/user/__tests__/user-update-request.dto.test.ts @@ -0,0 +1,86 @@ +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); + + expect(result.success).toBe(false); + expect(result.error?.issues[0].path).toEqual(['firstName']); + }); + + 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); + + expect(result.success).toBe(false); + expect(result.error?.issues[0].path).toEqual(['firstName']); + }); + + 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); + + expect(result.success).toBe(false); + expect(result.error?.issues[0].path).toEqual(['lastName']); + }); + + 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); + + expect(result.success).toBe(false); + expect(result.error?.issues[0].path).toEqual(['lastName']); + }); + + 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); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/user/password-update-request.dto.ts b/packages/@n8n/api-types/src/dto/user/password-update-request.dto.ts new file mode 100644 index 0000000000..1e1f02b0a3 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/user/password-update-request.dto.ts @@ -0,0 +1,8 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +export class PasswordUpdateRequestDto extends Z.class({ + currentPassword: z.string(), + newPassword: z.string(), + mfaCode: z.string().optional(), +}) {} diff --git a/packages/@n8n/api-types/src/dto/user/role-change-request.dto.ts b/packages/@n8n/api-types/src/dto/user/role-change-request.dto.ts new file mode 100644 index 0000000000..4ddae2cd58 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/user/role-change-request.dto.ts @@ -0,0 +1,8 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +export class RoleChangeRequestDto extends Z.class({ + newRoleName: z.enum(['global:admin', 'global:member'], { + required_error: 'New role is required', + }), +}) {} diff --git a/packages/@n8n/api-types/src/dto/user/settings-update-request.dto.ts b/packages/@n8n/api-types/src/dto/user/settings-update-request.dto.ts new file mode 100644 index 0000000000..f4c0eb0af3 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/user/settings-update-request.dto.ts @@ -0,0 +1,7 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +export class SettingsUpdateRequestDto extends Z.class({ + userActivated: z.boolean().optional(), + allowSSOManualLogin: z.boolean().optional(), +}) {} 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 new file mode 100644 index 0000000000..ad70071d81 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/user/user-update-request.dto.ts @@ -0,0 +1,31 @@ +import xss from 'xss'; +import { z } from 'zod'; +import { Z } from 'zod-class'; + +const xssCheck = (value: string) => + value === + xss(value, { + whiteList: {}, // no tags are allowed + }); + +const URL_REGEX = /^(https?:\/\/|www\.)|(\.[\p{L}\d-]+)/iu; +const urlCheck = (value: string) => !URL_REGEX.test(value); + +const nameSchema = () => + z + .string() + .min(1) + .max(32) + .refine(xssCheck, { + message: 'Potentially malicious string', + }) + .refine(urlCheck, { + message: 'Potentially malicious string', + }); + +export class UserUpdateRequestDto extends Z.class({ + email: z.string().email(), + firstName: nameSchema().optional(), + lastName: nameSchema().optional(), + mfaCode: z.string().optional(), +}) {} diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index 89b6a3541c..d0067f7fff 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -1,4 +1,5 @@ export type * from './datetime'; +export * from './dto'; export type * from './push'; export type * from './scaling'; export type * from './frontend-settings'; diff --git a/packages/cli/package.json b/packages/cli/package.json index 20a8d5253e..9d108c30d5 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -174,7 +174,7 @@ "ws": "8.17.1", "xml2js": "catalog:", "xmllint-wasm": "3.0.1", - "xss": "^1.0.14", + "xss": "catalog:", "yamljs": "0.3.0", "zod": "catalog:" } diff --git a/packages/cli/src/controllers/__tests__/me.controller.test.ts b/packages/cli/src/controllers/__tests__/me.controller.test.ts index 28640df791..3c9af48689 100644 --- a/packages/cli/src/controllers/__tests__/me.controller.test.ts +++ b/packages/cli/src/controllers/__tests__/me.controller.test.ts @@ -1,3 +1,4 @@ +import { UserUpdateRequestDto } from '@n8n/api-types'; import type { Response } from 'express'; import { mock, anyObject } from 'jest-mock-extended'; import jwt from 'jsonwebtoken'; @@ -35,20 +36,6 @@ describe('MeController', () => { const controller = Container.get(MeController); describe('updateCurrentUser', () => { - it('should throw BadRequestError if email is missing in the payload', async () => { - const req = mock({}); - await expect(controller.updateCurrentUser(req, mock())).rejects.toThrowError( - new BadRequestError('Email is mandatory'), - ); - }); - - it('should throw BadRequestError if email is invalid', async () => { - const req = mock({ body: { email: 'invalid-email' } }); - await expect(controller.updateCurrentUser(req, mock())).rejects.toThrowError( - new BadRequestError('Invalid email address'), - ); - }); - it('should update the user in the DB, and issue a new cookie', async () => { const user = mock({ id: '123', @@ -58,24 +45,24 @@ describe('MeController', () => { role: 'global:owner', mfaEnabled: false, }); - const req = mock({ user, browserId }); - req.body = { + const payload = new UserUpdateRequestDto({ email: 'valid@email.com', firstName: 'John', lastName: 'Potato', - }; + }); + const req = mock({ user, browserId }); const res = mock(); userRepository.findOneByOrFail.mockResolvedValue(user); userRepository.findOneOrFail.mockResolvedValue(user); jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); userService.toPublic.mockResolvedValue({} as unknown as PublicUser); - await controller.updateCurrentUser(req, res); + await controller.updateCurrentUser(req, res, payload); expect(externalHooks.run).toHaveBeenCalledWith('user.profile.beforeUpdate', [ user.id, user.email, - req.body, + payload, ]); expect(userService.update).toHaveBeenCalled(); @@ -100,35 +87,6 @@ describe('MeController', () => { ]); }); - it('should not allow updating any other fields on a user besides email and name', async () => { - const user = mock({ - id: '123', - password: 'password', - authIdentities: [], - role: 'global:member', - mfaEnabled: false, - }); - const req = mock({ user, browserId }); - req.body = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' }; - const res = mock(); - userRepository.findOneOrFail.mockResolvedValue(user); - jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); - - // Add invalid data to the request payload - Object.assign(req.body, { id: '0', role: 'global:owner' }); - - await controller.updateCurrentUser(req, res); - - expect(userService.update).toHaveBeenCalled(); - - const updatePayload = userService.update.mock.calls[0][1]; - expect(updatePayload.email).toBe(req.body.email); - expect(updatePayload.firstName).toBe(req.body.firstName); - expect(updatePayload.lastName).toBe(req.body.lastName); - expect(updatePayload.id).toBeUndefined(); - expect(updatePayload.role).toBeUndefined(); - }); - it('should throw BadRequestError if beforeUpdate hook throws BadRequestError', async () => { const user = mock({ id: '123', @@ -137,9 +95,7 @@ describe('MeController', () => { role: 'global:owner', mfaEnabled: false, }); - const reqBody = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' }; - const req = mock({ user, body: reqBody }); - req.body = reqBody; // We don't want the body to be a mock object + const req = mock({ user }); externalHooks.run.mockImplementationOnce(async (hookName) => { if (hookName === 'user.profile.beforeUpdate') { @@ -147,9 +103,13 @@ describe('MeController', () => { } }); - await expect(controller.updateCurrentUser(req, mock())).rejects.toThrowError( - new BadRequestError('Invalid email address'), - ); + await expect( + controller.updateCurrentUser( + req, + mock(), + mock({ email: 'valid@email.com', firstName: 'John', lastName: 'Potato' }), + ), + ).rejects.toThrowError(new BadRequestError('Invalid email address')); }); describe('when mfa is enabled', () => { @@ -162,12 +122,19 @@ describe('MeController', () => { role: 'global:owner', mfaEnabled: true, }); - const req = mock({ user, browserId }); - req.body = { email: 'new@email.com', firstName: 'John', lastName: 'Potato' }; + const req = mock({ user, browserId }); - await expect(controller.updateCurrentUser(req, mock())).rejects.toThrowError( - new BadRequestError('Two-factor code is required to change email'), - ); + await expect( + controller.updateCurrentUser( + req, + mock(), + new UserUpdateRequestDto({ + email: 'new@email.com', + firstName: 'John', + lastName: 'Potato', + }), + ), + ).rejects.toThrowError(new BadRequestError('Two-factor code is required to change email')); }); it('should throw InvalidMfaCodeError if mfa code is invalid', async () => { @@ -179,18 +146,21 @@ describe('MeController', () => { role: 'global:owner', mfaEnabled: true, }); - const req = mock({ user, browserId }); - req.body = { - email: 'new@email.com', - firstName: 'John', - lastName: 'Potato', - mfaCode: 'invalid', - }; + const req = mock({ user, browserId }); mockMfaService.validateMfa.mockResolvedValue(false); - await expect(controller.updateCurrentUser(req, mock())).rejects.toThrow( - InvalidMfaCodeError, - ); + 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 () => { @@ -202,13 +172,7 @@ describe('MeController', () => { role: 'global:owner', mfaEnabled: true, }); - const req = mock({ user, browserId }); - req.body = { - email: 'new@email.com', - firstName: 'John', - lastName: 'Potato', - mfaCode: '123456', - }; + const req = mock({ user, browserId }); const res = mock(); userRepository.findOneByOrFail.mockResolvedValue(user); userRepository.findOneOrFail.mockResolvedValue(user); @@ -216,7 +180,16 @@ describe('MeController', () => { userService.toPublic.mockResolvedValue({} as unknown as PublicUser); mockMfaService.validateMfa.mockResolvedValue(true); - const result = await controller.updateCurrentUser(req, res); + const result = await controller.updateCurrentUser( + req, + res, + mock({ + email: 'new@email.com', + firstName: 'John', + lastName: 'Potato', + mfaCode: '123456', + }), + ); expect(result).toEqual({}); }); @@ -227,51 +200,59 @@ describe('MeController', () => { const passwordHash = '$2a$10$ffitcKrHT.Ls.m9FfWrMrOod76aaI0ogKbc3S96Q320impWpCbgj6'; // Hashed 'old_password' it('should throw if the user does not have a password set', async () => { - const req = mock({ + const req = mock({ user: mock({ password: undefined }), - body: { currentPassword: '', newPassword: '' }, }); - await expect(controller.updatePassword(req, mock())).rejects.toThrowError( - new BadRequestError('Requesting user not set up.'), - ); + await expect( + controller.updatePassword(req, mock(), mock({ currentPassword: '', newPassword: '' })), + ).rejects.toThrowError(new BadRequestError('Requesting user not set up.')); }); it("should throw if currentPassword does not match the user's password", async () => { - const req = mock({ + const req = mock({ user: mock({ password: passwordHash }), - body: { currentPassword: 'not_old_password', newPassword: '' }, }); - await expect(controller.updatePassword(req, mock())).rejects.toThrowError( - new BadRequestError('Provided current password is incorrect.'), - ); + await expect( + controller.updatePassword( + req, + mock(), + mock({ currentPassword: 'not_old_password', newPassword: '' }), + ), + ).rejects.toThrowError(new BadRequestError('Provided current password is incorrect.')); }); describe('should throw if newPassword is not valid', () => { Object.entries(badPasswords).forEach(([newPassword, errorMessage]) => { it(newPassword, async () => { - const req = mock({ + const req = mock({ user: mock({ password: passwordHash }), - body: { currentPassword: 'old_password', newPassword }, browserId, }); - await expect(controller.updatePassword(req, mock())).rejects.toThrowError( - new BadRequestError(errorMessage), - ); + await expect( + controller.updatePassword( + req, + mock(), + mock({ currentPassword: 'old_password', newPassword }), + ), + ).rejects.toThrowError(new BadRequestError(errorMessage)); }); }); }); it('should update the password in the DB, and issue a new cookie', async () => { - const req = mock({ + const req = mock({ user: mock({ password: passwordHash, mfaEnabled: false }), - body: { currentPassword: 'old_password', newPassword: 'NewPassword123' }, browserId, }); const res = mock(); userRepository.save.calledWith(req.user).mockResolvedValue(req.user); jest.spyOn(jwt, 'sign').mockImplementation(() => 'new-signed-token'); - await controller.updatePassword(req, res); + await controller.updatePassword( + req, + res, + mock({ currentPassword: 'old_password', newPassword: 'NewPassword123' }), + ); expect(req.user.password).not.toBe(passwordHash); @@ -299,34 +280,43 @@ describe('MeController', () => { describe('mfa enabled', () => { it('should throw BadRequestError if mfa code is missing', async () => { - const req = mock({ + const req = mock({ user: mock({ password: passwordHash, mfaEnabled: true }), - body: { currentPassword: 'old_password', newPassword: 'NewPassword123' }, }); - await expect(controller.updatePassword(req, mock())).rejects.toThrowError( + await expect( + controller.updatePassword( + req, + mock(), + mock({ currentPassword: 'old_password', newPassword: 'NewPassword123' }), + ), + ).rejects.toThrowError( new BadRequestError('Two-factor code is required to change password.'), ); }); it('should throw InvalidMfaCodeError if invalid mfa code is given', async () => { - const req = mock({ + const req = mock({ user: mock({ password: passwordHash, mfaEnabled: true }), - body: { currentPassword: 'old_password', newPassword: 'NewPassword123', mfaCode: '123' }, }); mockMfaService.validateMfa.mockResolvedValue(false); - await expect(controller.updatePassword(req, mock())).rejects.toThrow(InvalidMfaCodeError); + await expect( + controller.updatePassword( + req, + mock(), + mock({ + currentPassword: 'old_password', + newPassword: 'NewPassword123', + mfaCode: '123', + }), + ), + ).rejects.toThrow(InvalidMfaCodeError); }); it('should succeed when mfa code is correct', async () => { - const req = mock({ + const req = mock({ user: mock({ password: passwordHash, mfaEnabled: true }), - body: { - currentPassword: 'old_password', - newPassword: 'NewPassword123', - mfaCode: 'valid', - }, browserId, }); const res = mock(); @@ -334,7 +324,15 @@ describe('MeController', () => { jest.spyOn(jwt, 'sign').mockImplementation(() => 'new-signed-token'); mockMfaService.validateMfa.mockResolvedValue(true); - const result = await controller.updatePassword(req, res); + const result = await controller.updatePassword( + req, + res, + mock({ + currentPassword: 'old_password', + newPassword: 'NewPassword123', + mfaCode: 'valid', + }), + ); expect(result).toEqual({ success: true }); expect(req.user.password).not.toBe(passwordHash); @@ -411,18 +409,6 @@ describe('MeController', () => { }); }); - describe('updateCurrentUserSettings', () => { - it('should throw BadRequestError on XSS attempt', async () => { - const req = mock({ - body: { - userActivated: '', - }, - }); - - await expect(controller.updateCurrentUserSettings(req)).rejects.toThrowError(BadRequestError); - }); - }); - describe('API Key methods', () => { let req: AuthenticatedRequest; beforeAll(() => { diff --git a/packages/cli/src/controllers/__tests__/users.controller.test.ts b/packages/cli/src/controllers/__tests__/users.controller.test.ts index 216360651e..91ad6bd28b 100644 --- a/packages/cli/src/controllers/__tests__/users.controller.test.ts +++ b/packages/cli/src/controllers/__tests__/users.controller.test.ts @@ -3,7 +3,7 @@ import { mock } from 'jest-mock-extended'; import type { User } from '@/databases/entities/user'; import type { UserRepository } from '@/databases/repositories/user.repository'; import type { EventService } from '@/events/event.service'; -import type { UserRequest } from '@/requests'; +import type { AuthenticatedRequest } from '@/requests'; import type { ProjectService } from '@/services/project.service'; import { UsersController } from '../users.controller'; @@ -33,15 +33,18 @@ describe('UsersController', () => { describe('changeGlobalRole', () => { it('should emit event user-changed-role', async () => { - const request = mock({ + const request = mock({ user: { id: '123' }, - params: { id: '456' }, - body: { newRoleName: 'global:member' }, }); - userRepository.findOne.mockResolvedValue(mock({ id: '456' })); + userRepository.findOneBy.mockResolvedValue(mock({ id: '456' })); projectService.getUserOwnedOrAdminProjects.mockResolvedValue([]); - await controller.changeGlobalRole(request); + await controller.changeGlobalRole( + request, + mock(), + mock({ newRoleName: 'global:member' }), + '456', + ); expect(eventService.emit).toHaveBeenCalledWith('user-changed-role', { userId: '123', diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index f42b63e4df..0e1bbb37a6 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -1,12 +1,16 @@ +import { + PasswordUpdateRequestDto, + SettingsUpdateRequestDto, + UserUpdateRequestDto, +} from '@n8n/api-types'; import { plainToInstance } from 'class-transformer'; import { randomBytes } from 'crypto'; import { type RequestHandler, Response } from 'express'; -import validator from 'validator'; import { AuthService } from '@/auth/auth.service'; import type { User } from '@/databases/entities/user'; import { UserRepository } from '@/databases/repositories/user.repository'; -import { Delete, Get, Patch, Post, RestController } from '@/decorators'; +import { Body, Delete, Get, Patch, Post, RestController } from '@/decorators'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { InvalidMfaCodeError } from '@/errors/response-errors/invalid-mfa-code.error'; import { EventService } from '@/events/event.service'; @@ -16,12 +20,7 @@ import type { PublicUser } from '@/interfaces'; import { Logger } from '@/logger'; import { MfaService } from '@/mfa/mfa.service'; import { isApiEnabled } from '@/public-api'; -import { - AuthenticatedRequest, - MeRequest, - UserSettingsUpdatePayload, - UserUpdatePayload, -} from '@/requests'; +import { AuthenticatedRequest, MeRequest } from '@/requests'; import { PasswordUtility } from '@/services/password.utility'; import { UserService } from '@/services/user.service'; import { isSamlLicensedAndEnabled } from '@/sso/saml/saml-helpers'; @@ -55,30 +54,14 @@ export class MeController { * Update the logged-in user's properties, except password. */ @Patch('/') - async updateCurrentUser(req: MeRequest.UserUpdate, res: Response): Promise { + async updateCurrentUser( + req: AuthenticatedRequest, + res: Response, + @Body payload: UserUpdateRequestDto, + ): Promise { const { id: userId, email: currentEmail, mfaEnabled } = req.user; - const payload = plainToInstance(UserUpdatePayload, req.body, { excludeExtraneousValues: true }); - const { email } = payload; - if (!email) { - this.logger.debug('Request to update user email failed because of missing email in payload', { - userId, - payload, - }); - throw new BadRequestError('Email is mandatory'); - } - - if (!validator.isEmail(email)) { - this.logger.debug('Request to update user email failed because of invalid email in payload', { - userId, - invalidEmail: email, - }); - throw new BadRequestError('Invalid email address'); - } - - await validateEntity(payload); - const isEmailBeingChanged = email !== currentEmail; // If SAML is enabled, we don't allow the user to change their email address @@ -134,9 +117,13 @@ export class MeController { * Update the logged-in user's password. */ @Patch('/password', { rateLimit: true }) - async updatePassword(req: MeRequest.Password, res: Response) { + async updatePassword( + req: AuthenticatedRequest, + res: Response, + @Body payload: PasswordUpdateRequestDto, + ) { const { user } = req; - const { currentPassword, newPassword, mfaCode } = req.body; + const { currentPassword, newPassword, mfaCode } = payload; // If SAML is enabled, we don't allow the user to change their password if (isSamlLicensedAndEnabled()) { @@ -270,13 +257,11 @@ export class MeController { * Update the logged-in user's settings. */ @Patch('/settings') - async updateCurrentUserSettings(req: MeRequest.UserSettingsUpdate): Promise { - const payload = plainToInstance(UserSettingsUpdatePayload, req.body, { - excludeExtraneousValues: true, - }); - - await validateEntity(payload); - + async updateCurrentUserSettings( + req: AuthenticatedRequest, + _: Response, + @Body payload: SettingsUpdateRequestDto, + ): Promise { const { id } = req.user; await this.userService.updateSettings(id, payload); diff --git a/packages/cli/src/controllers/users.controller.ts b/packages/cli/src/controllers/users.controller.ts index 7124013354..c00fe48ad8 100644 --- a/packages/cli/src/controllers/users.controller.ts +++ b/packages/cli/src/controllers/users.controller.ts @@ -1,4 +1,5 @@ -import { plainToInstance } from 'class-transformer'; +import { RoleChangeRequestDto, SettingsUpdateRequestDto } from '@n8n/api-types'; +import { Response } from 'express'; import { AuthService } from '@/auth/auth.service'; import { CredentialsService } from '@/credentials/credentials.service'; @@ -9,22 +10,17 @@ import { ProjectRepository } from '@/databases/repositories/project.repository'; import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; import { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository'; import { UserRepository } from '@/databases/repositories/user.repository'; -import { GlobalScope, Delete, Get, RestController, Patch, Licensed } from '@/decorators'; +import { GlobalScope, Delete, Get, RestController, Patch, Licensed, Body } from '@/decorators'; +import { Param } from '@/decorators/args'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { EventService } from '@/events/event.service'; import { ExternalHooks } from '@/external-hooks'; -import { validateEntity } from '@/generic-helpers'; import type { PublicUser } from '@/interfaces'; import { Logger } from '@/logger'; import { listQueryMiddleware } from '@/middlewares'; -import { - ListQuery, - UserRequest, - UserRoleChangePayload, - UserSettingsUpdatePayload, -} from '@/requests'; +import { AuthenticatedRequest, ListQuery, UserRequest } from '@/requests'; import { ProjectService } from '@/services/project.service'; import { UserService } from '@/services/user.service'; import { WorkflowService } from '@/workflows/workflow.service'; @@ -124,13 +120,12 @@ export class UsersController { @Patch('/:id/settings') @GlobalScope('user:update') - async updateUserSettings(req: UserRequest.UserSettingsUpdate) { - const payload = plainToInstance(UserSettingsUpdatePayload, req.body, { - excludeExtraneousValues: true, - }); - - const id = req.params.id; - + async updateUserSettings( + _req: AuthenticatedRequest, + _res: Response, + @Body payload: SettingsUpdateRequestDto, + @Param('id') id: string, + ) { await this.userService.updateSettings(id, payload); const user = await this.userRepository.findOneOrFail({ @@ -263,18 +258,16 @@ export class UsersController { @Patch('/:id/role') @GlobalScope('user:changeRole') @Licensed('feat:advancedPermissions') - async changeGlobalRole(req: UserRequest.ChangeRole) { + async changeGlobalRole( + req: AuthenticatedRequest, + _: Response, + @Body payload: RoleChangeRequestDto, + @Param('id') id: string, + ) { const { NO_ADMIN_ON_OWNER, NO_USER, NO_OWNER_ON_OWNER } = UsersController.ERROR_MESSAGES.CHANGE_ROLE; - const payload = plainToInstance(UserRoleChangePayload, req.body, { - excludeExtraneousValues: true, - }); - await validateEntity(payload); - - const targetUser = await this.userRepository.findOne({ - where: { id: req.params.id }, - }); + const targetUser = await this.userRepository.findOneBy({ id }); if (targetUser === null) { throw new NotFoundError(NO_USER); } diff --git a/packages/cli/src/decorators/__tests__/controller.registry.test.ts b/packages/cli/src/decorators/__tests__/controller.registry.test.ts index 0d7b9de0b7..3b058243f6 100644 --- a/packages/cli/src/decorators/__tests__/controller.registry.test.ts +++ b/packages/cli/src/decorators/__tests__/controller.registry.test.ts @@ -12,6 +12,8 @@ import { ControllerRegistry, Get, Licensed, RestController } from '@/decorators' import type { License } from '@/license'; import type { SuperAgentTest } from '@test-integration/types'; +import { Param } from '../args'; + describe('ControllerRegistry', () => { const license = mock(); const authService = mock(); @@ -114,4 +116,26 @@ describe('ControllerRegistry', () => { expect(license.isFeatureEnabled).toHaveBeenCalled(); }); }); + + describe('Args', () => { + @RestController('/test') + // @ts-expect-error tsc complains about unused class + class TestController { + @Get('/args/:id') + args(req: express.Request, res: express.Response, @Param('id') id: string) { + res.setHeader('Testing', 'true'); + return { url: req.url, id }; + } + } + + beforeEach(() => { + authService.authMiddleware.mockImplementation(async (_req, _res, next) => next()); + }); + + it('should pass in correct args to the route handler', async () => { + const { headers, body } = await agent.get('/rest/test/args/1234').expect(200); + expect(headers.testing).toBe('true'); + expect(body.data).toEqual({ url: '/args/1234', id: '1234' }); + }); + }); }); diff --git a/packages/cli/src/decorators/args.ts b/packages/cli/src/decorators/args.ts new file mode 100644 index 0000000000..21c9e94755 --- /dev/null +++ b/packages/cli/src/decorators/args.ts @@ -0,0 +1,18 @@ +import { getRouteMetadata } from './controller.registry'; +import type { Arg, Controller } from './types'; + +const ArgDecorator = + (arg: Arg): ParameterDecorator => + (target, handlerName, parameterIndex) => { + const routeMetadata = getRouteMetadata(target.constructor as Controller, String(handlerName)); + routeMetadata.args[parameterIndex] = arg; + }; + +/** Injects the request body into the handler */ +export const Body = ArgDecorator({ type: 'body' }); + +/** Injects the request query into the handler */ +export const Query = ArgDecorator({ type: 'query' }); + +/** Injects a request parameter into the handler */ +export const Param = (key: string) => ArgDecorator({ type: 'param', key }); diff --git a/packages/cli/src/decorators/controller.registry.ts b/packages/cli/src/decorators/controller.registry.ts index 7e8a17301d..41806cb958 100644 --- a/packages/cli/src/decorators/controller.registry.ts +++ b/packages/cli/src/decorators/controller.registry.ts @@ -2,7 +2,9 @@ import { GlobalConfig } from '@n8n/config'; import { Router } from 'express'; import type { Application, Request, Response, RequestHandler } from 'express'; import { rateLimit as expressRateLimit } from 'express-rate-limit'; +import { ApplicationError } from 'n8n-workflow'; import { Container, Service } from 'typedi'; +import type { ZodClass } from 'zod-class'; import { AuthService } from '@/auth/auth.service'; import { inProduction, RESPONSE_ERROR_MESSAGES } from '@/constants'; @@ -42,6 +44,7 @@ export const getRouteMetadata = (controllerClass: Controller, handlerName: Handl let route = metadata.routes.get(handlerName); if (!route) { route = {} as RouteMetadata; + route.args = []; metadata.routes.set(handlerName, route); } return route; @@ -76,8 +79,31 @@ export class ControllerRegistry { ); for (const [handlerName, route] of metadata.routes) { - const handler = async (req: Request, res: Response) => - await controller[handlerName](req, res); + const argTypes = Reflect.getMetadata( + 'design:paramtypes', + controller, + handlerName, + ) as unknown[]; + // eslint-disable-next-line @typescript-eslint/no-loop-func + const handler = async (req: Request, res: Response) => { + const args: unknown[] = [req, res]; + for (let index = 0; index < route.args.length; index++) { + const arg = route.args[index]; + if (!arg) continue; // Skip args without any decorators + if (arg.type === 'param') args.push(req.params[arg.key]); + else if (['body', 'query'].includes(arg.type)) { + const paramType = argTypes[index] as ZodClass; + if (paramType && 'parse' in paramType) { + const output = paramType.safeParse(req[arg.type]); + if (output.success) args.push(output.data); + else { + return res.status(400).json(output.error.errors[0]); + } + } + } else throw new ApplicationError('Unknown arg type: ' + arg.type); + } + return await controller[handlerName](...args); + }; router[route.method]( route.path, diff --git a/packages/cli/src/decorators/index.ts b/packages/cli/src/decorators/index.ts index 5ef6c15f52..bd32add475 100644 --- a/packages/cli/src/decorators/index.ts +++ b/packages/cli/src/decorators/index.ts @@ -1,3 +1,4 @@ +export { Body } from './args'; export { RestController } from './rest-controller'; export { Get, Post, Put, Patch, Delete } from './route'; export { Middleware } from './middleware'; diff --git a/packages/cli/src/decorators/types.ts b/packages/cli/src/decorators/types.ts index f18faf5ed5..f410df7831 100644 --- a/packages/cli/src/decorators/types.ts +++ b/packages/cli/src/decorators/types.ts @@ -6,6 +6,8 @@ import type { BooleanLicenseFeature } from '@/interfaces'; export type Method = 'get' | 'post' | 'put' | 'patch' | 'delete'; +export type Arg = { type: 'body' | 'query' } | { type: 'param'; key: string }; + export interface RateLimit { /** * The maximum number of requests to allow during the `window` before rate limiting the client. @@ -35,6 +37,7 @@ export interface RouteMetadata { rateLimit?: boolean | RateLimit; licenseFeature?: BooleanLicenseFeature; accessScope?: AccessScope; + args: Arg[]; } export interface ControllerMetadata { diff --git a/packages/cli/src/generic-helpers.ts b/packages/cli/src/generic-helpers.ts index 272cdb433f..47ef1a796b 100644 --- a/packages/cli/src/generic-helpers.ts +++ b/packages/cli/src/generic-helpers.ts @@ -5,11 +5,6 @@ import type { CredentialsEntity } from '@/databases/entities/credentials-entity' import type { TagEntity } from '@/databases/entities/tag-entity'; import type { User } from '@/databases/entities/user'; import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; -import type { - UserRoleChangePayload, - UserSettingsUpdatePayload, - UserUpdatePayload, -} from '@/requests'; import type { PersonalizationSurveyAnswersV4 } from './controllers/survey-answers.dto'; import { BadRequestError } from './errors/response-errors/bad-request.error'; @@ -21,9 +16,6 @@ export async function validateEntity( | TagEntity | AnnotationTagEntity | User - | UserUpdatePayload - | UserRoleChangePayload - | UserSettingsUpdatePayload | PersonalizationSurveyAnswersV4, ): Promise { const errors = await validate(entity); diff --git a/packages/cli/src/public-api/v1/handlers/users/users.handler.ee.ts b/packages/cli/src/public-api/v1/handlers/users/users.handler.ee.ts index e933d12901..3b84b89da3 100644 --- a/packages/cli/src/public-api/v1/handlers/users/users.handler.ee.ts +++ b/packages/cli/src/public-api/v1/handlers/users/users.handler.ee.ts @@ -1,3 +1,4 @@ +import { RoleChangeRequestDto } from '@n8n/api-types'; import type express from 'express'; import type { Response } from 'express'; import { Container } from 'typedi'; @@ -6,7 +7,7 @@ import { InvitationController } from '@/controllers/invitation.controller'; import { UsersController } from '@/controllers/users.controller'; import { ProjectRelationRepository } from '@/databases/repositories/project-relation.repository'; import { EventService } from '@/events/event.service'; -import type { UserRequest } from '@/requests'; +import type { AuthenticatedRequest, UserRequest } from '@/requests'; import { clean, getAllUsersAndCount, getUser } from './users.service.ee'; import { @@ -19,7 +20,7 @@ import { encodeNextCursor } from '../../shared/services/pagination.service'; type Create = UserRequest.Invite; type Delete = UserRequest.Delete; -type ChangeRole = UserRequest.ChangeRole; +type ChangeRole = AuthenticatedRequest<{ id: string }, {}, RoleChangeRequestDto, {}>; export = { getUser: [ @@ -98,7 +99,19 @@ export = { isLicensed('feat:advancedPermissions'), globalScope('user:changeRole'), async (req: ChangeRole, res: Response) => { - await Container.get(UsersController).changeGlobalRole(req); + const validation = RoleChangeRequestDto.safeParse(req.body); + if (validation.error) { + return res.status(400).json({ + message: validation.error.errors[0], + }); + } + + await Container.get(UsersController).changeGlobalRole( + req, + res, + validation.data, + req.params.id, + ); return res.status(204).send(); }, diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index 11990f63ba..7bdc0ac74d 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -1,7 +1,5 @@ import type { Scope } from '@n8n/permissions'; import type { AiAssistantSDK } from '@n8n_io/ai-assistant-sdk'; -import { Expose } from 'class-transformer'; -import { IsBoolean, IsEmail, IsIn, IsOptional, IsString, Length } from 'class-validator'; import type express from 'express'; import type { BannerName, @@ -18,61 +16,15 @@ import type { import type { CredentialsEntity } from '@/databases/entities/credentials-entity'; import type { Project, ProjectType } from '@/databases/entities/project'; -import { AssignableRole } from '@/databases/entities/user'; -import type { GlobalRole, User } from '@/databases/entities/user'; +import type { AssignableRole, GlobalRole, User } from '@/databases/entities/user'; import type { Variables } from '@/databases/entities/variables'; import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; import type { WorkflowHistory } from '@/databases/entities/workflow-history'; import type { PublicUser, SecretsProvider, SecretsProviderState } from '@/interfaces'; -import { NoUrl } from '@/validators/no-url.validator'; -import { NoXss } from '@/validators/no-xss.validator'; import type { ProjectRole } from './databases/entities/project-relation'; import type { ScopesField } from './services/role.service'; -export class UserUpdatePayload implements Pick { - @Expose() - @IsEmail() - email: string; - - @Expose() - @NoXss() - @NoUrl() - @IsString({ message: 'First name must be of type string.' }) - @Length(1, 32, { message: 'First name must be $constraint1 to $constraint2 characters long.' }) - firstName: string; - - @Expose() - @NoXss() - @NoUrl() - @IsString({ message: 'Last name must be of type string.' }) - @Length(1, 32, { message: 'Last name must be $constraint1 to $constraint2 characters long.' }) - lastName: string; - - @IsOptional() - @Expose() - @IsString({ message: 'Two factor code must be a string.' }) - mfaCode?: string; -} - -export class UserSettingsUpdatePayload { - @Expose() - @IsBoolean({ message: 'userActivated should be a boolean' }) - @IsOptional() - userActivated?: boolean; - - @Expose() - @IsBoolean({ message: 'allowSSOManualLogin should be a boolean' }) - @IsOptional() - allowSSOManualLogin?: boolean; -} - -export class UserRoleChangePayload { - @Expose() - @IsIn(['global:admin', 'global:member']) - newRoleName: AssignableRole; -} - export type APIRequest< RouteParams = {}, ResponseBody = {}, @@ -230,13 +182,6 @@ export declare namespace CredentialRequest { // ---------------------------------- export declare namespace MeRequest { - export type UserSettingsUpdate = AuthenticatedRequest<{}, {}, UserSettingsUpdatePayload>; - export type UserUpdate = AuthenticatedRequest<{}, {}, UserUpdatePayload>; - export type Password = AuthenticatedRequest< - {}, - {}, - { currentPassword: string; newPassword: string; mfaCode?: string } - >; export type SurveyAnswers = AuthenticatedRequest<{}, {}, IPersonalizationSurveyAnswersV4>; } @@ -311,8 +256,6 @@ export declare namespace UserRequest { { transferId?: string; includeRole: boolean } >; - export type ChangeRole = AuthenticatedRequest<{ id: string }, {}, UserRoleChangePayload, {}>; - export type Get = AuthenticatedRequest< { id: string; email: string; identifier: string }, {}, @@ -322,12 +265,6 @@ export declare namespace UserRequest { export type PasswordResetLink = AuthenticatedRequest<{ id: string }, {}, {}, {}>; - export type UserSettingsUpdate = AuthenticatedRequest< - { id: string }, - {}, - UserSettingsUpdatePayload - >; - export type Reinvite = AuthenticatedRequest<{ id: string }>; export type Update = AuthlessRequest< diff --git a/packages/cli/test/integration/public-api/users.test.ts b/packages/cli/test/integration/public-api/users.test.ts index f1e74684b4..48003d838c 100644 --- a/packages/cli/test/integration/public-api/users.test.ts +++ b/packages/cli/test/integration/public-api/users.test.ts @@ -225,6 +225,29 @@ describe('Users in Public API', () => { expect(response.body).toHaveProperty('message', 'Forbidden'); }); + it('should return a 400 on invalid payload', async () => { + /** + * Arrange + */ + testServer.license.enable('feat:advancedPermissions'); + const owner = await createOwner({ withApiKey: true }); + const member = await createMember(); + const payload = { newRoleName: 'invalid' }; + + /** + * Act + */ + const response = await testServer + .publicApiAgentFor(owner) + .patch(`/users/${member.id}/role`) + .send(payload); + + /** + * Assert + */ + expect(response.status).toBe(400); + }); + it("should change a user's role", async () => { /** * Arrange diff --git a/packages/cli/test/integration/saml/saml.api.test.ts b/packages/cli/test/integration/saml/saml.api.test.ts index 94110454bd..d30d57356a 100644 --- a/packages/cli/test/integration/saml/saml.api.test.ts +++ b/packages/cli/test/integration/saml/saml.api.test.ts @@ -22,9 +22,11 @@ const testServer = utils.setupTestServer({ enabledFeatures: ['feat:saml'], }); +const memberPassword = randomValidPassword(); + beforeAll(async () => { owner = await createOwner(); - someUser = await createUser(); + someUser = await createUser({ password: memberPassword }); authOwnerAgent = testServer.authAgentFor(owner); authMemberAgent = testServer.authAgentFor(someUser); }); @@ -60,10 +62,11 @@ describe('Instance owner', () => { describe('PATCH /password', () => { test('should throw BadRequestError if password is changed when SAML is enabled', async () => { await enableSaml(true); - await authOwnerAgent + await authMemberAgent .patch('/me/password') .send({ - password: randomValidPassword(), + currentPassword: memberPassword, + newPassword: randomValidPassword(), }) .expect(400, { code: 400, diff --git a/packages/cli/test/integration/users.api.test.ts b/packages/cli/test/integration/users.api.test.ts index d5d2d10a5d..9ecacdec0c 100644 --- a/packages/cli/test/integration/users.api.test.ts +++ b/packages/cli/test/integration/users.api.test.ts @@ -672,9 +672,6 @@ describe('PATCH /users/:id/role', () => { ])('%s', async (_, payload) => { const response = await adminAgent.patch(`/users/${member.id}/role`).send(payload); expect(response.statusCode).toBe(400); - expect(response.body.message).toBe( - 'newRoleName must be one of the following values: global:admin, global:member', - ); }); }); diff --git a/packages/design-system/package.json b/packages/design-system/package.json index ba750661af..18493521f2 100644 --- a/packages/design-system/package.json +++ b/packages/design-system/package.json @@ -54,6 +54,6 @@ "vue": "catalog:frontend", "vue-boring-avatars": "^1.3.0", "vue-router": "catalog:frontend", - "xss": "^1.0.14" + "xss": "catalog:" } } diff --git a/packages/editor-ui/package.json b/packages/editor-ui/package.json index 031bee1615..5e67b123d3 100644 --- a/packages/editor-ui/package.json +++ b/packages/editor-ui/package.json @@ -77,7 +77,7 @@ "vue-markdown-render": "catalog:frontend", "vue-router": "catalog:frontend", "vue3-touch-events": "^4.1.3", - "xss": "^1.0.14" + "xss": "catalog:" }, "devDependencies": { "@faker-js/faker": "^8.0.2", diff --git a/packages/editor-ui/src/api/users.ts b/packages/editor-ui/src/api/users.ts index aedbd0b594..ea14cb6c79 100644 --- a/packages/editor-ui/src/api/users.ts +++ b/packages/editor-ui/src/api/users.ts @@ -1,3 +1,8 @@ +import type { + PasswordUpdateRequestDto, + SettingsUpdateRequestDto, + UserUpdateRequestDto, +} from '@n8n/api-types'; import type { CurrentUserResponse, IPersonalizationLatestVersion, @@ -8,11 +13,6 @@ import type { import type { IDataObject, IUserSettings } from 'n8n-workflow'; import { makeRestApiRequest } from '@/utils/apiUtils'; -export interface IUpdateUserSettingsReqPayload { - allowSSOManualLogin?: boolean; - userActivated?: boolean; -} - export async function loginCurrentUser( context: IRestApiContext, ): Promise { @@ -89,23 +89,16 @@ export async function changePassword( await makeRestApiRequest(context, 'POST', '/change-password', params); } -export type UpdateCurrentUserParams = { - firstName?: string; - lastName?: string; - email: string; - mfaCode?: string; -}; - export async function updateCurrentUser( context: IRestApiContext, - params: UpdateCurrentUserParams, + params: UserUpdateRequestDto, ): Promise { return await makeRestApiRequest(context, 'PATCH', '/me', params); } export async function updateCurrentUserSettings( context: IRestApiContext, - settings: IUpdateUserSettingsReqPayload, + settings: SettingsUpdateRequestDto, ): Promise { return await makeRestApiRequest(context, 'PATCH', '/me/settings', settings); } @@ -113,20 +106,14 @@ export async function updateCurrentUserSettings( export async function updateOtherUserSettings( context: IRestApiContext, userId: string, - settings: IUpdateUserSettingsReqPayload, + settings: SettingsUpdateRequestDto, ): Promise { return await makeRestApiRequest(context, 'PATCH', `/users/${userId}/settings`, settings); } -export type UpdateUserPasswordParams = { - newPassword: string; - currentPassword: string; - mfaCode?: string; -}; - export async function updateCurrentUserPassword( context: IRestApiContext, - params: UpdateUserPasswordParams, + params: PasswordUpdateRequestDto, ): Promise { return await makeRestApiRequest(context, 'PATCH', '/me/password', params); } diff --git a/packages/editor-ui/src/stores/users.store.ts b/packages/editor-ui/src/stores/users.store.ts index 613d2ea48d..605d54e7a7 100644 --- a/packages/editor-ui/src/stores/users.store.ts +++ b/packages/editor-ui/src/stores/users.store.ts @@ -1,4 +1,9 @@ -import type { IUpdateUserSettingsReqPayload, UpdateGlobalRolePayload } from '@/api/users'; +import type { + PasswordUpdateRequestDto, + SettingsUpdateRequestDto, + UserUpdateRequestDto, +} from '@n8n/api-types'; +import type { UpdateGlobalRolePayload } from '@/api/users'; import * as usersApi from '@/api/users'; import { BROWSER_ID_STORAGE_KEY, PERSONALIZATION_MODAL_KEY, STORES, ROLE } from '@/constants'; import type { @@ -226,12 +231,12 @@ export const useUsersStore = defineStore(STORES.USERS, () => { await usersApi.changePassword(rootStore.restApiContext, params); }; - const updateUser = async (params: usersApi.UpdateCurrentUserParams) => { + const updateUser = async (params: UserUpdateRequestDto) => { const user = await usersApi.updateCurrentUser(rootStore.restApiContext, params); addUsers([user]); }; - const updateUserSettings = async (settings: IUpdateUserSettingsReqPayload) => { + const updateUserSettings = async (settings: SettingsUpdateRequestDto) => { const updatedSettings = await usersApi.updateCurrentUserSettings( rootStore.restApiContext, settings, @@ -242,10 +247,7 @@ export const useUsersStore = defineStore(STORES.USERS, () => { } }; - const updateOtherUserSettings = async ( - userId: string, - settings: IUpdateUserSettingsReqPayload, - ) => { + const updateOtherUserSettings = async (userId: string, settings: SettingsUpdateRequestDto) => { const updatedSettings = await usersApi.updateOtherUserSettings( rootStore.restApiContext, userId, @@ -255,7 +257,7 @@ export const useUsersStore = defineStore(STORES.USERS, () => { addUsers([usersById.value[userId]]); }; - const updateCurrentUserPassword = async (params: usersApi.UpdateUserPasswordParams) => { + const updateCurrentUserPassword = async (params: PasswordUpdateRequestDto) => { await usersApi.updateCurrentUserPassword(rootStore.restApiContext, params); }; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 50ad4c8478..2c1a5be995 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -54,6 +54,9 @@ catalogs: xml2js: specifier: 0.6.2 version: 0.6.2 + xss: + specifier: 1.0.15 + version: 1.0.15 zod: specifier: 3.23.8 version: 3.23.8 @@ -233,6 +236,16 @@ importers: version: link:../packages/workflow packages/@n8n/api-types: + dependencies: + xss: + specifier: 'catalog:' + version: 1.0.15 + zod: + specifier: 'catalog:' + version: 3.23.8 + zod-class: + specifier: 0.0.15 + version: 0.0.15(zod@3.23.8) devDependencies: n8n-workflow: specifier: workspace:* @@ -938,8 +951,8 @@ importers: specifier: 3.0.1 version: 3.0.1 xss: - specifier: ^1.0.14 - version: 1.0.14 + specifier: 'catalog:' + version: 1.0.15 yamljs: specifier: 0.3.0 version: 0.3.0 @@ -1171,8 +1184,8 @@ importers: specifier: catalog:frontend version: 4.4.2(vue@3.4.21(typescript@5.6.2)) xss: - specifier: ^1.0.14 - version: 1.0.14 + specifier: 'catalog:' + version: 1.0.15 devDependencies: '@n8n/storybook': specifier: workspace:* @@ -1430,8 +1443,8 @@ importers: specifier: ^4.1.3 version: 4.1.3 xss: - specifier: ^1.0.14 - version: 1.0.14 + specifier: 'catalog:' + version: 1.0.15 devDependencies: '@faker-js/faker': specifier: ^8.0.2 @@ -11844,6 +11857,10 @@ packages: resolution: {integrity: sha512-RAH822pAdBgcNMAfWnCBU3CFZcfZ/i1eZjwFU/dsLKumyuuP3niueg2UAukXYF0E2AAoc82ZSSf9J0WQBinzHA==} engines: {node: '>=12.20'} + type-fest@4.26.1: + resolution: {integrity: sha512-yOGpmOAL7CkKe/91I5O3gPICmJNLJ1G4zFYVAsRHg7M64biSnPtRj0WNQt++bRkjYOqjWXrhnUw1utzmVErAdg==} + engines: {node: '>=16'} + type-is@1.6.18: resolution: {integrity: sha512-TkRKr9sUTxEH8MdfuCSP7VizJyzRNMjj2J2do2Jr3Kym598JVdEksuzPQCnlFPW4ky9Q+iA+ma9BGm06XQBy8g==} engines: {node: '>= 0.6'} @@ -12528,8 +12545,8 @@ packages: xregexp@2.0.0: resolution: {integrity: sha512-xl/50/Cf32VsGq/1R8jJE5ajH1yMCQkpmoS10QbFZWl2Oor4H0Me64Pu2yxvsRWK3m6soJbmGfzSR7BYmDcWAA==} - xss@1.0.14: - resolution: {integrity: sha512-og7TEJhXvn1a7kzZGQ7ETjdQVS2UfZyTlsEdDOqvQF7GoxNfY+0YLCzBy1kPdsDDx4QuNAonQPddpsn6Xl/7sw==} + xss@1.0.15: + resolution: {integrity: sha512-FVdlVVC67WOIPvfOwhoMETV72f6GbW7aOabBC3WxN/oUdoEMDyLz4OgRv5/gck2ZeNqEQu+Tb0kloovXOfpYVg==} engines: {node: '>= 0.10.0'} hasBin: true @@ -12609,6 +12626,11 @@ packages: engines: {node: '>=8.0.0'} hasBin: true + zod-class@0.0.15: + resolution: {integrity: sha512-CD5B4e9unKPj1hiy7JOSwRV01WqbEBkFOlhws0C9s9wB0FSpECOnlKXOAkjo9tKYX2enQsXWyyOIBNPPNUHWRA==} + peerDependencies: + zod: ^3 + zod-to-json-schema@3.23.2: resolution: {integrity: sha512-uSt90Gzc/tUfyNqxnjlfBs8W6WSGpNBv0rVsNxP/BVSMHMKGdthPYff4xtCHYloJGM0CFxFsb3NbC0eqPhfImw==} peerDependencies: @@ -25550,6 +25572,8 @@ snapshots: type-fest@2.19.0: {} + type-fest@4.26.1: {} + type-is@1.6.18: dependencies: media-typer: 0.3.0 @@ -26278,7 +26302,7 @@ snapshots: xregexp@2.0.0: {} - xss@1.0.14: + xss@1.0.15: dependencies: commander: 2.20.3 cssfilter: 0.0.10 @@ -26376,6 +26400,11 @@ snapshots: optionalDependencies: commander: 9.4.1 + zod-class@0.0.15(zod@3.23.8): + dependencies: + type-fest: 4.26.1 + zod: 3.23.8 + zod-to-json-schema@3.23.2(zod@3.23.8): dependencies: zod: 3.23.8 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 07ee961ffe..501b6b7045 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -20,6 +20,7 @@ catalog: typedi: 0.10.0 uuid: 10.0.0 xml2js: 0.6.2 + xss: 1.0.15 zod: 3.23.8 '@langchain/core': 0.2.31