Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions packages/cli/src/workflows/workflows.controller.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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) => {
Expand Down
98 changes: 4 additions & 94 deletions packages/cli/src/workflows/workflows.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
*/
Expand Down Expand Up @@ -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);
}),
);

Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/workflows/workflows.services.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
});
});
Expand Down
98 changes: 96 additions & 2 deletions packages/cli/src/workflows/workflows.services.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/workflows/workflows.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ export interface WorkflowWithSharingsAndCredentials extends Omit<WorkflowEntity,
export interface CredentialUsedByWorkflow {
id: string;
name: string;
type?: string;
currentUserHasAccess: boolean;
}
74 changes: 71 additions & 3 deletions packages/cli/test/integration/workflows.controller.ee.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '';

Expand All @@ -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({
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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 });
Expand Down