Skip to content

Answer 401 when a token session holds no upstream access token - #6846

Open
alex-feel wants to merge 1 commit into
stacklok:mainfrom
alex-feel:reauth-on-missing-upstream-tokens
Open

alex-feel wants to merge 1 commit into
stacklok:mainfrom
alex-feel:reauth-on-missing-upstream-tokens

Conversation

@alex-feel

Copy link
Copy Markdown
Contributor

Summary

The embedded auth server stores a session's upstream provider tokens for the upstream access-token expiry plus DefaultRefreshTokenTTL (30 days) and rewrites the row only at login or at an upstream refresh. A client that keeps its session alive through refresh grants but makes no tool call for that long keeps a valid ToolHive token while every upstream row of its session expires, because a refresh grant slides the client's refresh token and touches no row. On the next call TokenValidator.Middleware loaded an empty credential map, projected it onto the identity, and admitted the request. Cedar, pinned to the provider, then failed claim resolution, and the client received HTTP 403 with JSON-RPC error 403 "Unauthorized" and no WWW-Authenticate, while initialize and ping kept succeeding, so it never re-authenticated. Today the validator answers 401 only when a stored token could not be refreshed (#5651).

  • Answer that same 401, a WWW-Authenticate: Bearer challenge with error="invalid_token" and the RFC 6750 body, also when a token session holds no usable upstream access token for any provider, in TokenValidator.Middleware, before any consumer of Identity.UpstreamTokens runs.
  • Decide on the projected access-token map, so rows carrying only an ID token count as none, and share one challenge writer between the refresh-failure branch and the new one.
  • Rewrite the comments that described the preserved empty map and document the behavior in docs/middleware.md and docs/authz.md.

Closes #6845

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

task lint-fix reports nothing in the changed files. task test: every package the change touches passes (pkg/auth and its subpackages, pkg/authz and its authorizers, pkg/authserver, pkg/runner, pkg/vmcp/auth); the remaining failures in that run come from packages whose tests depend on the environment (a container runtime, a keyring, non-root file permissions, lock timing) and do not involve this change, so the Unit tests box is left for CI.

New tests: TestMiddleware_UpstreamTokenEnrichment/returns 401 with WWW-Authenticate when the session holds no usable upstream access token in pkg/auth/token_test.go, beside the refresh-failure cases, with two rows (no rows stored; rows without an access token), asserting status, challenge, and body; and TestUpstreamSessionWithoutRowsEndToEnd in pkg/authz/authorizers/cedar/upstream_session_e2e_test.go, which drives the real validator middleware and the real upstream-token service over in-memory storage: a stored row authenticates and Cedar authorizes; with the rows deleted, authentication rejects the session and Cedar is never consulted. Both fail on main. Inverting the emptiness check, dropping the tsid condition, removing the header, and changing the description each fail at least one of them.

Does this introduce a user-facing change?

Yes. Before: a client whose session had lost every stored upstream token row received HTTP 403 with JSON-RPC error 403 "Unauthorized" on every tools/call, prompts/get, and resources/read, an empty tools/list, and a succeeding initialize, and had to be reconnected by hand. After: the first request after the rows are gone receives HTTP 401 with WWW-Authenticate: Bearer error="invalid_token", the same answer as a refused refresh, so the client signs in again and the login stores fresh rows.

Operators: the Cedar authorization denied WARN carrying upstream token for provider "..." not found in identity no longer appears for this condition. The validator logs token session holds no upstream access token; returning 401 at Debug with tsid and stored_providers (empty when every row is gone), because a client retrying on a stale token controls the rate and an expired row is ordinary lifecycle; the refused-refresh WARN is unchanged. The audit log records the request as an authentication failure (401).

Special notes for reviewers

  • Gate and precedent: the 401 is written by the authentication middleware, step 2 of the chain, so one check reaches every consumer at once: the upstream swap middleware (which answers 401 for a missing provider token itself, Return 401 with WWW-Authenticate for expired upstream tokens #3984), Cedar in pkg/authz and in the vMCP admission path, the list filters, and the vMCP outgoing strategies. It reuses the refresh-failure branch's challenge (Return 401+WWW-Authenticate when vMCP upstream token is unrefreshable #5651) through one helper, writeUpstreamReauthRequired, and, like that branch, decides without knowing which providers the consumers need. vMCP outgoing auth: unrefreshable upstream token surfaces as backend error, not 401 + WWW-Authenticate #5507 named an upstream token that is "missing or can no longer be refreshed" as the case to answer with 401; Return 401+WWW-Authenticate when vMCP upstream token is unrefreshable #5651 covered the second half, this covers the first.
  • Existing deployments: with upstream token injection enabled, the 401 the swap middleware already sent for this session now comes from the validator, with the RFC 6750 JSON body instead of a text line. With injection disabled and Cedar pinned to the provider (the runner pins it to the first upstream automatically), the 403 becomes the 401. With injection disabled and no authorizer reading the credential, a session idle past the row lifetime now re-authenticates once where it was admitted before, as it already does when a refresh is refused. Tokens without a tsid claim (RFC 8693 delegation, RFC 7523 JWT-bearer, a token-only auth server) are untouched.
  • Deliberately left out: a multi-provider session whose pinned provider's row is gone while another provider's row survives still reaches the consumers, where Cedar denies and the swap middleware answers 401; a validator told which providers are required would close that case and spare the consumer-less deployments above, and this change adds no such option. Sliding the row lifetime with the client's refresh token is a storage-model change (session-keyed storage is under discussion in Re-authorization walks the full upstream chain instead of evaluating existing credentials #6831) and would not cover rows removed by other means. Answering at the consumer, with a typed Cedar error mapped to 401 in pkg/authz, would leave tools/list, the vMCP call gate, and the vMCP outgoing strategies as they are for the same session.

The embedded auth server keeps a session's upstream provider tokens for the upstream expiry plus thirty days and rewrites the row only at login or at an upstream refresh.
A client that keeps its own refresh token alive but makes no tool call for that long keeps a valid ToolHive session while every stored upstream row expires.
The token validator then projected an empty credential map onto the identity and admitted the request, so a Cedar authorizer pinned to the provider denied every call with HTTP 403 and no WWW-Authenticate, and the client never re-authenticated.
Answer the same 401 challenge the validator already sends when an upstream refresh is refused, so the client signs in again and the login writes fresh rows.
The check runs on the projected access-token map, so rows that carry only an ID token count as none, and the two branches share one challenge writer.
A session that still holds a token for another provider is unchanged and keeps reaching the consumers.
Deployments that run the embedded auth server with upstream token injection disabled and no consumer of the upstream token see one re-authentication after the rows expire where they saw none before.
The rejection is logged at Debug with the session id and the providers whose rows were still stored, because a client retrying on a stale token sets the rate and the audit log records the 401.

Signed-off-by: Aleksandr Filippov <71711753+alex-feel@users.noreply.github.com>
@codecov

codecov Bot commented Oct 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.67%. Comparing base (60bfed8) to head (64a4ed1).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6846      +/-   ##
==========================================
- Coverage   79.67%   79.67%   -0.01%     
==========================================
  Files         817      817              
  Lines       83425    83430       +5     
==========================================
+ Hits        66470    66473       +3     
- Misses      16950    16952       +2     
  Partials        5        5              

☔ 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.

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.

Session whose upstream token rows expired is denied with 403 instead of a 401 challenge

1 participant