Skip to content

Commit

Permalink
Remove the ability to configure SAML metadata from file
Browse files Browse the repository at this point in the history 
Validate metadata XML for valid XML, not containing <!DOCTYPE> tag
Validate metadata URL for valid URL
https://www.pivotaltracker.com/story/show/106903904
[#106903904]
  • Loading branch information
@fhanik
fhanik committed Oct 30, 2015
1 parent f5ca839 commit 17ca866
Show file tree
Hide file tree
Showing 10 changed files with 339 additions and 154 deletions.
30 changes: 5 additions & 25 deletions .../main/java/org/cloudfoundry/identity/uaa/login/saml/SamlIdentityProviderConfigurator.java
View file
Expand Up @@ -19,7 +19,6 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.http.client.utils.URIBuilder;
import org.cloudfoundry.identity.uaa.login.util.FileLocator;
import org.cloudfoundry.identity.uaa.zone.IdentityZone;
import org.opensaml.saml2.metadata.provider.MetadataProviderException;
import org.opensaml.xml.parse.BasicParserPool;
Expand All @@ -28,8 +27,6 @@
import org.springframework.security.saml.metadata.ExtendedMetadataDelegate;
import org.springframework.util.StringUtils;

import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
Expand All @@ -46,8 +43,8 @@
import java.util.TimerTask;

import static org.cloudfoundry.identity.uaa.AbstractIdentityProviderDefinition.EMAIL_DOMAIN_ATTR;
import static org.cloudfoundry.identity.uaa.ExternalIdentityProviderDefinition.EXTERNAL_GROUPS_WHITELIST;
import static org.cloudfoundry.identity.uaa.ExternalIdentityProviderDefinition.ATTRIBUTE_MAPPINGS;
import static org.cloudfoundry.identity.uaa.ExternalIdentityProviderDefinition.EXTERNAL_GROUPS_WHITELIST;

public class SamlIdentityProviderConfigurator implements InitializingBean {
private static Log logger = LogFactory.getLog(SamlIdentityProviderConfigurator.class);
Expand Down Expand Up @@ -251,10 +248,6 @@ public ExtendedMetadataDelegate getExtendedMetadataDelegate(SamlIdentityProvider
metadata = configureXMLMetadata(def);
break;
}
case FILE: {
metadata = configureFileMetadata(def);
break;
}
case URL: {
metadata = configureURLMetadata(def);
break;
Expand All @@ -278,19 +271,6 @@ protected ExtendedMetadataDelegate configureXMLMetadata(SamlIdentityProviderDefi
return delegate;
}

protected ExtendedMetadataDelegate configureFileMetadata(SamlIdentityProviderDefinition def) throws MetadataProviderException {
try {
def = def.clone();
File metadataFile = FileLocator.locate(def.getMetaDataLocation());
FilesystemMetadataProvider filesystemMetadataProvider = new FilesystemMetadataProvider(dummyTimer, metadataFile);
byte[] metadata = filesystemMetadataProvider.fetchMetadata();
def.setMetaDataLocation(new String(metadata, StandardCharsets.UTF_8));
return configureXMLMetadata(def);
} catch (IOException e) {
throw new IllegalArgumentException("Invalid metadata file for alias["+def.getIdpEntityAlias()+"]:"+def.getMetaDataLocation());
}
}

protected ExtendedMetadataDelegate configureURLMetadata(SamlIdentityProviderDefinition def) throws MetadataProviderException {
Class<ProtocolSocketFactory> socketFactory = null;
try {
Expand All @@ -307,13 +287,13 @@ protected ExtendedMetadataDelegate configureURLMetadata(SamlIdentityProviderDefi
def.setMetaDataLocation(new String(metadata, StandardCharsets.UTF_8));
return configureXMLMetadata(def);
} catch (URISyntaxException e) {
throw new IllegalArgumentException("Invalid socket factory(invalid URI):"+def.getMetaDataLocation(), e);
throw new MetadataProviderException("Invalid socket factory(invalid URI):"+def.getMetaDataLocation(), e);
} catch (ClassNotFoundException e) {
throw new IllegalArgumentException("Invalid socket factory:"+def.getSocketFactoryClassName(), e);
throw new MetadataProviderException("Invalid socket factory:"+def.getSocketFactoryClassName(), e);
} catch (InstantiationException e) {
throw new IllegalArgumentException("Invalid socket factory:"+def.getSocketFactoryClassName(), e);
throw new MetadataProviderException("Invalid socket factory:"+def.getSocketFactoryClassName(), e);
} catch (IllegalAccessException e) {
throw new IllegalArgumentException("Invalid socket factory:"+def.getSocketFactoryClassName(), e);
throw new MetadataProviderException("Invalid socket factory:"+def.getSocketFactoryClassName(), e);
}
}

Expand Down
53 changes: 37 additions & 16 deletions ...rc/main/java/org/cloudfoundry/identity/uaa/login/saml/SamlIdentityProviderDefinition.java
View file
Expand Up @@ -14,13 +14,19 @@

import com.fasterxml.jackson.annotation.JsonIgnore;
import org.cloudfoundry.identity.uaa.ExternalIdentityProviderDefinition;
import org.cloudfoundry.identity.uaa.login.util.FileLocator;
import org.opensaml.saml2.metadata.provider.MetadataProviderException;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;

import java.io.File;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import java.io.IOException;
import java.io.StringReader;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

Expand All @@ -31,7 +37,6 @@ public class SamlIdentityProviderDefinition extends ExternalIdentityProviderDefi

public enum MetadataLocation {
URL,
FILE,
DATA,
UNKNOWN
};
Expand Down Expand Up @@ -87,19 +92,37 @@ public MetadataLocation getType() {
if (trimmedLocation.startsWith("<?xml") ||
trimmedLocation.startsWith("<md:EntityDescriptor") ||
trimmedLocation.startsWith("<EntityDescriptor")) {
return MetadataLocation.DATA;
try {
validateXml(trimmedLocation);
return MetadataLocation.DATA;
} catch (MetadataProviderException x) {
//invalid XML
}
} else if (trimmedLocation.startsWith("http")) {
return MetadataLocation.URL;
} else {
try {
File f = FileLocator.locate(metaDataLocation);
if (f.exists() && f.canRead()) {
return MetadataLocation.FILE;
}
} catch (IOException x) {
//file not found
URL uri = new URL(trimmedLocation);
return MetadataLocation.URL;
} catch (MalformedURLException e) {
//invalid URL
}
return MetadataLocation.UNKNOWN;
}
return MetadataLocation.UNKNOWN;
}

protected void validateXml(String xml) throws MetadataProviderException {
if (xml==null || xml.toUpperCase().contains("<!DOCTYPE")) {
throw new MetadataProviderException("Invalid metadata XML contents:"+xml);
}
try {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
DocumentBuilder builder = factory.newDocumentBuilder();
builder.parse(new InputSource(new StringReader(xml)));
} catch (ParserConfigurationException e) {
throw new MetadataProviderException("Unable to create document parser.", e);
} catch (SAXException e) {
throw new MetadataProviderException("Sax Parsing exception of XML:"+xml, e);
} catch (IOException e) {
throw new MetadataProviderException("IOException of XML:"+xml, e);
}
}

Expand Down Expand Up @@ -155,8 +178,6 @@ public String getSocketFactoryClassName() {
if (socketFactoryClassName!=null && socketFactoryClassName.trim().length()>0) {
return socketFactoryClassName;
}


if (getMetaDataLocation()==null || getMetaDataLocation().trim().length()==0) {
throw new IllegalStateException("Invalid meta data URL[" + getMetaDataLocation() + "] cannot determine socket factory.");
}
Expand Down
53 changes: 46 additions & 7 deletions ...test/java/org/cloudfoundry/identity/uaa/login/saml/IdentityProviderConfiguratorTests.java
View file
Expand Up @@ -42,7 +42,6 @@
import java.util.UUID;

import static java.util.Arrays.asList;
import static java.util.Collections.singletonMap;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
Expand All @@ -53,6 +52,46 @@

public class IdentityProviderConfiguratorTests {

public static final String testXmlFileData = "<?xml version=\"1.0\" encoding=\"UTF-8\"?><md:EntityDescriptor xmlns:md=\"urn:oasis:names:tc:SAML:2.0:metadata\" entityID=\"http://www.okta.com/k2lvtem0VAJDMINKEYJW\"><md:IDPSSODescriptor WantAuthnRequestsSigned=\"true\" protocolSupportEnumeration=\"urn:oasis:names:tc:SAML:2.0:protocol\"><md:KeyDescriptor use=\"signing\"><ds:KeyInfo xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\"><ds:X509Data><ds:X509Certificate>MIICmTCCAgKgAwIBAgIGAUPATqmEMA0GCSqGSIb3DQEBBQUAMIGPMQswCQYDVQQGEwJVUzETMBEG\n" +
" A1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNU2FuIEZyYW5jaXNjbzENMAsGA1UECgwET2t0YTEU\n" +
" MBIGA1UECwwLU1NPUHJvdmlkZXIxEDAOBgNVBAMMB1Bpdm90YWwxHDAaBgkqhkiG9w0BCQEWDWlu\n" +
" Zm9Ab2t0YS5jb20wHhcNMTQwMTIzMTgxMjM3WhcNNDQwMTIzMTgxMzM3WjCBjzELMAkGA1UEBhMC\n" +
" VVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDTALBgNVBAoM\n" +
" BE9rdGExFDASBgNVBAsMC1NTT1Byb3ZpZGVyMRAwDgYDVQQDDAdQaXZvdGFsMRwwGgYJKoZIhvcN\n" +
" AQkBFg1pbmZvQG9rdGEuY29tMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCeil67/TLOiTZU\n" +
" WWgW2XEGgFZ94bVO90v5J1XmcHMwL8v5Z/8qjdZLpGdwI7Ph0CyXMMNklpaR/Ljb8fsls3amdT5O\n" +
" Bw92Zo8ulcpjw2wuezTwL0eC0wY/GQDAZiXL59npE6U+fH1lbJIq92hx0HJSru/0O1q3+A/+jjZL\n" +
" 3tL/SwIDAQABMA0GCSqGSIb3DQEBBQUAA4GBAI5BoWZoH6Mz9vhypZPOJCEKa/K+biZQsA4Zqsuk\n" +
" vvphhSERhqk/Nv76Vkl8uvJwwHbQrR9KJx4L3PRkGCG24rix71jEuXVGZUsDNM3CUKnARx4MEab6\n" +
" GFHNkZ6DmoT/PFagngecHu+EwmuDtaG0rEkFrARwe+d8Ru0BN558abFb</ds:X509Certificate></ds:X509Data></ds:KeyInfo></md:KeyDescriptor><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</md:NameIDFormat><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</md:NameIDFormat><md:SingleSignOnService Binding=\"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST\" Location=\"https://pivotal.oktapreview.com/app/pivotal_pivotalcfdevelopment_1/k2lvtem0VAJDMINKEYJW/sso/saml\"/><md:SingleSignOnService Binding=\"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect\" Location=\"https://pivotal.oktapreview.com/app/pivotal_pivotalcfdevelopment_1/k2lvtem0VAJDMINKEYJW/sso/saml\"/></md:IDPSSODescriptor></md:EntityDescriptor>";

public static final String testXmlFileData2 = "<?xml version=\"1.0\" encoding=\"UTF-8\"?><!--\n" +
" ~ ******************************************************************************\n" +
" ~ Cloud Foundry\n" +
" ~ Copyright (c) [2009-2015] Pivotal Software, Inc. All Rights Reserved.\n" +
" ~ This product is licensed to you under the Apache License, Version 2.0 (the \"License\").\n" +
" ~ You may not use this product except in compliance with the License.\n" +
" ~\n" +
" ~ This product includes a number of subcomponents with\n" +
" ~ separate copyright notices and license terms. Your use of these\n" +
" ~ subcomponents is subject to the terms and conditions of the\n" +
" ~ subcomponent's license, as noted in the LICENSE file.\n" +
" ~ ******************************************************************************\n" +
" -->\n" +
"\n" +
"<md:EntityDescriptor xmlns:md=\"urn:oasis:names:tc:SAML:2.0:metadata\" entityID=\"http://www.okta.com/k2lvtem0VAJDMINKEYJX\"><md:IDPSSODescriptor WantAuthnRequestsSigned=\"true\" protocolSupportEnumeration=\"urn:oasis:names:tc:SAML:2.0:protocol\"><md:KeyDescriptor use=\"signing\"><ds:KeyInfo xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\"><ds:X509Data><ds:X509Certificate>MIICmTCCAgKgAwIBAgIGAUPATqmEMA0GCSqGSIb3DQEBBQUAMIGPMQswCQYDVQQGEwJVUzETMBEG\n" +
" A1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNU2FuIEZyYW5jaXNjbzENMAsGA1UECgwET2t0YTEU\n" +
" MBIGA1UECwwLU1NPUHJvdmlkZXIxEDAOBgNVBAMMB1Bpdm90YWwxHDAaBgkqhkiG9w0BCQEWDWlu\n" +
" Zm9Ab2t0YS5jb20wHhcNMTQwMTIzMTgxMjM3WhcNNDQwMTIzMTgxMzM3WjCBjzELMAkGA1UEBhMC\n" +
" VVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDTALBgNVBAoM\n" +
" BE9rdGExFDASBgNVBAsMC1NTT1Byb3ZpZGVyMRAwDgYDVQQDDAdQaXZvdGFsMRwwGgYJKoZIhvcN\n" +
" AQkBFg1pbmZvQG9rdGEuY29tMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCeil67/TLOiTZU\n" +
" WWgW2XEGgFZ94bVO90v5J1XmcHMwL8v5Z/8qjdZLpGdwI7Ph0CyXMMNklpaR/Ljb8fsls3amdT5O\n" +
" Bw92Zo8ulcpjw2wuezTwL0eC0wY/GQDAZiXL59npE6U+fH1lbJIq92hx0HJSru/0O1q3+A/+jjZL\n" +
" 3tL/SwIDAQABMA0GCSqGSIb3DQEBBQUAA4GBAI5BoWZoH6Mz9vhypZPOJCEKa/K+biZQsA4Zqsuk\n" +
" vvphhSERhqk/Nv76Vkl8uvJwwHbQrR9KJx4L3PRkGCG24rix71jEuXVGZUsDNM3CUKnARx4MEab6\n" +
" GFHNkZ6DmoT/PFagngecHu+EwmuDtaG0rEkFrARwe+d8Ru0BN558abFb</ds:X509Certificate></ds:X509Data></ds:KeyInfo></md:KeyDescriptor><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</md:NameIDFormat><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</md:NameIDFormat><md:SingleSignOnService Binding=\"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST\" Location=\"https://pivotal.oktapreview.com/app/pivotal_pivotalcfdevelopment_1/k2lvtem0VAJDMINKEYJW/sso/saml\"/><md:SingleSignOnService Binding=\"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect\" Location=\"https://pivotal.oktapreview.com/app/pivotal_pivotalcfdevelopment_1/k2lvtem0VAJDMINKEYJW/sso/saml\"/></md:IDPSSODescriptor></md:EntityDescriptor>";

@BeforeClass
public static void initializeOpenSAML() throws Exception {
if (!org.apache.xml.security.Init.isInitialized()) {
Expand Down Expand Up @@ -84,7 +123,8 @@ public static void initializeOpenSAML() throws Exception {

public static String sampleYaml = " providers:\n" +
" okta-local:\n" +
" idpMetadata: test-file-metadata.xml\n" +
" idpMetadata: |\n" +
" " + testXmlFileData.replace("\n","\n ") + "\n"+
" nameID: urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress\n" +
" assertionConsumerIndex: 0\n" +
" metadataTrustCheck: true\n" +
Expand Down Expand Up @@ -332,8 +372,8 @@ protected void testGetIdentityProviderDefinitions(int count, boolean addData) th
for (SamlIdentityProviderDefinition idp : idps) {
switch (idp.getIdpEntityAlias()) {
case "okta-local" : {
assertEquals(SamlIdentityProviderDefinition.MetadataLocation.FILE, idp.getType());
assertEquals("test-file-metadata.xml", idp.getMetaDataLocation());
assertEquals(SamlIdentityProviderDefinition.MetadataLocation.DATA, idp.getType());
assertEquals(testXmlFileData.trim(), idp.getMetaDataLocation().trim());
assertEquals("urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", idp.getNameID());
assertEquals(0, idp.getAssertionConsumerIndex());
assertEquals("Okta Preview 1", idp.getLinkText());
Expand All @@ -359,7 +399,7 @@ protected void testGetIdentityProviderDefinitions(int count, boolean addData) th
break;
}
case "okta-local-3" : {
assertEquals(SamlIdentityProviderDefinition.MetadataLocation.FILE, idp.getType());
assertEquals(SamlIdentityProviderDefinition.MetadataLocation.DATA, idp.getType());
assertEquals("urn:oasis:names:tc:SAML:2.0:nameid-format:persistent", idp.getNameID());
assertEquals(0, idp.getAssertionConsumerIndex());
assertEquals("Use your corporate credentials", idp.getLinkText());
Expand Down Expand Up @@ -393,7 +433,7 @@ public void testGetIdentityProvidersWithLegacy_Invalid_Provider() throws Excepti

@Test
public void testGetIdentityProvidersWithLegacy_Valid_Provider() throws Exception {
conf.setLegacyIdpMetaData("test-file-metadata-2.xml");
conf.setLegacyIdpMetaData(testXmlFileData2);
conf.setLegacyIdpIdentityAlias("okta-local-3");
conf.setLegacyShowSamlLink(true);
conf.setLegacyNameId("urn:oasis:names:tc:SAML:2.0:nameid-format:persistent");
Expand Down Expand Up @@ -512,5 +552,4 @@ public void testSetAddShadowUserOnLoginFromYaml() throws Exception {

}
}

}
71 changes: 0 additions & 71 deletions ...c/test/java/org/cloudfoundry/identity/uaa/login/saml/IdentityProviderDefinitionTests.java
View file

This file was deleted.

0 comments on commit 17ca866

Please sign in to comment.