Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): Fix broken API permissions in public API #5978

Merged
merged 2 commits into from
Apr 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
});
});