Introduce JWT Builder API

[sberyozkin]

No description provided.

[rdebusscher]

I did only a brief review of the PR, I did not check the API in detail (but I assume this is already in SmallRye and thus more or less proven to be useable)

Important question is why we define an API to generate any kind of JWT (JWS and JWE with many options) when there are already at least 3 libraries which does the same.

One remark from the past was that developers should not generate their own JWT tokens, this is something the authentication provider (like Keycloak) does. And if you really need such an API, I see only room for a very minimal API which generates JWTs indicated in the spec.

Also missing an extensive set of TCKs to test the implementations of this API.

[rdebusscher]

smallrye

[sberyozkin]

Will follow the resolution as per the above comment

[sberyozkin]

@rdebusscher Hi, thanks for the review so far, I'll deal with you individual comments later, here is feedback on your first general one

I did only a brief review of the PR, I did not check the API in detail (but I assume this is already in SmallRye and thus more or less proven to be useable)

We have Quarkus users trying it yes.

Important question is why we define an API to generate any kind of JWT (JWS and JWE with many options) when there are already at least 3 libraries which does the same.

There is some mix up here. It does not matter how many libraries are around, the goal is to enrich MP JWT with its own portable JWT generation API as proposed by @Emily-Jiang at #109 (as well as by my team colleagues) as opposed to everyone using their own snippets.

Note there is also a task to support MP JWT accepting the JWE-protected tokens. Hence the JWE option.

One remark from the past was that developers should not generate their own JWT tokens, this is something the authentication provider (like Keycloak) does.

I myself have been issuing such remarks but MP JWT has not been designed as an OIDC adapter, i.e the library that must be accepting the tokens as part of OIDC flows only and the reality is right now the developers use it for all sort of JWT token issuance scenarios, including self-issuing, with every developer code using, well, some specific library, which is what this API is about to address.

And if you really need such an API, I see only room for a very minimal API which generates JWTs indicated in the spec.

I disagree with it. It has not played well for MP JWT to require that groups for example must be there or upn as many certified OIDC servers don't set them (so your recommendation that only OIDC providers like Keycloak should issue tokens does not actually work with MP JWT, except for Keycloak :-) ), while it is in the interest of MP JWT to accept the tokens from as many providers as possible.

Likewise MP JWT Generation API should see further than just issuing MP-JWT tokens with the limited support which are only meant for MP JWT servers, this is a close world assumption.
This API is really very simple and will help with making MP JWT more visible to a larger develoiper audience. It is very simple really, sign, encrypt and inner-sign/encrypt, the latter two options will be part of #58 on the JWE verification side as both options are used by Jose libraries (as noted by @chunlongliang-ibm in the meeting notes).

Also missing an extensive set of TCKs to test the implementations of this API.

Lets move step by step, need to finalize this API first

[sberyozkin]

@rdebusscher Replied to all the comments. Thanks for the comprehensive feedback, lets keep going :-)

[rdebusscher]

I know there is an open task for JWE, so that should be decided and implemented in the spec firest before we should be think of creating a JWE.

An open spec doesn't work. Because if there is tomorrow a new signing algorithm, it is assumed to be supported automatically by this API which is not the cause of course.
Within a spec there needs to be a very detailed description of what algorithms, encryption methods and type of keys are valid.

[sberyozkin]

I know there is an open task for JWE, so that should be decided and implemented in the spec firest before we should be think of creating a JWE.

I don't think that it should be decided and implemented first.

An open spec doesn't work. Because if there is tomorrow a new signing algorithm, it is assumed to be supported automatically by this API which is not the cause of course.

You haven't looked carefully enough. The algorithms are typed and only those which are well known in the JWA spec are typed. Furthermore, only a bare minimum is required to be supported. The rest - is optional. The users won't be able to bypass it, see the header related docs...

Within a spec there needs to be a very detailed description of what algorithms, encryption methods and type of keys are valid.

Exactly, this will be done for the minimum required set

[sberyozkin]

@rdebusscher I've trimmed down the list of algorithms significantly; let me resolve some of the converstaions (feel free please to reopen as needed) and I will follow with some comments on the update. Thanks

[sberyozkin]

Hi @rdebusscher,

• I've trimmed down the list of algorithms significantly, but relaxed the requirement that only those in these enums can be accepted; instead a warning is added that neither the portability nor interoperability can be guaranteed with the guidance that supporting any other algorithm is optional. IMHO it is a good compromise as we only offer a typed support for the algorithms which must be supported while we need to keep a window open for the users to to try/experiment with other algorithms. I've simplified the enums too as a result.
• I've left only 2 values in SignatureAlgorithm, with ES256 being the only addition (I can link to our project's issue to confirm there is a practical issue which can affect all MP JWT implementations which need to verify the tokens). I'm going to follow with a small PR syncing MP JWT server requirements to enable ES256 on the server too. It will be useful anyway IMHO to start slowly extending the algorithm coverage. I've dropped HS256/etc as syncing up on the server is problematic at the moment so we'll need a separate dedicate effort for the symmetric keys.
• encryption - only RSA-OAEP (since it has a recommended+ JWA status) key management and AGCM256 content encryption algorithms are required. I can sync easily enough with a follow up PR for the server requirements, it would be really a single new property, with the verify key becoming optional on the JWE path unless the nested token is available.

