From 2c708d554c4ff959ad177edebf7ddbad12ed5539 Mon Sep 17 00:00:00 2001 From: Trong Huu Nguyen Date: Mon, 10 Aug 2026 12:32:04 +0200 Subject: [PATCH] feat(openid/config): require and expose the client JWK algorithm Both the assertion signer and the provider validation derived the algorithm from the key and had to handle a missing "alg" that NewClientConfig already rejects. Validate it once at construction and keep the result. --- docs/configuration.md | 2 +- pkg/config/openid.go | 2 +- pkg/mock/client.go | 12 +++++++ pkg/openid/client/client.go | 5 +-- pkg/openid/config/client.go | 14 ++++++++ pkg/openid/config/client_test.go | 62 ++++++++++++++++++++++++++++++++ 6 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 pkg/openid/config/client_test.go diff --git a/docs/configuration.md b/docs/configuration.md index 3ec9d91..9cad7f1 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -34,7 +34,7 @@ The following flags are available: | `openid.acr-values` | string | | Space separated string that configures the default security level (`acr_values`) parameter for authorization requests. | | `openid.audiences` | strings | | List of additional trusted audiences (other than the client_id) for OpenID Connect id_token validation. | | `openid.client-id` | string | | Client ID for the OpenID client. | -| `openid.client-jwk` | string | | JWK containing the private key for the OpenID client in string format. If configured, this takes precedence over `openid.client-secret`. | +| `openid.client-jwk` | string | | JWK containing the private key for the OpenID client in string format. Must declare the `alg` header. If configured, this takes precedence over `openid.client-secret`. | | `openid.client-secret` | string | | Client secret for the OpenID client. Overridden by `openid.client-jwk`, if configured. | | `openid.domain-hint` | string | | Domain hint to include in authorization request for IdPs that support this parameter (e.g. Entra ID). | | `openid.jwks-fallback-alg` | string | `RS256` | JWA value (as defined in RFC 7518) to assign to provider JWKS keys when their `alg` header is not set. | diff --git a/pkg/config/openid.go b/pkg/config/openid.go index c0a9543..f932aa0 100644 --- a/pkg/config/openid.go +++ b/pkg/config/openid.go @@ -75,7 +75,7 @@ func openidFlags() { flag.String(OpenIDACRValues, "", "Space separated string that configures the default security level (acr_values) parameter for authorization requests.") flag.StringSlice(OpenIDAudiences, []string{}, "List of additional trusted audiences (other than the client_id) for OpenID Connect id_token validation.") flag.String(OpenIDClientID, "", "Client ID for the OpenID client.") - flag.String(OpenIDClientJWK, "", "JWK containing the private key for the OpenID client in string format. If configured, this takes precedence over 'openid.client-secret'.") + flag.String(OpenIDClientJWK, "", "JWK containing the private key for the OpenID client in string format. Must declare the 'alg' header. If configured, this takes precedence over 'openid.client-secret'.") flag.String(OpenIDClientSecret, "", "Client secret for the OpenID client. Overridden by 'openid.client-jwk', if configured.") flag.String(OpenIDDomainHint, "", "Domain hint to include in authorization request for IdPs that support this parameter (e.g. Entra ID).") flag.String(OpenIDJWKSFallbackAlg, jwa.RS256().String(), "JWA value (as defined in RFC 7518) to assign to provider JWKS keys when their 'alg' header is not set.") diff --git a/pkg/mock/client.go b/pkg/mock/client.go index bc74259..a3cf2ec 100644 --- a/pkg/mock/client.go +++ b/pkg/mock/client.go @@ -1,6 +1,7 @@ package mock import ( + "github.com/lestrrat-go/jwx/v3/jwa" "github.com/lestrrat-go/jwx/v3/jwk" "github.com/nais/wonderwall/internal/crypto" "github.com/nais/wonderwall/pkg/config" @@ -11,6 +12,7 @@ import ( type TestClientConfiguration struct { *config.Config clientJwk jwk.Key + clientJwkAlg jwa.KeyAlgorithm trustedAudiences map[string]bool } @@ -36,6 +38,10 @@ func (c *TestClientConfiguration) ClientJWK() jwk.Key { return c.clientJwk } +func (c *TestClientConfiguration) ClientJWKAlgorithm() jwa.KeyAlgorithm { + return c.clientJwkAlg +} + func (c *TestClientConfiguration) ClientSecret() string { return c.OpenID.ClientSecret } @@ -78,9 +84,15 @@ func clientConfiguration(cfg *config.Config) *TestClientConfiguration { panic(err) } + alg, ok := key.Algorithm() + if !ok { + panic("test client JWK is missing an algorithm") + } + return &TestClientConfiguration{ Config: cfg, clientJwk: key, + clientJwkAlg: alg, trustedAudiences: cfg.OpenID.TrustedAudiences(), } } diff --git a/pkg/openid/client/client.go b/pkg/openid/client/client.go index c82eb6d..19dd745 100644 --- a/pkg/openid/client/client.go +++ b/pkg/openid/client/client.go @@ -184,10 +184,7 @@ func (c *Client) ClientAuthenticationAssertion(expiration time.Duration) (string return "", fmt.Errorf("building client assertion: %w", err) } - alg, ok := key.Algorithm() - if !ok { - return "", fmt.Errorf("missing algorithm on client key") - } + alg := clientCfg.ClientJWKAlgorithm() opts := make([]jwt.Option, 0) if c.cfg.Client().NewClientAuthJWTType() { diff --git a/pkg/openid/config/client.go b/pkg/openid/config/client.go index c43bbe5..40a33eb 100644 --- a/pkg/openid/config/client.go +++ b/pkg/openid/config/client.go @@ -3,6 +3,7 @@ package config import ( "fmt" + "github.com/lestrrat-go/jwx/v3/jwa" "github.com/lestrrat-go/jwx/v3/jwk" log "github.com/sirupsen/logrus" @@ -23,6 +24,7 @@ type Client interface { AuthMethod() AuthMethod ClientID() string ClientJWK() jwk.Key + ClientJWKAlgorithm() jwa.KeyAlgorithm ClientSecret() string DomainHint() string NewClientAuthJWTType() bool @@ -37,6 +39,7 @@ type client struct { config.OpenID authMethod AuthMethod clientJwk jwk.Key + clientJwkAlg jwa.KeyAlgorithm trustedAudiences map[string]bool } @@ -62,6 +65,12 @@ func (in *client) ClientJWK() jwk.Key { return in.clientJwk } +// ClientJWKAlgorithm returns the algorithm declared by the client JWK, or nil +// when authenticating with a client secret. NewClientConfig guarantees it is set. +func (in *client) ClientJWKAlgorithm() jwa.KeyAlgorithm { + return in.clientJwkAlg +} + func (in *client) ClientSecret() string { return in.OpenID.ClientSecret } @@ -117,8 +126,13 @@ func NewClientConfig(cfg *config.Config) (Client, error) { if err != nil { return nil, fmt.Errorf("parsing client JWK: %w", err) } + alg, ok := clientJwk.Algorithm() + if !ok { + return nil, fmt.Errorf("client JWK is missing required %q", jwk.AlgorithmKey) + } c.clientJwk = clientJwk + c.clientJwkAlg = alg c.authMethod = AuthMethodPrivateKeyJWT } diff --git a/pkg/openid/config/client_test.go b/pkg/openid/config/client_test.go new file mode 100644 index 0000000..b3a0cdf --- /dev/null +++ b/pkg/openid/config/client_test.go @@ -0,0 +1,62 @@ +package config_test + +import ( + "encoding/json" + "testing" + + "github.com/lestrrat-go/jwx/v3/jwa" + "github.com/nais/wonderwall/internal/crypto" + "github.com/nais/wonderwall/pkg/config" + openidconfig "github.com/nais/wonderwall/pkg/openid/config" + "github.com/stretchr/testify/require" +) + +func TestNewClientConfigRequiresClientJWKAlgorithm(t *testing.T) { + key, err := crypto.NewJwk() + require.NoError(t, err) + + rawKey, err := json.Marshal(key) + require.NoError(t, err) + + var keyFields map[string]any + require.NoError(t, json.Unmarshal(rawKey, &keyFields)) + + for _, tt := range []struct { + name string + removeAlg bool + wantConfig bool + }{ + {name: "algorithm is present", wantConfig: true}, + {name: "algorithm is missing", removeAlg: true}, + } { + t.Run(tt.name, func(t *testing.T) { + fields := make(map[string]any, len(keyFields)) + for name, value := range keyFields { + fields[name] = value + } + if tt.removeAlg { + delete(fields, "alg") + } + + clientJWK, err := json.Marshal(fields) + require.NoError(t, err) + + cfg := &config.Config{OpenID: config.OpenID{ + ClientID: "client-id", + ClientJWK: string(clientJWK), + Provider: config.ProviderOpenID, + WellKnownURL: "https://issuer.example/.well-known/openid-configuration", + }} + + client, err := openidconfig.NewClientConfig(cfg) + if tt.wantConfig { + require.NoError(t, err) + require.NotNil(t, client) + require.Equal(t, jwa.RS256(), client.ClientJWKAlgorithm()) + return + } + + require.ErrorContains(t, err, "client JWK is missing required") + }) + } +}