From 0bafec943a90c8ad4dcf7c1798a1cd444409bcd3 Mon Sep 17 00:00:00 2001 From: Atiqur Rahman Date: Tue, 29 Sep 2026 12:57:18 +0600 Subject: [PATCH 1/3] Handle missing Single Logout Service URL during SAML IdP logout (#53242) Signed-off-by: Atiqur Rahman --- .../keycloak/broker/saml/SAMLEndpoint.java | 16 ++- .../testsuite/broker/KcSamlLogoutTest.java | 97 +++++++++++++++++++ 2 files changed, 110 insertions(+), 3 deletions(-) diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java index 22444f4f00b9..9906edfe2d35 100755 --- a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java +++ b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java @@ -74,6 +74,7 @@ import org.keycloak.events.Errors; import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; +import org.keycloak.forms.login.LoginFormsProvider; import org.keycloak.keys.PublicKeyLoader; import org.keycloak.keys.PublicKeyStorageProvider; import org.keycloak.keys.PublicKeyStorageUtils; @@ -392,10 +393,19 @@ protected Response logoutRequest(LogoutRequestType request, String relayState) { } } + String logoutServiceUrl = config.getSingleLogoutServiceUrl(); + if (StringUtil.isBlank(logoutServiceUrl)) { + logger.debugf("IdP '%s' has no Single Logout Service URL; skipping LogoutResponse", config.getAlias()); + event.success(); + return session.getProvider(LoginFormsProvider.class) + .setSuccess(Messages.SUCCESS_LOGOUT) + .createInfoPage(); + } + String issuerURL = getEntityId(session.getContext().getUri(), realm); SAML2LogoutResponseBuilder builder = new SAML2LogoutResponseBuilder(); builder.logoutRequestID(request.getID()); - builder.destination(config.getSingleLogoutServiceUrl()); + builder.destination(logoutServiceUrl); builder.issuer(issuerURL); JaxrsSAML2BindingBuilder binding = new JaxrsSAML2BindingBuilder(session) .relayState(relayState); @@ -412,9 +422,9 @@ protected Response logoutRequest(LogoutRequestType request, String relayState) { } try { if (postBinding) { - return binding.postBinding(builder.buildDocument()).response(config.getSingleLogoutServiceUrl()); + return binding.postBinding(builder.buildDocument()).response(logoutServiceUrl); } else { - return binding.redirectBinding(builder.buildDocument()).response(config.getSingleLogoutServiceUrl()); + return binding.redirectBinding(builder.buildDocument()).response(logoutServiceUrl); } } catch (ConfigurationException e) { throw new RuntimeException(e); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java index 9151af9edf02..96dd651b3e2b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java @@ -187,4 +187,101 @@ public void testProviderInitiatedLogoutCorrectlyLogsOutConsumerClientsWhenPrinci testProviderInitiatedLogoutCorrectlyLogsOutConsumerClients(); } } + + @Test + public void testProviderInitiatedLogoutWithoutSingleLogoutServiceUrlLogsOutConsumerSession() throws Exception { + try (SamlMessageReceiver logoutReceiver = new SamlMessageReceiver(8082); + ClientAttributeUpdater cauConsumer = ClientAttributeUpdater.forClient(adminClient, bc.consumerRealmName(), AbstractSamlTest.SAML_CLIENT_ID_SALES_POST) + .setFrontchannelLogout(false) + .setAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE, logoutReceiver.getUrl()) + .update(); + ClientAttributeUpdater cauProvider = ClientAttributeUpdater.forClient(adminClient, bc.providerRealmName(), bc.getIDPClientIdInProviderRealm()) + .setFrontchannelLogout(true) + .update(); + Closeable idpUpdater = new IdentityProviderAttributeUpdater(identityProviderResource) + .removeAttribute(SAMLIdentityProviderConfig.SINGLE_LOGOUT_SERVICE_URL) + .update() + ) { + AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST, getConsumerRoot() + "/sales-post/saml", null); + + Document doc = SAML2Request.convert(loginRep); + + final AtomicReference nameIdRef = new AtomicReference<>(); + final AtomicReference sessionIndexRef = new AtomicReference<>(); + + new SamlClientBuilder() + // Login into SAML_CLIENT_ID_SALES_POST using provider realm as idp + .authnRequest(getConsumerSamlEndpoint(bc.consumerRealmName()), doc, SamlClient.Binding.POST).build() + .login().idp(bc.getIDPAlias()).build() + + .processSamlResponse(SamlClient.Binding.POST) + .targetAttributeSamlRequest() + .build() + + .login().user(bc.getUserLogin(), bc.getUserPassword()).build() + + .processSamlResponse(SamlClient.Binding.POST) + .build() + + // first-broker flow + .updateProfile().firstName("a").lastName("b").email(bc.getUserEmail()).username(bc.getUserLogin()).build() + .followOneRedirect() + + .processSamlResponse(SamlClient.Binding.POST) + .transformObject(saml2Object -> { + assertThat(saml2Object, Matchers.notNullValue()); + assertThat(saml2Object, isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); + return null; + }) + .build() + + // Login using a different client to the provider realm, should be already logged in + .authnRequest(getProviderSamlEndpoint(bc.providerRealmName()), PROVIDER_SAML_CLIENT_ID, PROVIDER_SAML_CLIENT_ID + "saml", POST).build() + .followOneRedirect() + + // Process saml response and store reference to nameId and sessionIndex so that we can initiate logout for the session + .processSamlResponse(POST) + .transformObject(saml2Object -> { + assertThat(saml2Object, isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); + ResponseType loginResp1 = (ResponseType) saml2Object; + final AssertionType firstAssertion = loginResp1.getAssertions().get(0).getAssertion(); + assertThat(firstAssertion, Matchers.notNullValue()); + assertThat(firstAssertion.getSubject().getSubType().getBaseID(), instanceOf(NameIDType.class)); + + NameIDType nameId = (NameIDType) firstAssertion.getSubject().getSubType().getBaseID(); + AuthnStatementType firstAssertionStatement = (AuthnStatementType) firstAssertion.getStatements().iterator().next(); + + nameIdRef.set(nameId); + sessionIndexRef.set(firstAssertionStatement.getSessionIndex()); + + return null; + }) + .build() + + // Send logout request to provider realm + .logoutRequest(getProviderSamlEndpoint(bc.providerRealmName()), PROVIDER_SAML_CLIENT_ID, POST) + .nameId(nameIdRef::get) + .sessionIndex(sessionIndexRef::get) + .build() + + // Provider realm should send LogoutRequest to consumer realm + .processSamlResponse(POST) + .transformObject(saml2Object -> { + assertThat(saml2Object, isSamlLogoutRequest(getConsumerRoot() + "/auth/realms/" + REALM_CONS_NAME + "/broker/" + IDP_SAML_ALIAS + "/endpoint")); + return saml2Object; + }) + .build() + + // Consumer realm receives LogoutRequest; without SLS URL it should NOT fail with 500, but return success (info page) + .executeAndTransform(response -> { + assertThat(response.getStatusLine().getStatusCode(), is(200)); + return null; + }); + + // Check whether logoutReceiver contains correct LogoutRequest + assertThat(logoutReceiver.isMessageReceived(), is(true)); + SAMLDocumentHolder message = logoutReceiver.getSamlDocumentHolder(); + assertThat(message.getSamlObject(), isSamlLogoutRequest(logoutReceiver.getUrl())); + } + } } From c0cf11dc5125983424f045b0f6f95216e6706715 Mon Sep 17 00:00:00 2001 From: Atiqur Rahman Date: Tue, 29 Sep 2026 13:15:31 +0600 Subject: [PATCH 2/3] Assert consumer user session is removed after logout Signed-off-by: Atiqur Rahman --- .../java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java index 96dd651b3e2b..aad3ded9bd13 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java @@ -18,6 +18,7 @@ import org.keycloak.saml.processing.api.saml.v2.request.SAML2Request; import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder; import org.keycloak.testframework.realm.ClientBuilder; +import org.keycloak.testsuite.admin.AdminApiUtil; import org.keycloak.testsuite.saml.AbstractSamlTest; import org.keycloak.testsuite.updaters.ClientAttributeUpdater; import org.keycloak.testsuite.updaters.IdentityProviderAttributeUpdater; @@ -278,6 +279,8 @@ public void testProviderInitiatedLogoutWithoutSingleLogoutServiceUrlLogsOutConsu return null; }); + assertThat(AdminApiUtil.findUserByUsernameId(adminClient.realm(bc.consumerRealmName()), bc.getUserLogin()).getUserSessions(), Matchers.empty()); + // Check whether logoutReceiver contains correct LogoutRequest assertThat(logoutReceiver.isMessageReceived(), is(true)); SAMLDocumentHolder message = logoutReceiver.getSamlDocumentHolder(); From 350cd887884d6cdf0d101dce7ee6e926cf82c2b6 Mon Sep 17 00:00:00 2001 From: Atiqur Rahman Date: Wed, 30 Sep 2026 17:59:36 +0600 Subject: [PATCH 3/3] Compact SAML broker logout test to comply with deprecation check threshold Signed-off-by: Atiqur Rahman --- .../testsuite/broker/KcSamlLogoutTest.java | 45 +++---------------- 1 file changed, 7 insertions(+), 38 deletions(-) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java index aad3ded9bd13..f213dca6cbdc 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlLogoutTest.java @@ -203,44 +203,27 @@ public void testProviderInitiatedLogoutWithoutSingleLogoutServiceUrlLogsOutConsu .removeAttribute(SAMLIdentityProviderConfig.SINGLE_LOGOUT_SERVICE_URL) .update() ) { - AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST, getConsumerRoot() + "/sales-post/saml", null); - - Document doc = SAML2Request.convert(loginRep); - + Document doc = SAML2Request.convert(SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST, getConsumerRoot() + "/sales-post/saml", null)); final AtomicReference nameIdRef = new AtomicReference<>(); final AtomicReference sessionIndexRef = new AtomicReference<>(); new SamlClientBuilder() - // Login into SAML_CLIENT_ID_SALES_POST using provider realm as idp .authnRequest(getConsumerSamlEndpoint(bc.consumerRealmName()), doc, SamlClient.Binding.POST).build() .login().idp(bc.getIDPAlias()).build() - .processSamlResponse(SamlClient.Binding.POST) - .targetAttributeSamlRequest() - .build() - + .targetAttributeSamlRequest().build() .login().user(bc.getUserLogin(), bc.getUserPassword()).build() - - .processSamlResponse(SamlClient.Binding.POST) - .build() - - // first-broker flow + .processSamlResponse(SamlClient.Binding.POST).build() .updateProfile().firstName("a").lastName("b").email(bc.getUserEmail()).username(bc.getUserLogin()).build() .followOneRedirect() - .processSamlResponse(SamlClient.Binding.POST) .transformObject(saml2Object -> { assertThat(saml2Object, Matchers.notNullValue()); assertThat(saml2Object, isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); return null; - }) - .build() - - // Login using a different client to the provider realm, should be already logged in + }).build() .authnRequest(getProviderSamlEndpoint(bc.providerRealmName()), PROVIDER_SAML_CLIENT_ID, PROVIDER_SAML_CLIENT_ID + "saml", POST).build() .followOneRedirect() - - // Process saml response and store reference to nameId and sessionIndex so that we can initiate logout for the session .processSamlResponse(POST) .transformObject(saml2Object -> { assertThat(saml2Object, isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); @@ -248,40 +231,26 @@ public void testProviderInitiatedLogoutWithoutSingleLogoutServiceUrlLogsOutConsu final AssertionType firstAssertion = loginResp1.getAssertions().get(0).getAssertion(); assertThat(firstAssertion, Matchers.notNullValue()); assertThat(firstAssertion.getSubject().getSubType().getBaseID(), instanceOf(NameIDType.class)); - NameIDType nameId = (NameIDType) firstAssertion.getSubject().getSubType().getBaseID(); AuthnStatementType firstAssertionStatement = (AuthnStatementType) firstAssertion.getStatements().iterator().next(); - nameIdRef.set(nameId); sessionIndexRef.set(firstAssertionStatement.getSessionIndex()); - return null; - }) - .build() - - // Send logout request to provider realm + }).build() .logoutRequest(getProviderSamlEndpoint(bc.providerRealmName()), PROVIDER_SAML_CLIENT_ID, POST) .nameId(nameIdRef::get) - .sessionIndex(sessionIndexRef::get) - .build() - - // Provider realm should send LogoutRequest to consumer realm + .sessionIndex(sessionIndexRef::get).build() .processSamlResponse(POST) .transformObject(saml2Object -> { assertThat(saml2Object, isSamlLogoutRequest(getConsumerRoot() + "/auth/realms/" + REALM_CONS_NAME + "/broker/" + IDP_SAML_ALIAS + "/endpoint")); return saml2Object; - }) - .build() - - // Consumer realm receives LogoutRequest; without SLS URL it should NOT fail with 500, but return success (info page) + }).build() .executeAndTransform(response -> { assertThat(response.getStatusLine().getStatusCode(), is(200)); return null; }); assertThat(AdminApiUtil.findUserByUsernameId(adminClient.realm(bc.consumerRealmName()), bc.getUserLogin()).getUserSessions(), Matchers.empty()); - - // Check whether logoutReceiver contains correct LogoutRequest assertThat(logoutReceiver.isMessageReceived(), is(true)); SAMLDocumentHolder message = logoutReceiver.getSamlDocumentHolder(); assertThat(message.getSamlObject(), isSamlLogoutRequest(logoutReceiver.getUrl()));