Thanks

[rdebusscher]

Thanks @sberyozkin,

The restricted algo list and encryption makes sense and we don't try to support all the algos and methods which exist.

[sberyozkin]

@rdebusscher Hi, may be of interest, right now I'll use smallrye-jwt to build a JWT to support a client_secret_jwt OIDC client authentication using HS256. This is one of the good cases where I'd prefer to use MP JWT in time :-), but for now I'll stick to the commitment not to push HS256 as a required algorithm until a server-side sync is available. cheers

[sberyozkin]

To-do list for myself:

• address the licence year issue, and most likely drop a smallrye.jwt reference
• properties should follow a pattern like key.location as opposed to key-location given that the former pattern is already in use in MP-JWT
• drop the requirement that an innerSign() call produces a none signature if mp.jwt.sign.key.location is missing because it can hide a bug where the property was not configured by mistake and none can not meet the sign-then-encrypt expectations (non-repudiation, etc)
• introduction to the builder API in the spec text
• TCK tests
• Update JwtClaimsBuilder to have a claim setter accepting Claims as done by @andreas-eberle for JsonWebToken (Note it would also be handy to bootstrap the builder with JsonWebToken to simplify the propagations where the claims are re-signed/re-encrypted but I think we should not rush with this one, JsonWebToken will need a toJson method)
• Also consider making shorter names for signatureKeyId() or signatureAlgorithm() etc
• Add shortcuts encouraging the use of properties as opposed to loading the keys in the code:
  • Jwt.sign("/Token1.json") is a shortcut for Jwt.claims("/Token1.json").sign()
  • Jwt.encrypt("/Token1.json") is a shortcut for Jwt.claims("/Token1.json").jwe().encrypt()
  • Jwt.signEncrypt("/Token1.json") is a shortcut for Jwt.claims("/Token1.json").innerSign().encrypt()
• Consider making this feature optional for the concerned implementers to opt out

[sberyozkin]

Marking for 2.0 for now. I'll see if I can find the time to complete it earlier, but it is pretty tight so 2.0 may be more appropriate, we'll see

[dblevins]

Catching up on this thread. I agree with @rdebusscher earlier comments that JWT creation has explicitly been identified in the past as out of scope. Here were some of the reasons:

• There are plenty of client libraries that do this
• It reads either as an endorsement that MicroProfile encourages people to write their own authorization servers or that the MicroProfile implementation should be an authorization server
• Doing this properly is actually very very hard due to constant change, you need to be intimately involved in the JOSE specs and latest developments.

If one were created we'd want it to in some way be optional so that we'd be free to not go that direction.

[sberyozkin]

@dblevins David, thanks, but you may have misunderstood the goal of the PR.

Instead of every developer having to hack some non-portable and non-controlled code this PR will give them a chance to write a code which works across multiple libraries, and this code will be TCK verified.

This API has been carefully designed such that it does not require an academic qualification to implement. I have a 1st hand experience in implementing JOSE so I know what I'm proposing, please don't take it as some over-confident remark. And please, don't say it is hard to implement :-), certainly not for the Tomitribe team :-). I can share a link to the branch where I coded it on top of Jose4j in half a day.

I'd like you to trust this API has not been baked in a rush - we have Quarkus users experimenting with it in our project and everyone I've talked to about it is happy,

The other important point which has been discussed earlier is that it is pretty important to open MP JWT both ways for it to appeal to a larger audience - for it not only be a server but become a producer and also show some lead in supporting not only signed but signed and/or encrypted tokens.

CC @rdebusscher

[sberyozkin]

if one were created we'd want it to in some way be optional so that we'd be free to not go that direction.

@dblevins I'll have no problems making it optional for the implementers to support, so if TomiTribe would prefer recommending its uses to code directly against Jose4J or other lower level library then I'll be fine with making the spec allowing for it :-)

[sberyozkin]

@dblevins, I've updated my to do list to consider making it optional, I'll discuss it with other implementers in a couple of months when we'll be getting closer to 2.0, but as I said I won't mind myself.

And... good to talk to you again :-)

[sberyozkin]

@dblevins Sorry, I've spent half a day at implementing the minimum JWE server support, got confused, but this API is pretty straghtforward to implement, certainly not longer than few days or not much longer. It took me awhile to implement it because I was experimenting, but I honestly don't expect anyone spend much time on implementing this API

[sberyozkin]

@rdebusscher Hi Rudy, I think I'll make a summary of our disccussion with you and David and the pending review comments in the linked issue and will open a new fresh PR otherwise it will be difficult to continue a good discussion with so many comments already available. I'll do it for MP JWT 2.0.