diff --git a/packages/cli/src/InternalHooks.ts b/packages/cli/src/InternalHooks.ts index 9eeed21458..6530f562fd 100644 --- a/packages/cli/src/InternalHooks.ts +++ b/packages/cli/src/InternalHooks.ts @@ -436,6 +436,7 @@ export class InternalHooksClass implements IInternalHooksClass { user: User; target_user_id: string[]; public_api: boolean; + email_sent: boolean; }): Promise { void Promise.all([ eventBus.sendAuditEvent({ @@ -449,6 +450,7 @@ export class InternalHooksClass implements IInternalHooksClass { user_id: userInviteData.user.id, target_user_id: userInviteData.target_user_id, public_api: userInviteData.public_api, + email_sent: userInviteData.email_sent, }), ]); } diff --git a/packages/cli/src/UserManagement/Interfaces.ts b/packages/cli/src/UserManagement/Interfaces.ts index a58991bbb0..9973cd8b5e 100644 --- a/packages/cli/src/UserManagement/Interfaces.ts +++ b/packages/cli/src/UserManagement/Interfaces.ts @@ -23,6 +23,7 @@ export interface PublicUser { passwordResetToken?: string; createdAt: Date; isPending: boolean; + inviteAcceptUrl?: string; } export interface N8nApp { diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index 45267b0d1b..4116282dcb 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -99,6 +99,10 @@ export function getInstanceBaseUrl(): string { return n8nBaseUrl.endsWith('/') ? n8nBaseUrl.slice(0, n8nBaseUrl.length - 1) : n8nBaseUrl; } +export function generateUserInviteUrl(inviterId: string, inviteeId: string): string { + return `${getInstanceBaseUrl()}/signup?inviterId=${inviterId}&inviteeId=${inviteeId}`; +} + // TODO: Enforce at model level export function validatePassword(password?: string): string { if (!password) { @@ -156,6 +160,13 @@ export function sanitizeUser(user: User, withoutKeys?: string[]): PublicUser { return sanitizedUser; } +export function addInviteLinktoUser(user: PublicUser, inviterId: string): PublicUser { + if (user.isPending) { + user.inviteAcceptUrl = generateUserInviteUrl(inviterId, user.id); + } + return user; +} + export async function getUserById(userId: string): Promise { const user = await Db.collections.User.findOneOrFail(userId, { relations: ['globalRole'], diff --git a/packages/cli/src/UserManagement/email/Interfaces.ts b/packages/cli/src/UserManagement/email/Interfaces.ts index 97da3d49ee..0bbeba781b 100644 --- a/packages/cli/src/UserManagement/email/Interfaces.ts +++ b/packages/cli/src/UserManagement/email/Interfaces.ts @@ -1,4 +1,5 @@ export interface UserManagementMailerImplementation { + init: () => Promise; sendMail: (mailData: MailData) => Promise; verifyConnection: () => Promise; } @@ -20,8 +21,7 @@ export type PasswordResetData = { }; export type SendEmailResult = { - success: boolean; - error?: Error; + emailSent: boolean; }; export type MailData = { diff --git a/packages/cli/src/UserManagement/email/NodeMailer.ts b/packages/cli/src/UserManagement/email/NodeMailer.ts index 7b4f8dcbc2..9913b9c4d6 100644 --- a/packages/cli/src/UserManagement/email/NodeMailer.ts +++ b/packages/cli/src/UserManagement/email/NodeMailer.ts @@ -5,9 +5,9 @@ import config from '@/config'; import { MailData, SendEmailResult, UserManagementMailerImplementation } from './Interfaces'; export class NodeMailer implements UserManagementMailerImplementation { - private transport: Transporter; + private transport?: Transporter; - constructor() { + async init(): Promise { this.transport = createTransport({ host: config.getEnv('userManagement.emails.smtp.host'), port: config.getEnv('userManagement.emails.smtp.port'), @@ -20,12 +20,15 @@ export class NodeMailer implements UserManagementMailerImplementation { } async verifyConnection(): Promise { + if (!this.transport) { + await this.init(); + } const host = config.getEnv('userManagement.emails.smtp.host'); const user = config.getEnv('userManagement.emails.smtp.auth.user'); const pass = config.getEnv('userManagement.emails.smtp.auth.pass'); try { - await this.transport.verify(); + await this.transport?.verify(); } catch (error) { const message: string[] = []; if (!host) message.push('SMTP host not defined (N8N_SMTP_HOST).'); @@ -36,6 +39,9 @@ export class NodeMailer implements UserManagementMailerImplementation { } async sendMail(mailData: MailData): Promise { + if (!this.transport) { + await this.init(); + } let sender = config.getEnv('userManagement.emails.smtp.sender'); const user = config.getEnv('userManagement.emails.smtp.auth.user'); @@ -44,7 +50,7 @@ export class NodeMailer implements UserManagementMailerImplementation { } try { - await this.transport.sendMail({ + await this.transport?.sendMail({ from: sender, to: mailData.emailRecipients, subject: mailData.subject, @@ -57,12 +63,9 @@ export class NodeMailer implements UserManagementMailerImplementation { } catch (error) { ErrorReporter.error(error); Logger.error('Failed to send email', { recipients: mailData.emailRecipients, error }); - return { - success: false, - error, - }; + throw error; } - return { success: true }; + return { emailSent: true }; } } diff --git a/packages/cli/src/UserManagement/email/UserManagementMailer.ts b/packages/cli/src/UserManagement/email/UserManagementMailer.ts index 43aed067a8..3455747f9d 100644 --- a/packages/cli/src/UserManagement/email/UserManagementMailer.ts +++ b/packages/cli/src/UserManagement/email/UserManagementMailer.ts @@ -44,57 +44,52 @@ export class UserManagementMailer { constructor() { // Other implementations can be used in the future. - if (config.getEnv('userManagement.emails.mode') === 'smtp') { + if ( + config.getEnv('userManagement.emails.mode') === 'smtp' && + config.getEnv('userManagement.emails.smtp.host') !== '' + ) { this.mailer = new NodeMailer(); } } async verifyConnection(): Promise { - if (!this.mailer) return Promise.reject(); + if (!this.mailer) throw new Error('No mailer configured.'); return this.mailer.verifyConnection(); } async invite(inviteEmailData: InviteEmailData): Promise { - if (!this.mailer) return Promise.reject(); - const template = await getTemplate('invite'); - const result = await this.mailer.sendMail({ + const result = await this.mailer?.sendMail({ emailRecipients: inviteEmailData.email, subject: 'You have been invited to n8n', body: template(inviteEmailData), }); // If mailer does not exist it means mail has been disabled. - return result ?? { success: true }; + // No error, just say no email was sent. + return result ?? { emailSent: false }; } async passwordReset(passwordResetData: PasswordResetData): Promise { - if (!this.mailer) return Promise.reject(); - const template = await getTemplate('passwordReset'); - const result = await this.mailer.sendMail({ + const result = await this.mailer?.sendMail({ emailRecipients: passwordResetData.email, subject: 'n8n password reset', body: template(passwordResetData), }); // If mailer does not exist it means mail has been disabled. - return result ?? { success: true }; + // No error, just say no email was sent. + return result ?? { emailSent: false }; } } let mailerInstance: UserManagementMailer | undefined; -export async function getInstance(): Promise { +export function getInstance(): UserManagementMailer { if (mailerInstance === undefined) { mailerInstance = new UserManagementMailer(); - try { - await mailerInstance.verifyConnection(); - } catch (error) { - mailerInstance = undefined; - throw error; - } } return mailerInstance; } diff --git a/packages/cli/src/UserManagement/routes/passwordReset.ts b/packages/cli/src/UserManagement/routes/passwordReset.ts index 12c2f5e2cd..0410f3107a 100644 --- a/packages/cli/src/UserManagement/routes/passwordReset.ts +++ b/packages/cli/src/UserManagement/routes/passwordReset.ts @@ -76,7 +76,7 @@ export function passwordResetNamespace(this: N8nApp): void { url.searchParams.append('token', resetPasswordToken); try { - const mailer = await UserManagementMailer.getInstance(); + const mailer = UserManagementMailer.getInstance(); await mailer.passwordReset({ email, firstName, diff --git a/packages/cli/src/UserManagement/routes/users.ts b/packages/cli/src/UserManagement/routes/users.ts index 7ee79d5262..a19eb30db6 100644 --- a/packages/cli/src/UserManagement/routes/users.ts +++ b/packages/cli/src/UserManagement/routes/users.ts @@ -14,6 +14,8 @@ import { UserRequest } from '@/requests'; import * as UserManagementMailer from '../email/UserManagementMailer'; import { N8nApp, PublicUser } from '../Interfaces'; import { + addInviteLinktoUser, + generateUserInviteUrl, getInstanceBaseUrl, hashPassword, isEmailSetUp, @@ -34,25 +36,7 @@ export function usersNamespace(this: N8nApp): void { this.app.post( `/${this.restEndpoint}/users`, ResponseHelper.send(async (req: UserRequest.Invite) => { - if (config.getEnv('userManagement.emails.mode') === '') { - Logger.debug( - 'Request to send email invite(s) to user(s) failed because emailing was not set up', - ); - throw new ResponseHelper.InternalServerError( - 'Email sending must be set up in order to request a password reset email', - ); - } - - let mailer: UserManagementMailer.UserManagementMailer | undefined; - try { - mailer = await UserManagementMailer.getInstance(); - } catch (error) { - if (error instanceof Error) { - throw new ResponseHelper.InternalServerError( - `There is a problem with your SMTP setup! ${error.message}`, - ); - } - } + const mailer = UserManagementMailer.getInstance(); // TODO: this should be checked in the middleware rather than here if (isUserManagementDisabled()) { @@ -143,19 +127,13 @@ export function usersNamespace(this: N8nApp): void { }), ); }); - - void InternalHooksManager.getInstance().onUserInvite({ - user: req.user, - target_user_id: Object.values(createUsers) as string[], - public_api: false, - }); } catch (error) { ErrorReporter.error(error); Logger.error('Failed to create user shells', { userShells: createUsers }); throw new ResponseHelper.InternalServerError('An error occurred during user creation'); } - Logger.info('Created user shell(s) successfully', { userId: req.user.id }); + Logger.debug('Created user shell(s) successfully', { userId: req.user.id }); Logger.verbose(total > 1 ? `${total} user shells created` : '1 user shell created', { userShells: createUsers, }); @@ -168,39 +146,61 @@ export function usersNamespace(this: N8nApp): void { const emailingResults = await Promise.all( usersPendingSetup.map(async ([email, id]) => { - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - const inviteAcceptUrl = `${baseUrl}/signup?inviterId=${req.user.id}&inviteeId=${id}`; - const result = await mailer?.invite({ - email, - inviteAcceptUrl, - domain: baseUrl, - }); - const resp: { user: { id: string | null; email: string }; error?: string } = { + if (!id) { + // This should never happen since those are removed from the list before reaching this point + throw new ResponseHelper.InternalServerError( + 'User ID is missing for user with email address', + ); + } + const inviteAcceptUrl = generateUserInviteUrl(req.user.id, id); + const resp: { + user: { id: string | null; email: string; inviteAcceptUrl: string; emailSent: boolean }; + error?: string; + } = { user: { id, email, + inviteAcceptUrl, + emailSent: false, }, }; - if (result?.success) { - void InternalHooksManager.getInstance().onUserTransactionalEmail({ - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - user_id: id!, - message_type: 'New user invite', - public_api: false, - }); - } else { - void InternalHooksManager.getInstance().onEmailFailed({ - user: req.user, - message_type: 'New user invite', - public_api: false, - }); - Logger.error('Failed to send email', { - userId: req.user.id, + try { + const result = await mailer.invite({ + email, inviteAcceptUrl, domain: baseUrl, - email, }); - resp.error = 'Email could not be sent'; + if (result.emailSent) { + resp.user.emailSent = true; + void InternalHooksManager.getInstance().onUserTransactionalEmail({ + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + user_id: id, + message_type: 'New user invite', + public_api: false, + }); + } + + void InternalHooksManager.getInstance().onUserInvite({ + user: req.user, + target_user_id: Object.values(createUsers) as string[], + public_api: false, + email_sent: result.emailSent, + }); + } catch (error) { + if (error instanceof Error) { + void InternalHooksManager.getInstance().onEmailFailed({ + user: req.user, + message_type: 'New user invite', + public_api: false, + }); + Logger.error('Failed to send email', { + userId: req.user.id, + inviteAcceptUrl, + domain: baseUrl, + email, + }); + resp.error = error.message; + } } return resp; }), @@ -361,10 +361,13 @@ export function usersNamespace(this: N8nApp): void { this.app.get( `/${this.restEndpoint}/users`, - ResponseHelper.send(async () => { + ResponseHelper.send(async (req: UserRequest.List) => { const users = await Db.collections.User.find({ relations: ['globalRole'] }); - return users.map((user): PublicUser => sanitizeUser(user, ['personalizationAnswers'])); + return users.map( + (user): PublicUser => + addInviteLinktoUser(sanitizeUser(user, ['personalizationAnswers']), req.user.id), + ); }), ); @@ -563,22 +566,27 @@ export function usersNamespace(this: N8nApp): void { const baseUrl = getInstanceBaseUrl(); const inviteAcceptUrl = `${baseUrl}/signup?inviterId=${req.user.id}&inviteeId=${reinvitee.id}`; - let mailer: UserManagementMailer.UserManagementMailer | undefined; + const mailer = UserManagementMailer.getInstance(); try { - mailer = await UserManagementMailer.getInstance(); - } catch (error) { - if (error instanceof Error) { - throw new ResponseHelper.InternalServerError(error.message); + const result = await mailer.invite({ + email: reinvitee.email, + inviteAcceptUrl, + domain: baseUrl, + }); + if (result.emailSent) { + void InternalHooksManager.getInstance().onUserReinvite({ + user: req.user, + target_user_id: reinvitee.id, + public_api: false, + }); + + void InternalHooksManager.getInstance().onUserTransactionalEmail({ + user_id: reinvitee.id, + message_type: 'Resend invite', + public_api: false, + }); } - } - - const result = await mailer?.invite({ - email: reinvitee.email, - inviteAcceptUrl, - domain: baseUrl, - }); - - if (!result?.success) { + } catch (error) { void InternalHooksManager.getInstance().onEmailFailed({ user: reinvitee, message_type: 'Resend invite', @@ -591,19 +599,6 @@ export function usersNamespace(this: N8nApp): void { }); throw new ResponseHelper.InternalServerError(`Failed to send email to ${reinvitee.email}`); } - - void InternalHooksManager.getInstance().onUserReinvite({ - user: reinvitee, - target_user_id: reinvitee.id, - public_api: false, - }); - - void InternalHooksManager.getInstance().onUserTransactionalEmail({ - user_id: reinvitee.id, - message_type: 'Resend invite', - public_api: false, - }); - return { success: true }; }), ); diff --git a/packages/cli/src/requests.d.ts b/packages/cli/src/requests.d.ts index e9a7578629..b6b14a6919 100644 --- a/packages/cli/src/requests.d.ts +++ b/packages/cli/src/requests.d.ts @@ -197,6 +197,8 @@ export declare namespace PasswordResetRequest { // ---------------------------------- export declare namespace UserRequest { + export type List = AuthenticatedRequest; + export type Invite = AuthenticatedRequest<{}, {}, Array<{ email: string }>>; export type ResolveSignUp = AuthlessRequest< diff --git a/packages/cli/test/integration/users.api.test.ts b/packages/cli/test/integration/users.api.test.ts index 25e1c4f38e..d2b978fcd0 100644 --- a/packages/cli/test/integration/users.api.test.ts +++ b/packages/cli/test/integration/users.api.test.ts @@ -65,7 +65,8 @@ beforeEach(async () => { config.set('userManagement.disabled', false); config.set('userManagement.isInstanceOwnerSetUp', true); - config.set('userManagement.emails.mode', ''); + config.set('userManagement.emails.mode', 'smtp'); + config.set('userManagement.emails.smtp.host', ''); }); afterAll(async () => { @@ -432,26 +433,28 @@ test('POST /users/:id should fail with already accepted invite', async () => { expect(storedMember.password).not.toBe(newMemberData.password); }); -test('POST /users should fail if emailing is not set up', async () => { +test('POST /users should succeed if emailing is not set up', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const response = await authAgent(owner) .post('/users') .send([{ email: randomEmail() }]); - expect(response.statusCode).toBe(500); + expect(response.statusCode).toBe(200); + expect(response.body.data[0].user.inviteAcceptUrl).toBeDefined(); }); test('POST /users should fail if user management is disabled', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); config.set('userManagement.disabled', true); + config.set('userManagement.isInstanceOwnerSetUp', false); const response = await authAgent(owner) .post('/users') .send([{ email: randomEmail() }]); - expect(response.statusCode).toBe(500); + expect(response.statusCode).toBe(400); }); test('POST /users should email invites and create user shells but ignore existing', async () => { @@ -567,16 +570,34 @@ test('POST /users/:id/reinvite should send reinvite, but fail if user already ac expect(reinviteMemberResponse.statusCode).toBe(400); }); -test('UserManagementMailer expect NodeMailer.verifyConnection have been called', async () => { - jest.spyOn(NodeMailer.prototype, 'verifyConnection').mockImplementation(async () => {}); +test('UserManagementMailer expect NodeMailer.verifyConnection not be called when SMTP not set up', async () => { + const mockVerifyConnection = jest.spyOn(NodeMailer.prototype, 'verifyConnection'); + mockVerifyConnection.mockImplementation(async () => {}); - // NodeMailer.verifyConnection called 1 time const userManagementMailer = UserManagementMailer.getInstance(); - // NodeMailer.verifyConnection called 2 time - (await userManagementMailer).verifyConnection(); + // NodeMailer.verifyConnection gets called only explicitly + expect(async () => await userManagementMailer.verifyConnection()).rejects.toThrow(); - expect(NodeMailer.prototype.verifyConnection).toHaveBeenCalledTimes(2); + expect(NodeMailer.prototype.verifyConnection).toHaveBeenCalledTimes(0); - // @ts-ignore - NodeMailer.prototype.verifyConnection.mockRestore(); + mockVerifyConnection.mockRestore(); +}); + +test('UserManagementMailer expect NodeMailer.verifyConnection to be called when SMTP set up', async () => { + const mockVerifyConnection = jest.spyOn(NodeMailer.prototype, 'verifyConnection'); + mockVerifyConnection.mockImplementation(async () => {}); + const mockInit = jest.spyOn(NodeMailer.prototype, 'init'); + mockInit.mockImplementation(async () => {}); + + // host needs to be set, otherwise smtp is skipped + config.set('userManagement.emails.smtp.host', 'host'); + config.set('userManagement.emails.mode', 'smtp'); + + const userManagementMailer = new UserManagementMailer.UserManagementMailer(); + // NodeMailer.verifyConnection gets called only explicitly + expect(async () => await userManagementMailer.verifyConnection()).not.toThrow(); + + // expect(NodeMailer.prototype.verifyConnection).toHaveBeenCalledTimes(1); + mockVerifyConnection.mockRestore(); + mockInit.mockRestore(); }); diff --git a/packages/design-system/src/components/N8nUsersList/UsersList.stories.ts b/packages/design-system/src/components/N8nUsersList/UsersList.stories.ts index f3e9e65d75..9aada29140 100644 --- a/packages/design-system/src/components/N8nUsersList/UsersList.stories.ts +++ b/packages/design-system/src/components/N8nUsersList/UsersList.stories.ts @@ -1,6 +1,7 @@ import N8nUsersList from './UsersList.vue'; import { action } from '@storybook/addon-actions'; import type { StoryFn } from '@storybook/vue'; +import { IUser } from '@/types'; export default { title: 'Modules/UsersList', @@ -21,12 +22,24 @@ const Template: StoryFn = (args, { argTypes }) => ({ components: { N8nUsersList, }, - template: '', + template: + '', methods, }); export const UsersList = Template.bind({}); UsersList.args = { + actions: [ + { + label: 'Resend Invite', + value: 'reinvite', + guard: (user: IUser) => !user.firstName, + }, + { + label: 'Delete User', + value: 'delete', + }, + ], users: [ { id: '1', diff --git a/packages/design-system/src/components/N8nUsersList/UsersList.vue b/packages/design-system/src/components/N8nUsersList/UsersList.vue index abe3c8a0a5..2dbfec678d 100644 --- a/packages/design-system/src/components/N8nUsersList/UsersList.vue +++ b/packages/design-system/src/components/N8nUsersList/UsersList.vue @@ -25,20 +25,14 @@ diff --git a/packages/editor-ui/src/components/WorkflowShareModal.ee.vue b/packages/editor-ui/src/components/WorkflowShareModal.ee.vue index c51c36df03..d874a3c2a0 100644 --- a/packages/editor-ui/src/components/WorkflowShareModal.ee.vue +++ b/packages/editor-ui/src/components/WorkflowShareModal.ee.vue @@ -12,7 +12,7 @@ {{ $locale.baseText( - contextBasedTranslationKeys.workflows.sharing.unavailable.description.modal, + uiStore.contextBasedTranslationKeys.workflows.sharing.unavailable.description.modal, ) }} @@ -50,7 +50,6 @@ :currentUserId="currentUser.id" :delete-label="$locale.baseText('workflows.shareModal.list.delete')" :readonly="!workflowPermissions.updateSharing" - @delete="onRemoveSharee" >