diff --git a/sdk/go/openshell/v1/oidc/credentials_auth.go b/sdk/go/openshell/v1/oidc/credentials_auth.go index 24012c8bf4..2c6d9a0e74 100644 --- a/sdk/go/openshell/v1/oidc/credentials_auth.go +++ b/sdk/go/openshell/v1/oidc/credentials_auth.go @@ -71,7 +71,13 @@ func (a *clientCredentialsAuth) accessTokenForExchange() (string, error) { return accessToken, nil } - exchangeCtx, cancel := context.WithTimeout(context.Background(), a.cfg.timeout) + // A zero timeout means "no deadline"; only bound the exchange when a + // positive timeout was configured (mirrors Login and DeviceFlow). + exchangeCtx := context.Background() + cancel := context.CancelFunc(func() {}) + if a.cfg.timeout > 0 { + exchangeCtx, cancel = context.WithTimeout(exchangeCtx, a.cfg.timeout) + } defer cancel() token, err := exchangeClientCredentials(exchangeCtx, a.cfg, true) if err != nil { diff --git a/sdk/go/openshell/v1/oidc/credentials_test.go b/sdk/go/openshell/v1/oidc/credentials_test.go index 3269c44d40..a2e11d1a80 100644 --- a/sdk/go/openshell/v1/oidc/credentials_test.go +++ b/sdk/go/openshell/v1/oidc/credentials_test.go @@ -308,6 +308,40 @@ func TestClientCredentialsAuthLateFlightReusesCachedToken(t *testing.T) { assert.Equal(t, "cached-token", accessToken) } +// A zero timeout means "no deadline". The exchange context must not be born +// expired, so the token exchange should still succeed. +func TestClientCredentialsAuthZeroTimeoutHasNoDeadline(t *testing.T) { + resetDiscoveryCache() + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/.well-known/openid-configuration" { + _ = json.NewEncoder(w).Encode(map[string]string{ + "issuer": server.URL, + "authorization_endpoint": server.URL + "/authorize", + "token_endpoint": server.URL + "/token", + }) + return + } + _, _ = w.Write([]byte(tokenResponseJSON("token", "", 3600))) + })) + t.Cleanup(server.Close) + + auth, err := NewClientCredentialsAuth( + WithIssuer(server.URL), + WithClientID("client"), + WithClientSecret("secret"), + WithTimeout(0), // explicitly no timeout + ) + require.NoError(t, err) + + // The explicit zero timeout must be preserved (not replaced by the default). + require.Zero(t, auth.(*clientCredentialsAuth).cfg.timeout) + + metadata, err := auth.GetRequestMetadata(context.Background()) + require.NoError(t, err) + assert.Equal(t, "Bearer token", metadata["authorization"]) +} + func TestClientCredentialsAuthCancellationDoesNotPoisonSharedExchange(t *testing.T) { resetDiscoveryCache() var server *httptest.Server diff --git a/sdk/go/openshell/v1/oidc/options.go b/sdk/go/openshell/v1/oidc/options.go index d4d856ba4b..141e964be5 100644 --- a/sdk/go/openshell/v1/oidc/options.go +++ b/sdk/go/openshell/v1/oidc/options.go @@ -33,6 +33,7 @@ type loginConfig struct { scopesSet bool callbackPort int timeout time.Duration + timeoutSet bool keyboardFlow bool inMemory bool displayFunc func(verificationURL, userCode string) @@ -48,12 +49,14 @@ type loginConfig struct { // applyDefaults fills in default values for fields that were not set // by any option function. func (c *loginConfig) applyDefaults() { - if len(c.scopes) == 0 { + // Check set-ness, not the zero value, so an explicitly-set empty scope + // list or zero timeout is honored instead of being replaced by defaults. + if !c.scopesSet { // Deep copy to avoid callers mutating the package-level slice. c.scopes = make([]string, len(defaultScopes)) copy(c.scopes, defaultScopes) } - if c.timeout == 0 { + if !c.timeoutSet { c.timeout = defaultTimeout } } @@ -125,6 +128,7 @@ func WithCallbackPort(port int) LoginOption { func WithTimeout(d time.Duration) LoginOption { return func(c *loginConfig) { c.timeout = d + c.timeoutSet = true } } diff --git a/sdk/go/openshell/v1/oidc/options_test.go b/sdk/go/openshell/v1/oidc/options_test.go index 28d17c06b4..8af2df7ff7 100644 --- a/sdk/go/openshell/v1/oidc/options_test.go +++ b/sdk/go/openshell/v1/oidc/options_test.go @@ -82,6 +82,26 @@ func TestWithTimeout(t *testing.T) { assert.Equal(t, 5*time.Minute, cfg.timeout) } +func TestWithScopes_ExplicitEmptyNotOverridden(t *testing.T) { + var cfg loginConfig + WithScopes()(&cfg) // caller explicitly requests no scopes + cfg.applyDefaults() + + // An explicitly-set empty scope list must be honored, not replaced by defaults. + assert.True(t, cfg.scopesSet) + assert.Empty(t, cfg.scopes) +} + +func TestWithTimeout_ExplicitZeroNotOverridden(t *testing.T) { + var cfg loginConfig + WithTimeout(0)(&cfg) // caller explicitly requests no timeout + cfg.applyDefaults() + + // An explicitly-set zero timeout must be honored, not replaced by the default. + assert.True(t, cfg.timeoutSet) + assert.Zero(t, cfg.timeout) +} + func TestWithKeyboardFlow(t *testing.T) { var cfg loginConfig WithKeyboardFlow()(&cfg)