From 4d9f2bb6937a536ca6882da63e5f92970356946a Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Wed, 14 Oct 2026 22:36:00 +1100 Subject: [PATCH] Release OIDC discovery waiters when discovery panics --- internal/auth/oidc.go | 40 +++++++++++++++++++++++++------------- internal/auth/oidc_test.go | 25 ++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 13 deletions(-) diff --git a/internal/auth/oidc.go b/internal/auth/oidc.go index 86c63d9..d544bd0 100644 --- a/internal/auth/oidc.go +++ b/internal/auth/oidc.go @@ -21,6 +21,7 @@ type BearerOIDC struct { audiences []string timeout time.Duration backoff time.Duration + discover func(context.Context, string) (*oidc.Provider, error) mu sync.Mutex verifier *oidc.IDTokenVerifier @@ -35,7 +36,7 @@ func NewBearerOIDC(issuer string, audiences []string) *BearerOIDC { if issuer == "" { return nil } - return &BearerOIDC{issuer: issuer, audiences: audiences, timeout: 10 * time.Second, backoff: 15 * time.Second} + return &BearerOIDC{issuer: issuer, audiences: audiences, timeout: 10 * time.Second, backoff: 15 * time.Second, discover: oidc.NewProvider} } // getVerifier runs at most one discovery at a time without holding the lock @@ -69,22 +70,35 @@ func (b *BearerOIDC) getVerifier(ctx context.Context) (*oidc.IDTokenVerifier, er b.inflight = done b.mu.Unlock() - // The provider keeps this context and client for JWKS refreshes, so the - // deadline lives on the client rather than the context. - client := &http.Client{Timeout: b.timeout} - p, err := oidc.NewProvider(oidc.ClientContext(context.Background(), client), b.issuer) - - b.mu.Lock() - defer b.mu.Unlock() - b.inflight = nil - defer close(done) + v, err := b.discoverOnce(done) if err != nil { - b.failedAt = time.Now() slog.Error("oidc discovery failed", "issuer", b.issuer, "error", err) return nil, err } - b.verifier = p.Verifier(&oidc.Config{SkipClientIDCheck: true}) - return b.verifier, nil + return v, nil +} + +// discoverOnce releases inflight and waiters on every exit, a panic included, +// recording failedAt unless discovery succeeded. +func (b *BearerOIDC) discoverOnce(done chan struct{}) (v *oidc.IDTokenVerifier, err error) { + defer func() { + b.mu.Lock() + b.inflight = nil + if v == nil { + b.failedAt = time.Now() + } + b.verifier = v + b.mu.Unlock() + close(done) + }() + // The provider keeps this context and client for JWKS refreshes, so the + // deadline lives on the client rather than the context. + client := &http.Client{Timeout: b.timeout} + p, err := b.discover(oidc.ClientContext(context.Background(), client), b.issuer) + if err != nil { + return nil, err + } + return p.Verifier(&oidc.Config{SkipClientIDCheck: true}), nil } // Require gates every request through next on a valid token. A nil receiver diff --git a/internal/auth/oidc_test.go b/internal/auth/oidc_test.go index 8c552b6..c7e9c6d 100644 --- a/internal/auth/oidc_test.go +++ b/internal/auth/oidc_test.go @@ -1,6 +1,7 @@ package auth import ( + "context" "crypto/rand" "crypto/rsa" "encoding/base64" @@ -12,6 +13,7 @@ import ( "testing" "time" + "github.com/coreos/go-oidc/v3/oidc" "github.com/go-jose/go-jose/v4" "github.com/go-jose/go-jose/v4/jwt" @@ -146,6 +148,29 @@ func TestBearerOIDCHangingIssuerBacksOff(t *testing.T) { } } +func TestBearerOIDCPanickingDiscoveryBacksOff(t *testing.T) { + b := NewBearerOIDC("http://issuer.invalid", nil) + b.backoff = time.Hour + b.discover = func(context.Context, string) (*oidc.Provider, error) { panic("boom") } + h := b.Require(ok) + + func() { + defer func() { _ = recover() }() + serve(h, http.MethodPost, "x.y.z") + }() + + got := make(chan int, 1) + go func() { got <- serve(h, http.MethodPost, "x.y.z") }() + select { + case code := <-got: + if code != http.StatusServiceUnavailable { + t.Fatalf("after panic: status %d, want 503", code) + } + case <-time.After(2 * time.Second): + t.Fatal("after panic: request hung on stale inflight discovery") + } +} + func TestBearerOIDCConcurrentFirstRequestsWaitForDiscovery(t *testing.T) { iss := testsupport.NewOIDCIssuer() defer iss.Close()