Customize application ObjectMapper can impact Keycloak admin client

[sdalichampt]

Describe the bug

For a front application that required a snake case naming strategy I've customize the application ObjectMapper by adding this ObjectMapperCustomizer.

@Singleton
public class CustomizeObjectMapper implements ObjectMapperCustomizer {
    public void customize(ObjectMapper mapper) {
        mapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);
        mapper.setPropertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE);
    }
}

the most important line is

mapper.setPropertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE);

The application also allows you to list the users managed in keycloak by using the quarkus-keycloak-admin-client-reactive for this.

But io.quarkus.keycloak.admin.client.reactive.runtime.ResteasyReactiveClientProvider#getObjectMapper have the comment :

// the whole idea here is to reuse the ObjectMapper instance

So keycloak admin client resuse a customized ObjectMapper which is not compatible with keycloak naming strategy.

Expected behavior

Keycloak users must be returned with all of their properties valued: Id, Username, FirstName, LastName, ....

Actual behavior

Keycloak users are all returned, but some properties are not set. These, for example FistName or LastName remain empty.

These are the properties whose name is impacted by the overloaded snake case strategy in ObjectMapperCustomizer.

How to Reproduce?

No response

Output of uname -a or ver

Ubuntu 22.10

Output of java -version

openjdk version "19.0.1" 2022-10-18

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.15.3.Final

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

Apache Maven 3.8.6

Additional information

No response

[quarkus-bot]

/cc @pedroigor (keycloak), @sberyozkin (keycloak)

[gsmet]

This is similar to the issue that was fixed here: #29984 .

We need the Keycloak client ObjectMapper to be specific (and compatible with the requirements of the Keycloak client).

[sberyozkin]

@sdalichampt Is it RestEasy reactive or classic ? Can you please try the reactive option if it is classic ? #30146 might be related.

[sberyozkin]

Hey @gsmet Not sure the direct use of Object mapper is realistic as KeycloakAdminClient in Quarkus is just a JAX-RS Client so it can only pick up a provider as a JAX-RS provider AFAIK... Though @michalvavrik, do you recall I asked you to update it to pick up the Quarkus Jackson provider, were there some issues in native mode without it ?

[sberyozkin]

Keycloak Admin Client Reactive:

quarkus/extensions/keycloak-admin-client-reactive/runtime/src/main/java/io/quarkus/keycloak/admin/client/reactive/runtime/ResteasyReactiveClientProvider.java

Line 19 in 5c98cc7

public class ResteasyReactiveClientProvider implements ResteasyClientProvider {

Keycloak Admin Client Classic:
https://github.com/quarkusio/quarkus/blob/main/extensions/keycloak-admin-client/runtime/src/main/java/io/quarkus/keycloak/adminclient/ResteasyKeycloakAdminClientRecorder.java#L60

I guess the classic version is affected, perhaps the reactive version can do better.

[sberyozkin]

#30436 might be related as well

[michalvavrik]

I'm pretty sure this is common problem for classic / reactive as we discussed it here #29076 (comment) (I'm still waiting for your answer there @sberyozkin :-) ). ATM we allow to customize KC client as it shares Quarkus Object Mapper, we can introduce a specific one. I wanted to do it, but waited till you might finish discussion with KC team as @gsmet suggested once you find a time. Please let me know..

[michalvavrik]

Also I forgot.

[sdalichampt]

@sberyozkin i use RestEasy reactive

[michalvavrik]

#30436 might be related as well

I don't think it's affected as the object mapper is picked up from CDI.

[sberyozkin]

@michalvavrik Sorry, not sure what you'd like us to discuss with the KC team - it feels to me it would be us going in the wrong direction.
IMHO, the fact that a custom mapper designed here to support server scope invocations is affecting the client invocations is a problem which I was hoping #30146 might address.
I.e, the client (keycloak-admin-client for ex) should be able to restrict its Jackson provider to the client scope only so then whatever the server Jackson provider is doing will not be affecting it.
Keycloak Admin API is widely used, I do not expect Keycloak team starting changing various Jackson annotations any time soon to avoid breaking various clients. @pedroigor, what is your opinion, can KC Admin model be tweaked a bit, for example, to avoid a problem discussed in this issue ?
Ideally though, we'd just add a client scope flag in KC admin client extensions to have Jackson instance isolated to KC Admin API only

[sberyozkin]

By the way, if we can just register a Keycloak Admin Client Object mapper which works for KC Admin client as Guillaume recommends, then it would be nice, I'm just not sure it is possible - since a single instance of Jackson is shared between the client and the server if I understand it correctly, or a single instance of ObjectMapper is shared between multiple Jackson instances. And there were native image problems with KC specific Jackson support...
But I'll be happy to find out the concrete KC Admin Client mapping requirements if registering a KC specific mapper is an option.

[michalvavrik]

@michalvavrik Sorry, not sure what you'd like us to discuss with the KC team - it feels to me it would be us going in the wrong direction.

