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

feat(core): Allow filtering executions and users by project in Public API #10250

Merged
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
1 change: 1 addition & 0 deletions packages/cli/src/PublicApi/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export declare namespace ExecutionRequest {
includeData?: boolean;
workflowId?: string;
lastId?: string;
projectId?: string;
}
>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,10 @@ export = {
status = undefined,
includeData = false,
workflowId = undefined,
projectId,
} = req.query;

const sharedWorkflowsIds = await getSharedWorkflowIds(req.user, ['workflow:read']);
const sharedWorkflowsIds = await getSharedWorkflowIds(req.user, ['workflow:read'], projectId);

// user does not have workflows hence no executions
// or the execution they are trying to access belongs to a workflow they do not own
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ get:
schema:
type: string
example: '1000'
- name: projectId
in: query
required: false
explode: false
allowReserved: true
schema:
type: string
example: VmwOO9HeTEj20kxM
- $ref: '../../../../shared/spec/parameters/limit.yml'
- $ref: '../../../../shared/spec/parameters/cursor.yml'
responses:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ get:
- $ref: '../../../../shared/spec/parameters/limit.yml'
- $ref: '../../../../shared/spec/parameters/cursor.yml'
- $ref: '../schemas/parameters/includeRole.yml'
- name: projectId
in: query
required: false
explode: false
allowReserved: true
schema:
type: string
example: VmwOO9HeTEj20kxM
responses:
'200':
description: Operation successful.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
} from '../../shared/middlewares/global.middleware';
import type { UserRequest } from '@/requests';
import { InternalHooks } from '@/InternalHooks';
import { ProjectRelationRepository } from '@/databases/repositories/projectRelation.repository';
import type { Response } from 'express';
import { InvitationController } from '@/controllers/invitation.controller';
import { UsersController } from '@/controllers/users.controller';
Expand Down Expand Up @@ -51,12 +52,17 @@ export = {
validCursor,
globalScope(['user:list', 'user:read']),
async (req: UserRequest.Get, res: express.Response) => {
const { offset = 0, limit = 100, includeRole = false } = req.query;
const { offset = 0, limit = 100, includeRole = false, projectId } = req.query;

const _in = projectId
? await Container.get(ProjectRelationRepository).findUserIdsByProjectId(projectId)
: undefined;

const [users, count] = await getAllUsersAndCount({
includeRole,
limit,
offset,
in: _in,
});

const telemetryData = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { UserRepository } from '@db/repositories/user.repository';
import type { User } from '@db/entities/User';
import pick from 'lodash/pick';
import { validate as uuidValidate } from 'uuid';
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
import { In } from '@n8n/typeorm';

export async function getUser(data: {
withIdentifier: string;
Expand All @@ -25,9 +27,12 @@ export async function getAllUsersAndCount(data: {
includeRole?: boolean;
limit?: number;
offset?: number;
in?: string[];
}): Promise<[User[], number]> {
const { in: _in } = data;

const users = await Container.get(UserRepository).find({
where: {},
where: { ...(_in && { id: In(_in) }) },
skip: data.offset,
take: data.limit,
});
Comment on lines 34 to 38
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eventually we should move all pagination code like this into a base repository, and make every "get-many" endpoint use the same code. we have more versions of pagination code than I'd like us to have.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,21 @@ function insertIf(condition: boolean, elements: string[]): string[] {
return condition ? elements : [];
}

export async function getSharedWorkflowIds(user: User, scopes: Scope[]): Promise<string[]> {
export async function getSharedWorkflowIds(
user: User,
scopes: Scope[],
projectId?: string,
): Promise<string[]> {
if (Container.get(License).isSharingEnabled()) {
return await Container.get(WorkflowSharingService).getSharedWorkflowIds(user, {
scopes,
projectId,
});
} else {
return await Container.get(WorkflowSharingService).getSharedWorkflowIds(user, {
workflowRoles: ['workflow:owner'],
projectRoles: ['project:personalOwner'],
projectId,
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,13 @@ export class ProjectRelationRepository extends Repository<ProjectRelation> {
{} as Record<ProjectRole, number>,
);
}

async findUserIdsByProjectId(projectId: string): Promise<string[]> {
const rows = await this.find({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should update .find to accept a distinct: boolean to do the deduplication on the DB itself, and not in the app.

select: ['userId'],
where: { projectId },
});

return [...new Set(rows.map((r) => r.userId))];
}
}
2 changes: 1 addition & 1 deletion packages/cli/src/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ export declare namespace UserRequest {
{ id: string; email: string; identifier: string },
{},
{},
{ limit?: number; offset?: number; cursor?: string; includeRole?: boolean }
{ limit?: number; offset?: number; cursor?: string; includeRole?: boolean; projectId?: string }
>;

export type PasswordResetLink = AuthenticatedRequest<{ id: string }, {}, {}, {}>;
Expand Down
11 changes: 8 additions & 3 deletions packages/cli/src/workflows/workflowSharing.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,16 @@ export class WorkflowSharingService {
async getSharedWorkflowIds(
user: User,
options:
| { scopes: Scope[] }
| { projectRoles: ProjectRole[]; workflowRoles: WorkflowSharingRole[] },
| { scopes: Scope[]; projectId?: string }
| { projectRoles: ProjectRole[]; workflowRoles: WorkflowSharingRole[]; projectId?: string },
): Promise<string[]> {
const { projectId } = options;

if (user.hasGlobalScope('workflow:read')) {
const sharedWorkflows = await this.sharedWorkflowRepository.find({ select: ['workflowId'] });
const sharedWorkflows = await this.sharedWorkflowRepository.find({
select: ['workflowId'],
...(projectId && { where: { projectId } }),
});
return sharedWorkflows.map(({ workflowId }) => workflowId);
}

Expand Down
38 changes: 38 additions & 0 deletions packages/cli/test/integration/publicApi/executions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
import type { SuperAgentTest } from '../shared/types';
import { mockInstance } from '@test/mocking';
import { Telemetry } from '@/telemetry';
import { createTeamProject } from '@test-integration/db/projects';
import type { ExecutionEntity } from '@/databases/entities/ExecutionEntity';

let owner: User;
let user1: User;
Expand Down Expand Up @@ -447,6 +449,42 @@ describe('GET /executions', () => {
}
});

test('should return executions filtered by project ID', async () => {
/**
* Arrange
*/
const [firstProject, secondProject] = await Promise.all([
createTeamProject(),
createTeamProject(),
]);
const [firstWorkflow, secondWorkflow] = await Promise.all([
createWorkflow({}, firstProject),
createWorkflow({}, secondProject),
]);
const [firstExecution, secondExecution, _] = await Promise.all([
createExecution({}, firstWorkflow),
createExecution({}, firstWorkflow),
createExecution({}, secondWorkflow),
]);

/**
* Act
*/
const response = await authOwnerAgent.get('/executions').query({
projectId: firstProject.id,
});

/**
* Assert
*/
expect(response.statusCode).toBe(200);
expect(response.body.data.length).toBe(2);
expect(response.body.nextCursor).toBeNull();
expect(response.body.data.map((execution: ExecutionEntity) => execution.id)).toEqual(
expect.arrayContaining([firstExecution.id, secondExecution.id]),
);
});

test('owner should retrieve all executions regardless of ownership', async () => {
const [firstWorkflowForUser1, secondWorkflowForUser1] = await createManyWorkflows(2, {}, user1);
await createManyExecutions(2, firstWorkflowForUser1, createSuccessfulExecution);
Expand Down
44 changes: 43 additions & 1 deletion packages/cli/test/integration/publicApi/users.ee.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import { mockInstance } from '../../shared/mocking';
import { randomApiKey } from '../shared/random';
import * as utils from '../shared/utils/';
import * as testDb from '../shared/testDb';
import { createUser, createUserShell } from '../shared/db/users';
import { createOwner, createUser, createUserShell } from '../shared/db/users';
import type { SuperAgentTest } from '../shared/types';
import { createTeamProject, linkUserToProject } from '@test-integration/db/projects';
import type { User } from '@/databases/entities/User';

mockInstance(License, {
getUsersLimit: jest.fn().mockReturnValue(-1),
Expand Down Expand Up @@ -84,6 +86,46 @@ describe('With license unlimited quota:users', () => {
expect(updatedAt).toBeDefined();
}
});

it('should return users filtered by project ID', async () => {
/**
* Arrange
*/
const [owner, firstMember, secondMember, thirdMember] = await Promise.all([
createOwner({ withApiKey: true }),
createUser({ role: 'global:member' }),
createUser({ role: 'global:member' }),
createUser({ role: 'global:member' }),
]);

const [firstProject, secondProject] = await Promise.all([
createTeamProject(),
createTeamProject(),
]);

await Promise.all([
linkUserToProject(firstMember, firstProject, 'project:admin'),
linkUserToProject(secondMember, firstProject, 'project:viewer'),
linkUserToProject(thirdMember, secondProject, 'project:admin'),
]);

/**
* Act
*/
const response = await testServer.publicApiAgentFor(owner).get('/users').query({
projectId: firstProject.id,
});

/**
* Assert
*/
expect(response.status).toBe(200);
expect(response.body.data.length).toBe(2);
expect(response.body.nextCursor).toBeNull();
expect(response.body.data.map((user: User) => user.id)).toEqual(
expect.arrayContaining([firstMember.id, secondMember.id]),
);
});
});

describe('GET /users/:id', () => {
Expand Down
Loading