diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java index 03ff614897..6ccbad3fb7 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java @@ -88,7 +88,11 @@ public class LogoutEndpoint { /** * Logout user session. User must be logged in via a session cookie. * + * When the logout is initiated by a remote idp, the parameter "initiating_idp" can be supplied. This param will + * prevent upstream logout (since the logout procedure has already been started in the remote idp). + * * @param redirectUri + * @param initiatingIdp The alias of the idp initiating the logout. * @return */ @GET @@ -96,7 +100,8 @@ public class LogoutEndpoint { public Response logout(@QueryParam(OIDCLoginProtocol.REDIRECT_URI_PARAM) String redirectUri, // deprecated @QueryParam("id_token_hint") String encodedIdToken, @QueryParam("post_logout_redirect_uri") String postLogoutRedirectUri, - @QueryParam("state") String state) { + @QueryParam("state") String state, + @QueryParam("initiating_idp") String initiatingIdp) { String redirect = postLogoutRedirectUri != null ? postLogoutRedirectUri : redirectUri; if (redirect != null) { @@ -130,7 +135,7 @@ public class LogoutEndpoint { if (state != null) userSession.setNote(OIDCLoginProtocol.LOGOUT_STATE_PARAM, state); userSession.setNote(AuthenticationManager.KEYCLOAK_LOGOUT_PROTOCOL, OIDCLoginProtocol.LOGIN_PROTOCOL); logger.debug("Initiating OIDC browser logout"); - Response response = AuthenticationManager.browserLogout(session, realm, authResult.getSession(), session.getContext().getUri(), clientConnection, headers); + Response response = AuthenticationManager.browserLogout(session, realm, authResult.getSession(), session.getContext().getUri(), clientConnection, headers, initiatingIdp); logger.debug("finishing OIDC browser logout"); return response; } else if (userSession != null) { // non browser logout diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlService.java b/services/src/main/java/org/keycloak/protocol/saml/SamlService.java index b78d5196c8..fbc2aa2ec1 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlService.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlService.java @@ -184,7 +184,7 @@ public class SamlService extends AuthorizationEndpointBase { session.getContext().setClient(client); logger.debug("logout response"); - Response response = authManager.browserLogout(session, realm, userSession, session.getContext().getUri(), clientConnection, headers); + Response response = authManager.browserLogout(session, realm, userSession, session.getContext().getUri(), clientConnection, headers, null); event.success(); return response; } @@ -422,7 +422,7 @@ public class SamlService extends AuthorizationEndpointBase { clientSession.setAction(AuthenticationSessionModel.Action.LOGGED_OUT.name()); } logger.debug("browser Logout"); - return authManager.browserLogout(session, realm, userSession, session.getContext().getUri(), clientConnection, headers); + return authManager.browserLogout(session, realm, userSession, session.getContext().getUri(), clientConnection, headers, null); } else if (logoutRequest.getSessionIndex() != null) { for (String sessionIndex : logoutRequest.getSessionIndex()) { diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index d6153f61fd..d8866cce27 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -482,14 +482,20 @@ public class AuthenticationManager { for (UserSessionModel userSession : userSessions) { AuthenticatedClientSessionModel clientSession = userSession.getAuthenticatedClientSessionByClient(client.getId()); if (clientSession != null) { - AuthenticationManager.backchannelLogoutClientSession(session, realm, clientSession, null, uriInfo, headers); + backchannelLogoutClientSession(session, realm, clientSession, null, uriInfo, headers); clientSession.setAction(AuthenticationSessionModel.Action.LOGGED_OUT.name()); org.keycloak.protocol.oidc.TokenManager.dettachClientSession(session.sessions(), realm, clientSession); } } } - public static Response browserLogout(KeycloakSession session, RealmModel realm, UserSessionModel userSession, UriInfo uriInfo, ClientConnection connection, HttpHeaders headers) { + public static Response browserLogout(KeycloakSession session, + RealmModel realm, + UserSessionModel userSession, + UriInfo uriInfo, + ClientConnection connection, + HttpHeaders headers, + String initiatingIdp) { if (userSession == null) return null; if (logger.isDebugEnabled()) { @@ -510,7 +516,7 @@ public class AuthenticationManager { } String brokerId = userSession.getNote(Details.IDENTITY_PROVIDER); - if (brokerId != null) { + if (brokerId != null && !brokerId.equals(initiatingIdp)) { IdentityProvider identityProvider = IdentityBrokerService.getIdentityProvider(session, realm, brokerId); response = identityProvider.keycloakInitiatedBrowserLogout(session, userSession, uriInfo, realm); if (response != null) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java index f80a4360f7..e04b7d5cf2 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java @@ -20,6 +20,7 @@ package org.keycloak.testsuite.broker; import java.util.List; import org.hamcrest.Matchers; +import org.apache.commons.lang.StringUtils; import org.jboss.arquillian.graphene.page.Page; import org.junit.After; import org.keycloak.admin.client.resource.RealmResource; @@ -163,10 +164,16 @@ public abstract class AbstractBaseBrokerTest extends AbstractKeycloakTest { } protected void logoutFromRealm(String realm) { + logoutFromRealm(realm, null); + } + + protected void logoutFromRealm(String realm, String initiatingIdp) { driver.navigate().to(BrokerTestTools.getAuthRoot(suiteContext) + "/auth/realms/" + realm + "/protocol/" + "openid-connect" - + "/logout?redirect_uri=" + encodeUrl(getAccountUrl(realm))); + + "/logout?redirect_uri=" + encodeUrl(getAccountUrl(realm)) + + (!StringUtils.isBlank(initiatingIdp) ? "&initiating_idp=" + initiatingIdp : "") + ); try { Retry.execute(() -> { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerLogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerLogoutTest.java new file mode 100644 index 0000000000..6d63a5c81b --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerLogoutTest.java @@ -0,0 +1,88 @@ +package org.keycloak.testsuite.broker; + +import org.junit.Before; +import org.junit.Test; +import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.UserRepresentation; + +import javax.ws.rs.core.Response; +import java.util.List; + +import static org.keycloak.testsuite.admin.ApiUtil.createUserWithAdminClient; +import static org.keycloak.testsuite.admin.ApiUtil.resetUserPassword; +import static org.keycloak.testsuite.broker.BrokerTestConstants.REALM_PROV_NAME; +import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage; + +public class KcOidcBrokerLogoutTest extends AbstractBaseBrokerTest { + @Override + protected BrokerConfiguration getBrokerConfiguration() { + return KcOidcBrokerConfiguration.INSTANCE; + } + + @Before + public void createUser() { + log.debug("creating user for realm " + bc.providerRealmName()); + + final UserRepresentation user = new UserRepresentation(); + user.setUsername(bc.getUserLogin()); + user.setEmail(bc.getUserEmail()); + user.setEmailVerified(true); + user.setEnabled(true); + + final RealmResource realmResource = adminClient.realm(bc.providerRealmName()); + final String userId = createUserWithAdminClient(realmResource, user); + + resetUserPassword(realmResource.users().get(userId), bc.getUserPassword(), false); + } + + @Before + public void addIdentityProviderToProviderRealm() { + log.debug("adding identity provider to realm " + bc.consumerRealmName()); + + final RealmResource realm = adminClient.realm(bc.consumerRealmName()); + realm.identityProviders().create(bc.setUpIdentityProvider(suiteContext)).close(); + } + + @Before + public void addClients() { + final List clients = bc.createProviderClients(suiteContext); + final RealmResource providerRealm = adminClient.realm(bc.providerRealmName()); + for (final ClientRepresentation client : clients) { + log.debug("adding client " + client.getClientId() + " to realm " + bc.providerRealmName()); + + final Response resp = providerRealm.clients().create(client); + resp.close(); + } + } + + @Test + public void logoutWithoutInitiatingIdpLogsOutOfIdp() { + logInAsUserInIDPForFirstTime(); + assertLoggedInAccountManagement(); + + logoutFromRealm(bc.consumerRealmName()); + driver.navigate().to(getAccountUrl(REALM_PROV_NAME)); + waitForPage(driver, "log in to provider", true); + } + + @Test + public void logoutWithActualIdpAsInitiatingIdpDoesNotLogOutOfIdp() { + logInAsUserInIDPForFirstTime(); + assertLoggedInAccountManagement(); + + logoutFromRealm(bc.consumerRealmName(), "kc-oidc-idp"); + driver.navigate().to(getAccountUrl(REALM_PROV_NAME)); + waitForPage(driver, "keycloak account management", true); + } + + @Test + public void logoutWithOtherIdpAsInitiatinIdpLogsOutOfIdp() { + logInAsUserInIDPForFirstTime(); + assertLoggedInAccountManagement(); + + logoutFromRealm(bc.consumerRealmName(), "something-else"); + driver.navigate().to(getAccountUrl(REALM_PROV_NAME)); + waitForPage(driver, "log in to provider", true); + } +}