diff --git a/packages/@n8n/db/src/entities/user.ts b/packages/@n8n/db/src/entities/user.ts index 4867730b7f..472a80a602 100644 --- a/packages/@n8n/db/src/entities/user.ts +++ b/packages/@n8n/db/src/entities/user.ts @@ -53,9 +53,9 @@ export class User extends WithTimestamps implements IUser, AuthPrincipal { @Length(1, 32, { message: 'Last name must be $constraint1 to $constraint2 characters long.' }) lastName: string; - @Column({ nullable: true }) + @Column({ type: String, nullable: true }) @IsString({ message: 'Password must be of type string.' }) - password: string; + password: string | null; @JsonColumn({ nullable: true, diff --git a/packages/cli/src/controllers/users.controller.ts b/packages/cli/src/controllers/users.controller.ts index 91f1798f1e..a101097474 100644 --- a/packages/cli/src/controllers/users.controller.ts +++ b/packages/cli/src/controllers/users.controller.ts @@ -41,6 +41,7 @@ import { FolderService } from '@/services/folder.service'; import { ProjectService } from '@/services/project.service.ee'; import { UserService } from '@/services/user.service'; import { WorkflowService } from '@/workflows/workflow.service'; +import { hasGlobalScope } from '@n8n/permissions'; @RestController('/users') export class UsersController { @@ -106,10 +107,12 @@ export class UsersController { const [users, count] = response; + const withInviteUrl = hasGlobalScope(req.user, 'user:create'); + const publicUsers = await Promise.all( users.map(async (u) => { const user = await this.userService.toPublic(u, { - withInviteUrl: true, + withInviteUrl, inviterId: req.user.id, }); return { diff --git a/packages/cli/src/external-hooks.ts b/packages/cli/src/external-hooks.ts index 8688c32fb0..07111b88d3 100644 --- a/packages/cli/src/external-hooks.ts +++ b/packages/cli/src/external-hooks.ts @@ -61,7 +61,7 @@ type ExternalHooksMap = { payload: UserUpdateRequestDto, ]; 'user.profile.update': [currentEmail: string, publicUser: PublicUser]; - 'user.password.update': [updatedEmail: string, updatedPassword: string]; + 'user.password.update': [updatedEmail: string, updatedPassword: string | null]; 'user.invited': [emails: string[]]; 'workflow.create': [createdWorkflow: IWorkflowBase]; diff --git a/packages/cli/src/services/password.utility.ts b/packages/cli/src/services/password.utility.ts index f4119c47a0..b75b180faa 100644 --- a/packages/cli/src/services/password.utility.ts +++ b/packages/cli/src/services/password.utility.ts @@ -9,7 +9,10 @@ export class PasswordUtility { return await hash(plaintext, SALT_ROUNDS); } - async compare(plaintext: string, hashed: string) { + async compare(plaintext: string, hashed: string | null) { + if (hashed === null) { + return false; + } return await compare(plaintext, hashed); } } diff --git a/packages/cli/test/integration/controllers/invitation/invitation.controller.integration.test.ts b/packages/cli/test/integration/controllers/invitation/invitation.controller.integration.test.ts index 47d9172047..ca00de5f99 100644 --- a/packages/cli/test/integration/controllers/invitation/invitation.controller.integration.test.ts +++ b/packages/cli/test/integration/controllers/invitation/invitation.controller.integration.test.ts @@ -188,7 +188,7 @@ describe('InvitationController', () => { expect(storedMember.password).not.toBe(memberProps.password); const comparisonResult = await Container.get(PasswordUtility).compare( - member.password, + member.password!, storedMember.password, ); diff --git a/packages/cli/test/integration/password-reset.api.test.ts b/packages/cli/test/integration/password-reset.api.test.ts index 3a22405b0b..1a13eac312 100644 --- a/packages/cli/test/integration/password-reset.api.test.ts +++ b/packages/cli/test/integration/password-reset.api.test.ts @@ -279,7 +279,7 @@ describe('POST /change-password', () => { id: owner.id, }); - const comparisonResult = await compare(passwordToStore, storedPassword); + const comparisonResult = await compare(passwordToStore, storedPassword!); expect(comparisonResult).toBe(true); expect(storedPassword).not.toBe(passwordToStore); diff --git a/packages/cli/test/integration/shared/db/users.ts b/packages/cli/test/integration/shared/db/users.ts index a2eb097f44..64aefcc929 100644 --- a/packages/cli/test/integration/shared/db/users.ts +++ b/packages/cli/test/integration/shared/db/users.ts @@ -18,12 +18,25 @@ type ApiKeyOptions = { // pre-computed bcrypt hash for the string 'password', using `await hash('password', 10)` const passwordHash = '$2a$10$njedH7S6V5898mj6p0Jr..IGY9Ms.qNwR7RbSzzX9yubJocKfvGGK'; +// A null password value means that no password will be set in the database +// rendering the user as pending, an undefined value means we default +// to 'password' as password. +// Also we are hashing the plaintext password here if necessary +async function handlePasswordSetup(password: string | null | undefined): Promise { + if (password === undefined) { + return passwordHash; + } else if (password === null) { + return null; + } + return await hash(password, 1); +} + /** Store a new user object, defaulting to a `member` */ export async function newUser(attributes: Partial = {}): Promise { const { email, password, firstName, lastName, role, ...rest } = attributes; return Container.get(UserRepository).create({ email: email ?? randomEmail(), - password: password ? await hash(password, 1) : passwordHash, + password: await handlePasswordSetup(password), firstName: firstName ?? randomName(), lastName: lastName ?? randomName(), role: role ?? 'global:member', diff --git a/packages/cli/test/integration/users.api.test.ts b/packages/cli/test/integration/users.api.test.ts index 860345169c..49167b1bc3 100644 --- a/packages/cli/test/integration/users.api.test.ts +++ b/packages/cli/test/integration/users.api.test.ts @@ -9,7 +9,7 @@ import { testDb, mockInstance, } from '@n8n/backend-test-utils'; -import type { User } from '@n8n/db'; +import type { PublicUser, User } from '@n8n/db'; import { FolderRepository, ProjectRelationRepository, @@ -52,6 +52,7 @@ describe('GET /users', () => { let member1: User; let member2: User; let ownerAgent: SuperAgentTest; + let memberAgent: SuperAgentTest; let userRepository: UserRepository; beforeAll(async () => { @@ -91,6 +92,7 @@ describe('GET /users', () => { } ownerAgent = testServer.authAgentFor(owner); + memberAgent = testServer.authAgentFor(member1); }); test('should return all users', async () => { @@ -570,51 +572,62 @@ describe('GET /users', () => { }); describe('inviteAcceptUrl', () => { - test('should include inviteAcceptUrl for pending users', async () => { - // Create a pending user - const pendingUser = await createUser({ + let pendingUser: User; + beforeAll(async () => { + pendingUser = await createUser({ role: 'global:member', email: 'pending@n8n.io', firstName: 'PendingFirstName', lastName: 'PendingLastName', + password: null, }); + }); - await userRepository.update( - { id: pendingUser.id }, - { - password: null as unknown as string, - }, + afterAll(async () => { + await userRepository.delete({ id: pendingUser.id }); + }); + + test('should include inviteAcceptUrl for pending users', async () => { + const response = await ownerAgent.get('/users').expect(200); + + const responseData = response.body.data as { + count: number; + items: PublicUser[]; + }; + + const pendingUserInResponse = responseData.items.find((user) => user.id === pendingUser.id); + + expect(pendingUserInResponse).toBeDefined(); + expect(pendingUserInResponse!.inviteAcceptUrl).toBeDefined(); + expect(pendingUserInResponse!.inviteAcceptUrl).toMatch( + new RegExp(`/signup\\?inviterId=${owner.id}&inviteeId=${pendingUser.id}`), ); - try { - const response = await ownerAgent.get('/users').expect(200); + const nonPendingUser = responseData.items.find((user) => user.id === member1.id); - expect(response.body.data).toHaveProperty('count'); - expect(response.body.data).toHaveProperty('items'); + expect(nonPendingUser).toBeDefined(); + expect(nonPendingUser!.isPending).toBe(false); + expect(nonPendingUser!.inviteAcceptUrl).toBeUndefined(); + }); - // Find the pending user in the response - const pendingUserInResponse = response.body.data.items.find( - (user: any) => user.id === pendingUser.id, - ); + test('should not include inviteAcceptUrl for pending users, if member requests it', async () => { + const response = await memberAgent.get('/users').expect(200); - expect(pendingUserInResponse).toBeDefined(); - expect(pendingUserInResponse.inviteAcceptUrl).toBeDefined(); - expect(pendingUserInResponse.inviteAcceptUrl).toMatch( - new RegExp(`/signup\\?inviterId=${owner.id}&inviteeId=${pendingUser.id}`), - ); + const responseData = response.body.data as { + count: number; + items: PublicUser[]; + }; - // Verify that non-pending users don't have inviteAcceptUrl - const nonPendingUser = response.body.data.items.find( - (user: any) => user.id === member1.id, - ); + const pendingUserInResponse = responseData.items.find((user) => user.id === pendingUser.id); - expect(nonPendingUser).toBeDefined(); - expect(nonPendingUser.isPending).toBe(false); - expect(nonPendingUser.inviteAcceptUrl).toBeUndefined(); - } finally { - // Clean up - await userRepository.delete({ id: pendingUser.id }); - } + expect(pendingUserInResponse).toBeDefined(); + expect(pendingUserInResponse!.inviteAcceptUrl).not.toBeDefined(); + + const nonPendingUser = responseData.items.find((user) => user.id === member1.id); + + expect(nonPendingUser).toBeDefined(); + expect(nonPendingUser!.isPending).toBe(false); + expect(nonPendingUser!.inviteAcceptUrl).toBeUndefined(); }); });