Initial JWT builder code and tests

[sberyozkin]

Fixes #137

Example1:

Jwt
  .claims()
      .issuer("https://server.com")
   .sign();

Example2:

Jwt
   .claims()
       .issuer("https://server.com")
       .claim("customClaim", 3)
    .headers()
        .keyId("key-id")
    .sign();

Example3:

Jwt
   .claims()
       .claim("custom-array1", Arrays.asList("1", "2"))
       .claim("custom-array2", Json.createArrayBuilder().add("1").add("2").build())
       .claim("mapClaim", Collections.singletonMap("key", "value"))
       .claim("mapClaim2", Json.createObjectBuilder().add("jsonKey", "jsonValue").build())
    .sign();

Example4:

Jwt.claims("/testJsonToken.json").sign();

There are also many utility functions in JwtSigningUtils - where an existing JSON web token can be signed, etc

[kenfinnigan]

there's actually a newer 1.5.0 out, but the groupId changed to io.smallrye.config

[kenfinnigan]

It looks like several of these classes are what I would call "api" classes and others that are "spi". I'd recommend putting them in packages that align with their use, as it will make it easier to know what is a possible MP spec candidate addition down the road.

So maybe io.smallrye.jwt.api?

[sberyozkin]

@kenfinnigan yes I was structuring similarly to how for ex JSONB packages are structured.

I put the API classes to io.smallrye.jwt.build, with the build being the API 'qualifier' because the API there is about building the token (could've been named builder but I'm a bit less keen on it, likewise, I was thinking about io.smallrye.jwt.token, but it seems like a duplication because jwt is short name for the Json Web Token)

While the spi is in build.spi.

(For JSONB there is javax.json and javax.json.spi, or javax.json.bind and javax.json.bind.spi).

So eventually we can have something like org.eclipse.microprofile.jwt.build, org.eclipse.microprofile.jwt.build.spi, and then smallrye-jwt will only end up having io.smallrye.jwt.build.impl

api sub-package looks too general.

What do you think ?

[kenfinnigan]

I just wasn't sure whether build would imply api, whereas everyone knows that an api package means the classes are part of an API for the module.

Not sure if others have views.

[sberyozkin]

@kenfinnigan sure, I've just not seen api as a typical package name in other provider modules :-). For example, everything in MP JWT etc is an API. Yeah, I see what you mean, you mean when we look at the smallrye-jwt itself we don't know immediately that io.smallrye.jwt.build is an API but it is a temporary situation.
If we have an api then I'd have problems adding another independent API I'm thinking about, to represent JsonWebKeySet and JsonWebKey, which I would like to put in io.smallrye.jwt.jwk or may be better to io.smallrye.jwk

I'll make sure the docs/README give a clear indication that the 'build' package is an API.

And I would like Quarkus users give it a try for a couple of iterations (Quarkus docs will say it is an experimental API) before doing a submission.

[kenfinnigan]

Put another way, if we're not wanting to have it be added to MP spec in the future why is it even an API?

Maybe io.smallrye.jwt.api.build is an option?

[sberyozkin]

@kenfinnigan But we won't add in the smallrye package namespace.
As I said in the MP JWT it would become org.eclipse.microprofile.jwt.build,org.eclipse.microprofile.jwt.build.spi.How would having the api in the package name simplify the possible submission ?

If we do it then we'll be left with io.smallrye.jwt.api.build.impl which implements org.eclipse.microprofile.jwt.build,org.eclipse.microprofile.jwt.build.spi. So the only place where api would remain will be in smallrye-jwt but there would no actual API/SPI left.
And we don't have io.smallrye.api.jwt which implements org.eclipse.microprofile.jwt API like JsonWebToken which would be inconsistent IMHO.

These are just interfaces at this stage in the build package, and this is not really a spec related issue in general, whether to have something as an interface (aka API) or not :-). Ex, Darran's PR is bringing a JwtParser interface and it won't be in the api subpackage.

[kenfinnigan]

It was mostly intended as a way to identify an API in amongst the implementation, but if that's not a concern then that's fine

[darranl]

Just adding a quick +1 to clearly defined API/SPI - where a project is split across multiple packages public visibility becomes required to allow use across a project but from this it is not always clear what may be public API used elsewhere with any associated compatibility considerations.

Having utilities for token generation in SmallRye and hopefully later within the specification defined APIs will be great - presently to write relatively simple getting started guides requires dependencies on other third party projects.

[darranl]

Do new classes to SmallRye projects require a copyright header or do these rely on a root LICENCE file now?

[sberyozkin]

@darranl on the root one AFAIK

[kenfinnigan]

Yes, root LICENCE file

[darranl]

One other comment, has any consideration been given to immutability of intermediate Builder instances?

Immutability can have a draw back that it may lead to intermediate instances being created and subsequently require garbage collection, however if a process was to use this API to issue many tokens a common builder could be initialised and shared across threads with those threads just adding their specifics without needing to repeat the common calls each time.

[sberyozkin]

@darranl sure, we were just discussing with Ken if adding an api package qualifier was needed but hopefully we've agreed that this in itself would not be needed to facilitate the submission.

Re the immutability. Hmm... My preference at this stage would be to document that the builder instances are not thread safe and do not recommend caching a builder returned from Jwt.claims() for this single instance be reused in scope of all the client requests and simply advise to always start with Jwt.claims() as it is very cheap. Otherwise, as you said, it would indeed require a lot of the GC work as a new totally self-contained builder would have to be created after every setter and it would be expensive.

[sberyozkin]

FYI, JsonObjectBuilder implementation in the glassfish project is not thread safe. It creates a snapshot of the data in build() which in this API would be sign() but I don't see right now any major benefit apart from reusing the same builder in scope of a current request to create more than one token - and I can't think of any good use case

[sberyozkin]

Going ahead with wiring the encryption support since we'll eventually need to support the validation of the JWE encrypted tokens, microprofile/microprofile-jwt-auth#58

[sberyozkin]

I'm going to close it and reopen as a draft as Ive completely rewamped the API