Add integration tests with Keycloak #2343
Conversation
| } | ||
| // Keycloak Admin Client brings RESTEasy Classic, which conflicts with Quarkus RESTEasy Reactive; | ||
| // it must not be present during Quarkus augmentation otherwise Quarkus tests won't start. | ||
| compileOnly(libs.keycloak.admin.client) |
There was a problem hiding this comment.
I spent 5 hours today trying to find a better solution for this, but in the end, having both RESTEasy Classic and Reactive on the classpath gets blocked here:
Maybe a better solution would be to isolate the Keycloak container in a separate module, and shade RESTEasy Classic.
There was a problem hiding this comment.
Having a server runner that launches pre-built jars would help here, I believe... There was a discussion or PR about that somewhere, right?
There was a problem hiding this comment.
I recall that RESTEasy classic vs reactive issue from Nessie, where it's resolved this way. But the effect's the same: exclude RESTEasy Classic.
There was a problem hiding this comment.
I suggest to leave it this way for now and I'll investigate if shading is an option.
There was a problem hiding this comment.
shading is too much for this case :) I'd go with a custom runner... not ok "as is" for now
adutra
force-pushed
the
keycloak-it
branch
3 times, most recently
from
22eba9a to
64249ed
Compare
| keycloak.createRole(principalRole); | ||
| keycloak.createUser( | ||
| principalName, | ||
| // Use the same password as the client secret |
There was a problem hiding this comment.
Why? As for me, it detracts from the test's validity because now it is not obvious whether authentication happens locally (client secret) or via Keycloak 🤔
There was a problem hiding this comment.
I'm not sure I get your point. This password is never used outside of this class (see method below).
There was a problem hiding this comment.
Anyways, I replaced this with a fixed password for all users. Let me know if that's better.
There was a problem hiding this comment.
I guessed the password was not used outside, but that's also a reason for not using the same password, right? ;) Thx for changing!
There was a problem hiding this comment.
My point was that if the password is the same (with a specific comment) it creates an expectation that it might be reused somewhere... which is what I wanted to avoid 😅
| tokenEndpoint = issuerUrl.resolve("protocol/openid-connect/token"); | ||
| createRole("ALL"); | ||
| createUser("root"); | ||
| assignRoleToUser("ALL", "root"); |
There was a problem hiding this comment.
FYI I initially forgot to assign PRINCIPAL_ROLE:ALL to root and yet, the tests passed: this is because ActiveRolesProvider automatically infers "all granted roles" when no role is found in the token. This is another reason why I am advocating for merging ActiveRolesProvider into Authenticator.
github-project-automation
bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
github-project-automation
bot
moved this from Ready to merge
to Done
in Basic Kanban Board
No description provided.