From 8d82390843743e51a888340fb24b34f17f4d3f2a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Anders=20B=C3=A5tstrand?= <anders.batstrand@dsb.no>
Date: Fri, 24 Feb 2017 14:02:54 +0100
Subject: [PATCH] KEYCLOAK-4504 New configuration option for SAML Broker:

* postBindingLogout: Indicates if POST or redirect should be used for the logout requests.

This applies to both IdP-initiated logout, and Keycloak-initiated logout. If unset (for example when upgrading Keycloak), the setting is initially set to the same as postBindingResponse.

The flag is also set when importing IdP metadata.
---
 .../org/keycloak/broker/saml/SAMLEndpoint.java    |  2 +-
 .../broker/saml/SAMLIdentityProvider.java         |  7 +++++--
 .../broker/saml/SAMLIdentityProviderConfig.java   | 14 ++++++++++++++
 .../broker/saml/SAMLIdentityProviderFactory.java  | 15 +++++++++------
 .../testsuite/admin/IdentityProviderTest.java     |  1 +
 .../admin/messages/admin-messages_en.properties   |  2 ++
 .../partials/realm-identity-provider-saml.html    |  7 +++++++
 7 files changed, 39 insertions(+), 9 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 38e57cb256..72d84ee5c0 100755
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
@@ -303,7 +303,7 @@ public class SAMLEndpoint {
             builder.issuer(issuerURL);
             JaxrsSAML2BindingBuilder binding = new JaxrsSAML2BindingBuilder()
                         .relayState(relayState);
-            boolean postBinding = config.isPostBindingResponse();
+            boolean postBinding = config.isPostBindingLogout();
             if (config.isWantAuthnRequestsSigned()) {
                 KeyManager.ActiveRsaKey keys = session.keys().getActiveRsaKey(realm);
                 String keyName = config.getXmlSigKeyInfoKeyNameTransformer().getKeyName(keys.getKid(), keys.getCertificate());
diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java
index 1f8f7933d8..f2ce37d538 100755
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java
@@ -184,12 +184,15 @@ public class SAMLIdentityProvider extends AbstractIdentityProvider<SAMLIdentityP
             try {
                 SAML2LogoutRequestBuilder logoutBuilder = buildLogoutRequest(userSession, uriInfo, realm, singleLogoutServiceUrl);
                 JaxrsSAML2BindingBuilder binding = buildLogoutBinding(session, userSession, realm);
-                return binding.postBinding(logoutBuilder.buildDocument()).request(singleLogoutServiceUrl);
+                if (getConfig().isPostBindingLogout()) {
+                    return binding.postBinding(logoutBuilder.buildDocument()).request(singleLogoutServiceUrl);
+                } else {
+                    return binding.redirectBinding(logoutBuilder.buildDocument()).request(singleLogoutServiceUrl);
+                }
             } catch (Exception e) {
                 throw new RuntimeException(e);
             }
         }
-
     }
 
     protected SAML2LogoutRequestBuilder buildLogoutRequest(UserSessionModel userSession, UriInfo uriInfo, RealmModel realm, String singleLogoutServiceUrl) {
diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java
index f07495f652..d2a410b4ea 100755
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java
@@ -161,6 +161,20 @@ public class SAMLIdentityProviderConfig extends IdentityProviderModel {
         getConfig().put("postBindingResponse", String.valueOf(postBindingResponse));
     }
 
+    public boolean isPostBindingLogout() {
+        String postBindingLogout = getConfig().get("postBindingLogout");
+        if (postBindingLogout == null) {
+            // To maintain unchanged behavior when adding this field, we set the inital value to equal that
+            // of the binding for the response:
+            return isPostBindingResponse();
+        }
+        return Boolean.valueOf(postBindingLogout);
+    }
+
+    public void setPostBindingLogout(boolean postBindingLogout) {
+        getConfig().put("postBindingLogout", String.valueOf(postBindingLogout));
+    }
+
     public boolean isBackchannelSupported() {
         return Boolean.valueOf(getConfig().get("backchannelSupported"));
     }
diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java
index 0cc72da4c1..75fbfdb40a 100755
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java
@@ -84,11 +84,12 @@ public class SAMLIdentityProviderFactory extends AbstractIdentityProviderFactory
                 if (idpDescriptor != null) {
                     SAMLIdentityProviderConfig samlIdentityProviderConfig = new SAMLIdentityProviderConfig();
                     String singleSignOnServiceUrl = null;
-                    boolean postBinding = false;
+                    boolean postBindingResponse = false;
+                    boolean postBindingLogout = false;
                     for (EndpointType endpoint : idpDescriptor.getSingleSignOnService()) {
                         if (endpoint.getBinding().toString().equals(JBossSAMLURIConstants.SAML_HTTP_POST_BINDING.get())) {
                             singleSignOnServiceUrl = endpoint.getLocation().toString();
-                            postBinding = true;
+                            postBindingResponse = true;
                             break;
                         } else if (endpoint.getBinding().toString().equals(JBossSAMLURIConstants.SAML_HTTP_REDIRECT_BINDING.get())){
                             singleSignOnServiceUrl = endpoint.getLocation().toString();
@@ -96,10 +97,11 @@ public class SAMLIdentityProviderFactory extends AbstractIdentityProviderFactory
                     }
                     String singleLogoutServiceUrl = null;
                     for (EndpointType endpoint : idpDescriptor.getSingleLogoutService()) {
-                        if (postBinding && endpoint.getBinding().toString().equals(JBossSAMLURIConstants.SAML_HTTP_POST_BINDING.get())) {
+                        if (postBindingResponse && endpoint.getBinding().toString().equals(JBossSAMLURIConstants.SAML_HTTP_POST_BINDING.get())) {
                             singleLogoutServiceUrl = endpoint.getLocation().toString();
+                            postBindingLogout = true;
                             break;
-                        } else if (!postBinding && endpoint.getBinding().toString().equals(JBossSAMLURIConstants.SAML_HTTP_REDIRECT_BINDING.get())){
+                        } else if (!postBindingResponse && endpoint.getBinding().toString().equals(JBossSAMLURIConstants.SAML_HTTP_REDIRECT_BINDING.get())){
                             singleLogoutServiceUrl = endpoint.getLocation().toString();
                             break;
                         }
@@ -110,8 +112,9 @@ public class SAMLIdentityProviderFactory extends AbstractIdentityProviderFactory
                     samlIdentityProviderConfig.setWantAuthnRequestsSigned(idpDescriptor.isWantAuthnRequestsSigned());
                     samlIdentityProviderConfig.setAddExtensionsElementWithKeyInfo(false);
                     samlIdentityProviderConfig.setValidateSignature(idpDescriptor.isWantAuthnRequestsSigned());
-                    samlIdentityProviderConfig.setPostBindingResponse(postBinding);
-                    samlIdentityProviderConfig.setPostBindingAuthnRequest(postBinding);
+                    samlIdentityProviderConfig.setPostBindingResponse(postBindingResponse);
+                    samlIdentityProviderConfig.setPostBindingAuthnRequest(postBindingResponse);
+                    samlIdentityProviderConfig.setPostBindingLogout(postBindingLogout);
 
                     List<KeyDescriptorType> keyDescriptor = idpDescriptor.getKeyDescriptor();
                     String defaultCertificate = null;
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java
index 2fbb5a30d6..a43591862b 100755
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java
@@ -529,6 +529,7 @@ public class IdentityProviderTest extends AbstractAdminTest {
         assertThat(config.keySet(), containsInAnyOrder(
           "validateSignature",
           "singleLogoutServiceUrl",
+          "postBindingLogout",
           "postBindingResponse",
           "postBindingAuthnRequest",
           "singleSignOnServiceUrl",
diff --git a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
index 76c6d5078e..c1550a7d6a 100644
--- a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
+++ b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
@@ -523,6 +523,8 @@ http-post-binding-response=HTTP-POST Binding Response
 http-post-binding-response.tooltip=Indicates whether to respond to requests using HTTP-POST binding. If false, HTTP-REDIRECT binding will be used.
 http-post-binding-for-authn-request=HTTP-POST Binding for AuthnRequest
 http-post-binding-for-authn-request.tooltip=Indicates whether the AuthnRequest must be sent using HTTP-POST binding. If false, HTTP-REDIRECT binding will be used.
+http-post-binding-logout=HTTP-POST Binding Logout
+http-post-binding-logout.tooltip=Indicates whether to respond to requests using HTTP-POST binding. If false, HTTP-REDIRECT binding will be used.
 want-authn-requests-signed=Want AuthnRequests Signed
 want-authn-requests-signed.tooltip=Indicates whether the identity provider expects signed a AuthnRequest.
 force-authentication=Force Authentication
diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html b/themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html
index 3aad92beb0..33623d38cb 100755
--- a/themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html
+++ b/themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html
@@ -142,6 +142,13 @@
                 </div>
                 <kc-tooltip>{{:: 'http-post-binding-for-authn-request.tooltip' | translate}}</kc-tooltip>
             </div>
+            <div class="form-group">
+                <label class="col-md-2 control-label" for="postBindingLogout">{{:: 'http-post-binding-logout' | translate}}</label>
+                <div class="col-md-6">
+                    <input ng-model="identityProvider.config.postBindingLogout" id="postBindingLogout" onoffswitchvalue on-text="{{:: 'onText' | translate}}" off-text="{{:: 'offText' | translate}}" />
+                </div>
+                <kc-tooltip>{{:: 'http-post-binding-logout.tooltip' | translate}}</kc-tooltip>
+            </div>
             <div class="form-group">
                 <label class="col-md-2 control-label" for="wantAuthnRequestsSigned">{{:: 'want-authn-requests-signed' | translate}}</label>
                 <div class="col-md-6">