Skip to content

Commit

Permalink
fix(core): Fix broken API permissions in public API (#5978)
Browse files Browse the repository at this point in the history
  • Loading branch information
krynble authored and netroy committed Apr 14, 2023
1 parent 8245508 commit f4be887
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
139 changes: 136 additions & 3 deletions packages/cli/test/integration/publicApi/executions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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();
Expand All @@ -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);
});
Expand All @@ -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);
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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 f4be887

Please sign in to comment.