From 8f964e5601b2f9045ff320b0b1cbcb3abc06b7ec Mon Sep 17 00:00:00 2001 From: Alex Leering Date: Tue, 24 Dec 2024 14:00:22 +0100 Subject: [PATCH 01/11] Add support for multiple audiences --- map_claims_test.go | 73 ++++++++++++++++++++++++++++++ parser_option.go | 13 ++++++ validator.go | 110 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 196 insertions(+) diff --git a/map_claims_test.go b/map_claims_test.go index 034173d2..ee9e7eaf 100644 --- a/map_claims_test.go +++ b/map_claims_test.go @@ -72,6 +72,79 @@ func TestVerifyAud(t *testing.T) { } } +func TestVerifyAuds(t *testing.T) { + var nilInterface interface{} + var nilListInterface []interface{} + var intListInterface interface{} = []int{1, 2, 3} + + type test struct { + Name string + MapClaims MapClaims // MapClaims to validate + Expected bool // Whether the validation is expected to pass + Comparison []string // Cmp audience values + + AllAudMatching bool // Whether to require all auds matching all cmps + Required bool // Whether the aud claim is required + } + + tests := []test{ + // Matching auds and cmps + {Name: "[]String aud with all expected cmps required and match required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + {Name: "[]String aud with any expected cmps required and match required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: false}, + + // match single expected auds + {Name: "[]String aud with any expected cmps required and match not required, single claim aud", MapClaims: MapClaims{"aud": []string{"example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: false}, + {Name: "[]String aud with any expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com"}, AllAudMatching: false}, + + // Non-matching auds and cmps + // Required = true + {Name: "[]String aud with all expected cmps required and match not required, single claim aud", MapClaims: MapClaims{"aud": []string{"example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + {Name: "[]String aud with all expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}, AllAudMatching: true}, + {Name: "[]String aud with all expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}, AllAudMatching: true}, + + {Name: "[]String aud with any expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}, AllAudMatching: false}, + + // Required = false + {Name: "[]String aud with all expected cmps required and match not required, single claim aud", MapClaims: MapClaims{"aud": []string{"example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + {Name: "[]String aud with all expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}, AllAudMatching: true}, + {Name: "[]String aud with all expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}, AllAudMatching: true}, + + {Name: "[]String aud with any expected cmps required and match not required, single claim aud", MapClaims: MapClaims{"aud": []string{"example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: false}, + {Name: "[]String aud with any expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}, AllAudMatching: false}, + {Name: "[]String aud with any expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}, AllAudMatching: false}, + + // Empty aud + {Name: "Empty aud, with all expected cmps required", MapClaims: MapClaims{"aud": []string{}}, Expected: false, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + {Name: "Empty aud, with any expected cmps required", MapClaims: MapClaims{"aud": []string{}}, Expected: false, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: false}, + + // []interface{} + {Name: "Empty []interface{} Aud without match required", MapClaims: MapClaims{"aud": nilListInterface}, Expected: true, Required: false, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + {Name: "[]interface{} Aud with match required", MapClaims: MapClaims{"aud": []interface{}{"a", "foo", "example.com"}}, Expected: true, Required: true, Comparison: []string{"a", "foo", "example.com"}, AllAudMatching: true}, + {Name: "[]interface{} Aud with match but invalid types", MapClaims: MapClaims{"aud": []interface{}{"a", 5, "example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + {Name: "[]interface{} Aud int with match required", MapClaims: MapClaims{"aud": intListInterface}, Expected: false, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + + // interface{} + {Name: "Empty interface{} Aud without match not required", MapClaims: MapClaims{"aud": nilInterface}, Expected: true, Required: false, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + } + + for _, test := range tests { + t.Run(test.Name, func(t *testing.T) { + var opts []ParserOption + + if test.Required { + opts = append(opts, WithAudiences(test.Comparison, test.AllAudMatching)) + } + + validator := NewValidator(opts...) + got := validator.Validate(test.MapClaims) + + if (got == nil) != test.Expected { + t.Errorf("Expected %v, got %v", test.Expected, (got == nil)) + } + }) + } +} + func TestMapclaimsVerifyIssuedAtInvalidTypeString(t *testing.T) { mapClaims := MapClaims{ "iat": "foo", diff --git a/parser_option.go b/parser_option.go index 88a780fb..bbfde46b 100644 --- a/parser_option.go +++ b/parser_option.go @@ -80,6 +80,19 @@ func WithAudience(aud string) ParserOption { } } +// WithAudiences configures the validator to require the specified audiences in +// the `auds` claim. Validation will fail if the audience is not listed in the +// token or the `aud` claim is missing. +// +// matchAll is a boolean flag that determines if all expected audiences must be present in the token. +// If matchAll is true, the token must contain all expected audiences. If matchAll is false, the token must contain at least one of the expected audiences. +func WithAudiences(auds []string, matchAll bool) ParserOption { + return func(p *Parser) { + p.validator.expectedAuds = auds + p.validator.expectedAudsMatchAll = matchAll + } +} + // WithIssuer configures the validator to require the specified issuer in the // `iss` claim. Validation will fail if a different issuer is specified in the // token or the `iss` claim is missing. diff --git a/validator.go b/validator.go index 008ecd87..df08c01c 100644 --- a/validator.go +++ b/validator.go @@ -55,6 +55,13 @@ type Validator struct { // string will disable aud checking. expectedAud string + //expectedAuds contains the audiences this token expects. Supplying an empty + // []string will disable auds checking. + expectedAuds []string + + // expectedAudsMatchAll specifies whether all expected audiences must match all auds from claim + expectedAudsMatchAll bool + // expectedIss contains the issuer this token expects. Supplying an empty // string will disable iss checking. expectedIss string @@ -126,6 +133,13 @@ func (v *Validator) Validate(claims Claims) error { } } + // If we have expected audiences, we also require the audiences claim + if len(v.expectedAuds) != 0 { + if err := v.verifyAudiences(claims, v.expectedAuds, true, v.expectedAudsMatchAll); err != nil { + errs = append(errs, err) + } + } + // If we have an expected issuer, we also require the issuer claim if v.expectedIss != "" { if err = v.verifyIssuer(claims, v.expectedIss, true); err != nil { @@ -255,6 +269,102 @@ func (v *Validator) verifyAudience(claims Claims, cmp string, required bool) err return errorIfFalse(result, ErrTokenInvalidAudience) } +// verifyAudiences compares the aud claim against cmps. +// If matchAllAuds is true, all cmps must match a aud. +// If matchAllAuds is false, at least one cmp must match a aud. +// +// If matchAllAuds is true and aud length does not match cmps length, an ErrTokenInvalidAudience error will be returned. +// Note that this does not account for any duplicate aud or cmps +// +// If aud is not set or an empty list, it will succeed if the claim is not required, +// otherwise ErrTokenRequiredClaimMissing will be returned. +// +// Additionally, if any error occurs while retrieving the claim, e.g., when its +// the wrong type, an ErrTokenUnverifiable error will be returned. +func (v *Validator) verifyAudiences(claims Claims, cmps []string, required bool, matchAllAuds bool) error { + + aud, err := claims.GetAudience() + if err != nil { + return err + } + + if len(aud) == 0 { + return errorIfRequired(required, "aud") + } + + var stringClaims string + + // If matchAllAuds is true, check if all the cmps matches any of the aud + if matchAllAuds { + + // cmps and aud length should match if matchAllAuds is true + // Note that this does not account for possible duplicates + if len(cmps) != len(aud) { + return errorIfFalse(false, ErrTokenInvalidAudience) + } + + // Check all cmps values + for _, cmp := range cmps { + matchFound := false + for _, a := range aud { + + // Perform constant time comparison + result := subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 + + stringClaims = stringClaims + a + + // If a match is found, set matchFound to true and break out of inner aud loop and continue to next cmp + if result { + matchFound = true + break + } + } + + // If no match was found for the current cmp, return a ErrTokenInvalidAudience error + if !matchFound { + return ErrTokenInvalidAudience + } + } + + } else { + // if matchAllAuds is false, check if any of the cmps matches any of the aud + + matchFound := false + + // Label to break out of both loops if a match is found + outer: + + // Check all aud values + for _, a := range aud { + for _, cmp := range cmps { + + // Perform constant time comparison + result := subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 + + stringClaims = stringClaims + a + + // If a match is found, break out of both loops and finish comparison + if result { + matchFound = true + break outer + } + } + } + + // If no match was found for any cmp, return an error + if !matchFound { + return errorIfFalse(false, ErrTokenInvalidAudience) + } + } + + // case where "" is sent in one or many aud claims + if stringClaims == "" { + return errorIfRequired(required, "aud") + } + + return nil +} + // verifyIssuer compares the iss claim in claims against cmp. // // If iss is not set, it will succeed if the claim is not required, From ca00a1fce38118cc3d49fda77fcd46807e64b17d Mon Sep 17 00:00:00 2001 From: hdonCr Date: Tue, 24 Dec 2024 14:52:15 +0100 Subject: [PATCH 02/11] Allow for multiple audiences --- map_claims_test.go | 73 ++++++++++++++++++++++++++++++ parser_option.go | 13 ++++++ validator.go | 110 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 196 insertions(+) diff --git a/map_claims_test.go b/map_claims_test.go index 034173d2..ee9e7eaf 100644 --- a/map_claims_test.go +++ b/map_claims_test.go @@ -72,6 +72,79 @@ func TestVerifyAud(t *testing.T) { } } +func TestVerifyAuds(t *testing.T) { + var nilInterface interface{} + var nilListInterface []interface{} + var intListInterface interface{} = []int{1, 2, 3} + + type test struct { + Name string + MapClaims MapClaims // MapClaims to validate + Expected bool // Whether the validation is expected to pass + Comparison []string // Cmp audience values + + AllAudMatching bool // Whether to require all auds matching all cmps + Required bool // Whether the aud claim is required + } + + tests := []test{ + // Matching auds and cmps + {Name: "[]String aud with all expected cmps required and match required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + {Name: "[]String aud with any expected cmps required and match required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: false}, + + // match single expected auds + {Name: "[]String aud with any expected cmps required and match not required, single claim aud", MapClaims: MapClaims{"aud": []string{"example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: false}, + {Name: "[]String aud with any expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com"}, AllAudMatching: false}, + + // Non-matching auds and cmps + // Required = true + {Name: "[]String aud with all expected cmps required and match not required, single claim aud", MapClaims: MapClaims{"aud": []string{"example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + {Name: "[]String aud with all expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}, AllAudMatching: true}, + {Name: "[]String aud with all expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}, AllAudMatching: true}, + + {Name: "[]String aud with any expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}, AllAudMatching: false}, + + // Required = false + {Name: "[]String aud with all expected cmps required and match not required, single claim aud", MapClaims: MapClaims{"aud": []string{"example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + {Name: "[]String aud with all expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}, AllAudMatching: true}, + {Name: "[]String aud with all expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}, AllAudMatching: true}, + + {Name: "[]String aud with any expected cmps required and match not required, single claim aud", MapClaims: MapClaims{"aud": []string{"example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: false}, + {Name: "[]String aud with any expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}, AllAudMatching: false}, + {Name: "[]String aud with any expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}, AllAudMatching: false}, + + // Empty aud + {Name: "Empty aud, with all expected cmps required", MapClaims: MapClaims{"aud": []string{}}, Expected: false, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + {Name: "Empty aud, with any expected cmps required", MapClaims: MapClaims{"aud": []string{}}, Expected: false, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: false}, + + // []interface{} + {Name: "Empty []interface{} Aud without match required", MapClaims: MapClaims{"aud": nilListInterface}, Expected: true, Required: false, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + {Name: "[]interface{} Aud with match required", MapClaims: MapClaims{"aud": []interface{}{"a", "foo", "example.com"}}, Expected: true, Required: true, Comparison: []string{"a", "foo", "example.com"}, AllAudMatching: true}, + {Name: "[]interface{} Aud with match but invalid types", MapClaims: MapClaims{"aud": []interface{}{"a", 5, "example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + {Name: "[]interface{} Aud int with match required", MapClaims: MapClaims{"aud": intListInterface}, Expected: false, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + + // interface{} + {Name: "Empty interface{} Aud without match not required", MapClaims: MapClaims{"aud": nilInterface}, Expected: true, Required: false, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, + } + + for _, test := range tests { + t.Run(test.Name, func(t *testing.T) { + var opts []ParserOption + + if test.Required { + opts = append(opts, WithAudiences(test.Comparison, test.AllAudMatching)) + } + + validator := NewValidator(opts...) + got := validator.Validate(test.MapClaims) + + if (got == nil) != test.Expected { + t.Errorf("Expected %v, got %v", test.Expected, (got == nil)) + } + }) + } +} + func TestMapclaimsVerifyIssuedAtInvalidTypeString(t *testing.T) { mapClaims := MapClaims{ "iat": "foo", diff --git a/parser_option.go b/parser_option.go index 88a780fb..bbfde46b 100644 --- a/parser_option.go +++ b/parser_option.go @@ -80,6 +80,19 @@ func WithAudience(aud string) ParserOption { } } +// WithAudiences configures the validator to require the specified audiences in +// the `auds` claim. Validation will fail if the audience is not listed in the +// token or the `aud` claim is missing. +// +// matchAll is a boolean flag that determines if all expected audiences must be present in the token. +// If matchAll is true, the token must contain all expected audiences. If matchAll is false, the token must contain at least one of the expected audiences. +func WithAudiences(auds []string, matchAll bool) ParserOption { + return func(p *Parser) { + p.validator.expectedAuds = auds + p.validator.expectedAudsMatchAll = matchAll + } +} + // WithIssuer configures the validator to require the specified issuer in the // `iss` claim. Validation will fail if a different issuer is specified in the // token or the `iss` claim is missing. diff --git a/validator.go b/validator.go index 008ecd87..df08c01c 100644 --- a/validator.go +++ b/validator.go @@ -55,6 +55,13 @@ type Validator struct { // string will disable aud checking. expectedAud string + //expectedAuds contains the audiences this token expects. Supplying an empty + // []string will disable auds checking. + expectedAuds []string + + // expectedAudsMatchAll specifies whether all expected audiences must match all auds from claim + expectedAudsMatchAll bool + // expectedIss contains the issuer this token expects. Supplying an empty // string will disable iss checking. expectedIss string @@ -126,6 +133,13 @@ func (v *Validator) Validate(claims Claims) error { } } + // If we have expected audiences, we also require the audiences claim + if len(v.expectedAuds) != 0 { + if err := v.verifyAudiences(claims, v.expectedAuds, true, v.expectedAudsMatchAll); err != nil { + errs = append(errs, err) + } + } + // If we have an expected issuer, we also require the issuer claim if v.expectedIss != "" { if err = v.verifyIssuer(claims, v.expectedIss, true); err != nil { @@ -255,6 +269,102 @@ func (v *Validator) verifyAudience(claims Claims, cmp string, required bool) err return errorIfFalse(result, ErrTokenInvalidAudience) } +// verifyAudiences compares the aud claim against cmps. +// If matchAllAuds is true, all cmps must match a aud. +// If matchAllAuds is false, at least one cmp must match a aud. +// +// If matchAllAuds is true and aud length does not match cmps length, an ErrTokenInvalidAudience error will be returned. +// Note that this does not account for any duplicate aud or cmps +// +// If aud is not set or an empty list, it will succeed if the claim is not required, +// otherwise ErrTokenRequiredClaimMissing will be returned. +// +// Additionally, if any error occurs while retrieving the claim, e.g., when its +// the wrong type, an ErrTokenUnverifiable error will be returned. +func (v *Validator) verifyAudiences(claims Claims, cmps []string, required bool, matchAllAuds bool) error { + + aud, err := claims.GetAudience() + if err != nil { + return err + } + + if len(aud) == 0 { + return errorIfRequired(required, "aud") + } + + var stringClaims string + + // If matchAllAuds is true, check if all the cmps matches any of the aud + if matchAllAuds { + + // cmps and aud length should match if matchAllAuds is true + // Note that this does not account for possible duplicates + if len(cmps) != len(aud) { + return errorIfFalse(false, ErrTokenInvalidAudience) + } + + // Check all cmps values + for _, cmp := range cmps { + matchFound := false + for _, a := range aud { + + // Perform constant time comparison + result := subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 + + stringClaims = stringClaims + a + + // If a match is found, set matchFound to true and break out of inner aud loop and continue to next cmp + if result { + matchFound = true + break + } + } + + // If no match was found for the current cmp, return a ErrTokenInvalidAudience error + if !matchFound { + return ErrTokenInvalidAudience + } + } + + } else { + // if matchAllAuds is false, check if any of the cmps matches any of the aud + + matchFound := false + + // Label to break out of both loops if a match is found + outer: + + // Check all aud values + for _, a := range aud { + for _, cmp := range cmps { + + // Perform constant time comparison + result := subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 + + stringClaims = stringClaims + a + + // If a match is found, break out of both loops and finish comparison + if result { + matchFound = true + break outer + } + } + } + + // If no match was found for any cmp, return an error + if !matchFound { + return errorIfFalse(false, ErrTokenInvalidAudience) + } + } + + // case where "" is sent in one or many aud claims + if stringClaims == "" { + return errorIfRequired(required, "aud") + } + + return nil +} + // verifyIssuer compares the iss claim in claims against cmp. // // If iss is not set, it will succeed if the claim is not required, From ab835f55359ad419a5616bbeab88cbdb8a9f3b68 Mon Sep 17 00:00:00 2001 From: hdonCr Date: Tue, 24 Dec 2024 15:21:34 +0100 Subject: [PATCH 03/11] Add new fields to validator test and formatting --- validator_test.go | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/validator_test.go b/validator_test.go index 08a6bd71..74be34bc 100644 --- a/validator_test.go +++ b/validator_test.go @@ -22,12 +22,14 @@ func (m MyCustomClaims) Validate() error { func Test_Validator_Validate(t *testing.T) { type fields struct { - leeway time.Duration - timeFunc func() time.Time - verifyIat bool - expectedAud string - expectedIss string - expectedSub string + leeway time.Duration + timeFunc func() time.Time + verifyIat bool + expectedAud string + expectedAuds []string + expectedAudsMatchAll bool + expectedIss string + expectedSub string } type args struct { claims Claims @@ -72,12 +74,14 @@ func Test_Validator_Validate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { v := &Validator{ - leeway: tt.fields.leeway, - timeFunc: tt.fields.timeFunc, - verifyIat: tt.fields.verifyIat, - expectedAud: tt.fields.expectedAud, - expectedIss: tt.fields.expectedIss, - expectedSub: tt.fields.expectedSub, + leeway: tt.fields.leeway, + timeFunc: tt.fields.timeFunc, + verifyIat: tt.fields.verifyIat, + expectedAud: tt.fields.expectedAud, + expectedAuds: tt.fields.expectedAuds, + expectedAudsMatchAll: tt.fields.expectedAudsMatchAll, + expectedIss: tt.fields.expectedIss, + expectedSub: tt.fields.expectedSub, } if err := v.Validate(tt.args.claims); (err != nil) && !errors.Is(err, tt.wantErr) { t.Errorf("validator.Validate() error = %v, wantErr %v", err, tt.wantErr) From f1c5576171b947ad9c3de75b34e6a980a96c6a2b Mon Sep 17 00:00:00 2001 From: Christian Banse Date: Fri, 3 Jan 2025 20:14:36 +0100 Subject: [PATCH 04/11] Formatting --- map_claims_test.go | 12 ++++++------ validator.go | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/map_claims_test.go b/map_claims_test.go index ee9e7eaf..91d61933 100644 --- a/map_claims_test.go +++ b/map_claims_test.go @@ -80,11 +80,11 @@ func TestVerifyAuds(t *testing.T) { type test struct { Name string MapClaims MapClaims // MapClaims to validate - Expected bool // Whether the validation is expected to pass - Comparison []string // Cmp audience values + Expected bool // Whether the validation is expected to pass + Comparison []string // Cmp audience values - AllAudMatching bool // Whether to require all auds matching all cmps - Required bool // Whether the aud claim is required + AllAudMatching bool // Whether to require all auds matching all cmps + Required bool // Whether the aud claim is required } tests := []test{ @@ -97,14 +97,14 @@ func TestVerifyAuds(t *testing.T) { {Name: "[]String aud with any expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com"}, AllAudMatching: false}, // Non-matching auds and cmps - // Required = true + // Required = true {Name: "[]String aud with all expected cmps required and match not required, single claim aud", MapClaims: MapClaims{"aud": []string{"example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, {Name: "[]String aud with all expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}, AllAudMatching: true}, {Name: "[]String aud with all expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}, AllAudMatching: true}, {Name: "[]String aud with any expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}, AllAudMatching: false}, - // Required = false + // Required = false {Name: "[]String aud with all expected cmps required and match not required, single claim aud", MapClaims: MapClaims{"aud": []string{"example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, {Name: "[]String aud with all expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}, AllAudMatching: true}, {Name: "[]String aud with all expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}, AllAudMatching: true}, diff --git a/validator.go b/validator.go index df08c01c..01eb3506 100644 --- a/validator.go +++ b/validator.go @@ -135,9 +135,9 @@ func (v *Validator) Validate(claims Claims) error { // If we have expected audiences, we also require the audiences claim if len(v.expectedAuds) != 0 { - if err := v.verifyAudiences(claims, v.expectedAuds, true, v.expectedAudsMatchAll); err != nil { - errs = append(errs, err) - } + if err := v.verifyAudiences(claims, v.expectedAuds, true, v.expectedAudsMatchAll); err != nil { + errs = append(errs, err) + } } // If we have an expected issuer, we also require the issuer claim @@ -331,7 +331,7 @@ func (v *Validator) verifyAudiences(claims Claims, cmps []string, required bool, matchFound := false - // Label to break out of both loops if a match is found + // Label to break out of both loops if a match is found outer: // Check all aud values From ca1d34c69b13c9967bade8ca3d7961178f058849 Mon Sep 17 00:00:00 2001 From: hdonCr Date: Tue, 28 Jan 2025 09:02:33 +0100 Subject: [PATCH 05/11] Rename variable for clarity and make expected auds check more specific --- parser_option.go | 2 +- validator.go | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/parser_option.go b/parser_option.go index bbfde46b..f52258bf 100644 --- a/parser_option.go +++ b/parser_option.go @@ -89,7 +89,7 @@ func WithAudience(aud string) ParserOption { func WithAudiences(auds []string, matchAll bool) ParserOption { return func(p *Parser) { p.validator.expectedAuds = auds - p.validator.expectedAudsMatchAll = matchAll + p.validator.matchAllAud = matchAll } } diff --git a/validator.go b/validator.go index df08c01c..b1c48758 100644 --- a/validator.go +++ b/validator.go @@ -59,8 +59,8 @@ type Validator struct { // []string will disable auds checking. expectedAuds []string - // expectedAudsMatchAll specifies whether all expected audiences must match all auds from claim - expectedAudsMatchAll bool + // matchAllAud specifies whether all expected audiences must match all auds from claim + matchAllAud bool // expectedIss contains the issuer this token expects. Supplying an empty // string will disable iss checking. @@ -134,10 +134,10 @@ func (v *Validator) Validate(claims Claims) error { } // If we have expected audiences, we also require the audiences claim - if len(v.expectedAuds) != 0 { - if err := v.verifyAudiences(claims, v.expectedAuds, true, v.expectedAudsMatchAll); err != nil { - errs = append(errs, err) - } + if len(v.expectedAuds) > 0 { + if err := v.verifyAudiences(claims, v.expectedAuds, true, v.matchAllAud); err != nil { + errs = append(errs, err) + } } // If we have an expected issuer, we also require the issuer claim @@ -331,7 +331,7 @@ func (v *Validator) verifyAudiences(claims Claims, cmps []string, required bool, matchFound := false - // Label to break out of both loops if a match is found + // Label to break out of both loops if a match is found outer: // Check all aud values From 8e711bcdf5c16c822423809110ad85d543bf653a Mon Sep 17 00:00:00 2001 From: hdonCr Date: Tue, 28 Jan 2025 09:23:02 +0100 Subject: [PATCH 06/11] Further variable rename --- validator_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/validator_test.go b/validator_test.go index 74be34bc..68b3476b 100644 --- a/validator_test.go +++ b/validator_test.go @@ -27,7 +27,7 @@ func Test_Validator_Validate(t *testing.T) { verifyIat bool expectedAud string expectedAuds []string - expectedAudsMatchAll bool + matchAllAud bool expectedIss string expectedSub string } @@ -79,7 +79,7 @@ func Test_Validator_Validate(t *testing.T) { verifyIat: tt.fields.verifyIat, expectedAud: tt.fields.expectedAud, expectedAuds: tt.fields.expectedAuds, - expectedAudsMatchAll: tt.fields.expectedAudsMatchAll, + matchAllAud: tt.fields.matchAllAud, expectedIss: tt.fields.expectedIss, expectedSub: tt.fields.expectedSub, } From de939e0e4860d21f593affc7caacab424235230e Mon Sep 17 00:00:00 2001 From: hdonCr Date: Tue, 28 Jan 2025 10:14:33 +0100 Subject: [PATCH 07/11] Ran formatter --- map_claims_test.go | 12 ++++++------ validator_test.go | 32 ++++++++++++++++---------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/map_claims_test.go b/map_claims_test.go index ee9e7eaf..91d61933 100644 --- a/map_claims_test.go +++ b/map_claims_test.go @@ -80,11 +80,11 @@ func TestVerifyAuds(t *testing.T) { type test struct { Name string MapClaims MapClaims // MapClaims to validate - Expected bool // Whether the validation is expected to pass - Comparison []string // Cmp audience values + Expected bool // Whether the validation is expected to pass + Comparison []string // Cmp audience values - AllAudMatching bool // Whether to require all auds matching all cmps - Required bool // Whether the aud claim is required + AllAudMatching bool // Whether to require all auds matching all cmps + Required bool // Whether the aud claim is required } tests := []test{ @@ -97,14 +97,14 @@ func TestVerifyAuds(t *testing.T) { {Name: "[]String aud with any expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com"}, AllAudMatching: false}, // Non-matching auds and cmps - // Required = true + // Required = true {Name: "[]String aud with all expected cmps required and match not required, single claim aud", MapClaims: MapClaims{"aud": []string{"example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, {Name: "[]String aud with all expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}, AllAudMatching: true}, {Name: "[]String aud with all expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}, AllAudMatching: true}, {Name: "[]String aud with any expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}, AllAudMatching: false}, - // Required = false + // Required = false {Name: "[]String aud with all expected cmps required and match not required, single claim aud", MapClaims: MapClaims{"aud": []string{"example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com", "example.example.com"}, AllAudMatching: true}, {Name: "[]String aud with all expected cmps required and match not required, single expected aud ", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}, AllAudMatching: true}, {Name: "[]String aud with all expected cmps required and match not required, different auds", MapClaims: MapClaims{"aud": []string{"example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}, AllAudMatching: true}, diff --git a/validator_test.go b/validator_test.go index 68b3476b..40dc1e1a 100644 --- a/validator_test.go +++ b/validator_test.go @@ -22,14 +22,14 @@ func (m MyCustomClaims) Validate() error { func Test_Validator_Validate(t *testing.T) { type fields struct { - leeway time.Duration - timeFunc func() time.Time - verifyIat bool - expectedAud string - expectedAuds []string - matchAllAud bool - expectedIss string - expectedSub string + leeway time.Duration + timeFunc func() time.Time + verifyIat bool + expectedAud string + expectedAuds []string + matchAllAud bool + expectedIss string + expectedSub string } type args struct { claims Claims @@ -74,14 +74,14 @@ func Test_Validator_Validate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { v := &Validator{ - leeway: tt.fields.leeway, - timeFunc: tt.fields.timeFunc, - verifyIat: tt.fields.verifyIat, - expectedAud: tt.fields.expectedAud, - expectedAuds: tt.fields.expectedAuds, - matchAllAud: tt.fields.matchAllAud, - expectedIss: tt.fields.expectedIss, - expectedSub: tt.fields.expectedSub, + leeway: tt.fields.leeway, + timeFunc: tt.fields.timeFunc, + verifyIat: tt.fields.verifyIat, + expectedAud: tt.fields.expectedAud, + expectedAuds: tt.fields.expectedAuds, + matchAllAud: tt.fields.matchAllAud, + expectedIss: tt.fields.expectedIss, + expectedSub: tt.fields.expectedSub, } if err := v.Validate(tt.args.claims); (err != nil) && !errors.Is(err, tt.wantErr) { t.Errorf("validator.Validate() error = %v, wantErr %v", err, tt.wantErr) From 622a310a91894e7f909fd6923a0febb9c5ca94f8 Mon Sep 17 00:00:00 2001 From: hdonCr Date: Sun, 9 Feb 2025 18:08:17 +0100 Subject: [PATCH 08/11] Merge verifying one or multiple audiences into one --- map_claims_test.go | 67 ---------------------------- parser_option.go | 19 +++----- validator.go | 109 ++++++++++++++++++++++++++++++++++++++++----- validator_test.go | 1 - 4 files changed, 103 insertions(+), 93 deletions(-) diff --git a/map_claims_test.go b/map_claims_test.go index 91d61933..132088fc 100644 --- a/map_claims_test.go +++ b/map_claims_test.go @@ -5,73 +5,6 @@ import ( "time" ) -func TestVerifyAud(t *testing.T) { - var nilInterface interface{} - var nilListInterface []interface{} - var intListInterface interface{} = []int{1, 2, 3} - type test struct { - Name string - MapClaims MapClaims - Expected bool - Comparison string - Required bool - } - tests := []test{ - // Matching Claim in aud - // Required = true - {Name: "String Aud matching required", MapClaims: MapClaims{"aud": "example.com"}, Expected: true, Required: true, Comparison: "example.com"}, - {Name: "[]String Aud with match required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: "example.com"}, - - // Required = false - {Name: "String Aud with match not required", MapClaims: MapClaims{"aud": "example.com"}, Expected: true, Required: false, Comparison: "example.com"}, - {Name: "Empty String Aud with match not required", MapClaims: MapClaims{}, Expected: true, Required: false, Comparison: "example.com"}, - {Name: "Empty String Aud with match not required", MapClaims: MapClaims{"aud": ""}, Expected: true, Required: false, Comparison: "example.com"}, - {Name: "Nil String Aud with match not required", MapClaims: MapClaims{"aud": nil}, Expected: true, Required: false, Comparison: "example.com"}, - - {Name: "[]String Aud with match not required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: false, Comparison: "example.com"}, - {Name: "Empty []String Aud with match not required", MapClaims: MapClaims{"aud": []string{}}, Expected: true, Required: false, Comparison: "example.com"}, - - // Non-Matching Claim in aud - // Required = true - {Name: "String Aud without match required", MapClaims: MapClaims{"aud": "not.example.com"}, Expected: false, Required: true, Comparison: "example.com"}, - {Name: "Empty String Aud without match required", MapClaims: MapClaims{"aud": ""}, Expected: false, Required: true, Comparison: "example.com"}, - {Name: "[]String Aud without match required", MapClaims: MapClaims{"aud": []string{"not.example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: "example.com"}, - {Name: "Empty []String Aud without match required", MapClaims: MapClaims{"aud": []string{""}}, Expected: false, Required: true, Comparison: "example.com"}, - {Name: "String Aud without match not required", MapClaims: MapClaims{"aud": "not.example.com"}, Expected: false, Required: true, Comparison: "example.com"}, - {Name: "Empty String Aud without match not required", MapClaims: MapClaims{"aud": ""}, Expected: false, Required: true, Comparison: "example.com"}, - {Name: "[]String Aud without match not required", MapClaims: MapClaims{"aud": []string{"not.example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: "example.com"}, - - // Required = false - {Name: "Empty []String Aud without match required", MapClaims: MapClaims{"aud": []string{""}}, Expected: true, Required: false, Comparison: "example.com"}, - - // []interface{} - {Name: "Empty []interface{} Aud without match required", MapClaims: MapClaims{"aud": nilListInterface}, Expected: true, Required: false, Comparison: "example.com"}, - {Name: "[]interface{} Aud with match required", MapClaims: MapClaims{"aud": []interface{}{"a", "foo", "example.com"}}, Expected: true, Required: true, Comparison: "example.com"}, - {Name: "[]interface{} Aud with match but invalid types", MapClaims: MapClaims{"aud": []interface{}{"a", 5, "example.com"}}, Expected: false, Required: true, Comparison: "example.com"}, - {Name: "[]interface{} Aud int with match required", MapClaims: MapClaims{"aud": intListInterface}, Expected: false, Required: true, Comparison: "example.com"}, - - // interface{} - {Name: "Empty interface{} Aud without match not required", MapClaims: MapClaims{"aud": nilInterface}, Expected: true, Required: false, Comparison: "example.com"}, - } - - for _, test := range tests { - t.Run(test.Name, func(t *testing.T) { - var opts []ParserOption - - if test.Required { - opts = append(opts, WithAudience(test.Comparison)) - } - - validator := NewValidator(opts...) - got := validator.Validate(test.MapClaims) - - if (got == nil) != test.Expected { - t.Errorf("Expected %v, got %v", test.Expected, (got == nil)) - } - }) - } -} - func TestVerifyAuds(t *testing.T) { var nilInterface interface{} var nilListInterface []interface{} diff --git a/parser_option.go b/parser_option.go index f52258bf..98aed0b8 100644 --- a/parser_option.go +++ b/parser_option.go @@ -66,26 +66,17 @@ func WithExpirationRequired() ParserOption { } } -// WithAudience configures the validator to require the specified audience in -// the `aud` claim. Validation will fail if the audience is not listed in the -// token or the `aud` claim is missing. -// -// NOTE: While the `aud` claim is OPTIONAL in a JWT, the handling of it is -// application-specific. Since this validation API is helping developers in -// writing secure application, we decided to REQUIRE the existence of the claim, -// if an audience is expected. -func WithAudience(aud string) ParserOption { - return func(p *Parser) { - p.validator.expectedAud = aud - } -} - // WithAudiences configures the validator to require the specified audiences in // the `auds` claim. Validation will fail if the audience is not listed in the // token or the `aud` claim is missing. // // matchAll is a boolean flag that determines if all expected audiences must be present in the token. // If matchAll is true, the token must contain all expected audiences. If matchAll is false, the token must contain at least one of the expected audiences. +// +// NOTE: While the `aud` claim is OPTIONAL in a JWT, the handling of it is +// application-specific. Since this validation API is helping developers in +// writing secure application, we decided to REQUIRE the existence of the claim, +// if an audience is expected. func WithAudiences(auds []string, matchAll bool) ParserOption { return func(p *Parser) { p.validator.expectedAuds = auds diff --git a/validator.go b/validator.go index b1c48758..f3578ae2 100644 --- a/validator.go +++ b/validator.go @@ -51,10 +51,6 @@ type Validator struct { // unrealistic, i.e., in the future. verifyIat bool - // expectedAud contains the audience this token expects. Supplying an empty - // string will disable aud checking. - expectedAud string - //expectedAuds contains the audiences this token expects. Supplying an empty // []string will disable auds checking. expectedAuds []string @@ -126,13 +122,6 @@ func (v *Validator) Validate(claims Claims) error { } } - // If we have an expected audience, we also require the audience claim - if v.expectedAud != "" { - if err = v.verifyAudience(claims, v.expectedAud, true); err != nil { - errs = append(errs, err) - } - } - // If we have expected audiences, we also require the audiences claim if len(v.expectedAuds) > 0 { if err := v.verifyAudiences(claims, v.expectedAuds, true, v.matchAllAud); err != nil { @@ -281,13 +270,111 @@ func (v *Validator) verifyAudience(claims Claims, cmp string, required bool) err // // Additionally, if any error occurs while retrieving the claim, e.g., when its // the wrong type, an ErrTokenUnverifiable error will be returned. +func (v *Validator) verifyAudiences2(claims Claims, cmps []string, required bool, matchAllAuds bool) error { + + aud, err := claims.GetAudience() + if err != nil { + return err + } + + if len(aud) == 0 { + return errorIfRequired(required, "aud") + } + + var stringClaims string + + // If matchAllAuds is true, check if all the cmps matches any of the aud + if matchAllAuds { + + // cmps and aud length should match if matchAllAuds is true + // Note that this does not account for possible duplicates + if len(cmps) != len(aud) { + return errorIfFalse(false, ErrTokenInvalidAudience) + } + + // Check all cmps values + for _, cmp := range cmps { + matchFound := false + for _, a := range aud { + + // Perform constant time comparison + result := subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 + + stringClaims = stringClaims + a + + // If a match is found, set matchFound to true and break out of inner aud loop and continue to next cmp + if result { + matchFound = true + break + } + } + + // If no match was found for the current cmp, return a ErrTokenInvalidAudience error + if !matchFound { + return ErrTokenInvalidAudience + } + } + + } else { + // if matchAllAuds is false, check if any of the cmps matches any of the aud + + matchFound := false + + // Label to break out of both loops if a match is found + outer: + + // Check all aud values + for _, a := range aud { + for _, cmp := range cmps { + + // Perform constant time comparison + result := subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 + + stringClaims = stringClaims + a + + // If a match is found, break out of both loops and finish comparison + if result { + matchFound = true + break outer + } + } + } + + // If no match was found for any cmp, return an error + if !matchFound { + return errorIfFalse(false, ErrTokenInvalidAudience) + } + } + + // case where "" is sent in one or many aud claims + if stringClaims == "" { + return errorIfRequired(required, "aud") + } + + return nil +} + +// / verifyAudiences compares the aud claim against cmps. +// If matchAllAuds is true, all cmps must match a aud. +// If matchAllAuds is false, at least one cmp must match a aud. +// +// If matchAllAuds is true and aud length does not match cmps length, an ErrTokenInvalidAudience error will be returned. +// Note that this does not account for any duplicate aud or cmps +// +// If aud is not set or an empty list, it will succeed if the claim is not required, +// otherwise ErrTokenRequiredClaimMissing will be returned. +// +// Additionally, if any error occurs while retrieving the claim, e.g., when its +// the wrong type, an ErrTokenUnverifiable error will be returned. func (v *Validator) verifyAudiences(claims Claims, cmps []string, required bool, matchAllAuds bool) error { + // Get the audience claim(s) from the token aud, err := claims.GetAudience() if err != nil { return err } + // If no audience is provided, return an error if required if len(aud) == 0 { return errorIfRequired(required, "aud") } diff --git a/validator_test.go b/validator_test.go index 40dc1e1a..388f3a88 100644 --- a/validator_test.go +++ b/validator_test.go @@ -77,7 +77,6 @@ func Test_Validator_Validate(t *testing.T) { leeway: tt.fields.leeway, timeFunc: tt.fields.timeFunc, verifyIat: tt.fields.verifyIat, - expectedAud: tt.fields.expectedAud, expectedAuds: tt.fields.expectedAuds, matchAllAud: tt.fields.matchAllAud, expectedIss: tt.fields.expectedIss, From 9144621be389dbb92ebcbdd1ce348d547d48fb52 Mon Sep 17 00:00:00 2001 From: hdonCr Date: Sun, 9 Feb 2025 18:12:56 +0100 Subject: [PATCH 09/11] Remove duplicate testing function --- validator.go | 96 ---------------------------------------------------- 1 file changed, 96 deletions(-) diff --git a/validator.go b/validator.go index f3578ae2..eb319e20 100644 --- a/validator.go +++ b/validator.go @@ -258,102 +258,6 @@ func (v *Validator) verifyAudience(claims Claims, cmp string, required bool) err return errorIfFalse(result, ErrTokenInvalidAudience) } -// verifyAudiences compares the aud claim against cmps. -// If matchAllAuds is true, all cmps must match a aud. -// If matchAllAuds is false, at least one cmp must match a aud. -// -// If matchAllAuds is true and aud length does not match cmps length, an ErrTokenInvalidAudience error will be returned. -// Note that this does not account for any duplicate aud or cmps -// -// If aud is not set or an empty list, it will succeed if the claim is not required, -// otherwise ErrTokenRequiredClaimMissing will be returned. -// -// Additionally, if any error occurs while retrieving the claim, e.g., when its -// the wrong type, an ErrTokenUnverifiable error will be returned. -func (v *Validator) verifyAudiences2(claims Claims, cmps []string, required bool, matchAllAuds bool) error { - - aud, err := claims.GetAudience() - if err != nil { - return err - } - - if len(aud) == 0 { - return errorIfRequired(required, "aud") - } - - var stringClaims string - - // If matchAllAuds is true, check if all the cmps matches any of the aud - if matchAllAuds { - - // cmps and aud length should match if matchAllAuds is true - // Note that this does not account for possible duplicates - if len(cmps) != len(aud) { - return errorIfFalse(false, ErrTokenInvalidAudience) - } - - // Check all cmps values - for _, cmp := range cmps { - matchFound := false - for _, a := range aud { - - // Perform constant time comparison - result := subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 - - stringClaims = stringClaims + a - - // If a match is found, set matchFound to true and break out of inner aud loop and continue to next cmp - if result { - matchFound = true - break - } - } - - // If no match was found for the current cmp, return a ErrTokenInvalidAudience error - if !matchFound { - return ErrTokenInvalidAudience - } - } - - } else { - // if matchAllAuds is false, check if any of the cmps matches any of the aud - - matchFound := false - - // Label to break out of both loops if a match is found - outer: - - // Check all aud values - for _, a := range aud { - for _, cmp := range cmps { - - // Perform constant time comparison - result := subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 - - stringClaims = stringClaims + a - - // If a match is found, break out of both loops and finish comparison - if result { - matchFound = true - break outer - } - } - } - - // If no match was found for any cmp, return an error - if !matchFound { - return errorIfFalse(false, ErrTokenInvalidAudience) - } - } - - // case where "" is sent in one or many aud claims - if stringClaims == "" { - return errorIfRequired(required, "aud") - } - - return nil -} - // / verifyAudiences compares the aud claim against cmps. // If matchAllAuds is true, all cmps must match a aud. // If matchAllAuds is false, at least one cmp must match a aud. From 773bd50736ffa7d7b26da3d71b3258660ed17f93 Mon Sep 17 00:00:00 2001 From: hdonCr Date: Sun, 9 Feb 2025 18:38:19 +0100 Subject: [PATCH 10/11] Deduplicate aud and cmp --- validator.go | 24 +++++++++++++++++++++++- validator_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/validator.go b/validator.go index eb319e20..4d8ed33f 100644 --- a/validator.go +++ b/validator.go @@ -283,13 +283,16 @@ func (v *Validator) verifyAudiences(claims Claims, cmps []string, required bool, return errorIfRequired(required, "aud") } + // Deduplicate the aud and cmps slices + aud = deduplicateStrings(aud) + cmps = deduplicateStrings(cmps) + var stringClaims string // If matchAllAuds is true, check if all the cmps matches any of the aud if matchAllAuds { // cmps and aud length should match if matchAllAuds is true - // Note that this does not account for possible duplicates if len(cmps) != len(aud) { return errorIfFalse(false, ErrTokenInvalidAudience) } @@ -297,11 +300,14 @@ func (v *Validator) verifyAudiences(claims Claims, cmps []string, required bool, // Check all cmps values for _, cmp := range cmps { matchFound := false + + // Check all aud values for _, a := range aud { // Perform constant time comparison result := subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 + // Concatenate all aud values to stringClaims stringClaims = stringClaims + a // If a match is found, set matchFound to true and break out of inner aud loop and continue to next cmp @@ -327,11 +333,14 @@ func (v *Validator) verifyAudiences(claims Claims, cmps []string, required bool, // Check all aud values for _, a := range aud { + + // Check all cmp values for _, cmp := range cmps { // Perform constant time comparison result := subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 + // Concatenate all aud values to stringClaims stringClaims = stringClaims + a // If a match is found, break out of both loops and finish comparison @@ -356,6 +365,19 @@ func (v *Validator) verifyAudiences(claims Claims, cmps []string, required bool, return nil } +// deduplicateStrings removes duplicate elements from a string slice +func deduplicateStrings(slice []string) []string { + unique := make(map[string]bool) + var result []string + for _, item := range slice { + if _, found := unique[item]; !found { + unique[item] = true + result = append(result, item) + } + } + return result +} + // verifyIssuer compares the iss claim in claims against cmp. // // If iss is not set, it will succeed if the claim is not required, diff --git a/validator_test.go b/validator_test.go index 388f3a88..f11b3ef0 100644 --- a/validator_test.go +++ b/validator_test.go @@ -4,6 +4,7 @@ import ( "errors" "testing" "time" + "slices" ) var ErrFooBar = errors.New("must be foobar") @@ -262,3 +263,41 @@ func Test_Validator_verifyIssuedAt(t *testing.T) { }) } } + +func Test_deduplicateStrings(t *testing.T) { + tests := []struct { + name string + input []string + expected []string + }{ + { + name: "With duplicates", + input: []string{"a", "b", "a", "c"}, + expected: []string{"a", "b", "c"}, + }, + { + name: "Without duplicates", + input: []string{"a", "b", "c"}, + expected: []string{"a", "b", "c"}, + }, + { + name: "Empty slice", + input: []string{}, + expected: []string{}, + }, + { + name: "All duplicates", + input: []string{"a", "a", "a"}, + expected: []string{"a"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + deduplicated := deduplicateStrings(tt.input) + if !slices.Equal(deduplicated, tt.expected) { + t.Errorf("deduplicateStrings() = %v, want %v", deduplicated, tt.expected) + } + }) + } +} From 2728fccb87483d125a650340419141bf0cec3133 Mon Sep 17 00:00:00 2001 From: hdonCr Date: Sun, 9 Feb 2025 18:51:04 +0100 Subject: [PATCH 11/11] Remove now unused function --- validator.go | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/validator.go b/validator.go index 4d8ed33f..39e0b4cb 100644 --- a/validator.go +++ b/validator.go @@ -222,42 +222,6 @@ func (v *Validator) verifyNotBefore(claims Claims, cmp time.Time, required bool) return errorIfFalse(!cmp.Before(nbf.Add(-v.leeway)), ErrTokenNotValidYet) } -// verifyAudience compares the aud claim against cmp. -// -// If aud is not set or an empty list, it will succeed if the claim is not required, -// otherwise ErrTokenRequiredClaimMissing will be returned. -// -// Additionally, if any error occurs while retrieving the claim, e.g., when its -// the wrong type, an ErrTokenUnverifiable error will be returned. -func (v *Validator) verifyAudience(claims Claims, cmp string, required bool) error { - aud, err := claims.GetAudience() - if err != nil { - return err - } - - if len(aud) == 0 { - return errorIfRequired(required, "aud") - } - - // use a var here to keep constant time compare when looping over a number of claims - result := false - - var stringClaims string - for _, a := range aud { - if subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 { - result = true - } - stringClaims = stringClaims + a - } - - // case where "" is sent in one or many aud claims - if stringClaims == "" { - return errorIfRequired(required, "aud") - } - - return errorIfFalse(result, ErrTokenInvalidAudience) -} - // / verifyAudiences compares the aud claim against cmps. // If matchAllAuds is true, all cmps must match a aud. // If matchAllAuds is false, at least one cmp must match a aud.