feat(auth): cache client credentials tokens in POST /token - #570
Merged
Conversation
The token endpoint proxies every request upstream, so caller token rate is upstream token rate. Cache successful client_credentials responses and collapse concurrent misses per key into a single upstream call. Off by default. The cache key is an HMAC over grant_type, client_id, the presented secret and the sorted scope and audience sets, so a caller with a wrong secret can never be served another caller's token. expires_in is recomputed from remaining lifetime on every hit, and reuse is capped at the shorter of a configured maximum and 80% of the token's own lifetime. Deployments that apply additional checks during issuance can install a TokenIssuanceGate, consulted on every hit so a cached response cannot bypass them. The upstream call is also detached from the caller's context: a caller that gives up must not abort an in-flight upstream transaction, and under single-flight must not abort the request other waiters depend on.
mfiedorowicz
requested review from
MicahParks,
davidlanouette,
grant-nbl,
jajeffries,
leoparente,
manrodrigues and
paulstuart
as code owners
July 30, 2026 17:58
Vulnerability Scan: Passed — diode-authImage:
Commit: 817efbf |
Vulnerability Scan: Passed — diode-ingesterImage: No vulnerabilities found. Commit: 817efbf |
Vulnerability Scan: Passed — diode-reconcilerImage: No vulnerabilities found. Commit: 817efbf |
|
Go test coverage
Total coverage: 56.6% |
davidlanouette
approved these changes
Jul 30, 2026
davidlanouette
left a comment
davidlanouette
left a comment
Contributor
There was a problem hiding this comment.
Some questions and some suggestions.
But, I wouldn't complain too much about merging it as-is.
Every client this service registers uses client_secret_post and the authorization server rejects any other method, so credentials carried in an Authorization header could only ever produce a rejection. Replace the extraction with a refusal to cache those requests. Keeping the detection still matters even though the extraction does not: without it we would key on the form credentials while the upstream may authenticate the header, which could cache one client's token under another client's key. Also records why an expired entry is dropped on read, and the invariant that a cached rejection can never overwrite a live token.
CVE-2026-56852 (HIGH) in x/text 0.37.0 fails the container scan for all three images. It is present on develop and unrelated to this branch, but the scan only runs on pull requests touching diode-server, so nothing has re-scanned develop since the advisory was published. Bumped here rather than separately to unblock this PR. x/sync moves to 0.21.0 as a side effect of the resolution.
The log message fires when hydra starts binding, not when the mapped port answers, so both hydra integration tests raced ahead and failed their first admin call with connection refused. Latent where port forwarding is instant, reproducible on Docker Desktop and Rancher Desktop. Locally the suite also needs the docker socket path, an IPv4 host override, and the reaper disabled, none of which are code issues.
A cached response is served without consulting the authorization server, so it must grant exactly what that server would have granted. Valid is not enough, it has to be the right token. Mints twice through a cache-enabled server and once directly against hydra, then compares every claim except the per-issuance ones. Identical token strings across the first two prove the second was a cache hit, since two issuances never share a jti; without that check the comparison would pass just as happily with the cache switched off. The comparison ignores a fixed set of claims rather than checking a fixed set, so a claim nobody anticipated is compared by default. That matters for claims injected by a token hook, where the wrong value would hand a caller another tenant's authorization. A control asserts the comparison actually distinguishes tokens issued for different clients.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
POST /tokenis a 1:1 proxy to the oauth2 server, so caller token rate is upstream token rate. Adds an optional per-credential cache with single-flight. Disabled by default.Safety
The presented secret is part of every cache key, so a valid
client_idwith the wrong secret derives a different key and always goes upstream. Onlyclient_credentialsand only200responses are cached.expires_inis recomputed on every hit. Reuse is bounded bymin(OAUTH2_TOKEN_CACHE_MAX_TTL, 80% of token lifetime).Deployments running extra checks at issuance can install a
TokenIssuanceGate, consulted on every hit. A denial or gate error falls through to the upstream rather than serving the entry, so failing closed costs a cache hit and not availability.Config
OAUTH2_TOKEN_CACHE_ENABLEDfalseOAUTH2_TOKEN_CACHE_MAX_ENTRIES4096OAUTH2_TOKEN_CACHE_MAX_TTL15mOAUTH2_TOKEN_CACHE_NEGATIVE_TTL5sMemory measured locally at roughly 7 KiB per entry, so the default bound is about 27 MiB when full, and flat past capacity.
Tests
14 new tests. Four are the ones that must never regress: a wrong secret is never served from cache, a narrower scope is never served from a broader entry, non-
client_credentialsgrants are never cached, and a gate denial never serves the entry.