From 1fa93db1b4df7c2133004d4ac96a0134372c7cb5 Mon Sep 17 00:00:00 2001 From: Konstantinos Georgilakis Date: Wed, 27 May 2020 11:06:31 +0300 Subject: [PATCH] KEYCLOAK-14304 Enhance SAML Identity Provider Metadata processing --- .../dom/saml/v2/mdattr/EntityAttributes.java | 61 +++++++++++++++++++ .../dom/saml/v2/metadata/ExtensionsType.java | 17 +++++- .../constants/JBossSAMLURIConstants.java | 2 + .../mdattr/SAMLEntityAttributesParser.java | 46 ++++++++++++++ .../saml/metadata/SAMLExtensionsParser.java | 20 ++++-- .../saml/metadata/SAMLMetadataQNames.java | 3 +- .../models/IdentityProviderModel.java | 11 ++++ .../saml/SAMLIdentityProviderConfig.java | 9 +++ .../saml/SAMLIdentityProviderFactory.java | 33 ++++++++-- .../testsuite/admin/IdentityProviderTest.java | 60 ++++++++++++++---- .../admin-test/saml-idp-metadata-disabled.xml | 38 ++++++++++++ .../saml-idp-metadata-two-signing-certs.xml | 9 +++ .../admin-test/saml-idp-metadata.xml | 9 +++ .../admin/resources/js/controllers/realm.js | 5 ++ 14 files changed, 295 insertions(+), 28 deletions(-) create mode 100644 saml-core-api/src/main/java/org/keycloak/dom/saml/v2/mdattr/EntityAttributes.java create mode 100644 saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/mdattr/SAMLEntityAttributesParser.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/resources/admin-test/saml-idp-metadata-disabled.xml diff --git a/saml-core-api/src/main/java/org/keycloak/dom/saml/v2/mdattr/EntityAttributes.java b/saml-core-api/src/main/java/org/keycloak/dom/saml/v2/mdattr/EntityAttributes.java new file mode 100644 index 00000000000..45de268d416 --- /dev/null +++ b/saml-core-api/src/main/java/org/keycloak/dom/saml/v2/mdattr/EntityAttributes.java @@ -0,0 +1,61 @@ +package org.keycloak.dom.saml.v2.mdattr; + +import java.io.Serializable; +import java.util.ArrayList; +import java.util.List; + +import org.keycloak.dom.saml.v2.assertion.AssertionType; +import org.keycloak.dom.saml.v2.assertion.AttributeType; + +/** + * + * * + *

+ * Java class for EntityAttributes complex type. + * + *

+ * The following schema fragment specifies the expected content contained within this class. + * + * 

+* 	<element name="EntityAttributes" type="mdattr:EntityAttributesType"/>
+* 	<complexType name="EntityAttributesType">
+* 		<choice maxOccurs="unbounded">
+* 			<element ref="saml:Attribute"/>
+* 			<element ref="saml:Assertion"/>
+* 		</sequence>
+* 	</complexType>
+ *
+ *
+ * + */ + +public class EntityAttributes implements Serializable { + + protected List attribute = new ArrayList<>(); + protected List assertion = new ArrayList<>(); + + public List getAttribute() { + return attribute; + } + + public void addAttribute(AttributeType attributeType) { + attribute.add(attributeType); + } + + public void removeAttribute(AttributeType attributeType) { + attribute.remove(attributeType); + } + + public List getAssertion() { + return assertion; + } + + public void addAssertion(AssertionType attributeType) { + assertion.add(attributeType); + } + + public void removeAssertion(AttributeType attributeType) { + assertion.remove(attributeType); + } + +} diff --git a/saml-core-api/src/main/java/org/keycloak/dom/saml/v2/metadata/ExtensionsType.java b/saml-core-api/src/main/java/org/keycloak/dom/saml/v2/metadata/ExtensionsType.java index 57f926528cb..38b08c131c6 100755 --- a/saml-core-api/src/main/java/org/keycloak/dom/saml/v2/metadata/ExtensionsType.java +++ b/saml-core-api/src/main/java/org/keycloak/dom/saml/v2/metadata/ExtensionsType.java @@ -16,12 +16,13 @@ */ package org.keycloak.dom.saml.v2.metadata; -import org.w3c.dom.Element; - import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.keycloak.dom.saml.v2.mdattr.EntityAttributes; +import org.w3c.dom.Element; + /** *

* Java class for ExtensionsType complex type. @@ -88,4 +89,14 @@ public class ExtensionsType { public List getAny() { return Collections.unmodifiableList(this.any); } -} \ No newline at end of file + + public EntityAttributes getEntityAttributes() { + for (Object o : this.any) { + if (o instanceof EntityAttributes) { + return (EntityAttributes) o; + } + } + return null; + } + +} diff --git a/saml-core-api/src/main/java/org/keycloak/saml/common/constants/JBossSAMLURIConstants.java b/saml-core-api/src/main/java/org/keycloak/saml/common/constants/JBossSAMLURIConstants.java index 988fe36b291..fcb317f535f 100755 --- a/saml-core-api/src/main/java/org/keycloak/saml/common/constants/JBossSAMLURIConstants.java +++ b/saml-core-api/src/main/java/org/keycloak/saml/common/constants/JBossSAMLURIConstants.java @@ -65,6 +65,8 @@ public enum JBossSAMLURIConstants { HOLDER_OF_KEY("urn:oasis:names:tc:SAML:2.0:cm:holder-of-key"), METADATA_NSURI("urn:oasis:names:tc:SAML:2.0:metadata"), + // http://docs.oasis-open.org/security/saml/Post2.0/sstc-metadata-attr-cd-01.pdf + METADATA_ENTITY_ATTRIBUTES_NSURI("urn:oasis:names:tc:SAML:metadata:attribute"), NAMEID_FORMAT_TRANSIENT("urn:oasis:names:tc:SAML:2.0:nameid-format:transient"), NAMEID_FORMAT_PERSISTENT("urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"), diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/mdattr/SAMLEntityAttributesParser.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/mdattr/SAMLEntityAttributesParser.java new file mode 100644 index 00000000000..dabafddddbe --- /dev/null +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/mdattr/SAMLEntityAttributesParser.java @@ -0,0 +1,46 @@ +package org.keycloak.saml.processing.core.parsers.saml.mdattr; + +import java.io.Serializable; + +import javax.xml.stream.XMLEventReader; +import javax.xml.stream.events.StartElement; + +import org.keycloak.dom.saml.v2.mdattr.EntityAttributes; +import org.keycloak.saml.common.exceptions.ParsingException; +import org.keycloak.saml.common.util.StaxParserUtil; +import org.keycloak.saml.processing.core.parsers.saml.assertion.SAMLAssertionParser; +import org.keycloak.saml.processing.core.parsers.saml.assertion.SAMLAttributeParser; +import org.keycloak.saml.processing.core.parsers.saml.metadata.AbstractStaxSamlMetadataParser; +import org.keycloak.saml.processing.core.parsers.saml.metadata.SAMLMetadataQNames; + +public class SAMLEntityAttributesParser extends AbstractStaxSamlMetadataParser implements Serializable { + private static final SAMLEntityAttributesParser INSTANCE = new SAMLEntityAttributesParser(); + + private SAMLEntityAttributesParser() { + super(SAMLMetadataQNames.ENTITY_ATTRIBUTES); + } + + public static SAMLEntityAttributesParser getInstance() { + return INSTANCE; + } + + @Override + protected EntityAttributes instantiateElement(XMLEventReader xmlEventReader, StartElement element) throws ParsingException { + return new EntityAttributes(); + } + + @Override + protected void processSubElement(XMLEventReader xmlEventReader, EntityAttributes target, SAMLMetadataQNames element, + StartElement elementDetail) throws ParsingException { + switch (element) { + case ATTRIBUTE: + target.addAttribute(SAMLAttributeParser.getInstance().parse(xmlEventReader)); + break; + case ASSERTION: + target.addAssertion(SAMLAssertionParser.getInstance().parse(xmlEventReader)); + break; + default: + throw LOGGER.parserUnknownTag(StaxParserUtil.getElementName(elementDetail), elementDetail.getLocation()); + } + } +} \ No newline at end of file diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/metadata/SAMLExtensionsParser.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/metadata/SAMLExtensionsParser.java index 4dc8ab53fd5..0c2d79bf6d3 100644 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/metadata/SAMLExtensionsParser.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/metadata/SAMLExtensionsParser.java @@ -16,12 +16,13 @@ */ package org.keycloak.saml.processing.core.parsers.saml.metadata; +import javax.xml.stream.XMLEventReader; +import javax.xml.stream.events.StartElement; + import org.keycloak.dom.saml.v2.metadata.ExtensionsType; import org.keycloak.saml.common.exceptions.ParsingException; import org.keycloak.saml.common.util.StaxParserUtil; - -import javax.xml.stream.XMLEventReader; -import javax.xml.stream.events.StartElement; +import org.keycloak.saml.processing.core.parsers.saml.mdattr.SAMLEntityAttributesParser; /** * Parses <samlp:Extensions> SAML2 element into series of DOM nodes. @@ -46,7 +47,16 @@ public class SAMLExtensionsParser extends AbstractStaxSamlMetadataParser config) { + private IdentityProviderRepresentation createRep(String id, String providerId,boolean enabled, Map config) { IdentityProviderRepresentation idp = new IdentityProviderRepresentation(); idp.setAlias(id); idp.setDisplayName(id); idp.setProviderId(providerId); - idp.setEnabled(true); + idp.setEnabled(enabled); if (config != null) { idp.setConfig(config); } @@ -603,14 +605,14 @@ public class IdentityProviderTest extends AbstractAdminTest { form.addFormData("file", body, MediaType.APPLICATION_XML_TYPE, "saml-idp-metadata.xml"); Map result = realm.identityProviders().importFrom(form); - assertSamlImport(result, SIGNING_CERT_1); + assertSamlImport(result, SIGNING_CERT_1,true); // Create new SAML identity provider using configuration retrieved from import-config - create(createRep("saml", "saml", result)); + create(createRep("saml", "saml",true, result)); IdentityProviderResource provider = realm.identityProviders().get("saml"); IdentityProviderRepresentation rep = provider.toRepresentation(); - assertCreatedSamlIdp(rep); + assertCreatedSamlIdp(rep,true); // Now list the providers - we should see the one just created List providers = realm.identityProviders().findAll(); @@ -626,6 +628,32 @@ public class IdentityProviderTest extends AbstractAdminTest { assertSamlExport(body); } + + @Test + public void testSamlImportAndExportDisabled() throws URISyntaxException, IOException, ParsingException { + + // Use import-config to convert IDPSSODescriptor file into key value pairs + // to use when creating a SAML Identity Provider + MultipartFormDataOutput form = new MultipartFormDataOutput(); + form.addFormData("providerId", "saml", MediaType.TEXT_PLAIN_TYPE); + + URL idpMeta = getClass().getClassLoader().getResource("admin-test/saml-idp-metadata-disabled.xml"); + byte[] content = Files.readAllBytes(Paths.get(idpMeta.toURI())); + String body = new String(content, Charset.forName("utf-8")); + form.addFormData("file", body, MediaType.APPLICATION_XML_TYPE, "saml-idp-metadata-disabled.xml"); + + Map result = realm.identityProviders().importFrom(form); + assertSamlImport(result, SIGNING_CERT_1, false); + + // Create new SAML identity provider using configuration retrieved from import-config + create(createRep("saml", "saml", false, result)); + + IdentityProviderResource provider = realm.identityProviders().get("saml"); + IdentityProviderRepresentation rep = provider.toRepresentation(); + assertCreatedSamlIdp(rep, false); + + } + @Test public void testSamlImportAndExportMultipleSigningKeys() throws URISyntaxException, IOException, ParsingException { @@ -641,14 +669,14 @@ public class IdentityProviderTest extends AbstractAdminTest { form.addFormData("file", body, MediaType.APPLICATION_XML_TYPE, "saml-idp-metadata-two-signing-certs"); Map result = realm.identityProviders().importFrom(form); - assertSamlImport(result, SIGNING_CERT_1 + "," + SIGNING_CERT_2); + assertSamlImport(result, SIGNING_CERT_1 + "," + SIGNING_CERT_2,true); // Create new SAML identity provider using configuration retrieved from import-config - create(createRep("saml", "saml", result)); + create(createRep("saml", "saml",true, result)); IdentityProviderResource provider = realm.identityProviders().get("saml"); IdentityProviderRepresentation rep = provider.toRepresentation(); - assertCreatedSamlIdp(rep); + assertCreatedSamlIdp(rep,true); // Now list the providers - we should see the one just created List providers = realm.identityProviders().findAll(); @@ -863,13 +891,13 @@ public class IdentityProviderTest extends AbstractAdminTest { Assert.assertEquals("config", expected.getConfig(), actual.getConfig()); } - private void assertCreatedSamlIdp(IdentityProviderRepresentation idp) { + private void assertCreatedSamlIdp(IdentityProviderRepresentation idp,boolean enabled) { //System.out.println("idp: " + idp); Assert.assertNotNull("IdentityProviderRepresentation not null", idp); Assert.assertNotNull("internalId", idp.getInternalId()); Assert.assertEquals("alias", "saml", idp.getAlias()); Assert.assertEquals("providerId", "saml", idp.getProviderId()); - Assert.assertTrue("enabled", idp.isEnabled()); + Assert.assertEquals("enabled",enabled, idp.isEnabled()); Assert.assertEquals("firstBrokerLoginFlowAlias", "first broker login",idp.getFirstBrokerLoginFlowAlias()); assertSamlConfig(idp.getConfig()); } @@ -889,7 +917,8 @@ public class IdentityProviderTest extends AbstractAdminTest { "nameIDPolicyFormat", "signingCertificate", "addExtensionsElementWithKeyInfo", - "loginHint" + "loginHint", + "hideOnLoginPage" )); assertThat(config, hasEntry("validateSignature", "true")); assertThat(config, hasEntry("singleLogoutServiceUrl", "http://localhost:8080/auth/realms/master/protocol/saml")); @@ -899,10 +928,15 @@ public class IdentityProviderTest extends AbstractAdminTest { assertThat(config, hasEntry("wantAuthnRequestsSigned", "true")); assertThat(config, hasEntry("addExtensionsElementWithKeyInfo", "false")); assertThat(config, hasEntry("nameIDPolicyFormat", "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent")); + assertThat(config, hasEntry("hideOnLoginPage", "true")); assertThat(config, hasEntry(is("signingCertificate"), notNullValue())); } - private void assertSamlImport(Map config, String expectedSigningCertificates) { + private void assertSamlImport(Map config, String expectedSigningCertificates,boolean enabled) { + //firtsly check and remove enabledFromMetadata from config + boolean enabledFromMetadata = Boolean.valueOf(config.get(SAMLIdentityProviderConfig.ENABLED_FROM_METADATA)); + config.remove(SAMLIdentityProviderConfig.ENABLED_FROM_METADATA); + Assert.assertEquals(enabledFromMetadata,enabled); assertSamlConfig(config); assertThat(config, hasEntry("signingCertificate", expectedSigningCertificates)); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/admin-test/saml-idp-metadata-disabled.xml b/testsuite/integration-arquillian/tests/base/src/test/resources/admin-test/saml-idp-metadata-disabled.xml new file mode 100644 index 00000000000..963107282d6 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/admin-test/saml-idp-metadata-disabled.xml @@ -0,0 +1,38 @@ + + + + + + http://refeds.org/category/hide-from-discovery + + + + + urn:oasis:names:tc:SAML:2.0:nameid-format:persistent + urn:oasis:names:tc:SAML:2.0:nameid-format:transient + urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified + urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress + + + + + + + + MIICmzCCAYMCBgFUYnC0OjANBgkqhkiG9w0BAQsFADARMQ8wDQYDVQQDDAZtYXN0ZXIwHhcNMTYwNDI5MTQzMjEzWhcNMjYwNDI5MTQzMzUzWjARMQ8wDQYDVQQDDAZtYXN0ZXIwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCN25AW1poMEZRbuMAHG58AThZmCwMV6/Gcui4mjGacRFyudgqzLjQ2rxpoW41JAtLjbjeAhuWvirUcFVcOeS3gM/ZC27qCpYighAcylZz6MYocnEe1+e8rPPk4JlID6Wv62dgu+pL/vYsQpRhvD3Y2c/ytgr5D32xF+KnzDehUy5BSyzypvu12Wq9mS5vK5tzkN37EjkhpY2ZxaXPubjDIITCAL4Q8M/m5IlacBaUZbzI4AQrHnMP1O1IH2dHSWuMiBe+xSDTco72PmuYPJKTV4wQdeBUIkYbfLc4RxVmXEvgkQgyW86EoMPxlWJpj7+mTIR+l+2thZPr/VgwTs82rAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAA/Ip/Hi8RoVu5ouaFFlc5whT7ltuK8slfLGW4tM4vJXhInYwsqIRQKBNDYW/64xle3eII4u1yAH1OYRRwEs7Em1pr4QuFuTY1at+aE0sE46XDlyESI0txJjWxYoT133vM0We2pj1b2nxgU30rwjKA3whnKEfTEYT/n3JBSqNggy6l8ZGw/oPSgvPaR4+xeB1tfQFC4VrLoYKoqH6hAL530nKxL+qV8AIfL64NDEE8ankIAEDAAFe8x3CPUfXR/p4KOANKkpz8ieQaHDb1eITkAwUwjESj6UF9D1aePlhWls/HX0gujFXtWfWfrJ8CU/ogwlH8y1jgRuLjFQYZk6llc= + + + + + + diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/admin-test/saml-idp-metadata-two-signing-certs.xml b/testsuite/integration-arquillian/tests/base/src/test/resources/admin-test/saml-idp-metadata-two-signing-certs.xml index 5b5f8f68c65..74618cf035c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/admin-test/saml-idp-metadata-two-signing-certs.xml +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/admin-test/saml-idp-metadata-two-signing-certs.xml @@ -1,8 +1,17 @@ + + + + http://refeds.org/category/hide-from-discovery + + + diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/admin-test/saml-idp-metadata.xml b/testsuite/integration-arquillian/tests/base/src/test/resources/admin-test/saml-idp-metadata.xml index 13681c1b1c5..a44e816a37a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/admin-test/saml-idp-metadata.xml +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/admin-test/saml-idp-metadata.xml @@ -1,8 +1,17 @@ + + + + http://refeds.org/category/hide-from-discovery + + + 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 d7f3fbbfbc2..83e7799020d 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 @@ -962,9 +962,14 @@ module.controller('RealmIdentityProviderCtrl', function($scope, $filter, $upload }; var setConfig = function(data) { + if (data["enabledFromMetadata"] !== undefined ) { + $scope.identityProvider.enabled = data["enabledFromMetadata"] == "true"; + delete data["enabledFromMetadata"]; + } for (var key in data) { $scope.identityProvider.config[key] = data[key]; } + } $scope.uploadFile = function() {