From e766e70dd2c8c8c1182ffd746182213cf6969390 Mon Sep 17 00:00:00 2001 From: rmartinc Date: Thu, 26 Jun 2025 12:35:35 +0200 Subject: [PATCH] Use POST binding for logout when REDIRECT url is not set and forced POST Closes #40637 Signed-off-by: rmartinc (cherry picked from commit 2db98b6a98c78a871921a82224f260b6fa383892) --- .../keycloak/protocol/saml/SamlProtocol.java | 38 ++++++++-------- .../protocol/saml/SamlProtocolTest.java | 6 ++- .../keycloak/testsuite/saml/LogoutTest.java | 43 +++++++++++++++++++ 3 files changed, 67 insertions(+), 20 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java index 58880e6cdf3b..525f2c164fb4 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java @@ -375,33 +375,35 @@ public static String getLogoutBindingTypeForClientSession(AuthenticatedClientSes boolean redirectUrlSet = !(logoutRedirectUrl == null || logoutRedirectUrl.trim().isEmpty()); boolean artifactUrlSet = !(logoutArtifactUrl == null || logoutArtifactUrl.trim().isEmpty()); - // Default to Redirect - String bindingType = SAML_REDIRECT_BINDING; + // client configured to force post binding and postUrl for logout is set + if (samlClient.forcePostBinding() && postUrlSet) { + return SAML_POST_BINDING; + } - // fall back to post binding if no redirect URL is set - if (postUrlSet && !redirectUrlSet) { - bindingType = SAML_POST_BINDING; + // client configured to force artifact binding and artifactUrl for logout is set + if (samlClient.forceArtifactBinding() && artifactUrlSet) { + return SAML_ARTIFACT_BINDING; } - // evaluate how the user logged in - if (clientSession != null) { - String sessionBinding = clientSession.getNote(SAML_BINDING); - if (SAML_ARTIFACT_BINDING.equals(sessionBinding) && artifactUrlSet) { - bindingType = SAML_ARTIFACT_BINDING; - } + final String bindingType = clientSession.getNote(SAML_BINDING); + + // if the login binding was artifact and url set, return artifact + if (SAML_ARTIFACT_BINDING.equals(bindingType) && artifactUrlSet) { + return SAML_ARTIFACT_BINDING; } - // client configured to force artifact binding and artifactUrl for logout is set - if (samlClient.forceArtifactBinding() && artifactUrlSet) { - bindingType = SAML_ARTIFACT_BINDING; + // if the login binding was POST and url set, return POST + if (SAML_POST_BINDING.equals(bindingType) && postUrlSet) { + return SAML_POST_BINDING; } - // client configured to force post binding and postUrl for logout ‚is set - if (samlClient.forcePostBinding() && postUrlSet) { - bindingType = SAML_POST_BINDING; + // fall back to post binding if no redirect URL + if (!redirectUrlSet && (postUrlSet || samlClient.forcePostBinding())) { + return SAML_POST_BINDING; } - return bindingType; + // Default to Redirect + return SAML_REDIRECT_BINDING; } protected String getNameIdFormat(SamlClient samlClient, AuthenticationSessionModel authSession) { diff --git a/services/src/test/java/org/keycloak/protocol/saml/SamlProtocolTest.java b/services/src/test/java/org/keycloak/protocol/saml/SamlProtocolTest.java index 550aaeedcbc3..cf7bd89b90ea 100644 --- a/services/src/test/java/org/keycloak/protocol/saml/SamlProtocolTest.java +++ b/services/src/test/java/org/keycloak/protocol/saml/SamlProtocolTest.java @@ -124,7 +124,11 @@ public void testGetLogoutBindingTypeForClientSession() { String bindingType = SamlProtocol.getLogoutBindingTypeForClientSession(clientSession); assertEquals(SamlProtocol.SAML_REDIRECT_BINDING, bindingType); + // default to POST if forced with no url + client.setForcePostBinding(true); + assertEquals(SamlProtocol.SAML_REDIRECT_BINDING, bindingType); + client.setForcePostBinding(false); clientSession.setNote(SamlProtocol.SAML_BINDING, SamlProtocol.SAML_ARTIFACT_BINDING); client.defineSetLogoutUrls(false, false, true, false); bindingType = SamlProtocol.getLogoutBindingTypeForClientSession(clientSession); @@ -143,10 +147,8 @@ public void testGetLogoutBindingTypeForClientSession() { client.defineSetLogoutUrls(false, false, true, false); bindingType = SamlProtocol.getLogoutBindingTypeForClientSession(clientSession); assertEquals(SamlProtocol.SAML_ARTIFACT_BINDING, bindingType); - } - @Test public void frontchannelLogoutSignsLogoutRequestsEvenThoughArtifactWasUsed() { TestClientModel client = new TestClientModel(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java index d1cb51e180da..9842e5c8e616 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java @@ -465,6 +465,49 @@ public void testLogoutWithPostBindingUnsetRedirectBindingSet() { assertLogoutEvent(SAML_CLIENT_ID_SALES_POST2); } + @Test + public void testFrontchannelLogoutInSameBrowserUsingDefaultAdminUrl() { + adminClient.realm(REALM_NAME) + .clients().get(sales2Rep.getId()) + .update(ClientBuilder.edit(sales2Rep) + .frontchannelLogout(true) + .attribute(SamlConfigAttributes.SAML_FORCE_POST_BINDING, Boolean.TRUE.toString()) + .attribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE, "") + .attribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE, "") + .adminUrl(SAML_ASSERTION_CONSUMER_URL_SALES_POST2) + .build()); + + SAMLDocumentHolder samlResponse = prepareLogIntoTwoApps() + .logoutRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, POST) + .nameId(nameIdRef::get) + .sessionIndex(sessionIndexRef::get) + .build() + + .processSamlResponse(POST) + .transformDocument(doc -> { + // Expect logout request for sales-post2 + SAML2Object so = (SAML2Object) SAMLParser.getInstance().parse(new DOMSource(doc)); + assertThat(so, isSamlLogoutRequest(SAML_ASSERTION_CONSUMER_URL_SALES_POST2)); + + // Emulate successful logout response from sales-post2 logout + return new SAML2LogoutResponseBuilder() + .destination(getAuthServerSamlEndpoint(REALM_NAME).toString()) + .issuer(SAML_CLIENT_ID_SALES_POST2) + .logoutRequestID(((LogoutRequestType) so).getID()) + .buildDocument(); + }) + .targetAttributeSamlResponse() + .targetUri(getAuthServerSamlEndpoint(REALM_NAME)) + .build() + + .getSamlResponse(POST); + + // Expect final successful logout response from auth server signalling final successful logout + assertThat(samlResponse.getSamlObject(), isSamlStatusResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); + assertThat(((StatusResponseType) samlResponse.getSamlObject()).getDestination(), is("http://url")); + assertLogoutEvent(SAML_CLIENT_ID_SALES_POST2); + } + private void assertLogoutEvent(String clientId) { List logoutEvents = adminClient.realm(REALM_NAME) .getEvents(Arrays.asList(EventType.LOGOUT.name()), clientId, null, null, null, null, null, null);