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

security-jwt should support the claim "groups" from the microprofile jwt #397

Closed
jensschirmer opened this issue Jun 14, 2021 · 4 comments · Fixed by #399
Closed

security-jwt should support the claim "groups" from the microprofile jwt #397

jensschirmer opened this issue Jun 14, 2021 · 4 comments · Fixed by #399
Labels
common Cross cutting stuff enhancement New feature or request security

Comments

@jensschirmer
Copy link

jensschirmer commented Jun 14, 2021

As a developer, I want that the module security-jwt support the claim groups from the microprofile jwt.

In the security-jwt module of devonfw, the JwtAuthenticatorImpl class expects in line 38 that the roles in the JWT claim "roles" are stored as a comma-separated string. Similarly, this is written back into the token in the JwtCreatorImpl class in line 50 .
In the specification of JWT RFC 7519 , there is no attribute in the Registered Claims area that covers the functionality of the roles. For the Public Claims, which are registered with the IANA, there is also no corresponding attribute. In order to eliminate this uncertainty, the Microproflie JWT was created (see https://www.eclipse.org/community/eclipse_newsletter/2017/september/article2.php). There, the claim "groups is defined, which defines the corresponding roles as a string array.
In the security-jwt module, a possibility should be created to read out this claim and fill it again.
This should also help to support Keycloak. By default, Keycloak writes the roles into the realm_access claim and the permissions are stored there as an array string in the map with the key roles. This can be mapped to the "groups" claim in Keycloak using a mapper.

ATTENTION: For backward compatibilty when upgrading from older devon4j versions to version 2021.04.003 you should add the following configuration to your application.properties if you want to preserve the old behaviour:

security.authentication.jwt.claims.access-controls-name=roles
security.authentication.jwt.claims.access-controls-array=false
@jensschirmer jensschirmer added the enhancement New feature or request label Jun 14, 2021
@hohwille hohwille added this to the release:2021.04.003 milestone Jun 15, 2021
@hohwille hohwille added common Cross cutting stuff security labels Jun 15, 2021
@hohwille
Copy link
Member

@jensschirmer thanks for this issue and your detailed description and suggestions for improvement.
I fully agree and will support that we get a PR for this.
IMHO we should do the following:

  • allow to make the name of the claim configurable
  • for backward compatibility the default could stay roles - however, if I corrently assume that this feature of devon4j is not widely used and you are referring to standards that imply groups would be a better default, I am also happy to change the default and document in the ChangeLog so projects can simply add a configuration to roles to keep their app compatible.
  • when reading JWT as JSON we should ideally support both String array as well as String with comma separated list. This should be easy to do.
  • when creating a JWT we should again make this configurable and choose a reasonable default. If we use the JWT Microprofile the default should be groups + String-Array.

hohwille added a commit to hohwille/devon4j that referenced this issue Jun 15, 2021
@hohwille hohwille linked a pull request Jun 15, 2021 that will close this issue
@hohwille
Copy link
Member

@jensschirmer I just created a PR implemeting this. If you could give review feedback this would be very much appreciated.

@jensschirmer
Copy link
Author

@hohwille : It looks good to me.

@hohwille
Copy link
Member

Official release is planned in ~2 weeks.
From tomorrow on you can already test the feature via CI SNAPSHOT:
https://github.com/devonfw/devon4j/blob/master/documentation/guide-testing-snapshots.asciidoc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Cross cutting stuff enhancement New feature or request security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants