stacklok / toolhive / 29513505872

68%

Build Type

push

github

Committed by web-flow

Commit Message

Add upstream identity to DCR credential cache key (#5824)

* Add upstream identity to DCR credential cache key

Within one embedded authserver, every OAuth2 upstream shares the
authserver's own Issuer and the single defaulted {issuer}/oauth/callback
RedirectURI, so the DCR credential cache key differed only by the scopes
hash. Two upstreams configured with equal scopes therefore collided: the
second read back the first's dynamically-registered client_id /
client_secret and never registered with its own authorization server.

Implements changes for issue #5823:
- Add an UpstreamID component to storage.DCRKey identifying the upstream
  authorization server (upstream issuer derived from DiscoveryURL, or the
  RegistrationEndpoint when set); require it in validation
- Derive UpstreamID in the DCR resolver and thread it through the cache
  key and the singleflight key
- Serialise UpstreamID in the Redis key format and JSON round-trip
- Add regression coverage for two same-scope upstreams and for keys that
  differ only by UpstreamID

* Address code review feedback

Fixed issues from code review:
- MEDIUM: Document the residual UpstreamID collision for custom
  (non-well-known) discovery URLs that resolve to the same issuer origin,
  on resolveUpstreamKeyIdentity and DCRKey.UpstreamID
- LOW: Add a resolver-level regression test for the RegistrationEndpoint
  branch of resolveUpstreamKeyIdentity

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Correct DCR cache-key and migration doc comments

Addresses stacklok/toolhive#5824 review comments:
- MEDIUM redis_keys.go (3592561840): non-expiring DCR rows carry no Redis
  TTL, so orphaned pre-upgrade rows persist indefinitely and need manual
  cleanup rather than expiring; correct the migration note
- MEDIUM discovery.go:710: update the CLI consumer's stale 3-component
  cache-key comment to the (Issuer, UpstreamID, RedirectURI, ScopesHash) tuple
- MEDIUM redis.go / memory_test.go: add UpstreamID to the... (continued)

Coverage Stats

36 of 36 new or added lines in 6 files covered. (100.0%)

13 existing lines in 4 files now uncovered.

73838 of 108430 relevant lines covered (68.1%)

63.9 hits per line