From af1cfee001db7f6b5d39061cc52be24bfdda9ad7 Mon Sep 17 00:00:00 2001 From: shreddedbacon Date: Thu, 5 Oct 2023 10:46:42 +1100 Subject: [PATCH] refactor: change the permission on adding or updating deploytarget configs for organization owners only if project is in organization --- .../resources/deploytargetconfig/resolvers.ts | 21 ++----- services/api/src/resources/project/helpers.ts | 43 ++++++++++++++ .../api/src/resources/project/resolvers.ts | 56 ++----------------- 3 files changed, 52 insertions(+), 68 deletions(-) diff --git a/services/api/src/resources/deploytargetconfig/resolvers.ts b/services/api/src/resources/deploytargetconfig/resolvers.ts index 6b3fb93527..7e2efdc1f3 100644 --- a/services/api/src/resources/deploytargetconfig/resolvers.ts +++ b/services/api/src/resources/deploytargetconfig/resolvers.ts @@ -11,7 +11,6 @@ import { Sql as EnvironmentSql } from '../environment/sql' import { Helpers as projectHelpers } from '../project/helpers'; import { Helpers as organizationHelpers } from '../organization/helpers'; - export const getDeployTargetConfigById = async ( root, args, @@ -26,9 +25,7 @@ export const getDeployTargetConfigById = async ( // since deploytargetconfigs are associated to a project // re-use the existing `project:view` permissions check, since the same sorts of fields // are viewable by the same permissions at the project scope - await hasPermission('project', 'view', { - project: deployTargetConfig.project - }); + await projectHelpers(sqlClientPool).checkOrgProjectViewPermission(hasPermission, deployTargetConfig.project) return deployTargetConfig; }; @@ -47,9 +44,7 @@ export const getDeployTargetConfigsByProjectId: ResolverFn = async ( // since deploytargetconfigs are associated to a project // re-use the existing `project:view` permissions check, since the same sorts of fields // are viewable by the same permissions at the project scope - await hasPermission('project', 'view', { - project: pid - }); + await projectHelpers(sqlClientPool).checkOrgProjectViewPermission(hasPermission, pid) const rows = await query(sqlClientPool, Sql.selectDeployTargetConfigsByProjectId(pid)); @@ -196,9 +191,7 @@ export const addDeployTargetConfig: ResolverFn = async ( // since deploytargetconfigs are associated to a project // re-use the existing `project:update` permissions check, since the same sorts of fields // are updateable by the same permissions at the project scope - await hasPermission('project', 'update', { - project: project - }); + await projectHelpers(sqlClientPool).checkOrgProjectUpdatePermission(hasPermission, project) // check the project has an organization id, if it does, check that the organization supports the requested deploytarget await checkProjectDeployTargetByOrg(project, deployTarget, sqlClientPool) @@ -246,9 +239,7 @@ export const deleteDeployTargetConfig: ResolverFn = async ( // re-use the existing `project:update` permissions check, since the same sorts of fields // are updateable by the same permissions at the project scope // deleting a deploytargetconfig from a project is classed as updating the project - await hasPermission('project', 'update', { - project: project - }); + await projectHelpers(sqlClientPool).checkOrgProjectUpdatePermission(hasPermission, project) try { await query(sqlClientPool, 'DELETE FROM deploy_target_config WHERE id = :id', { @@ -292,9 +283,7 @@ export const updateDeployTargetConfig: ResolverFn = async ( // since deploytargetconfigs are associated to a project // re-use the existing `project:update` permissions check, since the same sorts of fields // are updateable by the same permissions at the project scope - await hasPermission('project', 'update', { - project: deployTargetConfig.project - }); + await projectHelpers(sqlClientPool).checkOrgProjectUpdatePermission(hasPermission, deployTargetConfig.project) // check the project has an organization id, if it does, check that the organization supports the requested deploytarget await checkProjectDeployTargetByOrg(deployTargetConfig.project, deployTarget, sqlClientPool) diff --git a/services/api/src/resources/project/helpers.ts b/services/api/src/resources/project/helpers.ts index 1d3d9ecb82..69c7d89e83 100644 --- a/services/api/src/resources/project/helpers.ts +++ b/services/api/src/resources/project/helpers.ts @@ -6,6 +6,47 @@ import { Sql } from './sql'; // import { logger } from '../../loggers/logger'; export const Helpers = (sqlClientPool: Pool) => { + const checkOrgProjectViewPermission = async (hasPermission, pid) => { + // helper that allows fall through of permission check + // for viewProject:organization to view:project + // this allows queries to be performed by organization owners + // then falling through to the default project view for general users + const rows = await query(sqlClientPool, Sql.selectProject(pid)); + const project = rows[0]; + if (project.organization != null) { + try { + await hasPermission('organization', 'viewProject', { + organization: project.organization + }); + // if the organization owner has permission to view project, return + return + } catch (err) { + // otherwise fall through to project view permission check + } + } + // finally check the user view:project permission + await hasPermission('project', 'view', { + project: project.id + }); + } + const checkOrgProjectUpdatePermission = async (hasPermission, pid) => { + // helper checks the permission to updateProject:organization + // or the update:project permission + const rows = await query(sqlClientPool, Sql.selectProject(pid)); + const project = rows[0]; + if (project.organization != null) { + // if the project is in an organization, only the organization owner should be able to do this + await hasPermission('organization', 'updateProject', { + organization: project.organization + }); + } else { + // if not in a project, follow the standard rbac + await hasPermission('project', 'update', { + project: project.id + }); + } + } + const aliasOpenshiftToK8s = (projects: any[]) => { return projects.map(project => { return { @@ -52,6 +93,8 @@ export const Helpers = (sqlClientPool: Pool) => { query(sqlClientPool, Sql.selectProjectsByIds(projectIds)); return { + checkOrgProjectViewPermission, + checkOrgProjectUpdatePermission, aliasOpenshiftToK8s, getProjectById, getProjectsByIds, diff --git a/services/api/src/resources/project/resolvers.ts b/services/api/src/resources/project/resolvers.ts index a925bfa370..2c8c02ceb1 100644 --- a/services/api/src/resources/project/resolvers.ts +++ b/services/api/src/resources/project/resolvers.ts @@ -124,19 +124,7 @@ export const getProjectByEnvironmentId: ResolverFn = async ( const project = withK8s[0]; - try { - await hasPermission('project', 'view', { - project: project.id - }); - } catch (err) { - // if the user hasn't got permission to view the project, but the project is in the organization - // allow the user to get the project - if (project.organization != null) { - await hasPermission('organization', 'viewProject', { - organization: project.organization - }); - } - } + await Helpers(sqlClientPool).checkOrgProjectViewPermission(hasPermission, project.id) return project; }; @@ -152,19 +140,7 @@ export const getProjectById: ResolverFn = async ( const project = withK8s[0]; - try { - await hasPermission('project', 'view', { - project: project.id - }); - } catch (err) { - // if the user hasn't got permission to view the project, but the project is in the organization - // allow the user to get the project - if (project.organization != null) { - await hasPermission('organization', 'viewProject', { - organization: project.organization - }); - } - } + await Helpers(sqlClientPool).checkOrgProjectViewPermission(hasPermission, project.id) return project; }; @@ -180,19 +156,7 @@ export const getProjectByGitUrl: ResolverFn = async ( const project = withK8s[0]; - try { - await hasPermission('project', 'view', { - project: project.id - }); - } catch (err) { - // if the user hasn't got permission to view the project, but the project is in the organization - // allow the user to get the project - if (project.organization != null) { - await hasPermission('organization', 'viewProject', { - organization: project.organization - }); - } - } + await Helpers(sqlClientPool).checkOrgProjectViewPermission(hasPermission, project.id) return project; }; @@ -211,19 +175,7 @@ export const getProjectByName: ResolverFn = async ( return null; } - try { - await hasPermission('project', 'view', { - project: project.id - }); - } catch (err) { - // if the user hasn't got permission to view the project, but the project is in the organization - // allow the user to get the project - if (project.organization != null) { - await hasPermission('organization', 'viewProject', { - organization: project.organization - }); - } - } + await Helpers(sqlClientPool).checkOrgProjectViewPermission(hasPermission, project.id) return project; };