fix: When editing nodes only show the credentials in the dropdown that the user is allowed to use in that workflow #9718

despairblue · 2024-06-12T13:35:05Z

Summary

With the introduction of projects and RBAC we've been having issues with rendering the correct list of credentials for nodes in workflows in personal projects. We've patched GET /credentials multiple times to get it right and each time broke some edge case:
#9396
#9615
#9678

This PR introduces a new route that returns all credentials a user can use is a
specific workflow or project.

This route is used by the Node View and Node Credentials drop down.
They are not using the GET /credentials endpoint anymore, because they return

either all credentials a user have access to,
or all credentials a user have access to filtered by project.

But the credentials that a user can add to nodes in a workflow have further restrictions.
The new endpoint returns all credentials that the workflow can use when executing AND to which the user has access to.

For workflows in team project that's simple: It's only the credentials that exist in that project. Even having global read permissions does not circumvent this.

For workflows in personal projects it becomes more complex due to point to point sharing:

The y axis is the global role of the user, the x axis describes their relation ship to the workflow, either they have no relationship, the workflow was shared with them, or they own the workflow.

The new endpoint first gets all credentials the user has access to, then all the credentials the workflow can use and then returns the intersection of both.

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/PAY-1661/bug-cant-choose-the-right-credentials-in-a-personal-wf-as-an-owner-or

Review / Merge checklist

PR title and summary are descriptive. (conventions)
Tests included.
PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

…t the user is allowed to use in that workflow

krynble

Thanks a lot for addressing this, I tested all the combinations I could think of manually as well and it's all working well.

despairblue · 2024-06-13T19:25:43Z

@krynble I had to rebase this. Could you re-approve? 🙏🏾

github-actions · 2024-06-13T22:32:09Z

⚠️ Some Cypress E2E specs are failing, please fix them before merging

cypress · 2024-06-13T22:32:16Z

1 failed and 4 flaky tests on run #5473 ↗︎

