Wait on in-flight OIDC discovery instead of returning 503
This commit is contained in:
+31
-14
@@ -22,10 +22,10 @@ type BearerOIDC struct {
|
||||
timeout time.Duration
|
||||
backoff time.Duration
|
||||
|
||||
mu sync.Mutex
|
||||
verifier *oidc.IDTokenVerifier
|
||||
discovering bool
|
||||
failedAt time.Time
|
||||
mu sync.Mutex
|
||||
verifier *oidc.IDTokenVerifier
|
||||
inflight chan struct{}
|
||||
failedAt time.Time
|
||||
}
|
||||
|
||||
var errIssuerUnavailable = errors.New("oidc issuer unavailable")
|
||||
@@ -39,18 +39,34 @@ func NewBearerOIDC(issuer string, audiences []string) *BearerOIDC {
|
||||
}
|
||||
|
||||
// getVerifier runs at most one discovery at a time without holding the lock
|
||||
// across network I/O; callers arriving mid-discovery or within the backoff fail fast.
|
||||
func (b *BearerOIDC) getVerifier() (*oidc.IDTokenVerifier, error) {
|
||||
// across network I/O; concurrent callers wait for it, and only callers within
|
||||
// the post-failure backoff fail fast.
|
||||
func (b *BearerOIDC) getVerifier(ctx context.Context) (*oidc.IDTokenVerifier, error) {
|
||||
b.mu.Lock()
|
||||
if b.verifier != nil || b.discovering || time.Since(b.failedAt) < b.backoff {
|
||||
v := b.verifier
|
||||
if v := b.verifier; v != nil {
|
||||
b.mu.Unlock()
|
||||
if v == nil {
|
||||
return nil, errIssuerUnavailable
|
||||
}
|
||||
return v, nil
|
||||
}
|
||||
b.discovering = true
|
||||
if done := b.inflight; done != nil {
|
||||
b.mu.Unlock()
|
||||
select {
|
||||
case <-done:
|
||||
case <-ctx.Done():
|
||||
return nil, ctx.Err()
|
||||
}
|
||||
b.mu.Lock()
|
||||
defer b.mu.Unlock()
|
||||
if b.verifier == nil {
|
||||
return nil, errIssuerUnavailable
|
||||
}
|
||||
return b.verifier, nil
|
||||
}
|
||||
if time.Since(b.failedAt) < b.backoff {
|
||||
b.mu.Unlock()
|
||||
return nil, errIssuerUnavailable
|
||||
}
|
||||
done := make(chan struct{})
|
||||
b.inflight = done
|
||||
b.mu.Unlock()
|
||||
|
||||
// The provider keeps this context and client for JWKS refreshes, so the
|
||||
@@ -60,7 +76,8 @@ func (b *BearerOIDC) getVerifier() (*oidc.IDTokenVerifier, error) {
|
||||
|
||||
b.mu.Lock()
|
||||
defer b.mu.Unlock()
|
||||
b.discovering = false
|
||||
b.inflight = nil
|
||||
defer close(done)
|
||||
if err != nil {
|
||||
b.failedAt = time.Now()
|
||||
slog.Error("oidc discovery failed", "issuer", b.issuer, "error", err)
|
||||
@@ -83,7 +100,7 @@ func (b *BearerOIDC) Require(next http.Handler) http.Handler {
|
||||
unauthorized(w, "missing bearer token")
|
||||
return
|
||||
}
|
||||
v, err := b.getVerifier()
|
||||
v, err := b.getVerifier(r.Context())
|
||||
if err != nil {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusServiceUnavailable)
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strconv"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -145,6 +146,41 @@ func TestBearerOIDCHangingIssuerBacksOff(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestBearerOIDCConcurrentFirstRequestsWaitForDiscovery(t *testing.T) {
|
||||
iss := testsupport.NewOIDCIssuer()
|
||||
defer iss.Close()
|
||||
var hits atomic.Int32
|
||||
inner := iss.Config.Handler
|
||||
iss.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == "/.well-known/openid-configuration" {
|
||||
hits.Add(1)
|
||||
time.Sleep(200 * time.Millisecond)
|
||||
}
|
||||
inner.ServeHTTP(w, r)
|
||||
})
|
||||
|
||||
h := NewBearerOIDC(iss.URL, nil).Require(ok)
|
||||
tok := iss.Token(nil, "artifactapi", "ci", time.Hour)
|
||||
var wg sync.WaitGroup
|
||||
codes := make([]int, 20)
|
||||
for i := range codes {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
codes[i] = serve(h, http.MethodPost, tok)
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
for i, c := range codes {
|
||||
if c != http.StatusNoContent {
|
||||
t.Errorf("request %d: status %d, want 204", i, c)
|
||||
}
|
||||
}
|
||||
if n := hits.Load(); n != 1 {
|
||||
t.Fatalf("discovery hit %d times, want 1", n)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBearerOIDCRetriesAfterBackoff(t *testing.T) {
|
||||
iss := testsupport.NewOIDCIssuer()
|
||||
defer iss.Close()
|
||||
|
||||
Reference in New Issue
Block a user