From 95097a8bd7ea598219c5414cad662295a0962409 Mon Sep 17 00:00:00 2001 From: Jan Oberhauser Date: Sat, 16 May 2020 19:05:40 +0200 Subject: [PATCH] :bug: Fix issue with credentials which extend others --- packages/cli/src/CredentialsHelper.ts | 42 ++++++++++--- packages/cli/src/WorkflowHelpers.ts | 60 +++++++++++++++++++ packages/cli/src/WorkflowRunner.ts | 18 +++--- .../src/components/CredentialsEdit.vue | 23 +------ packages/workflow/src/NodeHelpers.ts | 24 ++++++++ 5 files changed, 131 insertions(+), 36 deletions(-) diff --git a/packages/cli/src/CredentialsHelper.ts b/packages/cli/src/CredentialsHelper.ts index 7fb35ee576..84e368adf3 100644 --- a/packages/cli/src/CredentialsHelper.ts +++ b/packages/cli/src/CredentialsHelper.ts @@ -6,6 +6,7 @@ import { ICredentialDataDecryptedObject, ICredentialsHelper, INodeParameters, + INodeProperties, NodeHelpers, } from 'n8n-workflow'; @@ -40,6 +41,38 @@ export class CredentialsHelper extends ICredentialsHelper { } + /** + * Returns all the properties of the credentials with the given name + * + * @param {string} type The name of the type to return credentials off + * @returns {INodeProperties[]} + * @memberof CredentialsHelper + */ + getCredentialsProperties(type: string): INodeProperties[] { + const credentialTypes = CredentialTypes(); + const credentialTypeData = credentialTypes.getByName(type); + + if (credentialTypeData === undefined) { + throw new Error(`The credentials of type "${type}" are not known.`); + } + + if (credentialTypeData.extends === undefined) { + return credentialTypeData.properties; + } + + const combineProperties = [] as INodeProperties[]; + for (const credentialsTypeName of credentialTypeData.extends) { + const mergeCredentialProperties = this.getCredentialsProperties(credentialsTypeName); + NodeHelpers.mergeNodeProperties(combineProperties, mergeCredentialProperties); + } + + // The properties defined on the parent credentials take presidence + NodeHelpers.mergeNodeProperties(combineProperties, credentialTypeData.properties); + + return combineProperties; + } + + /** * Returns the decrypted credential data with applied overwrites * @@ -71,15 +104,10 @@ export class CredentialsHelper extends ICredentialsHelper { * @memberof CredentialsHelper */ applyDefaultsAndOverwrites(decryptedDataOriginal: ICredentialDataDecryptedObject, type: string): ICredentialDataDecryptedObject { - const credentialTypes = CredentialTypes(); - const credentialType = credentialTypes.getByName(type); - - if (credentialType === undefined) { - throw new Error(`The credential type "${type}" is not known and can so not be decrypted.`); - } + const credentialsProperties = this.getCredentialsProperties(type); // Add the default credential values - const decryptedData = NodeHelpers.getNodeParameters(credentialType.properties, decryptedDataOriginal as INodeParameters, true, false) as ICredentialDataDecryptedObject; + const decryptedData = NodeHelpers.getNodeParameters(credentialsProperties, decryptedDataOriginal as INodeParameters, true, false) as ICredentialDataDecryptedObject; if (decryptedDataOriginal.oauthTokenData !== undefined) { // The OAuth data gets removed as it is not defined specifically as a parameter diff --git a/packages/cli/src/WorkflowHelpers.ts b/packages/cli/src/WorkflowHelpers.ts index 90a41952ca..0f824398f4 100644 --- a/packages/cli/src/WorkflowHelpers.ts +++ b/packages/cli/src/WorkflowHelpers.ts @@ -1,5 +1,7 @@ import { + CredentialTypes, Db, + ICredentialsTypeData, ITransferNodeTypes, IWorkflowExecutionDataProcess, IWorkflowErrorData, @@ -15,6 +17,7 @@ import { IRun, IRunExecutionData, ITaskData, + IWorkflowCredentials, Workflow, } from 'n8n-workflow'; @@ -217,6 +220,63 @@ export function getNodeTypeData(nodes: INode[]): ITransferNodeTypes { +/** + * Returns the credentials data of the given type and its parent types + * it extends + * + * @export + * @param {string} type The credential type to return data off + * @returns {ICredentialsTypeData} + */ +export function getCredentialsDataWithParents(type: string): ICredentialsTypeData { + const credentialTypes = CredentialTypes(); + const credentialType = credentialTypes.getByName(type); + + const credentialTypeData: ICredentialsTypeData = {}; + credentialTypeData[type] = credentialType; + + if (credentialType === undefined || credentialType.extends === undefined) { + return credentialTypeData; + } + + for (const typeName of credentialType.extends) { + if (credentialTypeData[typeName] !== undefined) { + continue; + } + + credentialTypeData[typeName] = credentialTypes.getByName(typeName); + Object.assign(credentialTypeData, getCredentialsDataWithParents(typeName)); + } + + return credentialTypeData; +} + + + +/** + * Returns all the credentialTypes which are needed to resolve + * the given workflow credentials + * + * @export + * @param {IWorkflowCredentials} credentials The credentials which have to be able to be resolved + * @returns {ICredentialsTypeData} + */ +export function getCredentialsData(credentials: IWorkflowCredentials): ICredentialsTypeData { + const credentialTypeData: ICredentialsTypeData = {}; + + for (const credentialType of Object.keys(credentials)) { + if (credentialTypeData[credentialType] !== undefined) { + continue; + } + + Object.assign(credentialTypeData, getCredentialsDataWithParents(credentialType)); + } + + return credentialTypeData; +} + + + /** * Returns the names of the NodeTypes which are are needed * to execute the gives nodes diff --git a/packages/cli/src/WorkflowRunner.ts b/packages/cli/src/WorkflowRunner.ts index f5a54ec858..48ecff4cc0 100644 --- a/packages/cli/src/WorkflowRunner.ts +++ b/packages/cli/src/WorkflowRunner.ts @@ -179,8 +179,8 @@ export class WorkflowRunner { const executionId = this.activeExecutions.add(data, subprocess); // Check if workflow contains a "executeWorkflow" Node as in this - // case we can not know which nodeTypes will be needed and so have - // to load all of them in the workflowRunnerProcess + // case we can not know which nodeTypes and credentialTypes will + // be needed and so have to load all of them in the workflowRunnerProcess let loadAllNodeTypes = false; for (const node of data.workflowData.nodes) { if (node.type === 'n8n-nodes-base.executeWorkflow') { @@ -190,19 +190,19 @@ export class WorkflowRunner { } let nodeTypeData: ITransferNodeTypes; + let credentialTypeData: ICredentialsTypeData; + if (loadAllNodeTypes === true) { - // Supply all nodeTypes + // Supply all nodeTypes and credentialTypes nodeTypeData = WorkflowHelpers.getAllNodeTypeData(); + const credentialTypes = CredentialTypes(); + credentialTypeData = credentialTypes.credentialTypes; } else { - // Supply only nodeTypes which the workflow needs + // Supply only nodeTypes and credentialTypes which the workflow needs nodeTypeData = WorkflowHelpers.getNodeTypeData(data.workflowData.nodes); + credentialTypeData = WorkflowHelpers.getCredentialsData(data.credentials); } - const credentialTypes = CredentialTypes(); - const credentialTypeData: ICredentialsTypeData = {}; - for (const credentialType of Object.keys(data.credentials)) { - credentialTypeData[credentialType] = credentialTypes.getByName(credentialType); - } (data as unknown as IWorkflowExecutionDataProcessWithExecution).executionId = executionId; (data as unknown as IWorkflowExecutionDataProcessWithExecution).nodeTypeData = nodeTypeData; diff --git a/packages/editor-ui/src/components/CredentialsEdit.vue b/packages/editor-ui/src/components/CredentialsEdit.vue index ed67e96279..a69ad78e24 100644 --- a/packages/editor-ui/src/components/CredentialsEdit.vue +++ b/packages/editor-ui/src/components/CredentialsEdit.vue @@ -37,6 +37,7 @@ import { } from '@/Interface'; import { + NodeHelpers, ICredentialType, INodeProperties, } from 'n8n-workflow'; @@ -172,20 +173,6 @@ export default mixins( }, }, methods: { - mergeCredentialProperties (mainProperties: INodeProperties[], addProperties: INodeProperties[]): void { - let existingIndex: number; - for (const property of addProperties) { - existingIndex = mainProperties.findIndex(element => element.name === property.name); - - if (existingIndex === -1) { - // Property does not exist yet, so add - mainProperties.push(property); - } else { - // Property exists already, so overwrite - mainProperties[existingIndex] = property; - } - } - }, getCredentialProperties (name: string): INodeProperties[] { const credentialsData = this.$store.getters.credentialType(name); @@ -200,11 +187,11 @@ export default mixins( const combineProperties = [] as INodeProperties[]; for (const credentialsTypeName of credentialsData.extends) { const mergeCredentialProperties = this.getCredentialProperties(credentialsTypeName); - this.mergeCredentialProperties(combineProperties, mergeCredentialProperties); + NodeHelpers.mergeNodeProperties(combineProperties, mergeCredentialProperties); } // The properties defined on the parent credentials take presidence - this.mergeCredentialProperties(combineProperties, credentialsData.properties); + NodeHelpers.mergeNodeProperties(combineProperties, credentialsData.properties); return combineProperties; }, @@ -215,10 +202,6 @@ export default mixins( return credentialData; } - // TODO: The credential-extend-resolve-logic is currently not needed in the backend - // as the whole credential data gets saved with the defaults. That logic should, - // however, probably also get improved in the future. - // Credentials extends another one. So get the properties of the one it // extends and add them. credentialData = JSON.parse(JSON.stringify(credentialData)); diff --git a/packages/workflow/src/NodeHelpers.ts b/packages/workflow/src/NodeHelpers.ts index a72ee565de..fcaca55a0a 100644 --- a/packages/workflow/src/NodeHelpers.ts +++ b/packages/workflow/src/NodeHelpers.ts @@ -1093,3 +1093,27 @@ export function mergeIssues(destination: INodeIssues, source: INodeIssues | null destination.typeUnknown = true; } } + + + +/** + * Merges the given node properties + * + * @export + * @param {INodeProperties[]} mainProperties + * @param {INodeProperties[]} addProperties + */ +export function mergeNodeProperties(mainProperties: INodeProperties[], addProperties: INodeProperties[]): void { + let existingIndex: number; + for (const property of addProperties) { + existingIndex = mainProperties.findIndex(element => element.name === property.name); + + if (existingIndex === -1) { + // Property does not exist yet, so add + mainProperties.push(property); + } else { + // Property exists already, so overwrite + mainProperties[existingIndex] = property; + } + } +}