fix(core): Don't allow using credentials that are not part of the same project (#9916)

This commit is contained in:
Danny Martini 2024-07-03 11:42:59 +02:00 committed by GitHub
parent 962f0d7134
commit ab2a548856
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 99 additions and 56 deletions

View file

@ -141,7 +141,10 @@ export class EnterpriseWorkflowService {
throw new NotFoundError('Workflow not found'); throw new NotFoundError('Workflow not found');
} }
const allCredentials = await this.credentialsService.getMany(user); const allCredentials = await this.credentialsService.getCredentialsAUserCanUseInAWorkflow(
user,
{ workflowId },
);
try { try {
return this.validateWorkflowCredentialUsage(workflow, previousVersion, allCredentials); return this.validateWorkflowCredentialUsage(workflow, previousVersion, allCredentials);
@ -158,7 +161,7 @@ export class EnterpriseWorkflowService {
validateWorkflowCredentialUsage( validateWorkflowCredentialUsage(
newWorkflowVersion: WorkflowEntity, newWorkflowVersion: WorkflowEntity,
previousWorkflowVersion: WorkflowEntity, previousWorkflowVersion: WorkflowEntity,
credentialsUserHasAccessTo: CredentialsEntity[], credentialsUserHasAccessTo: Array<{ id: string }>,
) { ) {
/** /**
* We only need to check nodes that use credentials the current user cannot access, * We only need to check nodes that use credentials the current user cannot access,

View file

@ -829,64 +829,104 @@ describe('PATCH /workflows/:workflowId', () => {
expect(response.statusCode).toBe(200); expect(response.statusCode).toBe(200);
}); });
it('Should prevent member from adding node containing credential inaccessible to member', async () => { it.each([
const savedCredential = await saveCredential(randomCredentialPayload(), { user: owner }); [
'the owner and shared with the member',
'the owner',
async function creteWorkflow() {
const workflow = await createWorkflow({}, owner);
await shareWorkflowWithUsers(workflow, [member]);
return workflow;
},
async function createCredential() {
return await saveCredential(randomCredentialPayload(), { user: owner });
},
],
[
'team 1',
'the member',
async function creteWorkflow() {
const team = await createTeamProject('Team 1', member);
return await createWorkflow({}, team);
},
async function createCredential() {
return await saveCredential(randomCredentialPayload(), { user: member });
},
],
[
'team 1',
'team 2',
async function creteWorkflow() {
const team1 = await createTeamProject('Team 1', member);
return await createWorkflow({}, team1);
},
async function createCredential() {
const team2 = await createTeamProject('Team 2', member);
return await saveCredential(randomCredentialPayload(), { project: team2 });
},
],
[
'the member',
'the owner',
async function creteWorkflow() {
return await createWorkflow({}, member);
},
async function createCredential() {
return await saveCredential(randomCredentialPayload(), { user: owner });
},
],
[
'the member',
'team 2',
async function creteWorkflow() {
return await createWorkflow({}, member);
},
async function createCredential() {
const team2 = await createTeamProject('Team 2', member);
return await saveCredential(randomCredentialPayload(), { project: team2 });
},
],
])(
'Tamper proofing kicks in if the workflow is owned by %s, the credentials is owned by %s, and the member tries to use the credential in the workflow',
async (_workflowText, _credentialText, createWorkflow, createCredential) => {
//
// ARRANGE
//
const workflow = await createWorkflow();
const credential = await createCredential();
const workflow = { //
name: 'test', // ACT
active: false, //
connections: {}, const response = await authMemberAgent.patch(`/workflows/${workflow.id}`).send({
nodes: [ versionId: workflow.versionId,
{ nodes: [
id: 'uuid-1234', {
name: 'Start', id: 'uuid-12345',
parameters: {}, name: 'Start',
position: [-20, 260], parameters: {},
type: 'n8n-nodes-base.start', position: [-20, 260],
typeVersion: 1, type: 'n8n-nodes-base.start',
credentials: { typeVersion: 1,
default: { credentials: {
id: savedCredential.id, default: {
name: savedCredential.name, id: credential.id,
name: credential.name,
},
}, },
}, },
}, ],
], });
};
const createResponse = await authOwnerAgent.post('/workflows').send(workflow); //
const { id, versionId } = createResponse.body.data; // ASSERT
//
const response = await authMemberAgent.patch(`/workflows/${id}`).send({ expect(response.statusCode).toBe(400);
versionId, expect(response.body.message).toBe(
nodes: [ "You don't have access to the credentials in the 'Start' node. Ask the owner to share them with you.",
{ );
id: 'uuid-1234', },
name: 'Start', );
parameters: {},
position: [-20, 260],
type: 'n8n-nodes-base.start',
typeVersion: 1,
credentials: {},
},
{
id: 'uuid-12345',
name: 'Start',
parameters: {},
position: [-20, 260],
type: 'n8n-nodes-base.start',
typeVersion: 1,
credentials: {
default: {
id: savedCredential.id,
name: savedCredential.name,
},
},
},
],
});
expect(response.statusCode).toBe(403);
});
it('Should succeed but prevent modifying node attributes other than position, name and disabled', async () => { it('Should succeed but prevent modifying node attributes other than position, name and disabled', async () => {
const savedCredential = await saveCredential(randomCredentialPayload(), { user: member }); const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });