From 211873d41b4c1b9dbc654f200cbd58fc7cd9ff50 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Fri, 18 Oct 2024 16:52:14 +0200 Subject: [PATCH] Improve handling for backchannel logout requests (#34104) - Guard against a null backchannel logout URL in ResourceAdminManager#logoutClientSessionWithBackchannelLogoutUrl. - When the host placeholder cannot be resolved, throw an IllegalStateException with diagnostic context (clientId, clientSessionId, both note values) instead of silently returning null. The exception is handled by AuthenticationManager#backchannelLogoutClientSession, which logs it and returns a 5xx response so the failure is visible to operators. - Expand debug logging around the backchannel logout flow to make it easier to correlate token requests with logout attempts. Fixes #34104 Signed-off-by: Thomas Darimont --- .../oidc/grants/OAuth2GrantTypeBase.java | 3 +- .../managers/ResourceAdminManager.java | 29 +++++++++++++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantTypeBase.java b/services/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantTypeBase.java index 6fc154353981..a4354a21fa44 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantTypeBase.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantTypeBase.java @@ -227,7 +227,8 @@ protected void updateClientSession(AuthenticatedClientSessionModel clientSession String adapterSessionId = formParams.getFirst(AdapterConstants.CLIENT_SESSION_STATE); if (adapterSessionId != null) { String adapterSessionHost = formParams.getFirst(AdapterConstants.CLIENT_SESSION_HOST); - logger.debugf("Adapter Session '%s' saved in ClientSession for client '%s'. Host is '%s'", adapterSessionId, client.getClientId(), adapterSessionHost); + logger.debugf("Adapter Session '%s' saved in ClientSession '%s' for client '%s'. Host is '%s'", + adapterSessionId, clientSession.getId(), client.getClientId(), adapterSessionHost); String oldClientSessionState = clientSession.getNote(AdapterConstants.CLIENT_SESSION_STATE); if (!adapterSessionId.equals(oldClientSessionState)) { diff --git a/services/src/main/java/org/keycloak/services/managers/ResourceAdminManager.java b/services/src/main/java/org/keycloak/services/managers/ResourceAdminManager.java index 77d75c8c6d71..62539dac5bee 100755 --- a/services/src/main/java/org/keycloak/services/managers/ResourceAdminManager.java +++ b/services/src/main/java/org/keycloak/services/managers/ResourceAdminManager.java @@ -52,6 +52,7 @@ import org.keycloak.representations.adapters.action.TestAvailabilityAction; import org.keycloak.services.ServicesLogger; import org.keycloak.services.util.ResolveRelative; +import org.keycloak.utils.StringUtil; import org.apache.http.NameValuePair; import org.apache.http.client.entity.UrlEncodedFormEntity; @@ -171,13 +172,33 @@ protected Response logoutClientSessions(RealmModel realm, ClientModel resource, public Response logoutClientSessionWithBackchannelLogoutUrl(ClientModel resource, AuthenticatedClientSessionModel clientSession) { String backchannelLogoutUrl = getBackchannelLogoutUrl(session, resource); + + if (backchannelLogoutUrl == null) { + return null; + } + // Send logout separately to each host (needed for single-sign-out in cluster for non-distributable apps - // KEYCLOAK-748) if (backchannelLogoutUrl.contains(CLIENT_SESSION_HOST_PROPERTY)) { String host = clientSession.getNote(AdapterConstants.CLIENT_SESSION_HOST); + if (StringUtil.isNullOrEmpty(host)) { + // Host placeholder in backchannel logout URL cannot be resolved. Usually the client did not send + // both 'client_session_host' and 'client_session_state' on its token request. + String adapterSessionId = clientSession.getNote(AdapterConstants.CLIENT_SESSION_STATE); + throw new IllegalStateException(String.format( + "Cannot resolve '%s' for backchannel-logout. " + + "clientId='%s' clientSessionId='%s' client_session_host='%s' client_session_state='%s'", + CLIENT_SESSION_HOST_PROPERTY, resource.getClientId(), clientSession.getId(), host, adapterSessionId)); + } + logger.debugf("Attempting backchannel-logout for client with host from client session. " + + "clientId='%s' clientSessionId='%s' host='%s' backchannelLogoutUrl='%s'", + resource.getClientId(), clientSession.getId(), host, backchannelLogoutUrl); String currentHostMgmtUrl = backchannelLogoutUrl.replace(CLIENT_SESSION_HOST_PROPERTY, host); return sendBackChannelLogoutRequestToClientUri(resource, clientSession, currentHostMgmtUrl); } else { + logger.debugf("Attempting backchannel-logout for client. " + + "clientId='%s' clientSessionId='%s' backchannelLogoutUrl='%s'", + resource.getClientId(), clientSession.getId(), backchannelLogoutUrl); return sendBackChannelLogoutRequestToClientUri(resource, clientSession, backchannelLogoutUrl); } } @@ -203,7 +224,9 @@ protected Response sendBackChannelLogoutRequestToClientUri(ClientModel resource, LogoutToken logoutToken = session.tokens().initLogoutToken(resource, user, clientSessionModel); String token = session.tokens().encode(logoutToken); if (logger.isDebugEnabled()) { - logger.debugv("logout resource {0} url: {1} sessionIds: ", resource.getClientId(), managementUrl); + logger.debugf("Sending backchannel-logout request to client. " + + "clientId='%s' clientSessionId='%s' backchannelLogoutUrl='%s'", + resource.getClientId(), clientSessionModel.getId(), managementUrl); } post = new HttpPost(managementUrl); @@ -220,7 +243,9 @@ protected Response sendBackChannelLogoutRequestToClientUri(ClientModel resource, int status = response.getStatusLine().getStatusCode(); EntityUtils.consumeQuietly(response.getEntity()); boolean success = status == 204 || status == 200; - logger.debugf("logout success for %s: %s", managementUrl, success); + logger.debugf("Received response for backchannel-logout from client. " + + "clientId='%s' clientSessionId='%s' backchannelLogoutUrl='%s' status=%s success=%s", + resource.getClientId(), clientSessionModel.getId(), managementUrl, status, success); return Response.status(status).build(); } finally { EntityUtils.consumeQuietly(response.getEntity());