Skip to content

CIMD client unresolved with 401 invalid_client when the metadata re-fetch fails transiently #6856

Open

Description

@alex-feel

Bug description

With cimd.enabled: true, a token refresh or an authorization request for a client the server resolved minutes earlier is answered 401 invalid_client when the re-fetch of its client metadata document fails for a transient reason: the host does not answer within the 5-second timeout, refuses the connection, or returns 5xx. The server still holds the expired cache entry and has persisted the same client, and the identical refresh succeeds a few minutes later, but invalid_client tells the client its credentials were rejected, so public clients discard their tokens and the user has to sign in again on every endpoint that was refreshing at that moment.

Mechanism (identical at v0.53.0 and main 16ffc0b): CIMDStorageDecorator.fetchOrCached serves a cached client only while time.Now().Before(entry.expires) (pkg/authserver/storage/cimd_decorator.go:183 and :194, TTL cimd.cache_fallback_ttl, default 5 minutes). Once the entry has expired, fetch (:209) calls cimd.FetchClientMetadataDocument (hardcoded 5-second dial, TLS, header, and client timeouts, pkg/oauthproto/cimd/fetch.go:109-140) and on any error returns fosite.ErrNotFound (:219) without consulting the expired entry it still holds. fosite v0.49.0 wraps every GetClient error into ErrInvalidClient with status 401 (client_authentication.go:200-202, errors.go:97-101) at /oauth/token, where the refresh grant cannot skip client authentication (handler/oauth2/flow_refresh.go:234-236), and at /oauth/authorize (authorize_request_handler.go:360-362).

Steps to reproduce

  1. Enable CIMD on the embedded authorization server and resolve a client by its metadata URL once, so the decorator caches and persists it.
  2. Let the cache entry expire (cache_fallback_ttl), then make the metadata host unreachable or have it answer 500.
  3. Send a refresh_token grant (or an authorization request) for that client.

A decorator-level test shows it without a network: resolve a client served by an httptest server, expire its cache entry, close the server, and call GetClient again. Without the fix it returns not_found right after the decorator's own CIMD document fetch failed warning. The test is TestCIMDStorageDecorator_TransientFetchFailure, row "host closed within window serves the expired entry", in the fix PR.

Expected behavior

A client the server has already resolved stays resolvable across a transient failure to re-fetch its document, for a bounded time: the request is answered from the last known document and the next lookup re-fetches again. A client the server has never resolved, a 3xx or a 4xx from the host other than 408 or 429, and an invalid document keep failing closed.

Actual behavior

401 invalid_client for the duration of the blip; the client signs the user out.

Environment (if relevant)

  • ToolHive version: v0.53.0, embedded authorization server (thv-proxyrunner), also present on main at 16ffc0b.

Additional context

Nothing in the docs or tests pins the current behavior: the only test on this path, TestCIMDStorageDecorator_FetchOrCached_FetchFailureReturnsNotFound, covers a client the server never resolved, which should keep failing closed. The fix is in #6857: the fetcher classifies transient failures (no usable response, a 5xx, a 408, or a 429), and the decorator serves the expired entry within the MaxStale window the fetch limiter's refusal path already uses (the configured access token lifespan) without renewing it or rewriting the persisted row; nothing new is configurable, and a definitive failure evicts the entry.

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions