refactor(core): Update invitation endpoints to use DTOs (#12377)
Some checks are pending
Test Master / install-and-build (push) Waiting to run
Test Master / Unit tests (18.x) (push) Blocked by required conditions
Test Master / Unit tests (20.x) (push) Blocked by required conditions
Test Master / Unit tests (22.4) (push) Blocked by required conditions
Test Master / Lint (push) Blocked by required conditions
Test Master / Notify Slack on failure (push) Blocked by required conditions

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™ 2024-12-26 18:24:14 +01:00 committed by GitHub
parent 371a09de96
commit 7b2630d1a0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 282 additions and 171 deletions

View file

@ -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';

View file

@ -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);
}
});
});
});

View file

@ -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);
}
});
});
});

View file

@ -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,
}) {}

View file

@ -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<z.infer<typeof invitedUserSchema>> {
static safeParse(data: unknown) {
return invitationsSchema.safeParse(data);
}
}

View file

@ -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';

View file

@ -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.');
});
});

View file

@ -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 });

View file

@ -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 });

View file

@ -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 {

View file

@ -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);
},
],

View file

@ -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;
}
>;
}
// ----------------------------------

View file

@ -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.');
});
});
});

View file

@ -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;
}
}