Skip to content

RHINENG-25551 fix(cost-management): surface SSO auth errors instead of silent failures - #2826

Merged
PreetiW merged 2 commits into
redhat-developer:mainfrom
PreetiW:RHINENG-25551/surface-auth-errors
Apr 20, 2026
Merged

RHINENG-25551 fix(cost-management): surface SSO auth errors instead of silent failures#2826
PreetiW merged 2 commits into
redhat-developer:mainfrom
PreetiW:RHINENG-25551/surface-auth-errors

Conversation

@PreetiW

@PreetiW PreetiW commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Description

RHINENG-25551 FLPATH-3237
fix(cost-management): surface SSO auth errors instead of silent failures

When the plugin is configured with invalid hybrid cloud service account credentials, the UI previously showed a generic "Bad Gateway" or empty table instead of an actionable error message.

  • tokenUtil: extract SSO error details (error_description/error) from the response body instead of throwing bare HTTP status text
  • secureProxy: return 502 with credential-specific message when SSO authentication fails, distinguishing it from other proxy errors
  • OptimizationsClient & CostManagementSlimClient: read the error field from JSON response bodies on non-OK responses so the frontend displays the backend's descriptive message

Made-with: Cursor

Screenshots

image image

Hey, I just made a Pull Request!

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Apr 20, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Brittle auth error detection🐞 Bug ⚙ Maintainability
Description
secureProxy classifies SSO authentication failures by checking whether error.message starts with the
hard-coded prefix "SSO authentication failed", coupling proxy behavior to tokenUtil’s message
formatting. Any future change to tokenUtil’s error text can silently break this behavior and revert
to generic 500 errors.
Code

workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[R412-416]

+      const message = error instanceof Error ? error.message : String(error);
      options.logger.error('Secure proxy error', error);
