From 9e939809575592622f6bdca112da1905ac9205ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 29 Jan 2024 16:15:30 +0100 Subject: [PATCH] fix(core): Prevent calling internal hook email event if emailing is disabled (#8462) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ --- .../email/UserManagementMailer.ts | 137 ++++-- .../credentials/credentials.controller.ee.ts | 38 +- .../cli/src/workflows/workflows.controller.ts | 35 +- .../controllers/invitation/assertions.ts | 29 ++ .../invitation.controller.integration.test.ts | 412 ++++++++++++++++ .../test/integration/credentials.ee.test.ts | 26 +- .../test/integration/invitations.api.test.ts | 443 ------------------ .../test/integration/shared/utils/users.ts | 26 +- .../workflows/workflows.controller.ee.test.ts | 17 +- .../test/unit/services/user.service.test.ts | 2 + 10 files changed, 597 insertions(+), 568 deletions(-) create mode 100644 packages/cli/test/integration/controllers/invitation/assertions.ts create mode 100644 packages/cli/test/integration/controllers/invitation/invitation.controller.integration.test.ts delete mode 100644 packages/cli/test/integration/invitations.api.test.ts diff --git a/packages/cli/src/UserManagement/email/UserManagementMailer.ts b/packages/cli/src/UserManagement/email/UserManagementMailer.ts index 95ad9614e0..b350da8e4c 100644 --- a/packages/cli/src/UserManagement/email/UserManagementMailer.ts +++ b/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; 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, - workflowName, - baseUrl, - workflowId, - sharerFirstName, + sharer, + newShareeIds, + workflow, }: { - recipientEmails: string[]; - workflowName: string; - baseUrl: string; - workflowId: string; - sharerFirstName: string; + sharer: User; + newShareeIds: string[]; + workflow: WorkflowEntity; }) { + 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({ - emailRecipients: recipientEmails, - subject: `${sharerFirstName} has shared an n8n workflow with you`, - body: populateTemplate({ workflowName, workflowUrl: `${baseUrl}/workflow/${workflowId}` }), - }); + const baseUrl = this.urlService.getInstanceBaseUrl(); - return result ?? { emailSent: false }; + try { + 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}`, + }), + }); + + 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({ - emailRecipients: recipientEmails, - subject: `${sharerFirstName} has shared an n8n credential with you`, - body: populateTemplate({ credentialsName, credentialsListUrl: `${baseUrl}/credentials` }), - }); + const baseUrl = this.urlService.getInstanceBaseUrl(); - return result ?? { emailSent: false }; + try { + const result = await this.mailer.sendMail({ + emailRecipients, + subject: `${sharer.firstName} has shared an n8n credential with you`, + body: populateTemplate({ credentialsName, credentialsListUrl: `${baseUrl}/credentials` }), + }); + + 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}`); + } } } diff --git a/packages/cli/src/credentials/credentials.controller.ee.ts b/packages/cli/src/credentials/credentials.controller.ee.ts index 4a8211caf2..09dc2a4e75 100644 --- a/packages/cli/src/credentials/credentials.controller.ee.ts +++ b/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, - 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, + await Container.get(UserManagementMailer).notifyCredentialsShared({ + sharer: req.user, + newShareeIds, + credentialsName: credential.name, }); }), ); diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 912457760a..799263ad5e 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/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), - 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, + await this.mailer.notifyWorkflowShared({ + sharer: req.user, + newShareeIds, + workflow, }); } } diff --git a/packages/cli/test/integration/controllers/invitation/assertions.ts b/packages/cli/test/integration/controllers/invitation/assertions.ts new file mode 100644 index 0000000000..349eac4e8e --- /dev/null +++ b/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); +}; 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 new file mode 100644 index 0000000000..971360bc05 --- /dev/null +++ b/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); + }); + }); +}); diff --git a/packages/cli/test/integration/credentials.ee.test.ts b/packages/cli/test/integration/credentials.ee.test.ts index 056d0f5329..be9fc74be4 100644 --- a/packages/cli/test/integration/credentials.ee.test.ts +++ b/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'); }); }); diff --git a/packages/cli/test/integration/invitations.api.test.ts b/packages/cli/test/integration/invitations.api.test.ts deleted file mode 100644 index 4ca48712ea..0000000000 --- a/packages/cli/test/integration/invitations.api.test.ts +++ /dev/null @@ -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); - }); -}); diff --git a/packages/cli/test/integration/shared/utils/users.ts b/packages/cli/test/integration/shared/utils/users.ts index c655f754b2..e812558674 100644 --- a/packages/cli/test/integration/shared/utils/users.ts +++ b/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) => { - 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 = { +export type UserInvitationResult = { user: Pick & { inviteAcceptUrl: string; emailSent: boolean }; error?: string; }; 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 e24973e013..ed3e13d9f0 100644 --- a/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts +++ b/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', () => { diff --git a/packages/cli/test/unit/services/user.service.test.ts b/packages/cli/test/unit/services/user.service.test.ts index dca81305af..6ab4f79c42 100644 --- a/packages/cli/test/unit/services/user.service.test.ts +++ b/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);