From 6cf48cd66828d1b5c31aa40fb36bccc242bae261 Mon Sep 17 00:00:00 2001 From: Trong Huu Nguyen Date: Mon, 10 Aug 2026 12:33:16 +0200 Subject: [PATCH] feat(openid/config): validate client assertion alg against provider metadata Fail at startup rather than on the first token request when the identity provider does not accept the client assertion algorithm. The check is skipped when the provider omits the field, which is optional in Discovery. --- pkg/openid/config/config.go | 2 +- pkg/openid/config/provider.go | 21 ++++++++++--- pkg/openid/config/provider_fetch_test.go | 4 +-- pkg/openid/config/provider_test.go | 40 +++++++++++++++++++++++- 4 files changed, 59 insertions(+), 8 deletions(-) diff --git a/pkg/openid/config/config.go b/pkg/openid/config/config.go index 8b5e9a2..e91ae3e 100644 --- a/pkg/openid/config/config.go +++ b/pkg/openid/config/config.go @@ -30,7 +30,7 @@ func NewConfig(ctx context.Context, cfg *wonderwallconfig.Config) (Config, error return nil, err } - providerCfg, err := NewProviderConfig(ctx, cfg) + providerCfg, err := NewProviderConfig(ctx, cfg, clientCfg.ClientJWKAlgorithm()) if err != nil { return nil, err } diff --git a/pkg/openid/config/provider.go b/pkg/openid/config/provider.go index b9a0c27..a39130f 100644 --- a/pkg/openid/config/provider.go +++ b/pkg/openid/config/provider.go @@ -89,7 +89,7 @@ func (p *provider) SidClaimRequired() bool { // wellKnownTimeout bounds the fetch of the provider's metadata document at startup. const wellKnownTimeout = 10 * time.Second -func NewProviderConfig(ctx context.Context, cfg *config.Config) (Provider, error) { +func NewProviderConfig(ctx context.Context, cfg *config.Config, clientAssertionAlg jwa.KeyAlgorithm) (Provider, error) { ctx, cancel := context.WithTimeout(ctx, wellKnownTimeout) defer cancel() @@ -115,7 +115,7 @@ func NewProviderConfig(ctx context.Context, cfg *config.Config) (Provider, error return nil, fmt.Errorf("decoding well known configuration: %w", err) } - err = providerCfg.Validate(cfg.OpenID) + err = providerCfg.Validate(cfg.OpenID, clientAssertionAlg) if err != nil { return nil, fmt.Errorf("validating well known configuration: %w", err) } @@ -163,6 +163,7 @@ type ProviderMetadata struct { SubjectTypesSupported []string `json:"subject_types_supported"` TokenEndpoint string `json:"token_endpoint"` TokenEndpointAuthMethodsSupported []string `json:"token_endpoint_auth_methods_supported"` + TokenEndpointAuthSigningAlgValuesSupported []string `json:"token_endpoint_auth_signing_alg_values_supported"` UILocalesSupported Supported `json:"ui_locales_supported"` UserInfoEndpoint string `json:"userinfo_endpoint"` } @@ -172,7 +173,7 @@ func (c *ProviderMetadata) Print() { Debugf("openid provider config: %+v", c) } -func (c *ProviderMetadata) Validate(cfg config.OpenID) error { +func (c *ProviderMetadata) Validate(cfg config.OpenID, clientAssertionAlg jwa.KeyAlgorithm) error { err := c.validateAcrValues(cfg.ACRValues) if err != nil { return err @@ -188,7 +189,7 @@ func (c *ProviderMetadata) Validate(cfg config.OpenID) error { return err } - return nil + return c.validateClientAssertionSigningAlg(clientAssertionAlg) } func (c *ProviderMetadata) validateAcrValues(acrValue string) error { @@ -221,6 +222,18 @@ func (c *ProviderMetadata) validateJWKSFallbackAlg(algorithm string) error { return fmt.Errorf("identity provider does not support '%s=%s', must be one of %s", config.OpenIDJWKSFallbackAlg, algorithm, c.IDTokenSigningAlgValuesSupported) } +func (c *ProviderMetadata) validateClientAssertionSigningAlg(algorithm jwa.KeyAlgorithm) error { + if len(c.TokenEndpointAuthSigningAlgValuesSupported) == 0 || algorithm == nil { + return nil + } + + if slices.Contains(c.TokenEndpointAuthSigningAlgValuesSupported, algorithm.String()) { + return nil + } + + return fmt.Errorf("identity provider does not support client assertion signing algorithm %q, must be one of %s", algorithm, c.TokenEndpointAuthSigningAlgValuesSupported) +} + type Supported []string func (in Supported) Contains(value string) bool { diff --git a/pkg/openid/config/provider_fetch_test.go b/pkg/openid/config/provider_fetch_test.go index 904fe17..df65b1a 100644 --- a/pkg/openid/config/provider_fetch_test.go +++ b/pkg/openid/config/provider_fetch_test.go @@ -24,7 +24,7 @@ func TestNewProviderConfig_NonOK(t *testing.T) { cfg := mock.Config() cfg.OpenID.WellKnownURL = server.URL - _, err := openidconfig.NewProviderConfig(context.Background(), cfg) + _, err := openidconfig.NewProviderConfig(context.Background(), cfg, nil) require.Error(t, err) assert.ErrorContains(t, err, "responded with HTTP") } @@ -37,6 +37,6 @@ func TestNewProviderConfig_CancelledContext(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - _, err := openidconfig.NewProviderConfig(ctx, cfg) + _, err := openidconfig.NewProviderConfig(ctx, cfg, nil) assert.Error(t, err) } diff --git a/pkg/openid/config/provider_test.go b/pkg/openid/config/provider_test.go index dccb3dd..0eeda36 100644 --- a/pkg/openid/config/provider_test.go +++ b/pkg/openid/config/provider_test.go @@ -3,8 +3,11 @@ package config_test import ( "testing" + "github.com/lestrrat-go/jwx/v3/jwa" + "github.com/nais/wonderwall/internal/crypto" "github.com/nais/wonderwall/pkg/config" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/nais/wonderwall/pkg/mock" openidconfig "github.com/nais/wonderwall/pkg/openid/config" @@ -65,8 +68,43 @@ func TestProviderMetadata_Validate(t *testing.T) { cfg.OpenID.JWKSFallbackAlg = tt.config.JWKSFallbackAlg } - err := metadata.Validate(cfg.OpenID) + err := metadata.Validate(cfg.OpenID, nil) tt.assertion(t, err) }) } } + +func TestProviderMetadata_ValidateClientAssertionSigningAlg(t *testing.T) { + key, err := crypto.NewJwk() + require.NoError(t, err) + algorithm, ok := key.Algorithm() + require.True(t, ok) + + for _, tt := range []struct { + name string + supported []string + clientAlg jwa.KeyAlgorithm + assertion assert.ErrorAssertionFunc + }{ + {name: "supported", supported: []string{"RS256"}, clientAlg: algorithm, assertion: assert.NoError}, + {name: "unsupported", supported: []string{"PS256"}, clientAlg: algorithm, assertion: func(t assert.TestingT, err error, msgAndArgs ...any) bool { + return assert.ErrorContains(t, err, "does not support client assertion signing algorithm", msgAndArgs...) + }}, + {name: "metadata field absent", clientAlg: algorithm, assertion: assert.NoError}, + // client_secret has no assertion alg; PS256 proves the check is skipped, not passed. + {name: "no client assertion algorithm", supported: []string{"PS256"}, assertion: assert.NoError}, + } { + t.Run(tt.name, func(t *testing.T) { + // acr and locale pass on empty input, so zeroing them isolates this test. + cfg := mock.Config() + cfg.OpenID.ACRValues = "" + cfg.OpenID.UILocales = "" + + metadata := &openidconfig.ProviderMetadata{ + IDTokenSigningAlgValuesSupported: openidconfig.Supported{cfg.OpenID.JWKSFallbackAlg}, + TokenEndpointAuthSigningAlgValuesSupported: tt.supported, + } + tt.assertion(t, metadata.Validate(cfg.OpenID, tt.clientAlg)) + }) + } +}