diff --git a/pkg/auth/discovery/discovery.go b/pkg/auth/discovery/discovery.go index 1d31e70b09..9c6979083c 100644 --- a/pkg/auth/discovery/discovery.go +++ b/pkg/auth/discovery/discovery.go @@ -943,6 +943,7 @@ func createOAuthConfig(ctx context.Context, issuer string, config *OAuthFlowConf true, // Enable PKCE by default for security config.CallbackPort, config.Resource, + !config.AllowPrivateIPs, ) if err != nil { return nil, err diff --git a/pkg/auth/discovery/discovery_test.go b/pkg/auth/discovery/discovery_test.go index 16b236dbdb..f76466081e 100644 --- a/pkg/auth/discovery/discovery_test.go +++ b/pkg/auth/discovery/discovery_test.go @@ -1391,6 +1391,29 @@ func startIssuerServer(t *testing.T, tokenEndpoint func(issuerURL string) string return server.URL } +// TestCreateOAuthConfig_BlocksPrivateIssuerDiscoveryFallback verifies that the +// OIDC fallback preserves OAuthFlowConfig's private-IP policy. +func TestCreateOAuthConfig_BlocksPrivateIssuerDiscoveryFallback(t *testing.T) { + t.Parallel() + + var discoveryHits atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + discoveryHits.Add(1) + http.Error(w, "unexpected discovery request", http.StatusInternalServerError) + })) + t.Cleanup(server.Close) + + config, err := createOAuthConfig(context.Background(), server.URL, &OAuthFlowConfig{ + ClientID: "test-client", + AllowPrivateIPs: false, + }) + + require.Error(t, err) + assert.ErrorContains(t, err, networking.ErrPrivateIpAddress) + assert.Nil(t, config) + assert.Zero(t, discoveryHits.Load(), "OIDC fallback must not reach a private issuer") +} + // TestCreateOAuthConfig_DiscoveredTokenEndpoint is the regression test for // GHSA-3768-rwj3-38p2. An operator-configured issuer that names its own // authority in its metadata keeps the operator's trust; one that names a @@ -1440,6 +1463,7 @@ func TestCreateOAuthConfig_DiscoveredTokenEndpoint(t *testing.T) { ClientID: "test-client", IssuerTrusted: true, TokenEndpointTrusted: true, + AllowPrivateIPs: true, }) require.NoError(t, err) diff --git a/pkg/auth/oauth/oidc.go b/pkg/auth/oauth/oidc.go index 08fed930d4..cce7f44376 100644 --- a/pkg/auth/oauth/oidc.go +++ b/pkg/auth/oauth/oidc.go @@ -238,8 +238,11 @@ func CreateOAuthConfigFromOIDC( usePKCE bool, callbackPort int, resource string, + blockPrivateIPs bool, ) (*Config, error) { - return createOAuthConfigFromOIDCWithClient(ctx, issuer, clientID, clientSecret, scopes, usePKCE, callbackPort, resource, nil) + return createOAuthConfigFromOIDCWithClient( + ctx, issuer, clientID, clientSecret, scopes, usePKCE, callbackPort, resource, nil, blockPrivateIPs, + ) } // createOAuthConfigFromOIDCWithClient creates an OAuth config from OIDC discovery with a custom HTTP client (private for testing) @@ -251,11 +254,10 @@ func createOAuthConfigFromOIDCWithClient( callbackPort int, resource string, client networking.HTTPClient, + blockPrivateIPs bool, ) (*Config, error) { // Discover OIDC endpoints (insecureAllowHTTP is false for OAuth config creation). - // blockPrivateIPs=false here preserves this call path's existing behavior; - // it is unrelated to the CLI DCR fallback fixed in DiscoverOIDCEndpoints. - doc, err := discoverOIDCEndpointsWithClient(ctx, issuer, client, false, false) + doc, err := discoverOIDCEndpointsWithClient(ctx, issuer, client, false, blockPrivateIPs) if err != nil { return nil, fmt.Errorf("failed to discover OIDC endpoints: %w", err) } diff --git a/pkg/auth/oauth/oidc_test.go b/pkg/auth/oauth/oidc_test.go index b3a51be416..4794b24df7 100644 --- a/pkg/auth/oauth/oidc_test.go +++ b/pkg/auth/oauth/oidc_test.go @@ -12,6 +12,7 @@ import ( "net/http/httptest" "net/url" "strings" + "sync/atomic" "testing" "time" @@ -1258,6 +1259,7 @@ func TestCreateOAuthConfigFromOIDC_Production(t *testing.T) { 0, // Use auto-select port for tests "", // No resource client, + false, ) if tt.expectError { @@ -1277,6 +1279,58 @@ func TestCreateOAuthConfigFromOIDC_Production(t *testing.T) { } } +func TestCreateOAuthConfigFromOIDC_PrivateIPPolicy(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + blockPrivateIPs bool + wantErr bool + }{ + {name: "blocks loopback issuer", blockPrivateIPs: true, wantErr: true}, + {name: "allows loopback issuer", blockPrivateIPs: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var hits atomic.Int32 + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + hits.Add(1) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(oauthproto.OIDCDiscoveryDocument{ + AuthorizationServerMetadata: oauthproto.AuthorizationServerMetadata{ + Issuer: server.URL, + AuthorizationEndpoint: server.URL + "/authorize", + TokenEndpoint: server.URL + "/token", + JWKSURI: server.URL + "/jwks", + RegistrationEndpoint: server.URL + "/register", + }, + }) + })) + t.Cleanup(server.Close) + + config, err := CreateOAuthConfigFromOIDC( + context.Background(), server.URL, "test-client", "", nil, true, 0, "", tt.blockPrivateIPs, + ) + if tt.wantErr { + require.Error(t, err) + assert.ErrorContains(t, err, networking.ErrPrivateIpAddress) + assert.Nil(t, config) + assert.Zero(t, hits.Load(), "blocked issuer must not reach the listener") + return + } + + require.NoError(t, err) + assert.Equal(t, server.URL+"/authorize", config.AuthURL) + assert.Equal(t, server.URL+"/token", config.TokenURL) + assert.Equal(t, int32(1), hits.Load()) + }) + } +} + func TestValidateEndpointURL_AdditionalCases(t *testing.T) { t.Parallel() tests := []struct { diff --git a/pkg/auth/tokensource/tokensource.go b/pkg/auth/tokensource/tokensource.go index 5d0895cdc5..0fa9beb885 100644 --- a/pkg/auth/tokensource/tokensource.go +++ b/pkg/auth/tokensource/tokensource.go @@ -402,6 +402,7 @@ func (t *OAuthTokenSource) buildFlowConfig(ctx context.Context) (*oauth.Config, true, // always use PKCE t.opts.OIDC.CallbackPort, t.opts.OIDC.Audience, + false, // issuer is operator-configured ) if err != nil { return nil, err