feat: Add external secrets #6477

alexgrozav · 2023-06-19T12:50:29Z

Github issue / Community forum post (link here to close automatically):

…-to-credentials-expressions

* feat: add secrets page with dummy data * chore: update api * feat: add secrets provider modal * feat: replace external-secrets dummy api call with actual REST api * fix: fix externalSecrets store name * feat: add bindings for secrets in code editor and expressions

…ub.com:n8n-io/n8n into pay-442-add-secrets-to-credentials-expressions

github-actions · 2023-06-19T12:51:01Z

Great PR! Please pay attention to the following items before merging:

Files matching packages/**:

If fixing bug, added test to cover scenario.
If addressing forum or Github issue, added link to description.

Files matching packages/**/*.ts:

Added unit tests to cover new or updated functionality.

Make sure to check off this list before asking for review.

…ub.com:n8n-io/n8n into pay-442-add-secrets-to-credentials-expressions

codecov · 2023-08-17T11:47:36Z

Codecov Report

Patch coverage: 46.20% and project coverage change: +6.90% 🎉

Comparison is base (72f65dc) 25.22% compared to head (c177788) 32.12%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6477      +/-   ##
==========================================
+ Coverage   25.22%   32.12%   +6.90%     
==========================================
  Files        3160     3183      +23     
  Lines      193299   195615    +2316     
  Branches    21236    21337     +101     
==========================================
+ Hits        48754    62842   +14088     
+ Misses     143565   131784   -11781     
- Partials      980      989       +9

Files Changed	Coverage Δ
packages/cli/src/CredentialsHelper.ts	`23.78% <0.00%> (-1.57%)`	⬇️
packages/cli/src/InternalHooks.ts	`5.70% <0.00%> (-0.06%)`	⬇️
packages/cli/src/License.ts	`54.25% <0.00%> (-1.18%)`	⬇️
packages/cli/src/Server.ts	`0.00% <0.00%> (ø)`
packages/cli/src/commands/start.ts	`0.00% <0.00%> (ø)`
packages/cli/src/commands/webhook.ts	`0.00% <0.00%> (ø)`
packages/cli/src/commands/worker.ts	`0.00% <0.00%> (ø)`
packages/cli/src/constants.ts	`100.00% <ø> (ø)`
packages/cli/src/controllers/e2e.controller.ts	`0.00% <ø> (ø)`
...ackages/cli/src/credentials/credentials.service.ts	`75.19% <0.00%> (ø)`
... and 66 more

... and 92 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

krynble

I've reviewed BE only but I've also found something that affects FE so might be worth checking with @alexgrozav how to address this.

packages/cli/src/CredentialsHelper.ts

krynble · 2023-08-21T19:51:14Z

packages/editor-ui/src/stores/externalSecrets.ee.store.ts

