Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use SmallRye Converters for JWT Claims conversions #211

Open
radcortez opened this issue Mar 30, 2020 · 3 comments
Open

Use SmallRye Converters for JWT Claims conversions #211

radcortez opened this issue Mar 30, 2020 · 3 comments

Comments

@radcortez
Copy link
Member

Conversion code from SmallRye Config was extracted to https://github.com/smallrye/smallrye-converters.

The plan is to implement a prototype to replace conversion code in SmallRye JWT and use the Smallrye Converters project.

This should help to address:
eclipse/microprofile-jwt-auth#100

radcortez added a commit to radcortez/smallrye-jwt that referenced this issue Mar 30, 2020
radcortez added a commit to radcortez/smallrye-jwt that referenced this issue Mar 30, 2020
radcortez added a commit to radcortez/smallrye-jwt that referenced this issue Apr 8, 2020
radcortez added a commit to radcortez/smallrye-jwt that referenced this issue Apr 8, 2020
radcortez added a commit to radcortez/smallrye-jwt that referenced this issue Apr 8, 2020
@sberyozkin
Copy link
Contributor

We have discussed it further with Roberto, and indeed the use of smallrye-converters looks very promising (see also the #212 WIP PR from Roberto).

The ultimate goal is to update all of smallrye-jwt conversion logic to use the various provided converters to convert claims to the custom injection target classes, for example, String to Long or String to Long etc.
Additionally, we already have all the non-standard complex claims represented as JsonObject.

The proposal is to do this migration to smallrye-converters in 2 phases.
Phase 1 - support the conversion of complex JsonObject claims only

  1. Add smallrye-converters dependency and check Converter<JsonObject, T> when processing returning JsonObject claim values (I guess we'll need to pass the converters injected into JWTContextInfoProvider as a JWTContextInfo property)
  2. add implementation/jsonb-claim-converter to support a JSONB based JsonObject to T converter
    (I can handle this step)

Users will chose either a provided JSONB converter or register a custom converter.

It will all have a very minimal impact. And in phase 2 we'll complete the whole migration.

CC @radcortez @darranl

@sberyozkin
Copy link
Contributor

Of course we can try to do it in one go - but I guess it can be a less sensitive refactoring if we do in 2 iterations...

@sberyozkin sberyozkin added this to the 3.3.0 milestone May 21, 2021
@sberyozkin
Copy link
Contributor

I propose to tackle it for 3.3.0 - soon to be released 3.2.0 already has the MP-JWT 1.2.1 dependency update with a couple of minor jwt build issues to be fixed which is enough for 3.2.0

@sberyozkin sberyozkin modified the milestones: 3.2.1, 3.3.0 Jul 15, 2021
@sberyozkin sberyozkin removed this from the 3.3.0 milestone Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants