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

Fix ParseInsecure to parse the token even when a key is given #1008

Merged
merged 2 commits into from
Oct 30, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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`)
}
Loading