Skip to content

Commit 8a57ca9

Browse files
committed
avoid hangs on oidc discovery unresponsive
1 parent 5405a5f commit 8a57ca9

2 files changed

Lines changed: 50 additions & 2 deletions

File tree

internal/node/oidc.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,9 +302,15 @@ func (n *SamNode) InteractiveLogin(ctx context.Context, authURL, tokenURL, clien
302302
return tokenResp.AccessToken, nil
303303
}
304304

305+
// oidcDiscoveryTimeout bounds a single OIDC discovery HTTP call so an
306+
// unresponsive issuer can't hang "join"/"run --join" indefinitely. A var
307+
// (not const) so tests can shrink it instead of waiting out the real value.
308+
var oidcDiscoveryTimeout = 10 * time.Second
309+
305310
// DiscoverTokenURL discovers the token URL from the OIDC issuer.
306311
func (n *SamNode) DiscoverTokenURL(ctx context.Context, issuerURL string) (string, error) {
307-
provider, err := oidc.NewProvider(ctx, issuerURL)
312+
client := &http.Client{Timeout: oidcDiscoveryTimeout}
313+
provider, err := oidc.NewProvider(oidc.ClientContext(ctx, client), issuerURL)
308314
if err != nil {
309315
return "", fmt.Errorf("failed to create OIDC provider: %w", err)
310316
}
@@ -322,7 +328,8 @@ func (n *SamNode) DiscoverTokenURL(ctx context.Context, issuerURL string) (strin
322328

323329
// DiscoverEndpoints discovers both token and authorization endpoints.
324330
func (n *SamNode) DiscoverEndpoints(ctx context.Context, issuerURL string) (tokenURL, authURL string, err error) {
325-
provider, err := oidc.NewProvider(ctx, issuerURL)
331+
client := &http.Client{Timeout: oidcDiscoveryTimeout}
332+
provider, err := oidc.NewProvider(oidc.ClientContext(ctx, client), issuerURL)
326333
if err != nil {
327334
return "", "", fmt.Errorf("failed to create OIDC provider: %w", err)
328335
}

internal/node/oidc_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package node
33
import (
44
"context"
55
"encoding/json"
6+
"net"
67
"net/http"
78
"net/http/httptest"
89
"net/url"
@@ -109,6 +110,46 @@ func TestDiscoverEndpoints(t *testing.T) {
109110
}
110111
}
111112

113+
// TestDiscoverEndpointsDoesNotHangOnUnresponsiveIssuer guards against an
114+
// unbounded hang: DiscoverEndpoints/DiscoverTokenURL used to call
115+
// oidc.NewProvider with no client timeout, so an issuer that accepts a
116+
// connection but never responds could block "join"/"run --join" forever.
117+
func TestDiscoverEndpointsDoesNotHangOnUnresponsiveIssuer(t *testing.T) {
118+
origTimeout := oidcDiscoveryTimeout
119+
oidcDiscoveryTimeout = 50 * time.Millisecond
120+
defer func() { oidcDiscoveryTimeout = origTimeout }()
121+
122+
ln, err := net.Listen("tcp", "127.0.0.1:0")
123+
if err != nil {
124+
t.Fatalf("failed to listen: %v", err)
125+
}
126+
defer func() { _ = ln.Close() }()
127+
go func() {
128+
for {
129+
conn, err := ln.Accept()
130+
if err != nil {
131+
return
132+
}
133+
_ = conn // accepted but deliberately never responds, to simulate a hang
134+
}
135+
}()
136+
137+
node := &SamNode{}
138+
done := make(chan struct{})
139+
go func() {
140+
defer close(done)
141+
if _, _, err := node.DiscoverEndpoints(context.Background(), "http://"+ln.Addr().String()); err == nil {
142+
t.Error("expected DiscoverEndpoints to fail against an unresponsive issuer")
143+
}
144+
}()
145+
146+
select {
147+
case <-done:
148+
case <-time.After(2 * time.Second):
149+
t.Fatal("DiscoverEndpoints hung on an unresponsive issuer instead of timing out")
150+
}
151+
}
152+
112153
func TestInteractiveLoginWithRefresh(t *testing.T) {
113154
t.Setenv("SSH_CLIENT", "")
114155
t.Setenv("SSH_TTY", "")

0 commit comments

Comments
 (0)