Quarkus: smallrye-jwt returns stale claim data from previous JWT

Created on 27 Mar 2019  路  24Comments  路  Source: quarkusio/quarkus

It seems that the quarkus smallrye-jwt integration holds on to values from previous JWT invocations and returns stale claim values instead of the ones from the current JWT.

E.g. accessing currentUsername always returns the value from the first JWT that was used to call the /data/user endpoint.

import org.eclipse.microprofile.jwt.Claim;
import org.eclipse.microprofile.jwt.Claims;

import javax.annotation.security.RolesAllowed;
import javax.inject.Inject;
import javax.json.Json;
import javax.json.JsonString;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.core.MediaType;
import java.util.Optional;

@Path("/data")
public class DataResource {

    private static final JsonString ANOYNMOUS = Json.createValue("anonymous");

    @Inject
    @Claim(standard = Claims.preferred_username)
    Optional<JsonString> currentUsername;

    @GET
    @Path("/user")
    @Produces(MediaType.TEXT_PLAIN)
    @RolesAllowed({"user"})
    public String userData() {
        return "data for user " + currentUsername.orElse(ANOYNMOUS);
    }
}

See discussion on twitter: https://twitter.com/thomasdarimont/status/1110569492520808450
The following example helps to reproduce the issue: https://github.com/thomasdarimont/quarkus-keycloak-demo

aresecurity aresmallrye kinbug pinned

All 24 comments

@starksm64 I believe this one would be for you.

I confirm I can reproduce this.

The issue is this:
https://github.com/quarkusio/quarkus/issues/1371

if you change the scope of the DataResource to @RequestScoped, you get the expected behavior:

@Path("/data")
@RequestScoped
public class DataResource {
...
Scotts-iMacPro:quarkus-keycloak-demo starksm$ ./run_test.sh 
data for user "test"
Access forbidden: role not allowed
data for admin "admin"
data for user "admin"

Oh, that's for the clarification! I didn't try to add @RequestScoped explicitly, because it should be implied by JAX-RS. The ticket mentions that quarkus decided to deviate from the spec where it seems appropriate... Is there a list of those deviations somewhere for reference?

@starksm64 Is this going to stay ? In this case maybe the https://github.com/quarkusio/quarkus-quickstarts/blob/master/using-jwt-rbac quickstart should be updated then ?

@sebastienblanc Yes, good point, we should not be promoting the default scope in the quickstart. I'll update that.

@thomasdarimont Good question, not that I know of so I'll bring it up with the team.

Another issue that was pointed out was the use of @Dependent scope by the producer of the claim values. This was something that the MP-JWT spec defined and I don't remember the reason now. So perhaps we could have a setting to change this behavior in the jwt extension and make this the default.

For now, the issue was documented. @starksm64 will work on a solution for the next release.

@starksm64 IIRC CDI can only inject final beans with @Dependent scope and Glassfish implementation of javax.json.JsonString is final.

@mkouba @sberyozkin if this is still an issue, I think we should throw an exception on build.

@mkouba @sberyozkin @kenfinnigan can you comment on https://github.com/quarkusio/quarkus/issues/1710#issuecomment-553567917 ?

So I have no idea what the problem in this issue is. However, if a producer returns javax.json.JsonString it's ok, because interfaces are proxyable and CDI does not care about the impl class. It's true that in CDI normal scoped beans (@RequestScoped) may not be declared final. In fact, even a @Dependent bean may not be final if it has a bound interceptor. In other words, if the producer returns JsonString and not the impl class it should work fine.

@mkouba Hi Martin, smallrye-jwt has a RawClaimProducer which returns Optional and quarkus-smallrye-jwt has some code to make Optional<JsonString> to work. But all RawClaimProducer does it returns a claim value.
I'm not sure it can really work for JsonString unless the JAX-RS service scope is per-request (in JAX-RS terms).
I'd be tempted to close this issue and recommend use either ClaimValue interface or JsonWebToken interface

Ok, so to sum up the problem:

  1. JAX-RS resources are @Singleton by default in Quarkus
  2. java.util.Optional is final hence unproxyable from CDI POV and that's probably the reason why the relevant smallrye producer is @Dependent
  3. @Dependent means that the bean instance is produced once, when injection is performed
  4. As a result the singleton jax-rs resource holds the stale claim value.

Workarounds and possible solutions

1. JAX-RS resource with @RequestScoped

If you annotate a JAX-RS resource with @RequestScoped a new instance is created per each HTTP request and actual claim is injected.

2. Configuration

Another solution is to disable the default behavior of Quarkus through the quarkus.resteasy.singleton-resources=false [1].

[1] https://quarkus.io/guides/all-config#quarkus-resteasy-server-common_quarkus.resteasy.singleton-resources

3. Programmatic lookup

You can use javax.enterprise.inject.Instance to obtain the claim lazily, when needed, i.e. @Inject @Claim(standard = Claims.preferred_username) Instance<Optional<JsonString>> currentUsername and currentUsername.get().orElse(ANONYMOUS).

4. SmallRye proxy

A little bit more complex solution would be to return some kind of proxy instead of a claim value from the producer. I'm not quite sure how this would work because if I understand it correctly the claim can be any type?

Am I right in assuming that it's ok to use @ApplicationScoped resource classes in conjunction with smallrye-jwt, as long as I don't use @Claim (without Instance)?
Or are there any other known problems, e.g. @Inject JsonWebToken jwt?

Another user just started a Zulip topic about "RBAC with ApplicationScoped resources": https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/RBAC.20with.20ApplicationScoped.20resources

Quoting @sberyozkin:

Injecting JWT into Applicationscoped beans will definitely work, we even have TCK tests now in 1.2.RC1

So I think my previous question is answered. 馃檪

Depends, using @Claim with a proxyble class is fine. The issue is with non-proxyble classes, where of course, the injected value is the one retrieve when the @ApplicationScoped bean was created and stays there forever.

Additional references from the MP JWT Spec:
https://github.com/eclipse/microprofile-jwt-auth/pull/180

I think it would make sense to add some more details to the documentation (guides). I found two slightly contradictory statements there (see Zulip), which may also apply to others. The explanations here make perfectly sense and explain the limitations very well.

My statement was indeed about JWT (>= JsonWebToken), not about some of its individual claims, especially those with the non-proxiable types.
This particular issue is about JsonString, I'm hoping we can close this really soon now :-), but need some help from @mkouba to clarify a few bits :-)

As proposed by @famod we can try to log a warning if a non-proxyable type is used in the AS bean. @mkouba Hey Martin, may be it is already possible in Quarkus ?

Yes, so if there is an issue with injection and proxyable types, we need to fix it. I can try to write some tests in SR JWT side to see where we are with it.

@radcortez @mkouba has confirmed that Optional<JsonString> can not be proxied,
I've had a branch opened awhile back to check it:
https://github.com/sberyozkin/quarkus/commit/5ef334901c806ad0a80de45a020c745711912e21
which is indeed failing. But JsonString should work.

I suppose this is because @Optional cannot be proxied, right?

Yes, java.util.Optional is final and at least in standard CDI this is not proxyable and must not be bound to a normal scope.

Was this page helpful?
0 / 5 - 0 ratings