1	278	0	0	4

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 despairblue 🗃️ e2e/*
Project: n8n	Commit: `71905beff0`
Status: Failed	Duration: 04:49 💡
Started: Jun 14, 2024 7:53 AM	Ended: Jun 14, 2024 7:58 AM

30-editor-after-route-changes.cy.ts • 1 failed test

View Output Video

Test		Artifacts
Editor zoom should work after route changes > after switching between Editor and Workflow history and Workflow list		`Test Replay` `Screenshots` `Video`

34-template-credentials-setup.cy.ts • 0 failed tests

View Output

Test		Artifacts

25-stickies.cy.ts • 0 failed tests

View Output

Test		Artifacts

24-ndv-paired-item.cy.ts • 0 failed tests

View Output

Test		Artifacts

42-nps-survey.cy.ts • 0 failed tests

View Output

Test		Artifacts

The first 5 failed specs are shown, see all 32 specs in Cypress Cloud.

5-ndv.cy.ts • 2 flaky tests

View Output Video

Test		Artifacts
NDV > should not retrieve remote options when required params throw errors		`Screenshots` `Video`
NDV > Stop listening for trigger event from NDV		`Screenshots` `Video`

10-undo-redo.cy.ts • 1 flaky test

View Output Video

Test		Artifacts
Undo/Redo > should undo/redo adding connected nodes		`Test Replay` `Screenshots` `Video`

20-workflow-executions.cy.ts • 1 flaky test

View Output Video

Test		Artifacts
Current Workflow Executions > should auto load more items if there is space and auto scroll		`Test Replay` `Screenshots` `Video`

Review all test suite changes for PR #9718 ↗︎

github-actions · 2024-06-14T07:58:26Z

⚠️ Some Cypress E2E specs are failing, please fix them before merging

despairblue · 2024-06-14T08:30:46Z

I'll check the failing test locally.

despairblue · 2024-06-14T09:45:04Z

cypress/e2e/30-editor-after-route-changes.cy.ts

@@ -102,7 +102,7 @@ const switchBetweenEditorAndHistory = () => {

 const switchBetweenEditorAndWorkflowlist = () => {
 	cy.getByTestId('menu-item').first().click();
-	cy.wait(['@getUsers', '@getWorkflows', '@getActiveWorkflows', '@getCredentials']);


Going to the home page does not actually load the credentials:

Maybe this worked before the new home page, but now the credentials are only loaded if you actually switch to the credentials tab.

On master this only works because the Node View calls GET /rest/credentials. With this PR that's not the case anymore.

I changed this cy.wait to do what I think the author intended, waiting until the workflow tab in the home page was completely loaded.

makes sense, ty for fixing!

despairblue · 2024-06-14T09:45:33Z

@krynble I fixed the test.

…t the user is allowed to use in that workflow (n8n-io#9718)

janober · 2024-06-20T19:25:18Z

Got released with [email protected]

janober · 2024-06-20T19:31:40Z

Got released with [email protected]

despairblue added 2 commits June 12, 2024 14:46

fix: When editing nodes only show the credentials in the dropdown tha…

d7285c1

…t the user is allowed to use in that workflow

add more docs

6231f31

despairblue added the release/backport Changes that need to be backported to older releases. label Jun 12, 2024

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 12, 2024

despairblue added 3 commits June 12, 2024 16:20

fix lints

f0fda17

reduce diff

c45139b

remove old api implementation

80bf204

despairblue force-pushed the filter-by-personal-project branch from a188f9c to 0b4459f Compare June 12, 2024 14:54

despairblue marked this pull request as ready for review June 12, 2024 14:59

krynble previously approved these changes Jun 13, 2024

View reviewed changes

despairblue dismissed krynble’s stale review via 80bf204 June 13, 2024 19:23

despairblue force-pushed the filter-by-personal-project branch from 0b4459f to 80bf204 Compare June 13, 2024 19:23

fix rebase

71905be

despairblue force-pushed the filter-by-personal-project branch from bc99637 to 71905be Compare June 13, 2024 19:24

despairblue requested a review from krynble June 13, 2024 19:25

krynble previously approved these changes Jun 13, 2024

View reviewed changes

fix test waiting on wrong request

b250e28

despairblue dismissed krynble’s stale review via b250e28 June 14, 2024 09:41

despairblue commented Jun 14, 2024

View reviewed changes

despairblue requested a review from krynble June 14, 2024 09:45

krynble approved these changes Jun 14, 2024

View reviewed changes

despairblue merged commit 2cf4364 into master Jun 14, 2024
17 of 20 checks passed

despairblue deleted the filter-by-personal-project branch June 14, 2024 12:48

adrian-martinez-onestic pushed a commit to onesdata/n8n-fork that referenced this pull request Jun 20, 2024

fix: When editing nodes only show the credentials in the dropdown tha…

e930c89

…t the user is allowed to use in that workflow (n8n-io#9718)

adrian-martinez-onestic pushed a commit to onesdata/n8n-fork that referenced this pull request Jun 20, 2024

fix: When editing nodes only show the credentials in the dropdown tha…

4bb9e9a

…t the user is allowed to use in that workflow (n8n-io#9718)

This was referenced Jun 20, 2024

🚀 Release 1.47.0 #9823

Merged

🚀 Release 1.47.0 #9826

Merged

netroy added the Released label Jun 26, 2024

despairblue mentioned this pull request Jul 3, 2024

fix(core): Allow owner and admin to edit nodes with credentials that haven't been shared with them explicitly #9922

Merged

4 tasks

despairblue mentioned this pull request Jan 2, 2025

fix(core): Only show personal credentials in the personal space #12433

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: When editing nodes only show the credentials in the dropdown that the user is allowed to use in that workflow #9718

fix: When editing nodes only show the credentials in the dropdown that the user is allowed to use in that workflow #9718

despairblue commented Jun 12, 2024 •

edited

Loading

krynble left a comment

despairblue commented Jun 13, 2024

github-actions bot commented Jun 13, 2024

cypress bot commented Jun 13, 2024 •

edited

Loading

github-actions bot commented Jun 14, 2024

despairblue commented Jun 14, 2024

despairblue Jun 14, 2024

krynble Jun 14, 2024

despairblue commented Jun 14, 2024

janober commented Jun 20, 2024

janober commented Jun 20, 2024

fix: When editing nodes only show the credentials in the dropdown that the user is allowed to use in that workflow #9718

fix: When editing nodes only show the credentials in the dropdown that the user is allowed to use in that workflow #9718

Conversation

despairblue commented Jun 12, 2024 • edited Loading

Summary

Related Linear tickets, Github issues, and Community forum posts

Review / Merge checklist

krynble left a comment

Choose a reason for hiding this comment

despairblue commented Jun 13, 2024

github-actions bot commented Jun 13, 2024

cypress bot commented Jun 13, 2024 • edited Loading

1 failed and 4 flaky tests on run #5473 ↗︎

Review all test suite changes for PR #9718 ↗︎

github-actions bot commented Jun 14, 2024

despairblue commented Jun 14, 2024

despairblue Jun 14, 2024

Choose a reason for hiding this comment

krynble Jun 14, 2024

Choose a reason for hiding this comment

despairblue commented Jun 14, 2024

janober commented Jun 20, 2024

janober commented Jun 20, 2024

despairblue commented Jun 12, 2024 •

edited

Loading

cypress bot commented Jun 13, 2024 •

edited

Loading