From 40c1eeeddd1259b92c64ff085adb3929cbbb01b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 2 Jan 2024 17:53:24 +0100 Subject: [PATCH] refactor(core): Continue moving `typeorm` operators to repositories (no-changelog) (#8186) Follow-up to: #8163 --- .../handlers/workflows/workflows.handler.ts | 21 ++++-- .../handlers/workflows/workflows.service.ts | 53 ++------------- .../src/UserManagement/PermissionChecker.ts | 18 ++--- .../collaboration/collaboration.service.ts | 7 +- .../cli/src/commands/export/credentials.ts | 11 +-- packages/cli/src/commands/export/workflow.ts | 9 +-- packages/cli/src/commands/ldap/reset.ts | 3 +- packages/cli/src/commands/list/workflow.ts | 13 ++-- packages/cli/src/commands/update/workflow.ts | 12 +--- .../cli/src/controllers/auth.controller.ts | 2 +- .../src/controllers/invitation.controller.ts | 2 +- packages/cli/src/controllers/me.controller.ts | 14 ++-- .../cli/src/controllers/owner.controller.ts | 4 +- .../controllers/passwordReset.controller.ts | 11 +-- .../cli/src/controllers/users.controller.ts | 68 ++++++------------- .../src/credentials/credentials.service.ee.ts | 4 +- .../sharedCredentials.repository.ts | 10 +++ .../repositories/sharedWorkflow.repository.ts | 55 ++++++++++++++- .../databases/repositories/tag.repository.ts | 56 +++++++++++++++ .../databases/repositories/user.repository.ts | 68 ++++++++++++++++++- .../repositories/workflow.repository.ts | 12 ++++ .../variables/variables.service.ee.ts | 7 +- .../MessageEventBusDestination.ee.ts | 5 +- .../src/eventbus/eventBus.controller.ee.ts | 3 +- packages/cli/src/services/import.service.ts | 41 +---------- packages/cli/src/services/role.service.ts | 5 ++ packages/cli/src/services/tag.service.ts | 14 ---- packages/cli/src/services/user.service.ts | 37 +--------- packages/cli/src/services/webhook.service.ts | 3 +- .../cli/src/workflows/workflow.service.ee.ts | 38 +++-------- .../cli/src/workflows/workflow.service.ts | 25 ------- .../cli/src/workflows/workflows.controller.ts | 12 ++-- .../collaboration.service.test.ts | 37 +++++----- .../unit/controllers/me.controller.test.ts | 8 ++- .../unit/controllers/owner.controller.test.ts | 7 +- 35 files changed, 341 insertions(+), 354 deletions(-) diff --git a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts index 542bd6a209..67f9b908a2 100644 --- a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts @@ -18,16 +18,16 @@ import { setWorkflowAsActive, setWorkflowAsInactive, updateWorkflow, - getSharedWorkflows, createWorkflow, - getWorkflowIdsViaTags, parseTagNames, - getWorkflowsAndCount, } from './workflows.service'; import { WorkflowService } from '@/workflows/workflow.service'; import { InternalHooks } from '@/InternalHooks'; import { RoleService } from '@/services/role.service'; import { WorkflowHistoryService } from '@/workflows/workflowHistory/workflowHistory.service.ee'; +import { SharedWorkflowRepository } from '@/databases/repositories/sharedWorkflow.repository'; +import { TagRepository } from '@/databases/repositories/tag.repository'; +import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; export = { createWorkflow: [ @@ -106,17 +106,24 @@ export = { if (['owner', 'admin'].includes(req.user.globalRole.name)) { if (tags) { - const workflowIds = await getWorkflowIdsViaTags(parseTagNames(tags)); + const workflowIds = await Container.get(TagRepository).getWorkflowIdsViaTags( + parseTagNames(tags), + ); where.id = In(workflowIds); } } else { const options: { workflowIds?: string[] } = {}; if (tags) { - options.workflowIds = await getWorkflowIdsViaTags(parseTagNames(tags)); + options.workflowIds = await Container.get(TagRepository).getWorkflowIdsViaTags( + parseTagNames(tags), + ); } - const sharedWorkflows = await getSharedWorkflows(req.user, options); + const sharedWorkflows = await Container.get(SharedWorkflowRepository).getSharedWorkflows( + req.user, + options, + ); if (!sharedWorkflows.length) { return res.status(200).json({ @@ -129,7 +136,7 @@ export = { where.id = In(workflowsIds); } - const [workflows, count] = await getWorkflowsAndCount({ + const [workflows, count] = await Container.get(WorkflowRepository).findAndCount({ skip: offset, take: limit, where, diff --git a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts index 9ca171612e..607aeb31d6 100644 --- a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts @@ -1,14 +1,9 @@ -import type { FindManyOptions, UpdateResult } from 'typeorm'; -import { In } from 'typeorm'; -import intersection from 'lodash/intersection'; - import * as Db from '@/Db'; import type { User } from '@db/entities/User'; import { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { SharedWorkflow } from '@db/entities/SharedWorkflow'; import type { Role } from '@db/entities/Role'; import config from '@/config'; -import { TagService } from '@/services/tag.service'; import Container from 'typedi'; import { WorkflowRepository } from '@db/repositories/workflow.repository'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; @@ -39,43 +34,12 @@ export async function getSharedWorkflow( }); } -export async function getSharedWorkflows( - user: User, - options: { - relations?: string[]; - workflowIds?: string[]; - }, -): Promise { - return Container.get(SharedWorkflowRepository).find({ - where: { - ...(!['owner', 'admin'].includes(user.globalRole.name) && { userId: user.id }), - ...(options.workflowIds && { workflowId: In(options.workflowIds) }), - }, - ...(options.relations && { relations: options.relations }), - }); -} - export async function getWorkflowById(id: string): Promise { return Container.get(WorkflowRepository).findOne({ where: { id }, }); } -/** - * Returns the workflow IDs that have certain tags. - * Intersection! e.g. workflow needs to have all provided tags. - */ -export async function getWorkflowIdsViaTags(tags: string[]): Promise { - const dbTags = await Container.get(TagService).findMany({ - where: { name: In(tags) }, - relations: ['workflows'], - }); - - const workflowIdsPerTag = dbTags.map((tag) => tag.workflows.map((workflow) => workflow.id)); - - return intersection(...workflowIdsPerTag); -} - export async function createWorkflow( workflow: WorkflowEntity, user: User, @@ -98,14 +62,14 @@ export async function createWorkflow( }); } -export async function setWorkflowAsActive(workflow: WorkflowEntity): Promise { - return Container.get(WorkflowRepository).update(workflow.id, { +export async function setWorkflowAsActive(workflow: WorkflowEntity) { + await Container.get(WorkflowRepository).update(workflow.id, { active: true, updatedAt: new Date(), }); } -export async function setWorkflowAsInactive(workflow: WorkflowEntity): Promise { +export async function setWorkflowAsInactive(workflow: WorkflowEntity) { return Container.get(WorkflowRepository).update(workflow.id, { active: false, updatedAt: new Date(), @@ -116,16 +80,7 @@ export async function deleteWorkflow(workflow: WorkflowEntity): Promise, -): Promise<[WorkflowEntity[], number]> { - return Container.get(WorkflowRepository).findAndCount(options); -} - -export async function updateWorkflow( - workflowId: string, - updateData: WorkflowEntity, -): Promise { +export async function updateWorkflow(workflowId: string, updateData: WorkflowEntity) { return Container.get(WorkflowRepository).update(workflowId, updateData); } diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts index b9099915e0..a5e1ffdb8a 100644 --- a/packages/cli/src/UserManagement/PermissionChecker.ts +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -1,9 +1,6 @@ import type { INode, Workflow } from 'n8n-workflow'; import { NodeOperationError, WorkflowOperationError } from 'n8n-workflow'; -import type { FindOptionsWhere } from 'typeorm'; -import { In } from 'typeorm'; import config from '@/config'; -import type { SharedCredentials } from '@db/entities/SharedCredentials'; import { isSharingEnabled } from './UserManagementHelper'; import { OwnershipService } from '@/services/ownership.service'; import Container from 'typedi'; @@ -48,17 +45,12 @@ export class PermissionChecker { workflowUserIds = workflowSharings.map((s) => s.userId); } - const credentialsWhere: FindOptionsWhere = { userId: In(workflowUserIds) }; + const roleId = await Container.get(RoleService).findCredentialOwnerRoleId(); - if (!isSharingEnabled()) { - const role = await Container.get(RoleService).findCredentialOwnerRole(); - // If credential sharing is not enabled, get only credentials owned by this user - credentialsWhere.roleId = role.id; - } - - const credentialSharings = await Container.get(SharedCredentialsRepository).find({ - where: credentialsWhere, - }); + const credentialSharings = await Container.get(SharedCredentialsRepository).findSharings( + workflowUserIds, + roleId, + ); const accessibleCredIds = credentialSharings.map((s) => s.credentialsId); diff --git a/packages/cli/src/collaboration/collaboration.service.ts b/packages/cli/src/collaboration/collaboration.service.ts index b19378e372..6d52d26558 100644 --- a/packages/cli/src/collaboration/collaboration.service.ts +++ b/packages/cli/src/collaboration/collaboration.service.ts @@ -10,6 +10,7 @@ import type { IActiveWorkflowUsersChanged } from '../Interfaces'; import type { OnPushMessageEvent } from '@/push/types'; import { CollaborationState } from '@/collaboration/collaboration.state'; import { TIME } from '@/constants'; +import { UserRepository } from '@/databases/repositories/user.repository'; /** * After how many minutes of inactivity a user should be removed @@ -28,6 +29,7 @@ export class CollaborationService { private readonly push: Push, private readonly state: CollaborationState, private readonly userService: UserService, + private readonly userRepository: UserRepository, ) { if (!push.isBidirectional) { logger.warn( @@ -89,7 +91,10 @@ export class CollaborationService { if (workflowUserIds.length === 0) { return; } - const users = await this.userService.getByIds(this.userService.getManager(), workflowUserIds); + const users = await this.userRepository.getByIds( + this.userService.getManager(), + workflowUserIds, + ); const msgData: IActiveWorkflowUsersChanged = { workflowId, diff --git a/packages/cli/src/commands/export/credentials.ts b/packages/cli/src/commands/export/credentials.ts index 8b2c5394d4..47008aa734 100644 --- a/packages/cli/src/commands/export/credentials.ts +++ b/packages/cli/src/commands/export/credentials.ts @@ -1,7 +1,6 @@ import { flags } from '@oclif/command'; import fs from 'fs'; import path from 'path'; -import type { FindOptionsWhere } from 'typeorm'; import { Credentials } from 'n8n-core'; import type { ICredentialsDb, ICredentialsDecryptedDb } from '@/Interfaces'; import { BaseCommand } from '../BaseCommand'; @@ -107,13 +106,9 @@ export class ExportCredentialsCommand extends BaseCommand { } } - const findQuery: FindOptionsWhere = {}; - if (flags.id) { - findQuery.id = flags.id; - } - - const credentials: ICredentialsDb[] = - await Container.get(CredentialsRepository).findBy(findQuery); + const credentials: ICredentialsDb[] = await Container.get(CredentialsRepository).findBy( + flags.id ? { id: flags.id } : {}, + ); if (flags.decrypted) { for (let i = 0; i < credentials.length; i++) { diff --git a/packages/cli/src/commands/export/workflow.ts b/packages/cli/src/commands/export/workflow.ts index aceaa29bbb..3aac28ffbb 100644 --- a/packages/cli/src/commands/export/workflow.ts +++ b/packages/cli/src/commands/export/workflow.ts @@ -1,8 +1,6 @@ import { flags } from '@oclif/command'; import fs from 'fs'; import path from 'path'; -import type { FindOptionsWhere } from 'typeorm'; -import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { BaseCommand } from '../BaseCommand'; import { WorkflowRepository } from '@db/repositories/workflow.repository'; import Container from 'typedi'; @@ -101,13 +99,8 @@ export class ExportWorkflowsCommand extends BaseCommand { } } - const findQuery: FindOptionsWhere = {}; - if (flags.id) { - findQuery.id = flags.id; - } - const workflows = await Container.get(WorkflowRepository).find({ - where: findQuery, + where: flags.id ? { id: flags.id } : {}, relations: ['tags'], }); diff --git a/packages/cli/src/commands/ldap/reset.ts b/packages/cli/src/commands/ldap/reset.ts index c67b9973e2..2693465239 100644 --- a/packages/cli/src/commands/ldap/reset.ts +++ b/packages/cli/src/commands/ldap/reset.ts @@ -1,6 +1,5 @@ import Container from 'typedi'; import { LDAP_DEFAULT_CONFIGURATION, LDAP_FEATURE_NAME } from '@/Ldap/constants'; -import { In } from 'typeorm'; import { AuthIdentityRepository } from '@db/repositories/authIdentity.repository'; import { AuthProviderSyncHistoryRepository } from '@db/repositories/authProviderSyncHistory.repository'; import { SettingsRepository } from '@db/repositories/settings.repository'; @@ -17,7 +16,7 @@ export class Reset extends BaseCommand { }); await Container.get(AuthProviderSyncHistoryRepository).delete({ providerType: 'ldap' }); await Container.get(AuthIdentityRepository).delete({ providerType: 'ldap' }); - await Container.get(UserRepository).delete({ id: In(ldapIdentities.map((i) => i.userId)) }); + await Container.get(UserRepository).deleteMany(ldapIdentities.map((i) => i.userId)); await Container.get(SettingsRepository).delete({ key: LDAP_FEATURE_NAME }); await Container.get(SettingsRepository).insert({ key: LDAP_FEATURE_NAME, diff --git a/packages/cli/src/commands/list/workflow.ts b/packages/cli/src/commands/list/workflow.ts index ce0659dcd9..abab56f2d5 100644 --- a/packages/cli/src/commands/list/workflow.ts +++ b/packages/cli/src/commands/list/workflow.ts @@ -1,6 +1,4 @@ import { flags } from '@oclif/command'; -import type { FindOptionsWhere } from 'typeorm'; -import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { BaseCommand } from '../BaseCommand'; import { WorkflowRepository } from '@db/repositories/workflow.repository'; import Container from 'typedi'; @@ -32,12 +30,13 @@ export class ListWorkflowCommand extends BaseCommand { this.error('The --active flag has to be passed using true or false'); } - const findQuery: FindOptionsWhere = {}; - if (flags.active !== undefined) { - findQuery.active = flags.active === 'true'; - } + const workflowRepository = Container.get(WorkflowRepository); + + const workflows = + flags.active !== undefined + ? await workflowRepository.findByActiveState(flags.active === 'true') + : await workflowRepository.find(); - const workflows = await Container.get(WorkflowRepository).findBy(findQuery); if (flags.onlyId) { workflows.forEach((workflow) => this.logger.info(workflow.id)); } else { diff --git a/packages/cli/src/commands/update/workflow.ts b/packages/cli/src/commands/update/workflow.ts index e78a6ebc77..08f9403368 100644 --- a/packages/cli/src/commands/update/workflow.ts +++ b/packages/cli/src/commands/update/workflow.ts @@ -1,7 +1,4 @@ import { flags } from '@oclif/command'; -import type { FindOptionsWhere } from 'typeorm'; -import type { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity'; -import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { BaseCommand } from '../BaseCommand'; import { WorkflowRepository } from '@db/repositories/workflow.repository'; import Container from 'typedi'; @@ -43,7 +40,6 @@ export class UpdateWorkflowCommand extends BaseCommand { return; } - const updateQuery: QueryDeepPartialEntity = {}; if (flags.active === undefined) { console.info('No update flag like "--active=true" has been set!'); return; @@ -54,18 +50,16 @@ export class UpdateWorkflowCommand extends BaseCommand { return; } - updateQuery.active = flags.active === 'true'; + const newState = flags.active === 'true'; - const findQuery: FindOptionsWhere = {}; if (flags.id) { this.logger.info(`Deactivating workflow with ID: ${flags.id}`); - findQuery.id = flags.id; + await Container.get(WorkflowRepository).updateActiveState(flags.id, newState); } else { this.logger.info('Deactivating all workflows'); - findQuery.active = true; + await Container.get(WorkflowRepository).deactivateAll(); } - await Container.get(WorkflowRepository).update(findQuery, updateQuery); this.logger.info('Done'); } diff --git a/packages/cli/src/controllers/auth.controller.ts b/packages/cli/src/controllers/auth.controller.ts index 235177023c..35bdacd16f 100644 --- a/packages/cli/src/controllers/auth.controller.ts +++ b/packages/cli/src/controllers/auth.controller.ts @@ -138,7 +138,7 @@ export class AuthController { } try { - user = await this.userService.findOneOrFail({ where: {} }); + user = await this.userRepository.findOneOrFail({ where: {}, relations: ['globalRole'] }); } catch (error) { throw new InternalServerError( 'No users found in database - did you wipe the users table? Create at least one user.', diff --git a/packages/cli/src/controllers/invitation.controller.ts b/packages/cli/src/controllers/invitation.controller.ts index 6d24d53965..379a993021 100644 --- a/packages/cli/src/controllers/invitation.controller.ts +++ b/packages/cli/src/controllers/invitation.controller.ts @@ -163,7 +163,7 @@ export class InvitationController { invitee.lastName = lastName; invitee.password = await this.passwordUtility.hash(validPassword); - const updatedUser = await this.userService.save(invitee); + const updatedUser = await this.userRepository.save(invitee); await issueCookie(res, updatedUser); diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index 1c518ef744..a7b306d5df 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -20,6 +20,7 @@ import { Logger } from '@/Logger'; import { ExternalHooks } from '@/ExternalHooks'; import { InternalHooks } from '@/InternalHooks'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { UserRepository } from '@/databases/repositories/user.repository'; @Authorized() @RestController('/me') @@ -30,6 +31,7 @@ export class MeController { private readonly internalHooks: InternalHooks, private readonly userService: UserService, private readonly passwordUtility: PasswordUtility, + private readonly userRepository: UserRepository, ) {} /** @@ -76,7 +78,10 @@ export class MeController { await this.externalHooks.run('user.profile.beforeUpdate', [userId, currentEmail, payload]); await this.userService.update(userId, payload); - const user = await this.userService.findOneOrFail({ where: { id: userId } }); + const user = await this.userRepository.findOneOrFail({ + where: { id: userId }, + relations: ['globalRole'], + }); this.logger.info('User updated successfully', { userId }); @@ -132,7 +137,7 @@ export class MeController { req.user.password = await this.passwordUtility.hash(validPassword); - const user = await this.userService.save(req.user); + const user = await this.userRepository.save(req.user); this.logger.info('Password updated successfully', { userId: user.id }); await issueCookie(res, user); @@ -164,7 +169,7 @@ export class MeController { throw new BadRequestError('Personalization answers are mandatory'); } - await this.userService.save({ + await this.userRepository.save({ id: req.user.id, // @ts-ignore personalizationAnswers, @@ -227,9 +232,10 @@ export class MeController { await this.userService.updateSettings(id, payload); - const user = await this.userService.findOneOrFail({ + const user = await this.userRepository.findOneOrFail({ select: ['settings'], where: { id }, + relations: ['globalRole'], }); return user.settings; diff --git a/packages/cli/src/controllers/owner.controller.ts b/packages/cli/src/controllers/owner.controller.ts index 2b496c26af..2caa1e0616 100644 --- a/packages/cli/src/controllers/owner.controller.ts +++ b/packages/cli/src/controllers/owner.controller.ts @@ -13,6 +13,7 @@ import { UserService } from '@/services/user.service'; import { Logger } from '@/Logger'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { InternalHooks } from '@/InternalHooks'; +import { UserRepository } from '@/databases/repositories/user.repository'; @Authorized(['global', 'owner']) @RestController('/owner') @@ -24,6 +25,7 @@ export class OwnerController { private readonly userService: UserService, private readonly passwordUtility: PasswordUtility, private readonly postHog: PostHogClient, + private readonly userRepository: UserRepository, ) {} /** @@ -85,7 +87,7 @@ export class OwnerController { await validateEntity(owner); - owner = await this.userService.save(owner); + owner = await this.userRepository.save(owner); this.logger.info('Owner was set up successfully', { userId }); diff --git a/packages/cli/src/controllers/passwordReset.controller.ts b/packages/cli/src/controllers/passwordReset.controller.ts index b8e26b8c75..45d23f3c25 100644 --- a/packages/cli/src/controllers/passwordReset.controller.ts +++ b/packages/cli/src/controllers/passwordReset.controller.ts @@ -1,6 +1,5 @@ import { Response } from 'express'; import { rateLimit } from 'express-rate-limit'; -import { IsNull, Not } from 'typeorm'; import validator from 'validator'; import { Get, Post, RestController } from '@/decorators'; @@ -23,6 +22,7 @@ import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { UnprocessableRequestError } from '@/errors/response-errors/unprocessable.error'; +import { UserRepository } from '@/databases/repositories/user.repository'; const throttle = rateLimit({ windowMs: 5 * 60 * 1000, // 5 minutes @@ -42,6 +42,7 @@ export class PasswordResetController { private readonly urlService: UrlService, private readonly license: License, private readonly passwordUtility: PasswordUtility, + private readonly userRepository: UserRepository, ) {} /** @@ -78,13 +79,7 @@ export class PasswordResetController { } // User should just be able to reset password if one is already present - const user = await this.userService.findOne({ - where: { - email, - password: Not(IsNull()), - }, - relations: ['authIdentities', 'globalRole'], - }); + const user = await this.userRepository.findNonShellUser(email); if (!user?.isOwner && !this.license.isWithinUsersLimit()) { this.logger.debug( diff --git a/packages/cli/src/controllers/users.controller.ts b/packages/cli/src/controllers/users.controller.ts index 376f047d15..9a62f0eba4 100644 --- a/packages/cli/src/controllers/users.controller.ts +++ b/packages/cli/src/controllers/users.controller.ts @@ -1,5 +1,4 @@ -import type { FindManyOptions } from 'typeorm'; -import { In, Not } from 'typeorm'; +import { In } from 'typeorm'; import { User } from '@db/entities/User'; import { SharedCredentials } from '@db/entities/SharedCredentials'; import { SharedWorkflow } from '@db/entities/SharedWorkflow'; @@ -21,6 +20,7 @@ import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { License } from '@/License'; import { ExternalHooks } from '@/ExternalHooks'; import { InternalHooks } from '@/InternalHooks'; +import { UserRepository } from '@/databases/repositories/user.repository'; @Authorized() @RestController('/users') @@ -35,6 +35,7 @@ export class UsersController { private readonly roleService: RoleService, private readonly userService: UserService, private readonly license: License, + private readonly userRepository: UserRepository, ) {} static ERROR_MESSAGES = { @@ -49,44 +50,6 @@ export class UsersController { }, } as const; - private async toFindManyOptions(listQueryOptions?: ListQuery.Options) { - const findManyOptions: FindManyOptions = {}; - - if (!listQueryOptions) { - findManyOptions.relations = ['globalRole', 'authIdentities']; - return findManyOptions; - } - - const { filter, select, take, skip } = listQueryOptions; - - if (select) findManyOptions.select = select; - if (take) findManyOptions.take = take; - if (skip) findManyOptions.skip = skip; - - if (take && !select) { - findManyOptions.relations = ['globalRole', 'authIdentities']; - } - - if (take && select && !select?.id) { - findManyOptions.select = { ...findManyOptions.select, id: true }; // pagination requires id - } - - if (filter) { - const { isOwner, ...otherFilters } = filter; - - findManyOptions.where = otherFilters; - - if (isOwner !== undefined) { - const ownerRole = await this.roleService.findGlobalOwnerRole(); - - findManyOptions.relations = ['globalRole']; - findManyOptions.where.globalRole = { id: isOwner ? ownerRole.id : Not(ownerRole.id) }; - } - } - - return findManyOptions; - } - private removeSupplementaryFields( publicUsers: Array>, listQueryOptions: ListQuery.Options, @@ -122,9 +85,14 @@ export class UsersController { async listUsers(req: ListQuery.Request) { const { listQueryOptions } = req; - const findManyOptions = await this.toFindManyOptions(listQueryOptions); + const globalOwner = await this.roleService.findGlobalOwnerRole(); - const users = await this.userService.findMany(findManyOptions); + const findManyOptions = await this.userRepository.toFindManyOptions( + listQueryOptions, + globalOwner.id, + ); + + const users = await this.userRepository.find(findManyOptions); const publicUsers: Array> = await Promise.all( users.map(async (u) => @@ -140,8 +108,9 @@ export class UsersController { @Get('/:id/password-reset-link') @RequireGlobalScope('user:resetPassword') async getUserPasswordResetLink(req: UserRequest.PasswordResetLink) { - const user = await this.userService.findOneOrFail({ + const user = await this.userRepository.findOneOrFail({ where: { id: req.params.id }, + relations: ['globalRole'], }); if (!user) { throw new NotFoundError('User not found'); @@ -160,9 +129,10 @@ export class UsersController { await this.userService.updateSettings(id, payload); - const user = await this.userService.findOneOrFail({ + const user = await this.userRepository.findOneOrFail({ select: ['settings'], where: { id }, + relations: ['globalRole'], }); return user.settings; @@ -192,10 +162,9 @@ export class UsersController { ); } - const users = await this.userService.findMany({ - where: { id: In([transferId, idToDelete]) }, - relations: ['globalRole'], - }); + const userIds = transferId ? [transferId, idToDelete] : [idToDelete]; + + const users = await this.userRepository.findManybyIds(userIds); if (!users.length || (transferId && users.length !== 2)) { throw new NotFoundError( @@ -354,8 +323,9 @@ export class UsersController { throw new UnauthorizedError(NO_USER_TO_OWNER); } - const targetUser = await this.userService.findOne({ + const targetUser = await this.userRepository.findOne({ where: { id: req.params.id }, + relations: ['globalRole'], }); if (targetUser === null) { diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index e9f016c75e..88f824b147 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -2,11 +2,11 @@ import type { EntityManager, FindOptionsWhere } from 'typeorm'; import { CredentialsEntity } from '@db/entities/CredentialsEntity'; import type { SharedCredentials } from '@db/entities/SharedCredentials'; import type { User } from '@db/entities/User'; -import { UserService } from '@/services/user.service'; import { CredentialsService, type CredentialsGetSharedOptions } from './credentials.service'; import { RoleService } from '@/services/role.service'; import Container from 'typedi'; import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository'; +import { UserRepository } from '@/databases/repositories/user.repository'; export class EECredentialsService extends CredentialsService { static async isOwned( @@ -66,7 +66,7 @@ export class EECredentialsService extends CredentialsService { credential: CredentialsEntity, shareWithIds: string[], ): Promise { - const users = await Container.get(UserService).getByIds(transaction, shareWithIds); + const users = await Container.get(UserRepository).getByIds(transaction, shareWithIds); const role = await Container.get(RoleService).findCredentialUserRole(); const newSharedCredentials = users diff --git a/packages/cli/src/databases/repositories/sharedCredentials.repository.ts b/packages/cli/src/databases/repositories/sharedCredentials.repository.ts index 660c569136..92918f237a 100644 --- a/packages/cli/src/databases/repositories/sharedCredentials.repository.ts +++ b/packages/cli/src/databases/repositories/sharedCredentials.repository.ts @@ -1,4 +1,5 @@ import { Service } from 'typedi'; +import type { FindOptionsWhere } from 'typeorm'; import { DataSource, In, Not, Repository } from 'typeorm'; import { SharedCredentials } from '../entities/SharedCredentials'; import type { User } from '../entities/User'; @@ -50,4 +51,13 @@ export class SharedCredentialsRepository extends Repository { return sharings.map((s) => s.credentialsId); } + + async findSharings(userIds: string[], roleId?: string) { + const where: FindOptionsWhere = { userId: In(userIds) }; + + // If credential sharing is not enabled, get only credentials owned by this user + if (roleId) where.roleId = roleId; + + return this.find({ where }); + } } diff --git a/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts b/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts index d371e2995a..2229b6b84d 100644 --- a/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts +++ b/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts @@ -1,9 +1,11 @@ import { Service } from 'typedi'; -import { DataSource, type FindOptionsWhere, Repository, In, Not } from 'typeorm'; +import { DataSource, Repository, In, Not } from 'typeorm'; +import type { EntityManager, FindOptionsWhere } from 'typeorm'; import { SharedWorkflow } from '../entities/SharedWorkflow'; import { type User } from '../entities/User'; import type { Scope } from '@n8n/permissions'; import type { Role } from '../entities/Role'; +import type { WorkflowEntity } from '../entities/WorkflowEntity'; @Service() export class SharedWorkflowRepository extends Repository { @@ -72,4 +74,55 @@ export class SharedWorkflowRepository extends Repository { async makeOwnerOfAllWorkflows(user: User, role: Role) { return this.update({ userId: Not(user.id), roleId: role.id }, { user }); } + + async getSharing( + user: User, + workflowId: string, + options: { allowGlobalScope: true; globalScope: Scope } | { allowGlobalScope: false }, + relations: string[] = ['workflow'], + ): Promise { + const where: FindOptionsWhere = { workflowId }; + + // Omit user from where if the requesting user has relevant + // global workflow permissions. This allows the user to + // access workflows they don't own. + if (!options.allowGlobalScope || !user.hasGlobalScope(options.globalScope)) { + where.userId = user.id; + } + + return this.findOne({ where, relations }); + } + + async getSharedWorkflows( + user: User, + options: { + relations?: string[]; + workflowIds?: string[]; + }, + ): Promise { + return this.find({ + where: { + ...(!['owner', 'admin'].includes(user.globalRole.name) && { userId: user.id }), + ...(options.workflowIds && { workflowId: In(options.workflowIds) }), + }, + ...(options.relations && { relations: options.relations }), + }); + } + + async share(transaction: EntityManager, workflow: WorkflowEntity, users: User[], roleId: string) { + const newSharedWorkflows = users.reduce((acc, user) => { + if (user.isPending) { + return acc; + } + const entity: Partial = { + workflowId: workflow.id, + userId: user.id, + roleId, + }; + acc.push(this.create(entity)); + return acc; + }, []); + + return transaction.save(newSharedWorkflows); + } } diff --git a/packages/cli/src/databases/repositories/tag.repository.ts b/packages/cli/src/databases/repositories/tag.repository.ts index dbd763a02a..285a4de096 100644 --- a/packages/cli/src/databases/repositories/tag.repository.ts +++ b/packages/cli/src/databases/repositories/tag.repository.ts @@ -1,6 +1,9 @@ import { Service } from 'typedi'; +import type { EntityManager } from 'typeorm'; import { DataSource, In, Repository } from 'typeorm'; import { TagEntity } from '../entities/TagEntity'; +import type { WorkflowEntity } from '../entities/WorkflowEntity'; +import intersection from 'lodash/intersection'; @Service() export class TagRepository extends Repository { @@ -14,4 +17,57 @@ export class TagRepository extends Repository { where: { id: In(tagIds) }, }); } + + /** + * Set tags on workflow to import while ensuring all tags exist in the database, + * either by matching incoming to existing tags or by creating them first. + */ + async setTags(tx: EntityManager, dbTags: TagEntity[], workflow: WorkflowEntity) { + if (!workflow?.tags?.length) return; + + for (let i = 0; i < workflow.tags.length; i++) { + const importTag = workflow.tags[i]; + + if (!importTag.name) continue; + + const identicalMatch = dbTags.find( + (dbTag) => + dbTag.id === importTag.id && + dbTag.createdAt && + importTag.createdAt && + dbTag.createdAt.getTime() === new Date(importTag.createdAt).getTime(), + ); + + if (identicalMatch) { + workflow.tags[i] = identicalMatch; + continue; + } + + const nameMatch = dbTags.find((dbTag) => dbTag.name === importTag.name); + + if (nameMatch) { + workflow.tags[i] = nameMatch; + continue; + } + + const tagEntity = this.create(importTag); + + workflow.tags[i] = await tx.save(tagEntity); + } + } + + /** + * Returns the workflow IDs that have certain tags. + * Intersection! e.g. workflow needs to have all provided tags. + */ + async getWorkflowIdsViaTags(tags: string[]): Promise { + const dbTags = await this.find({ + where: { name: In(tags) }, + relations: ['workflows'], + }); + + const workflowIdsPerTag = dbTags.map((tag) => tag.workflows.map((workflow) => workflow.id)); + + return intersection(...workflowIdsPerTag); + } } diff --git a/packages/cli/src/databases/repositories/user.repository.ts b/packages/cli/src/databases/repositories/user.repository.ts index cb6543e5c8..e6be912579 100644 --- a/packages/cli/src/databases/repositories/user.repository.ts +++ b/packages/cli/src/databases/repositories/user.repository.ts @@ -1,6 +1,8 @@ import { Service } from 'typedi'; -import { DataSource, In, Not, Repository } from 'typeorm'; +import type { EntityManager, FindManyOptions } from 'typeorm'; +import { DataSource, In, IsNull, Not, Repository } from 'typeorm'; import { User } from '../entities/User'; +import type { ListQuery } from '@/requests'; @Service() export class UserRepository extends Repository { @@ -18,4 +20,68 @@ export class UserRepository extends Repository { async deleteAllExcept(user: User) { await this.delete({ id: Not(user.id) }); } + + async getByIds(transaction: EntityManager, ids: string[]) { + return transaction.find(User, { where: { id: In(ids) } }); + } + + async findManyByEmail(emails: string[]) { + return this.find({ + where: { email: In(emails) }, + relations: ['globalRole'], + select: ['email', 'password', 'id'], + }); + } + + async deleteMany(userIds: string[]) { + return this.delete({ id: In(userIds) }); + } + + async findNonShellUser(email: string) { + return this.findOne({ + where: { + email, + password: Not(IsNull()), + }, + relations: ['authIdentities', 'globalRole'], + }); + } + + async toFindManyOptions(listQueryOptions?: ListQuery.Options, globalOwnerRoleId?: string) { + const findManyOptions: FindManyOptions = {}; + + if (!listQueryOptions) { + findManyOptions.relations = ['globalRole', 'authIdentities']; + return findManyOptions; + } + + const { filter, select, take, skip } = listQueryOptions; + + if (select) findManyOptions.select = select; + if (take) findManyOptions.take = take; + if (skip) findManyOptions.skip = skip; + + if (take && !select) { + findManyOptions.relations = ['globalRole', 'authIdentities']; + } + + if (take && select && !select?.id) { + findManyOptions.select = { ...findManyOptions.select, id: true }; // pagination requires id + } + + if (filter) { + const { isOwner, ...otherFilters } = filter; + + findManyOptions.where = otherFilters; + + if (isOwner !== undefined && globalOwnerRoleId) { + findManyOptions.relations = ['globalRole']; + findManyOptions.where.globalRole = { + id: isOwner ? globalOwnerRoleId : Not(globalOwnerRoleId), + }; + } + } + + return findManyOptions; + } } diff --git a/packages/cli/src/databases/repositories/workflow.repository.ts b/packages/cli/src/databases/repositories/workflow.repository.ts index 49e9169c32..5410967779 100644 --- a/packages/cli/src/databases/repositories/workflow.repository.ts +++ b/packages/cli/src/databases/repositories/workflow.repository.ts @@ -198,4 +198,16 @@ export class WorkflowRepository extends Repository { .innerJoin(WebhookEntity, 'webhook_entity', 'workflow.id = webhook_entity.workflowId') .execute() as Promise>; } + + async updateActiveState(workflowId: string, newState: boolean) { + return this.update({ id: workflowId }, { active: newState }); + } + + async deactivateAll() { + return this.update({ active: true }, { active: false }); + } + + async findByActiveState(activeState: boolean) { + return this.findBy({ active: activeState }); + } } diff --git a/packages/cli/src/environments/variables/variables.service.ee.ts b/packages/cli/src/environments/variables/variables.service.ee.ts index 96060e7a4a..282aff4c9e 100644 --- a/packages/cli/src/environments/variables/variables.service.ee.ts +++ b/packages/cli/src/environments/variables/variables.service.ee.ts @@ -5,7 +5,6 @@ import { generateNanoId } from '@db/utils/generators'; import { canCreateNewVariable } from './environmentHelpers'; import { CacheService } from '@/services/cache.service'; import { VariablesRepository } from '@db/repositories/variables.repository'; -import type { DeepPartial } from 'typeorm'; import { VariableCountLimitReachedError } from '@/errors/variable-count-limit-reached.error'; import { VariableValidationError } from '@/errors/variable-validation.error'; @@ -23,9 +22,7 @@ export class VariablesService { return Container.get(VariablesService).findAll(); }, }); - return (variables as Array>).map((v) => - this.variablesRepository.create(v), - ); + return (variables as Array>).map((v) => this.variablesRepository.create(v)); } async getCount(): Promise { @@ -38,7 +35,7 @@ export class VariablesService { if (!foundVariable) { return null; } - return this.variablesRepository.create(foundVariable as DeepPartial); + return this.variablesRepository.create(foundVariable as Partial); } async delete(id: string): Promise { diff --git a/packages/cli/src/eventbus/MessageEventBusDestination/MessageEventBusDestination.ee.ts b/packages/cli/src/eventbus/MessageEventBusDestination/MessageEventBusDestination.ee.ts index 33384e20d1..28736873cc 100644 --- a/packages/cli/src/eventbus/MessageEventBusDestination/MessageEventBusDestination.ee.ts +++ b/packages/cli/src/eventbus/MessageEventBusDestination/MessageEventBusDestination.ee.ts @@ -1,6 +1,5 @@ import { v4 as uuid } from 'uuid'; import { Container } from 'typedi'; -import type { DeleteResult, InsertResult } from 'typeorm'; import type { INodeCredentials, MessageEventBusDestinationOptions } from 'n8n-workflow'; import { MessageEventBusDestinationTypeNames } from 'n8n-workflow'; import { Logger } from '@/Logger'; @@ -92,7 +91,7 @@ export abstract class MessageEventBusDestination implements MessageEventBusDesti id: this.getId(), destination: this.serialize(), }; - const dbResult: InsertResult = await Container.get(EventDestinationsRepository).upsert(data, { + const dbResult = await Container.get(EventDestinationsRepository).upsert(data, { skipUpdateIfNoValuesChanged: true, conflictPaths: ['id'], }); @@ -103,7 +102,7 @@ export abstract class MessageEventBusDestination implements MessageEventBusDesti return MessageEventBusDestination.deleteFromDb(this.getId()); } - static async deleteFromDb(id: string): Promise { + static async deleteFromDb(id: string) { const dbResult = await Container.get(EventDestinationsRepository).delete({ id }); return dbResult; } diff --git a/packages/cli/src/eventbus/eventBus.controller.ee.ts b/packages/cli/src/eventbus/eventBus.controller.ee.ts index f60a70d534..32346dd27f 100644 --- a/packages/cli/src/eventbus/eventBus.controller.ee.ts +++ b/packages/cli/src/eventbus/eventBus.controller.ee.ts @@ -16,7 +16,6 @@ import type { import { MessageEventBusDestinationTypeNames } from 'n8n-workflow'; import { RestController, Get, Post, Delete, Authorized, RequireGlobalScope } from '@/decorators'; import type { MessageEventBusDestination } from './MessageEventBusDestination/MessageEventBusDestination.ee'; -import type { DeleteResult } from 'typeorm'; import { AuthenticatedRequest } from '@/requests'; import { logStreamingLicensedMiddleware } from './middleware/logStreamingEnabled.middleware.ee'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; @@ -123,7 +122,7 @@ export class EventBusControllerEE { @Delete('/destination', { middlewares: [logStreamingLicensedMiddleware] }) @RequireGlobalScope('eventBusDestination:delete') - async deleteDestination(req: AuthenticatedRequest): Promise { + async deleteDestination(req: AuthenticatedRequest) { if (isWithIdString(req.query)) { return eventBus.removeDestination(req.query.id); } else { diff --git a/packages/cli/src/services/import.service.ts b/packages/cli/src/services/import.service.ts index 9ba0715b77..df9050e82a 100644 --- a/packages/cli/src/services/import.service.ts +++ b/packages/cli/src/services/import.service.ts @@ -1,7 +1,6 @@ import { Service } from 'typedi'; import { v4 as uuid } from 'uuid'; import { type INode, type INodeCredentialsDetails } from 'n8n-workflow'; -import type { EntityManager } from 'typeorm'; import { Logger } from '@/Logger'; import * as Db from '@/Db'; @@ -72,7 +71,7 @@ export class ImportService { if (!workflow.tags?.length) continue; - await this.setTags(tx, workflow); + await this.tagRepository.setTags(tx, this.dbTags, workflow); for (const tag of workflow.tags) { await tx.upsert(WorkflowTagMapping, { tagId: tag.id, workflowId }, [ @@ -112,42 +111,4 @@ export class ImportService { node.credentials[type] = nodeCredential; } } - - /** - * Set tags on workflow to import while ensuring all tags exist in the database, - * either by matching incoming to existing tags or by creating them first. - */ - private async setTags(tx: EntityManager, workflow: WorkflowEntity) { - if (!workflow?.tags?.length) return; - - for (let i = 0; i < workflow.tags.length; i++) { - const importTag = workflow.tags[i]; - - if (!importTag.name) continue; - - const identicalMatch = this.dbTags.find( - (dbTag) => - dbTag.id === importTag.id && - dbTag.createdAt && - importTag.createdAt && - dbTag.createdAt.getTime() === new Date(importTag.createdAt).getTime(), - ); - - if (identicalMatch) { - workflow.tags[i] = identicalMatch; - continue; - } - - const nameMatch = this.dbTags.find((dbTag) => dbTag.name === importTag.name); - - if (nameMatch) { - workflow.tags[i] = nameMatch; - continue; - } - - const tagEntity = this.tagRepository.create(importTag); - - workflow.tags[i] = await tx.save(tagEntity); - } - } } diff --git a/packages/cli/src/services/role.service.ts b/packages/cli/src/services/role.service.ts index 5b81b8b99f..4149a7a88d 100644 --- a/packages/cli/src/services/role.service.ts +++ b/packages/cli/src/services/role.service.ts @@ -4,6 +4,7 @@ import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.reposi import { CacheService } from './cache.service'; import type { RoleNames, RoleScopes } from '@db/entities/Role'; import { InvalidRoleError } from '@/errors/invalid-role.error'; +import { isSharingEnabled } from '@/UserManagement/UserManagementHelper'; @Service() export class RoleService { @@ -100,4 +101,8 @@ export class RoleService { }) .then((shared) => shared?.role); } + + async findCredentialOwnerRoleId() { + return isSharingEnabled() ? undefined : (await this.findCredentialOwnerRole()).id; + } } diff --git a/packages/cli/src/services/tag.service.ts b/packages/cli/src/services/tag.service.ts index 63050ea149..0de35883af 100644 --- a/packages/cli/src/services/tag.service.ts +++ b/packages/cli/src/services/tag.service.ts @@ -3,8 +3,6 @@ import { Service } from 'typedi'; import { validateEntity } from '@/GenericHelpers'; import type { ITagWithCountDb } from '@/Interfaces'; import type { TagEntity } from '@db/entities/TagEntity'; -import type { FindManyOptions, FindOneOptions } from 'typeorm'; -import type { UpsertOptions } from 'typeorm/repository/UpsertOptions'; import { ExternalHooks } from '@/ExternalHooks'; type GetAllResult = T extends { withUsageCount: true } ? ITagWithCountDb[] : TagEntity[]; @@ -46,18 +44,6 @@ export class TagService { return deleteResult; } - async findOne(options: FindOneOptions) { - return this.tagRepository.findOne(options); - } - - async findMany(options: FindManyOptions) { - return this.tagRepository.find(options); - } - - async upsert(tag: TagEntity, options: UpsertOptions) { - return this.tagRepository.upsert(tag, options); - } - async getAll(options?: T): Promise> { if (options?.withUsageCount) { const allTags = await this.tagRepository.find({ diff --git a/packages/cli/src/services/user.service.ts b/packages/cli/src/services/user.service.ts index cae8c40c3c..b429e992d2 100644 --- a/packages/cli/src/services/user.service.ts +++ b/packages/cli/src/services/user.service.ts @@ -1,6 +1,4 @@ import { Container, Service } from 'typedi'; -import type { EntityManager, FindManyOptions, FindOneOptions, FindOptionsWhere } from 'typeorm'; -import { In } from 'typeorm'; import { User } from '@db/entities/User'; import type { IUserSettings } from 'n8n-workflow'; import { UserRepository } from '@db/repositories/user.repository'; @@ -29,38 +27,10 @@ export class UserService { private readonly urlService: UrlService, ) {} - async findOne(options: FindOneOptions) { - return this.userRepository.findOne({ relations: ['globalRole'], ...options }); - } - - async findOneOrFail(options: FindOneOptions) { - return this.userRepository.findOneOrFail({ relations: ['globalRole'], ...options }); - } - - async findMany(options: FindManyOptions) { - return this.userRepository.find(options); - } - - async findOneBy(options: FindOptionsWhere) { - return this.userRepository.findOneBy(options); - } - - create(data: Partial) { - return this.userRepository.create(data); - } - - async save(user: Partial) { - return this.userRepository.save(user); - } - async update(userId: string, data: Partial) { return this.userRepository.update(userId, data); } - async getByIds(transaction: EntityManager, ids: string[]) { - return transaction.find(User, { where: { id: In(ids) } }); - } - getManager() { return this.userRepository.manager; } @@ -257,12 +227,9 @@ export class UserService { async inviteUsers(owner: User, attributes: Array<{ email: string; role: 'member' | 'admin' }>) { const memberRole = await this.roleService.findGlobalMemberRole(); const adminRole = await this.roleService.findGlobalAdminRole(); + const emails = attributes.map(({ email }) => email); - const existingUsers = await this.findMany({ - where: { email: In(attributes.map(({ email }) => email)) }, - relations: ['globalRole'], - select: ['email', 'password', 'id'], - }); + const existingUsers = await this.userRepository.findManyByEmail(emails); const existUsersEmails = existingUsers.map((user) => user.email); diff --git a/packages/cli/src/services/webhook.service.ts b/packages/cli/src/services/webhook.service.ts index 741c6b42ad..d42ecca3fc 100644 --- a/packages/cli/src/services/webhook.service.ts +++ b/packages/cli/src/services/webhook.service.ts @@ -3,7 +3,6 @@ import { Service } from 'typedi'; import { CacheService } from './cache.service'; import type { WebhookEntity } from '@db/entities/WebhookEntity'; import type { IHttpRequestMethods } from 'n8n-workflow'; -import type { DeepPartial } from 'typeorm'; type Method = NonNullable; @@ -97,7 +96,7 @@ export class WebhookService { return this.webhookRepository.insert(webhook); } - createWebhook(data: DeepPartial) { + createWebhook(data: Partial) { return this.webhookRepository.create(data); } diff --git a/packages/cli/src/workflows/workflow.service.ee.ts b/packages/cli/src/workflows/workflow.service.ee.ts index 8ce3a39da5..7a86c7973a 100644 --- a/packages/cli/src/workflows/workflow.service.ee.ts +++ b/packages/cli/src/workflows/workflow.service.ee.ts @@ -1,17 +1,12 @@ -import type { EntityManager } from 'typeorm'; import * as WorkflowHelpers from '@/WorkflowHelpers'; -import type { SharedWorkflow } from '@db/entities/SharedWorkflow'; import type { User } from '@db/entities/User'; import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; -import { UserService } from '@/services/user.service'; -import { WorkflowService } from './workflow.service'; import type { CredentialUsedByWorkflow, WorkflowWithSharingsAndCredentials, } from './workflows.types'; import { CredentialsService } from '@/credentials/credentials.service'; import { ApplicationError, NodeOperationError } from 'n8n-workflow'; -import { RoleService } from '@/services/role.service'; import { Service } from 'typedi'; import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; @@ -19,23 +14,25 @@ import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; +import { RoleService } from '@/services/role.service'; +import type { EntityManager } from 'typeorm'; +import { UserRepository } from '@/databases/repositories/user.repository'; @Service() export class EnterpriseWorkflowService { constructor( - private readonly workflowService: WorkflowService, - private readonly userService: UserService, - private readonly roleService: RoleService, private readonly sharedWorkflowRepository: SharedWorkflowRepository, private readonly workflowRepository: WorkflowRepository, private readonly credentialsRepository: CredentialsRepository, + private readonly userRepository: UserRepository, + private readonly roleService: RoleService, ) {} async isOwned( user: User, workflowId: string, ): Promise<{ ownsWorkflow: boolean; workflow?: WorkflowEntity }> { - const sharing = await this.workflowService.getSharing( + const sharing = await this.sharedWorkflowRepository.getSharing( user, workflowId, { allowGlobalScope: false }, @@ -49,28 +46,11 @@ export class EnterpriseWorkflowService { return { ownsWorkflow: true, workflow }; } - async share( - transaction: EntityManager, - workflow: WorkflowEntity, - shareWithIds: string[], - ): Promise { - const users = await this.userService.getByIds(transaction, shareWithIds); + async share(transaction: EntityManager, workflow: WorkflowEntity, shareWithIds: string[]) { + const users = await this.userRepository.getByIds(transaction, shareWithIds); const role = await this.roleService.findWorkflowEditorRole(); - const newSharedWorkflows = users.reduce((acc, user) => { - if (user.isPending) { - return acc; - } - const entity: Partial = { - workflowId: workflow.id, - userId: user.id, - roleId: role?.id, - }; - acc.push(this.sharedWorkflowRepository.create(entity)); - return acc; - }, []); - - return transaction.save(newSharedWorkflows); + await this.sharedWorkflowRepository.share(transaction, workflow, users, role.id); } addOwnerAndSharings(workflow: WorkflowWithSharingsAndCredentials): void { diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index 7844316839..ed3d00f102 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -1,14 +1,12 @@ import Container, { Service } from 'typedi'; import type { INode, IPinData } from 'n8n-workflow'; import { NodeApiError, Workflow } from 'n8n-workflow'; -import type { FindOptionsWhere } from 'typeorm'; import pick from 'lodash/pick'; import omit from 'lodash/omit'; import { v4 as uuid } from 'uuid'; import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; import * as WorkflowHelpers from '@/WorkflowHelpers'; import config from '@/config'; -import type { SharedWorkflow } from '@db/entities/SharedWorkflow'; import type { User } from '@db/entities/User'; import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { validateEntity } from '@/GenericHelpers'; @@ -25,7 +23,6 @@ import { WorkflowRepository } from '@db/repositories/workflow.repository'; import { OwnershipService } from '@/services/ownership.service'; import { WorkflowHistoryService } from './workflowHistory/workflowHistory.service.ee'; import { BinaryDataService } from 'n8n-core'; -import type { Scope } from '@n8n/permissions'; import { Logger } from '@/Logger'; import { MultiMainSetup } from '@/services/orchestration/main/MultiMainSetup.ee'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; @@ -34,10 +31,6 @@ import { ExecutionRepository } from '@db/repositories/execution.repository'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; -export type WorkflowsGetSharedOptions = - | { allowGlobalScope: true; globalScope: Scope } - | { allowGlobalScope: false }; - @Service() export class WorkflowService { constructor( @@ -57,24 +50,6 @@ export class WorkflowService { private readonly activeWorkflowRunner: ActiveWorkflowRunner, ) {} - async getSharing( - user: User, - workflowId: string, - options: WorkflowsGetSharedOptions, - relations: string[] = ['workflow'], - ): Promise { - const where: FindOptionsWhere = { workflowId }; - - // Omit user from where if the requesting user has relevant - // global workflow permissions. This allows the user to - // access workflows they don't own. - if (!options.allowGlobalScope || !user.hasGlobalScope(options.globalScope)) { - where.userId = user.id; - } - - return this.sharedWorkflowRepository.findOne({ where, relations }); - } - /** * Find the pinned trigger to execute the workflow from, if any. * diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 1443387b03..9e7ee4cb5d 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -386,10 +386,14 @@ workflowsController.put( workflow = undefined; // Allow owners/admins to share if (req.user.hasGlobalScope('workflow:share')) { - const sharedRes = await Container.get(WorkflowService).getSharing(req.user, workflowId, { - allowGlobalScope: true, - globalScope: 'workflow:share', - }); + const sharedRes = await Container.get(SharedWorkflowRepository).getSharing( + req.user, + workflowId, + { + allowGlobalScope: true, + globalScope: 'workflow:share', + }, + ); workflow = sharedRes?.workflow; } if (!workflow) { diff --git a/packages/cli/test/unit/collaboration/collaboration.service.test.ts b/packages/cli/test/unit/collaboration/collaboration.service.test.ts index 3478d628b9..bb9041b3b6 100644 --- a/packages/cli/test/unit/collaboration/collaboration.service.test.ts +++ b/packages/cli/test/unit/collaboration/collaboration.service.test.ts @@ -8,30 +8,31 @@ import type { WorkflowClosedMessage, WorkflowOpenedMessage, } from '@/collaboration/collaboration.message'; +import type { UserRepository } from '@/databases/repositories/user.repository'; +import { mock } from 'jest-mock-extended'; describe('CollaborationService', () => { let collaborationService: CollaborationService; let mockLogger: Logger; let mockUserService: jest.Mocked; + let mockUserRepository: jest.Mocked; let state: CollaborationState; let push: Push; beforeEach(() => { - mockLogger = { - warn: jest.fn(), - error: jest.fn(), - } as unknown as jest.Mocked; - mockUserService = { - getByIds: jest.fn(), - getManager: jest.fn(), - } as unknown as jest.Mocked; - - push = { - on: jest.fn(), - sendToUsers: jest.fn(), - } as unknown as Push; + mockLogger = mock(); + mockUserService = mock(); + mockUserRepository = mock(); + push = mock(); state = new CollaborationState(); - collaborationService = new CollaborationService(mockLogger, push, state, mockUserService); + + collaborationService = new CollaborationService( + mockLogger, + push, + state, + mockUserService, + mockUserRepository, + ); }); describe('workflow opened message', () => { @@ -61,7 +62,7 @@ describe('CollaborationService', () => { describe('user is not yet active', () => { it('updates state correctly', async () => { - mockUserService.getByIds.mockResolvedValueOnce([{ id: userId } as User]); + mockUserRepository.getByIds.mockResolvedValueOnce([{ id: userId } as User]); await collaborationService.handleUserMessage(userId, message); expect(state.getActiveWorkflowUsers(workflowId)).toEqual([ @@ -73,7 +74,7 @@ describe('CollaborationService', () => { }); it('sends active workflow users changed message', async () => { - mockUserService.getByIds.mockResolvedValueOnce([{ id: userId } as User]); + mockUserRepository.getByIds.mockResolvedValueOnce([{ id: userId } as User]); await collaborationService.handleUserMessage(userId, message); expectActiveUsersChangedMessage([userId]); @@ -86,7 +87,7 @@ describe('CollaborationService', () => { }); it('updates state correctly', async () => { - mockUserService.getByIds.mockResolvedValueOnce([{ id: userId } as User]); + mockUserRepository.getByIds.mockResolvedValueOnce([{ id: userId } as User]); await collaborationService.handleUserMessage(userId, message); expect(state.getActiveWorkflowUsers(workflowId)).toEqual([ @@ -98,7 +99,7 @@ describe('CollaborationService', () => { }); it('sends active workflow users changed message', async () => { - mockUserService.getByIds.mockResolvedValueOnce([{ id: userId } as User]); + mockUserRepository.getByIds.mockResolvedValueOnce([{ id: userId } as User]); await collaborationService.handleUserMessage(userId, message); expectActiveUsersChangedMessage([userId]); diff --git a/packages/cli/test/unit/controllers/me.controller.test.ts b/packages/cli/test/unit/controllers/me.controller.test.ts index 9982b91963..aaaea5a86a 100644 --- a/packages/cli/test/unit/controllers/me.controller.test.ts +++ b/packages/cli/test/unit/controllers/me.controller.test.ts @@ -14,11 +14,13 @@ import { License } from '@/License'; import { badPasswords } from '../shared/testData'; import { mockInstance } from '../../shared/mocking'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { UserRepository } from '@/databases/repositories/user.repository'; describe('MeController', () => { const externalHooks = mockInstance(ExternalHooks); const internalHooks = mockInstance(InternalHooks); const userService = mockInstance(UserService); + const userRepository = mockInstance(UserRepository); mockInstance(License).isWithinUsersLimit.mockReturnValue(true); const controller = Container.get(MeController); @@ -47,7 +49,7 @@ describe('MeController', () => { const reqBody = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' }; const req = mock({ user, body: reqBody }); const res = mock(); - userService.findOneOrFail.mockResolvedValue(user); + userRepository.findOneOrFail.mockResolvedValue(user); jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); userService.toPublic.mockResolvedValue({} as unknown as PublicUser); @@ -82,7 +84,7 @@ describe('MeController', () => { const reqBody = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' }; const req = mock({ user, body: reqBody }); const res = mock(); - userService.findOneOrFail.mockResolvedValue(user); + userRepository.findOneOrFail.mockResolvedValue(user); jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); // Add invalid data to the request payload @@ -166,7 +168,7 @@ describe('MeController', () => { body: { currentPassword: 'old_password', newPassword: 'NewPassword123' }, }); const res = mock(); - userService.save.calledWith(req.user).mockResolvedValue(req.user); + userRepository.save.calledWith(req.user).mockResolvedValue(req.user); jest.spyOn(jwt, 'sign').mockImplementation(() => 'new-signed-token'); await controller.updatePassword(req, res); diff --git a/packages/cli/test/unit/controllers/owner.controller.test.ts b/packages/cli/test/unit/controllers/owner.controller.test.ts index 3ba61147dc..90c6e02eb6 100644 --- a/packages/cli/test/unit/controllers/owner.controller.test.ts +++ b/packages/cli/test/unit/controllers/owner.controller.test.ts @@ -16,11 +16,13 @@ import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { PasswordUtility } from '@/services/password.utility'; import Container from 'typedi'; import type { InternalHooks } from '@/InternalHooks'; +import { UserRepository } from '@/databases/repositories/user.repository'; describe('OwnerController', () => { const configGetSpy = jest.spyOn(config, 'getEnv'); const internalHooks = mock(); const userService = mockInstance(UserService); + const userRepository = mockInstance(UserRepository); const settingsRepository = mock(); mockInstance(License).isWithinUsersLimit.mockReturnValue(true); const controller = new OwnerController( @@ -30,6 +32,7 @@ describe('OwnerController', () => { userService, Container.get(PasswordUtility), mock(), + userRepository, ); describe('setupOwner', () => { @@ -87,12 +90,12 @@ describe('OwnerController', () => { }); const res = mock(); configGetSpy.mockReturnValue(false); - userService.save.calledWith(anyObject()).mockResolvedValue(user); + userRepository.save.calledWith(anyObject()).mockResolvedValue(user); jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); await controller.setupOwner(req, res); - expect(userService.save).toHaveBeenCalledWith(user); + expect(userRepository.save).toHaveBeenCalledWith(user); const cookieOptions = captor(); expect(res.cookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME, 'signed-token', cookieOptions);