Fix issue with MapClaims VerifyAudience []string (#12)

* Fix issue with MapClaims VerifyAudience []string

There was an issue in MapClaims's VerifyAudiance where a []string (which
is valid in the spec) would return true (claim is found, or nil) when required
was not set.
It now checks interface types correctly and has tests written

Signed-off-by: Alistair Hey <alistair@heyal.co.uk>

* Keep aud validation constant time compare

Keep aud validation using constant time compare by not instantly
returning on a true comparison, keep comparing all options and store
result in a variable

Signed-off-by: Alistair Hey <alistair@heyal.co.uk>

Co-authored-by: Banse, Christian <christian.banse@aisec.fraunhofer.de>

.gitignore

@@ -1,4 +1,4 @@
 .DS_Store
 bin
-
+.idea/
 

claims.go

@@ -61,7 +61,7 @@ func (c StandardClaims) Valid() error {
 // Compares the aud claim against cmp.
 // If required is false, this method will return true if the value matches or is unset
 func (c *StandardClaims) VerifyAudience(cmp string, req bool) bool {
-	return verifyAud(c.Audience, cmp, req)
+	return verifyAud([]string{c.Audience}, cmp, req)
 }
 
 // Compares the exp claim against cmp.
@@ -90,15 +90,27 @@ func (c *StandardClaims) VerifyNotBefore(cmp int64, req bool) bool {
 
 // ----- helpers
 
-func verifyAud(aud string, cmp string, required bool) bool {
-	if aud == "" {
+func verifyAud(aud []string, cmp string, required bool) bool {
+	if len(aud) == 0 {
 		return !required
 	}
-	if subtle.ConstantTimeCompare([]byte(aud), []byte(cmp)) != 0 {
-		return true
-	} else {
-		return false
+	// 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 len(stringClaims) == 0 {
+		return !required
+	}
+
+	return result
 }
 
 func verifyExp(exp int64, now int64, required bool) bool {

map_claims.go

@@ -10,10 +10,24 @@ import (
 // This is the default claims type if you don't supply one
 type MapClaims map[string]interface{}
 
-// Compares the aud claim against cmp.
+// VerifyAudience Compares the aud claim against cmp.
 // If required is false, this method will return true if the value matches or is unset
 func (m MapClaims) VerifyAudience(cmp string, req bool) bool {
-	aud, _ := m["aud"].(string)
+	var aud []string
+	switch v := m["aud"].(type) {
+	case string:
+		aud = append(aud, v)
+	case []string:
+		aud = v
+	case []interface{}:
+		for _, a := range v {
+			vs, ok := a.(string)
+			if !ok {
+				return false
+			}
+			aud = append(aud, vs)
+		}
+	}
 	return verifyAud(aud, cmp, req)
 }
 

map_claims_test.go

@@ -0,0 +1,68 @@
+package jwt
+
+import (
+	"testing"
+)
+
+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: false, Required: true, 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 wit match required", MapClaims: MapClaims{"aud": []interface{}{"a", "foo", "example.com"}}, Expected: true, Required: true, Comparison: "example.com"},
+		{ Name: "[]interface{} Aud wit match but invalid types", MapClaims: MapClaims{"aud": []interface{}{"a", 5, "example.com"}}, Expected: false, Required: true, Comparison: "example.com"},
+		{ Name: "[]interface{} Aud int wit 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) {
+			got := test.MapClaims.VerifyAudience(test.Comparison, test.Required)
+
+			if got != test.Expected {
+				t.Errorf("Expected %v, got %v", test.Expected, got)
+			}
+		})
+	}
+}