Skip to content

fix(cost-management): add structured audit logging with user identity - #2619

Merged
asmasarw merged 2 commits into
redhat-developer:mainfrom
hardengl:fix/audit-logging
Mar 31, 2026
Merged

fix(cost-management): add structured audit logging with user identity#2619
asmasarw merged 2 commits into
redhat-developer:mainfrom
hardengl:fix/audit-logging

Conversation

@hardengl

@hardengl hardengl commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Stacked on #2618 (which stacks on #2616) — review those PRs first. This PR adds commits on top.

Addresses FLPATH-3490 from the FLPATH-3503 security epic.

Problem

Data access events through the secure proxy and Apply Recommendation endpoint had no structured audit logging with user identity. Security teams need to be able to trace which user accessed which data, what RBAC decision was made, and what filters were applied.

Solution

Introduced structured audit logging across all secure backend endpoints:

  1. resolveActor(req, options) — Resolves the authenticated user's entity ref (e.g., user:default/admin) from the request via Backstage's UserInfoService, falling back to 'unknown' if resolution fails (with a warning log)
  2. emitAuditLog(options, entry) — Emits structured JSON log entries with standardized fields

Audit log entry format

{
  "audit": true,
  "actor": "user:default/admin",
  "action": "data_access",
  "resource": "recommendations/openshift",
  "decision": "ALLOW",
  "filters": {
    "clusters": ["cluster73"],
    "projects": ["rhdh"]
  }
}

Endpoints logged

Endpoint Action Logged fields
GET /proxy/* (Optimizations) data_access actor, resource, decision, cluster/project filters
GET /proxy/* (OpenShift/Cost) data_access actor, resource, decision, cluster/project filters
POST /apply-recommendation apply_recommendation actor, resourceType, workflowId, decision, response status
GET /access access_check actor, decision
GET /access/cost-management cost_access_check actor, decision

Changes

  • New src/util/auditLog.ts with resolveActor and emitAuditLog utilities
  • Injected coreServices.userInfo into the backend plugin
  • Added userInfo to RouterOptions interface
  • Audit log calls in secureProxy.ts, applyRecommendation.ts, access.ts, costManagementAccess.ts
  • Unit tests for resolveActor and emitAuditLog

Test plan

Unit tests

  • yarn tsc -b — clean compilation, zero errors
  • auditLog.test.ts — covers:
    • resolveActor returns entity ref from UserInfoService
    • resolveActor returns 'unknown' on failure
    • emitAuditLog formats structured JSON with all fields
    • emitAuditLog includes optional meta field
  • All other backend + common tests still pass (28 total)

CI

  • SonarQube Quality Gate: Passed

Deployment verification

  • Built and deployed to ocp-edge73 (image 2.0.3-rc.8)
  • Verified audit log entries appear in backstage pod logs
  • actor field correctly resolves to the authenticated user

Qodo bot findings (addressed)

  • Warning log on actor resolution failure (instead of silent fallback)

@rhdh-gh-app

rhdh-gh-app Bot commented Mar 26, 2026

Copy link
Copy Markdown

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

Add structured audit logging and secure server-side proxy with Apply Recommendation authorization

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add structured audit logging with user identity across secure endpoints
• Implement server-side secure proxy eliminating token exposure and RBAC bypass
• Add authorization and validation for Apply Recommendation workflow execution
• Move data fetching server-side with RBAC enforcement before API forwarding
• Add confirmation dialog to prevent accidental workflow execution
Diagram
flowchart LR
  A["Frontend Request"] -->|user-cookie| B["Backend Secure Proxy"]
  B -->|authenticate| C["Backstage httpAuth"]
  B -->|check permissions| D["Permission Framework"]
  D -->|ros.* or cost.*| E["RBAC Decision"]
  E -->|authorized clusters/projects| F["Strip Client Filters"]
  F -->|inject server filters| G["Forward to Cost Management API"]
  G -->|response| H["Return Filtered Data"]
  H -->|audit log| I["Structured JSON Log"]
  J["Apply Recommendation"] -->|POST| K["Backend Validation"]
  K -->|check ros.apply| L["Permission Check"]
  L -->|forward to Orchestrator| M["Workflow Execution"]
  M -->|audit log| I
Loading

Grey Divider

File Changes

1. workspaces/cost-management/plugins/cost-management-backend/src/models/RouterOptions.ts ⚙️ Configuration changes +6/-0

Add discovery, auth, and userInfo services to router options

workspaces/cost-management/plugins/cost-management-backend/src/models/RouterOptions.ts


2. workspaces/cost-management/plugins/cost-management-backend/src/plugin.ts ⚙️ Configuration changes +16/-3

Inject new core services and update auth policy routes

workspaces/cost-management/plugins/cost-management-backend/src/plugin.ts


3. workspaces/cost-management/plugins/cost-management-backend/src/routes/access.ts ✨ Enhancement +21/-0

Add audit logging to access check endpoint

workspaces/cost-management/plugins/cost-management-backend/src/routes/access.ts


View more (24)
4. workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts ✨ Enhancement +199/-0

New endpoint for Apply Recommendation with validation and authorization

workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts


5. workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.test.ts 🧪 Tests +210/-0

Comprehensive tests for Apply Recommendation endpoint

workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.test.ts


6. workspaces/cost-management/plugins/cost-management-backend/src/routes/costManagementAccess.ts ✨ Enhancement +21/-0

Add audit logging to cost management access endpoint

workspaces/cost-management/plugins/cost-management-backend/src/routes/costManagementAccess.ts


7. workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts ✨ Enhancement +372/-0

New server-side secure proxy with RBAC enforcement and audit logging

workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts


8. workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts ✨ Enhancement +16/-4

Register new proxy and apply-recommendation routes

workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts


9. workspaces/cost-management/plugins/cost-management-backend/src/util/auditLog.ts ✨ Enhancement +51/-0

New audit logging utilities for structured JSON entries

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


10. workspaces/cost-management/plugins/cost-management-backend/src/util/auditLog.test.ts 🧪 Tests +117/-0

Unit tests for audit logging and actor resolution

workspaces/cost-management/plugins/cost-management-backend/src/util/auditLog.test.ts


11. workspaces/cost-management/plugins/cost-management-backend/src/service/router.test.ts 🧪 Tests +3/-0

Update router tests with new service dependencies

workspaces/cost-management/plugins/cost-management-backend/src/service/router.test.ts


12. workspaces/cost-management/plugins/cost-management-backend/README.md 📝 Documentation +40/-2

Document secure proxy architecture and configuration

workspaces/cost-management/plugins/cost-management-backend/README.md


13. workspaces/cost-management/plugins/cost-management-backend/app-config.dynamic.yaml ⚙️ Configuration changes +0/-6

Remove dangerously-allow-unauthenticated proxy configuration

workspaces/cost-management/plugins/cost-management-backend/app-config.dynamic.yaml


14. workspaces/cost-management/plugins/cost-management-common/src/clients/cost-management/CostManagementSlimClient.ts ✨ Enhancement +37/-200

Route requests through backend secure proxy instead of direct API

workspaces/cost-management/plugins/cost-management-common/src/clients/cost-management/CostManagementSlimClient.ts


15. workspaces/cost-management/plugins/cost-management-common/src/clients/optimizations/OptimizationsClient.ts ✨ Enhancement +39/-128

Simplify client to route through backend proxy for frontend requests

workspaces/cost-management/plugins/cost-management-common/src/clients/optimizations/OptimizationsClient.ts


16. workspaces/cost-management/plugins/cost-management-common/src/clients/optimizations/OptimizationsClient.test.ts 🧪 Tests +9/-21

Update tests to use backend proxy base URL

workspaces/cost-management/plugins/cost-management-common/src/clients/optimizations/OptimizationsClient.test.ts


17. workspaces/cost-management/plugins/cost-management-common/src/clients/orchestrator-slim/OrchestratorSlimClient.ts ✨ Enhancement +24/-16

Route workflow execution through backend Apply Recommendation endpoint

workspaces/cost-management/plugins/cost-management-common/src/clients/orchestrator-slim/OrchestratorSlimClient.ts


18. workspaces/cost-management/plugins/cost-management-common/src/permissions.ts ✨ Enhancement +9/-0

Add ros.apply permission for workflow execution authorization

workspaces/cost-management/plugins/cost-management-common/src/permissions.ts


19. workspaces/cost-management/plugins/cost-management/src/pages/optimizations-breakdown/components/optimization-engine-tab/OptimizationEngineTab.tsx ✨ Enhancement +77/-27

Add confirmation dialog before workflow execution

workspaces/cost-management/plugins/cost-management/src/pages/optimizations-breakdown/components/optimization-engine-tab/OptimizationEngineTab.tsx


20. workspaces/cost-management/docs/rbac.md 📝 Documentation +45/-1

Document server-side RBAC enforcement and ros.apply permission

workspaces/cost-management/docs/rbac.md


21. workspaces/cost-management/docs/dynamic-plugin.md 📝 Documentation +27/-28

Remove proxy configuration and document server-side architecture

workspaces/cost-management/docs/dynamic-plugin.md


22. workspaces/cost-management/plugins/cost-management/README.md 📝 Documentation +12/-51

Remove proxy configuration from setup instructions

workspaces/cost-management/plugins/cost-management/README.md


23. workspaces/cost-management/.changeset/secure-proxy-server-side.md 📝 Documentation +13/-0

Changelog entry for secure proxy architecture changes

workspaces/cost-management/.changeset/secure-proxy-server-side.md


24. workspaces/cost-management/.changeset/apply-recommendation-auth.md 📝 Documentation +12/-0

Changelog entry for Apply Recommendation authorization

workspaces/cost-management/.changeset/apply-recommendation-auth.md


25. workspaces/cost-management/plugins/cost-management-common/report-clients.api.md 📝 Documentation +3/-2

Update API documentation for CostManagementSlimClient

workspaces/cost-management/plugins/cost-management-common/report-clients.api.md


26. workspaces/cost-management/plugins/cost-management-common/report-permissions.api.md 📝 Documentation +6/-0

Add rosApplyPermission to API documentation

workspaces/cost-management/plugins/cost-management-common/report-permissions.api.md


27. workspaces/cost-management/plugins/cost-management-common/report.api.md 📝 Documentation +1/-1

Update API documentation for CostManagementSlimClient

workspaces/cost-management/plugins/cost-management-common/report.api.md


Grey Divider

Qodo Logo

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Mar 26, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

Grey Divider


Remediation recommended

1. Proxy drops request bodies🐞 Bug ≡ Correctness
Description
secureProxy forwards the incoming HTTP method but never forwards the request body, so POST/PUT/PATCH
requests to /proxy/* will reach the upstream Cost Management API without the intended payload.
Code

workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[R347-354]

+      const upstreamResponse = await fetch(targetUrl.toString(), {
+        headers: {
+          'Content-Type': 'application/json',
+          Accept: acceptHeader,
+          Authorization: `Bearer ${token}`,
+        },
+        method: req.method,
+      });
Evidence
The router registers router.all('/proxy/*', secureProxy(...)), but the upstream fetch only sets
headers and method: req.method with no body, so non-GET requests cannot be proxied correctly.

workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts[55-58]
workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[345-354]

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

### Issue description
`secureProxy` is mounted with `router.all('/proxy/*', ...)` and forwards `req.method` upstream, but it does not forward the request body. Any client that uses POST/PUT/PATCH through this proxy will send an empty request body to the upstream API.

### Issue Context
Today most calls may be GET, but the route explicitly advertises support for all methods; the current implementation is inconsistent with that contract.

### Fix Focus Areas
- workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts[55-58]
- workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[345-354]

### Suggested fix (pick one)
**Option A (safer, likely intended):**
- Change router registration to only allow `GET` (and optionally `HEAD`).
- In handler, reject other methods with `405 Method Not Allowed`.

**Option B (full proxy behavior):**
- Forward request body for methods that can include one (POST/PUT/PATCH/DELETE).
- Forward the inbound `Content-Type` header (instead of hardcoding JSON).
- Consider adding `express.raw()` for non-JSON bodies if you need to support uploads/CSV/etc.

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


2. Malformed query yields 500🐞 Bug ☼ Reliability
Description
secureProxy uses decodeURIComponent on raw query keys/values; malformed percent-encoding throws and
is handled only by the outer catch, returning a generic 500 instead of rejecting the client request
as 400.
Code

workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[R299-307]

+      for (const param of rawParams) {
+        const eqIdx = param.indexOf('=');
+        const rawKey = eqIdx >= 0 ? param.substring(0, eqIdx) : param;
+        const rawVal = eqIdx >= 0 ? param.substring(eqIdx + 1) : '';
+        const decodedKey = decodeURIComponent(rawKey);
+
+        if (!rbacControlledKeys.has(decodedKey)) {
+          targetUrl.searchParams.append(decodedKey, decodeURIComponent(rawVal));
+        }
Evidence
The proxy decodes rawKey/rawVal with decodeURIComponent inside the main handler loop;
decodeURIComponent can throw on malformed inputs, and the only handling is the outer try/catch which
returns 500.

workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[254-308]
workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[368-371]

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

### Issue description
`secureProxy` decodes query keys/values using `decodeURIComponent`. If a client sends malformed percent-encoding (e.g. `%E0%A4`), `decodeURIComponent` throws and the handler falls into the outer `catch`, returning a 500. This should be treated as a client input error (400) to avoid noisy 500s and confusing monitoring.

### Issue Context
The decoding happens while reconstructing the query string to strip RBAC-controlled parameters.

### Fix Focus Areas
- workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[296-308]
- workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[368-371]

### Suggested fix
- Wrap `decodeURIComponent` calls for `rawKey`/`rawVal` in a small helper that catches `URIError`.
- If decoding fails, immediately return `res.status(400).json({ error: 'Invalid query encoding' })`.
- Keep the outer `catch` for genuine internal failures.

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


3. Upstream response assumed JSON🐞 Bug ☼ Reliability
Description
applyRecommendation unconditionally calls upstreamResponse.json() even when the upstream response
is an error or has a non-JSON/empty body, which can throw and cause the endpoint to return 500
instead of the upstream status.
Code

workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts[R146-156]

+      const upstreamResponse = await fetch(executeUrl, {
+        method: 'POST',
+        headers: {
+          'Content-Type': 'application/json',
+          Authorization: `Bearer ${token}`,
+        },
+        body: JSON.stringify({ inputData }),
+      });
+
+      const payload = await upstreamResponse.json();
+
Evidence
The handler parses JSON before checking upstreamResponse.ok or content-type; parse failures fall
into the catch block and are turned into a 500 Internal error executing workflow.

workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts[146-157]
workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts[193-198]

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

### Issue description
`applyRecommendation` always does `await upstreamResponse.json()`. If the upstream orchestrator returns a non-JSON error body (or an empty body), JSON parsing throws and the handler returns a generic 500, losing the upstream status and error details.

### Issue Context
This is a backend-to-backend call, but failures can still be non-JSON (reverse proxies, HTML error pages, empty responses).

### Fix Focus Areas
- workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts[146-175]
- workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts[193-198]

### Suggested fix
- Read `content-type` from `upstreamResponse.headers`.
- If JSON, parse JSON; otherwise read `text()`.
- For non-OK responses, propagate `status` and return `{ error: text }` (or best-effort JSON if parseable).
- Consider emitting an audit log entry in the `catch` path with `outcome: 'proxy_error'` so failures are auditable too.

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



Advisory comments

4. Unused token route remains🐞 Bug ⚙ Maintainability
Description
The /token endpoint was removed from the router, but the routes/token.ts implementation remains
in-tree and unreferenced, increasing maintenance surface and confusion about supported endpoints.
Code

workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts[R47-58]

  router.get('/health', (_req, res) => {
    res.json({ status: 'ok' });
  });
-  router.get('/token', getToken(options));

  router.get('/access', getAccess(options));

  router.get('/access/cost-management', getCostManagementAccess(options));

+  router.post('/apply-recommendation', applyRecommendation(options));
+
+  router.all('/proxy/*', secureProxy(options));
+
Evidence
createRouter no longer registers /token, while routes/token.ts still exports getToken (and
there are no remaining imports/registrations in the router).

workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts[47-58]
workspaces/cost-management/plugins/cost-management-backend/src/routes/token.ts[24-27]

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

### Issue description
The `/token` route is no longer registered, but `routes/token.ts` is still present and exports `getToken`. This is now dead code.

### Issue Context
The plugin’s architecture moved token handling server-side, so the explicit token endpoint appears intentionally removed.

### Fix Focus Areas
- workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts[47-58]
- workspaces/cost-management/plugins/cost-management-backend/src/routes/token.ts[1-68]

### Suggested fix
- Delete `routes/token.ts` if it’s no longer needed.
- Alternatively, add an explicit comment explaining why it remains (e.g., temporary compatibility) and add a tracking issue to remove it later.

ⓘ 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 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

hardengl and others added 2 commits March 31, 2026 10:30
- New auditLog utility resolves user identity via UserInfoService and emits
  structured JSON audit entries with actor, action, resource, decision, and filters
- All endpoints now produce audit logs: secureProxy, /access, /access/cost-management,
  and /apply-recommendation
- Inject coreServices.userInfo into the backend plugin

Fixes: FLPATH-3490
Made-with: Cursor
Add logger.warn in resolveActor catch block so operators can detect and
diagnose identity resolution failures instead of silently falling back
to 'unknown' actor.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@hardengl
hardengl force-pushed the fix/audit-logging branch from 50cb8ac to d71b8c4 Compare March 31, 2026 14:30
@sonarqubecloud

Copy link
Copy Markdown

@asmasarw
asmasarw merged commit b2fe9d7 into redhat-developer:main Mar 31, 2026
10 checks passed
PreetiW added a commit to PreetiW/rhdh-plugins that referenced this pull request Apr 2, 2026
…edhat-developer#2616-redhat-developer#2620

- Remove outdated proxy config from workspace README (PR redhat-developer#2616 moved
  data fetching server-side)
- Fix broken link typo in frontend plugin README
- Update backend README: clarify mixed dot/slash permission format,
  add missing endpoints (access, apply-recommendation), add audit
  logging section (PR redhat-developer#2619)
- Clarify permission name format in docs/rbac.md intro (dot for
  plugin-level, slash for cluster/project per PR redhat-developer#2620)
- Update ADR 0002 status to Accepted with implementation notes
  documenting the backend gateway pattern (PRs redhat-developer#2616, redhat-developer#2618, redhat-developer#2619)

Made-with: Cursor
PreetiW added a commit to PreetiW/rhdh-plugins that referenced this pull request Apr 2, 2026
…edhat-developer#2616-redhat-developer#2620

- Remove outdated proxy config from workspace README (PR redhat-developer#2616 moved
  data fetching server-side)
- Fix broken link typo in frontend plugin README
- Update backend README: clarify mixed dot/slash permission format,
  add missing endpoints (access, apply-recommendation), add audit
  logging section (PR redhat-developer#2619)
- Clarify permission name format in docs/rbac.md intro (dot for
  plugin-level, slash for cluster/project per PR redhat-developer#2620)
- Update ADR 0002 status to Accepted with implementation notes
  documenting the backend gateway pattern (PRs redhat-developer#2616, redhat-developer#2618, redhat-developer#2619)

Made-with: Cursor
PreetiW added a commit that referenced this pull request Apr 6, 2026
…2679)

* docs(cost-management): update documentation to reflect security PRs #2616-#2620

- Remove outdated proxy config from workspace README (PR #2616 moved
  data fetching server-side)
- Fix broken link typo in frontend plugin README
- Update backend README: clarify mixed dot/slash permission format,
  add missing endpoints (access, apply-recommendation), add audit
  logging section (PR #2619)
- Clarify permission name format in docs/rbac.md intro (dot for
  plugin-level, slash for cluster/project per PR #2620)
- Update ADR 0002 status to Accepted with implementation notes
  documenting the backend gateway pattern (PRs #2616, #2618, #2619)

Made-with: Cursor

* docs(cost-management): address Qodo review findings

- Fix inaccurate sanitization claim in ADR 0002: changed to
  "validated for presence and type" to match actual implementation
- Remove non-existent `cost_access_check` audit action from backend
  README — the code emits `access_check` for both access endpoints

Made-with: Cursor
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.

2 participants