KEYCLOAK-4062 - GUI changes for KeyName format + few tests

adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java

@@ -571,12 +571,8 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
                 key = locator.getKey(keyId);
                 boolean keyLocated = key != null;
 
-                if (validateRedirectBindingSignatureForKey(sigAlg, rawQueryBytes, decodedSignature, key)) {
-                    return true;
-                }
-
                 if (keyLocated) {
-                    return false;
+                    return validateRedirectBindingSignatureForKey(sigAlg, rawQueryBytes, decodedSignature, key);
                 }
             } catch (KeyManagementException ex) {
             }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java

@@ -31,6 +31,8 @@ import org.keycloak.common.util.PemUtils;
 import org.keycloak.keys.Attributes;
 import org.keycloak.keys.KeyProvider;
 import org.keycloak.keys.RsaKeyProviderFactory;
+import org.keycloak.protocol.saml.SamlClient;
+import org.keycloak.protocol.saml.SamlConfigAttributes;
 import org.keycloak.representations.idm.ComponentRepresentation;
 import org.keycloak.protocol.saml.mappers.AttributeStatementHelper;
 import org.keycloak.protocol.saml.mappers.RoleListMapper;
@@ -42,6 +44,7 @@ import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.saml.BaseSAML2BindingBuilder;
 import org.keycloak.saml.SAML2ErrorResponseBuilder;
 import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
+import org.keycloak.saml.common.util.XmlKeyInfoKeyNameTransformer;
 import org.keycloak.testsuite.adapter.AbstractServletsAdapterTest;
 import org.keycloak.testsuite.adapter.page.BadAssertionSalesPostSig;
 import org.keycloak.testsuite.adapter.page.BadClientSalesPostSigServlet;
@@ -71,6 +74,7 @@ import org.keycloak.testsuite.auth.page.login.Login;
 import org.keycloak.testsuite.auth.page.login.SAMLIDPInitiatedLogin;
 import org.keycloak.testsuite.page.AbstractPage;
 import org.keycloak.testsuite.util.IOUtil;
+import org.keycloak.testsuite.util.RealmBuilder;
 import org.keycloak.testsuite.util.UserBuilder;
 
 import org.openqa.selenium.By;
@@ -439,10 +443,11 @@ public abstract class AbstractSAMLServletsAdapterTest extends AbstractServletsAd
         testSuccessfulAndUnauthorizedLogin(employeeSigServletPage, testRealmSAMLRedirectLoginPage);
     }
 
+    private static final KeyPair NEW_KEY_PAIR = KeyUtils.generateRsaKeyPair(1024);
+    private static final String NEW_KEY_PRIVATE_KEY_PEM = PemUtils.encodeKey(NEW_KEY_PAIR.getPrivate());
+
     private PublicKey createKeys(String priority) throws Exception {
-        KeyPair keyPair = KeyUtils.generateRsaKeyPair(1024);
+        PublicKey publicKey = NEW_KEY_PAIR.getPublic();
-        String privateKeyPem = PemUtils.encodeKey(keyPair.getPrivate());
-        PublicKey publicKey = keyPair.getPublic();
 
         ComponentRepresentation rep = new ComponentRepresentation();
         rep.setName("mycomponent");
@@ -452,7 +457,7 @@ public abstract class AbstractSAMLServletsAdapterTest extends AbstractServletsAd
 
         org.keycloak.common.util.MultivaluedHashMap config = new org.keycloak.common.util.MultivaluedHashMap();
         config.addFirst("priority", priority);
-        config.addFirst(Attributes.PRIVATE_KEY_KEY, privateKeyPem);
+        config.addFirst(Attributes.PRIVATE_KEY_KEY, NEW_KEY_PRIVATE_KEY_PEM);
         rep.setConfig(config);
 
         testRealmResource().components().add(rep);
@@ -492,11 +497,53 @@ public abstract class AbstractSAMLServletsAdapterTest extends AbstractServletsAd
         testRotatedKeysPropagated(employeeSigPostNoIdpKeyServletPage, testRealmSAMLPostLoginPage);
     }
 
+    @Test
+    public void employeeSigPostNoIdpKeyTestNoKeyNameInKeyInfo() throws Exception {
+        RealmRepresentation r = testRealmResource().toRepresentation();
+        r.getAttributes().put(SamlConfigAttributes.SAML_SERVER_SIGNATURE_KEYINFO_KEY_NAME_TRANSFORMER, XmlKeyInfoKeyNameTransformer.NONE.name());
+        testRotatedKeysPropagated(employeeSigPostNoIdpKeyServletPage, testRealmSAMLPostLoginPage);
+    }
+
+    @Test
+    public void employeeSigPostNoIdpKeyTestCertSubjectAsKeyNameInKeyInfo() throws Exception {
+        RealmRepresentation r = testRealmResource().toRepresentation();
+        r.getAttributes().put(SamlConfigAttributes.SAML_SERVER_SIGNATURE_KEYINFO_KEY_NAME_TRANSFORMER, XmlKeyInfoKeyNameTransformer.CERT_SUBJECT.name());
+        testRotatedKeysPropagated(employeeSigPostNoIdpKeyServletPage, testRealmSAMLPostLoginPage);
+    }
+
+    @Test
+    public void employeeSigPostNoIdpKeyTestKeyIdAsKeyNameInKeyInfo() throws Exception {
+        RealmRepresentation r = testRealmResource().toRepresentation();
+        r.getAttributes().put(SamlConfigAttributes.SAML_SERVER_SIGNATURE_KEYINFO_KEY_NAME_TRANSFORMER, XmlKeyInfoKeyNameTransformer.KEY_ID.name());
+        testRotatedKeysPropagated(employeeSigPostNoIdpKeyServletPage, testRealmSAMLPostLoginPage);
+    }
+
     @Test
     public void employeeSigRedirNoIdpKeyTest() throws Exception {
         testRotatedKeysPropagated(employeeSigRedirNoIdpKeyServletPage, testRealmSAMLRedirectLoginPage);
     }
 
