fix(core): Prevent calling internal hook email event if emailing is disabled (#8462)

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>

packages/cli/src/UserManagement/email/UserManagementMailer.ts

@@ -1,12 +1,22 @@
+import { Container, Service } from 'typedi';
 import { existsSync } from 'fs';
 import { readFile } from 'fs/promises';
 import Handlebars from 'handlebars';
 import { join as pathJoin } from 'path';
-import { Container, Service } from 'typedi';
+import { ApplicationError } from 'n8n-workflow';
+
 import config from '@/config';
+import type { User } from '@db/entities/User';
+import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
+import { UserRepository } from '@db/repositories/user.repository';
+import { InternalHooks } from '@/InternalHooks';
+import { Logger } from '@/Logger';
+import { UrlService } from '@/services/url.service';
+import { InternalServerError } from '@/errors/response-errors/internal-server.error';
+import { toError } from '@/utils';
+
 import type { InviteEmailData, PasswordResetData, SendEmailResult } from './Interfaces';
 import { NodeMailer } from './NodeMailer';
-import { ApplicationError } from 'n8n-workflow';
 
 type Template = HandlebarsTemplateDelegate<unknown>;
 type TemplateName = 'invite' | 'passwordReset' | 'workflowShared' | 'credentialsShared';
@@ -39,7 +49,11 @@ export class UserManagementMailer {
 
 	private mailer: NodeMailer | undefined;
 
-	constructor() {
+	constructor(
+		private readonly userRepository: UserRepository,
+		private readonly logger: Logger,
+		private readonly urlService: UrlService,
+	) {
 		this.isEmailSetUp =
 			config.getEnv('userManagement.emails.mode') === 'smtp' &&
 			config.getEnv('userManagement.emails.smtp.host') !== '';
@@ -83,48 +97,109 @@ export class UserManagementMailer {
 	}
 
 	async notifyWorkflowShared({
-		recipientEmails,
+		sharer,
-		workflowName,
+		newShareeIds,
-		baseUrl,
+		workflow,
-		workflowId,
-		sharerFirstName,
 	}: {
-		recipientEmails: string[];
+		sharer: User;
-		workflowName: string;
+		newShareeIds: string[];
-		baseUrl: string;
+		workflow: WorkflowEntity;
-		workflowId: string;
-		sharerFirstName: string;
 	}) {
+		if (!this.mailer) return;
+
+		const recipients = await this.userRepository.getEmailsByIds(newShareeIds);
+
+		if (recipients.length === 0) return;
+
+		const emailRecipients = recipients.map(({ email }) => email);
+
 		const populateTemplate = await getTemplate('workflowShared', 'workflowShared.html');
 
-		const result = await this.mailer?.sendMail({
+		const baseUrl = this.urlService.getInstanceBaseUrl();
-			emailRecipients: recipientEmails,
+
-			subject: `${sharerFirstName} has shared an n8n workflow with you`,
+		try {
-			body: populateTemplate({ workflowName, workflowUrl: `${baseUrl}/workflow/${workflowId}` }),
+			const result = await this.mailer.sendMail({
+				emailRecipients,
+				subject: `${sharer.firstName} has shared an n8n workflow with you`,
+				body: populateTemplate({
+					workflowName: workflow.name,
+					workflowUrl: `${baseUrl}/workflow/${workflow.id}`,
+				}),
 			});
 
-		return result ?? { emailSent: false };
+			if (!result) return { emailSent: false };
+
+			this.logger.info('Sent workflow shared email successfully', { sharerId: sharer.id });
+
+			void Container.get(InternalHooks).onUserTransactionalEmail({
+				user_id: sharer.id,
+				message_type: 'Workflow shared',
+				public_api: false,
+			});
+
+			return result;
+		} catch (e) {
+			void Container.get(InternalHooks).onEmailFailed({
+				user: sharer,
+				message_type: 'Workflow shared',
+				public_api: false,
+			});
+
+			const error = toError(e);
+
+			throw new InternalServerError(`Please contact your administrator: ${error.message}`);
+		}
 	}
 
 	async notifyCredentialsShared({
-		sharerFirstName,
+		sharer,
+		newShareeIds,
 		credentialsName,
-		recipientEmails,
-		baseUrl,
 	}: {
-		sharerFirstName: string;
+		sharer: User;
+		newShareeIds: string[];
 		credentialsName: string;
-		recipientEmails: string[];
-		baseUrl: string;
 	}) {
+		if (!this.mailer) return;
+
+		const recipients = await this.userRepository.getEmailsByIds(newShareeIds);
+
+		if (recipients.length === 0) return;
+
+		const emailRecipients = recipients.map(({ email }) => email);
+
 		const populateTemplate = await getTemplate('credentialsShared', 'credentialsShared.html');
 
-		const result = await this.mailer?.sendMail({
+		const baseUrl = this.urlService.getInstanceBaseUrl();
-			emailRecipients: recipientEmails,
+
-			subject: `${sharerFirstName} has shared an n8n credential with you`,
+		try {
+			const result = await this.mailer.sendMail({
+				emailRecipients,
+				subject: `${sharer.firstName} has shared an n8n credential with you`,
 				body: populateTemplate({ credentialsName, credentialsListUrl: `${baseUrl}/credentials` }),
 			});
 
-		return result ?? { emailSent: false };
+			if (!result) return { emailSent: false };
+
+			this.logger.info('Sent credentials shared email successfully', { sharerId: sharer.id });
+
+			void Container.get(InternalHooks).onUserTransactionalEmail({
+				user_id: sharer.id,
+				message_type: 'Credentials shared',
+				public_api: false,
+			});
+
+			return result;
+		} catch (e) {
+			void Container.get(InternalHooks).onEmailFailed({
+				user: sharer,
+				message_type: 'Credentials shared',
+				public_api: false,
+			});
+
+			const error = toError(e);
+
+			throw new InternalServerError(`Please contact your administrator: ${error.message}`);
+		}
 	}
 }

packages/cli/src/credentials/credentials.controller.ee.ts

@@ -15,11 +15,7 @@ import { NotFoundError } from '@/errors/response-errors/not-found.error';
 import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error';
 import { CredentialsRepository } from '@/databases/repositories/credentials.repository';
 import * as utils from '@/utils';
-import { UserRepository } from '@/databases/repositories/user.repository';
 import { UserManagementMailer } from '@/UserManagement/email';
-import { UrlService } from '@/services/url.service';
-import { Logger } from '@/Logger';
-import { InternalServerError } from '@/errors/response-errors/internal-server.error';
 
 export const EECredentialsController = express.Router();
 
@@ -190,36 +186,10 @@ EECredentialsController.put(
 			sharees_removed: amountRemoved,
 		});
 
-		const recipients = await Container.get(UserRepository).getEmailsByIds(newShareeIds);
-
-		if (recipients.length === 0) return;
-
-		try {
 		await Container.get(UserManagementMailer).notifyCredentialsShared({
-				sharerFirstName: req.user.firstName,
+			sharer: req.user,
+			newShareeIds,
 			credentialsName: credential.name,
-				recipientEmails: recipients.map(({ email }) => email),
-				baseUrl: Container.get(UrlService).getInstanceBaseUrl(),
-			});
-		} catch (error) {
-			void Container.get(InternalHooks).onEmailFailed({
-				user: req.user,
-				message_type: 'Credentials shared',
-				public_api: false,
-			});
-			if (error instanceof Error) {
-				throw new InternalServerError(`Please contact your administrator: ${error.message}`);
-			}
-		}
-
-		Container.get(Logger).info('Sent credentials shared email successfully', {
-			sharerId: req.user.id,
-		});
-
-		void Container.get(InternalHooks).onUserTransactionalEmail({
-			user_id: req.user.id,
-			message_type: 'Credentials shared',
-			public_api: false,
 		});
 	}),
 );

packages/cli/src/workflows/workflows.controller.ts

@@ -39,7 +39,6 @@ import { EnterpriseWorkflowService } from './workflow.service.ee';
 import { WorkflowExecutionService } from './workflowExecution.service';
 import { WorkflowSharingService } from './workflowSharing.service';
 import { UserManagementMailer } from '@/UserManagement/email';
-import { UrlService } from '@/services/url.service';
 
 @Service()
 @Authorized()
@@ -63,7 +62,6 @@ export class WorkflowsController {
 		private readonly userRepository: UserRepository,
 		private readonly license: License,
 		private readonly mailer: UserManagementMailer,
-		private readonly urlService: UrlService,
 	) {}
 
 	@Post('/')
@@ -403,35 +401,10 @@ export class WorkflowsController {
 
 		void this.internalHooks.onWorkflowSharingUpdate(workflowId, req.user.id, shareWithIds);
 
-		const recipients = await this.userRepository.getEmailsByIds(newShareeIds);
-
-		if (recipients.length === 0) return;
-
-		try {
 		await this.mailer.notifyWorkflowShared({
-				recipientEmails: recipients.map(({ email }) => email),
+			sharer: req.user,
-				workflowName: workflow.name,
+			newShareeIds,
-				workflowId,
+			workflow,
-				sharerFirstName: req.user.firstName,
-				baseUrl: this.urlService.getInstanceBaseUrl(),
-			});
-		} catch (error) {
-			void this.internalHooks.onEmailFailed({
-				user: req.user,
-				message_type: 'Workflow shared',
-				public_api: false,
-			});
-			if (error instanceof Error) {
-				throw new InternalServerError(`Please contact your administrator: ${error.message}`);
-			}
-		}
-
-		this.logger.info('Sent workflow shared email successfully', { sharerId: req.user.id });
-
-		void this.internalHooks.onUserTransactionalEmail({
-			user_id: req.user.id,
-			message_type: 'Workflow shared',
-			public_api: false,
 		});
 	}
 }

packages/cli/test/integration/controllers/invitation/assertions.ts

@@ -0,0 +1,29 @@
+import validator from 'validator';
+import type { User } from '@/databases/entities/User';
+import type { UserInvitationResult } from '../../shared/utils/users';
+
+export function assertReturnedUserProps(user: User) {
+	expect(validator.isUUID(user.id)).toBe(true);
+	expect(user.email).toBeDefined();
+	expect(user.personalizationAnswers).toBeNull();
+	expect(user.password).toBeUndefined();
+	expect(user.isPending).toBe(false);
+	expect(user.apiKey).not.toBeDefined();
+	expect(user.globalScopes).toBeDefined();
+	expect(user.globalScopes).not.toHaveLength(0);
+}
+
+export const assertStoredUserProps = (user: User) => {
+	expect(user.firstName).toBeNull();
+	expect(user.lastName).toBeNull();
+	expect(user.personalizationAnswers).toBeNull();
+	expect(user.password).toBeNull();
+	expect(user.isPending).toBe(true);
+};
+
+export const assertUserInviteResult = (data: UserInvitationResult) => {
+	expect(validator.isUUID(data.user.id)).toBe(true);
+	expect(data.user.inviteAcceptUrl).toBeUndefined();
+	expect(data.user.email).toBeDefined();
+	expect(data.user.emailSent).toBe(true);
+};

packages/cli/test/integration/controllers/invitation/invitation.controller.integration.test.ts

@@ -0,0 +1,412 @@
+import { mocked } from 'jest-mock';
+import Container from 'typedi';
+import { Not } from 'typeorm';
+
+import { InternalHooks } from '@/InternalHooks';
+import { ExternalHooks } from '@/ExternalHooks';
+import { UserManagementMailer } from '@/UserManagement/email';
+import { UserRepository } from '@/databases/repositories/user.repository';
+import { PasswordUtility } from '@/services/password.utility';
+
+import {
+	randomEmail,
+	randomInvalidPassword,
+	randomName,
+	randomValidPassword,
+} from '../../shared/random';
+import { createMember, createOwner, createUserShell } from '../../shared/db/users';
+import { mockInstance } from '../../../shared/mocking';
+import * as utils from '../../shared/utils';
+
+import {
+	assertReturnedUserProps,
+	assertStoredUserProps,
+	assertUserInviteResult,
+} from './assertions';
+
+import type { User } from '@/databases/entities/User';
+import type { UserInvitationResult } from '../../shared/utils/users';
+
+describe('InvitationController', () => {
+	const mailer = mockInstance(UserManagementMailer);
+	const externalHooks = mockInstance(ExternalHooks);
+	const internalHooks = mockInstance(InternalHooks);
+
+	const testServer = utils.setupTestServer({ endpointGroups: ['invitations'] });
+
+	let instanceOwner: User;
+	let userRepository: UserRepository;
+
+	beforeAll(async () => {
+		userRepository = Container.get(UserRepository);
+		instanceOwner = await createOwner();
+	});
+
+	beforeEach(async () => {
+		jest.clearAllMocks();
+		await userRepository.delete({ role: Not('global:owner') });
+	});
+
+	afterEach(() => {
+		jest.restoreAllMocks();
+	});
+
+	describe('POST /invitations/:id/accept', () => {
+		test('should fill out a member shell', async () => {
+			const memberShell = await createUserShell('global:member');
+
+			const memberProps = {
+				inviterId: instanceOwner.id,
+				firstName: randomName(),
+				lastName: randomName(),
+				password: randomValidPassword(),
+			};
+
+			const response = await testServer.authlessAgent
+				.post(`/invitations/${memberShell.id}/accept`)
+				.send(memberProps)
+				.expect(200);
+
+			const { data: returnedMember } = response.body;
+
+			assertReturnedUserProps(returnedMember);
+
+			expect(returnedMember.firstName).toBe(memberProps.firstName);
+			expect(returnedMember.lastName).toBe(memberProps.lastName);
+			expect(returnedMember.role).toBe('global:member');
+			expect(utils.getAuthToken(response)).toBeDefined();
+
+			const storedMember = await userRepository.findOneByOrFail({ id: returnedMember.id });
+
+			expect(storedMember.firstName).toBe(memberProps.firstName);
+			expect(storedMember.lastName).toBe(memberProps.lastName);
+			expect(storedMember.password).not.toBe(memberProps.password);
+		});
+
+		test('should fill out an admin shell', async () => {
+			const adminShell = await createUserShell('global:admin');
+
+			const memberProps = {
+				inviterId: instanceOwner.id,
+				firstName: randomName(),
+				lastName: randomName(),
+				password: randomValidPassword(),
+			};
+
+			const response = await testServer.authlessAgent
+				.post(`/invitations/${adminShell.id}/accept`)
+				.send(memberProps)
+				.expect(200);
+
+			const { data: returnedAdmin } = response.body;
+
+			assertReturnedUserProps(returnedAdmin);
+
+			expect(returnedAdmin.firstName).toBe(memberProps.firstName);
+			expect(returnedAdmin.lastName).toBe(memberProps.lastName);
+			expect(returnedAdmin.role).toBe('global:admin');
+			expect(utils.getAuthToken(response)).toBeDefined();
+
+			const storedAdmin = await userRepository.findOneByOrFail({ id: returnedAdmin.id });
+
+			expect(storedAdmin.firstName).toBe(memberProps.firstName);
+			expect(storedAdmin.lastName).toBe(memberProps.lastName);
+			expect(storedAdmin.password).not.toBe(memberProps.password);
+		});
+
+		test('should fail with invalid payloads', async () => {
+			const memberShell = await userRepository.save({
+				email: randomEmail(),
+				role: 'global:member',
+			});
+
+			const invalidPaylods = [
+				{
+					firstName: randomName(),
+					lastName: randomName(),
+					password: randomValidPassword(),
+				},
+				{
+					inviterId: instanceOwner.id,
+					firstName: randomName(),
+					password: randomValidPassword(),
+				},
+				{
+					inviterId: instanceOwner.id,
+					firstName: randomName(),
+					password: randomValidPassword(),
+				},
+				{
+					inviterId: instanceOwner.id,
+					firstName: randomName(),
+					lastName: randomName(),
+				},
+				{
+					inviterId: instanceOwner.id,
+					firstName: randomName(),
+					lastName: randomName(),
+					password: randomInvalidPassword(),
+				},
+			];
+
+			for (const payload of invalidPaylods) {
+				await testServer.authlessAgent
+					.post(`/invitations/${memberShell.id}/accept`)
+					.send(payload)
+					.expect(400);
+
+				const storedMemberShell = await userRepository.findOneByOrFail({
+					email: memberShell.email,
+				});
+
+				expect(storedMemberShell.firstName).toBeNull();
+				expect(storedMemberShell.lastName).toBeNull();
+				expect(storedMemberShell.password).toBeNull();
+			}
+		});
+
+		test('should fail with already accepted invite', async () => {
+			const member = await createMember();
+
+			const memberProps = {
+				inviterId: instanceOwner.id,
+				firstName: randomName(),
+				lastName: randomName(),
+				password: randomValidPassword(),
+			};
+
+			await testServer.authlessAgent
+				.post(`/invitations/${member.id}/accept`)
+				.send(memberProps)
+				.expect(400);
+
+			const storedMember = await userRepository.findOneByOrFail({
+				email: member.email,
+			});
+
+			expect(storedMember.firstName).not.toBe(memberProps.firstName);
+			expect(storedMember.lastName).not.toBe(memberProps.lastName);
+			expect(storedMember.password).not.toBe(memberProps.password);
+
+			const comparisonResult = await Container.get(PasswordUtility).compare(
+				member.password,
+				storedMember.password,
+			);
+
+			expect(comparisonResult).toBe(false);
+		});
+	});
+
+	describe('POST /invitations', () => {
+		type InvitationResponse = { body: { data: UserInvitationResult[] } };
+
+		test('should fail with invalid payloads', async () => {
+			const invalidPayloads = [
+				randomEmail(),
+				[randomEmail()],
+				{},
+				[{ name: randomName() }],
+				[{ email: randomName() }],
+			];
+
+			for (const invalidPayload of invalidPayloads) {
+				await testServer
+					.authAgentFor(instanceOwner)
+					.post('/invitations')
+					.send(invalidPayload)
+					.expect(400);
+
+				await expect(userRepository.count()).resolves.toBe(2); // DB unaffected
+			}
+		});
+
+		test('should return 200 on empty payload', async () => {
+			const response = await testServer
+				.authAgentFor(instanceOwner)
+				.post('/invitations')
+				.send([])
+				.expect(200);
+
+			expect(response.body.data).toStrictEqual([]);
+
+			await expect(userRepository.count()).resolves.toBe(2); // DB unaffected
+		});
+
+		test('should return 200 if emailing is not set up', async () => {
+			mailer.invite.mockResolvedValue({ emailSent: false });
+
+			const response = await testServer
+				.authAgentFor(instanceOwner)
+				.post('/invitations')
+				.send([{ email: randomEmail() }]);
+
+			expect(response.body.data).toBeInstanceOf(Array);
+			expect(response.body.data.length).toBe(1);
+
+			const { user } = response.body.data[0];
+
+			expect(user.inviteAcceptUrl).toBeDefined();
+
+			const inviteUrl = new URL(user.inviteAcceptUrl);
+
+			expect(inviteUrl.searchParams.get('inviterId')).toBe(instanceOwner.id);
+			expect(inviteUrl.searchParams.get('inviteeId')).toBe(user.id);
+		});
+
+		test('should create member shell', async () => {
+			mailer.invite.mockResolvedValue({ emailSent: false });
+
+			const response: InvitationResponse = await testServer
+				.authAgentFor(instanceOwner)
+				.post('/invitations')
+				.send([{ email: randomEmail() }])
+				.expect(200);
+
+			const [result] = response.body.data;
+
+			const storedUser = await userRepository.findOneByOrFail({
+				id: result.user.id,
+			});
+
+			assertStoredUserProps(storedUser);
+		});
+
+		test('should create admin shell when advanced permissions is licensed', async () => {
+			testServer.license.enable('feat:advancedPermissions');
+
+			mailer.invite.mockResolvedValue({ emailSent: false });
+
+			const response: InvitationResponse = await testServer
+				.authAgentFor(instanceOwner)
+				.post('/invitations')
+				.send([{ email: randomEmail(), role: 'global:admin' }])
+				.expect(200);
+
+			const [result] = response.body.data;
+
+			const storedUser = await userRepository.findOneByOrFail({
+				id: result.user.id,
+			});
+
+			assertStoredUserProps(storedUser);
+		});
+
+		test('should reinvite member when sharing is licensed', async () => {
+			testServer.license.enable('feat:sharing');
+
+			mailer.invite.mockResolvedValue({ emailSent: false });
+
+			await testServer
+				.authAgentFor(instanceOwner)
+				.post('/invitations')
+				.send([{ email: randomEmail(), role: 'global:member' }]);
+
+			await testServer
+				.authAgentFor(instanceOwner)
+				.post('/invitations')
+				.send([{ email: randomEmail(), role: 'global:member' }])
+				.expect(200);
+		});
+
+		test('should reinvite admin when advanced permissions is licensed', async () => {
+			testServer.license.enable('feat:advancedPermissions');
+
+			mailer.invite.mockResolvedValue({ emailSent: false });
+
+			await testServer
+				.authAgentFor(instanceOwner)
+				.post('/invitations')
+				.send([{ email: randomEmail(), role: 'global:admin' }]);
+
+			await testServer
+				.authAgentFor(instanceOwner)
+				.post('/invitations')
+				.send([{ email: randomEmail(), role: 'global:admin' }])
+				.expect(200);
+		});
+
+		test('should return 403 on creating admin shell when advanced permissions is unlicensed', async () => {
+			testServer.license.disable('feat:advancedPermissions');
+
+			mailer.invite.mockResolvedValue({ emailSent: false });
+
+			await testServer
+				.authAgentFor(instanceOwner)
+				.post('/invitations')
+				.send([{ email: randomEmail(), role: 'global:admin' }])
+				.expect(403);
+		});
+
+		test('should email invites and create user shells, without inviting existing users', async () => {
+			mailer.invite.mockResolvedValue({ emailSent: true });
+
+			const member = await createMember();
+			const memberShell = await createUserShell('global:member');
+			const newUserEmail = randomEmail();
+
+			const existingUserEmails = [member.email];
+			const inviteeUserEmails = [memberShell.email, newUserEmail];
+			const payload = inviteeUserEmails.concat(existingUserEmails).map((email) => ({ email }));
+
+			const response: InvitationResponse = await testServer
+				.authAgentFor(instanceOwner)
+				.post('/invitations')
+				.send(payload)
+				.expect(200);
+
+			// invite results
+
+			const { data: results } = response.body;
+
+			for (const result of results) {
+				assertUserInviteResult(result);
+
+				const storedUser = await Container.get(UserRepository).findOneByOrFail({
+					id: result.user.id,
+				});
+
+				assertStoredUserProps(storedUser);
+			}
+
+			// external hooks
+
+			expect(externalHooks.run).toHaveBeenCalledTimes(1);
+
+			const [externalHookName, externalHookArg] = externalHooks.run.mock.calls[0];
+
+			expect(externalHookName).toBe('user.invited');
+			expect(externalHookArg?.[0]).toStrictEqual([newUserEmail]);
+
+			// internal hooks
+
+			const calls = mocked(internalHooks).onUserTransactionalEmail.mock.calls;
+
+			for (const [onUserTransactionalEmailArg] of calls) {
+				expect(onUserTransactionalEmailArg.user_id).toBeDefined();
+				expect(onUserTransactionalEmailArg.message_type).toBe('New user invite');
+				expect(onUserTransactionalEmailArg.public_api).toBe(false);
+			}
+		});
+
+		test('should return 200 and surface error when invite method throws error', async () => {
+			const errorMsg = 'Failed to send email';
+
+			mailer.invite.mockImplementation(async () => {
+				throw new Error(errorMsg);
+			});
+
+			const response: InvitationResponse = await testServer
+				.authAgentFor(instanceOwner)
+				.post('/invitations')
+				.send([{ email: randomEmail() }])
+				.expect(200);
+
+			expect(response.body.data).toBeInstanceOf(Array);
+			expect(response.body.data.length).toBe(1);
+
+			const [result] = response.body.data;
+
+			expect(result.error).toBe(errorMsg);
+		});
+	});
+});

packages/cli/test/integration/credentials.ee.test.ts

@@ -17,6 +17,7 @@ import { createManyUsers, createUser, createUserShell } from './shared/db/users'
 import { UserManagementMailer } from '@/UserManagement/email';
 
 import { mockInstance } from '../shared/mocking';
+import config from '@/config';
 
 const sharingSpy = jest.spyOn(License.prototype, 'isSharingEnabled').mockReturnValue(true);
 const testServer = utils.setupTestServer({ endpointGroups: ['credentials'] });
@@ -489,7 +490,6 @@ describe('PUT /credentials/:id/share', () => {
 
 		expect(sharedCredentials).toHaveLength(1);
 		expect(sharedCredentials[0].userId).toBe(owner.id);
-		expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(0);
 	});
 
 	test('should ignore non-existing sharee', async () => {
@@ -507,7 +507,7 @@ describe('PUT /credentials/:id/share', () => {
 
 		expect(sharedCredentials).toHaveLength(1);
 		expect(sharedCredentials[0].userId).toBe(owner.id);
-		expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(0);
+		expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(1);
 	});
 
 	test('should respond 400 if invalid payload is provided', async () => {
@@ -542,7 +542,27 @@ describe('PUT /credentials/:id/share', () => {
 
 		expect(sharedCredentials).toHaveLength(1);
 		expect(sharedCredentials[0].userId).toBe(owner.id);
-		expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(0);
+		expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(1);
+	});
+
+	test('should not call internal hooks listener for email sent if emailing is disabled', async () => {
+		config.set('userManagement.emails.mode', '');
+
+		const savedCredential = await saveCredential(randomCredentialPayload(), { user: owner });
+
+		const [member1, member2] = await createManyUsers(2, {
+			role: 'global:member',
+		});
+
+		await shareCredentialWithUsers(savedCredential, [member1, member2]);
+
+		const response = await authOwnerAgent
+			.put(`/credentials/${savedCredential.id}/share`)
+			.send({ shareWithIds: [] });
+
+		expect(response.statusCode).toBe(200);
+
+		config.set('userManagement.emails.mode', 'smtp');
 	});
 });
 

packages/cli/test/integration/invitations.api.test.ts

@@ -1,443 +0,0 @@
-import validator from 'validator';
-import type { SuperAgentTest } from 'supertest';
-
-import type { User } from '@db/entities/User';
-import { PasswordUtility } from '@/services/password.utility';
-import { UserManagementMailer } from '@/UserManagement/email/UserManagementMailer';
-
-import Container from 'typedi';
-import { UserRepository } from '@db/repositories/user.repository';
-
-import { mockInstance } from '../shared/mocking';
-import {
-	randomEmail,
-	randomInvalidPassword,
-	randomName,
-	randomValidPassword,
-} from './shared/random';
-import * as testDb from './shared/testDb';
-import * as utils from './shared/utils/';
-import { createMember, createOwner, createUser, createUserShell } from './shared/db/users';
-import { ExternalHooks } from '@/ExternalHooks';
-import { InternalHooks } from '@/InternalHooks';
-import type { UserInvitationResponse } from './shared/utils/users';
-import {
-	assertInviteUserSuccessResponse,
-	assertInvitedUsersOnDb,
-	assertInviteUserErrorResponse,
-} from './shared/utils/users';
-import { mocked } from 'jest-mock';
-import { License } from '@/License';
-
-mockInstance(InternalHooks);
-
-const license = mockInstance(License, {
-	isAdvancedPermissionsLicensed: jest.fn().mockReturnValue(true),
-	isWithinUsersLimit: jest.fn().mockReturnValue(true),
-});
-
-const externalHooks = mockInstance(ExternalHooks);
-const mailer = mockInstance(UserManagementMailer, { isEmailSetUp: true });
-
-const testServer = utils.setupTestServer({ endpointGroups: ['invitations'] });
-
-describe('POST /invitations/:id/accept', () => {
-	let owner: User;
-
-	let authlessAgent: SuperAgentTest;
-
-	beforeAll(async () => {
-		await testDb.truncate(['User']);
-
-		owner = await createOwner();
-
-		authlessAgent = testServer.authlessAgent;
-	});
-
-	test('should fill out a member shell', async () => {
-		const memberShell = await createUserShell('global:member');
-
-		const memberData = {
-			inviterId: owner.id,
-			firstName: randomName(),
-			lastName: randomName(),
-			password: randomValidPassword(),
-		};
-
-		const response = await authlessAgent
-			.post(`/invitations/${memberShell.id}/accept`)
-			.send(memberData)
-			.expect(200);
-
-		const {
-			id,
-			email,
-			firstName,
-			lastName,
-			personalizationAnswers,
-			password,
-			role,
-			isPending,
-			apiKey,
-			globalScopes,
-		} = response.body.data;
-
-		expect(validator.isUUID(id)).toBe(true);
-		expect(email).toBeDefined();
-		expect(firstName).toBe(memberData.firstName);
-		expect(lastName).toBe(memberData.lastName);
-		expect(personalizationAnswers).toBeNull();
-		expect(password).toBeUndefined();
-		expect(isPending).toBe(false);
-		expect(role).toBe('global:member');
-		expect(apiKey).not.toBeDefined();
-		expect(globalScopes).toBeDefined();
-		expect(globalScopes).not.toHaveLength(0);
-
-		const authToken = utils.getAuthToken(response);
-		expect(authToken).toBeDefined();
-
-		const storedMember = await Container.get(UserRepository).findOneByOrFail({
-			id: memberShell.id,
-		});
-
-		expect(storedMember.firstName).toBe(memberData.firstName);
-		expect(storedMember.lastName).toBe(memberData.lastName);
-		expect(storedMember.password).not.toBe(memberData.password);
-	});
-
-	test('should fill out an admin shell', async () => {
-		const adminShell = await createUserShell('global:admin');
-
-		const memberData = {
-			inviterId: owner.id,
-			firstName: randomName(),
-			lastName: randomName(),
-			password: randomValidPassword(),
-		};
-
-		const response = await authlessAgent
-			.post(`/invitations/${adminShell.id}/accept`)
-			.send(memberData)
-			.expect(200);
-
-		const {
-			id,
-			email,
-			firstName,
-			lastName,
-			personalizationAnswers,
-			password,
-			role,
-			isPending,
-			apiKey,
-			globalScopes,
-		} = response.body.data;
-
-		expect(validator.isUUID(id)).toBe(true);
-		expect(email).toBeDefined();
-		expect(firstName).toBe(memberData.firstName);
-		expect(lastName).toBe(memberData.lastName);
-		expect(personalizationAnswers).toBeNull();
-		expect(password).toBeUndefined();
-		expect(isPending).toBe(false);
-		expect(role).toBe('global:admin');
-		expect(apiKey).not.toBeDefined();
-		expect(globalScopes).toBeDefined();
-		expect(globalScopes).not.toHaveLength(0);
-
-		const authToken = utils.getAuthToken(response);
-		expect(authToken).toBeDefined();
-
-		const storedAdmin = await Container.get(UserRepository).findOneByOrFail({
-			id: adminShell.id,
-		});
-
-		expect(storedAdmin.firstName).toBe(memberData.firstName);
-		expect(storedAdmin.lastName).toBe(memberData.lastName);
-		expect(storedAdmin.password).not.toBe(memberData.password);
-	});
-
-	test('should fail with invalid payloads', async () => {
-		const memberShellEmail = randomEmail();
-
-		const memberShell = await Container.get(UserRepository).save({
-			email: memberShellEmail,
-			role: 'global:member',
-		});
-
-		const invalidPayloads = [
-			{
-				firstName: randomName(),
-				lastName: randomName(),
-				password: randomValidPassword(),
-			},
-			{
-				inviterId: owner.id,
-				firstName: randomName(),
-				password: randomValidPassword(),
-			},
-			{
-				inviterId: owner.id,
-				firstName: randomName(),
-				password: randomValidPassword(),
-			},
-			{
-				inviterId: owner.id,
-				firstName: randomName(),
-				lastName: randomName(),
-			},
-			{
-				inviterId: owner.id,
-				firstName: randomName(),
-				lastName: randomName(),
-				password: randomInvalidPassword(),
-			},
-		];
-
-		for (const invalidPayload of invalidPayloads) {
-			await authlessAgent
-				.post(`/invitations/${memberShell.id}/accept`)
-				.send(invalidPayload)
-				.expect(400);
-
-			const storedMemberShell = await Container.get(UserRepository).findOneOrFail({
-				where: { email: memberShellEmail },
-			});
-
-			expect(storedMemberShell.firstName).toBeNull();
-			expect(storedMemberShell.lastName).toBeNull();
-			expect(storedMemberShell.password).toBeNull();
-		}
-	});
-
-	test('should fail with already accepted invite', async () => {
-		const member = await createUser({ role: 'global:member' });
-
-		const memberData = {
-			inviterId: owner.id,
-			firstName: randomName(),
-			lastName: randomName(),
-			password: randomValidPassword(),
-		};
-
-		await authlessAgent.post(`/invitations/${member.id}/accept`).send(memberData).expect(400);
-
-		const storedMember = await Container.get(UserRepository).findOneOrFail({
-			where: { email: member.email },
-		});
-
-		expect(storedMember.firstName).not.toBe(memberData.firstName);
-		expect(storedMember.lastName).not.toBe(memberData.lastName);
-		expect(storedMember.password).not.toBe(memberData.password);
-
-		const comparisonResult = await Container.get(PasswordUtility).compare(
-			member.password,
-			storedMember.password,
-		);
-
-		expect(comparisonResult).toBe(false);
-	});
-});
-
-describe('POST /invitations', () => {
-	let owner: User;
-	let member: User;
-	let ownerAgent: SuperAgentTest;
-
-	beforeAll(async () => {
-		await testDb.truncate(['User']);
-
-		owner = await createOwner();
-		member = await createMember();
-		ownerAgent = testServer.authAgentFor(owner);
-	});
-
-	test('should fail with invalid payloads', async () => {
-		const invalidPayloads = [
-			randomEmail(),
-			[randomEmail()],
-			{},
-			[{ name: randomName() }],
-			[{ email: randomName() }],
-		];
-
-		await Promise.all(
-			invalidPayloads.map(async (invalidPayload) => {
-				await ownerAgent.post('/invitations').send(invalidPayload).expect(400);
-
-				const usersCount = await Container.get(UserRepository).count();
-
-				expect(usersCount).toBe(2); // DB unaffected
-			}),
-		);
-	});
-
-	test('should return 200 on empty payload', async () => {
-		const response = await ownerAgent.post('/invitations').send([]).expect(200);
-
-		expect(response.body.data).toStrictEqual([]);
-
-		const usersCount = await Container.get(UserRepository).count();
-
-		expect(usersCount).toBe(2);
-	});
-
-	test('should return 200 if emailing is not set up', async () => {
-		mailer.invite.mockResolvedValue({ emailSent: false });
-
-		const response = await ownerAgent
-			.post('/invitations')
-			.send([{ email: randomEmail() }])
-			.expect(200);
-
-		expect(response.body.data).toBeInstanceOf(Array);
-		expect(response.body.data.length).toBe(1);
-
-		const { user } = response.body.data[0];
-
-		expect(user.inviteAcceptUrl).toBeDefined();
-
-		const inviteUrl = new URL(user.inviteAcceptUrl);
-
-		expect(inviteUrl.searchParams.get('inviterId')).toBe(owner.id);
-		expect(inviteUrl.searchParams.get('inviteeId')).toBe(user.id);
-	});
-
-	test('should create member shell', async () => {
-		mailer.invite.mockResolvedValue({ emailSent: false });
-
-		const response = await ownerAgent
-			.post('/invitations')
-			.send([{ email: randomEmail() }])
-			.expect(200);
-
-		const [result] = response.body.data as UserInvitationResponse[];
-
-		const storedUser = await Container.get(UserRepository).findOneByOrFail({
-			id: result.user.id,
-		});
-
-		assertInvitedUsersOnDb(storedUser);
-	});
-
-	test('should create admin shell if licensed', async () => {
-		mailer.invite.mockResolvedValue({ emailSent: false });
-
-		const response = await ownerAgent
-			.post('/invitations')
-			.send([{ email: randomEmail(), role: 'global:admin' }])
-			.expect(200);
-
-		const [result] = response.body.data as UserInvitationResponse[];
-
-		const storedUser = await Container.get(UserRepository).findOneByOrFail({
-			id: result.user.id,
-		});
-
-		assertInvitedUsersOnDb(storedUser);
-	});
-
-	test('should reinvite member', async () => {
-		mailer.invite.mockResolvedValue({ emailSent: false });
-
-		await ownerAgent.post('/invitations').send([{ email: randomEmail(), role: 'global:member' }]);
-
-		await ownerAgent
-			.post('/invitations')
-			.send([{ email: randomEmail(), role: 'global:member' }])
-			.expect(200);
-	});
-
-	test('should reinvite admin if licensed', async () => {
-		license.isAdvancedPermissionsLicensed.mockReturnValue(true);
-		mailer.invite.mockResolvedValue({ emailSent: false });
-
-		await ownerAgent.post('/invitations').send([{ email: randomEmail(), role: 'global:admin' }]);
-
-		await ownerAgent
-			.post('/invitations')
-			.send([{ email: randomEmail(), role: 'global:admin' }])
-			.expect(200);
-	});
-
-	test('should fail to create admin shell if not licensed', async () => {
-		license.isAdvancedPermissionsLicensed.mockReturnValue(false);
-		mailer.invite.mockResolvedValue({ emailSent: false });
-
-		await ownerAgent
-			.post('/invitations')
-			.send([{ email: randomEmail(), role: 'global:admin' }])
-			.expect(403);
-	});
-
-	test('should email invites and create user shells but ignore existing', async () => {
-		externalHooks.run.mockClear();
-
-		mailer.invite.mockResolvedValue({ emailSent: true });
-
-		const memberShell = await createUserShell('global:member');
-
-		const newUser = randomEmail();
-
-		const shellUsers = [memberShell.email];
-		const usersToInvite = [newUser, ...shellUsers];
-		const usersToCreate = [newUser];
-		const existingUsers = [member.email];
-
-		const testEmails = [...usersToInvite, ...existingUsers];
-
-		const payload = testEmails.map((email) => ({ email }));
-
-		const response = await ownerAgent.post('/invitations').send(payload).expect(200);
-
-		const internalHooks = Container.get(InternalHooks);
-
-		expect(internalHooks.onUserTransactionalEmail).toHaveBeenCalledTimes(usersToInvite.length);
-
-		expect(externalHooks.run).toHaveBeenCalledTimes(1);
-
-		const [hookName, hookData] = externalHooks.run.mock.calls[0];
-
-		expect(hookName).toBe('user.invited');
-		expect(hookData?.[0]).toStrictEqual(usersToCreate);
-
-		const result = response.body.data as UserInvitationResponse[];
-
-		for (const invitationResponse of result) {
-			assertInviteUserSuccessResponse(invitationResponse);
-
-			const storedUser = await Container.get(UserRepository).findOneByOrFail({
-				id: invitationResponse.user.id,
-			});
-
-			assertInvitedUsersOnDb(storedUser);
-		}
-
-		const calls = mocked(internalHooks).onUserTransactionalEmail.mock.calls;
-
-		for (const [onUserTransactionalEmailParameter] of calls) {
-			expect(onUserTransactionalEmailParameter.user_id).toBeDefined();
-			expect(onUserTransactionalEmailParameter.message_type).toBe('New user invite');
-			expect(onUserTransactionalEmailParameter.public_api).toBe(false);
-		}
-	});
-
-	test('should return 200 when invite method throws error', async () => {
-		mailer.invite.mockImplementation(async () => {
-			throw new Error('failed to send email');
-		});
-
-		const response = await ownerAgent
-			.post('/invitations')
-			.send([{ email: randomEmail() }])
-			.expect(200);
-
-		expect(response.body.data).toBeInstanceOf(Array);
-		expect(response.body.data.length).toBe(1);
-
-		const [invitationResponse] = response.body.data;
-
-		assertInviteUserErrorResponse(invitationResponse);
-	});
-});

packages/cli/test/integration/shared/utils/users.ts

@@ -1,4 +1,3 @@
-import validator from 'validator';
 import type { PublicUser } from '@/Interfaces';
 import type { User } from '@/databases/entities/User';
 
@@ -16,30 +15,7 @@ export const validateUser = (user: PublicUser) => {
 	expect(user.role).toBeDefined();
 };
 
-export const assertInviteUserSuccessResponse = (data: UserInvitationResponse) => {
+export type UserInvitationResult = {
-	expect(validator.isUUID(data.user.id)).toBe(true);
-	expect(data.user.inviteAcceptUrl).toBeUndefined();
-	expect(data.user.email).toBeDefined();
-	expect(data.user.emailSent).toBe(true);
-};
-
-export const assertInviteUserErrorResponse = (data: UserInvitationResponse) => {
-	expect(validator.isUUID(data.user.id)).toBe(true);
-	expect(data.user.inviteAcceptUrl).toBeDefined();
-	expect(data.user.email).toBeDefined();
-	expect(data.user.emailSent).toBe(false);
-	expect(data.error).toBeDefined();
-};
-
-export const assertInvitedUsersOnDb = (user: User) => {
-	expect(user.firstName).toBeNull();
-	expect(user.lastName).toBeNull();
-	expect(user.personalizationAnswers).toBeNull();
-	expect(user.password).toBeNull();
-	expect(user.isPending).toBe(true);
-};
-
-export type UserInvitationResponse = {
 	user: Pick<User, 'id' | 'email'> & { inviteAcceptUrl: string; emailSent: boolean };
 	error?: string;
 };

packages/cli/test/integration/workflows/workflows.controller.ee.test.ts

@@ -19,6 +19,7 @@ import { createUser } from '../shared/db/users';
 import { createWorkflow, getWorkflowSharing, shareWorkflowWithUsers } from '../shared/db/workflows';
 import { License } from '@/License';
 import { UserManagementMailer } from '@/UserManagement/email';
+import config from '@/config';
 
 let owner: User;
 let member: User;
@@ -149,7 +150,7 @@ describe('PUT /workflows/:id', () => {
 
 		const secondSharedWorkflows = await getWorkflowSharing(workflow);
 		expect(secondSharedWorkflows).toHaveLength(2);
-		expect(mailer.notifyWorkflowShared).toHaveBeenCalledTimes(1);
+		expect(mailer.notifyWorkflowShared).toHaveBeenCalledTimes(2);
 	});
 
 	test('PUT /workflows/:id/share should allow sharing by the owner of the workflow', async () => {
@@ -225,6 +226,20 @@ describe('PUT /workflows/:id', () => {
 		expect(sharedWorkflows).toHaveLength(1);
 		expect(mailer.notifyWorkflowShared).toHaveBeenCalledTimes(0);
 	});
+
+	test('should not call internal hooks listener for email sent if emailing is disabled', async () => {
+		config.set('userManagement.emails.mode', '');
+
+		const workflow = await createWorkflow({}, owner);
+
+		const response = await authOwnerAgent
+			.put(`/workflows/${workflow.id}/share`)
+			.send({ shareWithIds: [member.id] });
+
+		expect(response.statusCode).toBe(200);
+
+		config.set('userManagement.emails.mode', 'smtp');
+	});
 });
 
 describe('GET /workflows/new', () => {

packages/cli/test/unit/services/user.service.test.ts

@@ -7,11 +7,13 @@ import { UserRepository } from '@db/repositories/user.repository';
 import { UserService } from '@/services/user.service';
 import { mockInstance } from '../../shared/mocking';
 import { v4 as uuid } from 'uuid';
+import { InternalHooks } from '@/InternalHooks';
 
 describe('UserService', () => {
 	config.set('userManagement.jwtSecret', 'random-secret');
 
 	mockInstance(Logger);
+	mockInstance(InternalHooks);
 
 	const userRepository = mockInstance(UserRepository);
 	const userService = Container.get(UserService);