From 7998ae7cdc5b1f93a1372920bfda26293f746b3c Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Fri, 18 Nov 2022 17:59:57 +0100 Subject: [PATCH 1/2] refactor: Adjust credential endpoints permissions --- .../src/UserManagement/PermissionChecker.ts | 19 +++++++---- .../src/credentials/credentials.controller.ts | 2 +- .../src/credentials/credentials.service.ee.ts | 32 ++++++++++++++++++- .../src/credentials/credentials.service.ts | 31 ++++++++++++++---- .../src/workflows/workflows.controller.ee.ts | 15 +++++---- 5 files changed, 78 insertions(+), 21 deletions(-) diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts index 9d911343a999d..b8cfa0d1e3b72 100644 --- a/packages/cli/src/UserManagement/PermissionChecker.ts +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -26,12 +26,19 @@ export class PermissionChecker { // allow if all creds used in this workflow are a subset of // all creds accessible to users who have access to this workflow - const workflowSharings = await Db.collections.SharedWorkflow.find({ - relations: ['workflow'], - where: { workflow: { id: Number(workflow.id) } }, - }); - - const workflowUserIds = workflowSharings.map((s) => s.userId); + let workflowUserIds = [] as string[]; + + // unsaved workflows have no id, so only get credentials for current user + if (workflow.id) { + const workflowSharings = await Db.collections.SharedWorkflow.find({ + relations: ['workflow'], + where: { workflow: { id: Number(workflow.id) } }, + }); + + workflowUserIds = workflowSharings.map((s) => s.userId); + } else { + workflowUserIds = [userId]; + } const credentialSharings = await Db.collections.SharedCredentials.find({ where: { user: In(workflowUserIds) }, diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index adbc54e41eaef..ff25a5ff47941 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -36,7 +36,7 @@ credentialsController.use('/', EECredentialsController); credentialsController.get( '/', ResponseHelper.send(async (req: CredentialRequest.GetAll): Promise => { - const credentials = await CredentialsService.getAll(req.user); + const credentials = await CredentialsService.getAll(req.user, { roles: ['owner'] }); return credentials.map((credential) => { // eslint-disable-next-line no-param-reassign diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index 6fcfe64a0976f..52a5a1f4ead8d 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -1,5 +1,5 @@ /* eslint-disable no-param-reassign */ -import { DeleteResult, EntityManager, In, Not } from 'typeorm'; +import { DeleteResult, EntityManager, FindOneOptions, In, Not } from 'typeorm'; import * as Db from '@/Db'; import { RoleService } from '@/role/role.service'; import { CredentialsEntity } from '@db/entities/CredentialsEntity'; @@ -25,6 +25,36 @@ export class EECredentialsService extends CredentialsService { return { ownsCredential: true, credential }; } + /** + * Retrieve the sharing that matches a user and a credential. + */ + static async getSharing( + user: User, + credentialId: number | string, + relations: string[] = ['credentials'], + { allowGlobalOwner } = { allowGlobalOwner: true }, + ): Promise { + const options: FindOneOptions = { + where: { + credentials: { id: credentialId }, + }, + }; + + // Omit user from where if the requesting user is the global + // owner. This allows the global owner to view and delete + // credentials they don't own. + if (!allowGlobalOwner || user.globalRole.name !== 'owner') { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + options.where.user = { id: user.id }; + } + + if (relations?.length) { + options.relations = relations; + } + + return Db.collections.SharedCredentials.findOne(options); + } + static async getSharings( transaction: EntityManager, credentialId: string, diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 2b5c04a932a2c..e4ee05f962c90 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -31,7 +31,10 @@ export class CredentialsService { }); } - static async getAll(user: User, options?: { relations: string[] }): Promise { + static async getAll( + user: User, + options?: { relations?: string[]; roles?: string[] }, + ): Promise { const SELECT_FIELDS: Array = [ 'id', 'name', @@ -52,11 +55,21 @@ export class CredentialsService { // if member, return credentials owned by or shared with member - const userSharings = await Db.collections.SharedCredentials.find({ + const whereConditions: FindManyOptions = { where: { user, }, - }); + }; + + if (options?.roles?.length) { + whereConditions.where = { + ...whereConditions.where, + role: { name: In(options.roles) }, + } as FindOneOptions; + whereConditions.relations = ['role']; + } + + const userSharings = await Db.collections.SharedCredentials.find(whereConditions); return Db.collections.Credentials.find({ select: SELECT_FIELDS, @@ -77,7 +90,7 @@ export class CredentialsService { static async getSharing( user: User, credentialId: number | string, - relations: string[] | undefined = ['credentials'], + relations: string[] = ['credentials'], { allowGlobalOwner } = { allowGlobalOwner: true }, ): Promise { const options: FindOneOptions = { @@ -90,8 +103,14 @@ export class CredentialsService { // owner. This allows the global owner to view and delete // credentials they don't own. if (!allowGlobalOwner || user.globalRole.name !== 'owner') { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - options.where.user = { id: user.id }; + options.where = { + ...options.where, + user: { id: user.id }, + role: { name: 'owner' }, + } as FindOneOptions; + if (!relations.includes('role')) { + relations.push('role'); + } } if (relations?.length) { diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts index 03894b448e7ae..a59c7ac687c8d 100644 --- a/packages/cli/src/workflows/workflows.controller.ee.ts +++ b/packages/cli/src/workflows/workflows.controller.ee.ts @@ -247,13 +247,14 @@ EEWorkflowController.post( const workflow = new WorkflowEntity(); Object.assign(workflow, req.body.workflowData); - const safeWorkflow = await EEWorkflows.preventTampering( - workflow, - workflow.id.toString(), - req.user, - ); - - req.body.workflowData.nodes = safeWorkflow.nodes; + if (workflow.id !== undefined) { + const safeWorkflow = await EEWorkflows.preventTampering( + workflow, + workflow.id?.toString(), + req.user, + ); + req.body.workflowData.nodes = safeWorkflow.nodes; + } return EEWorkflows.runManually(req.body, req.user, GenericHelpers.getSessionId(req)); }), From 41571162111045a32f8e53cecdca63c242a9f2c1 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Mon, 21 Nov 2022 14:25:56 +0100 Subject: [PATCH 2/2] fix: Types and minor changes --- packages/cli/src/UserManagement/PermissionChecker.ts | 4 ++-- packages/cli/src/credentials/credentials.service.ee.ts | 5 ++--- packages/cli/src/credentials/credentials.service.ts | 2 +- packages/cli/src/workflows/workflows.controller.ee.ts | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts index b8cfa0d1e3b72..658ee0c1c03b7 100644 --- a/packages/cli/src/UserManagement/PermissionChecker.ts +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -26,9 +26,8 @@ export class PermissionChecker { // allow if all creds used in this workflow are a subset of // all creds accessible to users who have access to this workflow - let workflowUserIds = [] as string[]; + let workflowUserIds: string[] = []; - // unsaved workflows have no id, so only get credentials for current user if (workflow.id) { const workflowSharings = await Db.collections.SharedWorkflow.find({ relations: ['workflow'], @@ -37,6 +36,7 @@ export class PermissionChecker { workflowUserIds = workflowSharings.map((s) => s.userId); } else { + // unsaved workflows have no id, so only get credentials for current user workflowUserIds = [userId]; } diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index 52a5a1f4ead8d..cc2935909d87b 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -1,5 +1,5 @@ /* eslint-disable no-param-reassign */ -import { DeleteResult, EntityManager, FindOneOptions, In, Not } from 'typeorm'; +import { DeleteResult, EntityManager, FindOneOptions, In, Not, ObjectLiteral } from 'typeorm'; import * as Db from '@/Db'; import { RoleService } from '@/role/role.service'; import { CredentialsEntity } from '@db/entities/CredentialsEntity'; @@ -34,7 +34,7 @@ export class EECredentialsService extends CredentialsService { relations: string[] = ['credentials'], { allowGlobalOwner } = { allowGlobalOwner: true }, ): Promise { - const options: FindOneOptions = { + const options: FindOneOptions & { where: ObjectLiteral } = { where: { credentials: { id: credentialId }, }, @@ -44,7 +44,6 @@ export class EECredentialsService extends CredentialsService { // owner. This allows the global owner to view and delete // credentials they don't own. if (!allowGlobalOwner || user.globalRole.name !== 'owner') { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access options.where.user = { id: user.id }; } diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index e4ee05f962c90..7833b87f5b2cc 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -65,7 +65,7 @@ export class CredentialsService { whereConditions.where = { ...whereConditions.where, role: { name: In(options.roles) }, - } as FindOneOptions; + } as FindManyOptions; whereConditions.relations = ['role']; } diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts index a59c7ac687c8d..7e76cbade4149 100644 --- a/packages/cli/src/workflows/workflows.controller.ee.ts +++ b/packages/cli/src/workflows/workflows.controller.ee.ts @@ -250,7 +250,7 @@ EEWorkflowController.post( if (workflow.id !== undefined) { const safeWorkflow = await EEWorkflows.preventTampering( workflow, - workflow.id?.toString(), + workflow.id.toString(), req.user, ); req.body.workflowData.nodes = safeWorkflow.nodes;