stacklok / toolhive / 27279018655

67%

Build Type

push

github

Committed by web-flow

Commit Message

Renew public client TTL on successful token exchange (#5469)

* Renew public client TTL on read in Redis store

Public clients registered via DCR get DefaultPublicClientTTL stamped on
their Redis registration at registration time, and GetClient only reads
it back. The TTL is therefore a fixed window from registration: an
actively-used public client (e.g. a long-lived MCP desktop/IDE client
that caches its client_id) is evicted DefaultPublicClientTTL after it
registered, regardless of use. Its next token request then fails at the
token endpoint with invalid_client, and the only recovery is to fully
re-register the client.

Renew the TTL to DefaultPublicClientTTL on a successful GetClient for
public clients, turning expiry into a sliding window of inactivity.
Abandoned clients still expire after the window, preserving the
anti-bloat intent of RegisterClient; confidential clients have no TTL
and are left untouched. Renewal is best-effort: a failure is logged and
does not fail the lookup.

The in-memory backend stores clients without a TTL, so it is unaffected.

Add a unit test covering both the public-client renewal and the
confidential-client no-TTL invariant.

Signed-off-by: Mani Japra <mani@muonspace.com>

* Renew client TTL on token success, not on read

Review flagged that renewing the public-client TTL inside GetClient
fires on the unauthenticated front-channel GET /oauth/authorize read
(fosite's NewAuthorizeRequest), not only on the token endpoint. Public
clients have no secret, so any caller who knows a public client_id could
keep its registration alive indefinitely by polling authorize, defeating
the anti-bloat TTL the renewal was meant to preserve.

Move renewal to the proven-use signal: a successful token exchange or
refresh. Add RenewClientTTL to the ClientRegistry interface — Redis
EXPIREs the public client's key (confidential clients and missing keys
are left untouched), the in-memory backend is a documented no-op — and
call it best-effort ... (continued)

Coverage Stats

21 of 36 new or added lines in 4 files covered. (58.33%)

23 existing lines in 6 files now uncovered.

67273 of 101503 relevant lines covered (66.28%)

62.85 hits per line