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

isSigned returns true if normal JSON provided or singed with different key #582

Open
SailReal opened this issue Mar 26, 2020 · 1 comment
Milestone

Comments

@SailReal
Copy link

SailReal commented Mar 26, 2020

I've two problems with the boolean isSigned(String jwt) method:

The following function call returns true if I provide a normal JSON (NOT a signed JWT):

Jwts //
	.parserBuilder() //
	.setSigningKey(getPublicKey()) //
	.build() //
	.isSigned(json);

If I change the method calls to the following:

Jwts //
	.parserBuilder() //
	.setSigningKey(getPublicKey()) //
	.build() //
	.parseClaimsJws(json);

a io.jsonwebtoken.MalformedJwtException: JWT strings must contain exactly 2 period characters. Found: 14 is thrown (which is the expected behavior).

As json a valid JSON is provided (NOT a JWT, maybe it can be any string?), e.g.

{
  "version": "foo",
  "url": "bar",
  "release_notes": "baz"
}

If I provide a valid JWT, signed with a different private key, isSigned also returns true.

From the doc:

* Returns {@code true} if the specified JWT compact string represents a signed JWT (aka a 'JWS'), {@code false}
* otherwise.
* <p>
* <p>Note that if you are reasonably sure that the token is signed, it is more efficient to attempt to
* parse the token (and catching exceptions if necessary) instead of calling this method first before parsing.</p>

Do I understand this method in a wrong way? I just want to check if a string is a JWT signed with the corresponding key. In my opinion isSigned should return false in both cases.

At a different code location I use parseClaimsJws, that works great 😍

As version I use the latest 0.11.1

@lhazlewood
Copy link
Contributor

Thanks for the issue!

That method is intended to be used for checking strings reasonably expected to be compact JWT strings - not generic JSON.

Based on the confusion, it is probable that others experience it as well, so it's likely we'll deprecate this method in favor of parsing directly - or perhaps at least change the implementation to delegate to parsing, catch a MalformedJwtException and then return false as a result, which isn't exactly efficient, so I'm not so sure it's a good idea to propagate its usage.

If we do keep it, some thought needs to be put a changed implementation however to ensure that the implementation is feasible, especially given that encrypted JWTs (JWEs) are also usually signed in addition to being encrypted. (i.e. isSigned should return true for these types of JWEs as well).

I think it makes sense to keep this ticket open to represent the work to make that change. Thanks for reporting it!

@lhazlewood lhazlewood added this to the 1.0 milestone Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants