Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "feat: port clockskew support (#139)" #184

Merged
merged 2 commits into from Mar 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
66 changes: 23 additions & 43 deletions claims.go
Expand Up @@ -9,10 +9,7 @@ import (
// Claims must just have a Valid method that determines
// if the token is invalid for any supported reason
type Claims interface {
// Valid implements claim validation. The opts are function style options that can
// be used to fine-tune the validation. The type used for the options is intentionally
// un-exported, since its API and its naming is subject to change.
Valid(opts ...validationOption) error
Valid() error

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to maintain backwards compatibility, a ValidWithOptions(...) could be added to the public Interface.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even adding a new function will break the interface, as someone who implements their own Claims structure, would need to implement ValidWithOptions as well. I think we are stuck with this new feature until v5 :(

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even adding a new function will break the interface, as someone who implements their own Claims structure, would need to implement ValidWithOptions as well. I think we are stuck with this new feature until v5 :(

Right. What you think about creating a new interface to extend Claims?

type Claims interface {
	Valid() bool
}

type ClaimsExtended interface {
	Claims
	ValidWithOptions() bool
}

type RegisteredClaims struct {
}

func (r *RegisteredClaims) Valid() bool {
	fmt.Println("RegisteredClaims::Valid")
	return true
}

type RegisteredClaimsExtended struct {
	RegisteredClaims
}

func (r *RegisteredClaimsExtended) ValidWithOptions() bool {
	fmt.Println("RegisteredClaimsExtended::ValidWithOptions")
	return true
}

// call function accordingly to interface type (https://github.com/golang-jwt/jwt/blob/1096e506e671d6d6fe134cc997bbd475937392c8/parser.go#L87)
func Verify(claims Claims) bool {
        // is this claims extended?
	if claimsExt, ok := claims.(ClaimsExtended); ok {
		return claimsExt.ValidWithOptions()
	}

	return claims.Valid()
}

func main() {
	claims := &RegisteredClaims{}
	fmt.Println(Verify(claims))

	claimsExt := &RegisteredClaimsExtended{}
	fmt.Println(Verify(claimsExt))
}

}

// RegisteredClaims are a structured version of the JWT Claims Set,
Expand Down Expand Up @@ -51,13 +48,13 @@ type RegisteredClaims struct {
// There is no accounting for clock skew.
// As well, if any of the above claims are not in the token, it will still
// be considered a valid claim.
func (c RegisteredClaims) Valid(opts ...validationOption) error {
func (c RegisteredClaims) Valid() error {
vErr := new(ValidationError)
now := TimeFunc()

// The claims below are optional, by default, so if they are set to the
// default value in Go, let's not fail the verification for them.
if !c.VerifyExpiresAt(now, false, opts...) {
if !c.VerifyExpiresAt(now, false) {
delta := now.Sub(c.ExpiresAt.Time)
vErr.Inner = fmt.Errorf("%s by %s", ErrTokenExpired, delta)
vErr.Errors |= ValidationErrorExpired
Expand All @@ -68,7 +65,7 @@ func (c RegisteredClaims) Valid(opts ...validationOption) error {
vErr.Errors |= ValidationErrorIssuedAt
}

if !c.VerifyNotBefore(now, false, opts...) {
if !c.VerifyNotBefore(now, false) {
vErr.Inner = ErrTokenNotValidYet
vErr.Errors |= ValidationErrorNotValidYet
}
Expand All @@ -88,16 +85,12 @@ 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)
}
func (c *RegisteredClaims) VerifyExpiresAt(cmp time.Time, req bool) bool {
if c.ExpiresAt == nil {
return verifyExp(nil, cmp, req, validator.leeway)
return verifyExp(nil, cmp, req)
}

return verifyExp(&c.ExpiresAt.Time, cmp, req, validator.leeway)
return verifyExp(&c.ExpiresAt.Time, cmp, req)
}

// VerifyIssuedAt compares the iat claim against cmp (cmp >= iat).
Expand All @@ -112,16 +105,12 @@ 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)
}
func (c *RegisteredClaims) VerifyNotBefore(cmp time.Time, req bool) bool {
if c.NotBefore == nil {
return verifyNbf(nil, cmp, req, validator.leeway)
return verifyNbf(nil, cmp, req)
}

return verifyNbf(&c.NotBefore.Time, cmp, req, validator.leeway)
return verifyNbf(&c.NotBefore.Time, cmp, req)
}

// VerifyIssuer compares the iss claim against cmp.
Expand Down Expand Up @@ -152,13 +141,13 @@ type StandardClaims struct {
// 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
// be considered a valid claim.
func (c StandardClaims) Valid(opts ...validationOption) error {
func (c StandardClaims) Valid() error {
vErr := new(ValidationError)
now := TimeFunc().Unix()

// The claims below are optional, by default, so if they are set to the
// default value in Go, let's not fail the verification for them.
if !c.VerifyExpiresAt(now, false, opts...) {
if !c.VerifyExpiresAt(now, false) {
delta := time.Unix(now, 0).Sub(time.Unix(c.ExpiresAt, 0))
vErr.Inner = fmt.Errorf("%s by %s", ErrTokenExpired, delta)
vErr.Errors |= ValidationErrorExpired
Expand All @@ -169,7 +158,7 @@ func (c StandardClaims) Valid(opts ...validationOption) error {
vErr.Errors |= ValidationErrorIssuedAt
}

if !c.VerifyNotBefore(now, false, opts...) {
if !c.VerifyNotBefore(now, false) {
vErr.Inner = ErrTokenNotValidYet
vErr.Errors |= ValidationErrorNotValidYet
}
Expand All @@ -189,17 +178,13 @@ 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)
}
func (c *StandardClaims) VerifyExpiresAt(cmp int64, req bool) bool {
if c.ExpiresAt == 0 {
return verifyExp(nil, time.Unix(cmp, 0), req, validator.leeway)
return verifyExp(nil, time.Unix(cmp, 0), req)
}

t := time.Unix(c.ExpiresAt, 0)
return verifyExp(&t, time.Unix(cmp, 0), req, validator.leeway)
return verifyExp(&t, time.Unix(cmp, 0), req)
}

// VerifyIssuedAt compares the iat claim against cmp (cmp >= iat).
Expand All @@ -215,17 +200,13 @@ 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)
}
func (c *StandardClaims) VerifyNotBefore(cmp int64, req bool) bool {
if c.NotBefore == 0 {
return verifyNbf(nil, time.Unix(cmp, 0), req, validator.leeway)
return verifyNbf(nil, time.Unix(cmp, 0), req)
}

t := time.Unix(c.NotBefore, 0)
return verifyNbf(&t, time.Unix(cmp, 0), req, validator.leeway)
return verifyNbf(&t, time.Unix(cmp, 0), req)
}

// VerifyIssuer compares the iss claim against cmp.
Expand Down Expand Up @@ -259,11 +240,11 @@ func verifyAud(aud []string, cmp string, required bool) bool {
return result
}

func verifyExp(exp *time.Time, now time.Time, required bool, skew time.Duration) bool {
func verifyExp(exp *time.Time, now time.Time, required bool) bool {
if exp == nil {
return !required
}
return now.Before((*exp).Add(+skew))
return now.Before(*exp)
}

func verifyIat(iat *time.Time, now time.Time, required bool) bool {
Expand All @@ -273,12 +254,11 @@ func verifyIat(iat *time.Time, now time.Time, required bool) bool {
return now.After(*iat) || now.Equal(*iat)
}

func verifyNbf(nbf *time.Time, now time.Time, required bool, skew time.Duration) bool {
func verifyNbf(nbf *time.Time, now time.Time, required bool) bool {
if nbf == nil {
return !required
}
t := (*nbf).Add(-skew)
return now.After(t) || now.Equal(t)
return now.After(*nbf) || now.Equal(*nbf)
}

func verifyIss(iss string, cmp string, required bool) bool {
Expand Down
4 changes: 4 additions & 0 deletions go.mod
@@ -1,3 +1,7 @@
module github.com/golang-jwt/jwt/v4

go 1.16

retract (
v4.4.0 // Contains a backwards incompatible change to the Claims interface.
)
32 changes: 11 additions & 21 deletions map_claims.go
Expand Up @@ -34,30 +34,25 @@ func (m MapClaims) 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 (m MapClaims) VerifyExpiresAt(cmp int64, req bool, opts ...validationOption) bool {
func (m MapClaims) VerifyExpiresAt(cmp int64, req bool) bool {
cmpTime := time.Unix(cmp, 0)

v, ok := m["exp"]
if !ok {
return !req
}

validator := validator{}
for _, o := range opts {
o(&validator)
}

switch exp := v.(type) {
case float64:
if exp == 0 {
return verifyExp(nil, cmpTime, req, validator.leeway)
return verifyExp(nil, cmpTime, req)
}

return verifyExp(&newNumericDateFromSeconds(exp).Time, cmpTime, req, validator.leeway)
return verifyExp(&newNumericDateFromSeconds(exp).Time, cmpTime, req)
case json.Number:
v, _ := exp.Float64()

return verifyExp(&newNumericDateFromSeconds(v).Time, cmpTime, req, validator.leeway)
return verifyExp(&newNumericDateFromSeconds(v).Time, cmpTime, req)
}

return false
Expand Down Expand Up @@ -91,30 +86,25 @@ func (m MapClaims) 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 (m MapClaims) VerifyNotBefore(cmp int64, req bool, opts ...validationOption) bool {
func (m MapClaims) VerifyNotBefore(cmp int64, req bool) bool {
cmpTime := time.Unix(cmp, 0)

v, ok := m["nbf"]
if !ok {
return !req
}

validator := validator{}
for _, o := range opts {
o(&validator)
}

switch nbf := v.(type) {
case float64:
if nbf == 0 {
return verifyNbf(nil, cmpTime, req, validator.leeway)
return verifyNbf(nil, cmpTime, req)
}

return verifyNbf(&newNumericDateFromSeconds(nbf).Time, cmpTime, req, validator.leeway)
return verifyNbf(&newNumericDateFromSeconds(nbf).Time, cmpTime, req)
case json.Number:
v, _ := nbf.Float64()

return verifyNbf(&newNumericDateFromSeconds(v).Time, cmpTime, req, validator.leeway)
return verifyNbf(&newNumericDateFromSeconds(v).Time, cmpTime, req)
}

return false
Expand All @@ -131,11 +121,11 @@ func (m MapClaims) VerifyIssuer(cmp string, req bool) bool {
// There is no accounting for clock skew.
// As well, if any of the above claims are not in the token, it will still
// be considered a valid claim.
func (m MapClaims) Valid(opts ...validationOption) error {
func (m MapClaims) Valid() error {
vErr := new(ValidationError)
now := TimeFunc().Unix()

if !m.VerifyExpiresAt(now, false, opts...) {
if !m.VerifyExpiresAt(now, false) {
// TODO(oxisto): this should be replaced with ErrTokenExpired
vErr.Inner = errors.New("Token is expired")
vErr.Errors |= ValidationErrorExpired
Expand All @@ -147,7 +137,7 @@ func (m MapClaims) Valid(opts ...validationOption) error {
vErr.Errors |= ValidationErrorIssuedAt
}

if !m.VerifyNotBefore(now, false, opts...) {
if !m.VerifyNotBefore(now, false) {
// TODO(oxisto): this should be replaced with ErrTokenNotValidYet
vErr.Inner = errors.New("Token is not valid yet")
vErr.Errors |= ValidationErrorNotValidYet
Expand Down
5 changes: 2 additions & 3 deletions parser.go
Expand Up @@ -22,8 +22,6 @@ type Parser struct {
//
// Deprecated: In future releases, this field will not be exported anymore and should be set with an option to NewParser instead.
SkipClaimsValidation bool

validationOptions []validationOption
}

// NewParser creates a new Parser with the specified options
Expand Down Expand Up @@ -84,7 +82,8 @@ func (p *Parser) ParseWithClaims(tokenString string, claims Claims, keyFunc Keyf

// Validate Claims
if !p.SkipClaimsValidation {
if err := token.Claims.Valid(p.validationOptions...); err != nil {
if err := token.Claims.Valid(); err != nil {

// If the Claims Valid returned an error, check if it is a validation error,
// If it was another error type, create a ValidationError with a generic ClaimsInvalid flag set
if e, ok := err.(*ValidationError); !ok {
Expand Down
9 changes: 0 additions & 9 deletions parser_option.go
@@ -1,7 +1,5 @@
package jwt

import "time"

// ParserOption is used to implement functional-style options that modify the behavior of the parser. To add
// new options, just create a function (ideally beginning with With or Without) that returns an anonymous function that
// takes a *Parser type as input and manipulates its configuration accordingly.
Expand Down Expand Up @@ -29,10 +27,3 @@ func WithoutClaimsValidation() ParserOption {
p.SkipClaimsValidation = true
}
}

// WithLeeway returns the ParserOption for specifying the leeway window.
func WithLeeway(d time.Duration) ParserOption {
return func(p *Parser) {
p.validationOptions = append(p.validationOptions, withLeeway(d))
}
}
44 changes: 0 additions & 44 deletions parser_test.go
Expand Up @@ -78,28 +78,6 @@ var jwtTestData = []struct {
nil,
jwt.SigningMethodRS256,
},
{
"basic expired with 60s skew",
"", // autogen
defaultKeyFunc,
jwt.MapClaims{"foo": "bar", "exp": float64(time.Now().Unix() - 100)},
false,
jwt.ValidationErrorExpired,
[]error{jwt.ErrTokenExpired},
jwt.NewParser(jwt.WithLeeway(time.Minute)),
jwt.SigningMethodRS256,
},
{
"basic expired with 120s skew",
"", // autogen
defaultKeyFunc,
jwt.MapClaims{"foo": "bar", "exp": float64(time.Now().Unix() - 100)},
true,
0,
nil,
jwt.NewParser(jwt.WithLeeway(2 * time.Minute)),
jwt.SigningMethodRS256,
},
{
"basic nbf",
"", // autogen
Expand All @@ -111,28 +89,6 @@ var jwtTestData = []struct {
nil,
jwt.SigningMethodRS256,
},
{
"basic nbf with 60s skew",
"", // autogen
defaultKeyFunc,
jwt.MapClaims{"foo": "bar", "nbf": float64(time.Now().Unix() + 100)},
false,
jwt.ValidationErrorNotValidYet,
[]error{jwt.ErrTokenNotValidYet},
jwt.NewParser(jwt.WithLeeway(time.Minute)),
jwt.SigningMethodRS256,
},
{
"basic nbf with 120s skew",
"", // autogen
defaultKeyFunc,
jwt.MapClaims{"foo": "bar", "nbf": float64(time.Now().Unix() + 100)},
true,
0,
nil,
jwt.NewParser(jwt.WithLeeway(2 * time.Minute)),
jwt.SigningMethodRS256,
},
{
"expired and nbf",
"", // autogen
Expand Down
29 changes: 0 additions & 29 deletions validator_option.go

This file was deleted.