fix(oidc): retry issuer discovery with bounded back-off at start-up
A 503 from the issuer's discovery endpoint made `serve` exit and the pod restart. New now retries discovery 5 times (2+4+8+16 s back-off, ~30 s) before failing as before; a cancelled ctx ends the back-off at once. WithDiscoveryRetry lets tests shrink the schedule. Closes #59 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5.5
@@ -0,0 +1,61 @@
|
||||
package oidc_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/web/oidc"
|
||||
)
|
||||
|
||||
func testConfig(issuer string) oidc.Config {
|
||||
return oidc.Config{
|
||||
Issuer: issuer,
|
||||
ClientID: testClientID,
|
||||
ClientSecret: "test-client-secret",
|
||||
RedirectURL: "http://tapir.test/auth/callback",
|
||||
SessionSecret: "test-session-secret-please-change",
|
||||
}
|
||||
}
|
||||
|
||||
// #59: a discovery 503 at start-up is retried instead of failing New.
|
||||
func TestNewRetriesDiscoveryAfter503UntilIssuerAnswers(t *testing.T) {
|
||||
f := newFakeIssuer(t)
|
||||
f.unavailableFor = 2
|
||||
|
||||
auth, err := oidc.New(context.Background(), testConfig(f.server.URL),
|
||||
oidc.WithDiscoveryRetry(5, time.Millisecond))
|
||||
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, auth)
|
||||
require.Equal(t, int32(3), f.discoveryHits.Load())
|
||||
}
|
||||
|
||||
func TestNewReturnsDiscoveryErrorAfterAttemptsExhausted(t *testing.T) {
|
||||
f := newFakeIssuer(t)
|
||||
f.unavailableFor = 1000
|
||||
|
||||
_, err := oidc.New(context.Background(), testConfig(f.server.URL),
|
||||
oidc.WithDiscoveryRetry(3, time.Millisecond))
|
||||
|
||||
require.ErrorContains(t, err, "503")
|
||||
require.Equal(t, int32(3), f.discoveryHits.Load())
|
||||
}
|
||||
|
||||
func TestNewStopsDiscoveryBackoffWhenContextCancelled(t *testing.T) {
|
||||
f := newFakeIssuer(t)
|
||||
f.unavailableFor = 1000
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
|
||||
defer cancel()
|
||||
|
||||
start := time.Now()
|
||||
_, err := oidc.New(ctx, testConfig(f.server.URL),
|
||||
oidc.WithDiscoveryRetry(5, time.Hour))
|
||||
|
||||
require.True(t, errors.Is(err, context.DeadlineExceeded), "err = %v", err)
|
||||
require.Less(t, time.Since(start), 5*time.Second)
|
||||
require.Equal(t, int32(1), f.discoveryHits.Load())
|
||||
}
|
||||
+66
-16
@@ -57,6 +57,10 @@ const (
|
||||
pendingTTL = 10 * time.Minute
|
||||
sessionCookie = "tapir_session"
|
||||
loginPath = "/auth/login"
|
||||
|
||||
// Discovery retry (#59): 5 tries, 2+4+8+16 s back-off ≈ 30 s, then fail.
|
||||
defaultDiscoveryAttempts = 5
|
||||
defaultDiscoveryDelay = 2 * time.Second
|
||||
)
|
||||
|
||||
// DexAuth implements web.Auth against Dex.
|
||||
@@ -70,6 +74,9 @@ type DexAuth struct {
|
||||
sessionTTL time.Duration
|
||||
now func() time.Time
|
||||
insecure bool // tests only: drop the Secure attribute so cookies flow over http
|
||||
|
||||
discoveryAttempts int
|
||||
discoveryDelay time.Duration
|
||||
}
|
||||
|
||||
var _ web.Auth = (*DexAuth)(nil)
|
||||
@@ -96,14 +103,55 @@ func WithSessionTTL(ttl time.Duration) Option {
|
||||
}
|
||||
}
|
||||
|
||||
// WithDiscoveryRetry overrides the discovery attempt count and the initial
|
||||
// back-off delay (doubled after each failed attempt).
|
||||
func WithDiscoveryRetry(attempts int, delay time.Duration) Option {
|
||||
return func(d *DexAuth) {
|
||||
if attempts > 0 {
|
||||
d.discoveryAttempts = attempts
|
||||
}
|
||||
if delay > 0 {
|
||||
d.discoveryDelay = delay
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// WithInsecureCookies drops the Secure cookie attribute. TEST ONLY — it lets
|
||||
// the session cookie travel over plain http; never use it in production.
|
||||
func WithInsecureCookies() Option {
|
||||
return func(d *DexAuth) { d.insecure = true }
|
||||
}
|
||||
|
||||
// New discovers the issuer and returns a configured DexAuth. ctx bounds the
|
||||
// discovery request only.
|
||||
// discover runs OIDC discovery up to attempts times, doubling delay between
|
||||
// tries (#59: an issuer briefly unavailable behind its proxy answered 503 and
|
||||
// serve exited). A cancelled ctx ends the back-off immediately with ctx's error.
|
||||
func discover(ctx context.Context, issuer string, attempts int, delay time.Duration) (*oidc.Provider, error) {
|
||||
var lastErr error
|
||||
for i := range attempts {
|
||||
if i > 0 {
|
||||
t := time.NewTimer(delay)
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
t.Stop()
|
||||
return nil, fmt.Errorf("after %d attempts (last: %v): %w", i, lastErr, ctx.Err())
|
||||
case <-t.C:
|
||||
}
|
||||
delay *= 2
|
||||
}
|
||||
p, err := oidc.NewProvider(ctx, issuer)
|
||||
if err == nil {
|
||||
return p, nil
|
||||
}
|
||||
lastErr = err
|
||||
if ctx.Err() != nil {
|
||||
return nil, fmt.Errorf("after %d attempts: %w", i+1, ctx.Err())
|
||||
}
|
||||
}
|
||||
return nil, fmt.Errorf("after %d attempts: %w", attempts, lastErr)
|
||||
}
|
||||
|
||||
// New discovers the issuer and returns a configured DexAuth. ctx bounds discovery,
|
||||
// including its retry back-off.
|
||||
func New(ctx context.Context, cfg Config, opts ...Option) (*DexAuth, error) {
|
||||
for name, val := range map[string]string{
|
||||
"issuer": cfg.Issuer,
|
||||
@@ -117,28 +165,30 @@ func New(ctx context.Context, cfg Config, opts ...Option) (*DexAuth, error) {
|
||||
}
|
||||
}
|
||||
|
||||
provider, err := oidc.NewProvider(ctx, cfg.Issuer)
|
||||
d := &DexAuth{
|
||||
cfg: cfg,
|
||||
pending: newPendingStore(),
|
||||
secret: []byte(cfg.SessionSecret),
|
||||
sessionTTL: defaultSessionTTL,
|
||||
now: time.Now,
|
||||
discoveryAttempts: defaultDiscoveryAttempts,
|
||||
discoveryDelay: defaultDiscoveryDelay,
|
||||
}
|
||||
for _, o := range opts {
|
||||
o(d)
|
||||
}
|
||||
|
||||
provider, err := discover(ctx, cfg.Issuer, d.discoveryAttempts, d.discoveryDelay)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("oidc: discover issuer: %w", err)
|
||||
}
|
||||
|
||||
d := &DexAuth{
|
||||
cfg: cfg,
|
||||
verifier: provider.Verifier(&oidc.Config{ClientID: cfg.ClientID}),
|
||||
oauth: &oauth2.Config{
|
||||
d.verifier = provider.Verifier(&oidc.Config{ClientID: cfg.ClientID})
|
||||
d.oauth = &oauth2.Config{
|
||||
ClientID: cfg.ClientID,
|
||||
ClientSecret: cfg.ClientSecret,
|
||||
Endpoint: provider.Endpoint(),
|
||||
RedirectURL: cfg.RedirectURL,
|
||||
Scopes: []string{oidc.ScopeOpenID, "profile", "email"},
|
||||
},
|
||||
pending: newPendingStore(),
|
||||
secret: []byte(cfg.SessionSecret),
|
||||
sessionTTL: defaultSessionTTL,
|
||||
now: time.Now,
|
||||
}
|
||||
for _, o := range opts {
|
||||
o(d)
|
||||
}
|
||||
return d, nil
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -34,6 +35,11 @@ type fakeIssuer struct {
|
||||
sub string
|
||||
email string
|
||||
nonce string
|
||||
|
||||
// discoveryHits counts requests to the discovery document; the first
|
||||
// unavailableFor of them answer 503 (issuer briefly down behind its proxy).
|
||||
discoveryHits atomic.Int32
|
||||
unavailableFor int32
|
||||
}
|
||||
|
||||
const testKID = "test-key"
|
||||
@@ -47,6 +53,10 @@ func newFakeIssuer(t *testing.T) *fakeIssuer {
|
||||
|
||||
mux := http.NewServeMux()
|
||||
mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, _ *http.Request) {
|
||||
if f.discoveryHits.Add(1) <= f.unavailableFor {
|
||||
http.Error(w, "no available server", http.StatusServiceUnavailable)
|
||||
return
|
||||
}
|
||||
writeJSON(t, w, map[string]any{
|
||||
"issuer": f.server.URL,
|
||||
"authorization_endpoint": f.server.URL + "/authorize",
|
||||
|
||||
Reference in New Issue
Block a user