feat(core): Add ownership, sharing and credential details to GET /workflows (#4510)

*  Abstract into `getMany()`

*  Use `getMany()` from free controller

*  Use `getMany()` from paid controller

* 🧪 Add tests

* 🧪 Fix tests

*  Add credential usage info

* 🧪 Update tests

*  Add type and adjust test
This commit is contained in:
Iván Ovejero 2022-11-08 17:52:42 +01:00 committed by GitHub
parent 01171912e7
commit 026fb50512
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 193 additions and 99 deletions

View file

@ -11,6 +11,7 @@ import { SharedWorkflow } from '../databases/entities/SharedWorkflow';
import { LoggerProxy } from 'n8n-workflow';
import * as TagHelpers from '../TagHelpers';
import { EECredentialsService as EECredentials } from '../credentials/credentials.service.ee';
import { WorkflowsService } from './workflows.services';
// eslint-disable-next-line @typescript-eslint/naming-convention
export const EEWorkflowController = express.Router();
@ -181,6 +182,25 @@ EEWorkflowController.post(
}),
);
/**
* (EE) GET /workflows
*/
EEWorkflowController.get(
'/',
ResponseHelper.send(async (req: WorkflowRequest.GetAll) => {
const workflows = (await WorkflowsService.getMany(
req.user,
req.query.filter,
)) as unknown as WorkflowEntity[];
return Promise.all(
workflows.map(async (workflow) =>
EEWorkflows.addCredentialsToWorkflow(EEWorkflows.addOwnerAndSharings(workflow), req.user),
),
);
}),
);
EEWorkflowController.patch(
'/:id(\\d+)',
ResponseHelper.send(async (req: WorkflowRequest.Update) => {

View file

@ -2,10 +2,9 @@
/* eslint-disable import/no-cycle */
import express from 'express';
import { INode, IPinData, JsonObject, jsonParse, LoggerProxy, Workflow } from 'n8n-workflow';
import { INode, IPinData, LoggerProxy, Workflow } from 'n8n-workflow';
import axios from 'axios';
import { FindManyOptions, In } from 'typeorm';
import {
ActiveWorkflowRunner,
Db,
@ -31,32 +30,13 @@ import { InternalHooksManager } from '../InternalHooksManager';
import { externalHooks } from '../Server';
import { getLogger } from '../Logger';
import type { WorkflowRequest } from '../requests';
import { getSharedWorkflowIds, isBelowOnboardingThreshold } from '../WorkflowHelpers';
import { isBelowOnboardingThreshold } from '../WorkflowHelpers';
import { EEWorkflowController } from './workflows.controller.ee';
import { WorkflowsService } from './workflows.services';
import { validate as jsonSchemaValidate } from 'jsonschema';
const activeWorkflowRunner = ActiveWorkflowRunner.getInstance();
export const workflowsController = express.Router();
const schemaGetWorkflowsQueryFilter = {
$id: '/IGetWorkflowsQueryFilter',
type: 'object',
properties: {
id: { anyOf: [{ type: 'integer' }, { type: 'string' }] },
name: { type: 'string' },
active: { type: 'boolean' },
},
};
const allowedWorkflowsQueryFilterFields = Object.keys(schemaGetWorkflowsQueryFilter.properties);
interface IGetWorkflowsQueryFilter {
id?: number | string;
name?: string;
active?: boolean;
}
/**
* Initialize Logger if needed
*/
@ -155,83 +135,13 @@ workflowsController.post(
}),
);
// Returns workflows
/**
* GET /workflows
*/
workflowsController.get(
`/`,
'/',
ResponseHelper.send(async (req: WorkflowRequest.GetAll) => {
const sharedWorkflowIds = await getSharedWorkflowIds(req.user);
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 [];
}
// parse incoming filter object and remove non-valid fields
let filter: IGetWorkflowsQueryFilter | undefined = undefined;
if (req.query.filter) {
try {
const filterJson: JsonObject = jsonParse(req.query.filter);
if (filterJson) {
Object.keys(filterJson).map((key) => {
if (!allowedWorkflowsQueryFilterFields.includes(key)) delete filterJson[key];
});
if (jsonSchemaValidate(filterJson, schemaGetWorkflowsQueryFilter).valid) {
filter = filterJson as IGetWorkflowsQueryFilter;
}
}
} catch (error) {
LoggerProxy.error('Failed to parse filter', {
userId: req.user.id,
filter: req.query.filter,
});
throw new ResponseHelper.ResponseError(
`Parameter "filter" contained invalid JSON string.`,
500,
500,
);
}
}
// safeguard against querying ids not shared with the user
if (filter?.id !== undefined) {
const workflowId = parseInt(filter.id.toString());
if (workflowId && !sharedWorkflowIds.includes(workflowId)) {
LoggerProxy.verbose(
`User ${req.user.id} attempted to query non-shared workflow ${workflowId}`,
);
return [];
}
}
const query: FindManyOptions<WorkflowEntity> = {
select: ['id', 'name', 'active', 'createdAt', 'updatedAt'],
relations: ['tags'],
};
if (config.getEnv('workflowTagsDisabled')) {
delete query.relations;
}
const workflows = await Db.collections.Workflow.find(
Object.assign(query, {
where: {
id: In(sharedWorkflowIds),
...filter,
},
}),
);
return workflows.map((workflow) => {
const { id, ...rest } = workflow;
return {
id: id.toString(),
...rest,
};
});
return WorkflowsService.getMany(req.user, req.query.filter);
}),
);

View file

@ -127,6 +127,7 @@ export class EEWorkflowsService extends WorkflowsService {
workflow.usedCredentials?.push({
id: credential.id.toString(),
name: credential.name,
type: credential.type,
currentUserHasAccess: userCredentialIds.includes(credentialId),
});
});

