From 73daabbd0ea860bb962e3fcc84f2a607f5a23b34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 24 Sep 2024 11:02:39 +0200 Subject: [PATCH] docs(core): Document access checks (#10929) --- .../src/credentials/credentials.service.ts | 4 +- .../cli/src/decorators/controller.registry.ts | 4 +- packages/cli/src/permissions/check-access.ts | 75 ++++++++----------- .../shared/middlewares/global.middleware.ts | 4 +- 4 files changed, 38 insertions(+), 49 deletions(-) diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 43041a84b1..dc5ab4e6c7 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -34,7 +34,7 @@ import { ExternalHooks } from '@/external-hooks'; import { validateEntity } from '@/generic-helpers'; import type { ICredentialsDb } from '@/interfaces'; import { Logger } from '@/logger'; -import { userHasScope } from '@/permissions/check-access'; +import { userHasScopes } from '@/permissions/check-access'; import type { CredentialRequest, ListQuery } from '@/requests'; import { CredentialsTester } from '@/services/credentials-tester.service'; import { OwnershipService } from '@/services/ownership.service'; @@ -598,7 +598,7 @@ export class CredentialsService { // could actually be testing the credential before saving it, so this should cover // the cases we need it for. if ( - !(await userHasScope(user, ['credential:update'], false, { credentialId: credential.id })) + !(await userHasScopes(user, ['credential:update'], false, { credentialId: credential.id })) ) { mergedCredentials.data = decryptedData; } diff --git a/packages/cli/src/decorators/controller.registry.ts b/packages/cli/src/decorators/controller.registry.ts index 41806cb958..3a22090db1 100644 --- a/packages/cli/src/decorators/controller.registry.ts +++ b/packages/cli/src/decorators/controller.registry.ts @@ -11,7 +11,7 @@ import { inProduction, RESPONSE_ERROR_MESSAGES } from '@/constants'; import { UnauthenticatedError } from '@/errors/response-errors/unauthenticated.error'; import type { BooleanLicenseFeature } from '@/interfaces'; import { License } from '@/license'; -import { userHasScope } from '@/permissions/check-access'; +import { userHasScopes } from '@/permissions/check-access'; import type { AuthenticatedRequest } from '@/requests'; import { send } from '@/response-helper'; // TODO: move `ResponseHelper.send` to this file @@ -151,7 +151,7 @@ export class ControllerRegistry { const { scope, globalOnly } = accessScope; - if (!(await userHasScope(req.user, [scope], globalOnly, req.params))) { + if (!(await userHasScopes(req.user, [scope], globalOnly, req.params))) { return res.status(403).json({ status: 'error', message: RESPONSE_ERROR_MESSAGES.MISSING_SCOPE, diff --git a/packages/cli/src/permissions/check-access.ts b/packages/cli/src/permissions/check-access.ts index deea44ca99..f4abfcc00f 100644 --- a/packages/cli/src/permissions/check-access.ts +++ b/packages/cli/src/permissions/check-access.ts @@ -10,7 +10,15 @@ import { SharedCredentialsRepository } from '@/databases/repositories/shared-cre import { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository'; import { RoleService } from '@/services/role.service'; -export const userHasScope = async ( +/** + * Check if a user has the required scopes. The check can be: + * + * - only for scopes in the user's global role, or + * - for scopes in the user's global role, else for scopes in the resource roles + * of projects including the user and the resource, else for scopes in the + * project roles in those projects. + */ +export async function userHasScopes( user: User, scopes: Scope[], globalOnly: boolean, @@ -18,15 +26,14 @@ export const userHasScope = async ( credentialId, workflowId, projectId, - }: { credentialId?: string; workflowId?: string; projectId?: string }, -): Promise => { - // Short circuit here since a global role will always have access - if (user.hasGlobalScope(scopes, { mode: 'allOf' })) { - return true; - } else if (globalOnly) { - // The above check already failed so the user doesn't have access - return false; - } + }: { credentialId?: string; workflowId?: string; projectId?: string } /* only one */, +): Promise { + if (user.hasGlobalScope(scopes, { mode: 'allOf' })) return true; + + if (globalOnly) return false; + + // Find which project roles are defined to contain the required scopes. + // Then find projects having this user and having those project roles. const roleService = Container.get(RoleService); const projectRoles = roleService.rolesWithScope('project', scopes); @@ -42,47 +49,29 @@ export const userHasScope = async ( }) ).map((p) => p.id); + // Find which resource roles are defined to contain the required scopes. + // Then find at least one of the above qualifying projects having one of + // those resource roles over the resource being checked. + if (credentialId) { - const exists = await Container.get(SharedCredentialsRepository).find({ - where: { - projectId: In(userProjectIds), - credentialsId: credentialId, - role: In(roleService.rolesWithScope('credential', scopes)), - }, + return await Container.get(SharedCredentialsRepository).existsBy({ + credentialsId: credentialId, + projectId: In(userProjectIds), + role: In(roleService.rolesWithScope('credential', scopes)), }); - - if (!exists.length) { - return false; - } - - return true; } if (workflowId) { - const exists = await Container.get(SharedWorkflowRepository).find({ - where: { - projectId: In(userProjectIds), - workflowId, - role: In(roleService.rolesWithScope('workflow', scopes)), - }, + return await Container.get(SharedWorkflowRepository).existsBy({ + workflowId, + projectId: In(userProjectIds), + role: In(roleService.rolesWithScope('workflow', scopes)), }); - - if (!exists.length) { - return false; - } - - return true; } - if (projectId) { - if (!userProjectIds.includes(projectId)) { - return false; - } - - return true; - } + if (projectId) return userProjectIds.includes(projectId); throw new ApplicationError( - "@ProjectScope decorator was used but does not have a credentialId, workflowId, or projectId in it's URL parameters. This is likely an implementation error. If you're a developer, please check you're URL is correct or that this should be using @GlobalScope.", + "`@ProjectScope` decorator was used but does not have a `credentialId`, `workflowId`, or `projectId` in its URL parameters. This is likely an implementation error. If you're a developer, please check your URL is correct or that this should be using `@GlobalScope`.", ); -}; +} diff --git a/packages/cli/src/public-api/v1/shared/middlewares/global.middleware.ts b/packages/cli/src/public-api/v1/shared/middlewares/global.middleware.ts index 8a49a48093..ed68d4761c 100644 --- a/packages/cli/src/public-api/v1/shared/middlewares/global.middleware.ts +++ b/packages/cli/src/public-api/v1/shared/middlewares/global.middleware.ts @@ -6,7 +6,7 @@ import { Container } from 'typedi'; import { FeatureNotLicensedError } from '@/errors/feature-not-licensed.error'; import type { BooleanLicenseFeature } from '@/interfaces'; import { License } from '@/license'; -import { userHasScope } from '@/permissions/check-access'; +import { userHasScopes } from '@/permissions/check-access'; import type { AuthenticatedRequest } from '@/requests'; import type { PaginatedRequest } from '../../../types'; @@ -34,7 +34,7 @@ const buildScopeMiddleware = ( params.credentialId = req.params.id; } } - if (!(await userHasScope(req.user, scopes, globalOnly, params))) { + if (!(await userHasScopes(req.user, scopes, globalOnly, params))) { return res.status(403).json({ message: 'Forbidden' }); }