From 4a87f0b3ee6774af2645b892da76212f25a6a2cf Mon Sep 17 00:00:00 2001 From: John Landa Date: Fri, 2 Feb 2024 14:03:49 -0700 Subject: [PATCH 1/7] Allow for multiple keysets configured Remove extra error variable Add jwks pairs tests --- backend.go | 16 ++++- go.mod | 2 + path_config.go | 68 ++++++++++++++----- path_config_test.go | 159 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 227 insertions(+), 18 deletions(-) diff --git a/backend.go b/backend.go index cdd8665e..04800723 100644 --- a/backend.go +++ b/backend.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/cap/jwt" "github.com/hashicorp/cap/oidc" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" "github.com/patrickmn/go-cache" @@ -145,15 +146,28 @@ func (b *jwtAuthBackend) jwtValidator(config *jwtConfig) (*jwt.Validator, error) var err error var keySet jwt.KeySet + var keySets []jwt.KeySet // Configure the key set for the validator switch config.authType() { case JWKS: keySet, err = jwt.NewJSONWebKeySet(b.providerCtx, config.JWKSURL, config.JWKSCAPEM) + keySets = []jwt.KeySet{keySet} + case MultiJWKS: + for k, v := range config.JWKSPairs { + // TODO (@johnlanda): read this in as map[string]string in config so we don't need type conversion. + keySet, keySetErr := jwt.NewJSONWebKeySet(b.providerCtx, k, v.(string)) + if keySetErr != nil { + err = multierror.Append(err, keySetErr) + } + keySets = append(keySets, keySet) + } case StaticKeys: keySet, err = jwt.NewStaticKeySet(config.ParsedJWTPubKeys) + keySets = []jwt.KeySet{keySet} case OIDCDiscovery: keySet, err = jwt.NewOIDCDiscoveryKeySet(b.providerCtx, config.OIDCDiscoveryURL, config.OIDCDiscoveryCAPEM) + keySets = []jwt.KeySet{keySet} default: return nil, errors.New("unsupported config type") } @@ -162,7 +176,7 @@ func (b *jwtAuthBackend) jwtValidator(config *jwtConfig) (*jwt.Validator, error) return nil, fmt.Errorf("keyset configuration error: %w", err) } - validator, err := jwt.NewValidator(keySet) + validator, err := jwt.NewValidator(keySets...) if err != nil { return nil, fmt.Errorf("JWT validator configuration error: %w", err) } diff --git a/go.mod b/go.mod index e4122ac2..b336b376 100644 --- a/go.mod +++ b/go.mod @@ -97,3 +97,5 @@ require ( google.golang.org/protobuf v1.31.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) + +replace github.com/hashicorp/cap v0.4.1 => /Users/john.landa/Dev/cap diff --git a/path_config.go b/path_config.go index c4acf0a1..b768042b 100644 --- a/path_config.go +++ b/path_config.go @@ -9,6 +9,7 @@ import ( "crypto/tls" "crypto/x509" "errors" + "fmt" "net/http" "strings" @@ -72,6 +73,10 @@ func pathConfig(b *jwtAuthBackend) *framework.Path { Type: framework.TypeString, Description: "The CA certificate or chain of certificates, in PEM format, to use to validate connections to the JWKS URL. If not set, system certificates are used.", }, + "jwks_pairs": { + Type: framework.TypeMap, + Description: `Set of JWKS Url and CA certificate (or chain of certificates) pairs. CA certificates must be in PEM format. Cannot be used with "jwks_url" or "jwks_ca_pem".`, + }, "default_role": { Type: framework.TypeLowerCaseString, Description: "The default role to use if none is provided during login. If not set, a role is required during login.", @@ -200,6 +205,7 @@ func (b *jwtAuthBackend) pathConfigRead(ctx context.Context, req *logical.Reques "jwt_validation_pubkeys": config.JWTValidationPubKeys, "jwt_supported_algs": config.JWTSupportedAlgs, "jwks_url": config.JWKSURL, + "jwks_pairs": config.JWKSPairs, "jwks_ca_pem": config.JWKSCAPEM, "bound_issuer": config.BoundIssuer, "provider_config": providerConfig, @@ -219,6 +225,7 @@ func (b *jwtAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Reque OIDCResponseMode: d.Get("oidc_response_mode").(string), OIDCResponseTypes: d.Get("oidc_response_types").([]string), JWKSURL: d.Get("jwks_url").(string), + JWKSPairs: d.Get("jwks_pairs").(map[string]interface{}), JWKSCAPEM: d.Get("jwks_ca_pem").(string), DefaultRole: d.Get("default_role").(string), JWTValidationPubKeys: d.Get("jwt_validation_pubkeys").([]string), @@ -255,10 +262,13 @@ func (b *jwtAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Reque if config.JWKSURL != "" { methodCount++ } + if len(config.JWKSPairs) > 0 { + methodCount++ + } switch { case methodCount != 1: - return logical.ErrorResponse("exactly one of 'jwt_validation_pubkeys', 'jwks_url' or 'oidc_discovery_url' must be set"), nil + return logical.ErrorResponse("exactly one of 'jwt_validation_pubkeys', 'jwks_url', 'jwks_urls' or 'oidc_discovery_url' must be set"), nil case config.OIDCClientID != "" && config.OIDCClientSecret == "", config.OIDCClientID == "" && config.OIDCClientSecret != "": @@ -279,30 +289,27 @@ func (b *jwtAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Reque case config.OIDCClientID != "" && config.OIDCDiscoveryURL == "": return logical.ErrorResponse("'oidc_discovery_url' must be set for OIDC"), nil - case config.JWKSURL != "": - keyset, err := jwt.NewJSONWebKeySet(ctx, config.JWKSURL, config.JWKSCAPEM) - if err != nil { - b.Logger().Error("error checking jwks_ca_pem", "error", err) - return logical.ErrorResponse("error checking jwks_ca_pem"), nil - } + case config.JWKSCAPEM != "" && len(config.JWKSPairs) > 0: + return logical.ErrorResponse("CA PEMs must be provided as part of the 'jwks_pairs' when using multiple JWKS URLs"), nil - // Try to verify a correctly formatted JWT. The signature will fail to match, but other - // errors with fetching the remote keyset should be reported. - testJWT := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.e30.Hf3E3iCHzqC5QIQ0nCqS1kw78IiQTRVzsLTuKoDIpdk" - _, err = keyset.VerifySignature(ctx, testJWT) - if err == nil { - err = errors.New("unexpected verification of JWT") + case config.JWKSURL != "": + if r := b.validateJWKSURL(ctx, config.JWKSURL, config.JWKSCAPEM); r != nil { + return r, nil } - if !strings.Contains(err.Error(), "failed to verify id token signature") { - b.Logger().Error("error checking jwks URL", "error", err) - return logical.ErrorResponse("error checking jwks URL"), nil + case len(config.JWKSPairs) > 0: + for k, v := range config.JWKSPairs { + // TODO (@johnlanda): we should just read this in as a map[string]string if possible so we don't have to do + // the type conversion. + if r := b.validateJWKSURL(ctx, k, v.(string)); r != nil { + return r, nil + } } case len(config.JWTValidationPubKeys) != 0: for _, v := range config.JWTValidationPubKeys { if _, err := certutil.ParsePublicKeyPEM([]byte(v)); err != nil { - return logical.ErrorResponse(errwrap.Wrapf("error parsing public key: {{err}}", err).Error()), nil + return logical.ErrorResponse(fmt.Errorf("error parsing public key: %w", err).Error()), nil } } @@ -403,6 +410,29 @@ func (b *jwtAuthBackend) createCAContext(ctx context.Context, caPEM string) (con return caCtx, nil } +func (b *jwtAuthBackend) validateJWKSURL(ctx context.Context, JWKSURL, JWKSCAPEM string) *logical.Response { + keyset, err := jwt.NewJSONWebKeySet(ctx, JWKSURL, JWKSCAPEM) + if err != nil { + b.Logger().Error("error checking jwks_ca_pem", "error", err) + return logical.ErrorResponse("error checking jwks_ca_pem") + } + + // Try to verify a correctly formatted JWT. The signature will fail to match, but other + // errors with fetching the remote keyset should be reported. + testJWT := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.e30.Hf3E3iCHzqC5QIQ0nCqS1kw78IiQTRVzsLTuKoDIpdk" + _, err = keyset.VerifySignature(ctx, testJWT) + if err == nil { + err = errors.New("unexpected verification of JWT") + } + + if !strings.Contains(err.Error(), "failed to verify id token signature") { + b.Logger().Error("error checking jwks URL", "url", JWKSURL, "error", err) + return logical.ErrorResponse("error checking jwks URL %s", JWKSURL) + } + + return nil +} + type jwtConfig struct { OIDCDiscoveryURL string `json:"oidc_discovery_url"` OIDCDiscoveryCAPEM string `json:"oidc_discovery_ca_pem"` @@ -412,6 +442,7 @@ type jwtConfig struct { OIDCResponseTypes []string `json:"oidc_response_types"` JWKSURL string `json:"jwks_url"` JWKSCAPEM string `json:"jwks_ca_pem"` + JWKSPairs map[string]interface{} `json:"jwks_pairs"` JWTValidationPubKeys []string `json:"jwt_validation_pubkeys"` JWTSupportedAlgs []string `json:"jwt_supported_algs"` BoundIssuer string `json:"bound_issuer"` @@ -425,6 +456,7 @@ type jwtConfig struct { const ( StaticKeys = iota JWKS + MultiJWKS OIDCDiscovery OIDCFlow unconfigured @@ -437,6 +469,8 @@ func (c jwtConfig) authType() int { return StaticKeys case c.JWKSURL != "": return JWKS + case len(c.JWKSPairs) > 0: + return MultiJWKS case c.OIDCDiscoveryURL != "": if c.OIDCClientID != "" && c.OIDCClientSecret != "" { return OIDCFlow diff --git a/path_config_test.go b/path_config_test.go index 474fcc91..011f7761 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -30,6 +30,7 @@ func TestConfig_JWT_Read(t *testing.T) { "jwt_supported_algs": []string{}, "jwks_url": "", "jwks_ca_pem": "", + "jwks_pairs": map[string]interface{}{}, "bound_issuer": "http://vault.example.com/", "provider_config": map[string]interface{}{}, "namespace_in_state": false, @@ -139,6 +140,7 @@ func TestConfig_JWT_Write(t *testing.T) { JWTValidationPubKeys: []string{testJWTPubKey}, JWTSupportedAlgs: []string{}, OIDCResponseTypes: []string{}, + JWKSPairs: map[string]interface{}{}, BoundIssuer: "http://vault.example.com/", ProviderConfig: map[string]interface{}{}, NamespaceInState: true, @@ -168,6 +170,7 @@ func TestConfig_JWKS_Update(t *testing.T) { data := map[string]interface{}{ "jwks_url": s.server.URL + "/certs", "jwks_ca_pem": cert, + "jwks_pairs": map[string]interface{}{}, "oidc_discovery_url": "", "oidc_discovery_ca_pem": "", "oidc_client_id": "", @@ -224,6 +227,7 @@ func TestConfig_JWKS_Update_Invalid(t *testing.T) { data := map[string]interface{}{ "jwks_url": s.server.URL + "/certs_missing", "jwks_ca_pem": cert, + "jwks_pairs": map[string]interface{}{}, "oidc_discovery_url": "", "oidc_discovery_ca_pem": "", "oidc_client_id": "", @@ -272,6 +276,151 @@ func TestConfig_JWKS_Update_Invalid(t *testing.T) { } } +func TestConfig_JWKS_Pairs_Update(t *testing.T) { + b, storage := getBackend(t) + + s := newOIDCProvider(t) + defer s.server.Close() + + s2 := newOIDCProvider(t) + defer s2.server.Close() + + cert, err := s.getTLSCert() + if err != nil { + t.Fatal(err) + } + + cert2, err := s2.getTLSCert() + if err != nil { + t.Fatal(err) + } + + data := map[string]interface{}{ + "jwks_url": "", + "jwks_ca_pem": "", + "jwks_pairs": map[string]interface{}{ + s.server.URL + "/certs": cert, + s2.server.URL + "/certs": cert2, + }, + "oidc_discovery_url": "", + "oidc_discovery_ca_pem": "", + "oidc_client_id": "", + "oidc_response_mode": "form_post", + "oidc_response_types": []string{}, + "default_role": "", + "jwt_validation_pubkeys": []string{}, + "jwt_supported_algs": []string{}, + "bound_issuer": "", + "provider_config": map[string]interface{}{}, + "namespace_in_state": false, + } + + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: configPath, + Storage: storage, + Data: data, + } + + resp, err := b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + req = &logical.Request{ + Operation: logical.ReadOperation, + Path: configPath, + Storage: storage, + Data: nil, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + if diff := deep.Equal(resp.Data, data); diff != nil { + t.Fatalf("Expected did not equal actual: %v", diff) + } +} + +func TestConfig_JWKS_Pairs_Update_Invalid(t *testing.T) { + b, storage := getBackend(t) + + s := newOIDCProvider(t) + defer s.server.Close() + + s2 := newOIDCProvider(t) + defer s.server.Close() + + cert, err := s.getTLSCert() + if err != nil { + t.Fatal(err) + } + + cert2, err := s2.getTLSCert() + if err != nil { + t.Fatal(err) + } + + data := map[string]interface{}{ + "jwks_url": "", + "jwks_ca_pem": "", + "jwks_pairs": map[string]interface{}{ + s.server.URL + "/certs_missing": cert, + s2.server.URL + "/certs": cert2, + }, + "oidc_discovery_url": "", + "oidc_discovery_ca_pem": "", + "oidc_client_id": "", + "default_role": "", + "jwt_validation_pubkeys": []string{}, + "jwt_supported_algs": []string{}, + "bound_issuer": "", + } + + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: configPath, + Storage: storage, + Data: data, + } + + resp, err := b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp == nil || !resp.IsError() { + t.Fatal("expected error") + } + if !strings.Contains(resp.Error().Error(), "error checking jwks URL") { + t.Fatalf("got unexpected error: %v", resp.Error()) + } + + // remove the /certs_missing url from the map + delete(data["jwks_pairs"].(map[string]interface{}), s.server.URL+"/certs_missing") + + data["jwks_pairs"].(map[string]interface{})[s.server.URL+"/certs_invalid"] = cert + + req = &logical.Request{ + Operation: logical.UpdateOperation, + Path: configPath, + Storage: storage, + Data: data, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp == nil || !resp.IsError() { + t.Fatal("expected error") + } + if !strings.Contains(resp.Error().Error(), "error checking jwks URL") { + t.Fatalf("got unexpected error: %v", resp.Error()) + } +} + func TestConfig_ResponseMode(t *testing.T) { b, storage := getBackend(t) @@ -348,6 +497,7 @@ func TestConfig_OIDC_Write(t *testing.T) { expected := &jwtConfig{ JWTValidationPubKeys: []string{}, JWTSupportedAlgs: []string{}, + JWKSPairs: map[string]interface{}{}, OIDCResponseTypes: []string{}, OIDCDiscoveryURL: "https://team-vault.auth0.com/", OIDCClientID: "abc", @@ -439,6 +589,7 @@ func TestConfig_OIDC_Write_ProviderConfig(t *testing.T) { expected := &jwtConfig{ JWTValidationPubKeys: []string{}, JWTSupportedAlgs: []string{}, + JWKSPairs: map[string]interface{}{}, OIDCResponseTypes: []string{}, OIDCDiscoveryURL: "https://team-vault.auth0.com/", ProviderConfig: map[string]interface{}{ @@ -499,6 +650,7 @@ func TestConfig_OIDC_Write_ProviderConfig(t *testing.T) { expected := &jwtConfig{ JWTValidationPubKeys: []string{}, JWTSupportedAlgs: []string{}, + JWKSPairs: map[string]interface{}{}, OIDCResponseTypes: []string{}, OIDCDiscoveryURL: "https://team-vault.auth0.com/", ProviderConfig: map[string]interface{}{}, @@ -530,6 +682,7 @@ func TestConfig_OIDC_Create_Namespace(t *testing.T) { OIDCDiscoveryURL: "https://team-vault.auth0.com/", NamespaceInState: true, OIDCResponseTypes: []string{}, + JWKSPairs: map[string]interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, @@ -544,6 +697,7 @@ func TestConfig_OIDC_Create_Namespace(t *testing.T) { OIDCDiscoveryURL: "https://team-vault.auth0.com/", NamespaceInState: true, OIDCResponseTypes: []string{}, + JWKSPairs: map[string]interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, @@ -558,6 +712,7 @@ func TestConfig_OIDC_Create_Namespace(t *testing.T) { OIDCDiscoveryURL: "https://team-vault.auth0.com/", NamespaceInState: false, OIDCResponseTypes: []string{}, + JWKSPairs: map[string]interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, @@ -606,6 +761,7 @@ func TestConfig_OIDC_Update_Namespace(t *testing.T) { OIDCDiscoveryURL: "https://team-vault.auth0.com/", NamespaceInState: true, OIDCResponseTypes: []string{}, + JWKSPairs: map[string]interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, @@ -625,6 +781,7 @@ func TestConfig_OIDC_Update_Namespace(t *testing.T) { NamespaceInState: false, DefaultRole: "ui", OIDCResponseTypes: []string{}, + JWKSPairs: map[string]interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, @@ -643,6 +800,7 @@ func TestConfig_OIDC_Update_Namespace(t *testing.T) { OIDCDiscoveryURL: "https://team-vault.auth0.com/", NamespaceInState: false, OIDCResponseTypes: []string{}, + JWKSPairs: map[string]interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, @@ -662,6 +820,7 @@ func TestConfig_OIDC_Update_Namespace(t *testing.T) { NamespaceInState: true, DefaultRole: "ui", OIDCResponseTypes: []string{}, + JWKSPairs: map[string]interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, From a48a40356844bba48de60e9b9f94245104317feb Mon Sep 17 00:00:00 2001 From: John Landa Date: Fri, 2 Feb 2024 18:36:04 -0700 Subject: [PATCH 2/7] Correct typo --- path_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path_config.go b/path_config.go index b768042b..94006259 100644 --- a/path_config.go +++ b/path_config.go @@ -268,7 +268,7 @@ func (b *jwtAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Reque switch { case methodCount != 1: - return logical.ErrorResponse("exactly one of 'jwt_validation_pubkeys', 'jwks_url', 'jwks_urls' or 'oidc_discovery_url' must be set"), nil + return logical.ErrorResponse("exactly one of 'jwt_validation_pubkeys', 'jwks_url', 'jwks_pairs' or 'oidc_discovery_url' must be set"), nil case config.OIDCClientID != "" && config.OIDCClientSecret == "", config.OIDCClientID == "" && config.OIDCClientSecret != "": From 6e9cf572b732d677b12c55048682bd7ef3604ef9 Mon Sep 17 00:00:00 2001 From: John Landa Date: Tue, 6 Feb 2024 12:16:33 -0700 Subject: [PATCH 3/7] Update cap library --- go.mod | 6 ++---- go.sum | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index b336b376..66a294ec 100644 --- a/go.mod +++ b/go.mod @@ -4,10 +4,11 @@ go 1.20 require ( github.com/go-test/deep v1.1.0 - github.com/hashicorp/cap v0.4.1 + github.com/hashicorp/cap v0.5.0 github.com/hashicorp/errwrap v1.1.0 github.com/hashicorp/go-cleanhttp v0.5.2 github.com/hashicorp/go-hclog v1.6.2 + github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-secure-stdlib/base62 v0.1.2 github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 github.com/hashicorp/go-sockaddr v1.0.6 @@ -54,7 +55,6 @@ require ( github.com/hashicorp/go-immutable-radix v1.3.1 // indirect github.com/hashicorp/go-kms-wrapping/entropy/v2 v2.0.0 // indirect github.com/hashicorp/go-kms-wrapping/v2 v2.0.8 // indirect - github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/hashicorp/go-plugin v1.5.2 // indirect github.com/hashicorp/go-retryablehttp v0.7.1 // indirect github.com/hashicorp/go-rootcerts v1.0.2 // indirect @@ -97,5 +97,3 @@ require ( google.golang.org/protobuf v1.31.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) - -replace github.com/hashicorp/cap v0.4.1 => /Users/john.landa/Dev/cap diff --git a/go.sum b/go.sum index ffcc48b4..3c3d1988 100644 --- a/go.sum +++ b/go.sum @@ -117,8 +117,8 @@ github.com/googleapis/enterprise-certificate-proxy v0.3.2 h1:Vie5ybvEvT75RniqhfF github.com/googleapis/enterprise-certificate-proxy v0.3.2/go.mod h1:VLSiSSBs/ksPL8kq3OBOQ6WRI2QnaFynd1DCjZ62+V0= github.com/googleapis/gax-go/v2 v2.12.0 h1:A+gCJKdRfqXkr+BIRGtZLibNXf0m1f9E4HG56etFpas= github.com/googleapis/gax-go/v2 v2.12.0/go.mod h1:y+aIqrI5eb1YGMVJfuV3185Ts/D7qKpsEkdD5+I6QGU= -github.com/hashicorp/cap v0.4.1 h1:LVYrTLbPV8W6DPwIm/zC/fbc4UTpCQ7nJhCAPshLuG4= -github.com/hashicorp/cap v0.4.1/go.mod h1:oOoohCNd2JAgfvLz2NpFJTZiZ6CqH9dW8dZ2js52lGA= +github.com/hashicorp/cap v0.5.0 h1:YIlAYxdXXtx2IL1JDvP2OyEl55Ooi0yl573kSB9Orlw= +github.com/hashicorp/cap v0.5.0/go.mod h1:IAy00Er+ZFpMo+5x6B4bkO2HgpzgrkfsuDWMmHAuKUE= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= From f967efd1e9fdc1ea1467b6abb905b615f2b49778 Mon Sep 17 00:00:00 2001 From: John Landa Date: Wed, 7 Feb 2024 13:39:20 -0700 Subject: [PATCH 4/7] Unmarshall to slice of struct for jwks_pairs --- backend.go | 7 ++++--- jwks_pairs.go | 36 ++++++++++++++++++++++++++++++++++++ path_config.go | 17 ++++++++++------- path_config_test.go | 38 +++++++++++++++++++------------------- 4 files changed, 69 insertions(+), 29 deletions(-) create mode 100644 jwks_pairs.go diff --git a/backend.go b/backend.go index 04800723..a3232606 100644 --- a/backend.go +++ b/backend.go @@ -154,9 +154,10 @@ func (b *jwtAuthBackend) jwtValidator(config *jwtConfig) (*jwt.Validator, error) keySet, err = jwt.NewJSONWebKeySet(b.providerCtx, config.JWKSURL, config.JWKSCAPEM) keySets = []jwt.KeySet{keySet} case MultiJWKS: - for k, v := range config.JWKSPairs { - // TODO (@johnlanda): read this in as map[string]string in config so we don't need type conversion. - keySet, keySetErr := jwt.NewJSONWebKeySet(b.providerCtx, k, v.(string)) + pairs, err := NewJWKSPairsConfig(config) + + for _, p := range pairs { + keySet, keySetErr := jwt.NewJSONWebKeySet(b.providerCtx, p.JWKSUrl, p.JWKSCAPEM) if keySetErr != nil { err = multierror.Append(err, keySetErr) } diff --git a/jwks_pairs.go b/jwks_pairs.go new file mode 100644 index 00000000..42b210e6 --- /dev/null +++ b/jwks_pairs.go @@ -0,0 +1,36 @@ +package jwtauth + +import ( + "github.com/mitchellh/mapstructure" +) + +type JWKSPair struct { + JWKSUrl string `mapstructure:"jwks_url"` + JWKSCAPEM string `mapstructure:"jwks_ca_pem"` +} + +func NewJWKSPairsConfig(jc *jwtConfig) ([]*JWKSPair, error) { + if len(jc.JWKSPairs) <= 0 { + return nil, nil + } + + pairs := make([]*JWKSPair, 0, len(jc.JWKSPairs)) + for i := 0; i < len(jc.JWKSPairs); i++ { + jp, err := Initialize(jc.JWKSPairs[i].(map[string]interface{})) + if err != nil { + return nil, err + } + pairs = append(pairs, jp) + } + + return pairs, nil +} + +func Initialize(jp map[string]interface{}) (*JWKSPair, error) { + var newJp JWKSPair + if err := mapstructure.Decode(jp, &newJp); err != nil { + return nil, err + } + + return &newJp, nil +} diff --git a/path_config.go b/path_config.go index 94006259..c9fa2799 100644 --- a/path_config.go +++ b/path_config.go @@ -74,7 +74,7 @@ func pathConfig(b *jwtAuthBackend) *framework.Path { Description: "The CA certificate or chain of certificates, in PEM format, to use to validate connections to the JWKS URL. If not set, system certificates are used.", }, "jwks_pairs": { - Type: framework.TypeMap, + Type: framework.TypeSlice, Description: `Set of JWKS Url and CA certificate (or chain of certificates) pairs. CA certificates must be in PEM format. Cannot be used with "jwks_url" or "jwks_ca_pem".`, }, "default_role": { @@ -225,7 +225,7 @@ func (b *jwtAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Reque OIDCResponseMode: d.Get("oidc_response_mode").(string), OIDCResponseTypes: d.Get("oidc_response_types").([]string), JWKSURL: d.Get("jwks_url").(string), - JWKSPairs: d.Get("jwks_pairs").(map[string]interface{}), + JWKSPairs: d.Get("jwks_pairs").([]interface{}), JWKSCAPEM: d.Get("jwks_ca_pem").(string), DefaultRole: d.Get("default_role").(string), JWTValidationPubKeys: d.Get("jwt_validation_pubkeys").([]string), @@ -266,6 +266,7 @@ func (b *jwtAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Reque methodCount++ } + var jwksPairs []*JWKSPair switch { case methodCount != 1: return logical.ErrorResponse("exactly one of 'jwt_validation_pubkeys', 'jwks_url', 'jwks_pairs' or 'oidc_discovery_url' must be set"), nil @@ -298,10 +299,12 @@ func (b *jwtAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Reque } case len(config.JWKSPairs) > 0: - for k, v := range config.JWKSPairs { - // TODO (@johnlanda): we should just read this in as a map[string]string if possible so we don't have to do - // the type conversion. - if r := b.validateJWKSURL(ctx, k, v.(string)); r != nil { + if jwksPairs, err = NewJWKSPairsConfig(config); err != nil { + return logical.ErrorResponse("invalid jwks_pairs: %s", err), nil + } + + for _, p := range jwksPairs { + if r := b.validateJWKSURL(ctx, p.JWKSUrl, p.JWKSCAPEM); r != nil { return r, nil } } @@ -442,7 +445,7 @@ type jwtConfig struct { OIDCResponseTypes []string `json:"oidc_response_types"` JWKSURL string `json:"jwks_url"` JWKSCAPEM string `json:"jwks_ca_pem"` - JWKSPairs map[string]interface{} `json:"jwks_pairs"` + JWKSPairs []interface{} `json:"jwks_pairs"` JWTValidationPubKeys []string `json:"jwt_validation_pubkeys"` JWTSupportedAlgs []string `json:"jwt_supported_algs"` BoundIssuer string `json:"bound_issuer"` diff --git a/path_config_test.go b/path_config_test.go index 011f7761..28572bf0 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -30,7 +30,7 @@ func TestConfig_JWT_Read(t *testing.T) { "jwt_supported_algs": []string{}, "jwks_url": "", "jwks_ca_pem": "", - "jwks_pairs": map[string]interface{}{}, + "jwks_pairs": []interface{}{}, "bound_issuer": "http://vault.example.com/", "provider_config": map[string]interface{}{}, "namespace_in_state": false, @@ -140,7 +140,7 @@ func TestConfig_JWT_Write(t *testing.T) { JWTValidationPubKeys: []string{testJWTPubKey}, JWTSupportedAlgs: []string{}, OIDCResponseTypes: []string{}, - JWKSPairs: map[string]interface{}{}, + JWKSPairs: []interface{}{}, BoundIssuer: "http://vault.example.com/", ProviderConfig: map[string]interface{}{}, NamespaceInState: true, @@ -170,7 +170,7 @@ func TestConfig_JWKS_Update(t *testing.T) { data := map[string]interface{}{ "jwks_url": s.server.URL + "/certs", "jwks_ca_pem": cert, - "jwks_pairs": map[string]interface{}{}, + "jwks_pairs": []interface{}{}, "oidc_discovery_url": "", "oidc_discovery_ca_pem": "", "oidc_client_id": "", @@ -298,9 +298,9 @@ func TestConfig_JWKS_Pairs_Update(t *testing.T) { data := map[string]interface{}{ "jwks_url": "", "jwks_ca_pem": "", - "jwks_pairs": map[string]interface{}{ - s.server.URL + "/certs": cert, - s2.server.URL + "/certs": cert2, + "jwks_pairs": []map[string]interface{}{ + {"jwks_url": s.server.URL + "/certs", "jwks_ca_pem": cert}, + {"jwks_url": s2.server.URL + "/certs", "jwks_ca_pem": cert2}, }, "oidc_discovery_url": "", "oidc_discovery_ca_pem": "", @@ -366,9 +366,9 @@ func TestConfig_JWKS_Pairs_Update_Invalid(t *testing.T) { data := map[string]interface{}{ "jwks_url": "", "jwks_ca_pem": "", - "jwks_pairs": map[string]interface{}{ - s.server.URL + "/certs_missing": cert, - s2.server.URL + "/certs": cert2, + "jwks_pairs": []map[string]interface{}{ + {"jwks_url": s.server.URL + "/certs_missing", "jwks_ca_pem": cert}, + {"jwks_url": s2.server.URL + "/certs", "jwks_ca_pem": cert2}, }, "oidc_discovery_url": "", "oidc_discovery_ca_pem": "", @@ -497,7 +497,7 @@ func TestConfig_OIDC_Write(t *testing.T) { expected := &jwtConfig{ JWTValidationPubKeys: []string{}, JWTSupportedAlgs: []string{}, - JWKSPairs: map[string]interface{}{}, + JWKSPairs: []interface{}{}, OIDCResponseTypes: []string{}, OIDCDiscoveryURL: "https://team-vault.auth0.com/", OIDCClientID: "abc", @@ -589,7 +589,7 @@ func TestConfig_OIDC_Write_ProviderConfig(t *testing.T) { expected := &jwtConfig{ JWTValidationPubKeys: []string{}, JWTSupportedAlgs: []string{}, - JWKSPairs: map[string]interface{}{}, + JWKSPairs: []interface{}{}, OIDCResponseTypes: []string{}, OIDCDiscoveryURL: "https://team-vault.auth0.com/", ProviderConfig: map[string]interface{}{ @@ -650,7 +650,7 @@ func TestConfig_OIDC_Write_ProviderConfig(t *testing.T) { expected := &jwtConfig{ JWTValidationPubKeys: []string{}, JWTSupportedAlgs: []string{}, - JWKSPairs: map[string]interface{}{}, + JWKSPairs: []interface{}{}, OIDCResponseTypes: []string{}, OIDCDiscoveryURL: "https://team-vault.auth0.com/", ProviderConfig: map[string]interface{}{}, @@ -682,7 +682,7 @@ func TestConfig_OIDC_Create_Namespace(t *testing.T) { OIDCDiscoveryURL: "https://team-vault.auth0.com/", NamespaceInState: true, OIDCResponseTypes: []string{}, - JWKSPairs: map[string]interface{}{}, + JWKSPairs: []interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, @@ -697,7 +697,7 @@ func TestConfig_OIDC_Create_Namespace(t *testing.T) { OIDCDiscoveryURL: "https://team-vault.auth0.com/", NamespaceInState: true, OIDCResponseTypes: []string{}, - JWKSPairs: map[string]interface{}{}, + JWKSPairs: []interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, @@ -712,7 +712,7 @@ func TestConfig_OIDC_Create_Namespace(t *testing.T) { OIDCDiscoveryURL: "https://team-vault.auth0.com/", NamespaceInState: false, OIDCResponseTypes: []string{}, - JWKSPairs: map[string]interface{}{}, + JWKSPairs: []interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, @@ -761,7 +761,7 @@ func TestConfig_OIDC_Update_Namespace(t *testing.T) { OIDCDiscoveryURL: "https://team-vault.auth0.com/", NamespaceInState: true, OIDCResponseTypes: []string{}, - JWKSPairs: map[string]interface{}{}, + JWKSPairs: []interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, @@ -781,7 +781,7 @@ func TestConfig_OIDC_Update_Namespace(t *testing.T) { NamespaceInState: false, DefaultRole: "ui", OIDCResponseTypes: []string{}, - JWKSPairs: map[string]interface{}{}, + JWKSPairs: []interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, @@ -800,7 +800,7 @@ func TestConfig_OIDC_Update_Namespace(t *testing.T) { OIDCDiscoveryURL: "https://team-vault.auth0.com/", NamespaceInState: false, OIDCResponseTypes: []string{}, - JWKSPairs: map[string]interface{}{}, + JWKSPairs: []interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, @@ -820,7 +820,7 @@ func TestConfig_OIDC_Update_Namespace(t *testing.T) { NamespaceInState: true, DefaultRole: "ui", OIDCResponseTypes: []string{}, - JWKSPairs: map[string]interface{}{}, + JWKSPairs: []interface{}{}, JWTSupportedAlgs: []string{}, JWTValidationPubKeys: []string{}, ProviderConfig: map[string]interface{}{}, From d10c600ed38ca021347a80758bd6f7af4972f22a Mon Sep 17 00:00:00 2001 From: John Landa Date: Wed, 7 Feb 2024 13:56:05 -0700 Subject: [PATCH 5/7] Fix types in tests --- path_config_test.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/path_config_test.go b/path_config_test.go index 28572bf0..8e9041d7 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -298,9 +298,9 @@ func TestConfig_JWKS_Pairs_Update(t *testing.T) { data := map[string]interface{}{ "jwks_url": "", "jwks_ca_pem": "", - "jwks_pairs": []map[string]interface{}{ - {"jwks_url": s.server.URL + "/certs", "jwks_ca_pem": cert}, - {"jwks_url": s2.server.URL + "/certs", "jwks_ca_pem": cert2}, + "jwks_pairs": []interface{}{ + map[string]interface{}{"jwks_url": s.server.URL + "/certs", "jwks_ca_pem": cert}, + map[string]interface{}{"jwks_url": s2.server.URL + "/certs", "jwks_ca_pem": cert2}, }, "oidc_discovery_url": "", "oidc_discovery_ca_pem": "", @@ -366,9 +366,9 @@ func TestConfig_JWKS_Pairs_Update_Invalid(t *testing.T) { data := map[string]interface{}{ "jwks_url": "", "jwks_ca_pem": "", - "jwks_pairs": []map[string]interface{}{ - {"jwks_url": s.server.URL + "/certs_missing", "jwks_ca_pem": cert}, - {"jwks_url": s2.server.URL + "/certs", "jwks_ca_pem": cert2}, + "jwks_pairs": []interface{}{ + map[string]interface{}{"jwks_url": s.server.URL + "/certs_missing", "jwks_ca_pem": cert}, + map[string]interface{}{"jwks_url": s2.server.URL + "/certs", "jwks_ca_pem": cert2}, }, "oidc_discovery_url": "", "oidc_discovery_ca_pem": "", @@ -397,10 +397,13 @@ func TestConfig_JWKS_Pairs_Update_Invalid(t *testing.T) { t.Fatalf("got unexpected error: %v", resp.Error()) } - // remove the /certs_missing url from the map - delete(data["jwks_pairs"].(map[string]interface{}), s.server.URL+"/certs_missing") + // remove the /certs_missing url from the config + newPairs := []interface{}{ + map[string]interface{}{"jwks_url": s.server.URL + "/certs_invalid", "jwks_ca_pem": cert}, + map[string]interface{}{"jwks_url": s2.server.URL + "/certs", "jwks_ca_pem": cert2}, + } - data["jwks_pairs"].(map[string]interface{})[s.server.URL+"/certs_invalid"] = cert + data["jwks_pairs"] = newPairs req = &logical.Request{ Operation: logical.UpdateOperation, From f7733e98b32797fde5277d2056f794e8d53da445 Mon Sep 17 00:00:00 2001 From: John Landa Date: Wed, 7 Feb 2024 15:25:03 -0700 Subject: [PATCH 6/7] Add type assertion --- jwks_pairs.go | 8 +++++++- path_config_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/jwks_pairs.go b/jwks_pairs.go index 42b210e6..8924b483 100644 --- a/jwks_pairs.go +++ b/jwks_pairs.go @@ -1,6 +1,8 @@ package jwtauth import ( + "fmt" + "github.com/mitchellh/mapstructure" ) @@ -16,7 +18,11 @@ func NewJWKSPairsConfig(jc *jwtConfig) ([]*JWKSPair, error) { pairs := make([]*JWKSPair, 0, len(jc.JWKSPairs)) for i := 0; i < len(jc.JWKSPairs); i++ { - jp, err := Initialize(jc.JWKSPairs[i].(map[string]interface{})) + pairsMap, ok := jc.JWKSPairs[i].(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("jwks_pairs must be provided as a list of json objects with the fields jwks_url and jwks_ca_pem") + } + jp, err := Initialize(pairsMap) if err != nil { return nil, err } diff --git a/path_config_test.go b/path_config_test.go index 8e9041d7..ab59d9ed 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -156,6 +156,45 @@ func TestConfig_JWT_Write(t *testing.T) { } } +func TestConfig_JWKS_Write_Invalid(t *testing.T) { + b, storage := getBackend(t) + + // Create a config with invalid jwks_pairs format + data := map[string]interface{}{ + "jwks_url": "", + "jwks_ca_pem": "", + "jwks_pairs": []interface{}{ + "foo", + "bar", + }, + "oidc_discovery_url": "", + "oidc_discovery_ca_pem": "", + "oidc_client_id": "", + "default_role": "", + "jwt_validation_pubkeys": []string{}, + "jwt_supported_algs": []string{}, + "bound_issuer": "", + } + + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: configPath, + Storage: storage, + Data: data, + } + + resp, err := b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp == nil || !resp.IsError() { + t.Fatal("expected error") + } + if !strings.HasPrefix(resp.Error().Error(), "invalid jwks_pairs") { + t.Fatalf("got unexpected error: %v", resp.Error()) + } +} + func TestConfig_JWKS_Update(t *testing.T) { b, storage := getBackend(t) From 3a96352f2d1973f3e31a448f14f1a9acee2297b7 Mon Sep 17 00:00:00 2001 From: John Landa Date: Wed, 7 Feb 2024 16:55:26 -0700 Subject: [PATCH 7/7] Additional tests and validation --- backend.go | 6 ++++- backend_test.go | 54 +++++++++++++++++++++++++++++++++++++++++++++ path_config.go | 3 +++ path_config_test.go | 39 ++++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 backend_test.go diff --git a/backend.go b/backend.go index a3232606..85041de0 100644 --- a/backend.go +++ b/backend.go @@ -154,12 +154,16 @@ func (b *jwtAuthBackend) jwtValidator(config *jwtConfig) (*jwt.Validator, error) keySet, err = jwt.NewJSONWebKeySet(b.providerCtx, config.JWKSURL, config.JWKSCAPEM) keySets = []jwt.KeySet{keySet} case MultiJWKS: - pairs, err := NewJWKSPairsConfig(config) + pairs, pairsErr := NewJWKSPairsConfig(config) + if pairsErr != nil { + return nil, pairsErr + } for _, p := range pairs { keySet, keySetErr := jwt.NewJSONWebKeySet(b.providerCtx, p.JWKSUrl, p.JWKSCAPEM) if keySetErr != nil { err = multierror.Append(err, keySetErr) + continue } keySets = append(keySets, keySet) } diff --git a/backend_test.go b/backend_test.go new file mode 100644 index 00000000..88d023ad --- /dev/null +++ b/backend_test.go @@ -0,0 +1,54 @@ +package jwtauth + +import ( + "context" + "testing" + + "github.com/hashicorp/cap/jwt" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_jwtAuthBackend_jwtValidator(t *testing.T) { + type args struct { + config *jwtConfig + } + tests := []struct { + name string + args args + want *jwt.Validator + expectedErr string + }{ + { + name: "invalid ca pem", + args: args{ + config: &jwtConfig{ + JWKSPairs: []interface{}{ + map[string]any{ + "jwks_url": "https://www.foobar.com/something", + "jwks_ca_pem": "defg", + }, + map[string]any{ + "jwks_url": "https://www.barbaz.com/something", + "jwks_ca_pem": "", + }, + }, + }, + }, + expectedErr: "could not parse CA PEM value successfully", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := &jwtAuthBackend{} + b.providerCtx = context.TODO() + + got, err := b.jwtValidator(tt.args.config) + if tt.expectedErr != "" { + require.ErrorContains(t, err, tt.expectedErr) + return + } + assert.Equalf(t, tt.want, got, "jwtValidator(%v)", tt.args.config) + }) + } +} diff --git a/path_config.go b/path_config.go index c9fa2799..21fed23c 100644 --- a/path_config.go +++ b/path_config.go @@ -293,6 +293,9 @@ func (b *jwtAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Reque case config.JWKSCAPEM != "" && len(config.JWKSPairs) > 0: return logical.ErrorResponse("CA PEMs must be provided as part of the 'jwks_pairs' when using multiple JWKS URLs"), nil + case len(config.JWKSPairs) > 0 && config.BoundIssuer != "": + return logical.ErrorResponse("Bound issuer is not supported for use with 'jwks_pairs'"), nil + case config.JWKSURL != "": if r := b.validateJWKSURL(ctx, config.JWKSURL, config.JWKSCAPEM); r != nil { return r, nil diff --git a/path_config_test.go b/path_config_test.go index ab59d9ed..32f3059b 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -195,6 +195,45 @@ func TestConfig_JWKS_Write_Invalid(t *testing.T) { } } +func TestConfig_JWKS_Write_BoundIssuer_Invalid(t *testing.T) { + b, storage := getBackend(t) + + // Create a config with invalid jwks_pairs and bound issuer combination + data := map[string]interface{}{ + "jwks_url": "", + "jwks_ca_pem": "", + "jwks_pairs": []interface{}{ + map[string]interface{}{"jwks_url": "https://www.foobar.com/certs", "jwks_ca_pem": "cert"}, + map[string]interface{}{"jwks_url": "https://www.barbaz.com/certs", "jwks_ca_pem": "cert2"}, + }, + "oidc_discovery_url": "", + "oidc_discovery_ca_pem": "", + "oidc_client_id": "", + "default_role": "", + "jwt_validation_pubkeys": []string{}, + "jwt_supported_algs": []string{}, + "bound_issuer": "foobar", + } + + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: configPath, + Storage: storage, + Data: data, + } + + resp, err := b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp == nil || !resp.IsError() { + t.Fatal("expected error") + } + if !strings.HasPrefix(resp.Error().Error(), "Bound issuer is not supported for use with 'jwks_pairs'") { + t.Fatalf("got unexpected error: %v", resp.Error()) + } +} + func TestConfig_JWKS_Update(t *testing.T) { b, storage := getBackend(t)