+
+      if (message.startsWith('SSO authentication failed')) {
+        return res.status(502).json({
Relevance

⭐⭐⭐ High

String-prefix coupling is brittle; no evidence team prefers this over constants/error types.

PR-#2616

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
secureProxy relies on string-prefix matching to detect SSO auth failures, while tokenUtil constructs
that string inline; there is no shared constant or error type enforcing this contract at compile
time.

workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[411-416]
workspaces/cost-management/plugins/cost-management-backend/src/util/tokenUtil.ts[102-115]

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` detects SSO auth failures via `message.startsWith('SSO authentication failed')`, which is brittle and can silently break if the error message changes.

### Issue Context
`tokenUtil.getTokenFromApi` creates these messages as plain strings and throws `Error`. `secureProxy` then pattern-matches the string to decide whether to return a credential-specific 502.

### Fix Focus Areas
- workspaces/cost-management/plugins/cost-management-backend/src/util/tokenUtil.ts[102-115]
- workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[411-421]

### Suggested change
- Introduce a custom error class in the backend (e.g., `class SsoAuthenticationError extends Error { ... }`) exported from `tokenUtil.ts`.
- Throw `new SsoAuthenticationError(detail, rhSsoResponse.status)` from `getTokenFromApi`.
- In `secureProxy`, replace the string-prefix check with `if (error instanceof SsoAuthenticationError)` (or check `error.name`) to decide the 502 response path.

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



Advisory comments

2. Auth error becomes RBAC deny 🐞 Bug ≡ Correctness
Description
For Cost Management proxy paths, resolveCostManagementAccess swallows getTokenFromApi failures and
returns DENY, so secureProxy responds 403 "Access denied by RBAC policy" instead of the new 502
SSO-auth error response. This defeats the PR’s goal for cost-management endpoints because invalid
service-account credentials can be misclassified as an authorization failure.
Code

workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[R415-421]

+      if (message.startsWith('SSO authentication failed')) {
+        return res.status(502).json({
+          error:
+            'Unable to authenticate with the hybrid cloud console. ' +
+            'Please check your service account credentials (clientId/clientSecret) in the RHDH Cost Management configuration.',
+        });
+      }
Relevance

⭐ Low

Team previously chose upstream/access-fetch failures -> DENY/403; likely intentional, not a bug to
change.

PR-#2620
PR-#2616

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
secureProxy only returns the new 502 JSON error when an exception reaches its catch block and the
message matches the SSO prefix. However, for Cost Management (non-optimizations) access resolution,
getTokenFromApi errors are caught and converted to a null result, which resolveAccessForSection
turns into a DENY decision; the request then returns 403 before reaching the token fetch inside the
main handler, so the new 502 path is bypassed.

workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[191-207]
workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[71-118]
workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[326-342]
workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[411-421]

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

### Issue description
Cost Management (non-ROS) proxy requests can return a 403 RBAC denial when SSO token acquisition fails, because `resolveCostManagementAccess` converts token failures into `null` (which becomes `DENY`). This prevents the new `secureProxy` 502 auth-error response from being used.

### Issue Context
`secureProxy` added a special-case 502 response when it catches an error whose message starts with `SSO authentication failed...`. That only works if the token acquisition failure is allowed to throw up to `secureProxy`’s `catch`.

### Fix Focus Areas
- workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[191-207]
- workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[326-345]
- workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts[411-421]

### Suggested change
- In `resolveCostManagementAccess`’s `fetchData` callback, do **not** swallow `getTokenFromApi` errors. Let them throw so `secureProxy` can map them to the intended 502 response (or rethrow a dedicated auth error type).
- If you still need to return `null` for non-auth upstream failures, only catch those downstream fetch/parsing errors, not the token acquisition error.

ⓘ 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

@PreetiW
PreetiW requested a review from hardengl April 20, 2026 06:37
@rhdh-gh-app

rhdh-gh-app Bot commented Apr 20, 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
  • @red-hat-developer-hub/plugin-cost-management-common

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
@red-hat-developer-hub/plugin-cost-management-common workspaces/cost-management/plugins/cost-management-common none v2.0.1

@rhdh-qodo-merge

Copy link
Copy Markdown

Review Summary by Qodo

Surface SSO authentication errors with descriptive messages

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Extract SSO error details from response body instead of generic HTTP status text
• Return 502 with credential-specific message when SSO authentication fails
• Read error field from JSON response bodies in client classes for actionable frontend messages
Diagram
flowchart LR
  A["SSO Auth Failure"] -->|"Extract error_description/error"| B["tokenUtil"]
  B -->|"Throw descriptive error"| C["secureProxy"]
  C -->|"Return 502 with credential message"| D["Frontend"]
  E["API Response"] -->|"Read error field"| F["OptimizationsClient<br/>CostManagementSlimClient"]
  F -->|"Throw error message"| D
Loading

Grey Divider

File Changes

1. workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts Error handling +10/-0

Handle SSO auth failures with credential-specific response

• Added error message extraction from caught exceptions
• Detect SSO authentication failures by message prefix
• Return 502 status with credential-specific error message instead of generic 500 error
• Provide actionable guidance about service account credentials in error response

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


2. workspaces/cost-management/plugins/cost-management-backend/src/util/tokenUtil.ts Error handling +12/-1

Extract SSO error details from response body

• Extract error details from SSO response body (error_description or error fields)
• Construct descriptive error message including extracted details
• Fall back to HTTP status text if response is not JSON
• Log error message before throwing for better observability

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


3. workspaces/cost-management/plugins/cost-management-common/src/clients/cost-management/CostManagementSlimClient.ts Error handling +24/-3

Extract error messages from API response bodies

• Updated three error handling blocks to extract error field from JSON response bodies
• Replace generic statusText with descriptive error message from response
• Gracefully handle non-JSON responses by catching parse exceptions
• Applied consistently across multiple API methods

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


View more (1)
4. workspaces/cost-management/plugins/cost-management-common/src/clients/optimizations/OptimizationsClient.ts Error handling +16/-2

Extract error messages from API response bodies

• Updated two error handling blocks to extract error field from JSON response bodies
• Replace generic statusText with descriptive error message from response
• Gracefully handle non-JSON responses by catching parse exceptions
• Applied consistently across getRecommendationById and getRecommendationList methods

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


Grey Divider

Qodo Logo

@rhdh-qodo-merge rhdh-qodo-merge Bot added enhancement New feature or request bug_fix labels Apr 20, 2026
PreetiW added 2 commits April 20, 2026 12:48
…f silent failures

When the plugin is configured with invalid hybrid cloud service account
credentials, the UI previously showed a generic "Bad Gateway" or empty
table instead of an actionable error message.

- tokenUtil: extract SSO error details (error_description/error) from
  the response body instead of throwing bare HTTP status text
- secureProxy: return 502 with credential-specific message when SSO
  authentication fails, distinguishing it from other proxy errors
- OptimizationsClient & CostManagementSlimClient: read the error field
  from JSON response bodies on non-OK responses so the frontend
  displays the backend's descriptive message

Made-with: Cursor
…SO auth failures

Replace brittle string-prefix matching with SsoAuthenticationError
class so secureProxy detects auth failures via instanceof instead of
message.startsWith(), making the contract compile-time enforced.

Made-with: Cursor
@PreetiW
PreetiW force-pushed the RHINENG-25551/surface-auth-errors branch from 25f5733 to 4b6549c Compare April 20, 2026 07:18
@sonarqubecloud

Copy link
Copy Markdown

@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

@PreetiW
PreetiW merged commit dc43ffa into redhat-developer:main Apr 20, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants