From 0c16e829e1624064f1cb763af3af6bfe5a606d7e Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Tue, 8 Mar 2022 17:46:03 -0600 Subject: [PATCH 01/13] refactor: make issued at (iat) claim validation optional Based on https://datatracker.ietf.org/doc/html/rfc7519\#section-4.1.6 we should only validate the type for map claims. This should permit anyone that relies on the time validation to continue doing so. --- claims.go | 38 ++++++++++++++++++-------------------- map_claims.go | 24 ++++++++++++++---------- map_claims_test.go | 4 ++-- parser_option.go | 7 +++++++ validator_option.go | 19 +++++++++++++++++++ 5 files changed, 60 insertions(+), 32 deletions(-) diff --git a/claims.go b/claims.go index b2dc6b2d..4a2c048d 100644 --- a/claims.go +++ b/claims.go @@ -63,7 +63,7 @@ func (c RegisteredClaims) Valid(opts ...validationOption) error { vErr.Errors |= ValidationErrorExpired } - if !c.VerifyIssuedAt(now, false) { + if !c.VerifyIssuedAt(now, false, opts...) { vErr.Inner = ErrTokenUsedBeforeIssued vErr.Errors |= ValidationErrorIssuedAt } @@ -89,10 +89,7 @@ func (c *RegisteredClaims) VerifyAudience(cmp string, req bool) bool { // VerifyExpiresAt compares the exp claim against cmp (cmp < exp). // If req is false, it will return true, if exp is unset. func (c *RegisteredClaims) VerifyExpiresAt(cmp time.Time, req bool, opts ...validationOption) bool { - validator := validator{} - for _, o := range opts { - o(&validator) - } + validator := getValidator(opts...) if c.ExpiresAt == nil { return verifyExp(nil, cmp, req, validator.leeway) } @@ -102,7 +99,12 @@ func (c *RegisteredClaims) VerifyExpiresAt(cmp time.Time, req bool, opts ...vali // VerifyIssuedAt compares the iat claim against cmp (cmp >= iat). // If req is false, it will return true, if iat is unset. -func (c *RegisteredClaims) VerifyIssuedAt(cmp time.Time, req bool) bool { +func (c *RegisteredClaims) VerifyIssuedAt(cmp time.Time, req bool, opts ...validationOption) bool { + validator := getValidator(opts...) + if !validator.iat { + return true + } + if c.IssuedAt == nil { return verifyIat(nil, cmp, req) } @@ -113,10 +115,7 @@ func (c *RegisteredClaims) VerifyIssuedAt(cmp time.Time, req bool) bool { // VerifyNotBefore compares the nbf claim against cmp (cmp >= nbf). // If req is false, it will return true, if nbf is unset. func (c *RegisteredClaims) VerifyNotBefore(cmp time.Time, req bool, opts ...validationOption) bool { - validator := validator{} - for _, o := range opts { - o(&validator) - } + validator := getValidator(opts...) if c.NotBefore == nil { return verifyNbf(nil, cmp, req, validator.leeway) } @@ -164,7 +163,7 @@ func (c StandardClaims) Valid(opts ...validationOption) error { vErr.Errors |= ValidationErrorExpired } - if !c.VerifyIssuedAt(now, false) { + if !c.VerifyIssuedAt(now, false, opts...) { vErr.Inner = ErrTokenUsedBeforeIssued vErr.Errors |= ValidationErrorIssuedAt } @@ -190,10 +189,7 @@ func (c *StandardClaims) VerifyAudience(cmp string, req bool) bool { // VerifyExpiresAt compares the exp claim against cmp (cmp < exp). // If req is false, it will return true, if exp is unset. func (c *StandardClaims) VerifyExpiresAt(cmp int64, req bool, opts ...validationOption) bool { - validator := validator{} - for _, o := range opts { - o(&validator) - } + validator := getValidator(opts...) if c.ExpiresAt == 0 { return verifyExp(nil, time.Unix(cmp, 0), req, validator.leeway) } @@ -204,7 +200,12 @@ func (c *StandardClaims) VerifyExpiresAt(cmp int64, req bool, opts ...validation // VerifyIssuedAt compares the iat claim against cmp (cmp >= iat). // If req is false, it will return true, if iat is unset. -func (c *StandardClaims) VerifyIssuedAt(cmp int64, req bool) bool { +func (c *StandardClaims) VerifyIssuedAt(cmp int64, req bool, opts ...validationOption) bool { + validator := getValidator(opts...) + if !validator.iat { + return true + } + if c.IssuedAt == 0 { return verifyIat(nil, time.Unix(cmp, 0), req) } @@ -216,10 +217,7 @@ func (c *StandardClaims) VerifyIssuedAt(cmp int64, req bool) bool { // VerifyNotBefore compares the nbf claim against cmp (cmp >= nbf). // If req is false, it will return true, if nbf is unset. func (c *StandardClaims) VerifyNotBefore(cmp int64, req bool, opts ...validationOption) bool { - validator := validator{} - for _, o := range opts { - o(&validator) - } + validator := getValidator(opts...) if c.NotBefore == 0 { return verifyNbf(nil, time.Unix(cmp, 0), req, validator.leeway) } diff --git a/map_claims.go b/map_claims.go index e4a08079..51d0422a 100644 --- a/map_claims.go +++ b/map_claims.go @@ -42,10 +42,7 @@ func (m MapClaims) VerifyExpiresAt(cmp int64, req bool, opts ...validationOption return !req } - validator := validator{} - for _, o := range opts { - o(&validator) - } + validator := getValidator(opts...) switch exp := v.(type) { case float64: @@ -65,7 +62,7 @@ func (m MapClaims) VerifyExpiresAt(cmp int64, req bool, opts ...validationOption // VerifyIssuedAt compares the exp claim against cmp (cmp >= iat). // If req is false, it will return true, if iat is unset. -func (m MapClaims) VerifyIssuedAt(cmp int64, req bool) bool { +func (m MapClaims) VerifyIssuedAt(cmp int64, req bool, opts ...validationOption) bool { cmpTime := time.Unix(cmp, 0) v, ok := m["iat"] @@ -73,6 +70,16 @@ func (m MapClaims) VerifyIssuedAt(cmp int64, req bool) bool { return !req } + // validate the type + switch v.(type) { + case float64, json.Number: + if !getValidator(opts...).iat { + return true + } + default: + return false + } + switch iat := v.(type) { case float64: if iat == 0 { @@ -99,10 +106,7 @@ func (m MapClaims) VerifyNotBefore(cmp int64, req bool, opts ...validationOption return !req } - validator := validator{} - for _, o := range opts { - o(&validator) - } + validator := getValidator(opts...) switch nbf := v.(type) { case float64: @@ -141,7 +145,7 @@ func (m MapClaims) Valid(opts ...validationOption) error { vErr.Errors |= ValidationErrorExpired } - if !m.VerifyIssuedAt(now, false) { + if !m.VerifyIssuedAt(now, false, opts...) { // TODO(oxisto): this should be replaced with ErrTokenUsedBeforeIssued vErr.Inner = errors.New("Token used before issued") vErr.Errors |= ValidationErrorIssuedAt diff --git a/map_claims_test.go b/map_claims_test.go index b8b9eb74..361c49d2 100644 --- a/map_claims_test.go +++ b/map_claims_test.go @@ -110,13 +110,13 @@ func TestMapClaimsVerifyExpiresAtExpire(t *testing.T) { t.Fatalf("Failed to verify claims, wanted: %v got %v", want, got) } - got = mapClaims.VerifyExpiresAt(exp + 1, true) + got = mapClaims.VerifyExpiresAt(exp+1, true) if want != got { t.Fatalf("Failed to verify claims, wanted: %v got %v", want, got) } want = true - got = mapClaims.VerifyExpiresAt(exp - 1, true) + got = mapClaims.VerifyExpiresAt(exp-1, true) if want != got { t.Fatalf("Failed to verify claims, wanted: %v got %v", want, got) } diff --git a/parser_option.go b/parser_option.go index a7976645..af2f6660 100644 --- a/parser_option.go +++ b/parser_option.go @@ -36,3 +36,10 @@ func WithLeeway(d time.Duration) ParserOption { p.validationOptions = append(p.validationOptions, withLeeway(d)) } } + +// WithIssuedAt is an option to enable the validation of the issued at (iat) claim. +func WithIssuedAt() ParserOption { + return func(p *Parser) { + p.validationOptions = append(p.validationOptions, withIssuedAt()) + } +} diff --git a/validator_option.go b/validator_option.go index eb29dc30..2db63bad 100644 --- a/validator_option.go +++ b/validator_option.go @@ -16,6 +16,7 @@ type validationOption func(*validator) // the API is more stable. type validator struct { leeway time.Duration // Leeway to provide when validating time values + iat bool } // withLeeway is an option to set the clock skew (leeway) window @@ -27,3 +28,21 @@ func withLeeway(d time.Duration) validationOption { v.leeway = d } } + +// withIssuedAth is an option to enable the validation of the issued at (iat) claim +// +// Note that this function is (currently) un-exported, its naming is subject to change and will only be exported once +// the API is more stable. +func withIssuedAt() validationOption { + return func(v *validator) { + v.iat = true + } +} + +func getValidator(opts ...validationOption) validator { + v := validator{} + for _, o := range opts { + o(&v) + } + return v +} From b97c0bb75d0c78ace8058654c5c85aef5c5a45f0 Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Tue, 8 Mar 2022 19:17:40 -0600 Subject: [PATCH 02/13] feat: port audience validation functions from v4 --- claims.go | 72 +++++++++++++++++++++++++++++++---------- map_claims.go | 34 +++++++++++++++----- parser_option.go | 14 ++++++++ parser_test.go | 78 +++++++++++++++++++++++++++++++++++++++++++++ validator_option.go | 33 ++++++++++++++++++- 5 files changed, 206 insertions(+), 25 deletions(-) diff --git a/claims.go b/claims.go index b2dc6b2d..be2ab45e 100644 --- a/claims.go +++ b/claims.go @@ -73,6 +73,11 @@ func (c RegisteredClaims) Valid(opts ...validationOption) error { vErr.Errors |= ValidationErrorNotValidYet } + if !c.validateAudience(false, opts...) { + vErr.Inner = ErrTokenInvalidAudience + vErr.Errors |= ValidationErrorAudience + } + if vErr.valid() { return nil } @@ -89,10 +94,7 @@ func (c *RegisteredClaims) VerifyAudience(cmp string, req bool) bool { // VerifyExpiresAt compares the exp claim against cmp (cmp < exp). // If req is false, it will return true, if exp is unset. func (c *RegisteredClaims) VerifyExpiresAt(cmp time.Time, req bool, opts ...validationOption) bool { - validator := validator{} - for _, o := range opts { - o(&validator) - } + validator := getValidator(opts...) if c.ExpiresAt == nil { return verifyExp(nil, cmp, req, validator.leeway) } @@ -113,10 +115,7 @@ func (c *RegisteredClaims) VerifyIssuedAt(cmp time.Time, req bool) bool { // VerifyNotBefore compares the nbf claim against cmp (cmp >= nbf). // If req is false, it will return true, if nbf is unset. func (c *RegisteredClaims) VerifyNotBefore(cmp time.Time, req bool, opts ...validationOption) bool { - validator := validator{} - for _, o := range opts { - o(&validator) - } + validator := getValidator(opts...) if c.NotBefore == nil { return verifyNbf(nil, cmp, req, validator.leeway) } @@ -130,6 +129,27 @@ func (c *RegisteredClaims) VerifyIssuer(cmp string, req bool) bool { return verifyIss(c.Issuer, cmp, req) } +func (c *RegisteredClaims) validateAudience(req bool, opts ...validationOption) bool { + if len(c.Audience) == 0 { + return !req + } + + validator := getValidator(opts...) + + if validator.skipAudience { + return true + } + + // Based on my reading of https://datatracker.ietf.org/doc/html/rfc7519/#section-4.1.3 + // this should technically fail. This is left as a decision for the maintainers to alter + // the behavior as it would be a breaking change. + if validator.audience != nil { + return c.VerifyAudience(*validator.audience, true) + } + + return !req +} + // StandardClaims are a structured version of the JWT Claims Set, as referenced at // https://datatracker.ietf.org/doc/html/rfc7519#section-4. They do not follow the // specification exactly, since they were based on an earlier draft of the @@ -174,6 +194,11 @@ func (c StandardClaims) Valid(opts ...validationOption) error { vErr.Errors |= ValidationErrorNotValidYet } + if !c.validateAudience(false, opts...) { + vErr.Inner = ErrTokenInvalidAudience + vErr.Errors |= ValidationErrorAudience + } + if vErr.valid() { return nil } @@ -190,10 +215,7 @@ func (c *StandardClaims) VerifyAudience(cmp string, req bool) bool { // VerifyExpiresAt compares the exp claim against cmp (cmp < exp). // If req is false, it will return true, if exp is unset. func (c *StandardClaims) VerifyExpiresAt(cmp int64, req bool, opts ...validationOption) bool { - validator := validator{} - for _, o := range opts { - o(&validator) - } + validator := getValidator(opts...) if c.ExpiresAt == 0 { return verifyExp(nil, time.Unix(cmp, 0), req, validator.leeway) } @@ -216,10 +238,7 @@ func (c *StandardClaims) VerifyIssuedAt(cmp int64, req bool) bool { // VerifyNotBefore compares the nbf claim against cmp (cmp >= nbf). // If req is false, it will return true, if nbf is unset. func (c *StandardClaims) VerifyNotBefore(cmp int64, req bool, opts ...validationOption) bool { - validator := validator{} - for _, o := range opts { - o(&validator) - } + validator := getValidator(opts...) if c.NotBefore == 0 { return verifyNbf(nil, time.Unix(cmp, 0), req, validator.leeway) } @@ -234,6 +253,27 @@ func (c *StandardClaims) VerifyIssuer(cmp string, req bool) bool { return verifyIss(c.Issuer, cmp, req) } +func (c *StandardClaims) validateAudience(req bool, opts ...validationOption) bool { + if c.Audience == "" { + return !req + } + + validator := getValidator(opts...) + + if validator.skipAudience { + return true + } + + // Based on my reading of https://datatracker.ietf.org/doc/html/rfc7519/#section-4.1.3 + // this should technically fail. This is left as a decision for the maintainers to alter + // the behavior as it would be a breaking change. + if validator.audience != nil { + return c.VerifyAudience(*validator.audience, true) + } + + return !req +} + // ----- helpers func verifyAud(aud []string, cmp string, required bool) bool { diff --git a/map_claims.go b/map_claims.go index e4a08079..7b623f02 100644 --- a/map_claims.go +++ b/map_claims.go @@ -42,10 +42,7 @@ func (m MapClaims) VerifyExpiresAt(cmp int64, req bool, opts ...validationOption return !req } - validator := validator{} - for _, o := range opts { - o(&validator) - } + validator := getValidator(opts...) switch exp := v.(type) { case float64: @@ -99,10 +96,7 @@ func (m MapClaims) VerifyNotBefore(cmp int64, req bool, opts ...validationOption return !req } - validator := validator{} - for _, o := range opts { - o(&validator) - } + validator := getValidator(opts...) switch nbf := v.(type) { case float64: @@ -127,6 +121,25 @@ func (m MapClaims) VerifyIssuer(cmp string, req bool) bool { return verifyIss(iss, cmp, req) } +func (m MapClaims) validateAudience(req bool, opts ...validationOption) bool { + _, ok := m["aud"] + if !ok { + return !req + } + + validator := getValidator(opts...) + + if validator.skipAudience { + return true + } + + if validator.audience == nil { + return false + } + + return m.VerifyAudience(*validator.audience, true) +} + // Valid validates time based claims "exp, iat, nbf". // There is no accounting for clock skew. // As well, if any of the above claims are not in the token, it will still @@ -153,6 +166,11 @@ func (m MapClaims) Valid(opts ...validationOption) error { vErr.Errors |= ValidationErrorNotValidYet } + if !m.validateAudience(false, opts...) { + vErr.Inner = ErrTokenInvalidAudience + vErr.Errors |= ValidationErrorAudience + } + if vErr.valid() { return nil } diff --git a/parser_option.go b/parser_option.go index a7976645..95748161 100644 --- a/parser_option.go +++ b/parser_option.go @@ -36,3 +36,17 @@ func WithLeeway(d time.Duration) ParserOption { p.validationOptions = append(p.validationOptions, withLeeway(d)) } } + +// WithAudience returns the ParserOption for specifying an expected aud member value +func WithAudience(aud string) ParserOption { + return func(p *Parser) { + p.validationOptions = append(p.validationOptions, withAudience(aud)) + } +} + +// WithoutAudienceValidation returns the ParserOption that specifies audience check should be skipped +func WithoutAudienceValidation() ParserOption { + return func(p *Parser) { + p.validationOptions = append(p.validationOptions, withoutAudienceValidation()) + } +} diff --git a/parser_test.go b/parser_test.go index e25ff0b2..d35ebe22 100644 --- a/parser_test.go +++ b/parser_test.go @@ -339,6 +339,84 @@ var jwtTestData = []struct { &jwt.Parser{UseJSONNumber: true}, jwt.SigningMethodRS256, }, + { + "RFC7519 Claims - single aud without validation", + "", + defaultKeyFunc, + &jwt.RegisteredClaims{ + Audience: jwt.ClaimStrings{"test"}, + }, + true, + 0, + nil, + jwt.NewParser(jwt.WithoutAudienceValidation()), + jwt.SigningMethodRS256, + }, + { + "RFC7519 Claims - multiple aud without validation", + "", + defaultKeyFunc, + &jwt.RegisteredClaims{ + Audience: jwt.ClaimStrings{"test", "test2"}, + }, + true, + 0, + nil, + jwt.NewParser(jwt.WithoutAudienceValidation()), + jwt.SigningMethodRS256, + }, + { + "RFC7519 Claims - single aud with valid audience", + "", + defaultKeyFunc, + &jwt.RegisteredClaims{ + Audience: jwt.ClaimStrings{"test"}, + }, + true, + 0, + nil, + jwt.NewParser(jwt.WithAudience("test")), + jwt.SigningMethodRS256, + }, + { + "RFC7519 Claims - multiple aud with valid audience", + "", + defaultKeyFunc, + &jwt.RegisteredClaims{ + Audience: jwt.ClaimStrings{"test", "test2"}, + }, + true, + 0, + nil, + jwt.NewParser(jwt.WithAudience("test")), + jwt.SigningMethodRS256, + }, + { + "RFC7519 Claims - single aud with invalid audience", + "", + defaultKeyFunc, + &jwt.RegisteredClaims{ + Audience: jwt.ClaimStrings{"test"}, + }, + false, + jwt.ValidationErrorAudience, + []error{jwt.ErrTokenInvalidAudience}, + jwt.NewParser(jwt.WithAudience("bad")), + jwt.SigningMethodRS256, + }, + { + "RFC7519 Claims - multiple aud with invalid audience", + "", + defaultKeyFunc, + &jwt.RegisteredClaims{ + Audience: jwt.ClaimStrings{"test", "test2"}, + }, + false, + jwt.ValidationErrorAudience, + []error{jwt.ErrTokenInvalidAudience}, + jwt.NewParser(jwt.WithAudience("bad")), + jwt.SigningMethodRS256, + }, { "RFC7519 Claims - single aud with wrong type", "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOjF9.8mAIDUfZNQT3TGm1QFIQp91OCpJpQpbB1-m9pA2mkHc", // { "aud": 1 } diff --git a/validator_option.go b/validator_option.go index eb29dc30..e52729c3 100644 --- a/validator_option.go +++ b/validator_option.go @@ -15,7 +15,9 @@ type validationOption func(*validator) // Note that this struct is (currently) un-exported, its naming is subject to change and will only be exported once // the API is more stable. type validator struct { - leeway time.Duration // Leeway to provide when validating time values + audience *string // Expected audience value + skipAudience bool // Ignore aud check + leeway time.Duration // Leeway to provide when validating time values } // withLeeway is an option to set the clock skew (leeway) window @@ -27,3 +29,32 @@ func withLeeway(d time.Duration) validationOption { v.leeway = d } } + +// WithAudience returns the ParserOption for specifying an expected aud member value +// +// Note that this function is (currently) un-exported, its naming is subject to change and will only be exported once +// the API is more stable. +func withAudience(aud string) validationOption { + return func(v *validator) { + v.audience = &aud + } +} + +// WithoutAudienceValidation returns the ParserOption that specifies audience check should be skipped +// +// Note that this function is (currently) un-exported, its naming is subject to change and will only be exported once +// the API is more stable. +func withoutAudienceValidation() validationOption { + return func(v *validator) { + v.skipAudience = true + } +} + +// getValidator return the validation given the options +func getValidator(opts ...validationOption) validator { + v := validator{} + for _, o := range opts { + o(&v) + } + return v +} From ce02ed3214d4e1ee9c9d5d9f86d8d22de1d77b5a Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Tue, 8 Mar 2022 19:20:04 -0600 Subject: [PATCH 03/13] fix comments --- validator_option.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/validator_option.go b/validator_option.go index e52729c3..a2b48154 100644 --- a/validator_option.go +++ b/validator_option.go @@ -30,7 +30,7 @@ func withLeeway(d time.Duration) validationOption { } } -// WithAudience returns the ParserOption for specifying an expected aud member value +// withAudience returns the ParserOption for specifying an expected aud member value // // Note that this function is (currently) un-exported, its naming is subject to change and will only be exported once // the API is more stable. @@ -40,7 +40,7 @@ func withAudience(aud string) validationOption { } } -// WithoutAudienceValidation returns the ParserOption that specifies audience check should be skipped +// withoutAudienceValidation returns the ParserOption that specifies audience check should be skipped // // Note that this function is (currently) un-exported, its naming is subject to change and will only be exported once // the API is more stable. From 5ea9aab9030d4cb368cfe04df4e48828e92750b4 Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Tue, 8 Mar 2022 19:23:51 -0600 Subject: [PATCH 04/13] update map_claims --- map_claims.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/map_claims.go b/map_claims.go index 7b623f02..c9ba700e 100644 --- a/map_claims.go +++ b/map_claims.go @@ -128,16 +128,19 @@ func (m MapClaims) validateAudience(req bool, opts ...validationOption) bool { } validator := getValidator(opts...) - + if validator.skipAudience { return true } - if validator.audience == nil { - return false + // Based on my reading of https://datatracker.ietf.org/doc/html/rfc7519/#section-4.1.3 + // this should technically fail. This is left as a decision for the maintainers to alter + // the behavior as it would be a breaking change. + if validator.audience != nil { + return m.VerifyAudience(*validator.audience, true) } - return m.VerifyAudience(*validator.audience, true) + return !req } // Valid validates time based claims "exp, iat, nbf". From 70f0feb754df53346ddff2a762bd188ab8642a69 Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Wed, 9 Mar 2022 05:26:02 -0600 Subject: [PATCH 05/13] add iat tests --- parser_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/parser_test.go b/parser_test.go index e25ff0b2..5418a678 100644 --- a/parser_test.go +++ b/parser_test.go @@ -188,6 +188,28 @@ var jwtTestData = []struct { nil, jwt.SigningMethodRS256, }, + { + "basic iat", + "", // autogen + defaultKeyFunc, + jwt.MapClaims{"foo": "bar", "iat": float64(time.Now().Unix() + 100)}, + true, + 0, + nil, + nil, + jwt.SigningMethodRS256, + }, + { + "basic iat with validation", + "", // autogen + defaultKeyFunc, + jwt.MapClaims{"foo": "bar", "iat": float64(time.Now().Unix() + 100)}, + false, + jwt.ValidationErrorIssuedAt, + []error{jwt.ErrTokenUsedBeforeIssued}, + jwt.NewParser(jwt.WithIssuedAt()), + jwt.SigningMethodRS256, + }, { "invalid signing method", "", From b964c145a06ebe124f3d87e19dc33db104856cdd Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Wed, 9 Mar 2022 05:57:54 -0600 Subject: [PATCH 06/13] keep the default behavior --- claims.go | 4 ++-- map_claims.go | 12 ++++-------- parser_option.go | 6 +++--- parser_test.go | 20 ++++++++++---------- validator_option.go | 10 +++++----- 5 files changed, 24 insertions(+), 28 deletions(-) diff --git a/claims.go b/claims.go index 4a2c048d..3463c365 100644 --- a/claims.go +++ b/claims.go @@ -101,7 +101,7 @@ func (c *RegisteredClaims) VerifyExpiresAt(cmp time.Time, req bool, opts ...vali // If req is false, it will return true, if iat is unset. func (c *RegisteredClaims) VerifyIssuedAt(cmp time.Time, req bool, opts ...validationOption) bool { validator := getValidator(opts...) - if !validator.iat { + if validator.skipIssuedAt { return true } @@ -202,7 +202,7 @@ func (c *StandardClaims) VerifyExpiresAt(cmp int64, req bool, opts ...validation // If req is false, it will return true, if iat is unset. func (c *StandardClaims) VerifyIssuedAt(cmp int64, req bool, opts ...validationOption) bool { validator := getValidator(opts...) - if !validator.iat { + if validator.skipIssuedAt { return true } diff --git a/map_claims.go b/map_claims.go index 51d0422a..906259c9 100644 --- a/map_claims.go +++ b/map_claims.go @@ -2,7 +2,6 @@ package jwt import ( "encoding/json" - "errors" "time" // "fmt" ) @@ -73,7 +72,7 @@ func (m MapClaims) VerifyIssuedAt(cmp int64, req bool, opts ...validationOption) // validate the type switch v.(type) { case float64, json.Number: - if !getValidator(opts...).iat { + if getValidator(opts...).skipIssuedAt { return true } default: @@ -140,20 +139,17 @@ func (m MapClaims) Valid(opts ...validationOption) error { now := TimeFunc().Unix() if !m.VerifyExpiresAt(now, false, opts...) { - // TODO(oxisto): this should be replaced with ErrTokenExpired - vErr.Inner = errors.New("Token is expired") + vErr.Inner = ErrTokenExpired vErr.Errors |= ValidationErrorExpired } if !m.VerifyIssuedAt(now, false, opts...) { - // TODO(oxisto): this should be replaced with ErrTokenUsedBeforeIssued - vErr.Inner = errors.New("Token used before issued") + vErr.Inner = ErrTokenUsedBeforeIssued vErr.Errors |= ValidationErrorIssuedAt } if !m.VerifyNotBefore(now, false, opts...) { - // TODO(oxisto): this should be replaced with ErrTokenNotValidYet - vErr.Inner = errors.New("Token is not valid yet") + vErr.Inner = ErrTokenNotValidYet vErr.Errors |= ValidationErrorNotValidYet } diff --git a/parser_option.go b/parser_option.go index af2f6660..968ced91 100644 --- a/parser_option.go +++ b/parser_option.go @@ -37,9 +37,9 @@ func WithLeeway(d time.Duration) ParserOption { } } -// WithIssuedAt is an option to enable the validation of the issued at (iat) claim. -func WithIssuedAt() ParserOption { +// WithIssuedAt is an option to disable the validation of the issued at (iat) claim. +func WithoutIssuedAt() ParserOption { return func(p *Parser) { - p.validationOptions = append(p.validationOptions, withIssuedAt()) + p.validationOptions = append(p.validationOptions, withoutIssuedAt()) } } diff --git a/parser_test.go b/parser_test.go index 5418a678..2b95125b 100644 --- a/parser_test.go +++ b/parser_test.go @@ -192,22 +192,22 @@ var jwtTestData = []struct { "basic iat", "", // autogen defaultKeyFunc, - jwt.MapClaims{"foo": "bar", "iat": float64(time.Now().Unix() + 100)}, - true, - 0, - nil, + jwt.MapClaims{"foo": "bar", "nbf": float64(time.Now().Unix()), "iat": float64(time.Now().Unix() + 100)}, + false, + jwt.ValidationErrorIssuedAt, + []error{jwt.ErrTokenUsedBeforeIssued}, nil, jwt.SigningMethodRS256, }, { - "basic iat with validation", + "basic iat skip validation", "", // autogen defaultKeyFunc, - jwt.MapClaims{"foo": "bar", "iat": float64(time.Now().Unix() + 100)}, - false, - jwt.ValidationErrorIssuedAt, - []error{jwt.ErrTokenUsedBeforeIssued}, - jwt.NewParser(jwt.WithIssuedAt()), + jwt.MapClaims{"foo": "bar", "nbf": float64(time.Now().Unix()), "iat": float64(time.Now().Unix() + 100)}, + true, + 0, + nil, + jwt.NewParser(jwt.WithoutIssuedAt()), jwt.SigningMethodRS256, }, { diff --git a/validator_option.go b/validator_option.go index 2db63bad..401436a1 100644 --- a/validator_option.go +++ b/validator_option.go @@ -15,8 +15,8 @@ type validationOption func(*validator) // Note that this struct is (currently) un-exported, its naming is subject to change and will only be exported once // the API is more stable. type validator struct { - leeway time.Duration // Leeway to provide when validating time values - iat bool + leeway time.Duration // Leeway to provide when validating time values + skipIssuedAt bool } // withLeeway is an option to set the clock skew (leeway) window @@ -29,13 +29,13 @@ func withLeeway(d time.Duration) validationOption { } } -// withIssuedAth is an option to enable the validation of the issued at (iat) claim +// withoutIssuedAth is an option to disable the validation of the issued at (iat) claim // // Note that this function is (currently) un-exported, its naming is subject to change and will only be exported once // the API is more stable. -func withIssuedAt() validationOption { +func withoutIssuedAt() validationOption { return func(v *validator) { - v.iat = true + v.skipIssuedAt = true } } From 540f33386dfd8cfb932c61e503a5588652c7e47e Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Wed, 9 Mar 2022 06:00:11 -0600 Subject: [PATCH 07/13] fix comment --- parser_option.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parser_option.go b/parser_option.go index 968ced91..a2c4a8d6 100644 --- a/parser_option.go +++ b/parser_option.go @@ -37,7 +37,7 @@ func WithLeeway(d time.Duration) ParserOption { } } -// WithIssuedAt is an option to disable the validation of the issued at (iat) claim. +// WithoutIssuedAt is an option to disable the validation of the issued at (iat) claim. func WithoutIssuedAt() ParserOption { return func(p *Parser) { p.validationOptions = append(p.validationOptions, withoutIssuedAt()) From ce20254bf01244491ef64cae9b2f36d03dde6d6c Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Wed, 9 Mar 2022 06:06:04 -0600 Subject: [PATCH 08/13] update comment --- parser_option.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/parser_option.go b/parser_option.go index a2c4a8d6..7c55dfb6 100644 --- a/parser_option.go +++ b/parser_option.go @@ -38,6 +38,8 @@ func WithLeeway(d time.Duration) ParserOption { } // WithoutIssuedAt is an option to disable the validation of the issued at (iat) claim. +// The current `iat` time based validation is planned to be deprecated in v5 + func WithoutIssuedAt() ParserOption { return func(p *Parser) { p.validationOptions = append(p.validationOptions, withoutIssuedAt()) From 840d358a4755591d37d236975927726cde061049 Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Wed, 9 Mar 2022 06:35:28 -0600 Subject: [PATCH 09/13] add comment --- validator_option.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validator_option.go b/validator_option.go index f961b269..fd0e2318 100644 --- a/validator_option.go +++ b/validator_option.go @@ -18,7 +18,7 @@ type validator struct { leeway time.Duration // Leeway to provide when validating time values audience *string // Expected audience value skipAudience bool // Ignore aud check - skipIssuedAt bool + skipIssuedAt bool // Ignore iat check } // withLeeway is an option to set the clock skew (leeway) window From d5ac9dabb2bd01a65e6d90e34d5b4e1a0911859e Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Wed, 9 Mar 2022 14:58:06 -0600 Subject: [PATCH 10/13] add leeway support to iat validation --- claims.go | 13 +++++++------ map_claims.go | 10 ++++++---- parser_test.go | 22 ++++++++++++++++++++++ 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/claims.go b/claims.go index cc54f015..b45fcaad 100644 --- a/claims.go +++ b/claims.go @@ -111,10 +111,10 @@ func (c *RegisteredClaims) VerifyIssuedAt(cmp time.Time, req bool, opts ...valid } if c.IssuedAt == nil { - return verifyIat(nil, cmp, req) + return verifyIat(nil, cmp, req, validator.leeway) } - return verifyIat(&c.IssuedAt.Time, cmp, req) + return verifyIat(&c.IssuedAt.Time, cmp, req, validator.leeway) } // VerifyNotBefore compares the nbf claim against cmp (cmp >= nbf). @@ -238,11 +238,11 @@ func (c *StandardClaims) VerifyIssuedAt(cmp int64, req bool, opts ...validationO } if c.IssuedAt == 0 { - return verifyIat(nil, time.Unix(cmp, 0), req) + return verifyIat(nil, time.Unix(cmp, 0), req, validator.leeway) } t := time.Unix(c.IssuedAt, 0) - return verifyIat(&t, time.Unix(cmp, 0), req) + return verifyIat(&t, time.Unix(cmp, 0), req, validator.leeway) } // VerifyNotBefore compares the nbf claim against cmp (cmp >= nbf). @@ -316,11 +316,12 @@ func verifyExp(exp *time.Time, now time.Time, required bool, skew time.Duration) return now.Before((*exp).Add(+skew)) } -func verifyIat(iat *time.Time, now time.Time, required bool) bool { +func verifyIat(iat *time.Time, now time.Time, required bool, skew time.Duration) bool { if iat == nil { return !required } - return now.After(*iat) || now.Equal(*iat) + t := (*iat).Add(-skew) + return now.After(t) || now.Equal(*iat) } func verifyNbf(nbf *time.Time, now time.Time, required bool, skew time.Duration) bool { diff --git a/map_claims.go b/map_claims.go index 56dfe2d4..c72cf08d 100644 --- a/map_claims.go +++ b/map_claims.go @@ -69,10 +69,12 @@ func (m MapClaims) VerifyIssuedAt(cmp int64, req bool, opts ...validationOption) return !req } + validator := getValidator(opts...) + // validate the type switch v.(type) { case float64, json.Number: - if getValidator(opts...).skipIssuedAt { + if validator.skipIssuedAt { return true } default: @@ -82,14 +84,14 @@ func (m MapClaims) VerifyIssuedAt(cmp int64, req bool, opts ...validationOption) switch iat := v.(type) { case float64: if iat == 0 { - return verifyIat(nil, cmpTime, req) + return verifyIat(nil, cmpTime, req, validator.leeway) } - return verifyIat(&newNumericDateFromSeconds(iat).Time, cmpTime, req) + return verifyIat(&newNumericDateFromSeconds(iat).Time, cmpTime, req, validator.leeway) case json.Number: v, _ := iat.Float64() - return verifyIat(&newNumericDateFromSeconds(v).Time, cmpTime, req) + return verifyIat(&newNumericDateFromSeconds(v).Time, cmpTime, req, validator.leeway) } return false diff --git a/parser_test.go b/parser_test.go index 5f6f4d9a..4644695b 100644 --- a/parser_test.go +++ b/parser_test.go @@ -210,6 +210,28 @@ var jwtTestData = []struct { jwt.NewParser(jwt.WithoutIssuedAt()), jwt.SigningMethodRS256, }, + { + "basic iat with 60s skew", + "", // autogen + defaultKeyFunc, + jwt.MapClaims{"foo": "bar", "iat": float64(time.Now().Unix() + 100)}, + false, + jwt.ValidationErrorIssuedAt, + []error{jwt.ErrTokenUsedBeforeIssued}, + jwt.NewParser(jwt.WithLeeway(time.Minute)), + jwt.SigningMethodRS256, + }, + { + "basic iat with 120s skew", + "", // autogen + defaultKeyFunc, + jwt.MapClaims{"foo": "bar", "iat": float64(time.Now().Unix() + 100)}, + true, + 0, + nil, + jwt.NewParser(jwt.WithLeeway(2 * time.Minute)), + jwt.SigningMethodRS256, + }, { "invalid signing method", "", From 8d48d0555ec0bd997cf6ad79facfdc2c130bec2d Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Sun, 13 Mar 2022 22:08:11 -0500 Subject: [PATCH 11/13] address review comments --- claims.go | 28 ++++++---------------------- map_claims.go | 14 +++----------- parser_option.go | 6 +++--- parser_test.go | 4 ++-- validator_option.go | 17 +++++++++++++++-- 5 files changed, 29 insertions(+), 40 deletions(-) diff --git a/claims.go b/claims.go index b45fcaad..3d6b7a06 100644 --- a/claims.go +++ b/claims.go @@ -135,21 +135,13 @@ func (c *RegisteredClaims) VerifyIssuer(cmp string, req bool) bool { } func (c *RegisteredClaims) validateAudience(req bool, opts ...validationOption) bool { - if len(c.Audience) == 0 { - return !req - } - - validator := getValidator(opts...) - - if validator.skipAudience { - return true - } + aud, skip := getAudienceValidationOpts(len(c.Audience) != 0, opts...) // Based on my reading of https://datatracker.ietf.org/doc/html/rfc7519/#section-4.1.3 // this should technically fail. This is left as a decision for the maintainers to alter // the behavior as it would be a breaking change. - if validator.audience != nil { - return c.VerifyAudience(*validator.audience, true) + if !skip && aud != nil { + return c.VerifyAudience(*aud, req) } return !req @@ -264,21 +256,13 @@ func (c *StandardClaims) VerifyIssuer(cmp string, req bool) bool { } func (c *StandardClaims) validateAudience(req bool, opts ...validationOption) bool { - if c.Audience == "" { - return !req - } - - validator := getValidator(opts...) - - if validator.skipAudience { - return true - } + aud, skip := getAudienceValidationOpts(c.Audience != "", opts...) // Based on my reading of https://datatracker.ietf.org/doc/html/rfc7519/#section-4.1.3 // this should technically fail. This is left as a decision for the maintainers to alter // the behavior as it would be a breaking change. - if validator.audience != nil { - return c.VerifyAudience(*validator.audience, true) + if !skip && aud != nil { + return c.VerifyAudience(*aud, req) } return !req diff --git a/map_claims.go b/map_claims.go index c72cf08d..143ebd63 100644 --- a/map_claims.go +++ b/map_claims.go @@ -134,21 +134,13 @@ func (m MapClaims) VerifyIssuer(cmp string, req bool) bool { func (m MapClaims) validateAudience(req bool, opts ...validationOption) bool { _, ok := m["aud"] - if !ok { - return !req - } - - validator := getValidator(opts...) - - if validator.skipAudience { - return true - } + aud, skip := getAudienceValidationOpts(ok, opts...) // Based on my reading of https://datatracker.ietf.org/doc/html/rfc7519/#section-4.1.3 // this should technically fail. This is left as a decision for the maintainers to alter // the behavior as it would be a breaking change. - if validator.audience != nil { - return m.VerifyAudience(*validator.audience, true) + if !skip && aud != nil { + return m.VerifyAudience(*aud, req) } return !req diff --git a/parser_option.go b/parser_option.go index 6681c0f9..d86c5f1c 100644 --- a/parser_option.go +++ b/parser_option.go @@ -37,12 +37,12 @@ func WithLeeway(d time.Duration) ParserOption { } } -// WithoutIssuedAt is an option to disable the validation of the issued at (iat) claim. +// WithoutIssuedAtValidation is an option to disable the validation of the issued at (iat) claim. // The current `iat` time based validation is planned to be deprecated in v5 -func WithoutIssuedAt() ParserOption { +func WithoutIssuedAtValidation() ParserOption { return func(p *Parser) { - p.validationOptions = append(p.validationOptions, withoutIssuedAt()) + p.validationOptions = append(p.validationOptions, withoutIssuedAtValidation()) } } diff --git a/parser_test.go b/parser_test.go index 4644695b..a94b3b39 100644 --- a/parser_test.go +++ b/parser_test.go @@ -207,7 +207,7 @@ var jwtTestData = []struct { true, 0, nil, - jwt.NewParser(jwt.WithoutIssuedAt()), + jwt.NewParser(jwt.WithoutIssuedAtValidation()), jwt.SigningMethodRS256, }, { @@ -375,7 +375,7 @@ var jwtTestData = []struct { "", defaultKeyFunc, &jwt.RegisteredClaims{ - Audience: jwt.ClaimStrings{"test", "test"}, + Audience: jwt.ClaimStrings{"test", "test2"}, }, true, 0, diff --git a/validator_option.go b/validator_option.go index fd0e2318..54fbeb9f 100644 --- a/validator_option.go +++ b/validator_option.go @@ -31,11 +31,11 @@ func withLeeway(d time.Duration) validationOption { } } -// withoutIssuedAth is an option to disable the validation of the issued at (iat) claim +// withoutIssuedAtValidation is an option to disable the validation of the issued at (iat) claim // // Note that this function is (currently) un-exported, its naming is subject to change and will only be exported once // the API is more stable. -func withoutIssuedAt() validationOption { +func withoutIssuedAtValidation() validationOption { return func(v *validator) { v.skipIssuedAt = true } @@ -69,3 +69,16 @@ func getValidator(opts ...validationOption) validator { } return v } + +// getAudienceValidationOpts returns the aud, and skip validation values from the +// options. If validation is not required then function will return true for skip. +func getAudienceValidationOpts(req bool, opts ...validationOption) (*string, bool) { + if !req { + return nil, true + } + validator := getValidator(opts...) + if validator.skipAudience { + return nil, true + } + return validator.audience, false +} From 9dbe240dd43826d4c48f61475c27a3a913aad07b Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Sun, 13 Mar 2022 22:29:47 -0500 Subject: [PATCH 12/13] refactor getAudienceValidationOpts --- claims.go | 6 ++++-- map_claims.go | 3 ++- validator_option.go | 7 +++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/claims.go b/claims.go index 3d6b7a06..3c3ac96f 100644 --- a/claims.go +++ b/claims.go @@ -135,7 +135,8 @@ func (c *RegisteredClaims) VerifyIssuer(cmp string, req bool) bool { } func (c *RegisteredClaims) validateAudience(req bool, opts ...validationOption) bool { - aud, skip := getAudienceValidationOpts(len(c.Audience) != 0, opts...) + v := getValidator(opts...) + aud, skip := v.getAudienceValidationOpts(len(c.Audience) != 0) // Based on my reading of https://datatracker.ietf.org/doc/html/rfc7519/#section-4.1.3 // this should technically fail. This is left as a decision for the maintainers to alter @@ -256,7 +257,8 @@ func (c *StandardClaims) VerifyIssuer(cmp string, req bool) bool { } func (c *StandardClaims) validateAudience(req bool, opts ...validationOption) bool { - aud, skip := getAudienceValidationOpts(c.Audience != "", opts...) + v := getValidator(opts...) + aud, skip := v.getAudienceValidationOpts(c.Audience != "") // Based on my reading of https://datatracker.ietf.org/doc/html/rfc7519/#section-4.1.3 // this should technically fail. This is left as a decision for the maintainers to alter diff --git a/map_claims.go b/map_claims.go index 143ebd63..832a6afb 100644 --- a/map_claims.go +++ b/map_claims.go @@ -134,7 +134,8 @@ func (m MapClaims) VerifyIssuer(cmp string, req bool) bool { func (m MapClaims) validateAudience(req bool, opts ...validationOption) bool { _, ok := m["aud"] - aud, skip := getAudienceValidationOpts(ok, opts...) + v := getValidator(opts...) + aud, skip := v.getAudienceValidationOpts(ok) // Based on my reading of https://datatracker.ietf.org/doc/html/rfc7519/#section-4.1.3 // this should technically fail. This is left as a decision for the maintainers to alter diff --git a/validator_option.go b/validator_option.go index 54fbeb9f..f5727c5a 100644 --- a/validator_option.go +++ b/validator_option.go @@ -72,13 +72,12 @@ func getValidator(opts ...validationOption) validator { // getAudienceValidationOpts returns the aud, and skip validation values from the // options. If validation is not required then function will return true for skip. -func getAudienceValidationOpts(req bool, opts ...validationOption) (*string, bool) { +func (v *validator) getAudienceValidationOpts(req bool) (*string, bool) { if !req { return nil, true } - validator := getValidator(opts...) - if validator.skipAudience { + if v.skipAudience { return nil, true } - return validator.audience, false + return v.audience, false } From 5a1d3a2ec963c40212bafe7fc9975e11213451ef Mon Sep 17 00:00:00 2001 From: Kolawole Segun Date: Sun, 13 Mar 2022 22:35:21 -0500 Subject: [PATCH 13/13] combine required and skipAudience check --- validator_option.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/validator_option.go b/validator_option.go index f5727c5a..abc0fbf4 100644 --- a/validator_option.go +++ b/validator_option.go @@ -73,10 +73,7 @@ func getValidator(opts ...validationOption) validator { // getAudienceValidationOpts returns the aud, and skip validation values from the // options. If validation is not required then function will return true for skip. func (v *validator) getAudienceValidationOpts(req bool) (*string, bool) { - if !req { - return nil, true - } - if v.skipAudience { + if !req || v.skipAudience { return nil, true } return v.audience, false