Long or JsonValue

[ivanjunckes]

Hey guys I have an implementation question. What should be returned when we do:

jwt.getClaim("customClaimLong");

Should it be a Long or a JsonValue? It makes more sense to me to be a Long as we are in Java and all the other claims for the Claims enum return the long wrapper.

[starksm64]

I think how we define the behavior is that non-standard claims should be the underlying JsonValue type as these are just direct mappings from the payload.

[arjantijms]

@starksm64 perhaps an idea, but for a next version of JWT it might make sense to remove all the conversion and mapping requirements from the JWT spec, and delegate that all to a potential MP Conversion Spec?

[tomas-langer]

The jwt.getClaim("") method is not properly typed - there is no way how to find out which type is expected by the consumer.
If you do:

String customClaim = jwt.getClaim("customClaim");
Integer customClaimInt = jwt.getClaim("customClaim");
Long customClaimLong = jwt.getClaim("customClaim");
JsonString customClaimJson = jwt.getClaim("customClaim");

You will compile successfully, yet fail horribly in runtime, as the "getClaim" method has no chance of knowing what to return.
If this should work in a reasonable way, there would have to be a method (similar to Config):
<T> T getClaim(String claimName, Class<T> expectedType);
Then there could be appropriate convertors to map json types to java types. This would still be quite complex implementation, as there are a lot of options...

[arjantijms]

This would still be quite complex implementation, as there are a lot of options...

It's been a few months later now when I last proposed a convertor spec. Wonder how people think about it now.

[radcortez]

@arjantijms I think a converter spec makes sense. Did you propose this to a larger group, or just here? There is also a chat in MP Config about a converter spec: eclipse/microprofile-config#514

[arjantijms]

@radcortez Didn't propose it so much to a larger group, though I did had a blog about it:

https://arjan-tijms.omnifaces.org/2014/08/high-time-to-standardize-java-ee.html

[radcortez]

I think this should be proposed in the new groups (either MP or Jakarta). I guess your blog is from a time, when things were a little "dormant" so to say. Should have more acceptance now :)

[arjantijms]

@radcortez Indeed, the either MP or Jakarta is however a new problem ;)

Since both groups are separate pending discussions, it does make quite a difference. If I'm not mistaken Jakarta can't use/depend on MP now, but MP can't (easily) add new Jakarta stuff since that would mean additional dependencies,

[radcortez]

Well, right now it would be easier for MP to include stuff from Jakarta than the other way around. There are a few things being discussed in the MP group to allow Jakarta to use MP stuff. For instance, a lot of specs are interested and could benefit from MP Config.

I don't want in anyway way to "highjack" your work. Are you willing to try to propose this again to either group, and we can try to figure out where it should fall? I'm happy to help.

[sberyozkin]

Hi All, adding David's @dblevins requirement from #114, IMHO we'd be better off keeping it all within a single issue to do with type safe retrieval of the claims.
David's requirement:

Our ability to convert Claims from strings to specific data types should be expanded to reach the same level of functionality found in JAX-RS or Config API. Specifically, java types that have a public constructor that accept a string should be supported, as should java types that have a static factory method that accepts string as an argument and returns an instance of that type.

[sberyozkin]

And here is a related question/requirement from @ivanjunckes:

Shouldn't we also support Enum classes? Strings could be transformed to a Enum using valueOf if the injection point is an Enum class.

[radcortez]

There is a discussion in MP Config about exposing the available Converter facilities. Since MP JWT depends directly from Config already, maybe we can leverage some of this work together.

[sberyozkin]

@radcortez Sounds reasonable to have the typed claims retrieval and injection built around them.
Would those converters help with the complex claims (which are JSON objects), example, would it work if we write:

Address address = jwt.getClaim(Address.class);

given this Address claim ?

I.e would the converter accept JSON objects so that the custom converter could bind it to an Address java bean ?

[radcortez]

If we use the current Converter API present in MP Config, the source object is always a String. So on your converter you could just use Jsonb and pick up the string and convert it into the object.

[sberyozkin]

@radcortez
Thanks; that won't really work for MP JWT though. The implementation would need to convert some arbitrary internal representation of a given complex claim which has already been parsed from JSON (Jose4J would have a map, other Jose libs that MP JWT implementation use - something else, and MP JWT may already have it converted to JSONB, ex, smallrye-jwt) back to String and the custom converter - parse it yet again.
Accepting String values is OK. But I'd encourage to have the overloads accepting JSONB object and array. May be JSONB does not make sense for MP Config though. In that case may be we can have our own small TypeConverter facility in MP JWT

[sberyozkin]

We should probably not rush in any case :-), and discuss it properly in time for 2.0. IMHO this is the main area of improvement for 2.0

[radcortez]

Yes, the proper way to solve this is to have a separate Converter spec flexible enough that each spec can use.

We are going to try to do some working prototype in the SmallRye project to extract the conversion code to its own library and figure out if we can make that an API on its own.

[sberyozkin]

@radcortez That is a neat idea, but what would happen if an application combines several MP specs, each of them having their own Converter providers. If it works out it would be massive :-), but it may make sense also consider small type-converter facilities which are optimized for a specific spec. IMHO we'll need to get this issue done for the major release, do we have just a few months for it ?

Thanks