diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index 96f55087a1..70ea346a39 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -5,6 +5,9 @@ export { AiApplySuggestionRequestDto } from './ai/ai-apply-suggestion-request.dt export { LoginRequestDto } from './auth/login-request.dto'; export { ResolveSignupTokenQueryDto } from './auth/resolve-signup-token-query.dto'; +export { InviteUsersRequestDto } from './invitation/invite-users-request.dto'; +export { AcceptInvitationRequestDto } from './invitation/accept-invitation-request.dto'; + export { OwnerSetupRequestDto } from './owner/owner-setup-request.dto'; export { DismissBannerRequestDto } from './owner/dismiss-banner-request.dto'; diff --git a/packages/@n8n/api-types/src/dto/invitation/__tests__/accept-invitation-request.dto.test.ts b/packages/@n8n/api-types/src/dto/invitation/__tests__/accept-invitation-request.dto.test.ts new file mode 100644 index 0000000000..f78de8ab6d --- /dev/null +++ b/packages/@n8n/api-types/src/dto/invitation/__tests__/accept-invitation-request.dto.test.ts @@ -0,0 +1,94 @@ +import { AcceptInvitationRequestDto } from '../accept-invitation-request.dto'; + +describe('AcceptInvitationRequestDto', () => { + const validUuid = '123e4567-e89b-12d3-a456-426614174000'; + + describe('Valid requests', () => { + test.each([ + { + name: 'complete valid invitation acceptance', + request: { + inviterId: validUuid, + firstName: 'John', + lastName: 'Doe', + password: 'SecurePassword123', + }, + }, + ])('should validate $name', ({ request }) => { + const result = AcceptInvitationRequestDto.safeParse(request); + expect(result.success).toBe(true); + }); + }); + + describe('Invalid requests', () => { + test.each([ + { + name: 'missing inviterId', + request: { + firstName: 'John', + lastName: 'Doe', + password: 'SecurePassword123', + }, + expectedErrorPath: ['inviterId'], + }, + { + name: 'invalid inviterId', + request: { + inviterId: 'not-a-valid-uuid', + firstName: 'John', + lastName: 'Doe', + password: 'SecurePassword123', + }, + expectedErrorPath: ['inviterId'], + }, + { + name: 'missing first name', + request: { + inviterId: validUuid, + firstName: '', + lastName: 'Doe', + password: 'SecurePassword123', + }, + expectedErrorPath: ['firstName'], + }, + { + name: 'missing last name', + request: { + inviterId: validUuid, + firstName: 'John', + lastName: '', + password: 'SecurePassword123', + }, + expectedErrorPath: ['lastName'], + }, + { + name: 'password too short', + request: { + inviterId: validUuid, + firstName: 'John', + lastName: 'Doe', + password: 'short', + }, + expectedErrorPath: ['password'], + }, + { + name: 'password without number', + request: { + inviterId: validUuid, + firstName: 'John', + lastName: 'Doe', + password: 'NoNumberPassword', + }, + expectedErrorPath: ['password'], + }, + ])('should fail validation for $name', ({ request, expectedErrorPath }) => { + const result = AcceptInvitationRequestDto.safeParse(request); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/invitation/__tests__/invite-users-request.dto.test.ts b/packages/@n8n/api-types/src/dto/invitation/__tests__/invite-users-request.dto.test.ts new file mode 100644 index 0000000000..f47a138ed5 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/invitation/__tests__/invite-users-request.dto.test.ts @@ -0,0 +1,60 @@ +import { InviteUsersRequestDto } from '../invite-users-request.dto'; + +describe('InviteUsersRequestDto', () => { + describe('Valid requests', () => { + test.each([ + { + name: 'empty array', + request: [], + }, + { + name: 'single user invitation with default role', + request: [{ email: 'user@example.com' }], + }, + { + name: 'multiple user invitations with different roles', + request: [ + { email: 'user1@example.com', role: 'global:member' }, + { email: 'user2@example.com', role: 'global:admin' }, + ], + }, + ])('should validate $name', ({ request }) => { + const result = InviteUsersRequestDto.safeParse(request); + expect(result.success).toBe(true); + }); + + it('should default role to global:member', () => { + const result = InviteUsersRequestDto.safeParse([{ email: 'user@example.com' }]); + expect(result.success).toBe(true); + expect(result.data?.[0].role).toBe('global:member'); + }); + }); + + describe('Invalid requests', () => { + test.each([ + { + name: 'invalid email', + request: [{ email: 'invalid-email' }], + expectedErrorPath: [0, 'email'], + }, + { + name: 'invalid role', + request: [ + { + email: 'user@example.com', + role: 'invalid-role', + }, + ], + expectedErrorPath: [0, 'role'], + }, + ])('should fail validation for $name', ({ request, expectedErrorPath }) => { + const result = InviteUsersRequestDto.safeParse(request); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/invitation/accept-invitation-request.dto.ts b/packages/@n8n/api-types/src/dto/invitation/accept-invitation-request.dto.ts new file mode 100644 index 0000000000..7c93e708ba --- /dev/null +++ b/packages/@n8n/api-types/src/dto/invitation/accept-invitation-request.dto.ts @@ -0,0 +1,11 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +import { passwordSchema } from '../../schemas/password.schema'; + +export class AcceptInvitationRequestDto extends Z.class({ + inviterId: z.string().uuid(), + firstName: z.string().min(1, 'First name is required'), + lastName: z.string().min(1, 'Last name is required'), + password: passwordSchema, +}) {} diff --git a/packages/@n8n/api-types/src/dto/invitation/invite-users-request.dto.ts b/packages/@n8n/api-types/src/dto/invitation/invite-users-request.dto.ts new file mode 100644 index 0000000000..9693234c64 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/invitation/invite-users-request.dto.ts @@ -0,0 +1,16 @@ +import { z } from 'zod'; + +const roleSchema = z.enum(['global:member', 'global:admin']); + +const invitedUserSchema = z.object({ + email: z.string().email(), + role: roleSchema.default('global:member'), +}); + +const invitationsSchema = z.array(invitedUserSchema); + +export class InviteUsersRequestDto extends Array> { + static safeParse(data: unknown) { + return invitationsSchema.safeParse(data); + } +} diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index b446ca3971..a003d54201 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -5,6 +5,8 @@ export type * from './scaling'; export type * from './frontend-settings'; export type * from './user'; -export type { BannerName } from './schemas/bannerName.schema'; export type { Collaborator } from './push/collaboration'; export type { SendWorkerStatusMessage } from './push/worker'; + +export type { BannerName } from './schemas/bannerName.schema'; +export { passwordSchema } from './schemas/password.schema'; diff --git a/packages/@n8n/api-types/src/schemas/__tests__/paswword.schema.test.ts b/packages/@n8n/api-types/src/schemas/__tests__/paswword.schema.test.ts new file mode 100644 index 0000000000..c3bcd5f4c8 --- /dev/null +++ b/packages/@n8n/api-types/src/schemas/__tests__/paswword.schema.test.ts @@ -0,0 +1,54 @@ +import { passwordSchema } from '../password.schema'; + +describe('passwordSchema', () => { + test('should throw on empty password', () => { + const check = () => passwordSchema.parse(''); + + expect(check).toThrowError('Password must be 8 to 64 characters long'); + }); + + test('should return same password if valid', () => { + const validPassword = 'abcd1234X'; + + const validated = passwordSchema.parse(validPassword); + + expect(validated).toBe(validPassword); + }); + + test('should require at least one uppercase letter', () => { + const invalidPassword = 'abcd1234'; + + const failingCheck = () => passwordSchema.parse(invalidPassword); + + expect(failingCheck).toThrowError('Password must contain at least 1 uppercase letter.'); + }); + + test('should require at least one number', () => { + const validPassword = 'abcd1234X'; + const invalidPassword = 'abcdEFGH'; + + const validated = passwordSchema.parse(validPassword); + + expect(validated).toBe(validPassword); + + const check = () => passwordSchema.parse(invalidPassword); + + expect(check).toThrowError('Password must contain at least 1 number.'); + }); + + test('should require a minimum length of 8 characters', () => { + const invalidPassword = 'a'.repeat(7); + + const check = () => passwordSchema.parse(invalidPassword); + + expect(check).toThrowError('Password must be 8 to 64 characters long.'); + }); + + test('should require a maximum length of 64 characters', () => { + const invalidPassword = 'a'.repeat(65); + + const check = () => passwordSchema.parse(invalidPassword); + + expect(check).toThrowError('Password must be 8 to 64 characters long.'); + }); +}); diff --git a/packages/cli/src/controllers/invitation.controller.ts b/packages/cli/src/controllers/invitation.controller.ts index d28b0671e0..d8a0e503f6 100644 --- a/packages/cli/src/controllers/invitation.controller.ts +++ b/packages/cli/src/controllers/invitation.controller.ts @@ -1,20 +1,20 @@ +import { AcceptInvitationRequestDto, InviteUsersRequestDto } from '@n8n/api-types'; import { Response } from 'express'; import { Logger } from 'n8n-core'; -import validator from 'validator'; import { AuthService } from '@/auth/auth.service'; import config from '@/config'; import { RESPONSE_ERROR_MESSAGES } from '@/constants'; import type { User } from '@/databases/entities/user'; import { UserRepository } from '@/databases/repositories/user.repository'; -import { Post, GlobalScope, RestController } from '@/decorators'; +import { Post, GlobalScope, RestController, Body, Param } from '@/decorators'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { EventService } from '@/events/event.service'; import { ExternalHooks } from '@/external-hooks'; import { License } from '@/license'; import { PostHogClient } from '@/posthog'; -import { UserRequest } from '@/requests'; +import { AuthenticatedRequest, AuthlessRequest } from '@/requests'; import { PasswordUtility } from '@/services/password.utility'; import { UserService } from '@/services/user.service'; import { isSamlLicensedAndEnabled } from '@/sso.ee/saml/saml-helpers'; @@ -39,7 +39,13 @@ export class InvitationController { @Post('/', { rateLimit: { limit: 10 } }) @GlobalScope('user:create') - async inviteUser(req: UserRequest.Invite) { + async inviteUser( + req: AuthenticatedRequest, + _res: Response, + @Body invitations: InviteUsersRequestDto, + ) { + if (invitations.length === 0) return []; + const isWithinUsersLimit = this.license.isWithinUsersLimit(); if (isSamlLicensedAndEnabled()) { @@ -65,50 +71,15 @@ export class InvitationController { throw new BadRequestError('You must set up your own account before inviting others'); } - if (!Array.isArray(req.body)) { - this.logger.debug( - 'Request to send email invite(s) to user(s) failed because the payload is not an array', - { - payload: req.body, - }, - ); - throw new BadRequestError('Invalid payload'); - } - - if (!req.body.length) return []; - - req.body.forEach((invite) => { - if (typeof invite !== 'object' || !invite.email) { - throw new BadRequestError( - 'Request to send email invite(s) to user(s) failed because the payload is not an array shaped Array<{ email: string }>', - ); - } - - if (!validator.isEmail(invite.email)) { - this.logger.debug('Invalid email in payload', { invalidEmail: invite.email }); - throw new BadRequestError( - `Request to send email invite(s) to user(s) failed because of an invalid email address: ${invite.email}`, - ); - } - - if (invite.role && !['global:member', 'global:admin'].includes(invite.role)) { - throw new BadRequestError( - `Cannot invite user with invalid role: ${invite.role}. Please ensure all invitees' roles are either 'global:member' or 'global:admin'.`, - ); - } - - if (invite.role === 'global:admin' && !this.license.isAdvancedPermissionsLicensed()) { + const attributes = invitations.map(({ email, role }) => { + if (role === 'global:admin' && !this.license.isAdvancedPermissionsLicensed()) { throw new ForbiddenError( 'Cannot invite admin user without advanced permissions. Please upgrade to a license that includes this feature.', ); } + return { email, role }; }); - const attributes = req.body.map(({ email, role }) => ({ - email, - role: role ?? 'global:member', - })); - const { usersInvited, usersCreated } = await this.userService.inviteUsers(req.user, attributes); await this.externalHooks.run('user.invited', [usersCreated]); @@ -120,20 +91,13 @@ export class InvitationController { * Fill out user shell with first name, last name, and password. */ @Post('/:id/accept', { skipAuth: true }) - async acceptInvitation(req: UserRequest.Update, res: Response) { - const { id: inviteeId } = req.params; - - const { inviterId, firstName, lastName, password } = req.body; - - if (!inviterId || !inviteeId || !firstName || !lastName || !password) { - this.logger.debug( - 'Request to fill out a user shell failed because of missing properties in payload', - { payload: req.body }, - ); - throw new BadRequestError('Invalid payload'); - } - - const validPassword = this.passwordUtility.validate(password); + async acceptInvitation( + req: AuthlessRequest, + res: Response, + @Body payload: AcceptInvitationRequestDto, + @Param('id') inviteeId: string, + ) { + const { inviterId, firstName, lastName, password } = payload; const users = await this.userRepository.findManyByIds([inviterId, inviteeId]); @@ -160,7 +124,7 @@ export class InvitationController { invitee.firstName = firstName; invitee.lastName = lastName; - invitee.password = await this.passwordUtility.hash(validPassword); + invitee.password = await this.passwordUtility.hash(password); const updatedUser = await this.userRepository.save(invitee, { transaction: false }); diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index 5de2e49e96..bb42d13878 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -1,4 +1,5 @@ import { + passwordSchema, PasswordUpdateRequestDto, SettingsUpdateRequestDto, UserUpdateRequestDto, @@ -122,10 +123,6 @@ export class MeController { ); } - if (typeof currentPassword !== 'string' || typeof newPassword !== 'string') { - throw new BadRequestError('Invalid payload.'); - } - if (!user.password) { throw new BadRequestError('Requesting user not set up.'); } @@ -135,7 +132,12 @@ export class MeController { throw new BadRequestError('Provided current password is incorrect.'); } - const validPassword = this.passwordUtility.validate(newPassword); + const passwordValidation = passwordSchema.safeParse(newPassword); + if (!passwordValidation.success) { + throw new BadRequestError( + passwordValidation.error.errors.map(({ message }) => message).join(' '), + ); + } if (user.mfaEnabled) { if (typeof mfaCode !== 'string') { @@ -148,7 +150,7 @@ export class MeController { } } - user.password = await this.passwordUtility.hash(validPassword); + user.password = await this.passwordUtility.hash(newPassword); const updatedUser = await this.userRepository.save(user, { transaction: false }); this.logger.info('Password updated successfully', { userId: user.id }); diff --git a/packages/cli/src/decorators/controller.registry.ts b/packages/cli/src/decorators/controller.registry.ts index fd0cbf4eec..c68945ca07 100644 --- a/packages/cli/src/decorators/controller.registry.ts +++ b/packages/cli/src/decorators/controller.registry.ts @@ -93,7 +93,7 @@ export class ControllerRegistry { 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) { + if (paramType && 'safeParse' in paramType) { const output = paramType.safeParse(req[arg.type]); if (output.success) args.push(output.data); else { 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 3b84b89da3..e6c36be3ce 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,4 +1,4 @@ -import { RoleChangeRequestDto } from '@n8n/api-types'; +import { InviteUsersRequestDto, RoleChangeRequestDto } from '@n8n/api-types'; import type express from 'express'; import type { Response } from 'express'; import { Container } from 'typedi'; @@ -18,7 +18,7 @@ import { } from '../../shared/middlewares/global.middleware'; import { encodeNextCursor } from '../../shared/services/pagination.service'; -type Create = UserRequest.Invite; +type Create = AuthenticatedRequest<{}, {}, InviteUsersRequestDto>; type Delete = UserRequest.Delete; type ChangeRole = AuthenticatedRequest<{ id: string }, {}, RoleChangeRequestDto, {}>; @@ -82,8 +82,16 @@ export = { createUser: [ globalScope('user:create'), async (req: Create, res: Response) => { - const usersInvited = await Container.get(InvitationController).inviteUser(req); + const { data, error } = InviteUsersRequestDto.safeParse(req.body); + if (error) { + return res.status(400).json(error.errors[0]); + } + const usersInvited = await Container.get(InvitationController).inviteUser( + req, + res, + data as InviteUsersRequestDto, + ); return res.status(201).json(usersInvited); }, ], diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index b49c46c7ff..12a3533c99 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -199,12 +199,6 @@ export declare namespace MeRequest { // ---------------------------------- export declare namespace UserRequest { - export type Invite = AuthenticatedRequest< - {}, - {}, - Array<{ email: string; role?: AssignableRole }> - >; - export type InviteResponse = { user: { id: string; @@ -231,19 +225,6 @@ export declare namespace UserRequest { >; export type PasswordResetLink = AuthenticatedRequest<{ id: string }, {}, {}, {}>; - - export type Reinvite = AuthenticatedRequest<{ id: string }>; - - export type Update = AuthlessRequest< - { id: string }, - {}, - { - inviterId: string; - firstName: string; - lastName: string; - password: string; - } - >; } // ---------------------------------- diff --git a/packages/cli/src/services/__tests__/password.utility.test.ts b/packages/cli/src/services/__tests__/password.utility.test.ts index 48be7588c2..ff2956844b 100644 --- a/packages/cli/src/services/__tests__/password.utility.test.ts +++ b/packages/cli/src/services/__tests__/password.utility.test.ts @@ -49,57 +49,4 @@ describe('PasswordUtility', () => { expect(isMatch).toBe(false); }); }); - - describe('validate()', () => { - test('should throw on empty password', () => { - const check = () => passwordUtility.validate(); - - expect(check).toThrowError('Password is mandatory'); - }); - - test('should return same password if valid', () => { - const validPassword = 'abcd1234X'; - - const validated = passwordUtility.validate(validPassword); - - expect(validated).toBe(validPassword); - }); - - test('should require at least one uppercase letter', () => { - const invalidPassword = 'abcd1234'; - - const failingCheck = () => passwordUtility.validate(invalidPassword); - - expect(failingCheck).toThrowError('Password must contain at least 1 uppercase letter.'); - }); - - test('should require at least one number', () => { - const validPassword = 'abcd1234X'; - const invalidPassword = 'abcdEFGH'; - - const validated = passwordUtility.validate(validPassword); - - expect(validated).toBe(validPassword); - - const check = () => passwordUtility.validate(invalidPassword); - - expect(check).toThrowError('Password must contain at least 1 number.'); - }); - - test('should require a minimum length of 8 characters', () => { - const invalidPassword = 'a'.repeat(7); - - const check = () => passwordUtility.validate(invalidPassword); - - expect(check).toThrowError('Password must be 8 to 64 characters long.'); - }); - - test('should require a maximum length of 64 characters', () => { - const invalidPassword = 'a'.repeat(65); - - const check = () => passwordUtility.validate(invalidPassword); - - expect(check).toThrowError('Password must be 8 to 64 characters long.'); - }); - }); }); diff --git a/packages/cli/src/services/password.utility.ts b/packages/cli/src/services/password.utility.ts index 6ae6aad61f..352002fa95 100644 --- a/packages/cli/src/services/password.utility.ts +++ b/packages/cli/src/services/password.utility.ts @@ -1,12 +1,6 @@ import { compare, hash } from 'bcryptjs'; import { Service as Utility } from 'typedi'; -import { - MAX_PASSWORD_CHAR_LENGTH as maxLength, - MIN_PASSWORD_CHAR_LENGTH as minLength, -} from '@/constants'; -import { BadRequestError } from '@/errors/response-errors/bad-request.error'; - const SALT_ROUNDS = 10; @Utility() @@ -18,29 +12,4 @@ export class PasswordUtility { async compare(plaintext: string, hashed: string) { return await compare(plaintext, hashed); } - - /** @deprecated. All input validation should move to DTOs */ - validate(plaintext?: string) { - if (!plaintext) throw new BadRequestError('Password is mandatory'); - - const errorMessages: string[] = []; - - if (plaintext.length < minLength || plaintext.length > maxLength) { - errorMessages.push(`Password must be ${minLength} to ${maxLength} characters long.`); - } - - if (!/\d/.test(plaintext)) { - errorMessages.push('Password must contain at least 1 number.'); - } - - if (!/[A-Z]/.test(plaintext)) { - errorMessages.push('Password must contain at least 1 uppercase letter.'); - } - - if (errorMessages.length > 0) { - throw new BadRequestError(errorMessages.join(' ')); - } - - return plaintext; - } }