RESTEasy Classic comment - Admin client by default uses org.jboss.resteasy.plugins.providers.jackson.ResteasyJackson2Provider so it makes sense that providers defined by RESTEasy Classic are always going to be used (here, it is io.quarkus.resteasy.common.runtime.jackson.QuarkusObjectMapperContextResolver). I thought whether KC admin client should not create it's own mapper as this other users may customize the mapper used by the admin client while KC team should now best what is desired set up, because the client is used to communicate with resources they produced.

RESTEasy Reactive comment - nothing to discuss with KC team.

IMHO, the fact that a custom mapper designed here to support server scope invocations is affecting the client invocations is a problem which I was hoping #30146 might address.

Reactive comments:

• Properly segregate Json MessageBodyReader/Writer classes for server and client #30146 deals exclusively with JSONB and from context I think this issue is related to Jackson (guessing)
• Always use ClientJacksonMessageBodyReader over ServerJacksonMessageBody #30436 won't help as ClientJacksonMessageBodyReader still uses injected ObjectMapper, thanks to Rest Client Reactive: Provide custom ObjectMapper in request scope #27203 it's possible to define your custom ObjectMapper for certain client build manually, but it still must be dedicated ObjectMapper we create ourselves, otherwise the same one as now is used.

I.e, the client (keycloak-admin-client for ex) should be able to restrict its Jackson provider to the client scope only so then whatever the server Jackson provider is doing will not be affecting it.

Reactive comment: +1; we don't need this option from Quarkus as we register providers manually when we are creating the client. It's just down to decision whether to reuse ObjectMapper - @geoand opinion who wrote these parts is more important than mine.

By the way, if we can just register a Keycloak Admin Client Object mapper which works for KC Admin client as Guillaume recommends, then it would be nice, I'm just not sure it is possible - since a single instance of Jackson is shared between the client and the server if I understand it correctly, or a single instance of ObjectMapper is shared between multiple Jackson instances. And there were native image problems with KC specific Jackson support...
But I'll be happy to find out the concrete KC Admin Client mapping requirements if registering a KC specific mapper is an option.

I think here you are talking about classic KC admin client, right (because reactive already gives us easy option to set the mapper)? We can either set org.keycloak.admin.client.spi.ResteasyClientProvider as reactive does, set existing client via org.keycloak.admin.client.KeycloakBuilder#resteasyClient or create Keycloak via org.keycloak.admin.client.Keycloak#getInstance that allows to set custom jackson provider (not shared).

[sberyozkin]

Hi Michal @michalvavrik, thanks for the analysis/feedback, so as far as this specific issue is concerned, we have a case of the shared ObjectMapper, and it is really down to fixing

quarkus/extensions/keycloak-admin-client-reactive/runtime/src/main/java/io/quarkus/keycloak/admin/client/reactive/runtime/ResteasyReactiveClientProvider.java

Line 59 in 5c98cc7

private ObjectMapper getObjectMapper(ObjectMapper value,

just to return new ObjectMapper() (possibly configured the same way as the default shared mapper), right ?

[michalvavrik]

That's exactly how I see it @sberyozkin .

[geoand]

I think we can probably be a little more intelligent and only create a new ObjectMapper if necessary. I am saying this because ObjectMapper is very heavy, so we definitely do not want to make a duplicate unless it is necessary.

I have an idea for how start. Maybe in the future we can make it even smarter.

[geoand]

#30529 is what I have in mind as a first step

[sberyozkin]

Thanks @geoand

[sberyozkin]

@michalvavrik Hey Michal, so re the classic RestEasy, since it is Resteasy classic specific provider (shipped with the Resteasy classic source) which is used with Keycloak Admin Client Classic, it would not pick up a custom Object Mapper set up at the Quarkus level, right ?

[michalvavrik]

@michalvavrik Hey Michal, so re the classic RestEasy, since it is Resteasy classic specific provider (shipped with the Resteasy classic source) which is used with Keycloak Admin Client Classic, it would not pick up a custom Object Mapper set up at the Quarkus level, right ?

Few months later io.quarkus.resteasy.common.runtime.jackson.QuarkusObjectMapperContextResolver was used by classic KC admin cl. (I suppose this is still a case) as ResteasyClientClassicProvider -> JacksonProvider -> ResteasyJackson2Provider -> QuarkusObjectMapperContextResolver, and it injects customized ObjectMapper. We can set alternative jackson provider that works analogously to what @geoand does in reactive twin. I didn't tested it this since, so that's provided nothing changed. Should I do it?

[sberyozkin]

@michalvavrik

We can set alternative jackson provider that works analogously to what @geoand does in reactive twin. I didn't tested it this since, so that's provided nothing changed. Should I do it?

This issue affects Keycloak Admin Client + ResteasyReactive combination, so having it working for RestEasy classic, given that we recommend ResteasyReactive in general, is less urgent. If you'll find some time in the next few weeks to have a look then it would be appreciated, you have the queue of issues already AFAIK :-)