Skip to content

fix: forward cookie header in createServerApiClient for oauth2-proxy - #6750

Open
naveenkumarsp wants to merge 4 commits into
keephq:mainfrom
naveenkumarsp:fix/oauth2-proxy-cookie-forwarding
Open

naveenkumarsp wants to merge 4 commits into
keephq:mainfrom
naveenkumarsp:fix/oauth2-proxy-cookie-forwarding

Conversation

@naveenkumarsp

Copy link
Copy Markdown

Summary

When using AUTH_TYPE=OAUTH2PROXY, server-side rendered (SSR) components make internal requests to the Keep backend via createServerApiClient().

Currently, createServerApiClient() forwards only identity headers (x-forwarded-user, x-forwarded-email, x-forwarded-groups, x-forwarded-access-token). In deployments where reverse proxies or sidecars independently authenticate inbound backend requests with session validation, internal server-to-server calls fail with 401 Unauthorized / 302 Redirect because the session cookie header is dropped.

This causes SSR pages like /incidents and /alerts/feed (which preload initial facets using Server Components) to fail during server rendering.

Changes

  • Forward the incoming request's cookie header in createServerApiClient() when AUTH_TYPE === AuthType.OAUTH2PROXY.
  • Removed leftover debug console.log statements in createServerApiClient.ts.
  • Added unit tests in keep-ui/shared/tests/createServerApiClient.test.ts covering header & cookie forwarding behavior.

Testing

  • Verified that createServerApiClient() propagates the cookie header alongside standard OAuth2-proxy identity headers.
  • Tested against a full local reproduction stack with reverse proxy auth: server-side components on /incidents and /alerts/feed load initial facets successfully.

@CLAassistant

CLAassistant commented Sep 3, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

No linked issues found. Please add the corresponding issues in the pull request description.
Use GitHub automation to close the issue when a PR is merged

@naveenkumarsp naveenkumarsp changed the title Fix cookie header forwarding in createServerApiClient and reorganize tests fix(auth): forward cookie header in createServerApiClient for oauth2-proxy Sep 3, 2026
@naveenkumarsp naveenkumarsp changed the title fix(auth): forward cookie header in createServerApiClient for oauth2-proxy fix: forward cookie header in createServerApiClient for oauth2-proxy Sep 3, 2026
@shahargl

shahargl commented Sep 6, 2026

Copy link
Copy Markdown
Member

@naveenkumarsp can u link the ticket? (if there is nothing exist - create one?)
also , can you sign the CLA?

@codecov

codecov Bot commented Sep 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 30.46%. Comparing base (ce302f6) to head (0c249a1).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #6750       +/-   ##
===========================================
- Coverage   46.42%   30.46%   -15.96%     
===========================================
  Files         178      101       -77     
  Lines       18694    11773     -6921     
===========================================
- Hits         8679     3587     -5092     
+ Misses      10015     8186     -1829     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@naveenkumarsp

Copy link
Copy Markdown
Author

@shahargl - raised a ticket - #6803 and also accepted the CLA. please review and let me know if there is any questions or clarification requried.

@shahargl shahargl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why did u remove the console.log?

@shahargl

Copy link
Copy Markdown
Member

@naveenkumarsp also frontend tests fails now

@naveenkumarsp

Copy link
Copy Markdown
Author

@naveenkumarsp also frontend tests fails now

@shahargl - apologies as i had missed this notification. Let me check and update the branch. also enable the console logs which were removed.

@naveenkumarsp

Copy link
Copy Markdown
Author

@shahargl - i have fixed the issue as well as reverted the console logs.. please review.

This branch has not been deployed

No deployments
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.

3 participants