+	});
+
+	async function fetchAllSecrets() {
+		state.secrets = await externalSecretsApi.getExternalSecrets(rootStore.getRestApiContext);


Running n8n as non-owner is causing this endpoint to return 403, causing the UI to break when browsing /credentials

I think we discussed opening up the endpoint. Did you guys decide against that? Should I handle this on the front end instead?

packages/cli/src/ExternalSecrets/ExternalSecrets.controller.ee.ts

packages/cli/src/ExternalSecrets/providers/infisical.ts

packages/cli/src/eventbus/MessageEventBusDestination/MessageEventBusDestinationWebhook.ee.ts

mutdmour

nice. love it

mutdmour · 2023-08-23T11:49:12Z

packages/editor-ui/src/components/ExternalSecretsProviderImage.ee.vue

+const image = computed(
+	() =>
+		({
+			doppler,


is this used?

We will add doppler support sometime soon. It'll allow back end to add the integration without any front end intervention.

seems like tech debt until we do it..

packages/editor-ui/src/assets/images/doppler.webp

packages/editor-ui/src/components/CodeNodeEditor/completions/secrets.completions.ts

mutdmour · 2023-08-23T12:25:12Z

packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue

 				(accu: boolean, value: CredentialInformation) =>
-					accu || (typeof value === 'string' && value.startsWith('=')),
+					accu ||
+					(typeof value === 'string' && isExpression(value) && !isTestableExpression(value)),


hmm should this be or instead? if expression, then fail.. if has secret, then test..
anyways, neither seem to work now, when I test like this

@valya I think you worked on this part. Can you check and answer please?

It needs to be an and since the true case causes it to not be tested. Either way, it should be working now. There was a backend issue but I've sorted it now

packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue

packages/editor-ui/src/plugins/codemirror/completions/datatype.completions.ts

mutdmour · 2023-08-23T12:35:51Z

packages/editor-ui/src/plugins/codemirror/completions/datatype.completions.ts

@@ -37,12 +39,18 @@ export function datatypeCompletions(context: CompletionContext): CompletionResul

 	let options: Completion[] = [];

+	const isCredential = isCredentialsModalOpen();


can we update the completion tests? and add some e2e tests for this?

mutdmour · 2023-08-23T12:44:24Z

packages/editor-ui/src/components/ExternalSecretsProviderModal.ee.vue

+	infisical: {
+		siteURL: 'https://app.infisical.com',
+	},


why are we setting the default here if we are getting the providers from the BE?

Requested by product. I didn't like it either, but it is what it is...

Product requested that this be implemented on the FE? why cannot the backend return this?

Because the default config is blank, and we're using the blank state of the config to determine the provider connection state. I suggested using it as a fallback and making the field optional, but there was push back for it.

…-to-credentials-expressions

cypress · 2023-08-25T08:27:14Z

1 flaky tests on run #1999 ↗︎

0	232	0	0	1

Details:

🌳 pay-442-add-secrets-to-credentials-expressions 🖥️ browsers:node18.12.0-chrom...
Project: n8n	Commit: `c177788a6b`
Status: Passed	Duration: 07:56 💡
Started: Aug 25, 2023 8:19 AM	Ended: Aug 25, 2023 8:27 AM

cypress/e2e/24-ndv-paired-item.cy.ts • 1 flaky test

View Output Video

Test		Artifacts
NDV > resolves expression with default item when input node is not parent, while still pairing items		`Output` `Screenshots` `Video`

_{This comment has been generated by cypress-bot as a result of this
project's GitHub integration settings.}

github-actions · 2023-08-25T08:27:14Z

✅ All Cypress E2E specs passed

# [1.5.0](https://github.com/n8n-io/n8n/compare/n8n@1.4.0...n8n@1.5.0) (2023-08-31) ### Bug Fixes * **Agile CRM Node:** Fix issue with company address not working ([#6997](#6997)) ([2f81652](2f81652)) * **Code Node:** Switch over to vm2 fork ([#7018](#7018)) ([dfe0fa6](dfe0fa6)) * **core:** Invalid NODES_INCLUDE should not crash the app ([#7038](#7038)) ([04e3178](04e3178)), closes [#6683](#6683) * **core:** Setup websocket keep-live messages ([#6866](#6866)) ([8bdb07d](8bdb07d)), closes [#6757](#6757) * **core:** Throw `NodeSSLError` only for nodes that allow ignoring SSL issues ([#6928](#6928)) ([a01c3fb](a01c3fb)) * **Date & Time Node:** Dont parse date if it's not set (null or undefined) ([#7050](#7050)) ([d72f79f](d72f79f)) * **editor:** Fix sending of Ask AI tracking events ([#7002](#7002)) ([fb05afa](fb05afa)) * **Microsoft Excel 365 Node:** Support for more extensions in workbook rlc ([#7020](#7020)) ([d6e1cf2](d6e1cf2)) * **MongoDB Node:** Stringify response ObjectIDs ([#6990](#6990)) ([9ca990b](9ca990b)) * **MongoDB Node:** Upgrade mongodb package to address CVE-2021-32050 ([#7054](#7054)) ([d3f6356](d3f6356)) * **Postgres Node:** Empty return data fix for Postgres and MySQL ([#7016](#7016)) ([176ccd6](176ccd6)) * **Webhook Node:** Fix URL params for webhooks ([#6986](#6986)) ([596b569](596b569)) ### Features * **core:** External Secrets storage for credentials ([#6477](#6477)) ([ed927d3](ed927d3)) * **core:** Add MFA ([#4767](#4767)) ([2b7ba6f](2b7ba6f)) * **core:** Add filtering, selection and pagination to users ([#6994](#6994)) ([b716241](b716241)) * **editor:** Debug executions in the editor ([#6834](#6834)) ([c833078](c833078)) * **RSS Read Node:** Add support for self signed certificates ([#7039](#7039)) ([3b9f0fe](3b9f0fe)) Co-authored-by: netroy <netroy@users.noreply.github.com>

janober · 2023-08-31T13:29:17Z

Got released with n8n@1.5.1

romainminaud and others added 26 commits May 17, 2023 15:40

use vars in creds expressions

b7afbaa

remove console.log

ff26cfe

Merge remote-tracking branch 'origin/master' into pay-442-add-secrets…

7029369

…-to-credentials-expressions

feat: add $secrets variable for credentials

034a8e8

add some REST endpoints

9cda5d8

add secrets redacting

4275cee

add infisical secrets provider

03fe092

Merge remote-tracking branch 'origin/master' into pay-442-add-secrets…

9e1b36d

…-to-credentials-expressions

add connect and secrets list apis

c69e9e6

feat: add correct secret provider images

7eba8da

test endpoint

084f372

feat: add $secrets to credentials modal expressions

f3f7d1e

Merge branch 'pay-442-add-secrets-to-credentials-expressions' of gith…

08bb319

…ub.com:n8n-io/n8n into pay-442-add-secrets-to-credentials-expressions

feat: add external secrets connection test

5078e11

fix: update test connection data format

eb21acc

lock secrets down to owners

91a8e71

add secrets update interval

b2b16fd

chore: merge master

e7e1752

feat: add external secrets connection switch

e79f97d

return settings on connect api and return status on test api

be14436

Merge branch 'pay-442-add-secrets-to-credentials-expressions' of gith…

e2a83ff

…ub.com:n8n-io/n8n into pay-442-add-secrets-to-credentials-expressions

rename test state field

6d875a7

Merge branch 'pay-442-add-secrets-to-credentials-expressions' of gith…

4290283

…ub.com:n8n-io/n8n into pay-442-add-secrets-to-credentials-expressions

fix: fix connection testing

d0d9142

fix: add $vars to credentials expressions

3188f92

n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Jun 19, 2023

alexgrozav and others added 6 commits August 16, 2023 16:43

fix: update callout snapshot

de26f7a

remove deleted helpers file

1461d10

Merge branch 'pay-442-add-secrets-to-credentials-expressions' of gith…

c91c98f

…ub.com:n8n-io/n8n into pay-442-add-secrets-to-credentials-expressions

misc linting fixes

06eb022

fix: fix failing external secrets tests

9b9393f

Merge branch 'pay-442-add-secrets-to-credentials-expressions' of gith…

80203c9

…ub.com:n8n-io/n8n into pay-442-add-secrets-to-credentials-expressions

alexgrozav marked this pull request as ready for review August 17, 2023 11:38

krynble suggested changes Aug 21, 2023

View reviewed changes

mutdmour reviewed Aug 23, 2023

View reviewed changes

valya and others added 11 commits August 23, 2023 15:33

Merge remote-tracking branch 'origin/master' into pay-442-add-secrets…

bf553f3

…-to-credentials-expressions

hide secrets with invalid auto complete names

5fae83d

review changes

8ba8d49

test: add completion tests for secrets

3685036

fix: remove localStorage check for external secrets route

82d3b26

fix: remove console.log

65cae1c

fix: add try/catch to external secrets view

0e65ebb

fix credentials testing

580bb9e

chore: Merge master

e6fc273

fix: Remove .only from test

8856b23

chore: merge master

c177788

krynble approved these changes Aug 25, 2023

View reviewed changes

krynble merged commit ed927d3 into master Aug 25, 2023
18 checks passed

krynble deleted the pay-442-add-secrets-to-credentials-expressions branch August 25, 2023 08:33

github-actions bot mentioned this pull request Aug 31, 2023

🚀 Release 1.5.0 #7056

Merged

janober added the Released label Aug 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add external secrets #6477

feat: Add external secrets #6477

alexgrozav commented Jun 19, 2023

github-actions bot commented Jun 19, 2023

codecov bot commented Aug 17, 2023 •

edited

Loading

krynble left a comment

krynble Aug 21, 2023

alexgrozav Aug 22, 2023

mutdmour left a comment

mutdmour Aug 23, 2023

alexgrozav Aug 24, 2023

mutdmour Aug 24, 2023

mutdmour Aug 23, 2023

alexgrozav Aug 24, 2023

valya Aug 24, 2023

mutdmour Aug 23, 2023

mutdmour Aug 23, 2023

alexgrozav Aug 24, 2023

mutdmour Aug 24, 2023

alexgrozav Aug 24, 2023 •

edited

Loading

mutdmour Aug 24, 2023

cypress bot commented Aug 25, 2023

github-actions bot commented Aug 25, 2023

janober commented Aug 31, 2023

		@@ -37,12 +39,18 @@ export function datatypeCompletions(context: CompletionContext): CompletionResul

		let options: Completion[] = [];

		const isCredential = isCredentialsModalOpen();

feat: Add external secrets #6477

feat: Add external secrets #6477

Conversation

alexgrozav commented Jun 19, 2023

github-actions bot commented Jun 19, 2023

codecov bot commented Aug 17, 2023 • edited Loading

Codecov Report

krynble left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mutdmour left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexgrozav Aug 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress bot commented Aug 25, 2023

1 flaky tests on run #1999 ↗︎

github-actions bot commented Aug 25, 2023

janober commented Aug 31, 2023

codecov bot commented Aug 17, 2023 •

edited

Loading

alexgrozav Aug 24, 2023 •

edited

Loading