Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(core): Fix broken API permissions in public API (#5978)
Browse files Browse the repository at this point in the history
krynble authored and netroy committed Apr 14, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 471be3b commit b76ab31
Showing 3 changed files with 142 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -91,7 +91,7 @@ export = {

// user does not have workflows hence no executions
// or the execution he is trying to access belongs to a workflow he does not own
if (!sharedWorkflowsIds.length) {
if (!sharedWorkflowsIds.length || (workflowId && !sharedWorkflowsIds.includes(workflowId))) {
return res.status(200).json({ data: [], nextCursor: null });
}

@@ -105,7 +105,7 @@ export = {
limit,
lastId,
includeData,
...(workflowId && { workflowIds: [workflowId] }),
workflowIds: workflowId ? [workflowId] : sharedWorkflowsIds,
excludedExecutionsIds: runningExecutionsIds,
};

Original file line number Diff line number Diff line change
@@ -18,9 +18,12 @@ function insertIf(condition: boolean, elements: string[]): string[] {
}

export async function getSharedWorkflowIds(user: User): Promise<string[]> {
const where = user.globalRole.name === 'owner' ? {} : { userId: user.id };
const sharedWorkflows = await Db.collections.SharedWorkflow.find({
where: { userId: user.id },
where,
select: ['workflowId'],
});
return sharedWorkflows.map(({ workflowId }) => workflowId);

return sharedWorkflows.map(({ workflowId }) => workflowId);
}
139 changes: 136 additions & 3 deletions packages/cli/test/integration/publicApi/executions.test.ts
Original file line number Diff line number Diff line change
@@ -10,7 +10,11 @@ import * as testDb from '../shared/testDb';

let app: Application;
let owner: User;
let user1: User;
let user2: User;
let authOwnerAgent: SuperAgentTest;
let authUser1Agent: SuperAgentTest;
let authUser2Agent: SuperAgentTest;
let workflowRunner: ActiveWorkflowRunner;

beforeAll(async () => {
@@ -21,7 +25,10 @@ beforeAll(async () => {
});

const globalOwnerRole = await testDb.getGlobalOwnerRole();
const globalUserRole = await testDb.getGlobalMemberRole();
owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() });
user1 = await testDb.createUser({ globalRole: globalUserRole, apiKey: randomApiKey() });
user2 = await testDb.createUser({ globalRole: globalUserRole, apiKey: randomApiKey() });

await utils.initBinaryManager();
await utils.initNodeTypes();
@@ -46,6 +53,20 @@ beforeEach(async () => {
version: 1,
});

authUser1Agent = utils.createAgent(app, {
apiPath: 'public',
auth: true,
user: user1,
version: 1,
});

authUser2Agent = utils.createAgent(app, {
apiPath: 'public',
auth: true,
user: user2,
version: 1,
});

config.set('userManagement.disabled', false);
config.set('userManagement.isInstanceOwnerSetUp', true);
});
@@ -70,7 +91,7 @@ describe('GET /executions/:id', () => {

test('should fail due to invalid API Key', testWithAPIKey('get', '/executions/1', 'abcXYZ'));

test('should get an execution', async () => {
test('owner should be able to get an execution owned by him', async () => {
const workflow = await testDb.createWorkflow({}, owner);

const execution = await testDb.createSuccessfulExecution(workflow);
@@ -101,6 +122,46 @@ describe('GET /executions/:id', () => {
expect(workflowId).toBe(execution.workflowId);
expect(waitTill).toBeNull();
});

test('owner should be able to read executions of other users', async () => {
const workflow = await testDb.createWorkflow({}, user1);
const execution = await testDb.createSuccessfulExecution(workflow);

const response = await authOwnerAgent.get(`/executions/${execution.id}`);

expect(response.statusCode).toBe(200);
});

test('member should be able to fetch his own executions', async () => {
const workflow = await testDb.createWorkflow({}, user1);
const execution = await testDb.createSuccessfulExecution(workflow);

const response = await authUser1Agent.get(`/executions/${execution.id}`);

expect(response.statusCode).toBe(200);
});

test('member should not get an execution of another user without the workflow being shared', async () => {
const workflow = await testDb.createWorkflow({}, owner);

const execution = await testDb.createSuccessfulExecution(workflow);

const response = await authUser1Agent.get(`/executions/${execution.id}`);

expect(response.statusCode).toBe(404);
});

test('member should be able to fetch executions of workflows shared with him', async () => {
const workflow = await testDb.createWorkflow({}, user1);

const execution = await testDb.createSuccessfulExecution(workflow);

await testDb.shareWorkflowWithUsers(workflow, [user2]);

const response = await authUser2Agent.get(`/executions/${execution.id}`);

expect(response.statusCode).toBe(200);
});
});

describe('DELETE /executions/:id', () => {
@@ -324,10 +385,8 @@ describe('GET /executions', () => {
const savedExecutions = await testDb.createManyExecutions(
2,
workflow,
// @ts-ignore
testDb.createSuccessfulExecution,
);
// @ts-ignore
await testDb.createManyExecutions(2, workflow2, testDb.createSuccessfulExecution);

const response = await authOwnerAgent.get(`/executions`).query({
@@ -362,4 +421,78 @@ describe('GET /executions', () => {
expect(waitTill).toBeNull();
}
});

test('owner should retrieve all executions regardless of ownership', async () => {
const [firstWorkflowForUser1, secondWorkflowForUser1] = await testDb.createManyWorkflows(
2,
{},
user1,
);
await testDb.createManyExecutions(2, firstWorkflowForUser1, testDb.createSuccessfulExecution);
await testDb.createManyExecutions(2, secondWorkflowForUser1, testDb.createSuccessfulExecution);

const [firstWorkflowForUser2, secondWorkflowForUser2] = await testDb.createManyWorkflows(
2,
{},
user2,
);
await testDb.createManyExecutions(2, firstWorkflowForUser2, testDb.createSuccessfulExecution);
await testDb.createManyExecutions(2, secondWorkflowForUser2, testDb.createSuccessfulExecution);

const response = await authOwnerAgent.get(`/executions`);

expect(response.statusCode).toBe(200);
expect(response.body.data.length).toBe(8);
expect(response.body.nextCursor).toBe(null);
});

test('member should not see executions of workflows not shared with him', async () => {
const [firstWorkflowForUser1, secondWorkflowForUser1] = await testDb.createManyWorkflows(
2,
{},
user1,
);
await testDb.createManyExecutions(2, firstWorkflowForUser1, testDb.createSuccessfulExecution);
await testDb.createManyExecutions(2, secondWorkflowForUser1, testDb.createSuccessfulExecution);

const [firstWorkflowForUser2, secondWorkflowForUser2] = await testDb.createManyWorkflows(
2,
{},
user2,
);
await testDb.createManyExecutions(2, firstWorkflowForUser2, testDb.createSuccessfulExecution);
await testDb.createManyExecutions(2, secondWorkflowForUser2, testDb.createSuccessfulExecution);

const response = await authUser1Agent.get(`/executions`);

expect(response.statusCode).toBe(200);
expect(response.body.data.length).toBe(4);
expect(response.body.nextCursor).toBe(null);
});

test('member should also see executions of workflows shared with him', async () => {
const [firstWorkflowForUser1, secondWorkflowForUser1] = await testDb.createManyWorkflows(
2,
{},
user1,
);
await testDb.createManyExecutions(2, firstWorkflowForUser1, testDb.createSuccessfulExecution);
await testDb.createManyExecutions(2, secondWorkflowForUser1, testDb.createSuccessfulExecution);

const [firstWorkflowForUser2, secondWorkflowForUser2] = await testDb.createManyWorkflows(
2,
{},
user2,
);
await testDb.createManyExecutions(2, firstWorkflowForUser2, testDb.createSuccessfulExecution);
await testDb.createManyExecutions(2, secondWorkflowForUser2, testDb.createSuccessfulExecution);

await testDb.shareWorkflowWithUsers(firstWorkflowForUser2, [user1]);

const response = await authUser1Agent.get(`/executions`);

expect(response.statusCode).toBe(200);
expect(response.body.data.length).toBe(6);
expect(response.body.nextCursor).toBe(null);
});
});

0 comments on commit b76ab31

Please sign in to comment.