Fix Keycloak test server account setup that results in invalid grant with 25

[michalvavrik]

I was helping to my colleague to migrate from very old Keycloak 11 to 25 in the quarkus-qe/beefy-scenarios#510 and I mentioned that io.quarkus:quarkus-test-keycloak-server doesn't work anymore because a first name and last name are not set. Maybe changes are forced by https://www.keycloak.org/docs/25.0.6/upgrading/#default-validations that describes that we will be forced to update invalid values of the first name and the last name from previous versions. And here https://www.keycloak.org/docs/25.0.6/upgrading/#changes-to-the-user-representation-in-both-admin-api-and-account-contexts it is described that last name and the first name were moved to a different base class.

2024-09-27 20:36:28,729 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (executor-thread-1) HTTP Request to /oidc-client/annotation/user-name failed, error id: 87eabea9-9f6e-45c4-8c56-4dad3778f78e-1: io.quarkus.oidc.client.OidcClientException: {"error":"invalid_grant","error_description":"Account is not fully set up"}
	at io.quarkus.oidc.client.runtime.OidcClientImpl.emitGrantTokens(OidcClientImpl.java:261)
	at io.quarkus.oidc.client.runtime.OidcClientImpl$1.lambda$get$0(OidcClientImpl.java:158)
	at io.smallrye.context.impl.wrappers.SlowContextualFunction.apply(SlowContextualFunction.java:21)
	at io.smallrye.mutiny.operators.uni.UniOnItemTransform$UniOnItemTransformProcessor.onItem(UniOnItemTransform.java:36)
	at io.smallrye.mutiny.operators.uni.UniOperatorProcessor.onItem(UniOperatorProcessor.java:47)
	at io.smallrye.mutiny.operators.uni.UniOperatorProcessor.onItem(UniOperatorProcessor.java:47)
	at io.smallrye.mutiny.vertx.AsyncResultUni.lambda$subscribe$1(AsyncResultUni.java:35)
	at io.smallrye.mutiny.vertx.DelegatingHandler.handle(DelegatingHandler.java:25)
	at io.vertx.ext.web.client.impl.HttpContext.handleDispatchResponse(HttpContext.java:402)
	at io.vertx.ext.web.client.impl.HttpContext.execute(HttpContext.java:384)
	at io.vertx.ext.web.client.impl.HttpContext.next(HttpContext.java:362)
	at io.vertx.ext.web.client.impl.HttpContext.fire(HttpContext.java:329)
	at io.vertx.ext.web.client.impl.HttpContext.dispatchResponse(HttpContext.java:291)
	at io.vertx.ext.web.client.impl.HttpContext.lambda$null$7(HttpContext.java:512)
	at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:270)
	at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:252)
	at io.vertx.core.impl.ContextInternal.lambda$runOnContext$0(ContextInternal.java:50)
	at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:173)
	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:166)
	at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:469)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:569)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:994)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:1583)

The changes I made in the quarkus-rest-client-oidc-filter-deployment (legacy to 25) are reproducing the issue. Also it seems that setting these fields with the legacy image leads to failures.

[michalvavrik]

I'll check the failures. I only tested module where I made changes.

[github-actions]

🎊 PR Preview 1ff7c69 has been successfully built and deployed to https://quarkus-pr-main-43571-preview.surge.sh/version/main/guides/

• Images of blog posts older than 3 months are not available.
• Newsletters older than 3 months are not available.

[sberyozkin]

I don't think we can do it, we still have tests running against WildFly based distro

[michalvavrik]

why? isn't it much more desirable to test this module with Keycloak 25? anyway, I don't mind to drop it if you want. it's your area, just tell me where else can I test it.

[sberyozkin]

Yeah, if I recall it right it causes a legacy KC load, but with this change it would switch to the latest version. What do you need to test ?

[michalvavrik]

I changed this, because then this module uses Keycloak Test Server with 25 rather then legacy 19. It has only test NamedOidcClientFilterDevModeTest and it reproduces the issue. I pasted error message in the description of this PR, it is the error thrown by this test when last name and first name are not set. Therefore it is a reproducer.

I don't know what is policy, but I tried to look in the Keycloak.org and it seems that they strongly recommended to use Quarkus-based version https://www.keycloak.org/2023/03/adapter-deprecation-update.html (this is for adapters because it's the newest article I found), so I thought it was not bad change.

[sberyozkin]

Dev service for KC does not set those properties but all works. I think you may instead do exactly what it and
some other tests do, load upconfig.json if it is non legacy

[michalvavrik]

Okay, I didn't realize that this KeycloakTestResourceLifecycleManager is not supposed to work with Keycloak 25. I'll read docs properly next time. You recommend there Dev Svcs, https://quarkus.io/guides/security-oidc-bearer-token-authentication#bearer-token-integration-testing-keycloak, though honestly I didn't get that I must use legacy.

I'll use Dev Svc.

[michalvavrik]

@sberyozkin I run smoke test with KC 24.0, 23.0 and it worked, but I cannot guarantee it will work with all the KC versions, I don't have in depth knowledge here.

I have also mentioned that CodeTenantReauthenticateTestCase starts both Dev Svc and test resource. I suppose it won't be only one.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 8b92c52.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

[sberyozkin]

@michalvavrik so what is the plan now? FYI, we already deal with this issue in similar cases, by loading upconfig, IMHO makes sense to do the same here, butvwe don't need to change tests

[michalvavrik]

@michalvavrik so what is the plan now? FYI, we already deal with this issue in similar cases, by loading upconfig, IMHO makes sense to do the same here, butvwe don't need to change tests

I have no plan. Based on your comment and fact that https://quarkus.io/guides/security-oidc-bearer-token-authentication#bearer-token-integration-testing-keycloak says Use [Dev Services for Keycloak](https://quarkus.io/guides/security-oidc-bearer-token-authentication#bearer-token-integration-testing-keycloak-devservices) instead of KeycloakTestResourceLifecycleManager for integration testing with Keycloak, unless you have specific requirements for using KeycloakTestResourceLifecycleManager. I think that you want users to use Dev Svc and you don't want this to work for 25 OOTB. That's alright, I just wished it was documented better.

No, I don't want to specify my own json for trivial cases, so this PR worked and fixes the issue.

butvwe don't need to change tests

well, I thought I need to prove I am fixing the issue :-) considering this needs to be set via system properties, I am not sure I can reliably test it in just one added unit test, so unless I am allowed to change some test, I cannot test this. I can be wrong for sure...

[michalvavrik]

btw I used dev svc and it worked, thank you.

[sberyozkin]

@michalvavrik Sure, thanks, I'll do the minor update here at some point, hopefully most people are now using dev service

Enjoy the weekend