Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/auth/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions pkg/auth/discovery/discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1440,6 +1463,7 @@ func TestCreateOAuthConfig_DiscoveredTokenEndpoint(t *testing.T) {
ClientID: "test-client",
IssuerTrusted: true,
TokenEndpointTrusted: true,
AllowPrivateIPs: true,
})
require.NoError(t, err)

Expand Down
10 changes: 6 additions & 4 deletions pkg/auth/oauth/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down
54 changes: 54 additions & 0 deletions pkg/auth/oauth/oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"net/http/httptest"
"net/url"
"strings"
"sync/atomic"
"testing"
"time"

Expand Down Expand Up @@ -1258,6 +1259,7 @@ func TestCreateOAuthConfigFromOIDC_Production(t *testing.T) {
0, // Use auto-select port for tests
"", // No resource
client,
false,
)

if tt.expectError {
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions pkg/auth/tokensource/tokensource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading