cloudtrust / keycloak-wsfed

WS-Federation implementation for keycloak
GNU Affero General Public License v3.0
37 stars 30 forks source link

Backchannel logout problem #48

Open vanrar68 opened 4 years ago

vanrar68 commented 4 years ago

I'm taking a shot in the dark here: There is no such thing as "backchannel logout" in WSFED. There simply can't be. Keycloak has zero knowledge of a client side session id and even if it had, the WSFED specs proposes no method to pass such an id to the client along with the wa=signoutcleanup1.0 request. The only way to achieve a Keycloak initiated logout with a WSFED connected client is for Keycloak to ask the browser for a redirect. The browser will then send the logout request along with a client session cookie, enabling the client to close the session associated with the cookie.

Again, this is my understanding of the current implementation of the backchannel logout in keycloak-wsfed. I'd gladly hear anyone explain to me how things are supposed to work.

For the reference, here's the current keycloak wsfed code handling the backchannel logout in WSFedLoginProtocol.java:

URIBuilder builder = new URIBuilder(logoutUrl)
        .addParameter(WSFedConstants.WSFED_ACTION, WSFedConstants.WSFED_SIGNOUT_CLEANUP_ACTION)
        .addParameter(WSFedConstants.WSFED_REALM, client.getClientId());
HttpGet get = new HttpGet(builder.build());
HttpResponse response = httpClient.execute(get);

As you can see, no data related to the logged in user or to the client side session is passed along with the request.

lagess commented 4 years ago

The WS-Fed specs indeed does not mention anything like backchannel logout, at least to my knowledge. For this module, we forked the basis of this code from https://github.com/dbarentine/keycloak-wsfed, and to be honest, we haven't used the SLO functionality yet in our deployments. Thus, as you have already noticed, the logout feature suffers issues and lacks cleaning, for sure.

Presumably, backchannel logout should be removed from this module, and we would only keep frontchannel (serial and parallel). Thus the code you mention might need to be simply cleaned.

vanrar68 commented 4 years ago

Ok, I'll submit a PR to remove the backchannel logout feature