diff --git a/internal/server/scheduledauth/kidguard.go b/internal/server/scheduledauth/kidguard.go new file mode 100644 index 000000000..6ab090a40 --- /dev/null +++ b/internal/server/scheduledauth/kidguard.go @@ -0,0 +1,246 @@ +package scheduledauth + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "sync" + "sync/atomic" + "time" + + "github.com/coreos/go-oidc/v3/oidc" + jose "github.com/go-jose/go-jose/v4" +) + +// Negative kid cache (issue #75). +// +// go-oidc's RemoteKeySet refetches the JWKS every time a token fails to +// verify against its cached keys, with no cooldown. Each bad token is +// therefore proxied 1:1 to the provider's JWKS endpoint. guardedKeySet +// wraps the RemoteKeySet and remembers, for negativeKidTTL, any kid that a +// JWKS fetch issued AFTER the token arrived could not verify. Further +// tokens with that kid are rejected without calling go-oidc, so a repeated +// kid costs at most one JWKS fetch per negativeKidTTL. +// +// Safety properties: +// - Fail closed: a cache hit only ever rejects; it never accepts. +// - A kid is cached only when a JWKS fetch that started after the token +// arrived completed successfully and still could not verify it. A failed +// fetch, or one that was already in flight when the token arrived (the +// go-oidc stale-inflight case), never caches anything. +// - A cached kid never blocks a token the latest fetched JWKS can verify, +// so forged tokens that reuse a published kid cannot lock out genuine +// tokens carrying that kid, and a rotated key is accepted as soon as any +// refresh picks it up. +// - Entries expire after negativeKidTTL (the same window as the verifier +// rebuild limit), and the cache holds at most maxNegativeKids entries, +// keyed by a SHA-256 of the kid so attacker-sized kids cost 32 bytes. +// +// Revisit if go-oidc adds its own refresh cooldown: bump it and drop this. + +// negativeKidTTL is how long a kid that a fresh JWKS fetch did not contain +// is rejected without a refetch. It matches minRebuildInterval, so the +// negative cache never delays rotation recovery beyond the existing bound. +const negativeKidTTL = minRebuildInterval + +// maxNegativeKids caps the negative cache. When it is full of live entries +// a new kid is not stored; that kid then behaves as it did before the cache +// existed (one fetch per token), so overflow costs no correctness. +const maxNegativeKids = 1024 + +// errKidRecentlyAbsent is returned for a token whose kid a recent JWKS +// fetch did not contain. go-oidc wraps it as "failed to verify signature". +var errKidRecentlyAbsent = errors.New("token key ID was absent from a recent JWKS fetch; not refetching") + +// jwksSnapshot is the key set returned by one successful JWKS fetch. +type jwksSnapshot struct { + keys []jose.JSONWebKey + seq uint64 // jwksObserver.started value when this fetch began +} + +// jwksObserver is the RoundTripper for the validator's JWKS client. It +// numbers every fetch as it starts and records the key set from each +// successful response, so guardedKeySet can tell whether a fetch began +// after a given token arrived and what that fetch returned. The client is +// used only for the JWKS URL (go-oidc fetches and Warmup). +type jwksObserver struct { + base http.RoundTripper + started atomic.Uint64 + latest atomic.Pointer[jwksSnapshot] +} + +// RoundTrip forwards to the hardened base transport, then buffers and +// parses a 200 response body before handing it back unchanged. +func (o *jwksObserver) RoundTrip(req *http.Request) (*http.Response, error) { + seq := o.started.Add(1) + resp, err := o.base.RoundTrip(req) + if err != nil || resp.StatusCode != http.StatusOK { + return resp, err + } + body, err := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if err != nil { + return nil, fmt.Errorf("reading JWKS response: %w", err) + } + resp.Body = io.NopCloser(bytes.NewReader(body)) + o.record(seq, body) + return resp, nil +} + +// record stores the parsed key set unless a fetch that started later has +// already been recorded. Unparseable bodies are ignored: go-oidc rejects +// them too, so they never become its cached keys. +func (o *jwksObserver) record(seq uint64, body []byte) { + var set jose.JSONWebKeySet + if err := json.Unmarshal(body, &set); err != nil { + return + } + snap := &jwksSnapshot{keys: set.Keys, seq: seq} + for { + cur := o.latest.Load() + if cur != nil && cur.seq >= seq { + return + } + if o.latest.CompareAndSwap(cur, snap) { + return + } + } +} + +// negativeKidCache is a size-bounded set of kids with per-entry expiry. +type negativeKidCache struct { + mu sync.Mutex + entries map[[sha256.Size]byte]time.Time // kid hash -> expiry + max int +} + +func newNegativeKidCache(maxEntries int) *negativeKidCache { + return &negativeKidCache{entries: make(map[[sha256.Size]byte]time.Time), max: maxEntries} +} + +// contains reports whether kid has a live entry at now, dropping it if it +// has expired. +func (c *negativeKidCache) contains(kid string, now time.Time) bool { + h := sha256.Sum256([]byte(kid)) + c.mu.Lock() + defer c.mu.Unlock() + exp, ok := c.entries[h] + if !ok { + return false + } + if now.Before(exp) { + return true + } + delete(c.entries, h) + return false +} + +// add records kid until now+negativeKidTTL. When the cache is full it first +// sweeps expired entries; if it is still full the kid is not stored. +func (c *negativeKidCache) add(kid string, now time.Time) { + h := sha256.Sum256([]byte(kid)) + c.mu.Lock() + defer c.mu.Unlock() + if _, ok := c.entries[h]; !ok && len(c.entries) >= c.max { + for k, exp := range c.entries { + if !now.Before(exp) { + delete(c.entries, k) + } + } + if len(c.entries) >= c.max { + return + } + } + c.entries[h] = now.Add(negativeKidTTL) +} + +// kidGuard is the validator-wide state shared by every guardedKeySet, so +// it survives verifier rebuilds. +type kidGuard struct { + observer *jwksObserver + negative *negativeKidCache + now func() time.Time + algs []jose.SignatureAlgorithm +} + +// newKidGuard installs a jwksObserver in front of client's transport. +func newKidGuard(client *http.Client, now func() time.Time, algs []jose.SignatureAlgorithm) *kidGuard { + obs := &jwksObserver{base: client.Transport} + client.Transport = obs + return &kidGuard{ + observer: obs, + negative: newNegativeKidCache(maxNegativeKids), + now: now, + algs: algs, + } +} + +// guardedKeySet is the oidc.KeySet handed to the IDTokenVerifier. +type guardedKeySet struct { + inner oidc.KeySet + g *kidGuard +} + +// VerifySignature rejects tokens whose kid is negatively cached and that +// the latest JWKS cannot verify; everything else goes to go-oidc. When +// go-oidc fails, the kid is cached if a fresh fetch confirmed it unknown. +func (k *guardedKeySet) VerifySignature(ctx context.Context, rawJWT string) ([]byte, error) { + arrival := k.g.observer.started.Load() + jws, err := jose.ParseSigned(rawJWT, k.g.algs) + if err != nil { + return nil, fmt.Errorf("malformed jwt: %w", err) + } + kid := jwsKeyID(jws) + if k.g.negative.contains(kid, k.g.now()) && !k.g.latestVerifies(jws, kid) { + return nil, errKidRecentlyAbsent + } + payload, err := k.inner.VerifySignature(ctx, rawJWT) + if err != nil { + k.g.rememberIfConfirmed(jws, kid, arrival) + } + return payload, err +} + +// latestVerifies reports whether the most recently fetched JWKS holds a +// key that verifies jws. +func (g *kidGuard) latestVerifies(jws *jose.JSONWebSignature, kid string) bool { + snap := g.observer.latest.Load() + return snap != nil && verifiesWith(jws, kid, snap.keys) +} + +// rememberIfConfirmed caches kid when a JWKS fetch that started after the +// token arrived (seq > arrival) completed and still cannot verify it. +func (g *kidGuard) rememberIfConfirmed(jws *jose.JSONWebSignature, kid string, arrival uint64) { + snap := g.observer.latest.Load() + if snap == nil || snap.seq <= arrival || verifiesWith(jws, kid, snap.keys) { + return + } + g.negative.add(kid, g.now()) +} + +// verifiesWith mirrors go-oidc's key selection: keys whose kid matches, or +// every key when the token has no kid. +func verifiesWith(jws *jose.JSONWebSignature, kid string, keys []jose.JSONWebKey) bool { + for i := range keys { + if kid != "" && keys[i].KeyID != kid { + continue + } + if _, err := jws.Verify(&keys[i]); err == nil { + return true + } + } + return false +} + +// jwsKeyID returns the kid of the first signature, as go-oidc does. +func jwsKeyID(jws *jose.JSONWebSignature) string { + if len(jws.Signatures) == 0 { + return "" + } + return jws.Signatures[0].Header.KeyID +} diff --git a/internal/server/scheduledauth/kidguard_test.go b/internal/server/scheduledauth/kidguard_test.go new file mode 100644 index 000000000..72ab4dde0 --- /dev/null +++ b/internal/server/scheduledauth/kidguard_test.go @@ -0,0 +1,287 @@ +package scheduledauth + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "sync" + "sync/atomic" + "testing" + "time" +) + +// signedFor signs a token that passes every claim check, so the only +// thing that can fail is the signature/key lookup. +func signedFor(t *testing.T, key *testKey, now time.Time) string { + t.Helper() + return "Bearer " + signToken(t, key, baseClaims(now, + testSchedulerSubject, "https://api.example.com", "https://accounts.example.com")) +} + +// swapJWKS replaces the document newJWKSServer serves. +func swapJWKS(t *testing.T, srv *jwksServer, body []byte) { + t.Helper() + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, srv.URL+"/swap", bytes.NewReader(body)) + if err != nil { + t.Fatalf("build swap request: %v", err) + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("swap: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + b, _ := io.ReadAll(resp.Body) + t.Fatalf("swap: status %d: %s", resp.StatusCode, b) + } +} + +func negativeKidCount(v *Validator) int { + v.kids.negative.mu.Lock() + defer v.kids.negative.mu.Unlock() + return len(v.kids.negative.entries) +} + +// Issue #75: once a JWKS fetched after a token arrived has shown its kid +// is unknown, further tokens with that kid must be rejected without +// another JWKS fetch. Before the fix every such token cost one fetch +// (21 for this loop). +func TestValidate_OIDC_RepeatedUnknownKidSkipsRefetch(t *testing.T) { + keyA := newTestKey(t, "kid-A") + keyX := newTestKey(t, "kid-X") // never published + srv := newJWKSServer(t, jwks(keyA)) + v := newOIDCValidator(t, srv.URL) + t0 := time.Now() + v.now = func() time.Time { return t0 } + + if err := v.Validate(context.Background(), signedFor(t, keyA, t0)); err != nil { + t.Fatalf("warm-up token: %v", err) + } + before := srv.hits.Load() + + bad := signedFor(t, keyX, t0) + for i := 0; i < 20; i++ { + if err := v.Validate(context.Background(), bad); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("unknown-kid token %d: expected ErrUnauthorized, got %v", i, err) + } + } + if got := srv.hits.Load() - before; got != 1 { + t.Fatalf("20 tokens with the same unknown kid made %d JWKS fetches; want 1", got) + } + + // Legitimate traffic is unaffected. + if err := v.Validate(context.Background(), signedFor(t, keyA, t0)); err != nil { + t.Fatalf("valid token after unknown-kid spam: %v", err) + } +} + +// A negative entry is bounded by negativeKidTTL: a kid that is published +// after it was cached is accepted once the entry expires. +func TestValidate_OIDC_NegativeKidExpiresAfterTTL(t *testing.T) { + keyA := newTestKey(t, "kid-A") + keyB := newTestKey(t, "kid-B") + srv := newJWKSServer(t, jwks(keyA)) + v := newOIDCValidator(t, srv.URL) + t0 := time.Now() + current := t0 + v.now = func() time.Time { return current } + + tokB := signedFor(t, keyB, t0) + if err := v.Validate(context.Background(), tokB); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("kid-B before publication: expected ErrUnauthorized, got %v", err) + } + swapJWKS(t, srv, jwks(keyA, keyB)) + + current = t0.Add(negativeKidTTL - time.Second) + before := srv.hits.Load() + if err := v.Validate(context.Background(), tokB); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("kid-B inside the negative TTL: expected ErrUnauthorized, got %v", err) + } + if got := srv.hits.Load() - before; got != 0 { + t.Fatalf("negatively cached kid made %d JWKS fetches inside its TTL; want 0", got) + } + + current = t0.Add(negativeKidTTL) + if err := v.Validate(context.Background(), tokB); err != nil { + t.Fatalf("kid-B once the negative entry expired: %v", err) + } +} + +// A negative entry never hides a key the most recent JWKS fetch already +// contains: once any refresh has picked up the rotated key, tokens signed +// by it are accepted at once, even inside the entry's TTL. +func TestValidate_OIDC_NegativeKidBypassedOncePublished(t *testing.T) { + keyA := newTestKey(t, "kid-A") + keyB := newTestKey(t, "kid-B") + keyY := newTestKey(t, "kid-Y") // never published + srv := newJWKSServer(t, jwks(keyA)) + v := newOIDCValidator(t, srv.URL) + t0 := time.Now() + v.now = func() time.Time { return t0 } + + tokB := signedFor(t, keyB, t0) + if err := v.Validate(context.Background(), tokB); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("kid-B before publication: expected ErrUnauthorized, got %v", err) + } + swapJWKS(t, srv, jwks(keyA, keyB)) + + // Any other refresh (here a different unknown kid) fetches the rotated + // document that now carries kid-B. + if err := v.Validate(context.Background(), signedFor(t, keyY, t0)); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("kid-Y: expected ErrUnauthorized, got %v", err) + } + if err := v.Validate(context.Background(), tokB); err != nil { + t.Fatalf("kid-B after a refresh published it, inside its negative TTL: %v", err) + } +} + +// Forged tokens that reuse a published kid must not trigger a refetch +// each, and must not lock out genuine tokens carrying that kid. +func TestValidate_OIDC_ForgedPublishedKidDoesNotBlockLegit(t *testing.T) { + keyA := newTestKey(t, "kid-A") + imposter := newTestKey(t, "kid-A") // same kid, different key + srv := newJWKSServer(t, jwks(keyA)) + v := newOIDCValidator(t, srv.URL) + t0 := time.Now() + v.now = func() time.Time { return t0 } + + legit := signedFor(t, keyA, t0) + if err := v.Validate(context.Background(), legit); err != nil { + t.Fatalf("warm-up token: %v", err) + } + before := srv.hits.Load() + forged := signedFor(t, imposter, t0) + for i := 0; i < 10; i++ { + if err := v.Validate(context.Background(), forged); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("forged token %d: expected ErrUnauthorized, got %v", i, err) + } + } + if got := srv.hits.Load() - before; got != 1 { + t.Fatalf("10 forged tokens with a published kid made %d JWKS fetches; want 1", got) + } + for i := 0; i < 3; i++ { + if err := v.Validate(context.Background(), legit); err != nil { + t.Fatalf("legit token %d after forged spam: %v", i, err) + } + } +} + +// A kid is only cached when a JWKS fetch that completed successfully +// after the token arrived lacked it. If the refresh fails, nothing is +// cached and the next token tries again (fail closed, no poisoning). +func TestValidate_OIDC_FailedRefreshDoesNotCacheKid(t *testing.T) { + keyA := newTestKey(t, "kid-A") + keyX := newTestKey(t, "kid-X") + var ( + hits atomic.Int64 + failing atomic.Bool + ) + body := jwks(keyA) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + if failing.Load() { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + return + } + _, _ = w.Write(body) + })) + t.Cleanup(ts.Close) + v := newOIDCValidator(t, ts.URL) + t0 := time.Now() + v.now = func() time.Time { return t0 } + + if err := v.Validate(context.Background(), signedFor(t, keyA, t0)); err != nil { + t.Fatalf("warm-up token: %v", err) + } + failing.Store(true) + bad := signedFor(t, keyX, t0) + for i := 0; i < 3; i++ { + before := hits.Load() + if err := v.Validate(context.Background(), bad); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("token %d: expected ErrUnauthorized, got %v", i, err) + } + if hits.Load() == before { + t.Fatalf("token %d: no JWKS fetch although no refresh ever confirmed the kid unknown", i) + } + } + if n := negativeKidCount(v); n != 0 { + t.Fatalf("negative cache holds %d entries after failed refreshes; want 0", n) + } +} + +// Concurrent valid and unknown-kid traffic: run with -race. Valid tokens +// must always pass, bad ones always fail, and JWKS fetches stay bounded. +func TestValidate_OIDC_NegativeKidConcurrent(t *testing.T) { + keyA := newTestKey(t, "kid-A") + keyX := newTestKey(t, "kid-X") + srv := newJWKSServer(t, jwks(keyA)) + v := newOIDCValidator(t, srv.URL) + t0 := time.Now() + v.now = func() time.Time { return t0 } + + good := signedFor(t, keyA, t0) + bad := signedFor(t, keyX, t0) + const workers, rounds = 16, 25 + var wg sync.WaitGroup + errCh := make(chan error, workers*rounds) + for w := 0; w < workers; w++ { + wg.Add(1) + go func(w int) { + defer wg.Done() + for i := 0; i < rounds; i++ { + if (w+i)%2 == 0 { + if err := v.Validate(context.Background(), good); err != nil { + errCh <- fmt.Errorf("valid token rejected: %w", err) + } + } else if err := v.Validate(context.Background(), bad); !errors.Is(err, ErrUnauthorized) { + errCh <- fmt.Errorf("unknown-kid token: expected ErrUnauthorized, got %w", err) + } + } + }(w) + } + wg.Wait() + close(errCh) + for err := range errCh { + t.Error(err) + } + // Cold fetch, the unknown-kid refresh, the rebuilt key set's lazy + // fetch, plus slack for go-oidc's inflight scheduling. Before the fix + // this grew with the number of unknown-kid tokens. + if hits := srv.hits.Load(); hits > 6 { + t.Errorf("expected <=6 JWKS fetches, got %d", hits) + } +} + +func TestNegativeKidCache_BoundedAndExpiring(t *testing.T) { + c := newNegativeKidCache(4) + t0 := time.Now() + for i := 0; i < 10; i++ { + c.add(fmt.Sprintf("kid-%d", i), t0) + } + if n := len(c.entries); n != 4 { + t.Fatalf("cache holds %d entries; want the 4-entry cap", n) + } + if !c.contains("kid-0", t0) { + t.Fatal("kid-0 should be cached inside its TTL") + } + if c.contains("kid-9", t0) { + t.Fatal("kid-9 arrived with the cache full of live entries and should not have been stored") + } + + // Once the old entries expire, an add into the full cache sweeps them. + late := t0.Add(negativeKidTTL) + c.add("late", late) + if n := len(c.entries); n != 1 { + t.Fatalf("cache holds %d entries after expiry sweep; want 1", n) + } + if !c.contains("late", late) { + t.Fatal("late entry should be cached") + } + if c.contains("kid-0", late) { + t.Fatal("kid-0 should have expired at its TTL") + } +} diff --git a/internal/server/scheduledauth/validator.go b/internal/server/scheduledauth/validator.go index ba5a87dbe..29eca74fc 100644 --- a/internal/server/scheduledauth/validator.go +++ b/internal/server/scheduledauth/validator.go @@ -15,6 +15,7 @@ import ( "time" "github.com/coreos/go-oidc/v3/oidc" + jose "github.com/go-jose/go-jose/v4" "github.com/LeanerCloud/cloud-commitments-go/pkg/httpclient" ) @@ -68,7 +69,7 @@ type Validator struct { verifier *oidc.IDTokenVerifier // nil unless mode == ModeOIDC; backed by go-oidc's single-flight RemoteKeySet verMu sync.RWMutex // guards verifier and lastRebuild during rotation-recovery rebuilds lastRebuild time.Time // last verifier rebuild; rate-limits attacker-driven JWKS refetches - keySet *oidc.RemoteKeySet // nil unless mode == ModeOIDC; powered by go-oidc's single-flight cache + kids *kidGuard // nil unless mode == ModeOIDC; negative kid cache shared across rebuilds (issue #75) jwksURL string // remembered for Warmup and rotation rebuild; empty unless mode == ModeOIDC issuer string // OIDC issuer URL; empty unless mode == ModeOIDC; needed to rebuild verifier httpClient *http.Client // hardened client for all JWKS fetches; nil unless mode == ModeOIDC @@ -178,8 +179,19 @@ func configureOIDC(v *Validator, cfg Config) (*Validator, error) { // misconfigured or compromised value must not be able to reach // internal/metadata endpoints via the default transport. v.httpClient = httpclient.New() - v.keySet = oidc.NewRemoteKeySet(oidc.ClientContext(context.Background(), v.httpClient), cfg.JWKSURL) - v.verifier = oidc.NewVerifier(cfg.Issuer, v.keySet, &oidc.Config{ + // The kid guard wraps the hardened transport to observe JWKS + // responses; it does not replace it. + v.kids = newKidGuard(v.httpClient, func() time.Time { return v.now() }, []jose.SignatureAlgorithm{jose.RS256}) + v.verifier = v.newVerifier() + return v, nil +} + +// newVerifier builds a fresh go-oidc RemoteKeySet (empty cache, no +// inflight fetch) behind the negative kid guard, and the IDTokenVerifier +// on top of it. Used at startup and by the rotation-recovery rebuild. +func (v *Validator) newVerifier() *oidc.IDTokenVerifier { + ks := oidc.NewRemoteKeySet(oidc.ClientContext(context.Background(), v.httpClient), v.jwksURL) + return oidc.NewVerifier(v.issuer, &guardedKeySet{inner: ks, g: v.kids}, &oidc.Config{ // Pin to RS256. Google's tokens are RS256; rejecting anything // else closes the alg=none / alg=HS256 confusion family. SupportedSigningAlgs: []string{string(oidc.RS256)}, @@ -189,7 +201,6 @@ func configureOIDC(v *Validator, cfg Config) (*Validator, error) { SkipClientIDCheck: true, SkipExpiryCheck: true, }) - return v, nil } func configureBearer(v *Validator, cfg Config) (*Validator, error) { @@ -402,8 +413,9 @@ func (v *Validator) validateOIDC(ctx context.Context, authz string) error { // attacker-driven fetches to ~6/min while a genuine key rotation still // recovers within one interval (providers publish the new key well before // signing with it, so a >10s outage window here is not a realistic -// rotation pattern - and go-oidc's own refresh-on-unknown-kid still runs -// on every request regardless of this limit). +// rotation pattern). go-oidc's own refresh-on-unknown-kid would still run +// on every request regardless of this limit; the negative kid cache in +// kidguard.go bounds that to one fetch per kid per negativeKidTTL. const minRebuildInterval = 10 * time.Second // verifyWithRotationRetry calls Verify on the current verifier. If the call @@ -450,12 +462,7 @@ func (v *Validator) verifyWithRotationRetry(ctx context.Context, rawToken string return nil, err } log.Printf("scheduledauth: oidc signature verification failed; rebuilding key set for rotation retry") - newKS := oidc.NewRemoteKeySet(oidc.ClientContext(context.Background(), v.httpClient), v.jwksURL) - v.verifier = oidc.NewVerifier(v.issuer, newKS, &oidc.Config{ - SupportedSigningAlgs: []string{string(oidc.RS256)}, - SkipClientIDCheck: true, - SkipExpiryCheck: true, - }) + v.verifier = v.newVerifier() v.lastRebuild = v.now() } newVer := v.verifier diff --git a/internal/server/scheduledauth/validator_test.go b/internal/server/scheduledauth/validator_test.go index da3cc623d..51ff2d9c5 100644 --- a/internal/server/scheduledauth/validator_test.go +++ b/internal/server/scheduledauth/validator_test.go @@ -625,12 +625,12 @@ func TestVerifyRetry_RebuildRateLimited(t *testing.T) { t.Fatalf("lastRebuild must not change on rate-limited token, got %v", rb2) } - // Sanity: at least two JWKS fetches must have occurred (the lazy initial - // fetch and the rebuild retry). The exact count is non-deterministic due - // to go-oidc's goroutine scheduling (see comment above), so we only - // assert a lower bound. - if hits := srv.hits.Load(); hits < 2 { - t.Fatalf("expected at least 2 JWKS fetches, got %d", hits) + // Exactly one JWKS fetch: token 1's refresh, which started after the + // token arrived and lacked kid-X, puts kid-X in the negative kid cache + // (issue #75). The rebuild retry and token 2 are then rejected without + // reaching go-oidc, so neither fetches. + if hits := srv.hits.Load(); hits != 1 { + t.Fatalf("expected exactly 1 JWKS fetch, got %d", hits) } }