Skip to content

FLPATH-3236 | [Bug] OpenShift Cost Management page accessible without RBAC authorization #2614

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.

Sign up for GitHub

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

Open
asmasarw wants to merge 1 commit into
base: main
Choose a base branch
from
+42 −12
Open

FLPATH-3236 | [Bug] OpenShift Cost Management page accessible without RBAC authorization #2614

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
54 changes: 42 additions & 12 deletions workspaces/cost-management/plugins/cost-management-backend/src/util/checkPermissions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,37 @@ import {
costClusterProjectPermission,
} from '@red-hat-developer-hub/plugin-cost-management-common/permissions';

const AUTHORIZE_CHUNK_SIZE = 250;

async function authorizeInChunks(
permissionsSvc: PermissionsService,
requests: AuthorizePermissionRequest[],
credentials: Awaited<ReturnType<HttpAuthService['credentials']>>,
): Promise<AuthorizePermissionResponse[]> {
if (requests.length === 0) return [];

const decisions: AuthorizePermissionResponse[] = [];
const requestChunks = requests.reduce<AuthorizePermissionRequest[][]>(
(chunks, request, index) => {
if (index % AUTHORIZE_CHUNK_SIZE === 0) {
chunks.push([]);
}
chunks.at(-1)?.push(request);
return chunks;
},
[],
);

for (const chunk of requestChunks) {
const chunkDecisions = await permissionsSvc.authorize(chunk, {
credentials,
});
decisions.push(...chunkDecisions);
}
Comment on lines +56 to +61
Copy link
Copy Markdown

@rhdh-qodo-merge rhdh-qodo-merge bot Mar 26, 2026

Choose a reason for hiding this comment

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

Action required

1. Sequential chunk auth too slow 🐞 Bug ➹ Performance

authorizeInChunks awaits each chunk serially, so large permission sets can turn into
hundreds/thousands of sequential PermissionsService.authorize calls and make /access endpoints slow
enough to time out. This is amplified by the clustersWithoutFullAccess × allProjects
cartesian-product request building in filterAuthorizedClustersAndProjects.
Agent Prompt 
### Issue description
`authorizeInChunks` performs chunk authorization strictly sequentially. When `filterAuthorizedClustersAndProjects` generates a large number of permission requests (clustersWithoutFullAccess × allProjects), this can produce a very large number of sequential `permissionsSvc.authorize` calls, causing high latency and potential timeouts.

### Issue Context
This PR introduces chunking to avoid 413 payload errors, but chunking alone does not control end-to-end latency at scale.

### Fix Focus Areas
- workspaces/cost-management/plugins/cost-management-backend/src/util/checkPermissions.ts[35-64]
- workspaces/cost-management/plugins/cost-management-backend/src/util/checkPermissions.ts[202-241]

### Suggested fix
1. Add bounded parallelism for chunk authorization (e.g., process chunks with a small concurrency limit like 5–10) so latency is not O(number_of_chunks).
2. If possible, reduce the number of permission checks generated (avoid full cartesian product where upstream data can provide per-cluster project lists; otherwise consider adding a hard cap/guardrail and returning a controlled error/deny when inputs are too large).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


return decisions;
}

/** Permission type for cluster-level access */
export type ClusterPermissionType = 'ros' | 'cost';

Expand Down Expand Up @@ -138,11 +169,10 @@ export const filterAuthorizedClustersAndProjects = async (
return { permission: perm };
});

const clusterDecisions = await permissionsSvc.authorize(
const clusterDecisions = await authorizeInChunks(
permissionsSvc,
clusterPermissionRequests,
{
credentials,
},
credentials,
);

// Track clusters with and without full access
Expand Down Expand Up @@ -187,8 +217,7 @@ export const filterAuthorizedClustersAndProjects = async (
const clusterIdentifier =
permissionType === 'cost' ? clusterName : clusterId;

for (let j = 0; j < allProjects.length; j++) {
const projectName = allProjects[j];
for (const projectName of allProjects) {
const perm = getClusterProjectPermission(clusterName, projectName);

projectPermissionRequests[idx] = {
Expand All @@ -205,17 +234,18 @@ export const filterAuthorizedClustersAndProjects = async (
}

// Batch check project-level permissions
const projectDecisions = await permissionsSvc.authorize(
const projectDecisions = await authorizeInChunks(
permissionsSvc,
projectPermissionRequests,
{ credentials },
credentials,
);

// Process project-level results
for (let i = 0; i < projectDecisions.length; i++) {
const decision = projectDecisions[i].result;
for (const [index, decision] of projectDecisions.entries()) {
const decisionResult = decision.result;

if (decision === AuthorizeResult.ALLOW) {
const result = projectPermissionMap[i];
if (decisionResult === AuthorizeResult.ALLOW) {
const result = projectPermissionMap[index];
authorizedClusterProjects.push(result);
// Project-level permission also grants cluster access
clustersGrantedViaProjects.add(result.cluster);
Expand Down
Loading