diff --git a/workspaces/cost-management/.changeset/fix-rbac-dynamic-permissions.md b/workspaces/cost-management/.changeset/fix-rbac-dynamic-permissions.md new file mode 100644 index 0000000000..d9031a6cf4 --- /dev/null +++ b/workspaces/cost-management/.changeset/fix-rbac-dynamic-permissions.md @@ -0,0 +1,12 @@ +--- +'@red-hat-developer-hub/plugin-cost-management-backend': patch +--- + +fix: register dynamic RBAC permissions for cluster/project tiers (FLPATH-4207) + +Cluster-specific permissions (ros/, ros//) were created +at runtime but never registered with createPermissionIntegrationRouter. The RHDH +RBAC backend only evaluates registered permissions — unregistered ones get DENY by +default, breaking the 3-tier RBAC model. Now fetches cluster/project data at router +init and registers all dynamic permissions. Also improves secureProxy.ts error +messages to include request path and error details. diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts b/workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts index 66ffaa2ad9..9ec75c5627 100644 --- a/workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts +++ b/workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts @@ -409,7 +409,12 @@ export const secureProxy: (options: RouterOptions) => RequestHandler = res.set('Content-Type', contentType); return res.send(await upstreamResponse.text()); } catch (error) { - options.logger.error('Secure proxy error', error); + const errorMessage = + error instanceof Error ? error.message : String(error); + options.logger.error( + `Secure proxy error for path "${proxyPath}": ${errorMessage}`, + error instanceof Error ? { stack: error.stack } : {}, + ); if (error instanceof SsoAuthenticationError) { return res.status(502).json({ @@ -419,6 +424,8 @@ export const secureProxy: (options: RouterOptions) => RequestHandler = }); } - return res.status(500).json({ error: 'Internal proxy error' }); + return res + .status(500) + .json({ error: `Internal proxy error: ${errorMessage}` }); } }; diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/service/router.test.ts b/workspaces/cost-management/plugins/cost-management-backend/src/service/router.test.ts index 08104462e1..d8474a8a29 100644 --- a/workspaces/cost-management/plugins/cost-management-backend/src/service/router.test.ts +++ b/workspaces/cost-management/plugins/cost-management-backend/src/service/router.test.ts @@ -18,7 +18,111 @@ import express from 'express'; import request from 'supertest'; import { mockServices } from '@backstage/backend-test-utils'; -import { createRouter } from './router'; +import { + createRouter, + extractStrings, + buildClusterProjectPermissions, +} from './router'; +import type { BasicPermission } from '@backstage/plugin-permission-common'; + +describe('extractStrings', () => { + it('returns values from a fulfilled result', () => { + const result: PromiseSettledResult<{ + data?: { name: string; alias?: string }[]; + }> = { + status: 'fulfilled', + value: { + data: [ + { name: 'a', alias: 'x' }, + { name: 'b' }, + { name: 'c', alias: 'y' }, + ], + }, + }; + const out = extractStrings(result, item => item.alias); + expect(out).toEqual(new Set(['x', 'y'])); + }); + + it('returns empty set for rejected result', () => { + const result: PromiseSettledResult<{ data?: unknown[] }> = { + status: 'rejected', + reason: new Error('fail'), + }; + expect(extractStrings(result, () => 'x')).toEqual(new Set()); + }); + + it('returns empty set when data is undefined', () => { + const result: PromiseSettledResult<{ data?: unknown[] }> = { + status: 'fulfilled', + value: {}, + }; + expect(extractStrings(result, () => 'x')).toEqual(new Set()); + }); + + it('deduplicates values', () => { + const result: PromiseSettledResult<{ data?: { v: string }[] }> = { + status: 'fulfilled', + value: { data: [{ v: 'a' }, { v: 'a' }, { v: 'b' }] }, + }; + expect(extractStrings(result, i => i.v)).toEqual(new Set(['a', 'b'])); + }); + + it('skips falsy values from accessor', () => { + const result: PromiseSettledResult<{ + data?: { v: string | undefined }[]; + }> = { + status: 'fulfilled', + value: { data: [{ v: undefined }, { v: '' }, { v: 'ok' }] }, + }; + expect(extractStrings(result, i => i.v)).toEqual(new Set(['ok'])); + }); +}); + +describe('buildClusterProjectPermissions', () => { + const mockClusterFn = (c: string) => + ({ name: `ros/${c}`, type: 'basic', attributes: {} } as BasicPermission); + const mockProjectFn = (c: string, p: string) => + ({ + name: `ros/${c}/${p}`, + type: 'basic', + attributes: {}, + } as BasicPermission); + + it('builds cluster + cluster/project combinations', () => { + const perms = buildClusterProjectPermissions( + new Set(['c1', 'c2']), + new Set(['p1']), + mockClusterFn, + mockProjectFn, + ); + expect(perms.map(p => p.name)).toEqual([ + 'ros/c1', + 'ros/c1/p1', + 'ros/c2', + 'ros/c2/p1', + ]); + }); + + it('returns only cluster perms when projects is empty', () => { + const perms = buildClusterProjectPermissions( + new Set(['c1']), + new Set(), + mockClusterFn, + mockProjectFn, + ); + expect(perms.map(p => p.name)).toEqual(['ros/c1']); + }); + + it('returns empty array when clusters is empty', () => { + const perms = buildClusterProjectPermissions( + new Set(), + new Set(['p1']), + mockClusterFn, + mockProjectFn, + ); + expect(perms).toEqual([]); + }); +}); describe('createRouter', () => { let app: express.Express; diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts b/workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts index e30e631d51..15c4148b94 100644 --- a/workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts +++ b/workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts @@ -18,26 +18,143 @@ import express from 'express'; import Router from 'express-promise-router'; import type { RouterOptions } from '../models/RouterOptions'; import { createPermissionIntegrationRouter } from '@backstage/plugin-permission-node'; +import type { BasicPermission } from '@backstage/plugin-permission-common'; import { rosPluginPermissions, rosApplyPermissions, costPluginPermissions, + rosClusterSpecificPermission, + rosClusterProjectPermission, + costClusterSpecificPermission, + costClusterProjectPermission, } from '@red-hat-developer-hub/plugin-cost-management-common/permissions'; import { getAccess } from '../routes/access'; import { getCostManagementAccess } from '../routes/costManagementAccess'; import { secureProxy } from '../routes/secureProxy'; import { applyRecommendation } from '../routes/applyRecommendation'; +import { getTokenFromApi } from '../util/tokenUtil'; + +/** @internal Visible for testing */ +export function extractStrings( + result: PromiseSettledResult<{ data?: T[] }>, + accessor: (item: T) => string | undefined, +): Set { + const values = new Set(); + if (result.status !== 'fulfilled' || !result.value?.data) { + return values; + } + for (const item of result.value.data) { + const v = accessor(item); + if (v) values.add(v); + } + return values; +} + +/** @internal Visible for testing */ +export function buildClusterProjectPermissions( + clusters: Set, + projects: Set, + clusterFn: (cluster: string) => BasicPermission, + projectFn: (cluster: string, project: string) => BasicPermission, +): BasicPermission[] { + const perms: BasicPermission[] = []; + for (const cluster of clusters) { + perms.push(clusterFn(cluster)); + for (const project of projects) { + perms.push(projectFn(cluster, project)); + } + } + return perms; +} + +/** + * Fetches cluster and project data from the upstream APIs and builds + * permission objects for every ros/{cluster}, ros/{cluster}/{project}, + * cost/{cluster}, and cost/{cluster}/{project} combination. + * + * These permissions must be registered with the permission integration + * router so that the RBAC backend recognises them as valid and evaluates + * the corresponding policies instead of returning DENY by default. + */ +async function fetchDynamicPermissions( + options: RouterOptions, +): Promise { + const { logger } = options; + + try { + const token = await getTokenFromApi(options); + + const [rosData, costClusters, costProjects] = await Promise.allSettled([ + options.optimizationApi + .getRecommendationList( + { query: { limit: -1, orderHow: 'desc', orderBy: 'last_reported' } }, + { token }, + ) + .then(r => r.json()), + options.costManagementApi + .searchOpenShiftClusters('', { token, limit: 1000 }) + .then(r => r.json()), + options.costManagementApi + .searchOpenShiftProjects('', { token, limit: 1000 }) + .then(r => r.json()), + ]); + + const rosClusterNames = extractStrings(rosData, r => r.clusterAlias); + const rosProjectNames = extractStrings(rosData, r => r.project); + const costClusterNames = extractStrings( + costClusters, + (c: { cluster_alias: string }) => c.cluster_alias, + ); + const costProjectNames = extractStrings( + costProjects, + (p: { value: string }) => p.value, + ); + + const permissions = [ + ...buildClusterProjectPermissions( + rosClusterNames, + rosProjectNames, + rosClusterSpecificPermission, + rosClusterProjectPermission, + ), + ...buildClusterProjectPermissions( + costClusterNames, + costProjectNames, + costClusterSpecificPermission, + costClusterProjectPermission, + ), + ]; + + logger.info( + `Registered ${permissions.length} dynamic RBAC permissions ` + + `(${rosClusterNames.size} ROS clusters, ${costClusterNames.size} cost clusters)`, + ); + + return permissions; + } catch (error) { + logger.warn( + 'Could not fetch cluster/project data for dynamic permission registration. ' + + 'Cluster-specific RBAC permissions will not be evaluated until the next refresh.', + error instanceof Error ? { error: error.message } : {}, + ); + return []; + } +} /** @public */ export async function createRouter( options: RouterOptions, ): Promise { const router = Router(); + + const dynamicPermissions = await fetchDynamicPermissions(options); + const permissionsIntegrationRouter = createPermissionIntegrationRouter({ permissions: [ ...rosPluginPermissions, ...rosApplyPermissions, ...costPluginPermissions, + ...dynamicPermissions, ], });