Release OIDC discovery waiters when discovery panics
This commit is contained in:
+27
-13
@@ -21,6 +21,7 @@ type BearerOIDC struct {
|
|||||||
audiences []string
|
audiences []string
|
||||||
timeout time.Duration
|
timeout time.Duration
|
||||||
backoff time.Duration
|
backoff time.Duration
|
||||||
|
discover func(context.Context, string) (*oidc.Provider, error)
|
||||||
|
|
||||||
mu sync.Mutex
|
mu sync.Mutex
|
||||||
verifier *oidc.IDTokenVerifier
|
verifier *oidc.IDTokenVerifier
|
||||||
@@ -35,7 +36,7 @@ func NewBearerOIDC(issuer string, audiences []string) *BearerOIDC {
|
|||||||
if issuer == "" {
|
if issuer == "" {
|
||||||
return nil
|
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
|
// 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.inflight = done
|
||||||
b.mu.Unlock()
|
b.mu.Unlock()
|
||||||
|
|
||||||
// The provider keeps this context and client for JWKS refreshes, so the
|
v, err := b.discoverOnce(done)
|
||||||
// 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)
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
b.failedAt = time.Now()
|
|
||||||
slog.Error("oidc discovery failed", "issuer", b.issuer, "error", err)
|
slog.Error("oidc discovery failed", "issuer", b.issuer, "error", err)
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
b.verifier = p.Verifier(&oidc.Config{SkipClientIDCheck: true})
|
return v, nil
|
||||||
return b.verifier, 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
|
// Require gates every request through next on a valid token. A nil receiver
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package auth
|
package auth
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"context"
|
||||||
"crypto/rand"
|
"crypto/rand"
|
||||||
"crypto/rsa"
|
"crypto/rsa"
|
||||||
"encoding/base64"
|
"encoding/base64"
|
||||||
@@ -12,6 +13,7 @@ import (
|
|||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/coreos/go-oidc/v3/oidc"
|
||||||
"github.com/go-jose/go-jose/v4"
|
"github.com/go-jose/go-jose/v4"
|
||||||
"github.com/go-jose/go-jose/v4/jwt"
|
"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) {
|
func TestBearerOIDCConcurrentFirstRequestsWaitForDiscovery(t *testing.T) {
|
||||||
iss := testsupport.NewOIDCIssuer()
|
iss := testsupport.NewOIDCIssuer()
|
||||||
defer iss.Close()
|
defer iss.Close()
|
||||||
|
|||||||
Reference in New Issue
Block a user