From 9500aa6c8c3db006c446525360bd66f8edf0d072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 22 Jan 2024 11:00:47 +0100 Subject: [PATCH] feat(core): Email recipients on resource shared --- packages/cli/src/InternalHooks.ts | 14 +++++- .../email/UserManagementMailer.ts | 48 ++++++++++++++++++- .../email/templates/credentialsShared.html | 4 ++ .../email/templates/workflowShared.html | 4 ++ packages/cli/src/config/schema.ts | 12 +++++ .../credentials/credentials.controller.ee.ts | 37 ++++++++++++++ .../databases/repositories/user.repository.ts | 10 ++++ .../cli/src/workflows/workflows.controller.ts | 35 ++++++++++++++ .../test/integration/credentials.ee.test.ts | 19 ++++++++ .../workflows/workflows.controller.ee.test.ts | 14 ++++++ 10 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 packages/cli/src/UserManagement/email/templates/credentialsShared.html create mode 100644 packages/cli/src/UserManagement/email/templates/workflowShared.html diff --git a/packages/cli/src/InternalHooks.ts b/packages/cli/src/InternalHooks.ts index a7f69d19ab6b4..0e88c2c8f6c62 100644 --- a/packages/cli/src/InternalHooks.ts +++ b/packages/cli/src/InternalHooks.ts @@ -648,7 +648,12 @@ export class InternalHooks { async onUserTransactionalEmail(userTransactionalEmailData: { user_id: string; - message_type: 'Reset password' | 'New user invite' | 'Resend invite'; + message_type: + | 'Reset password' + | 'New user invite' + | 'Resend invite' + | 'Workflow shared' + | 'Credentials shared'; public_api: boolean; }): Promise { return await this.telemetry.track( @@ -737,7 +742,12 @@ export class InternalHooks { async onEmailFailed(failedEmailData: { user: User; - message_type: 'Reset password' | 'New user invite' | 'Resend invite'; + message_type: + | 'Reset password' + | 'New user invite' + | 'Resend invite' + | 'Workflow shared' + | 'Credentials shared'; public_api: boolean; }): Promise { void Promise.all([ diff --git a/packages/cli/src/UserManagement/email/UserManagementMailer.ts b/packages/cli/src/UserManagement/email/UserManagementMailer.ts index 6963f809833cd..95ad9614e0daa 100644 --- a/packages/cli/src/UserManagement/email/UserManagementMailer.ts +++ b/packages/cli/src/UserManagement/email/UserManagementMailer.ts @@ -9,7 +9,7 @@ import { NodeMailer } from './NodeMailer'; import { ApplicationError } from 'n8n-workflow'; type Template = HandlebarsTemplateDelegate; -type TemplateName = 'invite' | 'passwordReset'; +type TemplateName = 'invite' | 'passwordReset' | 'workflowShared' | 'credentialsShared'; const templates: Partial> = {}; @@ -81,4 +81,50 @@ export class UserManagementMailer { // No error, just say no email was sent. return result ?? { emailSent: false }; } + + async notifyWorkflowShared({ + recipientEmails, + workflowName, + baseUrl, + workflowId, + sharerFirstName, + }: { + recipientEmails: string[]; + workflowName: string; + baseUrl: string; + workflowId: string; + sharerFirstName: string; + }) { + 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}` }), + }); + + return result ?? { emailSent: false }; + } + + async notifyCredentialsShared({ + sharerFirstName, + credentialsName, + recipientEmails, + baseUrl, + }: { + sharerFirstName: string; + credentialsName: string; + recipientEmails: string[]; + baseUrl: string; + }) { + const populateTemplate = await getTemplate('credentialsShared', 'credentialsShared.html'); + + const result = await this.mailer?.sendMail({ + emailRecipients: recipientEmails, + subject: `${sharerFirstName} has shared an n8n credential with you`, + body: populateTemplate({ credentialsName, credentialsListUrl: `${baseUrl}/credentials` }), + }); + + return result ?? { emailSent: false }; + } } diff --git a/packages/cli/src/UserManagement/email/templates/credentialsShared.html b/packages/cli/src/UserManagement/email/templates/credentialsShared.html new file mode 100644 index 0000000000000..b4634a9ddfdf2 --- /dev/null +++ b/packages/cli/src/UserManagement/email/templates/credentialsShared.html @@ -0,0 +1,4 @@ +

Hi there,

+

"{{ credentialsName }}" credential has been shared with you.

+

To view all the credentials you have access to within n8n, click the following link:

+

{{ credentialsListUrl }}

diff --git a/packages/cli/src/UserManagement/email/templates/workflowShared.html b/packages/cli/src/UserManagement/email/templates/workflowShared.html new file mode 100644 index 0000000000000..d6fa692759ff7 --- /dev/null +++ b/packages/cli/src/UserManagement/email/templates/workflowShared.html @@ -0,0 +1,4 @@ +

Hi there,

+

"{{ workflowName }}" workflow has been shared with you.

+

To access the workflow, click the following link:

+

{{ workflowUrl }}

diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index fca700fc7890d..067c24fb9719a 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -846,6 +846,18 @@ export const schema = { default: '', env: 'N8N_UM_EMAIL_TEMPLATES_PWRESET', }, + workflowShared: { + doc: 'Overrides default HTML template for notifying that a workflow was shared (use full path)', + format: String, + default: '', + env: 'N8N_UM_EMAIL_TEMPLATES_WORKFLOW_SHARED', + }, + credentialsShared: { + doc: 'Overrides default HTML template for notifying that credentials were shared (use full path)', + format: String, + default: '', + env: 'N8N_UM_EMAIL_TEMPLATES_CREDENTIALS_SHARED', + }, }, }, authenticationMethod: { diff --git a/packages/cli/src/credentials/credentials.controller.ee.ts b/packages/cli/src/credentials/credentials.controller.ee.ts index 25045c9aa505c..fadf0df2279e8 100644 --- a/packages/cli/src/credentials/credentials.controller.ee.ts +++ b/packages/cli/src/credentials/credentials.controller.ee.ts @@ -15,6 +15,11 @@ 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(); @@ -185,5 +190,37 @@ EECredentialsController.put( user_ids_sharees_added: newShareeIds, 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, + 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, + }); }), ); diff --git a/packages/cli/src/databases/repositories/user.repository.ts b/packages/cli/src/databases/repositories/user.repository.ts index c21684d51323f..07553511255be 100644 --- a/packages/cli/src/databases/repositories/user.repository.ts +++ b/packages/cli/src/databases/repositories/user.repository.ts @@ -84,4 +84,14 @@ export class UserRepository extends Repository { return findManyOptions; } + + /** + * Get emails of users who have completed setup, by user IDs. + */ + async getEmailsByIds(userIds: string[]) { + return await this.find({ + select: ['email'], + where: { id: In(userIds), password: Not(IsNull()) }, + }); + } } diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 425d384dd3e46..71a2a054c4e75 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -40,6 +40,8 @@ import { WorkflowRequest } from './workflow.request'; 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() @@ -62,6 +64,8 @@ export class WorkflowsController { private readonly workflowSharingService: WorkflowSharingService, private readonly sharedWorkflowRepository: SharedWorkflowRepository, private readonly userRepository: UserRepository, + private readonly mailer: UserManagementMailer, + private readonly urlService: UrlService, ) {} @Post('/') @@ -401,5 +405,36 @@ 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, + 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, + }); } } diff --git a/packages/cli/test/integration/credentials.ee.test.ts b/packages/cli/test/integration/credentials.ee.test.ts index f0792244aaff0..e6a7413925365 100644 --- a/packages/cli/test/integration/credentials.ee.test.ts +++ b/packages/cli/test/integration/credentials.ee.test.ts @@ -16,6 +16,8 @@ import { getCredentialOwnerRole, getGlobalMemberRole, getGlobalOwnerRole } from import { createManyUsers, createUser, createUserShell } from './shared/db/users'; import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository'; import Container from 'typedi'; +import { mockInstance } from '../shared/mocking'; +import { UserManagementMailer } from '@/UserManagement/email'; const sharingSpy = jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(true); const testServer = utils.setupTestServer({ endpointGroups: ['credentials'] }); @@ -27,6 +29,7 @@ let anotherMember: User; let authOwnerAgent: SuperAgentTest; let authAnotherMemberAgent: SuperAgentTest; let saveCredential: SaveCredentialFunction; +const mailer = mockInstance(UserManagementMailer); beforeAll(async () => { const globalOwnerRole = await getGlobalOwnerRole(); @@ -47,6 +50,10 @@ beforeEach(async () => { await testDb.truncate(['SharedCredentials', 'Credentials']); }); +afterEach(() => { + jest.clearAllMocks(); +}); + // ---------------------------------------- // dynamic router switching // ---------------------------------------- @@ -363,6 +370,8 @@ describe('PUT /credentials/:id/share', () => { expect(sharedCredential.role.name).toBe('user'); expect(sharedCredential.role.scope).toBe('credential'); }); + + expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(1); }); test('should share the credential with the provided userIds', async () => { @@ -400,6 +409,7 @@ describe('PUT /credentials/:id/share', () => { expect(ownerSharedCredential.role.name).toBe('owner'); expect(ownerSharedCredential.role.scope).toBe('credential'); + expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(1); }); test('should respond 403 for non-existing credentials', async () => { @@ -408,6 +418,7 @@ describe('PUT /credentials/:id/share', () => { .send({ shareWithIds: [member.id] }); expect(response.statusCode).toBe(403); + expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(0); }); test('should respond 403 for non-owned credentials for shared members', async () => { @@ -424,6 +435,7 @@ describe('PUT /credentials/:id/share', () => { where: { credentialsId: savedCredential.id }, }); expect(sharedCredentials).toHaveLength(2); + expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(0); }); test('should respond 403 for non-owned credentials for non-shared members sharing with self', async () => { @@ -439,6 +451,7 @@ describe('PUT /credentials/:id/share', () => { where: { credentialsId: savedCredential.id }, }); expect(sharedCredentials).toHaveLength(1); + expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(0); }); test('should respond 403 for non-owned credentials for non-shared members sharing', async () => { @@ -455,6 +468,7 @@ describe('PUT /credentials/:id/share', () => { where: { credentialsId: savedCredential.id }, }); expect(sharedCredentials).toHaveLength(1); + expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(0); }); test('should respond 200 for non-owned credentials for owners', async () => { @@ -469,6 +483,7 @@ describe('PUT /credentials/:id/share', () => { where: { credentialsId: savedCredential.id }, }); expect(sharedCredentials).toHaveLength(2); + expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(1); }); test('should ignore pending sharee', async () => { @@ -487,6 +502,7 @@ 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 () => { @@ -504,6 +520,7 @@ describe('PUT /credentials/:id/share', () => { expect(sharedCredentials).toHaveLength(1); expect(sharedCredentials[0].userId).toBe(owner.id); + expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(0); }); test('should respond 400 if invalid payload is provided', async () => { @@ -515,6 +532,7 @@ describe('PUT /credentials/:id/share', () => { ]); responses.forEach((response) => expect(response.statusCode).toBe(400)); + expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(0); }); test('should unshare the credential', async () => { const savedCredential = await saveCredential(randomCredentialPayload(), { user: owner }); @@ -537,6 +555,7 @@ describe('PUT /credentials/:id/share', () => { expect(sharedCredentials).toHaveLength(1); expect(sharedCredentials[0].userId).toBe(owner.id); + expect(mailer.notifyCredentialsShared).toHaveBeenCalledTimes(0); }); }); diff --git a/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts b/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts index e49267c0f642f..e33bbadde7a1f 100644 --- a/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts +++ b/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts @@ -25,6 +25,7 @@ import { } from '../shared/db/roles'; import { createUser } from '../shared/db/users'; import { createWorkflow, getWorkflowSharing, shareWorkflowWithUsers } from '../shared/db/workflows'; +import { UserManagementMailer } from '@/UserManagement/email'; let globalMemberRole: Role; let owner: User; @@ -44,6 +45,7 @@ const testServer = utils.setupTestServer({ enabledFeatures: ['feat:sharing'], }); const license = testServer.license; +const mailer = mockInstance(UserManagementMailer); beforeAll(async () => { const globalOwnerRole = await getGlobalOwnerRole(); @@ -70,6 +72,10 @@ beforeEach(async () => { await testDb.truncate(['Workflow', 'SharedWorkflow', 'WorkflowHistory']); }); +afterEach(() => { + jest.clearAllMocks(); +}); + describe('router should switch based on flag', () => { let savedWorkflowId: string; @@ -107,6 +113,7 @@ describe('PUT /workflows/:id', () => { const sharedWorkflows = await getWorkflowSharing(workflow); expect(sharedWorkflows).toHaveLength(2); + expect(mailer.notifyWorkflowShared).toHaveBeenCalledTimes(1); }); test('PUT /workflows/:id/share should succeed when sharing with invalid user-id', async () => { @@ -133,6 +140,7 @@ describe('PUT /workflows/:id', () => { const sharedWorkflows = await getWorkflowSharing(workflow); expect(sharedWorkflows).toHaveLength(3); + expect(mailer.notifyWorkflowShared).toHaveBeenCalledTimes(1); }); test('PUT /workflows/:id/share should override sharing', async () => { @@ -154,6 +162,7 @@ describe('PUT /workflows/:id', () => { const secondSharedWorkflows = await getWorkflowSharing(workflow); expect(secondSharedWorkflows).toHaveLength(2); + expect(mailer.notifyWorkflowShared).toHaveBeenCalledTimes(1); }); test('PUT /workflows/:id/share should allow sharing by the owner of the workflow', async () => { @@ -167,6 +176,7 @@ describe('PUT /workflows/:id', () => { const sharedWorkflows = await getWorkflowSharing(workflow); expect(sharedWorkflows).toHaveLength(2); + expect(mailer.notifyWorkflowShared).toHaveBeenCalledTimes(1); }); test('PUT /workflows/:id/share should allow sharing by the instance owner', async () => { @@ -180,6 +190,7 @@ describe('PUT /workflows/:id', () => { const sharedWorkflows = await getWorkflowSharing(workflow); expect(sharedWorkflows).toHaveLength(2); + expect(mailer.notifyWorkflowShared).toHaveBeenCalledTimes(1); }); test('PUT /workflows/:id/share should not allow sharing by another shared member', async () => { @@ -195,6 +206,7 @@ describe('PUT /workflows/:id', () => { const sharedWorkflows = await getWorkflowSharing(workflow); expect(sharedWorkflows).toHaveLength(2); + expect(mailer.notifyWorkflowShared).toHaveBeenCalledTimes(0); }); test('PUT /workflows/:id/share should not allow sharing with self by another non-shared member', async () => { @@ -208,6 +220,7 @@ describe('PUT /workflows/:id', () => { const sharedWorkflows = await getWorkflowSharing(workflow); expect(sharedWorkflows).toHaveLength(1); + expect(mailer.notifyWorkflowShared).toHaveBeenCalledTimes(0); }); test('PUT /workflows/:id/share should not allow sharing by another non-shared member', async () => { @@ -223,6 +236,7 @@ describe('PUT /workflows/:id', () => { const sharedWorkflows = await getWorkflowSharing(workflow); expect(sharedWorkflows).toHaveLength(1); + expect(mailer.notifyWorkflowShared).toHaveBeenCalledTimes(0); }); });