chore(core): Hide invite URL in users list if not an admin (#17101)

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,

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 {

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];

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

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,
 			);
 

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

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<string | null> {
+	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<User> = {}): Promise<User> {
 	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',

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