Skip to content

Commit

Permalink
Fix ParseInsecure to parse the token even when a key is given (#1008)
Browse files Browse the repository at this point in the history
* Fix ParseInsecure to parse the token even when a key is given

* appease linter
  • Loading branch information
lestrrat authored Oct 30, 2023
1 parent 2ee2c13 commit 16acb8b
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 0 deletions.
4 changes: 4 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions jwt/jwt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)`)
Expand Down
30 changes: 30 additions & 0 deletions jwt/jwt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
}

0 comments on commit 16acb8b

Please sign in to comment.