Add http root to OIDC back channel logout handlers

[sberyozkin]

Fixes #42483.

Simple PR to take the http root path into consideration when creating OIDC back channel logout handlers.

Backchannel logout is typically used to logout a user from all the applications, to support a global logout. For example, a user explicitly logs out from one Quarkus application and then the OIDC provider will separately notify all other registered Quarkus services to clear the session when the user attempts to access them.

The existing test code is here, and I can confirm it failed initially with this PR before I fixed it to use en empty http root prefix if it is set as /, since the back channel logout path also starts from /.

To really test that an http root like /a is taken account, I'd need to create a new integration test module, which I'm not too keen :-) given the simplicity of the fix. If I add an http root configuration to integration-tests/oidc-wiremock where the backchannel test is available, then it would impact a real lot of tests.

Let me know what you think please

[michalvavrik]

Change makes sense.

• I think you should add information to the https://quarkus.io/guides/all-config#quarkus-oidc_quarkus-oidc-logout-backchannel-path JavaDoc about that. For example look here https://quarkus.io/guides/all-config#quarkus-rest_quarkus-rest-path as they mention that path is relative to the root path.
• I think you are expecting too much from users, I don't know if they always set root path in form /path or they do /path/ or path etc. Same goes for the backchannel path. I wonder if you should do something like io.quarkus.deployment.util.UriNormalizationUtil.normalizeWithBase(httpRootPath, oidcTenantConfig.logout.backchannel.path.get(), true).toString() (UPDATE: well, that's in deployment module, so probably some other util). If you don't want to do that, maybe just document expected value for this config property. (talking about rootPath + backChannelPath code)

[sberyozkin]

Thanks @michalvavrik,

I think you should add information to the https://quarkus.io/guides/all-config#quarkus-oidc_quarkus-oidc-logout-backchannel-path JavaDoc about that. For example look here https://quarkus.io/guides/all-config#quarkus-rest_quarkus-rest-path as they mention that path is relative to the root path.

Sure, just did it, also noting the back channel logout path must start from '/' for now at least, as it has been an expectation from the very start

I think you are expecting too much from users, I don't know if they always set root path in form /path or they do /path/ or path etc. Same goes for the backchannel path. I wonder if you should do something like io.quarkus.deployment.util.UriNormalizationUtil.normalizeWithBase(httpRootPath, oidcTenantConfig.logout.backchannel.path.get(), true).toString() (UPDATE: well, that's in deployment module, so probably some other util). If you don't want to do that, maybe just document expected value for this config property. (talking about rootPath + backChannelPath code)

May be we should have that utility code available in the runtime as well... For now, I did a few tweaks to ensure it is always in the /some-root-path or / format, in the latter case an empty value is used due to the backchannel path expected to start from '/'. FYI, when I was looking at how to get to the root path, I was tracing its use in the path matching code from the default tenant resolver, but your suggestions make total sense

Would you like me to tweak something else ?

[michalvavrik]

Would you like me to tweak something else ?

No, LGTM. Thanks.

[sberyozkin]

Sounds good, thanks.

[github-actions]

🎊 PR Preview 35f4c4a has been successfully built and deployed to https://quarkus-pr-main-42524-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.

[quarkus-bot]

Status for workflow Quarkus CI

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

✅ 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.