jhipster / generator-jhipster-quarkus

Quarkus blueprint for JHipster
https://www.jhipster.tech/blueprints/quarkus/
Apache License 2.0
141 stars 55 forks source link

Keycloak Accesstoken does not contain groups claim - no authorization #167

Closed dminkovski closed 1 year ago

dminkovski commented 3 years ago

Describe the Bug

JDL Setup

application { config { baseName gateway, applicationType gateway, packageName com.test.application, authenticationType oauth2, prodDatabaseType mariadb, databaseType sql, devDatabaseType mariadb, serverPort 8080, clientFramework react, blueprint quarkus, buildTool maven, languages [en, de], nativeLanguage en, serviceDiscoveryType eureka, skipUserManagement true, } }

When you setup the quarkus application and choose to use Keycloak as the main authentication service you can login but you cannot acccess the frontend routes because there is no authorization. This is due to the react app rendering certain routes and components only when the authorities/roles are matching.

private-route.tsx export const hasAnyAuthority = (authorities: string[], hasAnyAuthorities: string[]) => { /* eslint-disable no-console */ if (authorities && authorities.length !== 0) { if (hasAnyAuthorities.length === 0) { return true; } return hasAnyAuthorities.some(auth => authorities.includes(auth)); } return false; };

Based on the spring boot configuration the group roles of the users from Keycloak get prefixed and capitalized and are thus used as ROLE_USER and ROLE_ADMIN. The current quarkus build however, reads the Accesstoken claim "groups" and returns them as authorities in the user object.

AccountResource.java user.authorities = accessToken.getClaim("groups");

The groups claim does not get passed by default from the initial keycloak setup.

To Reproduce

  1. RUN jhipster-quarkus jdl .\jhipster-jdl.jdl (authenticationType oauth2)
  2. RUN jhipster-quarkus docker-compose
  3. UPDATE src/main/resources/application.props: %dev.quarkus.datasource.username=root
  4. Set 127.0.0.1 keycloak in hosts file on local machine
  5. RUN docker-compose -f .\keycloak.yml up
  6. Go to src/main/docker and RUN docker-compose -f .\mariadb.yml up
  7. Go to gateway folder and RUN .\mvnw
  8. Login using admin:admin against keycloak server
  9. See no access to administration or entities, authorization error

Expected behavior I would expect to be able to see the Administration Dropdown, and be able to see the entitiy CRUD table.

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

Solved I wrote up a JSON file that creates the mapping for each user and passes the groups attribute using a keycloak protocol mapper.

keycloak-config.zip

Screenshots JHipster keycloak login loggedIn notauthorized

danielpetisme commented 3 years ago

Hi @dminkovski ! Thank you for reporting the issue.

It looks like you identify a fix. Do you think you could propose a PR? It would be the faster way to solve the problem ;-) I can guide you if you want to.

dminkovski commented 3 years ago

Hi @danielpetisme ! Thank you for your quick reply.

I will try to propose a PR, appreciate any help and guidance! :)

danielpetisme commented 3 years ago

@dminkovski I finally took time to have a look at your issue and I don't like what I've found 😢 .

TL;DR: Keycloak Oauth2 is not working...

As you correctly described, once authenticated, the UI does not reflect the user's role. This is because the JHipster Quarkus backend does not load the roles properly. https://github.com/jhipster/generator-jhipster-quarkus/blob/main/generators/server/templates/quarkus/src/main/java/package/web/rest/AccountResource.java.ejs#L182

We expect the JWT payload to have a groups claim. JHipster Keycloak image is using the roles claim name to store the user's roles, like image

I already had this issue in the past... Jhipster is not using the default claim name and it's very confusing (now I understand why you proposed a custom realm definition).

I tested with Okta as OIDC provider and it work nicely since Okta uses the groups claim. image

To fix the issue, Jhipster introduce a custom role extractor https://github.com/jhipster/generator-jhipster/blob/7edbf65e3c9920689c5c5fe44fc7848696afb83a/generators/server/templates/src/main/java/package/security/SecurityUtils.java.ejs#L164 The code test if the roles claim exists and load the roles otherwise it fallbacks to authentication.getAuthorities() (which should be accessToken.getGroups() in the Quarkus context).

