Skip to content

Commit

Permalink
Merge branch 'master' into feature/undo-redo
Browse files Browse the repository at this point in the history
* master:
  fix: Lazy load nodes for credentials testing (#4760)
  fix(core): Fix `$items().length` in Execute Once mode (#4755)
  feat(Google Calendar Node): Use resource locator component for calendar parameters (#4410)
  fix: Change the currentUserHasAccess flag behavior (no-changelog) (#4763)
  • Loading branch information
MiloradFilipovic committed Nov 30, 2022
2 parents b0b35fa + 0a7a2f3 commit fda6e8f
Show file tree
Hide file tree
Showing 24 changed files with 475 additions and 254 deletions.
4 changes: 4 additions & 0 deletions packages/cli/src/CredentialTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ class CredentialTypesClass implements ICredentialTypes {
return this.getCredential(credentialType).type;
}

getNodeTypesToTestWith(type: string): string[] {
return this.knownCredentials[type]?.nodesToTestWith ?? [];
}

private getCredential(type: string): LoadedClass<ICredentialType> {
const loadedCredentials = this.loadedCredentials;
if (type in loadedCredentials) {
Expand Down
119 changes: 59 additions & 60 deletions packages/cli/src/CredentialsHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
IHttpRequestHelper,
INodeTypeData,
INodeTypes,
ICredentialTypes,
} from 'n8n-workflow';

import * as Db from '@/Db';
Expand All @@ -47,25 +48,48 @@ import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'
import { User } from '@db/entities/User';
import { CredentialsEntity } from '@db/entities/CredentialsEntity';
import { NodeTypes } from '@/NodeTypes';
import { CredentialsOverwrites } from '@/CredentialsOverwrites';
import { CredentialTypes } from '@/CredentialTypes';
import { CredentialsOverwrites } from '@/CredentialsOverwrites';
import { whereClause } from './UserManagement/UserManagementHelper';
import { RESPONSE_ERROR_MESSAGES } from './constants';

const mockNode = {
name: '',
typeVersion: 1,
type: 'mock',
position: [0, 0],
parameters: {} as INodeParameters,
} as INode;

const mockNodesData: INodeTypeData = {
mock: {
sourcePath: '',
type: {
description: { properties: [] as INodeProperties[] },
} as INodeType,
},
};

const mockNodesData: INodeTypeData = {};
const mockNodeTypes: INodeTypes = {
getAll(): Array<INodeType | IVersionedNodeType> {
return Object.values(mockNodesData).map((data) => data.type);
getByName(nodeType: string): INodeType | IVersionedNodeType {
return mockNodesData[nodeType]?.type;
},
getByNameAndVersion(nodeType: string, version?: number): INodeType | undefined {
if (mockNodesData[nodeType] === undefined) {
return undefined;
getByNameAndVersion(nodeType: string, version?: number): INodeType {
if (!mockNodesData[nodeType]) {
throw new Error(`${RESPONSE_ERROR_MESSAGES.NO_NODE}: ${nodeType}`);
}
return NodeHelpers.getVersionedNodeType(mockNodesData[nodeType].type, version);
},
};

export class CredentialsHelper extends ICredentialsHelper {
private credentialTypes = CredentialTypes();
constructor(
encryptionKey: string,
private credentialTypes: ICredentialTypes = CredentialTypes(),
private nodeTypes: INodeTypes = NodeTypes(),
) {
super(encryptionKey);
}

/**
* Add the required authentication information to the request
Expand Down Expand Up @@ -387,24 +411,16 @@ export class CredentialsHelper extends ICredentialsHelper {
throw e;
}
} else {
const node = {
name: '',
typeVersion: 1,
type: 'mock',
position: [0, 0],
parameters: {} as INodeParameters,
} as INode;

const workflow = new Workflow({
nodes: [node],
nodes: [mockNode],
connections: {},
active: false,
nodeTypes: mockNodeTypes,
});

// Resolve expressions if any are set
decryptedData = workflow.expression.getComplexParameterValue(
node,
mockNode,
decryptedData as INodeParameters,
mode,
defaultTimezone,
Expand Down Expand Up @@ -457,28 +473,27 @@ export class CredentialsHelper extends ICredentialsHelper {
await Db.collections.Credentials.update(findQuery, newCredentialsData);
}

getCredentialTestFunction(
private getCredentialTestFunction(
credentialType: string,
nodeToTestWith?: string,
): ICredentialTestFunction | ICredentialTestRequestData | undefined {
const nodeTypes = NodeTypes();
const allNodes = nodeTypes.getAll();

// Check all the nodes one by one if they have a test function defined
for (let i = 0; i < allNodes.length; i++) {
const node = allNodes[i];
// Check if test is defined on credentials
const type = this.credentialTypes.getByName(credentialType);
if (type.test) {
return {
testRequest: type.test,
};
}

if (nodeToTestWith && node.description.name !== nodeToTestWith) {
// eslint-disable-next-line no-continue
continue;
}
const nodeTypesToTestWith = this.credentialTypes.getNodeTypesToTestWith(credentialType);
for (const nodeName of nodeTypesToTestWith) {
const node = this.nodeTypes.getByName(nodeName);

// Always set to an array even if node is not versioned to not having
// to duplicate the logic
const allNodeTypes: INodeType[] = [];
if (node instanceof VersionedNodeType) {
// Node is versioned
allNodeTypes.push(...Object.values((node as IVersionedNodeType).nodeVersions));
allNodeTypes.push(...Object.values(node.nodeVersions));
} else {
// Node is not versioned
allNodeTypes.push(node as INodeType);
Expand All @@ -487,59 +502,44 @@ export class CredentialsHelper extends ICredentialsHelper {
// Check each of the node versions for credential tests
for (const nodeType of allNodeTypes) {
// Check each of teh credentials
for (const credential of nodeType.description.credentials ?? []) {
if (credential.name === credentialType && !!credential.testedBy) {
if (typeof credential.testedBy === 'string') {
if (Object.prototype.hasOwnProperty.call(node, 'nodeVersions')) {
for (const { name, testedBy } of nodeType.description.credentials ?? []) {
if (name === credentialType && !!testedBy) {
if (typeof testedBy === 'string') {
if (node instanceof VersionedNodeType) {
// The node is versioned. So check all versions for test function
// starting with the latest
const versions = Object.keys((node as IVersionedNodeType).nodeVersions)
.sort()
.reverse();
const versions = Object.keys(node.nodeVersions).sort().reverse();
for (const version of versions) {
const versionedNode = (node as IVersionedNodeType).nodeVersions[
parseInt(version, 10)
];
if (
versionedNode.methods?.credentialTest &&
versionedNode.methods?.credentialTest[credential.testedBy]
) {
return versionedNode.methods?.credentialTest[credential.testedBy];
const versionedNode = node.nodeVersions[parseInt(version, 10)];
const credentialTest = versionedNode.methods?.credentialTest;
if (credentialTest && testedBy in credentialTest) {
return credentialTest[testedBy];
}
}
}
// Test is defined as string which links to a function
return (node as unknown as INodeType).methods?.credentialTest![credential.testedBy];
return (node as unknown as INodeType).methods?.credentialTest![testedBy];
}

// Test is defined as JSON with a definition for the request to make
return {
nodeType,
testRequest: credential.testedBy,
testRequest: testedBy,
};
}
}
}
}

// Check if test is defined on credentials
const type = this.credentialTypes.getByName(credentialType);
if (type.test) {
return {
testRequest: type.test,
};
}

return undefined;
}

async testCredentials(
user: User,
credentialType: string,
credentialsDecrypted: ICredentialsDecrypted,
nodeToTestWith?: string,
): Promise<INodeCredentialTestResult> {
const credentialTestFunction = this.getCredentialTestFunction(credentialType, nodeToTestWith);
const credentialTestFunction = this.getCredentialTestFunction(credentialType);
if (credentialTestFunction === undefined) {
return Promise.resolve({
status: 'Error',
Expand Down Expand Up @@ -570,8 +570,7 @@ export class CredentialsHelper extends ICredentialsHelper {
if (credentialTestFunction.nodeType) {
nodeType = credentialTestFunction.nodeType;
} else {
const nodeTypes = NodeTypes();
nodeType = nodeTypes.getByNameAndVersion('n8n-nodes-base.noOp');
nodeType = this.nodeTypes.getByNameAndVersion('n8n-nodes-base.noOp');
}

const node: INode = {
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import type {
ITelemetrySettings,
ITelemetryTrackProperties,
IWorkflowBase as IWorkflowBaseWorkflow,
LoadingDetails,
CredentialLoadingDetails,
Workflow,
WorkflowActivateMode,
WorkflowExecuteMode,
Expand Down Expand Up @@ -59,7 +59,7 @@ export interface ICustomRequest extends Request {
}

export interface ICredentialsTypeData {
[key: string]: LoadingDetails;
[key: string]: CredentialLoadingDetails;
}

export interface ICredentialsOverwrite {
Expand Down
8 changes: 6 additions & 2 deletions packages/cli/src/LoadNodesAndCredentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,12 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {
}

for (const type in known.credentials) {
const { className, sourcePath } = known.credentials[type];
this.known.credentials[type] = { className, sourcePath: path.join(dir, sourcePath) };
const { className, sourcePath, nodesToTestWith } = known.credentials[type];
this.known.credentials[type] = {
className,
sourcePath: path.join(dir, sourcePath),
nodesToTestWith: nodesToTestWith?.map((nodeName) => `${packageName}.${nodeName}`),
};
}
}

Expand Down
8 changes: 4 additions & 4 deletions packages/cli/src/NodeTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ class NodeTypesClass implements INodeTypes {
}
}

getAll(): Array<INodeType | IVersionedNodeType> {
return Object.values(this.loadedNodes).map(({ type }) => type);
}

/**
* Variant of `getByNameAndVersion` that includes the node's source path, used to locate a node's translations.
*/
Expand All @@ -43,6 +39,10 @@ class NodeTypesClass implements INodeTypes {
return { description: { ...description }, sourcePath: nodeType.sourcePath };
}

getByName(nodeType: string): INodeType | IVersionedNodeType {
return this.getNode(nodeType).type;
}

getByNameAndVersion(nodeType: string, version?: number): INodeType {
return NodeHelpers.getVersionedNodeType(this.getNode(nodeType).type, version);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/credentials/credentials.controller.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ EECredentialsController.get(
EECredentialsController.post(
'/test',
ResponseHelper.send(async (req: CredentialRequest.Test): Promise<INodeCredentialTestResult> => {
const { credentials, nodeToTestWith } = req.body;
const { credentials } = req.body;

const encryptionKey = await EECredentials.getEncryptionKey();

Expand All @@ -122,7 +122,7 @@ EECredentialsController.post(
Object.assign(credentials, { data: decryptedData });
}

return EECredentials.test(req.user, encryptionKey, credentials, nodeToTestWith);
return EECredentials.test(req.user, encryptionKey, credentials);
}),
);

Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/credentials/credentials.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ credentialsController.get(
credentialsController.post(
'/test',
ResponseHelper.send(async (req: CredentialRequest.Test): Promise<INodeCredentialTestResult> => {
const { credentials, nodeToTestWith } = req.body;
const { credentials } = req.body;

const encryptionKey = await CredentialsService.getEncryptionKey();
return CredentialsService.test(req.user, encryptionKey, credentials, nodeToTestWith);
return CredentialsService.test(req.user, encryptionKey, credentials);
}),
);

Expand Down
7 changes: 3 additions & 4 deletions packages/cli/src/credentials/credentials.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class CredentialsService {

static async getAll(
user: User,
options?: { relations?: string[]; roles?: string[] },
options?: { relations?: string[]; roles?: string[]; disableGlobalRole?: boolean },
): Promise<ICredentialsDb[]> {
const SELECT_FIELDS: Array<keyof ICredentialsDb> = [
'id',
Expand All @@ -46,7 +46,7 @@ export class CredentialsService {

// if instance owner, return all credentials

if (user.globalRole.name === 'owner') {
if (user.globalRole.name === 'owner' && options?.disableGlobalRole !== true) {
return Db.collections.Credentials.find({
select: SELECT_FIELDS,
relations: options?.relations,
Expand Down Expand Up @@ -282,10 +282,9 @@ export class CredentialsService {
user: User,
encryptionKey: string,
credentials: ICredentialsDecrypted,
nodeToTestWith: string | undefined,
): Promise<INodeCredentialTestResult> {
const helper = new CredentialsHelper(encryptionKey);

return helper.testCredentials(user, credentials.type, credentials, nodeToTestWith);
return helper.testCredentials(user, credentials.type, credentials);
}
}
2 changes: 1 addition & 1 deletion packages/cli/src/workflows/workflows.services.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class EEWorkflowsService extends WorkflowsService {
currentUser: User,
): Promise<void> {
workflow.usedCredentials = [];
const userCredentials = await EECredentials.getAll(currentUser);
const userCredentials = await EECredentials.getAll(currentUser, { disableGlobalRole: true });
const credentialIdsUsedByWorkflow = new Set<number>();
workflow.nodes.forEach((node) => {
if (!node.credentials) {
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/test/integration/workflows.controller.ee.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ describe('GET /workflows/:id', () => {
expect(response.body.data.sharedWith).toHaveLength(0);
});

test('GET should return workflow with credentials saying owner has access even when not shared', async () => {
test('GET should return workflow with credentials saying owner does not have access when not shared', async () => {
const owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole });
const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });
Expand All @@ -351,7 +351,7 @@ describe('GET /workflows/:id', () => {
{
id: savedCredential.id.toString(),
name: savedCredential.name,
currentUserHasAccess: true, // owner has access to any cred
currentUserHasAccess: false, // although owner can see, he does not have access
},
]);

Expand Down
9 changes: 6 additions & 3 deletions packages/cli/test/unit/CredentialsHelper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {
IAuthenticateGeneric,
ICredentialDataDecryptedObject,
ICredentialType,
ICredentialTypeData,
IHttpRequestOptions,
INode,
INodeProperties,
Expand Down Expand Up @@ -234,9 +233,13 @@ describe('CredentialsHelper', () => {
},
};

CredentialTypes(mockNodesAndCredentials);
const credentialTypes = CredentialTypes(mockNodesAndCredentials);

const credentialsHelper = new CredentialsHelper(TEST_ENCRYPTION_KEY);
const credentialsHelper = new CredentialsHelper(
TEST_ENCRYPTION_KEY,
credentialTypes,
nodeTypes,
);

const result = await credentialsHelper.authenticate(
testData.input.credentials,
Expand Down
Loading

0 comments on commit fda6e8f

Please sign in to comment.