-
Notifications
You must be signed in to change notification settings - Fork 40
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
AccessTokenVerifier can throw undocumented exceptions #86
Comments
Happy to file a pull request for this by the way. |
Hey @Sovietaced! When this happens can you grab the JWT you are trying to validate? That should help narrow down the problem |
I can try to grab the JWT. We typically don't log it but I can change that. |
I was about to raise this an issue. @bdemers I noticed this today when the signing keys are rotated. How will this library know that the signing key is rotated and Use RemoteJwksSigningKeyResolver? Can you please let jus know what is the cache duration and the frequency of requests to remote jwks uri? Also, please confirm if the backend calls from UI can validate id token and access token using this library or should the backend just validate signature? |
There are a few questions in here, so I'll try to answer all of them, but there is a healthy dose of "it depends" First, if possible your backend should handle authentication(e.g. redirect to login), then your frontend simply uses cookies to deal with the user's session. This isn't a one size fits all solution, but when possible, it's usually the best option. If your frontend handles authentication (which sounds like what you are doing), and ends up with an access token, you treat your REST API as an OAuth "Resource Server". Your frontend sets a header: 1.) Remotely (per OAuth spec) NOTE: The fact that Okta's access tokens are JWTs is an implementation detail (a common one, as a lot of other vendors, do this as well). This post is Spring specific, but it talks about this topic in a more general sense as well. I bring this up because when using a JWT, there is always the possibility of stale tokens (from the above post):
That said, back to the answers. |
@bdemers the headline here is still an issue (the method throws undocumented |
@ethanmills any content inside the JWT bod cannot be trusted until the signature is validated. |
Is there a concern about malicious content, or just false information? I think you have to decode the payload to get the issuer before you can fetch the keys anyway, so rejecting early (never accepting) based on an |
@ethanmills What you are suggesting was a common vulnerability in JWT libraries early on. It would be like asking someone who handed you counterfeit money to validate it. The high level is a JWT library MUST be configured with the issuer (more specifically, the key information) before a JWT is parsed. |
Thanks for the reply @bdemers . I appreciate what you're getting at, but think that it doesn't quite cover the whole picture. Consider the case where a resource server trusts tokens issued by two different issuers. The flow then looks like
That's the flow followed by e.g. https://docs.spring.io/spring-security/site/docs/current/api/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerAuthenticationManagerResolver.html |
ℹ️ If you have a question, please post it on the Okta Developer Forum instead. Issues in this repository are reserved for bug reports and feature requests only.
I'm submitting a
Background info
In some rare cases I am seeing an
IllegalArgumentException
being thrown from theAccessTokenVerifier#decode
. According to the documentation it should throw aJwtVerificationException
when parsing or validation errors occur.It appears that the try/catch around the code that parses the JWT does not adequately handle all of the exceptions advertised by the JWT parser: https://github.com/okta/okta-jwt-verifier-java/blob/master/impl/src/main/java/com/okta/jwt/impl/jjwt/TokenVerifierSupport.java#L63-L71
Expected behavior
A
JwtVerificationException
should be thrown.What went wrong?
Steps to reproduce
I'm not sure what the content of the JWT is unfortunately but this happens very rarely.
Library Version
The text was updated successfully, but these errors were encountered: