-
Notifications
You must be signed in to change notification settings - Fork 351
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
New validation API #236
Merged
Merged
New validation API #236
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
dc52415
New Validation API
oxisto 066f850
Fixed linting errors
oxisto 0e79f91
GetExpiresAt() -> GetExpirationTime()
oxisto 4990d2c
Added timeFunc, made iat optional
oxisto eedf3eb
Added option for audience check
oxisto 91f51d0
Apply suggestions from code review
oxisto 06a12c1
More documentation
oxisto 2281dd9
exported CustomClaims
oxisto 5d57c29
Re-enabled map claim tests. Added error return value to claim getter …
oxisto 5a65c47
Added more options to check iss and sub
oxisto 1d6e6dc
Simplified Validation API
oxisto 2036f52
Made validator private for now
oxisto File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,177 +1,16 @@ | ||
package jwt | ||
|
||
import ( | ||
"crypto/subtle" | ||
"fmt" | ||
"time" | ||
) | ||
|
||
// Claims must just have a Valid method that determines | ||
// if the token is invalid for any supported reason | ||
// Claims represent any form of a JWT Claims Set according to | ||
// https://datatracker.ietf.org/doc/html/rfc7519#section-4. In order to have a | ||
// common basis for validation, it is required that an implementation is able to | ||
// supply at least the claim names provided in | ||
// https://datatracker.ietf.org/doc/html/rfc7519#section-4.1 namely `exp`, | ||
// `iat`, `nbf`, `iss` and `aud`. | ||
type Claims interface { | ||
Valid() error | ||
} | ||
|
||
// RegisteredClaims are a structured version of the JWT Claims Set, | ||
// restricted to Registered Claim Names, as referenced at | ||
// https://datatracker.ietf.org/doc/html/rfc7519#section-4.1 | ||
// | ||
// This type can be used on its own, but then additional private and | ||
// public claims embedded in the JWT will not be parsed. The typical usecase | ||
// therefore is to embedded this in a user-defined claim type. | ||
// | ||
// See examples for how to use this with your own claim types. | ||
type RegisteredClaims struct { | ||
// the `iss` (Issuer) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.1 | ||
Issuer string `json:"iss,omitempty"` | ||
|
||
// the `sub` (Subject) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.2 | ||
Subject string `json:"sub,omitempty"` | ||
|
||
// the `aud` (Audience) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.3 | ||
Audience ClaimStrings `json:"aud,omitempty"` | ||
|
||
// the `exp` (Expiration Time) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.4 | ||
ExpiresAt *NumericDate `json:"exp,omitempty"` | ||
|
||
// the `nbf` (Not Before) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.5 | ||
NotBefore *NumericDate `json:"nbf,omitempty"` | ||
|
||
// the `iat` (Issued At) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.6 | ||
IssuedAt *NumericDate `json:"iat,omitempty"` | ||
|
||
// the `jti` (JWT ID) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.7 | ||
ID string `json:"jti,omitempty"` | ||
} | ||
|
||
// 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 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) { | ||
delta := now.Sub(c.ExpiresAt.Time) | ||
vErr.Inner = fmt.Errorf("%s by %s", ErrTokenExpired, delta) | ||
vErr.Errors |= ValidationErrorExpired | ||
} | ||
|
||
if !c.VerifyIssuedAt(now, false) { | ||
vErr.Inner = ErrTokenUsedBeforeIssued | ||
vErr.Errors |= ValidationErrorIssuedAt | ||
} | ||
|
||
if !c.VerifyNotBefore(now, false) { | ||
vErr.Inner = ErrTokenNotValidYet | ||
vErr.Errors |= ValidationErrorNotValidYet | ||
} | ||
|
||
if vErr.valid() { | ||
return nil | ||
} | ||
|
||
return vErr | ||
} | ||
|
||
// 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 *RegisteredClaims) VerifyAudience(cmp string, req bool) bool { | ||
return verifyAud(c.Audience, cmp, req) | ||
} | ||
|
||
// 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) bool { | ||
if c.ExpiresAt == nil { | ||
return verifyExp(nil, cmp, req) | ||
} | ||
|
||
return verifyExp(&c.ExpiresAt.Time, cmp, req) | ||
} | ||
|
||
// 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 { | ||
if c.IssuedAt == nil { | ||
return verifyIat(nil, cmp, req) | ||
} | ||
|
||
return verifyIat(&c.IssuedAt.Time, cmp, req) | ||
} | ||
|
||
// 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) bool { | ||
if c.NotBefore == nil { | ||
return verifyNbf(nil, cmp, req) | ||
} | ||
|
||
return verifyNbf(&c.NotBefore.Time, cmp, req) | ||
} | ||
|
||
// VerifyIssuer compares the iss claim against cmp. | ||
// If required is false, this method will return true if the value matches or is unset | ||
func (c *RegisteredClaims) VerifyIssuer(cmp string, req bool) bool { | ||
return verifyIss(c.Issuer, cmp, req) | ||
} | ||
|
||
// ----- helpers | ||
|
||
func verifyAud(aud []string, cmp string, required bool) bool { | ||
if len(aud) == 0 { | ||
return !required | ||
} | ||
// 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 *time.Time, now time.Time, required bool) bool { | ||
if exp == nil { | ||
return !required | ||
} | ||
return now.Before(*exp) | ||
} | ||
|
||
func verifyIat(iat *time.Time, now time.Time, required bool) bool { | ||
if iat == nil { | ||
return !required | ||
} | ||
return now.After(*iat) || now.Equal(*iat) | ||
} | ||
|
||
func verifyNbf(nbf *time.Time, now time.Time, required bool) bool { | ||
if nbf == nil { | ||
return !required | ||
} | ||
return now.After(*nbf) || now.Equal(*nbf) | ||
} | ||
|
||
func verifyIss(iss string, cmp string, required bool) bool { | ||
if iss == "" { | ||
return !required | ||
} | ||
if subtle.ConstantTimeCompare([]byte(iss), []byte(cmp)) != 0 { | ||
return true | ||
} else { | ||
return false | ||
} | ||
GetExpirationTime() (*NumericDate, error) | ||
GetIssuedAt() (*NumericDate, error) | ||
GetNotBefore() (*NumericDate, error) | ||
GetIssuer() (string, error) | ||
GetSubject() (string, error) | ||
GetAudience() (ClaimStrings, error) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered an alternate scenario where we remove this (and map claims) entirely?
It might be a bad idea, but just throwing this thought out .. the user still has to assert the claims.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a lot of people still use map claims, so I am not 100 % convinced tbh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ye that's fair. My thought was .. there are too many ways to pass "claims"
If we remove
Claims
interface entirely, and take anany
.. would that simply the overall API?Probably a bad idea, figured I'd bring it up but I think you're probably right .. a lot of people use map claims.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it actually gets easier with the new interface, because you do not need to implement any validation on your claim, but rather you can use any kind of struct/map type that supplies the needed standard claim values, such as exp, iat, etc. I think that is a pretty straight forward way of modelling claims now.