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 Encoding/Decoding changes for better compatibility #117

Merged
merged 10 commits into from Nov 6, 2021
126 changes: 117 additions & 9 deletions parser_test.go
Expand Up @@ -16,25 +16,32 @@ import (
var errKeyFuncError error = fmt.Errorf("error loading key")

var (
jwtTestDefaultKey *rsa.PublicKey
jwtTestRSAPrivateKey *rsa.PrivateKey
jwtTestEC256PublicKey crypto.PublicKey
jwtTestEC256PrivateKey crypto.PrivateKey
defaultKeyFunc jwt.Keyfunc = func(t *jwt.Token) (interface{}, error) { return jwtTestDefaultKey, nil }
ecdsaKeyFunc jwt.Keyfunc = func(t *jwt.Token) (interface{}, error) { return jwtTestEC256PublicKey, nil }
emptyKeyFunc jwt.Keyfunc = func(t *jwt.Token) (interface{}, error) { return nil, nil }
errorKeyFunc jwt.Keyfunc = func(t *jwt.Token) (interface{}, error) { return nil, errKeyFuncError }
nilKeyFunc jwt.Keyfunc = nil
jwtTestDefaultKey *rsa.PublicKey
jwtTestRSAPrivateKey *rsa.PrivateKey
jwtTestEC256PublicKey crypto.PublicKey
jwtTestEC256PrivateKey crypto.PrivateKey
exampleCognitoPublicKey crypto.PublicKey
defaultKeyFunc jwt.Keyfunc = func(t *jwt.Token) (interface{}, error) { return jwtTestDefaultKey, nil }
ecdsaKeyFunc jwt.Keyfunc = func(t *jwt.Token) (interface{}, error) { return jwtTestEC256PublicKey, nil }
exampleCognitoKeyFunc jwt.Keyfunc = func(t *jwt.Token) (interface{}, error) { return exampleCognitoPublicKey, nil }
emptyKeyFunc jwt.Keyfunc = func(t *jwt.Token) (interface{}, error) { return nil, nil }
errorKeyFunc jwt.Keyfunc = func(t *jwt.Token) (interface{}, error) { return nil, errKeyFuncError }
nilKeyFunc jwt.Keyfunc = nil
)

func init() {
// Load public keys
jwtTestDefaultKey = test.LoadRSAPublicKeyFromDisk("test/sample_key.pub")
jwtTestEC256PublicKey = test.LoadECPublicKeyFromDisk("test/ec256-public.pem")

// Load cognito public key - note there is only a public key for this key pair and should only be used for the
mfridman marked this conversation as resolved.
Show resolved Hide resolved
// two test cases below.
exampleCognitoPublicKey = test.LoadECPublicKeyFromDisk("test/exampleCognito-public.pem")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest avoiding a product name here and just name it paddedKey or something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


// Load private keys
jwtTestRSAPrivateKey = test.LoadRSAPrivateKeyFromDisk("test/sample_key")
jwtTestEC256PrivateKey = test.LoadECPrivateKeyFromDisk("test/ec256-private.pem")

}

var jwtTestData = []struct {
Expand Down Expand Up @@ -435,6 +442,107 @@ func TestParser_ParseUnverified(t *testing.T) {
}
}

var setPaddingTestData = []struct {
name string
tokenString string
Copy link
Member

Choose a reason for hiding this comment

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

Do you think we can toss in the example JWT token from this comment?

#92 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just added! Note that I had to add the public key to the repo, added a note where we reference it.

claims jwt.Claims
paddedDecode bool
signingMethod jwt.SigningMethod
keyfunc jwt.Keyfunc
valid bool
}{
{
name: "Validated non-padded token with padding disabled",
tokenString: "",
claims: jwt.MapClaims{"foo": "paddedbar"},
paddedDecode: false,
signingMethod: jwt.SigningMethodRS256,
keyfunc: defaultKeyFunc,
valid: true,
},
{
name: "Validated non-padded token with padding enabled",
tokenString: "",
claims: jwt.MapClaims{"foo": "paddedbar"},
paddedDecode: true,
signingMethod: jwt.SigningMethodRS256,
keyfunc: defaultKeyFunc,
valid: true,
},
{
name: "Error for padded token with padding disabled",
tokenString: "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJwYWRkZWRiYXIifQ==.20kGGJaYekGTRFf8b0TwhuETcR8lv5z2363X5jf7G1yTWVTwOmte5Ii8L8_OQbYwPoiVHmZY6iJPbt_DhCN42AeFY74BcsUhR-BVrYUVhKK0RppuzEcSlILDNeQsJDLEL035CPm1VO6Jrgk7enQPIctVxUesRgswP71OpGvJxy3j1k_J8p0WzZvRZTe1D_2Misa0UDGwnEIHhmr97fIpMSZjFxlcygQw8QN34IHLHIXMaTY1eiCf4CCr6rOS9wUeu7P3CPkmFq9XhxBT_LLCmIMhHnxP5x27FUJE_JZlfek0MmARcrhpsZS2sFhHAiWrjxjOE27jkDtv1nEwn65wMw==",
claims: jwt.MapClaims{"foo": "paddedbar"},
paddedDecode: false,
signingMethod: jwt.SigningMethodRS256,
keyfunc: defaultKeyFunc,
valid: false,
},
{
name: "Validated padded token with padding enabled",
tokenString: "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJwYWRkZWRiYXIifQ==.20kGGJaYekGTRFf8b0TwhuETcR8lv5z2363X5jf7G1yTWVTwOmte5Ii8L8_OQbYwPoiVHmZY6iJPbt_DhCN42AeFY74BcsUhR-BVrYUVhKK0RppuzEcSlILDNeQsJDLEL035CPm1VO6Jrgk7enQPIctVxUesRgswP71OpGvJxy3j1k_J8p0WzZvRZTe1D_2Misa0UDGwnEIHhmr97fIpMSZjFxlcygQw8QN34IHLHIXMaTY1eiCf4CCr6rOS9wUeu7P3CPkmFq9XhxBT_LLCmIMhHnxP5x27FUJE_JZlfek0MmARcrhpsZS2sFhHAiWrjxjOE27jkDtv1nEwn65wMw==",
claims: jwt.MapClaims{"foo": "paddedbar"},
paddedDecode: true,
signingMethod: jwt.SigningMethodRS256,
keyfunc: defaultKeyFunc,
valid: true,
},
{
name: "Error for padded cognito token with padding disabled",
Copy link
Collaborator

Choose a reason for hiding this comment

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

See above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

tokenString: "eyJ0eXAiOiJKV1QiLCJraWQiOiIxMjM0NTY3OC1hYmNkLTEyMzQtYWJjZC0xMjM0NTY3OGFiY2QiLCJhbGciOiJFUzI1NiIsImlzcyI6Imh0dHBzOi8vY29nbml0by1pZHAuZXUtd2VzdC0yLmFtYXpvbmF3cy5jb20vIiwiY2xpZW50IjoiN0xUY29QWnJWNDR6ZVg2WUs5VktBcHZPM3EiLCJzaWduZXIiOiJhcm46YXdzOmVsYXN0aWNsb2FkYmFsYW5jaW5nIiwiZXhwIjoxNjI5NDcwMTAxfQ==.eyJzdWIiOiIxMjM0NTY3OC1hYmNkLTEyMzQtYWJjZC0xMjM0NTY3OGFiY2QiLCJlbWFpbF92ZXJpZmllZCI6InRydWUiLCJlbWFpbCI6InVzZXJAZXhhbXBsZS5jb20iLCJ1c2VybmFtZSI6IjEyMzQ1Njc4LWFiY2QtMTIzNC1hYmNkLTEyMzQ1Njc4YWJjZCIsImV4cCI6MTYyOTQ3MDEwMSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5ldS13ZXN0LTIuYW1hem9uYXdzLmNvbS8ifQ==.sx0muJ754glJvwWgkHaPrOI3L1gaPjRLLUvOQRk0WitnqC5Dtt1knorcbOzlEcH9zwPM2jYYIAYQz_qEyM3grw==",
claims: nil,
paddedDecode: false,
signingMethod: jwt.SigningMethodES256,
keyfunc: exampleCognitoKeyFunc,
valid: false,
},
{
name: "Validated padded cognito token with padding enabled",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also here avoid the product name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

tokenString: "eyJ0eXAiOiJKV1QiLCJraWQiOiIxMjM0NTY3OC1hYmNkLTEyMzQtYWJjZC0xMjM0NTY3OGFiY2QiLCJhbGciOiJFUzI1NiIsImlzcyI6Imh0dHBzOi8vY29nbml0by1pZHAuZXUtd2VzdC0yLmFtYXpvbmF3cy5jb20vIiwiY2xpZW50IjoiN0xUY29QWnJWNDR6ZVg2WUs5VktBcHZPM3EiLCJzaWduZXIiOiJhcm46YXdzOmVsYXN0aWNsb2FkYmFsYW5jaW5nIiwiZXhwIjoxNjI5NDcwMTAxfQ==.eyJzdWIiOiIxMjM0NTY3OC1hYmNkLTEyMzQtYWJjZC0xMjM0NTY3OGFiY2QiLCJlbWFpbF92ZXJpZmllZCI6InRydWUiLCJlbWFpbCI6InVzZXJAZXhhbXBsZS5jb20iLCJ1c2VybmFtZSI6IjEyMzQ1Njc4LWFiY2QtMTIzNC1hYmNkLTEyMzQ1Njc4YWJjZCIsImV4cCI6MTYyOTQ3MDEwMSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5ldS13ZXN0LTIuYW1hem9uYXdzLmNvbS8ifQ==.sx0muJ754glJvwWgkHaPrOI3L1gaPjRLLUvOQRk0WitnqC5Dtt1knorcbOzlEcH9zwPM2jYYIAYQz_qEyM3grw==",
claims: nil,
paddedDecode: true,
signingMethod: jwt.SigningMethodES256,
keyfunc: exampleCognitoKeyFunc,
valid: true,
},
}

// Extension of Parsing, this is to test out functionality specific to switching codecs with padding.
func TestSetPadding(t *testing.T) {
for _, data := range setPaddingTestData {
t.Run(data.name, func(t *testing.T) {

// If the token string is blank, use helper function to generate string
jwt.DecodePaddingAllowed = data.paddedDecode

if data.tokenString == "" {
data.tokenString = signToken(data.claims, data.signingMethod)

}

// Parse the token
var token *jwt.Token
var err error
parser := new(jwt.Parser)
parser.SkipClaimsValidation = true

// Figure out correct claims type
token, err = parser.ParseWithClaims(data.tokenString, jwt.MapClaims{}, data.keyfunc)

if (err == nil) != data.valid || token.Valid != data.valid {
t.Errorf("[%v] Error Parsing Token with decoding padding set to %v: %v",
data.name,
data.paddedDecode,
err,
)
}

})
jwt.DecodePaddingAllowed = false

}
}

func BenchmarkParseUnverified(b *testing.B) {

// Iterate over test data set and run tests
Expand Down
4 changes: 4 additions & 0 deletions test/exampleCognito-public.pem
@@ -0,0 +1,4 @@
-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEIcaUjXhC7Mn2OonyfHF+zjblKkns
4GLbILnHrZr+aQwddiff5urCDAZ177t81Mn39CDs3uhlNDxfRIRheGnK/Q==
-----END PUBLIC KEY-----
15 changes: 15 additions & 0 deletions token.go
Expand Up @@ -7,6 +7,14 @@ import (
"time"
)


// DecodePaddingAllowed will switch the codec used for encoding/decoding JWTs respectively. Note that the JWS RFC7515
Copy link
Collaborator

@oxisto oxisto Nov 4, 2021

Choose a reason for hiding this comment

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

It is only used for decoding, not encoding

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

// states that the tokens will utilize a Base64url encoding with no padding. Unfortunately, some implementations
// of JWT are producing non-standard tokens, and thus require support for decoding. Note that this is a global
// variable, and updating it will change the behavior on a package level, and is also NOT go-routine safe.
// To use the non-recommended decoding, set this boolean to `true` prior to using this package.
var DecodePaddingAllowed bool

// TimeFunc provides the current time when parsing token to validate "exp" claim (expiration time).
// You can override it to use another time value. This is useful for testing or if your
// server uses a different time zone than your tokens.
Expand Down Expand Up @@ -112,5 +120,12 @@ func EncodeSegment(seg []byte) string {
// Deprecated: In a future release, we will demote this function to a non-exported function, since it
// should only be used internally
func DecodeSegment(seg string) ([]byte, error) {
if DecodePaddingAllowed {
if l := len(seg) % 4; l > 0 {
seg += strings.Repeat("=", 4-l)
}
return base64.URLEncoding.DecodeString(seg)
}

return base64.RawURLEncoding.DecodeString(seg)
}