-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Support for OIDC RP-Initiated form post logout mode #48575
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
Conversation
2a4fbf8
to
cc80eb6
Compare
🙈 The PR is closed and the preview is expired. |
docs/src/main/asciidoc/security-oidc-expanded-configuration.adoc
Outdated
Show resolved
Hide resolved
3e791db
to
7d70ebd
Compare
7d70ebd
to
d2c9ec1
Compare
f843bf2
to
a4b0d5c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
a4b0d5c
to
b76af3b
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
b76af3b
to
a035762
Compare
I just did a minor update to try to get the flaky OidcClientTest test which checks the configured expiry time passing, which is absolutely not related to this PR |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
...runtime/src/main/java/io/quarkus/resteasy/runtime/AuthenticationRedirectExceptionMapper.java
Show resolved
Hide resolved
...-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityRecorder.java
Outdated
Show resolved
Hide resolved
a035762
to
8c6c2a4
Compare
I enhanced the existing test and also did play for a while with trying to do a similar test for Quarkus REST with Keycloak directly - but it appears the form post logout does not work with Keycloak right now... But indeed, both REST and Resteasy exception mappers are identical now, so the test should be enough I hope... Thanks |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@michalvavrik @gastaldi Hi, the failing test is definitely not related, I can run the build again, but I'm not sure it is necessary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for the OIDC RP-Initiated form-post logout mode by extending configuration, builder APIs, runtime handling, and tests.
- Introduce a
LogoutMode
enum with new config and builder methods. - Implement form-post logout HTML generation via
LogoutUtils
and throwAuthenticationRedirectException(200, ...)
. - Update HTTP handlers and exception mappers to emit the auto-submitting form, and add an integration test verifying the flow.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java | Added testFormPostLogoutWebApp() to verify form-post logout behavior |
integration-tests/oidc-tenancy/src/main/resources/application.properties | Configured logout endpoints, paths, and set logout-mode=form-post |
integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/TenantResource.java | Added formPostPostLogout endpoint to confirm logout |
integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/OidcResource.java | Simulates OIDC provider’s form-post logout handler |
integration-tests/oidc-client-wiremock/src/test/java/io/quarkus/it/keycloak/OidcClientTest.java | Adjusted expiry assertion bounds for token tests |
extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityRecorder.java | Emit form-post content on HTTP 200 in AuthenticationRedirectException |
extensions/resteasy-reactive/rest/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/exceptionmappers/AuthenticationRedirectExceptionMapper.java | Mapper now embeds form-post body for code 200 |
extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/AuthenticationRedirectExceptionMapper.java | Classic REST mapper updated similarly |
extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigImpl.java | Extended test config mapping to record logoutMode |
extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/builders/LogoutConfigBuilder.java | Added logoutMode(...) builder methods |
extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcTenantConfig.java | Defined LogoutMode enum and new logoutMode() config property |
extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java | Branch for form-post logout flow in code flow mechanism |
extensions/oidc/runtime/src/main/java/io/quarkus/oidc/LogoutUtils.java | Utility to build auto-submitting HTML form for logout |
docs/src/main/asciidoc/security-oidc-expanded-configuration.adoc | Documentation for quarkus.oidc.logout.logout-mode |
Comments suppressed due to low confidence (3)
extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/builders/LogoutConfigBuilder.java:145
- This Javadoc comment refers to
clearSiteData
but is placed onlogoutMode(...)
. Update the@param
description to match thelogoutMode
parameter or remove it for the no-arg overload.
* @param clear site data directives {@link OidcTenantConfig.Logout#clearSiteData()}
integration-tests/oidc-tenancy/src/main/resources/application.properties:58
- The
end-session-path
property is placed outside of thelogout
namespace. It should likely bequarkus.oidc.tenant-web-app.logout.end-session-path
to be picked up by the logout config.
quarkus.oidc.tenant-web-app.end-session-path=http://localhost:8081/oidc/form-post-logout
integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/OidcResource.java:384
- For the form-post logout simulation, this returns a 303 redirect instead of an auto-submitting HTML form. You should return a 200 response with the generated form HTML to match the intended flow.
return Response.seeOther(URI.create(postLogoutRedirectUri + "?username=" + userName)).build();
...-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityRecorder.java
Show resolved
Hide resolved
...resteasy/reactive/server/runtime/exceptionmappers/AuthenticationRedirectExceptionMapper.java
Outdated
Show resolved
Hide resolved
...runtime/src/main/java/io/quarkus/resteasy/runtime/AuthenticationRedirectExceptionMapper.java
Outdated
Show resolved
Hide resolved
Hey @gastaldi, I'll take all those 3 suggestions :-), thanks. Let me apply, I believe @michalvavrik is good with the rest of the changes, given his earlier feedback |
33d1042
to
bcb8215
Compare
bcb8215
to
c69ce54
Compare
@gastaldi Interesting, Copilot is not aware of our formatter rules :-) |
Status for workflow
|
Status for workflow
|
Fixes #48435.
This PR supports an OIDC RP-Initiated logout mode where the logout parameters are auto-submitted to the OIDC logout endpoint as form parameters.
FYI, initially, I thought that in this case, I'd write the response direct in the
CodeAuthenticationMechanism
, but then I thought, even though it is not an actual 302 redirect, it is still an implicit redirect via the auto-submitting HTML form and if users haveAuthenticationRedirectException
mappers then they should still be able to catch such exceptions, and also should be able to filter such implicit redirects.Initial idea how to to pass the form post logout content alongside AuthenticationRedirectException:
I ended up passing the form post logout data as a request context property that
AuthenticationRedirectException
handlers can access fromRoutingContext
How it was implemented in the end:
I simply pass the form post logout content that actually inlines OIDC logout endpoint URL as the
AuthenticationRedirectException
redirect URI parameter but with the 200 code, and the handlers can distinguish how to respond given the code. InjectingRoutingContext
into the exception mappers may not work and dealing with RoutingContext is more complex.Michal, @michalvavrik, I believe we can think how to extend
AuthenticationRedirectException
to represent the form post logout content cleaner in one of the nextquarkus-security
api releases.I've worked on the test today with HtmlUnit and it works well with the automatic form submission.
CC @calvernaz