Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

logout.backchannel.path fails when http.root-path is present - Again #42990

Closed
AB-xdev opened this issue Sep 3, 2024 · 12 comments · Fixed by #43000
Closed

logout.backchannel.path fails when http.root-path is present - Again #42990

AB-xdev opened this issue Sep 3, 2024 · 12 comments · Fixed by #43000
Labels
area/oidc kind/bug Something isn't working
Milestone

Comments

@AB-xdev
Copy link
Contributor

AB-xdev commented Sep 3, 2024

Describe the bug

Basically the same problem as in #42483 happens again with the latest release (3.14.1).
The fix was likely incomplete, as we also encountered the problem in 3.13.3.

How to Reproduce?

We created a full PoC repo with detailed information, how the bug can be reproduced:
https://github.com/AB-xdev/quarkus-backchannel-logout-poc

Quarkus version or git rev

3.14.1

@AB-xdev AB-xdev added the kind/bug Something isn't working label Sep 3, 2024
@geoand
Copy link
Contributor

geoand commented Sep 3, 2024

cc @sberyozkin

@sberyozkin
Copy link
Member

Thanks @AB-xdev for preparing a POC repo.

@AB-xdev
Copy link
Contributor Author

AB-xdev commented Sep 3, 2024

Problem is likely caused by

router.route(getRootPath() + oidcTenantConfig.logout.backchannel.path.get())

While debugging RouterImpl#route I noticed that nearly all paths are always registered without the /backend-rootpath-prefix.
Only the backchannel-logout endpoints are inconsistent in this behavior and get registered with it.

When I use the debugger to replace /backend/oidc/wim/back-channel-logout with /oidc/wim/back-channel-logout during registration and then perform a backchannel logout via keycloak everything works as expected:

2024-09-03 17:03:22,329 DEBUG [io.qua.oid.run.BackChannelLogoutHandler] (vert.x-eventloop-thread-4) Back channel logout request for the tenant wim received
// No further errors
// Browser gets redirect to login during next request

@sberyozkin
Copy link
Member

@AB-xdev Do you have some concrete examples in the Quarkus code where the root path is not used at the router registration but which can be successfully accessed with the http root path configured ?

@sberyozkin
Copy link
Member

@cescoffier Hi Clement, what is the right way to deal with the HTTP root when using router.route, do we use it HTTP route directly to calculate the root path or is it automatically implied ?

@cescoffier
Copy link
Member

it should be automatically mounted at the right path - except if the route is an absolute route.

@sberyozkin
Copy link
Member

sberyozkin commented Sep 3, 2024

Thanks @cescoffier, so we'll just need to remove that HTTP root prefix.
@AB-xdev Hi, since you are already looking at that code, can you give me a favor, and remove getRootPath() at the route setup and confirm with your POC it fixes it ? PR will be welcomed too, thanks

@sberyozkin
Copy link
Member

sberyozkin commented Sep 3, 2024

@AB-xdev I'll add a proper test as well to support your PR should you go with it, thanks

@AB-xdev
Copy link
Contributor Author

AB-xdev commented Sep 4, 2024

I created a PR here that removes the 16 characters: #43000

Note however that setting it up took me longer than doing the actual change so for future I would appreciate it if you could do small changes instead :)

@sberyozkin
Copy link
Member

Thanks @AB-xdev It is not about a number of characters but about the community willing to help, we can't just always momentarily switch off from whatever we do and fix the current issue, so well done for giving it a go.

@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Sep 5, 2024
@gsmet gsmet modified the milestones: 3.16 - main, 3.14.3 Sep 6, 2024
gsmet pushed a commit to gsmet/quarkus that referenced this issue Sep 6, 2024
Currently the backchannel logout route is registered with the root-path, resulting in a path like this:
``/backend/backend/back-channel-logout`` instead of ``/backend/back-channel-logout``

See also quarkusio#42990

(cherry picked from commit f92fc56)
@AB-xdev
Copy link
Contributor Author

AB-xdev commented Sep 18, 2024

Rechecked it and can confirm that this is fixed indeed now fixed in 1.14.3.

Thank you for the very quick response and release 👍

@geoand
Copy link
Contributor

geoand commented Sep 18, 2024

Great, thanks for checking!

danielsoro pushed a commit to danielsoro/quarkus that referenced this issue Sep 20, 2024
Currently the backchannel logout route is registered with the root-path, resulting in a path like this:
``/backend/backend/back-channel-logout`` instead of ``/backend/back-channel-logout``

See also quarkusio#42990
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/oidc kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants