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

undesired behavior for withClaimPresence / alternatively inadequate documentation #558

Closed
Redeeman opened this issue Mar 21, 2022 · 6 comments
Labels
waiting for customer This issue is waiting for a response from the issue or PR author

Comments

@Redeeman
Copy link

(i do not agree to the code of conduct)

Describe the problem

Implementing JWT in an application, I decided that I did not want to accept tokens without expiry, so I activated .withClaimPresence( "exp" ), to ensure that any token without expiration would not be accepted. This appears to have disabled verification of the expiry.

All the docs say for this method is "Require a claim to be present, with any value.", which to be fair, is what is happening, but since there is no other explicit configuration for expiry beyond the leeway settings, I had assumed the automatic verification of expiry would continue.

I would at the very least recommend putting a stern warning in the documentation for "withClaimPresence()" about this behavior. It might however be better to simply change the behavior to always process expiry if the claim exists. Functionality already exists to functionally disable validation if wanted, by for example setting the leeway extremely high (though I do not see why someone might want to do that).

Alternatively perhaps explicit control to enable/disable verification of expiry.

Let me know what you think, example:

Algorithm algorithm = Algorithm.HMAC256( REDACTED );
JWTVerifier verifier = JWT.require( algorithm )
.withClaimPresence( "sub" )
.withClaimPresence( "iss" )
.withClaimPresence( "exp" ) <-- this must not be activated, or it will NOT validate expiry
.acceptLeeway( 5 )
.withClaim( "pmtyp", "user" )
.build();

DecodedJWT jwt = verifier.verify( token );

What was the expected behavior?

that it validate the expiry even though .withClaimPresence( "exp" ) was called

Reproduction

run snippet

@poovamraj
Copy link
Contributor

Hi @Redeeman, we tried to reproduce the issue using the following code and it doesn't seem to reproduce for us. Every time the expiry is validated even if presence is checked for it. Can you provide a full sample where the issue reproduces?

Algorithm algorithm = Algorithm.HMAC256( "REDACTED" );
            JWTVerifier verifier = JWT.require( algorithm )
                    .withClaimPresence( "sub" )
                    .withClaimPresence( "exp" )
                    .acceptLeeway( 5 )
                    .build();

            verifier.verify("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjEzMTYyMzkwMjJ9.Y1N9T3igBDFgiJ1gNaD6XpUeSKswD_VFyVaKAEIyg68");

@poovamraj poovamraj added the waiting for customer This issue is waiting for a response from the issue or PR author label Mar 22, 2022
@poovamraj
Copy link
Contributor

Hi @Redeeman 👋 we noticed that you have mentioned that you don't agree with the code of conduct. We were curious about which part of it that you don't agree with.

@hniemchenko
Copy link

I ran into the same issue as described here. @poovamraj, your exact snippet for me does not throw an exception (it should, as the token is expired). If .withClaimPresence( "exp" ) is removed, the exception is thrown:
com.auth0.jwt.exceptions.TokenExpiredException: The Token has expired on Sat Sep 17 08:57:02 EEST 2011.

@poovamraj
Copy link
Contributor

poovamraj commented Mar 29, 2022

Hi @hniemchenko, Thanks for that and we are able to reproduce this issue 👍 We are internally tracking this in SDK-3231. We will provide you with an update on how we are planning to handle this.

@Redeeman
Copy link
Author

Redeeman commented Mar 29, 2022

@poovamraj im glad you were able to reproduce. I guess the "waiting for customer" label can be removed then.

Regarding your previous question, what I do not agree about with the contributor covenant. It is not most of what is in it I disagree with, it is the process of its creation, and its wording. To provide one example, the contributor covenant says:

we as contributors and maintainers pledge to making participation in our project and our community a harassment-free experience for everyone, regardless of age, body size, disability, ethnicity, sex characteristics, gender identity and expression, level of experience, education, socio-economic status, nationality, personal appearance, race, religion, or sexual identity and orientation.

I think that this is an extremely poor wording, in fact, one might argue that things not listed on this would then perhaps be seen as okay, or atleast less bad. I think that it is mindless virtue signalling with this explicit word soup of special things that cannot be harassed about. How about simply writing:

In the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to making participation in our project and our community a harassment-free experience for everyone.

and stopping there? the rest is just pure virtue signalling, and it doesn't even say "including but not limited to", so this non exhaustive list just makes it worse for everyone.

And this is also the example of what i meant with "process" of the contributor covenant. I would argue that this document is written not in good faith, but I do not think it is appropriate in this forum to write a longer explanation.

@poovamraj
Copy link
Contributor

Hello 👋

We have fixed this behaviour in our latest v4.0.0-beta.0 release. We will close this issue now. Please try the new release and provide your feedback. Please note that there are changes in the library's behaviour in the new major and check out our Migration Guide to migrate your library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for customer This issue is waiting for a response from the issue or PR author
Projects
None yet
Development

No branches or pull requests

3 participants