diff --git a/packages/cli/src/middlewares/listQuery/dtos/pagination.dto.ts b/packages/cli/src/middlewares/listQuery/dtos/pagination.dto.ts new file mode 100644 index 000000000..a7750aee6 --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/dtos/pagination.dto.ts @@ -0,0 +1,22 @@ +import { isIntegerString } from '@/utils'; + +export class Pagination { + static fromString(rawTake: string, rawSkip: string) { + if (!isIntegerString(rawTake)) { + throw new Error('Parameter take is not an integer string'); + } + + if (!isIntegerString(rawSkip)) { + throw new Error('Parameter skip is not an integer string'); + } + + const [take, skip] = [rawTake, rawSkip].map((o) => parseInt(o, 10)); + + const MAX_ITEMS_PER_PAGE = 50; + + return { + take: Math.min(take, MAX_ITEMS_PER_PAGE), + skip, + }; + } +} diff --git a/packages/cli/src/middlewares/listQuery/dtos/workflow.filter.dto.ts b/packages/cli/src/middlewares/listQuery/dtos/workflow.filter.dto.ts new file mode 100644 index 000000000..4eb9da41e --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/dtos/workflow.filter.dto.ts @@ -0,0 +1,43 @@ +import { IsOptional, IsString, IsBoolean, IsArray, validate } from 'class-validator'; +import { Expose, instanceToPlain, plainToInstance } from 'class-transformer'; +import { jsonParse } from 'n8n-workflow'; +import { isObjectLiteral } from '@/utils'; + +export class WorkflowFilter { + @IsString() + @IsOptional() + @Expose() + name?: string; + + @IsBoolean() + @IsOptional() + @Expose() + active?: boolean; + + @IsArray() + @IsString({ each: true }) + @IsOptional() + @Expose() + tags?: string[]; + + static async fromString(rawFilter: string) { + const dto = jsonParse(rawFilter, { errorMessage: 'Failed to parse filter JSON' }); + + if (!isObjectLiteral(dto)) throw new Error('Filter must be an object literal'); + + const instance = plainToInstance(WorkflowFilter, dto, { + excludeExtraneousValues: true, // remove fields not in class + exposeUnsetFields: false, // remove in-class undefined fields + }); + + await instance.validate(); + + return instanceToPlain(instance); + } + + private async validate() { + const result = await validate(this); + + if (result.length > 0) throw new Error('Parsed filter does not fit the schema'); + } +} diff --git a/packages/cli/src/middlewares/listQuery/dtos/workflow.select.dto.ts b/packages/cli/src/middlewares/listQuery/dtos/workflow.select.dto.ts new file mode 100644 index 000000000..dd74e8126 --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/dtos/workflow.select.dto.ts @@ -0,0 +1,30 @@ +import { isStringArray } from '@/utils'; +import { jsonParse } from 'n8n-workflow'; + +export class WorkflowSelect { + fields: string[]; + + static get selectableFields() { + return new Set([ + 'id', // always included downstream + 'name', + 'active', + 'tags', + 'createdAt', + 'updatedAt', + 'versionId', + 'ownedBy', // non-entity field + ]); + } + + static fromString(rawFilter: string) { + const dto = jsonParse(rawFilter, { errorMessage: 'Failed to parse filter JSON' }); + + if (!isStringArray(dto)) throw new Error('Parsed select is not a string array'); + + return dto.reduce>((acc, field) => { + if (!WorkflowSelect.selectableFields.has(field)) return acc; + return (acc[field] = true), acc; + }, {}); + } +} diff --git a/packages/cli/src/middlewares/listQuery/error.ts b/packages/cli/src/middlewares/listQuery/error.ts deleted file mode 100644 index 12deebfab..000000000 --- a/packages/cli/src/middlewares/listQuery/error.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { BadRequestError } from '@/ResponseHelper'; -import { LoggerProxy } from 'n8n-workflow'; -import * as utils from '@/utils'; - -export function handleListQueryError( - paramName: 'filter' | 'select', - paramValue: string, - maybeError: unknown, -) { - const error = utils.toError(maybeError); - - LoggerProxy.error(`Invalid "${paramName}" query parameter`, { - paramName, - paramValue, - error, - }); - - throw new BadRequestError( - `Invalid "${paramName}" query parameter: ${paramValue}. Error: ${error.message}`, - ); -} diff --git a/packages/cli/src/middlewares/listQuery/filter.ts b/packages/cli/src/middlewares/listQuery/filter.ts index 931719e12..6ca297ef9 100644 --- a/packages/cli/src/middlewares/listQuery/filter.ts +++ b/packages/cli/src/middlewares/listQuery/filter.ts @@ -1,44 +1,38 @@ -import { jsonParse } from 'n8n-workflow'; -import { handleListQueryError } from './error'; -import { WorkflowSchema } from './workflow.schema'; -import type { ListQueryRequest } from '@/requests'; -import type { RequestHandler } from 'express'; -import type { Schema } from './schema'; +/* eslint-disable @typescript-eslint/naming-convention */ -function toQueryFilter(rawFilter: string, schema: typeof Schema) { - const parsedFilter = new schema( - jsonParse(rawFilter, { errorMessage: 'Failed to parse filter JSON' }), - ); +import * as ResponseHelper from '@/ResponseHelper'; +import { WorkflowFilter } from './dtos/workflow.filter.dto'; +import { toError } from '@/utils'; - return Object.fromEntries( - Object.entries(parsedFilter) - .filter(([_, value]) => value !== undefined) - .map(([key, _]: [keyof Schema, unknown]) => [key, parsedFilter[key]]), - ); -} +import type { NextFunction, Response } from 'express'; +import type { ListQuery } from '@/requests'; -export const filterListQueryMiddleware: RequestHandler = (req: ListQueryRequest, res, next) => { +export const filterListQueryMiddleware = async ( + req: ListQuery.Request, + res: Response, + next: NextFunction, +) => { const { filter: rawFilter } = req.query; if (!rawFilter) return next(); - let schema; + let Filter; if (req.baseUrl.endsWith('workflows')) { - schema = WorkflowSchema; + Filter = WorkflowFilter; } else { return next(); } try { - const filter = toQueryFilter(rawFilter, schema); + const filter = await Filter.fromString(rawFilter); if (Object.keys(filter).length === 0) return next(); req.listQueryOptions = { ...req.listQueryOptions, filter }; next(); - } catch (error) { - handleListQueryError('filter', rawFilter, error); + } catch (maybeError) { + ResponseHelper.sendErrorResponse(res, toError(maybeError)); } }; diff --git a/packages/cli/src/middlewares/listQuery/pagination.ts b/packages/cli/src/middlewares/listQuery/pagination.ts index cfe38e003..9a16ee3a5 100644 --- a/packages/cli/src/middlewares/listQuery/pagination.ts +++ b/packages/cli/src/middlewares/listQuery/pagination.ts @@ -1,27 +1,25 @@ -import type { ListQueryRequest } from '@/requests'; -import { isIntegerString } from '@/utils'; +import { toError } from '@/utils'; +import * as ResponseHelper from '@/ResponseHelper'; +import { Pagination } from './dtos/pagination.dto'; +import type { ListQuery } from '@/requests'; import type { RequestHandler } from 'express'; -function toPaginationOptions(rawTake: string, rawSkip: string) { - const MAX_ITEMS = 50; - - if ([rawTake, rawSkip].some((i) => !isIntegerString(i))) { - throw new Error('Parameter take or skip is not an integer string'); - } - - const [take, skip] = [rawTake, rawSkip].map((o) => parseInt(o, 10)); - - return { skip, take: Math.min(take, MAX_ITEMS) }; -} - -export const paginationListQueryMiddleware: RequestHandler = (req: ListQueryRequest, res, next) => { +export const paginationListQueryMiddleware: RequestHandler = ( + req: ListQuery.Request, + res, + next, +) => { const { take: rawTake, skip: rawSkip = '0' } = req.query; if (!rawTake) return next(); - const { take, skip } = toPaginationOptions(rawTake, rawSkip); + try { + const { take, skip } = Pagination.fromString(rawTake, rawSkip); - req.listQueryOptions = { ...req.listQueryOptions, take, skip }; + req.listQueryOptions = { ...req.listQueryOptions, skip, take }; - next(); + next(); + } catch (maybeError) { + ResponseHelper.sendErrorResponse(res, toError(maybeError)); + } }; diff --git a/packages/cli/src/middlewares/listQuery/schema.ts b/packages/cli/src/middlewares/listQuery/schema.ts deleted file mode 100644 index bfd4e84f5..000000000 --- a/packages/cli/src/middlewares/listQuery/schema.ts +++ /dev/null @@ -1,7 +0,0 @@ -export class Schema { - constructor(private data: unknown = {}) {} - - static get fieldNames(): string[] { - return []; - } -} diff --git a/packages/cli/src/middlewares/listQuery/select.ts b/packages/cli/src/middlewares/listQuery/select.ts index ff342a3be..e5813bbc0 100644 --- a/packages/cli/src/middlewares/listQuery/select.ts +++ b/packages/cli/src/middlewares/listQuery/select.ts @@ -1,46 +1,34 @@ -import { handleListQueryError } from './error'; -import { jsonParse } from 'n8n-workflow'; -import { WorkflowSchema } from './workflow.schema'; -import * as utils from '@/utils'; -import type { ListQueryRequest } from '@/requests'; +/* eslint-disable @typescript-eslint/naming-convention */ + +import { WorkflowSelect } from './dtos/workflow.select.dto'; +import * as ResponseHelper from '@/ResponseHelper'; +import { toError } from '@/utils'; + import type { RequestHandler } from 'express'; -import type { Schema } from '@/middlewares/listQuery/schema'; +import type { ListQuery } from '@/requests'; -function toQuerySelect(rawSelect: string, schema: typeof Schema) { - const asArr = jsonParse(rawSelect, { errorMessage: 'Failed to parse select JSON' }); - - if (!utils.isStringArray(asArr)) { - throw new Error('Parsed select is not a string array'); - } - - return asArr.reduce>((acc, field) => { - if (!schema.fieldNames.includes(field)) return acc; - return (acc[field] = true), acc; - }, {}); -} - -export const selectListQueryMiddleware: RequestHandler = (req: ListQueryRequest, res, next) => { +export const selectListQueryMiddleware: RequestHandler = (req: ListQuery.Request, res, next) => { const { select: rawSelect } = req.query; if (!rawSelect) return next(); - let schema; + let Select; if (req.baseUrl.endsWith('workflows')) { - schema = WorkflowSchema; + Select = WorkflowSelect; } else { return next(); } try { - const select = toQuerySelect(rawSelect, schema); + const select = Select.fromString(rawSelect); if (Object.keys(select).length === 0) return next(); req.listQueryOptions = { ...req.listQueryOptions, select }; next(); - } catch (error) { - handleListQueryError('select', rawSelect, error); + } catch (maybeError) { + ResponseHelper.sendErrorResponse(res, toError(maybeError)); } }; diff --git a/packages/cli/src/middlewares/listQuery/workflow.schema.ts b/packages/cli/src/middlewares/listQuery/workflow.schema.ts deleted file mode 100644 index cf0dc1dba..000000000 --- a/packages/cli/src/middlewares/listQuery/workflow.schema.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { Schema } from '@/middlewares/listQuery/schema'; -import { validateSync, IsOptional, IsString, IsBoolean, IsDateString } from 'class-validator'; - -export class WorkflowSchema extends Schema { - constructor(data: unknown = {}) { - super(); - Object.assign(this, data); - - // strip out unknown fields - const result = validateSync(this, { whitelist: true }); - - if (result.length > 0) { - throw new Error('Parsed filter does not fit the schema'); - } - } - - @IsOptional() - @IsString() - id?: string = undefined; - - @IsOptional() - @IsString() - name?: string = undefined; - - @IsOptional() - @IsBoolean() - active?: boolean = undefined; - - @IsOptional() - @IsDateString() - createdAt?: Date = undefined; - - @IsOptional() - @IsDateString() - updatedAt?: Date = undefined; - - static get fieldNames() { - return Object.getOwnPropertyNames(new WorkflowSchema()); - } -} diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index 8050365a5..7fc0bb94f 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -8,6 +8,7 @@ import type { INodeCredentialTestRequest, IPinData, IRunData, + IUser, IWorkflowSettings, } from 'n8n-workflow'; @@ -18,6 +19,7 @@ import type { Role } from '@db/entities/Role'; import type { User } from '@db/entities/User'; import type { UserManagementMailer } from '@/UserManagement/email'; import type { Variables } from '@db/entities/Variables'; +import type { WorkflowEntity } from './databases/entities/WorkflowEntity'; export class UserUpdatePayload implements Pick { @IsEmail() @@ -113,23 +115,51 @@ export declare namespace WorkflowRequest { // list query // ---------------------------------- -export type ListQueryRequest = AuthenticatedRequest<{}, {}, {}, ListQueryParams> & { - listQueryOptions?: ListQueryOptions; -}; +export namespace ListQuery { + export type Request = AuthenticatedRequest<{}, {}, {}, Params> & { + listQueryOptions?: Options; + }; -type ListQueryParams = { - filter?: string; - skip?: string; - take?: string; - select?: string; -}; + export type Params = { + filter?: string; + skip?: string; + take?: string; + select?: string; + }; -export type ListQueryOptions = { - filter?: Record; - select?: Record; - skip?: number; - take?: number; -}; + export type Options = { + filter?: Record; + select?: Record; + skip?: number; + take?: number; + }; + + /** + * Slim workflow returned from a list query operation. + */ + export namespace Workflow { + type OptionalBaseFields = 'name' | 'active' | 'versionId' | 'createdAt' | 'updatedAt' | 'tags'; + + type BaseFields = Pick & + Partial>; + + type SharedField = Partial>; + + type OwnedByField = { ownedBy: Pick | null }; + + export type Plain = BaseFields; + + export type WithSharing = BaseFields & SharedField; + + export type WithOwnership = BaseFields & OwnedByField; + } +} + +export function hasSharing( + workflows: ListQuery.Workflow.Plain[] | ListQuery.Workflow.WithSharing[], +): workflows is ListQuery.Workflow.WithSharing[] { + return workflows.some((w) => 'shared' in w); +} // ---------------------------------- // /credentials diff --git a/packages/cli/src/services/ownership.service.ts b/packages/cli/src/services/ownership.service.ts index 5b7848900..c756f817e 100644 --- a/packages/cli/src/services/ownership.service.ts +++ b/packages/cli/src/services/ownership.service.ts @@ -3,6 +3,8 @@ import { CacheService } from './cache.service'; import { SharedWorkflowRepository, UserRepository } from '@/databases/repositories'; import type { User } from '@/databases/entities/User'; import { RoleService } from './role.service'; +import type { ListQuery } from '@/requests'; +import type { Role } from '@/databases/entities/Role'; @Service() export class OwnershipService { @@ -34,4 +36,17 @@ export class OwnershipService { return sharedWorkflow.user; } + + addOwnedBy( + workflow: ListQuery.Workflow.WithSharing, + workflowOwnerRole: Role, + ): ListQuery.Workflow.WithOwnership { + const { shared, ...rest } = workflow; + + const ownerId = shared?.find((s) => s.roleId.toString() === workflowOwnerRole.id)?.userId; + + return Object.assign(rest, { + ownedBy: ownerId ? { id: ownerId } : null, + }); + } } diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts index 6820f5963..bc7914852 100644 --- a/packages/cli/src/workflows/workflows.controller.ee.ts +++ b/packages/cli/src/workflows/workflows.controller.ee.ts @@ -6,7 +6,7 @@ import * as WorkflowHelpers from '@/WorkflowHelpers'; import config from '@/config'; import { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { validateEntity } from '@/GenericHelpers'; -import type { ListQueryRequest, WorkflowRequest } from '@/requests'; +import type { ListQuery, WorkflowRequest } from '@/requests'; import { isSharingEnabled, rightDiff } from '@/UserManagement/UserManagementHelper'; import { EEWorkflowsService as EEWorkflows } from './workflows.services.ee'; import { ExternalHooks } from '@/ExternalHooks'; @@ -205,18 +205,14 @@ EEWorkflowController.post( EEWorkflowController.get( '/', listQueryMiddleware, - async (req: ListQueryRequest, res: express.Response) => { + async (req: ListQuery.Request, res: express.Response) => { try { - const [workflows, count] = await EEWorkflows.getMany(req.user, req.listQueryOptions); + const sharedWorkflowIds = await WorkflowHelpers.getSharedWorkflowIds(req.user); - let data; - - if (req.listQueryOptions?.select) { - data = workflows; - } else { - const role = await Container.get(RoleService).findWorkflowOwnerRole(); - data = workflows.map((w) => EEWorkflows.addOwnerId(w, role)); - } + const { workflows: data, count } = await EEWorkflows.getMany( + sharedWorkflowIds, + req.listQueryOptions, + ); res.json({ count, data }); } catch (maybeError) { diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index a0874145a..aa0f5dcc6 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -14,7 +14,7 @@ import { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { validateEntity } from '@/GenericHelpers'; import { ExternalHooks } from '@/ExternalHooks'; import { getLogger } from '@/Logger'; -import type { ListQueryRequest, WorkflowRequest } from '@/requests'; +import type { ListQuery, WorkflowRequest } from '@/requests'; import { isBelowOnboardingThreshold } from '@/WorkflowHelpers'; import { EEWorkflowController } from './workflows.controller.ee'; import { WorkflowsService } from './workflows.services'; @@ -118,9 +118,14 @@ workflowsController.post( workflowsController.get( '/', listQueryMiddleware, - async (req: ListQueryRequest, res: express.Response) => { + async (req: ListQuery.Request, res: express.Response) => { try { - const [data, count] = await WorkflowsService.getMany(req.user, req.listQueryOptions); + const sharedWorkflowIds = await WorkflowHelpers.getSharedWorkflowIds(req.user, ['owner']); + + const { workflows: data, count } = await WorkflowsService.getMany( + sharedWorkflowIds, + req.listQueryOptions, + ); res.json({ count, data }); } catch (maybeError) { diff --git a/packages/cli/src/workflows/workflows.services.ee.ts b/packages/cli/src/workflows/workflows.services.ee.ts index f4dfcb7cb..91c1b44da 100644 --- a/packages/cli/src/workflows/workflows.services.ee.ts +++ b/packages/cli/src/workflows/workflows.services.ee.ts @@ -5,7 +5,6 @@ import * as ResponseHelper from '@/ResponseHelper'; import * as WorkflowHelpers from '@/WorkflowHelpers'; import type { ICredentialsDb } from '@/Interfaces'; import { SharedWorkflow } from '@db/entities/SharedWorkflow'; -import type { Role } from '@db/entities/Role'; import type { User } from '@db/entities/User'; import { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { UserService } from '@/user/user.service'; @@ -13,20 +12,13 @@ import { WorkflowsService } from './workflows.services'; import type { CredentialUsedByWorkflow, WorkflowWithSharingsAndCredentials, - WorkflowForList, } from './workflows.types'; import { EECredentialsService as EECredentials } from '@/credentials/credentials.service.ee'; -import { getSharedWorkflowIds } from '@/WorkflowHelpers'; import { NodeOperationError } from 'n8n-workflow'; import { RoleService } from '@/services/role.service'; import Container from 'typedi'; export class EEWorkflowsService extends WorkflowsService { - static async getWorkflowIdsForUser(user: User) { - // Get all workflows regardless of role - return getSharedWorkflowIds(user); - } - static async isOwned( user: User, workflowId: string, @@ -88,14 +80,6 @@ export class EEWorkflowsService extends WorkflowsService { return transaction.save(newSharedWorkflows); } - static addOwnerId(workflow: WorkflowForList, workflowOwnerRole: Role) { - const ownerId = workflow.shared?.find(({ roleId }) => String(roleId) === workflowOwnerRole.id) - ?.userId; - workflow.ownedBy = ownerId ? { id: ownerId } : null; - delete workflow.shared; - return workflow; - } - static addOwnerAndSharings(workflow: WorkflowWithSharingsAndCredentials): void { workflow.ownedBy = null; workflow.sharedWith = []; diff --git a/packages/cli/src/workflows/workflows.services.ts b/packages/cli/src/workflows/workflows.services.ts index 8f249d830..60b8c4306 100644 --- a/packages/cli/src/workflows/workflows.services.ts +++ b/packages/cli/src/workflows/workflows.services.ts @@ -11,23 +11,23 @@ import * as ResponseHelper from '@/ResponseHelper'; import * as WorkflowHelpers from '@/WorkflowHelpers'; import config from '@/config'; import type { SharedWorkflow } from '@db/entities/SharedWorkflow'; -import type { RoleNames } from '@db/entities/Role'; import type { User } from '@db/entities/User'; import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { validateEntity } from '@/GenericHelpers'; import { ExternalHooks } from '@/ExternalHooks'; +import { type WorkflowRequest, type ListQuery, hasSharing } from '@/requests'; import { TagService } from '@/services/tag.service'; -import type { ListQueryOptions, WorkflowRequest } from '@/requests'; import type { IWorkflowDb, IWorkflowExecutionDataProcess } from '@/Interfaces'; import { NodeTypes } from '@/NodeTypes'; import { WorkflowRunner } from '@/WorkflowRunner'; import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; import { TestWebhooks } from '@/TestWebhooks'; -import { getSharedWorkflowIds } from '@/WorkflowHelpers'; -import { isSharingEnabled, whereClause } from '@/UserManagement/UserManagementHelper'; +import { whereClause } from '@/UserManagement/UserManagementHelper'; import { InternalHooks } from '@/InternalHooks'; -import type { WorkflowForList } from './workflows.types'; import { WorkflowRepository } from '@/databases/repositories'; +import { RoleService } from '@/services/role.service'; +import { OwnershipService } from '@/services/ownership.service'; +import { isStringArray } from '@/utils'; export class WorkflowsService { static async getSharing( @@ -94,75 +94,85 @@ export class WorkflowsService { return Db.collections.Workflow.findOne({ where: workflow, relations: options?.relations }); } - // Warning: this function is overridden by EE to disregard role list. - static async getWorkflowIdsForUser(user: User, roles?: RoleNames[]): Promise { - return getSharedWorkflowIds(user, roles); - } + static async getMany(sharedWorkflowIds: string[], options?: ListQuery.Options) { + if (sharedWorkflowIds.length === 0) return { workflows: [], count: 0 }; - static async getMany( - user: User, - options?: ListQueryOptions, - ): Promise<[WorkflowForList[], number]> { - const sharedWorkflowIds = await this.getWorkflowIdsForUser(user, ['owner']); - if (sharedWorkflowIds.length === 0) { - // return early since without shared workflows there can be no hits - // (note: getSharedWorkflowIds() returns _all_ workflow ids for global owners) - return [[], 0]; - } - - const filter = options?.filter ?? {}; - - // safeguard against querying ids not shared with the user - const workflowId = filter?.id?.toString(); - if (workflowId !== undefined && !sharedWorkflowIds.includes(workflowId)) { - LoggerProxy.verbose(`User ${user.id} attempted to query non-shared workflow ${workflowId}`); - return [[], 0]; - } - - const DEFAULT_SELECT: FindOptionsSelect = { - id: true, - name: true, - active: true, - createdAt: true, - updatedAt: true, + const where: FindOptionsWhere = { + ...options?.filter, + id: In(sharedWorkflowIds), }; - const select: FindOptionsSelect = options?.select ?? DEFAULT_SELECT; + const reqTags = options?.filter?.tags; + + if (isStringArray(reqTags)) { + where.tags = reqTags.map((tag) => ({ name: tag })); + } + + type Select = FindOptionsSelect & { ownedBy?: true }; + + const select: Select = options?.select + ? { ...options.select } // copy to enable field removal without affecting original + : { + name: true, + active: true, + createdAt: true, + updatedAt: true, + versionId: true, + shared: { userId: true, roleId: true }, + }; + + delete select?.ownedBy; // remove non-entity field, handled after query const relations: string[] = []; + const areTagsEnabled = !config.getEnv('workflowTagsDisabled'); const isDefaultSelect = options?.select === undefined; + const areTagsRequested = isDefaultSelect || options?.select?.tags === true; + const isOwnedByIncluded = isDefaultSelect || options?.select?.ownedBy === true; - if (isDefaultSelect && !config.getEnv('workflowTagsDisabled')) { + if (areTagsEnabled && areTagsRequested) { relations.push('tags'); select.tags = { id: true, name: true }; } - if (isDefaultSelect && isSharingEnabled()) { - relations.push('shared'); - select.shared = { userId: true, roleId: true }; - select.versionId = true; - } + if (isOwnedByIncluded) relations.push('shared'); - filter.id = In(sharedWorkflowIds); - - if (typeof filter.name === 'string' && filter.name !== '') { - filter.name = Like(`%${filter.name}%`); + if (typeof where.name === 'string' && where.name !== '') { + where.name = Like(`%${where.name}%`); } const findManyOptions: FindManyOptions = { - select, - relations, - where: filter, - order: { updatedAt: 'ASC' }, + select: { ...select, id: true }, + where, }; + if (isDefaultSelect || options?.select?.updatedAt === true) { + findManyOptions.order = { updatedAt: 'ASC' }; + } + + if (relations.length > 0) { + findManyOptions.relations = relations; + } + if (options?.take) { findManyOptions.skip = options.skip; findManyOptions.take = options.take; } - return Container.get(WorkflowRepository).findAndCount(findManyOptions); + const [workflows, count] = (await Container.get(WorkflowRepository).findAndCount( + findManyOptions, + )) as [ListQuery.Workflow.Plain[] | ListQuery.Workflow.WithSharing[], number]; + + if (!hasSharing(workflows)) return { workflows, count }; + + const workflowOwnerRole = await Container.get(RoleService).findWorkflowOwnerRole(); + + return { + workflows: workflows.map((w) => + Container.get(OwnershipService).addOwnedBy(w, workflowOwnerRole), + ), + count, + }; } static async update( diff --git a/packages/cli/src/workflows/workflows.types.ts b/packages/cli/src/workflows/workflows.types.ts index 99813736a..ef30bb18c 100644 --- a/packages/cli/src/workflows/workflows.types.ts +++ b/packages/cli/src/workflows/workflows.types.ts @@ -9,12 +9,6 @@ export interface WorkflowWithSharingsAndCredentials extends Omit { - ownedBy?: Pick | null; - shared?: SharedWorkflow[]; -} - export interface CredentialUsedByWorkflow { id: string; name: string; diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index d6da8cbde..58ae9cef6 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -35,7 +35,7 @@ import type { ExecutionData } from '@db/entities/ExecutionData'; import { generateNanoId } from '@db/utils/generators'; import { RoleService } from '@/services/role.service'; import { VariablesService } from '@/environments/variables/variables.service'; -import { TagRepository } from '@/databases/repositories'; +import { TagRepository, WorkflowTagMappingRepository } from '@/databases/repositories'; import { separate } from '@/utils'; export type TestDBType = 'postgres' | 'mysql'; @@ -119,6 +119,7 @@ export async function truncate(collections: CollectionName[]) { if (tag) { await Container.get(TagRepository).delete({}); + await Container.get(WorkflowTagMappingRepository).delete({}); } for (const collection of rest) { @@ -389,14 +390,24 @@ export async function createWaitingExecution(workflow: WorkflowEntity) { // Tags // ---------------------------------- -export async function createTag(attributes: Partial = {}) { +export async function createTag(attributes: Partial = {}, workflow?: WorkflowEntity) { const { name } = attributes; - return Container.get(TagRepository).save({ + const tag = await Container.get(TagRepository).save({ id: generateNanoId(), name: name ?? randomName(), ...attributes, }); + + if (workflow) { + const mappingRepository = Container.get(WorkflowTagMappingRepository); + + const mapping = mappingRepository.create({ tagId: tag.id, workflowId: workflow.id }); + + await mappingRepository.save(mapping); + } + + return tag; } // ---------------------------------- diff --git a/packages/cli/test/integration/workflows.controller.ee.test.ts b/packages/cli/test/integration/workflows.controller.ee.test.ts index 23f455d59..6c63bbc8a 100644 --- a/packages/cli/test/integration/workflows.controller.ee.test.ts +++ b/packages/cli/test/integration/workflows.controller.ee.test.ts @@ -136,60 +136,6 @@ describe('PUT /workflows/:id', () => { }); }); -describe('GET /workflows', () => { - test('should return workflows without nodes, sharing and credential usage details', async () => { - const tag = await testDb.createTag({ name: 'test' }); - - const savedCredential = await saveCredential(randomCredentialPayload(), { user: owner }); - - const workflow = await createWorkflow( - { - nodes: [ - { - id: uuid(), - name: 'Action Network', - type: 'n8n-nodes-base.actionNetwork', - parameters: {}, - typeVersion: 1, - position: [0, 0], - credentials: { - actionNetworkApi: { - id: savedCredential.id, - name: savedCredential.name, - }, - }, - }, - ], - tags: [tag], - }, - owner, - ); - - await testDb.shareWorkflowWithUsers(workflow, [member]); - - const response = await authOwnerAgent.get('/workflows'); - - const [fetchedWorkflow] = response.body.data; - - expect(response.statusCode).toBe(200); - expect(fetchedWorkflow.ownedBy).toMatchObject({ - id: owner.id, - }); - - expect(fetchedWorkflow.sharedWith).not.toBeDefined(); - expect(fetchedWorkflow.usedCredentials).not.toBeDefined(); - expect(fetchedWorkflow.nodes).not.toBeDefined(); - expect(fetchedWorkflow.tags).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - id: expect.any(String), - name: expect.any(String), - }), - ]), - ); - }); -}); - describe('GET /workflows/new', () => { [true, false].forEach((sharingEnabled) => { test(`should return an auto-incremented name, even when sharing is ${ diff --git a/packages/cli/test/integration/workflows.controller.test.ts b/packages/cli/test/integration/workflows.controller.test.ts index dfc368976..fb56237fd 100644 --- a/packages/cli/test/integration/workflows.controller.test.ts +++ b/packages/cli/test/integration/workflows.controller.test.ts @@ -1,24 +1,32 @@ import type { SuperAgentTest } from 'supertest'; -import type { IPinData } from 'n8n-workflow'; +import type { INode, IPinData } from 'n8n-workflow'; import * as UserManagementHelpers from '@/UserManagement/UserManagementHelper'; import * as utils from './shared/utils/'; import * as testDb from './shared/testDb'; import { makeWorkflow, MOCK_PINDATA } from './shared/utils/'; +import type { User } from '@/databases/entities/User'; +import { randomCredentialPayload } from './shared/random'; +import { v4 as uuid } from 'uuid'; +import { RoleService } from '@/services/role.service'; +import Container from 'typedi'; +import type { ListQuery } from '@/requests'; +let owner: User; let authOwnerAgent: SuperAgentTest; jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(false); const testServer = utils.setupTestServer({ endpointGroups: ['workflows'] }); +const { objectContaining, arrayContaining, any } = expect; + beforeAll(async () => { - const globalOwnerRole = await testDb.getGlobalOwnerRole(); - const ownerShell = await testDb.createUserShell(globalOwnerRole); - authOwnerAgent = testServer.authAgentFor(ownerShell); + owner = await testDb.createOwner(); + authOwnerAgent = testServer.authAgentFor(owner); }); beforeEach(async () => { - await testDb.truncate(['Workflow', 'SharedWorkflow']); + await testDb.truncate(['Workflow', 'SharedWorkflow', 'Tag']); }); describe('POST /workflows', () => { @@ -53,3 +61,260 @@ describe('GET /workflows/:id', () => { expect(pinData).toMatchObject(MOCK_PINDATA); }); }); + +describe('GET /workflows', () => { + test('should return zero workflows if none exist', async () => { + const response = await authOwnerAgent.get('/workflows').expect(200); + + expect(response.body).toEqual({ count: 0, data: [] }); + }); + + test('should return workflows', async () => { + const credential = await testDb.saveCredential(randomCredentialPayload(), { + user: owner, + role: await Container.get(RoleService).findCredentialOwnerRole(), + }); + + const nodes: INode[] = [ + { + id: uuid(), + name: 'Action Network', + type: 'n8n-nodes-base.actionNetwork', + parameters: {}, + typeVersion: 1, + position: [0, 0], + credentials: { + actionNetworkApi: { + id: credential.id, + name: credential.name, + }, + }, + }, + ]; + + const tag = await testDb.createTag({ name: 'A' }); + + await testDb.createWorkflow({ name: 'First', nodes, tags: [tag] }, owner); + await testDb.createWorkflow({ name: 'Second' }, owner); + + const response = await authOwnerAgent.get('/workflows').expect(200); + + expect(response.body).toEqual({ + count: 2, + data: arrayContaining([ + objectContaining({ + id: any(String), + name: 'First', + active: any(Boolean), + createdAt: any(String), + updatedAt: any(String), + tags: [{ id: any(String), name: 'A' }], + versionId: null, + ownedBy: { id: owner.id }, + }), + objectContaining({ + id: any(String), + name: 'Second', + active: any(Boolean), + createdAt: any(String), + updatedAt: any(String), + tags: [], + versionId: null, + ownedBy: { id: owner.id }, + }), + ]), + }); + + const found = response.body.data.find( + (w: ListQuery.Workflow.WithOwnership) => w.name === 'First', + ); + + expect(found.nodes).toBeUndefined(); + expect(found.sharedWith).toBeUndefined(); + expect(found.usedCredentials).toBeUndefined(); + }); + + describe('filter', () => { + test('should filter workflows by field: name', async () => { + await testDb.createWorkflow({ name: 'First' }, owner); + await testDb.createWorkflow({ name: 'Second' }, owner); + + const response = await authOwnerAgent + .get('/workflows') + .query('filter={"name":"First"}') + .expect(200); + + expect(response.body).toEqual({ + count: 1, + data: [objectContaining({ name: 'First' })], + }); + }); + + test('should filter workflows by field: active', async () => { + await testDb.createWorkflow({ active: true }, owner); + await testDb.createWorkflow({ active: false }, owner); + + const response = await authOwnerAgent + .get('/workflows') + .query('filter={ "active": true }') + .expect(200); + + expect(response.body).toEqual({ + count: 1, + data: [objectContaining({ active: true })], + }); + }); + + test('should filter workflows by field: tags', async () => { + const workflow = await testDb.createWorkflow({ name: 'First' }, owner); + + await testDb.createTag({ name: 'A' }, workflow); + await testDb.createTag({ name: 'B' }, workflow); + + const response = await authOwnerAgent + .get('/workflows') + .query('filter={ "tags": ["A"] }') + .expect(200); + + expect(response.body).toEqual({ + count: 1, + data: [objectContaining({ name: 'First', tags: [{ id: any(String), name: 'A' }] })], + }); + }); + }); + + describe('select', () => { + test('should select workflow field: name', async () => { + await testDb.createWorkflow({ name: 'First' }, owner); + await testDb.createWorkflow({ name: 'Second' }, owner); + + const response = await authOwnerAgent.get('/workflows').query('select=["name"]').expect(200); + + expect(response.body).toEqual({ + count: 2, + data: arrayContaining([ + { id: any(String), name: 'First' }, + { id: any(String), name: 'Second' }, + ]), + }); + }); + + test('should select workflow field: active', async () => { + await testDb.createWorkflow({ active: true }, owner); + await testDb.createWorkflow({ active: false }, owner); + + const response = await authOwnerAgent + .get('/workflows') + .query('select=["active"]') + .expect(200); + + expect(response.body).toEqual({ + count: 2, + data: arrayContaining([ + { id: any(String), active: true }, + { id: any(String), active: false }, + ]), + }); + }); + + test('should select workflow field: tags', async () => { + const firstWorkflow = await testDb.createWorkflow({ name: 'First' }, owner); + const secondWorkflow = await testDb.createWorkflow({ name: 'Second' }, owner); + + await testDb.createTag({ name: 'A' }, firstWorkflow); + await testDb.createTag({ name: 'B' }, secondWorkflow); + + const response = await authOwnerAgent.get('/workflows').query('select=["tags"]').expect(200); + + expect(response.body).toEqual({ + count: 2, + data: arrayContaining([ + objectContaining({ id: any(String), tags: [{ id: any(String), name: 'A' }] }), + objectContaining({ id: any(String), tags: [{ id: any(String), name: 'B' }] }), + ]), + }); + }); + + test('should select workflow fields: createdAt and updatedAt', async () => { + const firstWorkflowCreatedAt = '2023-08-08T09:31:25.000Z'; + const firstWorkflowUpdatedAt = '2023-08-08T09:31:40.000Z'; + const secondWorkflowCreatedAt = '2023-07-07T09:31:25.000Z'; + const secondWorkflowUpdatedAt = '2023-07-07T09:31:40.000Z'; + + await testDb.createWorkflow( + { + createdAt: new Date(firstWorkflowCreatedAt), + updatedAt: new Date(firstWorkflowUpdatedAt), + }, + owner, + ); + await testDb.createWorkflow( + { + createdAt: new Date(secondWorkflowCreatedAt), + updatedAt: new Date(secondWorkflowUpdatedAt), + }, + owner, + ); + + const response = await authOwnerAgent + .get('/workflows') + .query('select=["createdAt", "updatedAt"]') + .expect(200); + + expect(response.body).toEqual({ + count: 2, + data: arrayContaining([ + objectContaining({ + id: any(String), + createdAt: firstWorkflowCreatedAt, + updatedAt: firstWorkflowUpdatedAt, + }), + objectContaining({ + id: any(String), + createdAt: secondWorkflowCreatedAt, + updatedAt: secondWorkflowUpdatedAt, + }), + ]), + }); + }); + + test('should select workflow field: versionId', async () => { + const firstWorkflowVersionId = 'e95ccdde-2b4e-4fd0-8834-220a2b5b4353'; + const secondWorkflowVersionId = 'd099b8dc-b1d8-4b2d-9b02-26f32c0ee785'; + + await testDb.createWorkflow({ versionId: firstWorkflowVersionId }, owner); + await testDb.createWorkflow({ versionId: secondWorkflowVersionId }, owner); + + const response = await authOwnerAgent + .get('/workflows') + .query('select=["versionId"]') + .expect(200); + + expect(response.body).toEqual({ + count: 2, + data: arrayContaining([ + { id: any(String), versionId: firstWorkflowVersionId }, + { id: any(String), versionId: secondWorkflowVersionId }, + ]), + }); + }); + + test('should select workflow field: ownedBy', async () => { + await testDb.createWorkflow({}, owner); + await testDb.createWorkflow({}, owner); + + const response = await authOwnerAgent + .get('/workflows') + .query('select=["ownedBy"]') + .expect(200); + + expect(response.body).toEqual({ + count: 2, + data: arrayContaining([ + { id: any(String), ownedBy: { id: owner.id } }, + { id: any(String), ownedBy: { id: owner.id } }, + ]), + }); + }); + }); +}); diff --git a/packages/cli/test/unit/middleware/listQuery.test.ts b/packages/cli/test/unit/middleware/listQuery.test.ts index bee36a904..dc119d1ed 100644 --- a/packages/cli/test/unit/middleware/listQuery.test.ts +++ b/packages/cli/test/unit/middleware/listQuery.test.ts @@ -1,52 +1,89 @@ import { filterListQueryMiddleware } from '@/middlewares/listQuery/filter'; import { LoggerProxy } from 'n8n-workflow'; import { getLogger } from '@/Logger'; -import type { Request, Response, NextFunction } from 'express'; -import type { ListQueryRequest } from '@/requests'; import { selectListQueryMiddleware } from '@/middlewares/listQuery/select'; import { paginationListQueryMiddleware } from '@/middlewares/listQuery/pagination'; +import * as ResponseHelper from '@/ResponseHelper'; +import type { ListQuery } from '@/requests'; +import type { Response, NextFunction } from 'express'; describe('List query middleware', () => { - let mockReq: Partial; - let mockRes: Partial; + let mockReq: ListQuery.Request; + let mockRes: Response; let nextFn: NextFunction = jest.fn(); - let args: [Request, Response, NextFunction]; + let args: [ListQuery.Request, Response, NextFunction]; + + let sendErrorResponse: jest.SpyInstance; beforeEach(() => { - LoggerProxy.init(getLogger()); - mockReq = { baseUrl: '/rest/workflows' }; - args = [mockReq as Request, mockRes as Response, nextFn]; jest.restoreAllMocks(); + + LoggerProxy.init(getLogger()); + mockReq = { baseUrl: '/rest/workflows' } as ListQuery.Request; + mockRes = { status: () => ({ json: jest.fn() }) } as unknown as Response; + args = [mockReq, mockRes, nextFn]; + + sendErrorResponse = jest.spyOn(ResponseHelper, 'sendErrorResponse'); }); describe('Query filter', () => { - test('should parse valid filter', () => { + test('should not set filter on request if none sent', async () => { + mockReq.query = {}; + + await filterListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toBeUndefined(); + expect(nextFn).toBeCalledTimes(1); + }); + + test('should parse valid filter', async () => { mockReq.query = { filter: '{ "name": "My Workflow" }' }; - filterListQueryMiddleware(...args); + + await filterListQueryMiddleware(...args); expect(mockReq.listQueryOptions).toEqual({ filter: { name: 'My Workflow' } }); expect(nextFn).toBeCalledTimes(1); }); - test('should ignore invalid filter', () => { + test('should ignore invalid filter', async () => { mockReq.query = { filter: '{ "name": "My Workflow", "foo": "bar" }' }; - filterListQueryMiddleware(...args); + + await filterListQueryMiddleware(...args); expect(mockReq.listQueryOptions).toEqual({ filter: { name: 'My Workflow' } }); expect(nextFn).toBeCalledTimes(1); }); - test('should throw on invalid JSON', () => { + test('should throw on invalid JSON', async () => { mockReq.query = { filter: '{ "name" : "My Workflow"' }; - const call = () => filterListQueryMiddleware(...args); - expect(call).toThrowError('Failed to parse filter JSON'); + await filterListQueryMiddleware(...args); + + expect(sendErrorResponse).toHaveBeenCalledTimes(1); + }); + + test('should throw on valid filter with invalid type', async () => { + mockReq.query = { filter: '{ "name" : 123 }' }; + + await filterListQueryMiddleware(...args); + + expect(sendErrorResponse).toHaveBeenCalledTimes(1); }); }); describe('Query select', () => { + test('should not set select on request if none sent', async () => { + mockReq.query = {}; + + await filterListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toBeUndefined(); + expect(nextFn).toBeCalledTimes(1); + }); + test('should parse valid select', () => { mockReq.query = { select: '["name", "id"]' }; + selectListQueryMiddleware(...args); expect(mockReq.listQueryOptions).toEqual({ select: { name: true, id: true } }); @@ -55,6 +92,7 @@ describe('List query middleware', () => { test('ignore invalid select', () => { mockReq.query = { select: '["name", "foo"]' }; + selectListQueryMiddleware(...args); expect(mockReq.listQueryOptions).toEqual({ select: { name: true } }); @@ -63,20 +101,31 @@ describe('List query middleware', () => { test('throw on invalid JSON', () => { mockReq.query = { select: '["name"' }; - const call = () => selectListQueryMiddleware(...args); - expect(call).toThrowError('Failed to parse select JSON'); + selectListQueryMiddleware(...args); + + expect(sendErrorResponse).toHaveBeenCalledTimes(1); }); test('throw on non-string-array JSON for select', () => { mockReq.query = { select: '"name"' }; - const call = () => selectListQueryMiddleware(...args); - expect(call).toThrowError('Parsed select is not a string array'); + selectListQueryMiddleware(...args); + + expect(sendErrorResponse).toHaveBeenCalledTimes(1); }); }); describe('Query pagination', () => { + test('should not set pagination options on request if none sent', async () => { + mockReq.query = {}; + + await filterListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toBeUndefined(); + expect(nextFn).toBeCalledTimes(1); + }); + test('should parse valid pagination', () => { mockReq.query = { skip: '1', take: '2' }; paginationListQueryMiddleware(...args); @@ -103,6 +152,7 @@ describe('List query middleware', () => { test('should cap take at 50', () => { mockReq.query = { take: '51' }; + paginationListQueryMiddleware(...args); expect(mockReq.listQueryOptions).toEqual({ skip: 0, take: 50 }); @@ -111,9 +161,64 @@ describe('List query middleware', () => { test('should throw on non-numeric-integer take', () => { mockReq.query = { take: '3.2' }; - const call = () => paginationListQueryMiddleware(...args); - expect(call).toThrowError('Parameter take or skip is not an integer string'); + paginationListQueryMiddleware(...args); + + expect(sendErrorResponse).toHaveBeenCalledTimes(1); + }); + + test('should throw on non-numeric-integer skip', () => { + mockReq.query = { take: '3', skip: '3.2' }; + + paginationListQueryMiddleware(...args); + + expect(sendErrorResponse).toHaveBeenCalledTimes(1); + }); + }); + + describe('Combinations', () => { + test('should combine filter with select', async () => { + mockReq.query = { filter: '{ "name": "My Workflow" }', select: '["name", "id"]' }; + + await filterListQueryMiddleware(...args); + selectListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toEqual({ + select: { name: true, id: true }, + filter: { name: 'My Workflow' }, + }); + + expect(nextFn).toBeCalledTimes(2); + }); + + test('should combine filter with pagination options', async () => { + mockReq.query = { filter: '{ "name": "My Workflow" }', skip: '1', take: '2' }; + + await filterListQueryMiddleware(...args); + paginationListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toEqual({ + filter: { name: 'My Workflow' }, + skip: 1, + take: 2, + }); + + expect(nextFn).toBeCalledTimes(2); + }); + + test('should combine select with pagination options', async () => { + mockReq.query = { select: '["name", "id"]', skip: '1', take: '2' }; + + selectListQueryMiddleware(...args); + paginationListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toEqual({ + select: { name: true, id: true }, + skip: 1, + take: 2, + }); + + expect(nextFn).toBeCalledTimes(2); }); }); });