Skip to content

fix(k8s-client): redirect to oauth2-proxy on 401 instead of failing silently#34

Open
IvanHunters wants to merge 1 commit into
mainfrom
fix/auth-redirect-on-401
Open

fix(k8s-client): redirect to oauth2-proxy on 401 instead of failing silently#34
IvanHunters wants to merge 1 commit into
mainfrom
fix/auth-redirect-on-401

Conversation

@IvanHunters
Copy link
Copy Markdown
Contributor

@IvanHunters IvanHunters commented Jun 2, 2026

Summary

When the kc-access cookie expires, k8s API calls come back with 401 (after the matching cozystack/cozystack PR makes oauth2-proxy answer 401 for /api and /apis instead of redirecting). Previously the SPA was left stuck on a stale page until the user manually cleared the cache.

K8sClient now treats 401 as an auth boundary: it fires a one-shot top-level navigation to /oauth2/start?rd=<current-url>, which is the oauth2-proxy convention for re-authenticating and bouncing the user back. The handler is overridable through K8sClientConfig.onUnauthorized for non-oauth2-proxy deployments and is a no-op outside the browser.

Related: cozystack/cozystack#2788

Test plan

  • pnpm typecheck passes
  • After session expiry, hitting any console page triggers a navigation to /oauth2/start?rd=... and the user is bounced back after re-auth
  • When a valid session exists, behaviour is unchanged
  • Multiple concurrent 401s do not cause repeated redirects (unauthorizedHandled latch)

Summary by CodeRabbit

  • New Features
    • Added support for customizable handling of unauthorized (401) API responses, with automatic redirect to OAuth authentication endpoint as the default behavior when running in the browser.

…ilently

When the kc-access cookie expires, k8s API calls come back with 401
(after the matching cozystack PR makes oauth2-proxy answer 401 instead
of redirecting). The previous behaviour left the SPA stuck on a stale
page until the user cleared the cache.

K8sClient now treats 401 as an auth boundary: it fires a one-shot
top-level navigation to `/oauth2/start?rd=<current-url>`, which is
the oauth2-proxy convention for re-authenticating and bouncing the
user back. The handler is overridable through `K8sClientConfig.onUnauthorized`
for non-oauth2-proxy deployments and is a no-op outside the browser.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
@github-actions github-actions Bot added size/S This PR changes 10-29 lines, ignoring generated files area/k8s-client Issues or PRs related to packages/k8s-client — K8sClient, hooks, watch layer kind/bug Categorizes issue or PR as related to a bug labels Jun 2, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 2, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c0956b1e-49a2-44d1-ac21-c7f6e729d8a7

📥 Commits

Reviewing files that changed from the base of the PR and between cecb486 and 347e1dd.

📒 Files selected for processing (1)
  • packages/k8s-client/src/client.ts

📝 Walkthrough

Walkthrough

K8sClient now supports callback-driven handling of HTTP 401 unauthorized responses. A new optional onUnauthorized callback in K8sClientConfig controls this behavior, defaulting to browser-based OAuth redirect. The client tracks execution state to ensure the callback runs only once, and integrates this handler into both the request() and watch() response paths.

Changes

Unauthorized response callback handling

Layer / File(s) Summary
Configuration contract and default handler
packages/k8s-client/src/client.ts
K8sClientConfig interface adds optional onUnauthorized?: () => void callback, alongside a default handler that redirects the browser to the OAuth start endpoint when in browser context.
Handler state and callback integration
packages/k8s-client/src/client.ts
K8sClient constructor stores the callback; internal unauthorizedHandled flag tracks single execution; handleUnauthorized() method ensures the callback runs exactly once per client instance.
Integration into request paths
packages/k8s-client/src/client.ts
request() and watch() methods both invoke handleUnauthorized() on HTTP 401 responses before proceeding with existing error handling and throw logic.

🎯 2 (Simple) | ⏱️ ~8 minutes

🐰 A hop, a skip, a 401!
When auth fails, here's the rule:
One callback, once and true,
OAuth flows where browsers grew. 🔐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: handling 401 responses by redirecting to oauth2-proxy instead of failing silently. It is specific, concise, and clearly reflects the primary purpose of the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/auth-redirect-on-401

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds unauthorized request handling to the K8sClient via an optional onUnauthorized callback, which defaults to redirecting the user to an OAuth2 start page. Feedback on the changes highlights an issue where the unauthorizedHandled flag remains true indefinitely if a custom handler is used in a Single Page Application (SPA) context without a full page reload. It is recommended to provide a way to reset this flag, either automatically on successful requests or via a public method.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +43 to 47
private handleUnauthorized(): void {
if (this.unauthorizedHandled) return
this.unauthorizedHandled = true
this.onUnauthorized()
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If onUnauthorized is overridden with a custom handler that does not trigger a full page reload (e.g., showing a login modal, performing a silent token refresh, or navigating via a SPA router), unauthorizedHandled will remain true indefinitely. This prevents any subsequent session expirations from triggering the unauthorized handler again during the lifetime of the SPA.

To support these scenarios, we should expose a public method to reset this flag once re-authentication succeeds. Additionally, you may want to automatically reset this.unauthorizedHandled = false upon any successful request (where res.ok is true) in request and watch.

  public resetUnauthorized(): void {
    this.unauthorizedHandled = false
  }

  private handleUnauthorized(): void {
    if (this.unauthorizedHandled) return
    this.unauthorizedHandled = true
    this.onUnauthorized()
  }

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.

NOT LGTM — the change is correct and well-scoped, but it ships new auth behavior with no tests, which this repo's mandatory-TDD policy treats as a blocker.

Business context: when the oauth2-proxy session expires mid-session, Kubernetes API calls return 401 and currently fail silently (React Query doesn't retry 401, and the connection-error banner deliberately ignores 401/403); this PR redirects to /oauth2/start?rd=<current-url> so the user re-authenticates.

Blockers

B1: New 401-redirect behavior has no test coverage

File: packages/k8s-client/src/client.ts

Issue: The PR adds a public onUnauthorized config field, a default redirect side-effect, a stateful one-shot guard (unauthorizedHandled), and two new 401 call sites (request() and watch()) — with no accompanying test.

Evidence: No test references onUnauthorized / handleUnauthorized / 401 under apps/console/src/__tests__/k8s-client/. The repo's CLAUDE.md mandates TDD ("No code without corresponding tests"), and the client test harness already exists (client.subresource.test.ts stubs fetch via vi.stubGlobal and constructs a K8sClient with config), so the gap is cheap to close.

Impact: The subtlest logic in the diff — the one-shot guard and the 401-vs-403 distinction the design depends on — is unprotected against regression. A later refactor of the status check or the guard could silently re-introduce redirect storms, or redirect on 403 (which would break the in-page permission notices that rely on 403 being handled gracefully).

Fix: Add tests, each failing without the change and passing with it:

  • request() → 401 calls onUnauthorized once; 403 does not; a 2xx does not.
  • watch() initial fetch → 401 calls onUnauthorized (and the stream still rejects via onError).
  • one-shot guard: two 401s on one client instance call onUnauthorized exactly once.
  • default handler builds /oauth2/start?rd=<encodeURIComponent(pathname + search + hash)> (stub window.location, assert assign).

Non-blocking follow-ups

  1. The redirect assumes oauth2-proxy owns /oauth2/* upstream of nginx. The bundled nginx config doesn't proxy /oauth2/* (its location / falls through to try_files … /index.html), so hitting /oauth2/start against nginx alone would serve the SPA (200), not redirect. This holds in the documented production topology (oauth2-proxy in front) and matches the existing /oauth2/userinfo and /oauth2/sign_out usage — flagging only so the dependency is conscious; no change needed.
  2. rd includes window.location.hash. Browsers don't send the fragment to the server on a top-level navigation, so oauth2-proxy can't honor it; the encoded hash is harmless but inert. Optional to drop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/k8s-client Issues or PRs related to packages/k8s-client — K8sClient, hooks, watch layer kind/bug Categorizes issue or PR as related to a bug size/S This PR changes 10-29 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants