Support for OIDC FrontChannel logout

[sberyozkin]

Fixes #23478

This PR supports a front-channel logout request. If the current request path matches the configured frontchannel logout URL then the sid and iss query parameters are compared against the verified ID token's values and if all is good then the session cookie is removed.

As part of this work I also moved the backchannel logout check from the earlier PR to the same stage where frontchannel logout URL is checked, after id token has been verified (instead of doing a limited token validation in the backchannel check code) - this allowed to include sending both the backchannel and frontchannel logout CDI events, referencing the identity of the now logged out user.

Wiremock based test has been added.

I can also try to do a manual verification directly against Keycloak (it is not possible to set up the frontchannel URL in the Client using the admin API)

[sberyozkin]

Not sure if supporting POST requests is necessary, the spec talks about the query parameters only - but if needed it can be easily added. I'll test with Keycloak next

[sberyozkin]

@pedroigor I haven't managed to confirm it with Keycloak - I have enabled FrontChannel logout and added a quarkus endpoint url which should be called. Then I logged in to Quarkus as alice, then started an RP initiated logout which redirected me to Keycloak but at this point I expected, not exactly sure what, I guess some page which would internally call to this Quarkus frontchannel logout path, but it does not happen.
How can one tell Keycloak to return a page to the user with the iframe calling out to Quarkus frontchannel logout URL ?

[sberyozkin]

@tassadar81 Can you please check this PR and confirm it a front-channel logout works for you ? I can't make it work with Keycloak - I have configured Keycloak client to link to a Quarkus endpoint's frontchannel URL, and initiated a logout from Quarkus, however I'm not getting a frontchannel endpoint call, it is most likely to do with the way I'm trying to reproduce it, as far as this PR is concerned, it just adds a simple code to verify it is a valid frontchannel call

[gsmet]

@tassadar81 any chance you could check if this PR works for you?

[tassadar81]

@tassadar81 any chance you could check if this PR works for you?

Hello Guillaume, hello all,
it's in the todo list, sorry for that but i'm overwhelmed at work

[gsmet]

@sberyozkin this will need a rebase unfortunately
@tassadar81 friendly ping for the test, you should be able to test this branch even before the rebase

[gsmet]

@sberyozkin this will need a rebase unfortunately
@tassadar81 friendly ping for the test, you should be able to test this branch even before the rebase

[geoand]

What's the status of this one?

[sberyozkin]

@geoand Trying to finalize it next

[sberyozkin]

@pjgg Hi Pablo, FYI, this PR introduced a front-channel logout support, but along the way, as I mentioned earlier, events are generated for both initiating and completing back channel logout.
One test is failing now, I haven't had time to investigate yet, @michalvavrik, Hi Michal, quick question, this exception, which I have debugged is being thrown when this test line is run, but CodeAuthenticationMechanism#getChallenge is not invoked, it does appear to be similar to the problem you fixed with CodeFlowTest#testRPInitiatedLogout, do we need to tighten that fix a bit to take care of AuthenticationFailedException as well, when this PR was opened awhile back this test was passing. I'll be happy to have a look if you think it can be related, just hoping to clarify a few details first :-)

[michalvavrik]

@michalvavrik, Hi Michal, quick question, this exception, which I have debugged is being thrown when this test line is run, but CodeAuthenticationMechanism#getChallenge is not invoked, it does appear to be similar to the problem you fixed with CodeFlowTest#testRPInitiatedLogout, do we need to tighten that fix a bit to take care of AuthenticationFailedException as well, when this PR was opened awhile back this test was passing. I'll be happy to have a look if you think it can be related, just hoping to clarify a few details first :-)

When AuthenticationFailedException is thrown here https://github.com/quarkusio/quarkus/pull/25343/files#diff-d6d40795b1093d6a39a62041f2567d0931b1f7fa30ebfea44e6663f45441df75R261, it's caught here

quarkus/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java

Line 283 in 3e90e9a

LOG.errorf("ID token verification failure: %s", t.getCause());

and AuthenticationCompletionException is thrown, thus 401 and no challenge. I didn't investigate why it worked before is it's bit late, but if you want me to have a look, no problem, please let me know.

[sberyozkin]

@michalvavrik Oh sorry, should've guessed myself, I just got side-tracked by remembering your fix :-), I'll take care of fixing it. Thanks for having a look.

[sberyozkin]

Sorry, forgot to include a new class in the commit

[sberyozkin]

Hi @pjgg Can you please check this PR against your OIDC back channel demo, Pedro approved this PR awhile back and I only had to tweak it a bit for the tests to start passing again, but the backchannel code has changed compared to what you have been testing against: now the back channel logout can be completed only after the ID token has been verified, it is a bit stricter now, and in addition you can get backchannel initiate and completion CDI events - which you were interested in I believe.

Note the events are typically including a verified SecurityIdentity but with the back-channel initiation, when the request is coming from KC as opposed to from the user, SecurityIdentity is not available - so in this case one can get a JsonObject representing a parsed/verified logout token; the back-channel completion event will though get SecurityIdentity included - which is the main reason why the back channel logout completion now is done only after the ID token has been verified.

As far as the front-channel logout is concerned - it can be verified with a demo later, the PR wiremock test should be sufficient for now, I've added a release noteworthy label...
Thanks

[sberyozkin]

OK, I'm going to go ahead and merge once the builds pass; it is not going to be backported so it is not urgent for QE to verify, Pablo is very busy elsewhere right now but it will be eventually verified as well in a few weeks or so