From 16acb8bf5ce89020f6bffcfcc5b00b857f3ed55b Mon Sep 17 00:00:00 2001 From: lestrrat <49281+lestrrat@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:56:14 +0900 Subject: [PATCH] Fix ParseInsecure to parse the token even when a key is given (#1008) * Fix ParseInsecure to parse the token even when a key is given * appease linter --- Changes | 4 ++++ jwt/jwt.go | 4 ++++ jwt/jwt_test.go | 30 ++++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/Changes b/Changes index bbd22e973..31428de0b 100644 --- a/Changes +++ b/Changes @@ -42,6 +42,10 @@ v2.0.16 UNRELEASED * [jwk] Added `jwk.IsKeyValidationError` that checks if an error is an error from `key.Validate()`. +[Bug Fixes] + * [jwt] `jwt.ParseInsecure()` was running verification if you provided a key + via `jwt.WithKey()` or `jwt.WithKeySet()` (#1007) + v2.0.15 19 20 Oct 2023 [Bug fixes] * [jws] jws.Sign() now properly check for valid algorithm / key type pair when diff --git a/jwt/jwt.go b/jwt/jwt.go index cd059a263..fa0c20214 100644 --- a/jwt/jwt.go +++ b/jwt/jwt.go @@ -209,6 +209,10 @@ func parseBytes(data []byte, options ...ParseOption) (Token, error) { } } + if !verification { + ctx.skipVerification = true + } + lvo := len(verifyOpts) if lvo == 0 && verification { return nil, fmt.Errorf(`jwt.Parse: no keys for verification are provided (use jwt.WithVerify(false) to explicitly skip)`) diff --git a/jwt/jwt_test.go b/jwt/jwt_test.go index 9b8554e2a..8e94faa34 100644 --- a/jwt/jwt_test.go +++ b/jwt/jwt_test.go @@ -1743,3 +1743,33 @@ func TestGH951(t *testing.T) { require.True(t, jwt.Equal(verified, token), `tokens should be equal`) } + +func TestGH1007(t *testing.T) { + key, err := jwxtest.GenerateRsaJwk() + require.NoError(t, err, `jwxtest.GenerateRsaJwk should succeed`) + + tok, err := jwt.NewBuilder(). + Claim(`claim1`, `value1`). + Claim(`claim2`, `value2`). + Issuer(`github.com/lestrrat-go/jwx`). + Audience([]string{`users`}). + Build() + require.NoError(t, err, `jwt.NewBuilder should succeed`) + + signed, err := jwt.Sign(tok, jwt.WithKey(jwa.RS256, key)) + require.NoError(t, err, `jwt.Sign should succeed`) + + // This was the intended usage (no WithKey). This worked from the beginning + _, err = jwt.ParseInsecure(signed) + require.NoError(t, err, `jwt.ParseInsecure should succeed`) + + // This is the problematic behavior reporded in #1007. + // The fact that we're specifying a wrong key caused Parse() to check for + // verification and yet fail :/ + wrongPubKey, err := jwxtest.GenerateRsaPublicJwk() + require.NoError(t, err, `jwxtest.GenerateRsaPublicJwk should succeed`) + require.NoError(t, err, `jwk.PublicKeyOf should succeed`) + + _, err = jwt.ParseInsecure(signed, jwt.WithKey(jwa.RS256, wrongPubKey)) + require.NoError(t, err, `jwt.ParseInsecure with jwt.WithKey() should succeed`) +}