ToastShaman / dropwizard-auth-jwt

A Dropwizard authentication filter using JSON Web Token (JWT)
Apache License 2.0
116 stars 50 forks source link

Not compatible with CachingAuthenticator #16

Closed alexitooi closed 8 years ago

alexitooi commented 8 years ago

Hi, I've encountered an issue with dropwizard's 9.1 CachingAuthenticator: Using JsonWebToken as Credential Key for the cache is not possible because the hashing function used internally never produces the same result for equal tokens. This results in cache misses all time. I guess by overriding hashcode and equals method in JsonWebToken this problem can be solved, but was not able to test it so far.
Anyone else encountered that issue and has got a workaround?

--- EDIT --

added PR #17

..and thank you for this awesome library!

ToastShaman commented 8 years ago

Hi,

Thanks for raising this issue. I've closed your pull request because there weren't any unit tests. Anyhow, I've added equals() and hashCode() methods to the models (see 68b3299fb0e3f953691127aea97f1c8ee04f263e). There's also a unit test using the CachingAuthenticator. I've published this snapshot release to Sonatype.

Let me know whether this works for you and I'll publish a new release.

Thanks :)

alexitooi commented 8 years ago

Hi, My bad for not adding unit tests, was in a hurry..unfortunately your fix for equals and hashcode is not working! my understanding is that rawToken already represents all information that is saved in a jwt. And the concatenated string should be resolved in equals and hashcode (which was working in my PR).

ToastShaman commented 8 years ago

Hi,

Sorry to hear that. Can you provide me with a failing unit test or some example code showing me how you are using the CachingAuthenticator with the JWT tokens?

Have you had a look at this test?

alexitooi commented 8 years ago

This is how i use the CachingAuthenticator

  final JsonWebTokenParser tokenParser = new DefaultJsonWebTokenParser();
        final HmacSHA512Verifier tokenVerifier = new HmacSHA512Verifier(configuration.getAuth().getTokenSecret().getBytes("UTF-8"));
        final CachingAuthenticator<JsonWebToken, Users> cachingAuthenticator = new CachingAuthenticator<>(
                environment.metrics(),
                new MyAuthenticator(hibernateBundle.getSessionFactory(), usersDAO),
                configuration.getAuth().getAuthenticationCachePolicy());
        environment.jersey().register(new AuthDynamicFeature(
                new JWTAuthFilter.Builder<Users>()
                        .setTokenParser(tokenParser)
                        .setTokenVerifier(tokenVerifier)
                        .setRealm("GSA")
                        .setPrefix("JWT")
                        .setAuthenticator(cachingAuthenticator)
                        .setAuthorizer(new MyAuthorizer())
                        .buildAuthFilter()));

My cache policy:

authenticationCachePolicy: maximumSize=10000, expireAfterAccess=8h

This is the code in CachingAuthenticator that should return the principal an not null:

@Override
    public Optional<P> authenticate(C credentials) throws AuthenticationException {
                ....
            Optional<P> optionalPrincipal = cache.getIfPresent(credentials);
                  ...
}

I'll try to fail your test once I have some free minutes

---- EDIT @ToastShaman: I have added a failing test (after parsing&verifying equals fails ;-) https://github.com/alexitooi/dropwizard-auth-jwt/commit/5b2c8eca7356de5d927af7d993fa3a99c29821bc

ToastShaman commented 8 years ago

Hi,

Thanks for the failing test :) Your provided test is passing now and I've also just published another snapshot release. Would you mind giving it another try and see whether it works?

alexitooi commented 8 years ago

yess works! thank you...I'd recommend using the HashCodeBuilder with random chosen prime numbers, e.g.

return new HashCodeBuilder(11, 71)
+            .append(header)
+            .append(claim)
+            .append(signature)
+            .append(rawToken)
+            .hashCode();

Will you release a new version? =)

ToastShaman commented 8 years ago

Thank you and just pushed the new version. You should be able to get it with maven:

<dependency>
    <groupId>com.github.toastshaman</groupId>
    <artifactId>dropwizard-auth-jwt</artifactId>
    <version>0.9.1-1</version>
</dependency>