-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add support for OIDC BackChannel Logout #24611
Add support for OIDC BackChannel Logout #24611
Conversation
#24377 is still failing I have tested issue #23477 with the following commands: $ git clone https://github.com/quarkusio/quarkus.git $ cd quarkus In the existing getting-started project, I have update the pom.xml to point to io.quarkus quarkus-integration-tests-parent 999-SNAPSHOT quarkus-integration-test-resteasy-jackson Quarkus - Integration Tests - RESTEasy Jackson io.quarkus quarkus-resteasy-jackson io.quarkus quarkus-smallrye-openapi .... ...Testing I have executed the same test as in the bug description step 1: In a browser, enter http://localhost:8080/hello step 2: step 3: step 4: Correct behaviour should have been redirected to the quarkus realm login URL |
@orivat Thanks, this is actually what I'm seeing as well - I could not make Keycloak produce a backchannel logout request. We'll look with @pedroigor at it. You have mentioned you had it working with WildFly - can you please try to use the same Keycloak configuration you use there ? |
Did you configure the backchannel url into the realm? |
@tassadar81 I've set it directly in Keycloak admin console, in the |
@orivat See @tassadar81's question, while testing, did you add a backchannel URL in Keycloak ? |
@sberyozkin ok and then you called the logout endpoint as specified in the well known configuration isn't it? That's the path i was using when i opened the #23477; so i think you should get a backchannel logout as i got it |
In my local test (I modified But no backchannel request happened. Perhaps it is because only a single endpoint is used ? |
@sberyozkin Well i'm not sure i've understood what you did, i'm trying to explain myself better: the logout i was talking about the the one specified in the well known configuration: http://mykeycloakinstance/auth/realms/myrealm/.well-known/openid-configuration In the realm i guess you have configured this one in the "Backchannel Logout URL" client configuration: http://localhost:8080/myquarkusapp/back-channel-logout Which it will be called by keycloak once openid-connect/logout has called providing a jwt. No RP logout has to be called. Is it what you did? If is what you did maybe a callback is issued but the jwt authentication fails? |
@tassadar81 I was running an application endpoint in the debug mode with a breakpoint in the Vert.x handler. So, as far as the end session Keycloak endpoint is concerned, I've tried 2 options:
How do you call that endpoint ? Do you mean you direct the browser to it yourself ? Or from curl ? (Yes this is the end session URL which is returned in the well known config that an RP initiated logout redirects the users to) |
eb40be8
to
a28d221
Compare
@tassadar81 Can you please create a simple reproducer project, with a Keycloak realm configured to support the backchannel URL which will point to a Quarkus JAX-RS endpoint, and then show how you call the KC logout endpoint, i.e, so that even without this PR we could see that this JAX-RS endpoint gets the backchannel request (it will have no effect in this case, - but I'd like to see what is it in your setup that makes it work), thanks |
a28d221
to
203e920
Compare
Hi @pedroigor Thanks for the hint re calling out to Quarkus from a Docker hosted Keycloak - I did not get it myself in time that it won't work with I have tested today that it also works with live Keycloak sending a backchannel logout token - token signature is verified, I think the test added with this PR is quite close to the reality - the only difference is that the logout token is produced by the test code, the rest would be quite similar (same keys are used to sign both ID and logout tokens, etc). In general I think it would be indeed a good enhancement for the users of Keycloak (and other OIDC providers which support it) to start adding the options to logout the users from all of the current applications... Thanks for the various hints Let me know what you think may need to be changed |
This comment has been minimized.
This comment has been minimized.
Sorry I was checking the specific test related in this test suite and ignored the rest - looks like they have been affected by the new test configuration, will have a look tomorrow |
docs/src/main/asciidoc/security-openid-connect-web-authentication.adoc
Outdated
Show resolved
Hide resolved
docs/src/main/asciidoc/security-openid-connect-web-authentication.adoc
Outdated
Show resolved
Hide resolved
* if the logout token contains it. | ||
*/ | ||
@ConfigItem | ||
public Optional<Duration> age = Optional.empty(); |
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.
Wouldn't make more sense to have this property available only for the backchannel logout config category?
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.
I thought about for a moment but then decided it could of common interest, the same extra restriction (that this token was not created 6 months ago :-) ) can be applied to bearer or even code flow tokens (smallrye-jwt has it and MP-JWT 2.1 will have it too).
203e920
to
bc4706d
Compare
@pedroigor I've moved the backchannel logout config group under the existing logout one, it is looking OK, thanks. |
CI is looking not bad so far, JVM 11 Windows test are quite unstable: |
Failing Jobs - Building bc4706d
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 11 Windows #- Failing: extensions/reactive-routes/deployment
! Skipped: extensions/agroal/deployment extensions/elytron-security-jdbc/deployment extensions/flyway/deployment and 183 more 📦 extensions/reactive-routes/deployment✖
|
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.
Thanks
Thanks @pedroigor |
Fixes #23477.
This Draft PR introduces a support for OIDC
Back-Channel Logout
. The technical difficulty in supporting it in Quarkus OIDC is to do with it being stateless, with the session cookie keeping IdToken by default, therefore a map keeping the verified Logout tokens is used to check if the returning session IdToken matches a given logout token and the session has to be cleared.Here is a summary of the changes:
BackChannelLogoutHandler
addsVert.x
routes for the default and static tenants if they have the back-channel path configuredBackChannelLogoutHandler
also handles the back-channel logout requests, if it is a valid form url encoded POST request, by verifying that the logout token is generally valid (signature, optionaliss
,audience
, has eithersub
orsid
, has theevents
claim) and saves it in a concurrent mapCodeAuhenticationMechanism
, when reauthenticatingIdToken
checks if a matching verified Logout Token is available and if yes then compares the some of theIdToken
andLogoutToken
claims (iss
,sid
if set, etc) and if the match is confirmed then removes the session cookieonly
the logout tokens without theexp
claim (CC @pedroigor). ID and access JWT tokens still must provide a validexp
claim.Current restrictions:
TenantConfigResolver
) as it will involve the dynamic creation of Vert.x routes, and a need to restrict a total number of tenants - it can be done in the next phaseKeycloak
->Quarkus BackChannel Logout Request
(and nowQuarkus callback to Keycloak
to verify logout token while holding theKeycloak to Quarkus BackChannel Logout Request
) is not supported right now for the logout tokens