Skip to content

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

Open
asmasarw wants to merge 1 commit intoredhat-developer:mainfrom
asmasarw:feature/fix-rbac-cost
Open

FLPATH-3236 | [Bug] OpenShift Cost Management page accessible without RBAC authorization#2614
asmasarw wants to merge 1 commit intoredhat-developer:mainfrom
asmasarw:feature/fix-rbac-cost

Conversation

@asmasarw
Copy link
Copy Markdown
Contributor

@asmasarw asmasarw commented Mar 26, 2026

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

User always can see OpenShift Table Data regardless of what permission he has.

Main issue is that the API returns: 413 Payload Too Large

So the Solution is to call the API in chunks and then to see what is the DICISION - ALLOW or DENY.

UPDATED SCREENSHOT after fixing API Call with No-Permissions Granted.

image

@rhdh-gh-app
Copy link
Copy Markdown

rhdh-gh-app bot commented Mar 26, 2026

Missing Changesets

The following package(s) are changed by this PR but do not have a changeset:

  • @red-hat-developer-hub/plugin-cost-management-backend

See CONTRIBUTING.md for more information about how to add changesets.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/plugin-cost-management-backend workspaces/cost-management/plugins/cost-management-backend none v2.0.2

@rhdh-qodo-merge
Copy link
Copy Markdown

Review Summary by Qodo

Fix RBAC permission checks by chunking authorization requests

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Fix RBAC permission checks failing with 413 Payload Too Large error
• Implement chunked authorization requests to handle large permission datasets
• Improve code readability with modern JavaScript patterns
• Add constant for configurable chunk size (250 requests per batch)
Diagram
flowchart LR
  A["Large Permission Request"] -->|Split into chunks| B["authorizeInChunks function"]
  B -->|Process 250 at a time| C["permissionsSvc.authorize"]
  C -->|Aggregate results| D["Combined Authorization Decisions"]
  D -->|Apply RBAC filtering| E["Authorized Clusters & Projects"]
Loading

Grey Divider

File Changes

1. workspaces/cost-management/plugins/cost-management-backend/src/util/checkPermissions.ts 🐞 Bug fix +42/-12

Implement chunked authorization to fix payload size errors

• Added authorizeInChunks helper function to split large permission requests into 250-item batches
• Replaced direct permissionsSvc.authorize calls with chunked version to prevent 413 errors
• Refactored loops to use modern JavaScript patterns (for...of, entries()) for better readability
• Improved variable naming for clarity (decision vs decisionResult)

workspaces/cost-management/plugins/cost-management-backend/src/util/checkPermissions.ts


Grey Divider

Qodo Logo

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge bot commented Mar 26, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. Sequential chunk auth too slow 🐞 Bug ➹ Performance
Description
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.
Code

workspaces/cost-management/plugins/cost-management-backend/src/util/checkPermissions.ts[R56-61]

+  for (const chunk of requestChunks) {
+    const chunkDecisions = await permissionsSvc.authorize(chunk, {
+      credentials,
+    });
+    decisions.push(...chunkDecisions);
+  }
Evidence
authorizeInChunks processes chunks sequentially via an await-in-for-loop.
filterAuthorizedClustersAndProjects builds a cartesian product of clustersWithoutFullAccess and
allProjects, so the number of permission checks can explode; costManagementAccess can fetch up to
1000 clusters and 1000 projects, which in the worst case produces up to 1,000,000 checks (≈ 4,000
chunks at size 250) that would be authorized sequentially.

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]
workspaces/cost-management/plugins/cost-management-backend/src/routes/costManagementAccess.ts[82-90]
workspaces/cost-management/plugins/cost-management-backend/src/routes/access.ts[76-87]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### 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



Remediation recommended

2. Chunk authorize errors unhandled 🐞 Bug ⛯ Reliability
Description
If any chunk authorization throws, authorizeInChunks rejects and callers don’t handle it, causing
the access endpoints to fail with an error response instead of returning a controlled DENY decision.
With chunking, the number of calls increases, raising the probability that one call fails and breaks
the entire request.
Code

workspaces/cost-management/plugins/cost-management-backend/src/util/checkPermissions.ts[R56-60]

+  for (const chunk of requestChunks) {
+    const chunkDecisions = await permissionsSvc.authorize(chunk, {
+      credentials,
+    });
+    decisions.push(...chunkDecisions);
Evidence
authorizeInChunks calls permissionsSvc.authorize without try/catch, so any thrown error aborts the
entire permission computation. The access route’s try/catch only wraps upstream API fetching; the
later call to filterAuthorizedClustersAndProjects is outside that try/catch, so errors will
propagate to the router rather than returning a DENY response body.

workspaces/cost-management/plugins/cost-management-backend/src/util/checkPermissions.ts[37-61]
workspaces/cost-management/plugins/cost-management-backend/src/routes/access.ts[70-90]
workspaces/cost-management/plugins/cost-management-backend/src/routes/access.ts[152-160]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`authorizeInChunks` does not handle errors from `permissionsSvc.authorize`. Any exception aborts the entire authorization flow and bubbles up to the HTTP handler, which can result in a 500 instead of a deterministic DENY response.

### Issue Context
This endpoint is used for RBAC decisions; when the permission backend is unavailable or rejects a request, the system should fail closed in a controlled way (deny + log), and ideally include enough context for debugging.

### Fix Focus Areas
- workspaces/cost-management/plugins/cost-management-backend/src/util/checkPermissions.ts[37-61]
- workspaces/cost-management/plugins/cost-management-backend/src/routes/access.ts[152-160]

### Suggested fix
1. Wrap each `permissionsSvc.authorize` call in try/catch and either:
  - return DENY decisions for the entire chunk (fail closed), or
  - rethrow a typed error with context that the route handler catches and converts into a consistent `{ decision: DENY, ... }` response.
2. Add contextual logging (chunk index/size and total request count) so failures are diagnosable without dumping huge payloads.

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@asmasarw asmasarw changed the title RBAC is NOT Working in OpenShift FLPATH-3236 | [Bug] OpenShift Cost Management page accessible without RBAC authorization Mar 26, 2026
@sonarqubecloud
Copy link
Copy Markdown

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

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

@asmasarw asmasarw enabled auto-merge (squash) March 26, 2026 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant