From d3af071e025009d08d284656b87a051e93e44a1e Mon Sep 17 00:00:00 2001 From: Jordon Storfa Date: Mon, 14 Jan 2019 11:13:17 -0700 Subject: [PATCH 1/2] Supports for the aud claim as string or array This was created based on the following PR: https://github.com/dgrijalva/jwt-go/pull/286 As per the JWT spec, the aud claim field can be either a single string value or an array of strings. jwt-go would completely drop array values as the StandardClaims struct's Audience field is a string value and the value is dropped upon deserialization. --- claims.go | 45 +++++++++++++------ claims_test.go | 105 ++++++++++++++++++++++++++++++++++++++++++++ map_claims.go | 14 +++++- map_claims_tests.go | 46 +++++++++++++++++++ 4 files changed, 195 insertions(+), 15 deletions(-) create mode 100644 claims_test.go create mode 100644 map_claims_tests.go diff --git a/claims.go b/claims.go index f0228f02..b2d4e562 100644 --- a/claims.go +++ b/claims.go @@ -16,13 +16,13 @@ type Claims interface { // https://tools.ietf.org/html/rfc7519#section-4.1 // See examples for how to use this with your own claim types type StandardClaims struct { - Audience string `json:"aud,omitempty"` - ExpiresAt int64 `json:"exp,omitempty"` - Id string `json:"jti,omitempty"` - IssuedAt int64 `json:"iat,omitempty"` - Issuer string `json:"iss,omitempty"` - NotBefore int64 `json:"nbf,omitempty"` - Subject string `json:"sub,omitempty"` + Audience interface{} `json:"aud,omitempty"` + ExpiresAt int64 `json:"exp,omitempty"` + Id string `json:"jti,omitempty"` + IssuedAt int64 `json:"iat,omitempty"` + Issuer string `json:"iss,omitempty"` + NotBefore int64 `json:"nbf,omitempty"` + Subject string `json:"sub,omitempty"` } // Validates time based claims "exp, iat, nbf". @@ -58,10 +58,27 @@ func (c StandardClaims) Valid() error { return vErr } +// Extracts an array of audience values from the aud field. +func ExtractAudience(c *StandardClaims) []string { + switch c.Audience.(type) { + case []interface{}: + auds := make([]string, len(c.Audience.([]interface{}))) + for i, value := range c.Audience.([]interface{}) { + auds[i] = value.(string) + } + return auds + case []string: + return c.Audience.([]string) + default: + return []string{c.Audience.(string)} + } +} + // 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) + audiences := ExtractAudience(c) + return verifyAud(audiences, cmp, req) } // Compares the exp claim against cmp. @@ -90,13 +107,15 @@ func (c *StandardClaims) VerifyNotBefore(cmp int64, req bool) bool { // ----- helpers -func verifyAud(aud string, cmp string, required bool) bool { - if aud == "" { +func verifyAud(auds []string, cmp string, required bool) bool { + if len(auds) == 0 { return !required - } - if subtle.ConstantTimeCompare([]byte(aud), []byte(cmp)) != 0 { - return true } else { + for _, aud := range auds { + if len(aud) == len(cmp) && subtle.ConstantTimeCompare([]byte(aud), []byte(cmp)) != 0 { + return true + } + } return false } } diff --git a/claims_test.go b/claims_test.go new file mode 100644 index 00000000..e8ec9825 --- /dev/null +++ b/claims_test.go @@ -0,0 +1,105 @@ +package jwt + +import ( + "testing" +) + +// Test StandardClaims instances with an audience value populated in a string, []string and []interface{} +var audienceValue = "Aud" +var unmatchedAudienceValue = audienceValue + "Test" +var claimWithAudience = []StandardClaims{ + { + audienceValue, + 123123, + "Id", + 12312, + "Issuer", + 12312, + "Subject", + }, + { + []string{audienceValue, unmatchedAudienceValue}, + 123123, + "Id", + 12312, + "Issuer", + 12312, + "Subject", + }, + { + []interface{}{audienceValue, unmatchedAudienceValue}, + 123123, + "Id", + 12312, + "Issuer", + 12312, + "Subject", + }, +} + +// Test StandardClaims instances with no aduences within empty []string and []interface{} collections. +var claimWithoutAudience = []StandardClaims{ + { + []string{}, + 123123, + "Id", + 12312, + "Issuer", + 12312, + "Subject", + }, + { + []interface{}{}, + 123123, + "Id", + 12312, + "Issuer", + 12312, + "Subject", + }, +} + +func TestExtractAudienceWithAudienceValues(t *testing.T) { + for _, data := range claimWithAudience { + var aud = ExtractAudience(&data) + if len(aud) == 0 || aud[0] != audienceValue { + t.Errorf("The audience value was not extracted properly") + } + } +} + +func TestExtractAudience_WithoutAudienceValues(t *testing.T) { + for _, data := range claimWithoutAudience { + var aud = ExtractAudience(&data) + if len(aud) != 0 { + t.Errorf("An audience value should not have been extracted") + } + } +} + +var audWithValues = [][]string{ + []string{audienceValue}, + []string{"Aud1", "Aud2", audienceValue}, +} + +var audWithLackingOriginalValue = [][]string{ + []string{}, + []string{audienceValue + "1"}, + []string{"Aud1", "Aud2", audienceValue + "1"}, +} + +func TestVerifyAud_ShouldVerifyExists(t *testing.T) { + for _, data := range audWithValues { + if !verifyAud(data, audienceValue, true) { + t.Errorf("The audience value was not verified properly") + } + } +} + +func TestVerifyAud_ShouldVerifyDoesNotExist(t *testing.T) { + for _, data := range audWithValues { + if !verifyAud(data, audienceValue, true) { + t.Errorf("The audience value was not verified properly") + } + } +} diff --git a/map_claims.go b/map_claims.go index 291213c4..643e18f3 100644 --- a/map_claims.go +++ b/map_claims.go @@ -13,8 +13,18 @@ type MapClaims map[string]interface{} // 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) - return verifyAud(aud, cmp, req) + switch aud := m["aud"].(type) { + case []string: + return verifyAud(aud, cmp, req) + case []interface{}: + auds := make([]string, len(aud)) + for i, value := range aud { + auds[i] = value.(string) + } + return verifyAud(auds, cmp, req) + default: + return verifyAud([]string{aud.(string)}, cmp, req) + } } // Compares the exp claim against cmp. diff --git a/map_claims_tests.go b/map_claims_tests.go new file mode 100644 index 00000000..39ad4503 --- /dev/null +++ b/map_claims_tests.go @@ -0,0 +1,46 @@ +package jwt + +import ( + "testing" +) + +var audFixedValue = "Aud" +var audClaimsMapsWithValues = []MapClaims{ + { + "aud": audFixedValue, + }, + { + "aud": []string{audFixedValue}, + }, + { + "aud": []interface{}{audFixedValue}, + }, +} + +var audClaimsMapsWithoutValues = []MapClaims{ + {}, + { + "aud": []string{}, + }, + { + "aud": []interface{}{}, + }, +} + +// Verifies that for every form of the "aud" field, the audFixedValue is always verifiable +func TestVerifyAudienceWithVerifiableValues(t *testing.T) { + for _, data := range audClaimsMapsWithValues { + if !data.VerifyAudience(audFixedValue, true) { + t.Errorf("The audience value was not extracted properly") + } + } +} + +// Verifies that for every empty form of the "aud" field, the audFixedValue cannot be verified +func TestVerifyAudienceWithoutVerifiableValues(t *testing.T) { + for _, data := range audClaimsMapsWithoutValues { + if data.VerifyAudience(audFixedValue, true) { + t.Errorf("The audience should not verify") + } + } +} From e198a3b47d4c64417009bde8b230b58d88c25cf3 Mon Sep 17 00:00:00 2001 From: Jordon Storfa Date: Mon, 14 Jan 2019 12:32:53 -0700 Subject: [PATCH 2/2] Handles nil audience values --- claims.go | 6 ++++-- claims_test.go | 11 ++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/claims.go b/claims.go index b2d4e562..66653d6e 100644 --- a/claims.go +++ b/claims.go @@ -58,9 +58,11 @@ func (c StandardClaims) Valid() error { return vErr } -// Extracts an array of audience values from the aud field. +// ExtractAudience extracts an array of audience values from the aud field. func ExtractAudience(c *StandardClaims) []string { switch c.Audience.(type) { + case nil: + return []string{} case []interface{}: auds := make([]string, len(c.Audience.([]interface{}))) for i, value := range c.Audience.([]interface{}) { @@ -74,7 +76,7 @@ func ExtractAudience(c *StandardClaims) []string { } } -// 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 (c *StandardClaims) VerifyAudience(cmp string, req bool) bool { audiences := ExtractAudience(c) diff --git a/claims_test.go b/claims_test.go index e8ec9825..26f73ee7 100644 --- a/claims_test.go +++ b/claims_test.go @@ -37,8 +37,17 @@ var claimWithAudience = []StandardClaims{ }, } -// Test StandardClaims instances with no aduences within empty []string and []interface{} collections. +// Test StandardClaims instances with no audiences within empty []string and []interface{} collections. var claimWithoutAudience = []StandardClaims{ + { + nil, + 123123, + "Id", + 12312, + "Issuer", + 12312, + "Subject", + }, { []string{}, 123123,