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

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

@@ -7,6 +7,9 @@ import config from '@/config';
 import type { InviteEmailData, PasswordResetData, SendEmailResult } from './Interfaces';
 import { NodeMailer } from './NodeMailer';
 import { ApplicationError } from 'n8n-workflow';
+import { UserRepository } from '@/databases/repositories/user.repository';
+import { InternalHooks } from '@/InternalHooks';
+import { Logger } from '@/Logger';
 
 type Template = HandlebarsTemplateDelegate<unknown>;
 type TemplateName = 'invite' | 'passwordReset' | 'workflowShared' | 'credentialsShared';
@@ -39,7 +42,11 @@ export class UserManagementMailer {
 
 	private mailer: NodeMailer | undefined;
 
-	constructor() {
+	constructor(
+		private readonly userRepository: UserRepository,
+		private readonly logger: Logger,
+		private readonly internalHooks: InternalHooks,
+	) {
 		this.isEmailSetUp =
 			config.getEnv('userManagement.emails.mode') === 'smtp' &&
 			config.getEnv('userManagement.emails.smtp.host') !== '';
@@ -83,48 +90,85 @@ export class UserManagementMailer {
 	}
 
 	async notifyWorkflowShared({
-		recipientEmails,
-		workflowName,
+		sharer,
+		newShareeIds,
+		workflow,
 		baseUrl,
-		workflowId,
-		sharerFirstName,
 	}: {
-		recipientEmails: string[];
-		workflowName: string;
+		sharer: { id: string; firstName: string };
+		newShareeIds: string[];
+		workflow: { id: string; name: string };
 		baseUrl: string;
-		workflowId: string;
-		sharerFirstName: string;
 	}) {
+		if (this.isEmailSetUp) 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({
-			emailRecipients: recipientEmails,
-			subject: `${sharerFirstName} has shared an n8n workflow with you`,
-			body: populateTemplate({ workflowName, workflowUrl: `${baseUrl}/workflow/${workflowId}` }),
+			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 this.internalHooks.onUserTransactionalEmail({
+			user_id: sharer.id,
+			message_type: 'Workflow shared',
+			public_api: false,
+		});
+
+		return result;
 	}
 
 	async notifyCredentialsShared({
-		sharerFirstName,
+		sharer,
+		newShareeIds,
 		credentialsName,
-		recipientEmails,
 		baseUrl,
 	}: {
-		sharerFirstName: string;
+		sharer: { id: string; firstName: string };
+		newShareeIds: string[];
 		credentialsName: string;
-		recipientEmails: string[];
 		baseUrl: string;
 	}) {
+		if (this.isEmailSetUp) 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({
-			emailRecipients: recipientEmails,
-			subject: `${sharerFirstName} has shared an n8n credential with you`,
+			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 this.internalHooks.onUserTransactionalEmail({
+			user_id: sharer.id,
+			message_type: 'Credentials shared',
+			public_api: false,
+		});
+
+		return result;
 	}
 }

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

@@ -18,8 +18,8 @@ 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';
+import config from '@/config';
 
 export const EECredentialsController = express.Router();
 
@@ -190,15 +190,19 @@ EECredentialsController.put(
 			sharees_removed: amountRemoved,
 		});
 
+		const isEmailingEnabled = config.getEnv('userManagement.emails.mode') === 'smtp';
+
+		if (!isEmailingEnabled) return;
+
 		const recipients = await Container.get(UserRepository).getEmailsByIds(newShareeIds);
 
 		if (recipients.length === 0) return;
 
 		try {
 			await Container.get(UserManagementMailer).notifyCredentialsShared({
-				sharerFirstName: req.user.firstName,
+				sharer: { id: req.user.id, firstName: req.user.firstName },
+				newShareeIds,
 				credentialsName: credential.name,
-				recipientEmails: recipients.map(({ email }) => email),
 				baseUrl: Container.get(UrlService).getInstanceBaseUrl(),
 			});
 		} catch (error) {
@@ -207,19 +211,10 @@ EECredentialsController.put(
 				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

@@ -403,16 +403,11 @@ 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),
-				workflowName: workflow.name,
-				workflowId,
-				sharerFirstName: req.user.firstName,
+				sharer: { id: req.user.id, firstName: req.user.firstName },
+				newShareeIds,
+				workflow: { id: workflowId, name: workflow.name },
 				baseUrl: this.urlService.getInstanceBaseUrl(),
 			});
 		} catch (error) {
@@ -425,13 +420,5 @@ export class WorkflowsController {
 				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/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 () => {
@@ -544,6 +544,26 @@ describe('PUT /credentials/:id/share', () => {
 		expect(sharedCredentials[0].userId).toBe(owner.id);
 		expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(0);
 	});
+
+	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');
+	});
 });
 
 function validateMainCredentialData(credential: ListQuery.Credentials.WithOwnedByAndSharedWith) {

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', () => {