View file

@ -1,5 +1,5 @@
import { LoggerProxy } from 'n8n-workflow';
import { FindManyOptions, FindOneOptions, ObjectLiteral } from 'typeorm';
import { JsonObject, jsonParse, LoggerProxy } from 'n8n-workflow';
import { FindManyOptions, FindOneOptions, In, ObjectLiteral } from 'typeorm';
import {
ActiveWorkflowRunner,
Db,
@ -15,6 +15,26 @@ import { WorkflowEntity } from '../databases/entities/WorkflowEntity';
import { validateEntity } from '../GenericHelpers';
import { externalHooks } from '../Server';
import * as TagHelpers from '../TagHelpers';
import { getSharedWorkflowIds } from '../WorkflowHelpers';
import { validate as jsonSchemaValidate } from 'jsonschema';
export interface IGetWorkflowsQueryFilter {
id?: number | string;
name?: string;
active?: boolean;
}
const schemaGetWorkflowsQueryFilter = {
$id: '/IGetWorkflowsQueryFilter',
type: 'object',
properties: {
id: { anyOf: [{ type: 'integer' }, { type: 'string' }] },
name: { type: 'string' },
active: { type: 'boolean' },
},
};
const allowedWorkflowsQueryFilterFields = Object.keys(schemaGetWorkflowsQueryFilter.properties);
export class WorkflowsService {
static async getSharing(
@ -47,6 +67,80 @@ export class WorkflowsService {
return Db.collections.Workflow.findOne(workflow, options);
}
static async getMany(user: User, rawFilter: string) {
const sharedWorkflowIds = await getSharedWorkflowIds(user);
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 [];
}
let filter: IGetWorkflowsQueryFilter | undefined = undefined;
if (rawFilter) {
try {
const filterJson: JsonObject = jsonParse(rawFilter);
if (filterJson) {
Object.keys(filterJson).map((key) => {
if (!allowedWorkflowsQueryFilterFields.includes(key)) delete filterJson[key];
});
if (jsonSchemaValidate(filterJson, schemaGetWorkflowsQueryFilter).valid) {
filter = filterJson as IGetWorkflowsQueryFilter;
}
}
} catch (error) {
LoggerProxy.error('Failed to parse filter', {
userId: user.id,
filter,
});
throw new ResponseHelper.ResponseError(
`Parameter "filter" contained invalid JSON string.`,
500,
500,
);
}
}
// safeguard against querying ids not shared with the user
if (filter?.id !== undefined) {
const workflowId = parseInt(filter.id.toString());
if (workflowId && !sharedWorkflowIds.includes(workflowId)) {
LoggerProxy.verbose(`User ${user.id} attempted to query non-shared workflow ${workflowId}`);
return [];
}
}
const fields: Array<keyof WorkflowEntity> = ['id', 'name', 'active', 'createdAt', 'updatedAt'];
const query: FindManyOptions<WorkflowEntity> = {
select: config.get('enterprise.features.sharing') ? [...fields, 'nodes'] : fields,
relations: config.get('enterprise.features.sharing')
? ['tags', 'shared', 'shared.user', 'shared.role']
: ['tags'],
};
if (config.getEnv('workflowTagsDisabled')) {
delete query.relations;
}
const workflows = await Db.collections.Workflow.find(
Object.assign(query, {
where: {
id: In(sharedWorkflowIds),
...filter,
},
}),
);
return workflows.map((workflow) => {
const { id, ...rest } = workflow;
return {
id: id.toString(),
...rest,
};
});
}
static async updateWorkflow(
user: User,
workflow: WorkflowEntity,

View file

@ -12,5 +12,6 @@ export interface WorkflowWithSharingsAndCredentials extends Omit<WorkflowEntity,
export interface CredentialUsedByWorkflow {
id: string;
name: string;
type?: string;
currentUserHasAccess: boolean;
}

View file

@ -16,9 +16,6 @@ import { INode } from 'n8n-workflow';
jest.mock('../../src/telemetry');
// mock whether sharing is enabled or not
jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(true);
let app: express.Application;
let testDbName = '';
@ -28,6 +25,7 @@ let credentialOwnerRole: Role;
let authAgent: AuthAgent;
let saveCredential: SaveCredentialFunction;
let workflowRunner: ActiveWorkflowRunner.ActiveWorkflowRunner;
let sharingSpy: jest.SpyInstance<boolean>;
beforeAll(async () => {
app = await utils.initTestServer({
@ -52,6 +50,9 @@ beforeAll(async () => {
await utils.initNodeTypes();
workflowRunner = await utils.initActiveWorkflowRunner();
config.set('enterprise.features.sharing', true);
sharingSpy = jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(true); // @TODO: Remove on release
});
beforeEach(async () => {
@ -135,6 +136,73 @@ describe('PUT /workflows/:id', () => {
});
});
describe('GET /workflows', () => {
test('should return workflows with ownership, sharing and credential usage details', async () => {
const owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole });
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.toString(),
name: savedCredential.name,
},
},
},
],
},
owner,
);
await testDb.shareWorkflowWithUsers(workflow, [member]);
const response = await authAgent(owner).get('/workflows');
const [fetchedWorkflow] = response.body.data;
expect(response.statusCode).toBe(200);
expect(fetchedWorkflow.ownedBy).toMatchObject({
id: owner.id,
email: owner.email,
firstName: owner.firstName,
lastName: owner.lastName,
});
expect(fetchedWorkflow.sharedWith).toHaveLength(1);
const [sharee] = fetchedWorkflow.sharedWith;
expect(sharee).toMatchObject({
id: member.id,
email: member.email,
firstName: member.firstName,
lastName: member.lastName,
});
expect(fetchedWorkflow.usedCredentials).toHaveLength(1);
const [usedCredential] = fetchedWorkflow.usedCredentials;
expect(usedCredential).toMatchObject({
id: savedCredential.id.toString(),
name: savedCredential.name,
type: savedCredential.type,
currentUserHasAccess: true,
});
});
});
describe('GET /workflows/:id', () => {
test('GET should fail with invalid id due to route rule', async () => {
const owner = await testDb.createUser({ globalRole: globalOwnerRole });