From 5006fe2292afeb4dcef11f7e57777379337a09c1 Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Mon, 12 Dec 2016 22:29:01 +0100 Subject: [PATCH] KEYCLOAK-4062 - GUI changes for KeyName format + few tests --- .../AbstractSamlAuthenticationHandler.java | 6 +- .../AbstractSAMLServletsAdapterTest.java | 55 +++++++++++++++++-- .../messages/admin-messages_en.properties | 2 + .../admin/resources/js/controllers/clients.js | 17 ++++++ .../admin/resources/js/controllers/realm.js | 6 ++ .../resources/partials/client-detail.html | 13 +++++ .../realm-identity-provider-saml.html | 12 ++++ 7 files changed, 102 insertions(+), 9 deletions(-) diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java index 429d610dd4..1bbdf6dae5 100644 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java +++ b/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) { } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java index 5daec87c20..cf84a0dd12 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java +++ b/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); - String privateKeyPem = PemUtils.encodeKey(keyPair.getPrivate()); - PublicKey publicKey = keyPair.getPublic(); + PublicKey publicKey = NEW_KEY_PAIR.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); 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 173bcfbd91..18ffe2a392 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 @@ -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 diff --git a/themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js b/themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js index 84e25207aa..3997ce1d67 100755 --- a/themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js +++ b/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; diff --git a/themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js b/themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js index e451339015..300df381e2 100644 --- a/themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js +++ b/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]; } } diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/client-detail.html b/themes/src/main/resources/theme/base/admin/resources/partials/client-detail.html index 6b0f0a6a47..2dc08ea388 100755 --- a/themes/src/main/resources/theme/base/admin/resources/partials/client-detail.html +++ b/themes/src/main/resources/theme/base/admin/resources/partials/client-detail.html @@ -158,6 +158,19 @@ {{:: 'signature-algorithm.tooltip' | translate}} +
+ +
+
+ +
+
+ {{:: 'saml-signature-keyName-transformer.tooltip' | translate}} +
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 49f0e47976..3aad92beb0 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 @@ -161,6 +161,18 @@
{{:: 'signature-algorithm.tooltip' | translate}}
+
+ +
+
+ +
+
+ {{:: 'saml-signature-keyName-transformer.tooltip' | translate}} +