So now the big question is why we did not detect it sooner ... Because the tests were using the groups claim too... https://github.com/jhipster/generator-jhipster-quarkus/blob/main/generators/server/templates/quarkus/src/test/java/package/MockOidcServerTestResource.java.ejs#L64

The implementation would be:

  1. replace accessToken.getClaim("groups"); by a extractAuthorities method which would use the roles claim if defined or fallback to accessToken.getGroups() in order to work with both Okta and keycloak claims.
  2. Update the MockOidcServerTestResource to add a getAccessTokenWithRoleClaim method which would forge a token using the roles claim and not the group one and ensuring the /account API would return the proper value.

@dminkovski would you be interested in implementing this PR? Now that I finally understood the root cause, I can easily help.

mraible commented 3 years ago

I'm not sure if you're using Keycloak 12, but I found an issue today that our realm wasn't setting the groups claim for it. It works w/o this change in Keycloak 10. https://github.com/jhipster/generator-jhipster/pull/14353

dminkovski commented 3 years ago

@danielpetisme yes absolutely! Sorry for the inconvenience but I did not know how else to describe and explain the problem. Thanks for looking into it again. I would gladly implement a PR for this. Will make a new PR then. I am using keycloak 10.

Best regards

danielpetisme commented 3 years ago

@mraible I'll need your help here... Do you know the microprofile-jwt claim? It's defined in the JHipster Keycloak client scopes image And guess what... it's mapped to the groups claim image

So basically @dminkovski just adding the microprofile-jwt should fix the issue

quarkus.oidc.authentication.scopes=profile,address,email,address,phone,offline_access,microprofile-jwt
danielpetisme commented 3 years ago

Generally speaking, these are the scopes used by JHipster Spring

openid,address,email,jhipster,microprofile-jwt,offline_access,phone,profile,roles,web-origins

And these are the ones used by JHipster Quarkus

opened,profile,address,email,address,phone,offline_access,microprofile-jwt

It looks like the Oauth2 Spring client is inferring the scopes to use based on the client configuration where with Quarkus it has to be explicit. We should double-check this behavior because it would mean we would have to customize the Quarkus OAuth2 scopes based on the OIDC provider...(I remember for instance the offline_access was not working with Okta.

mraible commented 3 years ago

I doubt microprofile-jwt is a standard scope provided by OAuth. However, you can easily add it to your authorization server in Okta, just like you can with Keycloak. offline_access is a standard OAuth scope that says you want refresh tokens. I recommend refresh tokens for long-lived sessions.

dminkovski commented 3 years ago

@danielpetisme exactly. That is the solution I proposed. To add the microprofile-jwt in the properties if one selects keycloak. For me it seems like it would make sense to add that scope to the properties if keycloak is selected. What do you propose? Should I keep the PR then where I have implemented that?

danielpetisme commented 3 years ago

I double checked to understand the behavior between Spring and Quarkus. When no claims are defined, Spring, implicitly, uses all the claims defined in the client registration configuration. I discussed this mechanism with the Quakrus team. https://github.com/quarkusio/quarkus/issues/15880

As for now, it's not a feature Quarkus OIDC wants to cover. That means claims as to be explicitly set.

In our context, I don't want to ask explicitly to the end user which IdP they plan to use. The options I can see 1- Generate Keycloak values and document Okta claims into the README.md. 2- Generate Okta claims as a comment

# Okta Claims
# quarkus.oidc.authentication.scopes=profile,address,email,address,phone
# Keycloak Claims
quarkus.oidc.authentication.scopes=profile,address,email,address,phone,microprofile-jwt,jhipster

3- Define an Okta profile

%okta.quarkus.oidc.authentication.scopes=profile,address,email,address,phone
quarkus.oidc.authentication.scopes=profile,address,email,address,phone,microprofile-jwt,jhipster

The user would package the app by defining the wanted profile

mvn -Dquarkus-profile=okta package

4- Uses Okta cli and .okta.env As described in https://developer.okta.com/blog/2021/03/08/jhipster-quarkus-oidc Okta CLI generates a okta.env file with Quarkus env variable which override the properties definition. We could make Okta CLI add the following variable

export QUARKUS_OIDC_AUTHENTICATION_SCOPES=profile,address,email,address,phone

Then the user would source the file to use Okta claims.

@mraible @dminkovski WDYT?

mraible commented 3 years ago

I think the Quarkus blueprint should work like the main generator in that you can switch from Keycloak to Okta easily.

If you don't add a groups claim to the ID token on Okta, you're able to log in, but can't see any entities.

Are you proposing we add another claim on Okta for Quarkus? Is it really necessary for both Quarkus with JHipster and Quarkus without JHipster? If so, we should update the Okta CLI to add it.

danielpetisme commented 3 years ago

Bassically, Keycloak and Okta claims within JHipster context are not consistent. Honestly, I don't master all the claim definition, for instance why Keycloak is defining a jhipster claim which is not defined in Okta for instance.

My options are basically, either we generate Keycloak and Okta configurations and let dev to a manual switch to Okta if they want to or we only focus on Keycloak and recommend Okta CLI to handle the Okta proper configuration.

dminkovski commented 3 years ago

@danielpetisme honestly it might be the best solution to just add this in the readme / properties file as comments like you suggested. If this is not to become a choice in the generator, I am not sure its worth any other overhead... WDYT?

danielpetisme commented 3 years ago

Hi all, First of all, sorry for there is very very high latency in the answers...

As suggested by @mraible I tested the recent JHipster Keycloak configuration. First of, JHipster Spring has explicit the claim required https://github.com/jhipster/jhipster-sample-app-oauth2/blob/e334ed50a82346b26f8edf0d69173c3962d75813/src/main/resources/config/application.yml#L138

That means Quarkus and Spring application can align on the same claims! I had a look to the Keycloak configuration and by default, the rolesclaim is embedding in the access token.

Now, regarding the roles vs groups issue. The easier, for now, is to mimic JHipster Spring with a getRolesFromClaims method which extract roles from the groups claim and if not found explore the roles claim. https://github.com/jhipster/jhipster-sample-app-oauth2/blob/e334ed50a82346b26f8edf0d69173c3962d75813/src/main/java/io/github/jhipster/sample/security/SecurityUtils.java#L83 I made a prototype where have been using

 private Set<String> getRolesFromClaims() {
        if(!accessToken.containsClaim("groups") && !accessToken.containsClaim("roles")) {
            return Collections.emptySet();
        }
        JsonArray roles = accessToken.getClaim("groups");
        if (roles == null) {
            roles = accessToken.getClaim("roles");
        }
        return roles.stream().map(it -> it.toString().replace("\"", "")).collect(Collectors.toSet());
    }

I don't like this kind of snippet because it implies hard coded values. I tried to use the quarkus.oidc.roles.role-claim-path property but it requires adding the roles in the ID token too. I'll open the discussion on the JHipster ML.

What's next @dminkovski I'll close your PR for now and flag this issue as "on-hold".

We start to move to JHipster 7 thus take advantage of the new Keycloak configuration. Once done, if you're still motivated, feel free to update the Quarkus claim to be consistent with the Spring version and add the above snippet.

ebihimself commented 3 years ago

In case the Jhipster keycloak docker image is used for development, by adding the "protocolMappers" attribute to the web_app client, we will have the user group in the JWT claim. After adding group mapping either manually or with configurations, the accessToken.getClaim("groups") and @RolesAllowed annotation works.

src/main/docker/realm-config/jhipster-realm.json

{
  "clients": [
    {
      "id": "1eabef67-6473-4ba8-b07c-14bdbae4aaed",
      "clientId": "web_app",
      "protocolMappers": [
        {
          "id": "cb24d7e6-ad56-484b-988c-46206e51bf6d",
          "name": "groups",
          "protocol": "openid-connect",
          "protocolMapper": "oidc-usermodel-realm-role-mapper",
          "consentRequired": false,
          "config": {
            "multivalued": "true",
            "userinfo.token.claim": "true",
            "user.attribute": "foo",
            "id.token.claim": "true",
            "access.token.claim": "true",
            "claim.name": "groups",
            "jsonType.label": "String"
          }
        }
      ]
    }
  ]
}
mraible commented 3 years ago

Increased bug bounty on this one to $300.