quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.73k stars 2.67k forks source link

DevServices Keycloack roles are not generated properly #22668

Closed danielpetisme closed 2 years ago

danielpetisme commented 2 years ago

Describe the bug

There is an inconsistency in the way roles are created and managed in Keycloack Dev Services.

Given the following config

%dev.quarkus.keycloak.devservices.users.quarkus=quarkus
%dev.quarkus.keycloak.devservices.roles.quarkus=reader,writer

When the realm is created, the method createDefaultRealm is invoked. First the createRealmRep() method will be invoked. This is how roles are fetched:

List<String> distinctRoles = capturedDevServicesConfiguration.roles.values().stream().distinct()
                    .collect(Collectors.toList());
            for (String role : distinctRoles) {
                realm.getRoles().getRealm().add(new RoleRepresentation(role, null, false));
            }

https://github.com/quarkusio/quarkus/blob/a43f46e60adf656c67a3ea274094c21b857e9fbb/extensions/oidc/deployment/src/main/java/io/quarkus/oidc/deployment/devservices/keycloak/KeycloakDevServicesProcessor.java#L608

This means a unique role "reader,writer" will be created where the original intent was to create 2 roles ("reader", "writer").

After creating the roles, createDefaultRealm will create the user and call getUserRoles. This is how roles are fetched for a user

private String[] getUserRoles(String user) {
   String roles = capturedDevServicesConfiguration.roles.get(user);
   return roles == null ? ("alice".equals(user) ? new String[] { "admin", "user" } : new String[] { "user" })
       : roles.split(",");
}

https://github.com/quarkusio/quarkus/blob/a43f46e60adf656c67a3ea274094c21b857e9fbb/extensions/oidc/deployment/src/main/java/io/quarkus/oidc/deployment/devservices/keycloak/KeycloakDevServicesProcessor.java#L584 Here the role string is splitter on "," to collect a list.

Here is the result of the above config in Keycloak image

At the end of the day, we have n+1 role ("reader", "writer" and not "reader,writer") so I guess all the tests work pretty fine. My 2 cts are the UserRepresentation is creating behind the scene the correct roles hiding the lack of list management in the RoleRepresentation part.

Expected behavior

When providing a comma-separated list of roles, Dev Services should create each role, and not 1 is the exact text representation.

Actual behavior

No response

How to Reproduce?

See the bug description

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

quarkus-bot[bot] commented 2 years ago

/cc @stuartwdouglas

gsmet commented 2 years ago

@danielpetisme looks like you have it all figured out, care to provide a PR? :)