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

despairblue · 2024-07-03T09:19:45Z

Summary

Previously, the owners and admins were not allowed to use credentials in workflows that they did not own. They first had to share them with themselves, and then they could use them in workflows.

With #9718 we started showing credentials in the dropdown that they did not own and haven't been shared with them, but when choosing them the node edit view would turn read only.

With this PR the node edit view now stays editable.

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/PAY-1296/bug-owner-user-cant-edit-workflow

Review / Merge checklist

PR title and summary are descriptive. (conventions)
~~Docs updated or follow-up ticket created.~~
Tests included.
~~PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)~~

packages/cli/test/integration/workflows/workflows.controller.ee.test.ts

ivov · 2024-07-05T08:22:20Z

packages/cli/test/integration/workflows/workflows.controller.ee.test.ts

@@ -428,28 +428,38 @@ describe('GET /workflows/:workflowId', () => {
 		expect(responseWorkflow.sharedWithProjects).toHaveLength(0);
 	});

-	test('should return workflow with credentials saying owner does not have access when not shared', async () => {
-		const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });
+	test.each([


This converted the existing test rather than adding to it. I'd expect we'd want to keep a test for the member scenario?

I think the next test is covering the scenario for members.

n8n/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts

Lines 464 to 499 in 7f312ab

test('should return workflow with credentials for all users with or without access', async () => {

const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });

const workflowPayload = makeWorkflow({

withPinData: false,

withCredential: { id: savedCredential.id, name: savedCredential.name },

});

const workflow = await createWorkflow(workflowPayload, member);

await shareWorkflowWithUsers(workflow, [anotherMember]);

const responseMember1 = await authMemberAgent.get(`/workflows/${workflow.id}`).expect(200);

const member1Workflow: WorkflowWithSharingsMetaDataAndCredentials = responseMember1.body.data;

expect(member1Workflow.usedCredentials).toMatchObject([

{

id: savedCredential.id,

name: savedCredential.name,

currentUserHasAccess: true, // one user has access

},

]);

expect(member1Workflow.sharedWithProjects).toHaveLength(1);

const responseMember2 = await authAnotherMemberAgent

.get(`/workflows/${workflow.id}`)

.expect(200);

const member2Workflow: WorkflowWithSharingsMetaDataAndCredentials = responseMember2.body.data;

expect(member2Workflow.usedCredentials).toMatchObject([

{

id: savedCredential.id,

name: savedCredential.name,

currentUserHasAccess: false, // the other one doesn't

},

]);

expect(member2Workflow.sharedWithProjects).toHaveLength(1);

});

This test was only checking the owner scenario.

ivov · 2024-07-05T08:24:40Z

packages/cli/test/integration/workflows/workflows.controller.ee.test.ts

-	test('should return workflow with credentials saying owner does not have access when not shared', async () => {
-		const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });
+	test.each([
+		['owner', () => owner],


For learning, why does this need to be a function? I'd've passed the actor directly.

It's evaluated before the beforeAll block and thus it would just evaluate to undefined and pass this to each, whereas using a function creates a closure to the original binding and evaluates this only when called from inside the test. At that point beforeAll already ran.

I hope that makes sense.

packages/cli/src/workflows/workflow.service.ee.ts

ivov

🙏🏻

github-actions · 2024-07-05T10:50:19Z

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

cypress · 2024-07-05T10:50:25Z

2 flaky tests on run #5849 ↗︎

0	399	0	0	2

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 despairblue 🗃️ e2e/*
Project: n8n	Commit: `26078058cb`
Status: Passed	Duration: 04:56 💡
Started: Jul 9, 2024 1:42 PM	Ended: Jul 9, 2024 1:47 PM

5-ndv.cy.ts • 1 flaky test

View Output Video

Test		Artifacts
NDV > should not retrieve remote options when required params throw errors		`Screenshots` `Video`

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		`Test Replay` `Screenshots` `Video`

Review all test suite changes for PR #9922 ↗︎

github-actions · 2024-07-08T10:59:46Z

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

github-actions · 2024-07-09T07:22:56Z

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

…haven't been shared with them explicitly

github-actions · 2024-07-09T08:18:49Z

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

github-actions · 2024-07-09T09:12:30Z

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

github-actions · 2024-07-09T13:04:02Z

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

github-actions · 2024-07-09T13:47:10Z

✅ All Cypress E2E specs passed

janober · 2024-07-10T15:57:08Z

Got released with n8n@1.50.0

…haven't been shared with them explicitly (#9922)

n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Jul 3, 2024

despairblue marked this pull request as ready for review July 3, 2024 09:44

ivov reviewed Jul 5, 2024

View reviewed changes

despairblue requested a review from ivov July 5, 2024 09:12

ivov previously approved these changes Jul 5, 2024

View reviewed changes

despairblue dismissed ivov’s stale review via c49bdd9 July 8, 2024 15:59

despairblue requested a review from ivov July 8, 2024 16:00

ivov approved these changes Jul 9, 2024

View reviewed changes

despairblue added 4 commits July 9, 2024 10:01

fix(core): Allow owner and admin to edit nodes with credentials that …

e745633

…haven't been shared with them explicitly

remove unused onlyOwn option

e9f5e63

fix typo

548cd0a

fix test

2607805

despairblue force-pushed the pay-1296-bug-owner-user-cant-edit-workflow branch from c49bdd9 to 2607805 Compare July 9, 2024 08:03

despairblue requested a review from ivov July 9, 2024 08:03

ivov approved these changes Jul 9, 2024

View reviewed changes

despairblue merged commit 0f49598 into master Jul 9, 2024
27 checks passed

despairblue deleted the pay-1296-bug-owner-user-cant-edit-workflow branch July 9, 2024 14:25

This was referenced Jul 10, 2024

🚀 Release 1.50.0 #9993

Merged

🚀 Release 1.50.0 #9998

Merged

🚀 Release 1.50.0 #9999

Merged

🚀 Release 1.50.0 #10000

Merged

janober added the Released label Jul 10, 2024

jeanpaul pushed a commit that referenced this pull request Jul 11, 2024

fix(core): Allow owner and admin to edit nodes with credentials that …

1fbe42b

…haven't been shared with them explicitly (#9922)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

despairblue commented Jul 3, 2024

ivov Jul 5, 2024

despairblue Jul 5, 2024

ivov Jul 5, 2024

despairblue Jul 5, 2024

ivov left a comment

github-actions bot commented Jul 5, 2024

cypress bot commented Jul 5, 2024 •

edited

Loading

github-actions bot commented Jul 8, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

janober commented Jul 10, 2024

	test('should return workflow with credentials for all users with or without access', async () => {
	const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });

	const workflowPayload = makeWorkflow({
	withPinData: false,
	withCredential: { id: savedCredential.id, name: savedCredential.name },
	});
	const workflow = await createWorkflow(workflowPayload, member);
	await shareWorkflowWithUsers(workflow, [anotherMember]);

	const responseMember1 = await authMemberAgent.get(`/workflows/${workflow.id}`).expect(200);
	const member1Workflow: WorkflowWithSharingsMetaDataAndCredentials = responseMember1.body.data;

	expect(member1Workflow.usedCredentials).toMatchObject([
	{
	id: savedCredential.id,
	name: savedCredential.name,
	currentUserHasAccess: true, // one user has access
	},
	]);
	expect(member1Workflow.sharedWithProjects).toHaveLength(1);

	const responseMember2 = await authAnotherMemberAgent
	.get(`/workflows/${workflow.id}`)
	.expect(200);
	const member2Workflow: WorkflowWithSharingsMetaDataAndCredentials = responseMember2.body.data;

	expect(member2Workflow.usedCredentials).toMatchObject([
	{
	id: savedCredential.id,
	name: savedCredential.name,
	currentUserHasAccess: false, // the other one doesn't
	},
	]);
	expect(member2Workflow.sharedWithProjects).toHaveLength(1);
	});

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

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

Conversation

despairblue commented Jul 3, 2024

Summary

Related Linear tickets, Github issues, and Community forum posts

Review / Merge checklist

ivov Jul 5, 2024

Choose a reason for hiding this comment

despairblue Jul 5, 2024

Choose a reason for hiding this comment

ivov Jul 5, 2024

Choose a reason for hiding this comment

despairblue Jul 5, 2024

Choose a reason for hiding this comment

ivov left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 5, 2024

cypress bot commented Jul 5, 2024 • edited Loading

2 flaky tests on run #5849 ↗︎

Review all test suite changes for PR #9922 ↗︎

github-actions bot commented Jul 8, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

janober commented Jul 10, 2024

cypress bot commented Jul 5, 2024 •

edited

Loading