+    @Test
+    public void employeeSigRedirNoIdpKeyTestNoKeyNameInKeyInfo() throws Exception {
+        RealmRepresentation r = testRealmResource().toRepresentation();
+        r.getAttributes().put(SamlConfigAttributes.SAML_SERVER_SIGNATURE_KEYINFO_KEY_NAME_TRANSFORMER, XmlKeyInfoKeyNameTransformer.NONE.name());
+        testRotatedKeysPropagated(employeeSigRedirNoIdpKeyServletPage, testRealmSAMLRedirectLoginPage);
+    }
+
+    @Test
+    public void employeeSigRedirNoIdpKeyTestCertSubjectAsKeyNameInKeyInfo() throws Exception {
+        RealmRepresentation r = testRealmResource().toRepresentation();
+        r.getAttributes().put(SamlConfigAttributes.SAML_SERVER_SIGNATURE_KEYINFO_KEY_NAME_TRANSFORMER, XmlKeyInfoKeyNameTransformer.CERT_SUBJECT.name());
+        testRotatedKeysPropagated(employeeSigRedirNoIdpKeyServletPage, testRealmSAMLRedirectLoginPage);
+    }
+
+    @Test
+    public void employeeSigRedirNoIdpKeyTestKeyIdAsKeyNameInKeyInfo() throws Exception {
+        RealmRepresentation r = testRealmResource().toRepresentation();
+        r.getAttributes().put(SamlConfigAttributes.SAML_SERVER_SIGNATURE_KEYINFO_KEY_NAME_TRANSFORMER, XmlKeyInfoKeyNameTransformer.KEY_ID.name());
+        testRotatedKeysPropagated(employeeSigRedirNoIdpKeyServletPage, testRealmSAMLRedirectLoginPage);
+    }
+
     @Test
     public void employeeSigRedirOptNoIdpKeyTest() throws Exception {
         testRotatedKeysPropagated(employeeSigRedirOptNoIdpKeyServletPage, testRealmSAMLRedirectLoginPage);

themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties

@@ -272,6 +272,8 @@ logout-service-post-binding-url=Logout Service POST Binding URL
 logout-service-post-binding-url.tooltip=SAML POST Binding URL for the client's single logout service. You can leave this blank if you are using a different binding
 logout-service-redir-binding-url=Logout Service Redirect Binding URL
 logout-service-redir-binding-url.tooltip=SAML Redirect Binding URL for the client's single logout service. You can leave this blank if you are using a different binding.
+saml-signature-keyName-transformer=SAML Signature Key Name
+saml-signature-keyName-transformer.tooltip=Signed SAML documents contain identification of signing key in KeyName element. For Keycloak / RH-SSO counterparty, use KEY_ID, for MS AD FS use CERT_SUBJECT, for others check and use NONE if no other option works.
 
 # client import
 import-client=Import Client

themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js

@@ -837,6 +837,11 @@ module.controller('ClientDetailCtrl', function($scope, realm, client, templates,
         "transient",
         "persistent"
     ];
+    $scope.xmlKeyNameTranformers = [
+        "NONE",
+        "KEY_ID",
+        "CERT_SUBJECT"
+    ];
 
     $scope.canonicalization = [
         {name: "EXCLUSIVE", value:  "http://www.w3.org/2001/10/xml-exc-c14n#"  },
@@ -866,6 +871,7 @@ module.controller('ClientDetailCtrl', function($scope, realm, client, templates,
     $scope.samlEncrypt = false;
     $scope.samlForcePostBinding = false;
     $scope.samlForceNameIdFormat = false;
+    $scope.samlXmlKeyNameTranformer = $scope.xmlKeyNameTranformers[1];
     $scope.disableAuthorizationTab = !client.authorizationServicesEnabled;
     $scope.disableServiceAccountRolesTab = !client.serviceAccountsEnabled;
     $scope.disableCredentialsTab = client.publicClient;
@@ -918,6 +924,13 @@ module.controller('ClientDetailCtrl', function($scope, realm, client, templates,
                 $scope.samlServerSignatureEnableKeyInfoExtension = false;
             }
         }
+        if ($scope.client.attributes['saml.server.signature.keyinfo.xmlSigKeyInfoKeyNameTransformer'] === 'NONE') {
+            $scope.samlXmlKeyNameTranformer = $scope.xmlKeyNameTranformers[0];
+        } else if ($scope.client.attributes['saml.server.signature.keyinfo.xmlSigKeyInfoKeyNameTransformer'] === 'KEY_ID') {
+            $scope.samlXmlKeyNameTranformer = $scope.xmlKeyNameTranformers[1];
+        } else if ($scope.client.attributes['saml.server.signature.keyinfo.xmlSigKeyInfoKeyNameTransformer'] === 'CERT_SUBJECT') {
+            $scope.samlXmlKeyNameTranformer = $scope.xmlKeyNameTranformers[2];
+        }
         if ($scope.client.attributes["saml.assertion.signature"]) {
             if ($scope.client.attributes["saml.assertion.signature"] == "true") {
                 $scope.samlAssertionSignature = true;
@@ -1037,6 +1050,10 @@ module.controller('ClientDetailCtrl', function($scope, realm, client, templates,
         $scope.client.attributes['saml_name_id_format'] = $scope.nameIdFormat;
     };
 
+    $scope.changeSamlSigKeyNameTranformer = function() {
+        $scope.client.attributes['saml.server.signature.keyinfo.xmlSigKeyInfoKeyNameTransformer'] = $scope.samlXmlKeyNameTranformer;
+    };
+
     $scope.changeUserInfoSignedResponseAlg = function() {
         if ($scope.userInfoSignedResponseAlg === 'unsigned') {
             $scope.client.attributes['user.info.response.signature.alg'] = null;

themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js

@@ -764,11 +764,17 @@ module.controller('RealmIdentityProviderCtrl', function($scope, $filter, $upload
             "RSA_SHA512",
             "DSA_SHA1"
         ];
+        $scope.xmlKeyNameTranformers = [
+            "NONE",
+            "KEY_ID",
+            "CERT_SUBJECT"
+        ];
         if (instance && instance.alias) {
 
         } else {
             $scope.identityProvider.config.nameIDPolicyFormat = $scope.nameIdFormats[0].format;
             $scope.identityProvider.config.signatureAlgorithm = $scope.signatureAlgorithms[1];
+            $scope.identityProvider.config.samlXmlKeyNameTranformer = $scope.xmlKeyNameTranformers[1];
         }
     }
 

themes/src/main/resources/theme/base/admin/resources/partials/client-detail.html

@@ -158,6 +158,19 @@
                 </div>
                 <kc-tooltip>{{:: 'signature-algorithm.tooltip' | translate}}</kc-tooltip>
             </div>
+            <div class="form-group clearfix block" data-ng-show="(samlAssertionSignature || samlServerSignature) && protocol == 'saml'">
+                <label class="col-md-2 control-label" for="samlSigKeyNameTranformer">{{:: 'saml-signature-keyName-transformer' | translate}}</label>
+                <div class="col-sm-6">
+                    <div>
+                        <select class="form-control" id="xmlKeyNameTranformer"
+                                ng-change="changeSamlSigKeyNameTranformer()"
+                                ng-model="samlXmlKeyNameTranformer"
+                                ng-options="alg for alg in xmlKeyNameTranformers">
+                        </select>
+                    </div>
+                </div>
+                <kc-tooltip>{{:: 'saml-signature-keyName-transformer.tooltip' | translate}}</kc-tooltip>
+            </div>
             <div class="form-group" data-ng-show="(samlAssertionSignature || samlServerSignature) && protocol == 'saml'">
                 <label class="col-md-2 control-label" for="canonicalization">{{:: 'canonicalization-method' | translate}}</label>
                 <div class="col-sm-6">

themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html

@@ -161,6 +161,18 @@
                 </div>
                 <kc-tooltip>{{:: 'signature-algorithm.tooltip' | translate}}</kc-tooltip>
             </div>
+            <div class="form-group clearfix block" data-ng-show="identityProvider.config.wantAuthnRequestsSigned == 'true'">
+                <label class="col-md-2 control-label" for="samlSigKeyNameTranformer">{{:: 'saml-signature-keyName-transformer' | translate}}</label>
+                <div class="col-md-6">
+                    <div>
+                        <select class="form-control" id="samlSigKeyNameTranformer"
+                                ng-model="identityProvider.config.xmlSigKeyInfoKeyNameTransformer"
+                                ng-options="xmlKeyNameTranformer for xmlKeyNameTranformer in xmlKeyNameTranformers">
+                        </select>
+                    </div>
+                </div>
+                <kc-tooltip>{{:: 'saml-signature-keyName-transformer.tooltip' | translate}}</kc-tooltip>
+            </div>
             <div class="form-group">
                 <label class="col-md-2 control-label" for="forceAuthn">{{:: 'force-authentication' | translate}}</label>
                 <div class="col-md-6">