Skip to content

Commit 497a38e

Browse files
oxistooxisto
authored
Implementing validation of multiple audiences (#433)
1 parent 12384ea commit 497a38e

File tree

4 files changed

+99
-53
lines changed

4 files changed

+99
-53
lines changed

‎map_claims_test.go‎

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,56 +10,61 @@ func TestVerifyAud(t *testing.T) {
1010
var nilListInterface []interface{}
1111
var intListInterface interface{} = []int{1, 2, 3}
1212
type test struct {
13-
Name string
14-
MapClaims MapClaims
15-
Expected bool
16-
Comparison string
17-
Required bool
13+
Name string
14+
MapClaims MapClaims
15+
Expected bool
16+
Comparison []string
17+
MatchAllAud bool
18+
Required bool
1819
}
1920
tests := []test{
2021
// Matching Claim in aud
2122
// Required = true
22-
{Name: "String Aud matching required", MapClaims: MapClaims{"aud": "example.com"}, Expected: true, Required: true, Comparison: "example.com"},
23-
{Name: "[]String Aud with match required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: "example.com"},
23+
{Name: "String Aud matching required", MapClaims: MapClaims{"aud": "example.com"}, Expected: true, Required: true, Comparison: []string{"example.com"}},
24+
{Name: "[]String Aud with match required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com"}},
25+
{Name: "[]String Aud with []match any required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com", "auth.example.com"}},
26+
{Name: "[]String Aud with []match all required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com", "example.example.com"}, MatchAllAud: true},
2427

2528
// Required = false
26-
{Name: "String Aud with match not required", MapClaims: MapClaims{"aud": "example.com"}, Expected: true, Required: false, Comparison: "example.com"},
27-
{Name: "Empty String Aud with match not required", MapClaims: MapClaims{}, Expected: true, Required: false, Comparison: "example.com"},
28-
{Name: "Empty String Aud with match not required", MapClaims: MapClaims{"aud": ""}, Expected: true, Required: false, Comparison: "example.com"},
29-
{Name: "Nil String Aud with match not required", MapClaims: MapClaims{"aud": nil}, Expected: true, Required: false, Comparison: "example.com"},
29+
{Name: "String Aud with match not required", MapClaims: MapClaims{"aud": "example.com"}, Expected: true, Required: false, Comparison: []string{"example.com"}},
30+
{Name: "Empty String Aud with match not required", MapClaims: MapClaims{}, Expected: true, Required: false, Comparison: []string{"example.com"}},
31+
{Name: "Empty String Aud with match not required", MapClaims: MapClaims{"aud": ""}, Expected: true, Required: false, Comparison: []string{"example.com"}},
32+
{Name: "Nil String Aud with match not required", MapClaims: MapClaims{"aud": nil}, Expected: true, Required: false, Comparison: []string{"example.com"}},
3033

31-
{Name: "[]String Aud with match not required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: false, Comparison: "example.com"},
32-
{Name: "Empty []String Aud with match not required", MapClaims: MapClaims{"aud": []string{}}, Expected: true, Required: false, Comparison: "example.com"},
34+
{Name: "[]String Aud with match not required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: false, Comparison: []string{"example.com"}},
35+
{Name: "Empty []String Aud with match not required", MapClaims: MapClaims{"aud": []string{}}, Expected: true, Required: false, Comparison: []string{"example.com"}},
3336

3437
// Non-Matching Claim in aud
3538
// Required = true
36-
{Name: "String Aud without match required", MapClaims: MapClaims{"aud": "not.example.com"}, Expected: false, Required: true, Comparison: "example.com"},
37-
{Name: "Empty String Aud without match required", MapClaims: MapClaims{"aud": ""}, Expected: false, Required: true, Comparison: "example.com"},
38-
{Name: "[]String Aud without match required", MapClaims: MapClaims{"aud": []string{"not.example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: "example.com"},
39-
{Name: "Empty []String Aud without match required", MapClaims: MapClaims{"aud": []string{""}}, Expected: false, Required: true, Comparison: "example.com"},
40-
{Name: "String Aud without match not required", MapClaims: MapClaims{"aud": "not.example.com"}, Expected: false, Required: true, Comparison: "example.com"},
41-
{Name: "Empty String Aud without match not required", MapClaims: MapClaims{"aud": ""}, Expected: false, Required: true, Comparison: "example.com"},
42-
{Name: "[]String Aud without match not required", MapClaims: MapClaims{"aud": []string{"not.example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: "example.com"},
39+
{Name: "String Aud without match required", MapClaims: MapClaims{"aud": "not.example.com"}, Expected: false, Required: true, Comparison: []string{"example.com"}},
40+
{Name: "Empty String Aud without match required", MapClaims: MapClaims{"aud": ""}, Expected: false, Required: true, Comparison: []string{"example.com"}},
41+
{Name: "[]String Aud without match required", MapClaims: MapClaims{"aud": []string{"not.example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}},
42+
{Name: "Empty []String Aud without match required", MapClaims: MapClaims{"aud": []string{""}}, Expected: false, Required: true, Comparison: []string{"example.com"}},
43+
{Name: "String Aud without match not required", MapClaims: MapClaims{"aud": "not.example.com"}, Expected: false, Required: true, Comparison: []string{"example.com"}},
44+
{Name: "Empty String Aud without match not required", MapClaims: MapClaims{"aud": ""}, Expected: false, Required: true, Comparison: []string{"example.com"}},
45+
{Name: "[]String Aud without match not required", MapClaims: MapClaims{"aud": []string{"not.example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}},
4346

4447
// Required = false
45-
{Name: "Empty []String Aud without match required", MapClaims: MapClaims{"aud": []string{""}}, Expected: true, Required: false, Comparison: "example.com"},
48+
{Name: "Empty []String Aud without match required", MapClaims: MapClaims{"aud": []string{""}}, Expected: true, Required: false, Comparison: []string{"example.com"}},
4649

4750
// []interface{}
48-
{Name: "Empty []interface{} Aud without match required", MapClaims: MapClaims{"aud": nilListInterface}, Expected: true, Required: false, Comparison: "example.com"},
49-
{Name: "[]interface{} Aud with match required", MapClaims: MapClaims{"aud": []interface{}{"a", "foo", "example.com"}}, Expected: true, Required: true, Comparison: "example.com"},
50-
{Name: "[]interface{} Aud with match but invalid types", MapClaims: MapClaims{"aud": []interface{}{"a", 5, "example.com"}}, Expected: false, Required: true, Comparison: "example.com"},
51-
{Name: "[]interface{} Aud int with match required", MapClaims: MapClaims{"aud": intListInterface}, Expected: false, Required: true, Comparison: "example.com"},
51+
{Name: "Empty []interface{} Aud without match required", MapClaims: MapClaims{"aud": nilListInterface}, Expected: true, Required: false, Comparison: []string{"example.com"}},
52+
{Name: "[]interface{} Aud with match required", MapClaims: MapClaims{"aud": []interface{}{"a", "foo", "example.com"}}, Expected: true, Required: true, Comparison: []string{"example.com"}},
53+
{Name: "[]interface{} Aud with match but invalid types", MapClaims: MapClaims{"aud": []interface{}{"a", 5, "example.com"}}, Expected: false, Required: true, Comparison: []string{"example.com"}},
54+
{Name: "[]interface{} Aud int with match required", MapClaims: MapClaims{"aud": intListInterface}, Expected: false, Required: true, Comparison: []string{"example.com"}},
5255

5356
// interface{}
54-
{Name: "Empty interface{} Aud without match not required", MapClaims: MapClaims{"aud": nilInterface}, Expected: true, Required: false, Comparison: "example.com"},
57+
{Name: "Empty interface{} Aud without match not required", MapClaims: MapClaims{"aud": nilInterface}, Expected: true, Required: false, Comparison: []string{"example.com"}},
5558
}
5659

5760
for _, test := range tests {
5861
t.Run(test.Name, func(t *testing.T) {
5962
var opts []ParserOption
6063

61-
if test.Required {
62-
opts = append(opts, WithAudience(test.Comparison))
64+
if test.Required && test.MatchAllAud {
65+
opts = append(opts, WithAllAudiences(test.Comparison...))
66+
} else if test.Required {
67+
opts = append(opts, WithAudience(test.Comparison...))
6368
}
6469

6570
validator := NewValidator(opts...)

‎parser_option.go‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,20 +66,37 @@ func WithExpirationRequired() ParserOption {
6666
}
6767
}
6868

69-
// WithAudience configures the validator to require the specified audience in
70-
// the `aud` claim. Validation will fail if the audience is not listed in the
71-
// token or the `aud` claim is missing.
69+
// WithAudience configures the validator to require any of the specified
70+
// audiences in the `aud` claim. Validation will fail if the audience is not
71+
// listed in the token or the `aud` claim is missing.
7272
//
7373
// NOTE: While the `aud` claim is OPTIONAL in a JWT, the handling of it is
7474
// application-specific. Since this validation API is helping developers in
7575
// writing secure application, we decided to REQUIRE the existence of the claim,
7676
// if an audience is expected.
77-
func WithAudience(aud string) ParserOption {
77+
func WithAudience(aud ...string) ParserOption {
7878
return func(p *Parser) {
7979
p.validator.expectedAud = aud
8080
}
8181
}
8282

83+
// WithAllAudiences configures the validator to require all the specified
84+
// audiences in the `aud` claim. Validation will fail if the specified audiences
85+
// are not listed in the token or the `aud` claim is missing. Duplicates within
86+
// the list are de-duplicated since internally, we use a map to look up the
87+
// audiences.
88+
//
89+
// NOTE: While the `aud` claim is OPTIONAL in a JWT, the handling of it is
90+
// application-specific. Since this validation API is helping developers in
91+
// writing secure application, we decided to REQUIRE the existence of the claim,
92+
// if an audience is expected.
93+
func WithAllAudiences(aud ...string) ParserOption {
94+
return func(p *Parser) {
95+
p.validator.expectedAud = aud
96+
p.validator.expectAllAud = true
97+
}
98+
}
99+
83100
// WithIssuer configures the validator to require the specified issuer in the
84101
// `iss` claim. Validation will fail if a different issuer is specified in the
85102
// token or the `iss` claim is missing.

‎validator.go‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package jwt
22

33
import (
4-
"crypto/subtle"
54
"fmt"
65
"time"
76
)
@@ -52,8 +51,12 @@ type Validator struct {
5251
verifyIat bool
5352

5453
// expectedAud contains the audience this token expects. Supplying an empty
55-
// string will disable aud checking.
56-
expectedAud string
54+
// slice will disable aud checking.
55+
expectedAud []string
56+
57+
// expectAllAud specifies whether all expected audiences must be present in
58+
// the token. If false, only one of the expected audiences must be present.
59+
expectAllAud bool
5760

5861
// expectedIss contains the issuer this token expects. Supplying an empty
5962
// string will disable iss checking.
@@ -120,8 +123,8 @@ func (v *Validator) Validate(claims Claims) error {
120123
}
121124

122125
// If we have an expected audience, we also require the audience claim
123-
if v.expectedAud != "" {
124-
if err = v.verifyAudience(claims, v.expectedAud, true); err != nil {
126+
if len(v.expectedAud) > 0 {
127+
if err = v.verifyAudience(claims, v.expectedAud, v.expectAllAud, true); err != nil {
125128
errs = append(errs, err)
126129
}
127130
}
@@ -226,7 +229,7 @@ func (v *Validator) verifyNotBefore(claims Claims, cmp time.Time, required bool)
226229
//
227230
// Additionally, if any error occurs while retrieving the claim, e.g., when its
228231
// the wrong type, an ErrTokenUnverifiable error will be returned.
229-
func (v *Validator) verifyAudience(claims Claims, cmp string, required bool) error {
232+
func (v *Validator) verifyAudience(claims Claims, cmp []string, expectAllAud bool, required bool) error {
230233
aud, err := claims.GetAudience()
231234
if err != nil {
232235
return err
@@ -237,16 +240,35 @@ func (v *Validator) verifyAudience(claims Claims, cmp string, required bool) err
237240
}
238241

239242
// use a var here to keep constant time compare when looping over a number of claims
240-
result := false
243+
matching := make(map[string]bool, 0)
244+
245+
// build a matching hashmap out of the expected aud
246+
for _, expected := range cmp {
247+
matching[expected] = false
248+
}
241249

250+
// compare the expected aud with the actual aud in a constant time manner by looping over all actual values
242251
var stringClaims string
243252
for _, a := range aud {
244-
if subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 {
245-
result = true
253+
a := a
254+
_, ok := matching[a]
255+
if ok {
256+
matching[a] = true
246257
}
258+
247259
stringClaims = stringClaims + a
248260
}
249261

262+
// check if all expected auds are present
263+
result := true
264+
for _, match := range matching {
265+
if !expectAllAud && match {
266+
break
267+
} else if !match {
268+
result = false
269+
}
270+
}
271+
250272
// case where "" is sent in one or many aud claims
251273
if stringClaims == "" {
252274
return errorIfRequired(required, "aud")

‎validator_test.go‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,13 @@ func (m MyCustomClaims) Validate() error {
2222

2323
func Test_Validator_Validate(t *testing.T) {
2424
type fields struct {
25-
leeway time.Duration
26-
timeFunc func() time.Time
27-
verifyIat bool
28-
expectedAud string
29-
expectedIss string
30-
expectedSub string
25+
leeway time.Duration
26+
timeFunc func() time.Time
27+
verifyIat bool
28+
expectedAud []string
29+
expectAllAud bool
30+
expectedIss string
31+
expectedSub string
3132
}
3233
type args struct {
3334
claims Claims
@@ -72,12 +73,13 @@ func Test_Validator_Validate(t *testing.T) {
7273
for _, tt := range tests {
7374
t.Run(tt.name, func(t *testing.T) {
7475
v := &Validator{
75-
leeway: tt.fields.leeway,
76-
timeFunc: tt.fields.timeFunc,
77-
verifyIat: tt.fields.verifyIat,
78-
expectedAud: tt.fields.expectedAud,
79-
expectedIss: tt.fields.expectedIss,
80-
expectedSub: tt.fields.expectedSub,
76+
leeway: tt.fields.leeway,
77+
timeFunc: tt.fields.timeFunc,
78+
verifyIat: tt.fields.verifyIat,
79+
expectedAud: tt.fields.expectedAud,
80+
expectAllAud: tt.fields.expectAllAud,
81+
expectedIss: tt.fields.expectedIss,
82+
expectedSub: tt.fields.expectedSub,
8183
}
8284
if err := v.Validate(tt.args.claims); (err != nil) && !errors.Is(err, tt.wantErr) {
8385
t.Errorf("validator.Validate() error = %v, wantErr %v", err, tt.wantErr)

0 commit comments

Comments
 (0)