diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/RestTemplateConfig.java b/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/RestTemplateConfig.java new file mode 100644 index 00000000000..3ff5e75798d --- /dev/null +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/RestTemplateConfig.java @@ -0,0 +1,20 @@ +package org.cloudfoundry.identity.uaa.impl.config; + +import org.cloudfoundry.identity.uaa.util.UaaHttpRequestUtils; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.web.client.RestTemplate; + +@Configuration +public class RestTemplateConfig { + + @Bean + public RestTemplate nonTrustingRestTemplate() { + return new RestTemplate(UaaHttpRequestUtils.createRequestFactory(false, 30_000)); + } + + @Bean + public RestTemplate trustingRestTemplate() { + return new RestTemplate(UaaHttpRequestUtils.createRequestFactory(true, 30_000)); + } +} diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/FixedHttpMetaDataProvider.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/FixedHttpMetaDataProvider.java index ee2b6ac1232..3d1c2eb5a4c 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/FixedHttpMetaDataProvider.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/FixedHttpMetaDataProvider.java @@ -13,57 +13,46 @@ package org.cloudfoundry.identity.uaa.provider.saml; -import org.apache.commons.httpclient.HttpClient; -import org.apache.commons.httpclient.SimpleHttpConnectionManager; -import org.apache.commons.httpclient.params.HttpClientParams; -import org.apache.commons.httpclient.protocol.ProtocolSocketFactory; import org.cloudfoundry.identity.uaa.cache.UrlContentCache; -import org.opensaml.saml2.metadata.provider.HTTPMetadataProvider; import org.opensaml.saml2.metadata.provider.MetadataProviderException; import org.springframework.web.client.RestTemplate; -import java.util.Timer; +import java.net.URI; +import java.net.URISyntaxException; -/** - * This class works around the problem described in http://issues.apache.org/jira/browse/HTTPCLIENT-646 when a socket factory is set - * on the OpenSAML - * {@link HTTPMetadataProvider#setSocketFactory(ProtocolSocketFactory)} all - * subsequent GET Methods should be executed using a relative URL, otherwise the - * HttpClient - * resets the underlying socket factory. - * - * - */ -public class FixedHttpMetaDataProvider extends HTTPMetadataProvider { +public class FixedHttpMetaDataProvider { - private RestTemplate template; + private RestTemplate trustingRestTemplate; + private RestTemplate nonTrustingRestTemplate; private UrlContentCache cache; - public static FixedHttpMetaDataProvider buildProvider(Timer backgroundTaskTimer, - HttpClientParams params, - String metadataURL, - RestTemplate template, - UrlContentCache cache) throws MetadataProviderException { - SimpleHttpConnectionManager connectionManager = new SimpleHttpConnectionManager(true); - connectionManager.getParams().setDefaults(params); - HttpClient client = new HttpClient(connectionManager); - return new FixedHttpMetaDataProvider(backgroundTaskTimer, client, metadataURL, template, cache); + + public byte[] fetchMetadata(String metadataURL, boolean isSkipSSLValidation) throws MetadataProviderException, URISyntaxException { + validateMetadataURL(metadataURL); + + if (isSkipSSLValidation) { + return cache.getUrlContent(metadataURL, trustingRestTemplate); + } + return cache.getUrlContent(metadataURL, nonTrustingRestTemplate); } - private FixedHttpMetaDataProvider(Timer backgroundTaskTimer, - HttpClient client, - String metadataURL, - RestTemplate template, - UrlContentCache cache) throws MetadataProviderException { - super(backgroundTaskTimer, client, metadataURL); - this.template = template; - this.cache = cache; + private void validateMetadataURL(String metadataURL) throws MetadataProviderException { + try { + new URI(metadataURL); + } catch (URISyntaxException e) { + throw new MetadataProviderException("Illegal URL syntax", e); + } } - @Override - public byte[] fetchMetadata() throws MetadataProviderException { - return cache.getUrlContent(getMetadataURI(), template); + public void setTrustingRestTemplate(RestTemplate trustingRestTemplate) { + this.trustingRestTemplate = trustingRestTemplate; } + public void setNonTrustingRestTemplate(RestTemplate nonTrustingRestTemplate) { + this.nonTrustingRestTemplate = nonTrustingRestTemplate; + } + public void setCache(UrlContentCache cache) { + this.cache = cache; + } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/MetadataFetcher.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/MetadataFetcher.java deleted file mode 100644 index c8ad1aae45b..00000000000 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/MetadataFetcher.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.cloudfoundry.identity.uaa.provider.saml; - -import org.opensaml.saml2.metadata.provider.MetadataProviderException; - -public class MetadataFetcher { - public byte[] fetch(FixedHttpMetaDataProvider provider) throws MetadataProviderException { - return provider.fetchMetadata(); - } -} diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfigurator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfigurator.java index 5d94233396d..59f9a6fc56d 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfigurator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfigurator.java @@ -12,16 +12,11 @@ *******************************************************************************/ package org.cloudfoundry.identity.uaa.provider.saml; -import org.apache.commons.httpclient.params.HttpClientParams; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.apache.http.client.utils.URIBuilder; -import org.cloudfoundry.identity.uaa.cache.UrlContentCache; import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.provider.IdentityProvider; import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.provider.SamlIdentityProviderDefinition; -import org.cloudfoundry.identity.uaa.util.UaaHttpRequestUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import org.opensaml.saml2.metadata.provider.MetadataProviderException; @@ -30,50 +25,21 @@ import org.springframework.security.saml.metadata.ExtendedMetadata; import org.springframework.security.saml.metadata.ExtendedMetadataDelegate; import org.springframework.util.StringUtils; -import org.springframework.web.client.RestTemplate; import java.net.URI; import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; -import java.util.Date; import java.util.LinkedList; import java.util.List; -import java.util.Timer; -import java.util.TimerTask; import static org.springframework.util.StringUtils.hasText; public class SamlIdentityProviderConfigurator implements InitializingBean { - public static final int CONFIGURE_URL_DEFAULT_TIMEOUT = 30_000; - private static Log logger = LogFactory.getLog(SamlIdentityProviderConfigurator.class); - private HttpClientParams clientParams; private BasicParserPool parserPool; private IdentityProviderProvisioning providerProvisioning; - private UrlContentCache contentCache; - private MetadataFetcher metadataFetcher; - - private Timer dummyTimer = new Timer() { - @Override public void cancel() { super.cancel(); } - @Override public int purge() {return 0; } - @Override public void schedule(TimerTask task, long delay) {} - @Override public void schedule(TimerTask task, long delay, long period) {} - @Override public void schedule(TimerTask task, Date firstTime, long period) {} - @Override public void schedule(TimerTask task, Date time) {} - @Override public void scheduleAtFixedRate(TimerTask task, long delay, long period) {} - @Override public void scheduleAtFixedRate(TimerTask task, Date firstTime, long period) {} - }; + private FixedHttpMetaDataProvider fixedHttpMetaDataProvider; public SamlIdentityProviderConfigurator() { - dummyTimer.cancel(); - } - - public UrlContentCache getContentCache() { - return contentCache; - } - - public SamlIdentityProviderConfigurator setContentCache(UrlContentCache contentCache) { - this.contentCache = contentCache; - return this; } public List getIdentityProviderDefinitions() { @@ -82,7 +48,7 @@ public List getIdentityProviderDefinitions() { public List getIdentityProviderDefinitionsForZone(IdentityZone zone) { List result = new LinkedList<>(); - for (IdentityProvider provider: providerProvisioning.retrieveActive(zone.getId())) { + for (IdentityProvider provider : providerProvisioning.retrieveActive(zone.getId())) { if (OriginKeys.SAML.equals(provider.getType())) { result.add((SamlIdentityProviderDefinition) provider.getConfig()); } @@ -106,12 +72,13 @@ public List getIdentityProviderDefinitions(List< /** * adds or replaces a SAML identity proviider + * * @param providerDefinition - the provider to be added * @throws MetadataProviderException if the system fails to fetch meta data for this provider */ public synchronized void validateSamlIdentityProviderDefinition(SamlIdentityProviderDefinition providerDefinition) throws MetadataProviderException { - ExtendedMetadataDelegate added, deleted=null; - if (providerDefinition==null) { + ExtendedMetadataDelegate added, deleted = null; + if (providerDefinition == null) { throw new NullPointerException(); } if (!hasText(providerDefinition.getIdpEntityAlias())) { @@ -122,45 +89,25 @@ public synchronized void validateSamlIdentityProviderDefinition(SamlIdentityProv } SamlIdentityProviderDefinition clone = providerDefinition.clone(); added = getExtendedMetadataDelegate(clone); - String entityIDToBeAdded = ((ConfigMetadataProvider)added.getDelegate()).getEntityID(); + String entityIDToBeAdded = ((ConfigMetadataProvider) added.getDelegate()).getEntityID(); if (!StringUtils.hasText(entityIDToBeAdded)) { - throw new MetadataProviderException("Emtpy entityID for SAML provider with zoneId:"+providerDefinition.getZoneId()+" and origin:"+providerDefinition.getIdpEntityAlias()); + throw new MetadataProviderException("Emtpy entityID for SAML provider with zoneId:" + providerDefinition.getZoneId() + " and origin:" + providerDefinition.getIdpEntityAlias()); } boolean entityIDexists = false; for (SamlIdentityProviderDefinition existing : getIdentityProviderDefinitions()) { - ConfigMetadataProvider existingProvider = (ConfigMetadataProvider)getExtendedMetadataDelegate(existing).getDelegate(); + ConfigMetadataProvider existingProvider = (ConfigMetadataProvider) getExtendedMetadataDelegate(existing).getDelegate(); if (entityIDToBeAdded.equals(existingProvider.getEntityID()) && - !(existing.getUniqueAlias().equals(clone.getUniqueAlias()))) { + !(existing.getUniqueAlias().equals(clone.getUniqueAlias()))) { entityIDexists = true; break; } } if (entityIDexists) { - throw new MetadataProviderException("Duplicate entity ID:"+entityIDToBeAdded); - } - } - - public synchronized ExtendedMetadataDelegate removeIdentityProviderDefinition(SamlIdentityProviderDefinition providerDefinition) { - return null; - } - - public List getSamlIdentityProviders() { - return getSamlIdentityProviders(null); - } - - public List getSamlIdentityProviders(IdentityZone zone) { - List result = new LinkedList<>(); - for (SamlIdentityProviderDefinition def : getIdentityProviderDefinitionsForZone(zone)) { - try { - result.add(getExtendedMetadataDelegate(def)); - } catch (MetadataProviderException e) { - throw new RuntimeException(e); - } + throw new MetadataProviderException("Duplicate entity ID:" + entityIDToBeAdded); } - return result; } public ExtendedMetadataDelegate getExtendedMetadataDelegateFromCache(SamlIdentityProviderDefinition def) throws MetadataProviderException { @@ -175,7 +122,7 @@ public ExtendedMetadataDelegate getExtendedMetadataDelegate(SamlIdentityProvider break; } case URL: { - metadata = configureURLMetadata(def, CONFIGURE_URL_DEFAULT_TIMEOUT); + metadata = configureURLMetadata(def); break; } default: { @@ -197,19 +144,6 @@ protected ExtendedMetadataDelegate configureXMLMetadata(SamlIdentityProviderDefi return delegate; } - protected FixedHttpMetaDataProvider getFixedHttpMetaDataProvider(SamlIdentityProviderDefinition def, - Timer dummyTimer, - int timeout) throws ClassNotFoundException, MetadataProviderException, URISyntaxException, InstantiationException, IllegalAccessException { - ExtendedMetadata extendedMetadata = new ExtendedMetadata(); - extendedMetadata.setAlias(def.getIdpEntityAlias()); - RestTemplate template = new RestTemplate(UaaHttpRequestUtils.createRequestFactory(def.isSkipSslValidation(), timeout)); - FixedHttpMetaDataProvider fixedHttpMetaDataProvider = - FixedHttpMetaDataProvider.buildProvider(dummyTimer, getClientParams(), - adjustURIForPort(def.getMetaDataLocation()), - template, - this.contentCache); - return fixedHttpMetaDataProvider; - } protected String adjustURIForPort(String uri) throws URISyntaxException { URI metadataURI = new URI(uri); @@ -226,23 +160,17 @@ protected String adjustURIForPort(String uri) throws URISyntaxException { return uri; } - protected ExtendedMetadataDelegate configureURLMetadata(SamlIdentityProviderDefinition def, int timeoutInMs) throws MetadataProviderException { + protected ExtendedMetadataDelegate configureURLMetadata(SamlIdentityProviderDefinition def) throws MetadataProviderException { try { def = def.clone(); - ExtendedMetadata extendedMetadata = new ExtendedMetadata(); - extendedMetadata.setAlias(def.getIdpEntityAlias()); - FixedHttpMetaDataProvider fixedHttpMetaDataProvider = getFixedHttpMetaDataProvider(def, dummyTimer, timeoutInMs); - byte[] metadata = metadataFetcher.fetch(fixedHttpMetaDataProvider); + String adjustedMetatadataURIForPort = adjustURIForPort(def.getMetaDataLocation()); + + byte[] metadata = fixedHttpMetaDataProvider.fetchMetadata(adjustedMetatadataURIForPort, def.isSkipSslValidation()); + def.setMetaDataLocation(new String(metadata, StandardCharsets.UTF_8)); return configureXMLMetadata(def); } catch (URISyntaxException e) { - throw new MetadataProviderException("Invalid socket factory(invalid URI):"+def.getMetaDataLocation(), e); - } catch (ClassNotFoundException e) { - throw new MetadataProviderException("Invalid socket factory:"+def.getSocketFactoryClassName(), e); - } catch (InstantiationException e) { - throw new MetadataProviderException("Invalid socket factory:"+def.getSocketFactoryClassName(), e); - } catch (IllegalAccessException e) { - throw new MetadataProviderException("Invalid socket factory:"+def.getSocketFactoryClassName(), e); + throw new MetadataProviderException("Invalid socket factory(invalid URI):" + def.getMetaDataLocation(), e); } } @@ -254,13 +182,6 @@ public void setIdentityProviderProvisioning(IdentityProviderProvisioning provide this.providerProvisioning = providerProvisioning; } - public HttpClientParams getClientParams() { - return clientParams; - } - - public void setClientParams(HttpClientParams clientParams) { - this.clientParams = clientParams; - } public BasicParserPool getParserPool() { return parserPool; @@ -274,7 +195,7 @@ public void setParserPool(BasicParserPool parserPool) { public void afterPropertiesSet() throws Exception { } - public void setMetadataFetcher(MetadataFetcher metadataFetcher) { - this.metadataFetcher = metadataFetcher; + public void setFixedHttpMetaDataProvider(FixedHttpMetaDataProvider fixedHttpMetaDataProvider) { + this.fixedHttpMetaDataProvider = fixedHttpMetaDataProvider; } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/idp/SamlServiceProviderConfigurator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/idp/SamlServiceProviderConfigurator.java index e309f621f6d..6b13d0f2982 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/idp/SamlServiceProviderConfigurator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/idp/SamlServiceProviderConfigurator.java @@ -14,110 +14,42 @@ */ package org.cloudfoundry.identity.uaa.provider.saml.idp; -import java.net.URI; -import java.net.URISyntaxException; -import java.nio.charset.StandardCharsets; -import java.util.Arrays; -import java.util.Collections; -import java.util.Date; -import java.util.HashSet; -import java.util.LinkedList; -import java.util.List; -import java.util.Set; -import java.util.Timer; -import java.util.TimerTask; - -import org.cloudfoundry.identity.uaa.cache.UrlContentCache; import org.cloudfoundry.identity.uaa.provider.saml.ConfigMetadataProvider; import org.cloudfoundry.identity.uaa.provider.saml.FixedHttpMetaDataProvider; -import org.cloudfoundry.identity.uaa.util.UaaHttpRequestUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; - -import org.apache.commons.httpclient.params.HttpClientParams; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.apache.http.client.utils.URIBuilder; import org.opensaml.common.xml.SAMLConstants; import org.opensaml.saml2.core.NameIDType; import org.opensaml.saml2.metadata.SPSSODescriptor; import org.opensaml.saml2.metadata.provider.MetadataProviderException; import org.opensaml.xml.parse.BasicParserPool; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.security.saml.metadata.ExtendedMetadata; import org.springframework.security.saml.metadata.ExtendedMetadataDelegate; import org.springframework.util.StringUtils; -import org.springframework.web.client.RestTemplate; + +import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.List; +import java.util.Set; /** * Holds internal state of available SAML Service Providers. */ public class SamlServiceProviderConfigurator { - private static final Log logger = LogFactory.getLog(SamlServiceProviderConfigurator.class); + private static final Logger LOG = LoggerFactory.getLogger(SamlServiceProviderConfigurator.class); - private HttpClientParams clientParams; + private FixedHttpMetaDataProvider fixedHttpMetaDataProvider; private BasicParserPool parserPool; - - private SamlServiceProviderProvisioning providerProvisioning; - - private Set supportedNameIDs = new HashSet<>(Arrays.asList(NameIDType.EMAIL, NameIDType.PERSISTENT, - NameIDType.UNSPECIFIED)); - private UrlContentCache contentCache; - - private Timer dummyTimer = new Timer() { - - @Override - public void cancel() { - super.cancel(); - } - - @Override - public int purge() { - return 0; - } - - @Override - public void schedule(TimerTask task, long delay) { - // Do nothing. - } - - @Override - public void schedule(TimerTask task, long delay, long period) { - // Do nothing. - } - - @Override - public void schedule(TimerTask task, Date firstTime, long period) { - // Do nothing. - } - - @Override - public void schedule(TimerTask task, Date time) { - // Do nothing. - } - - @Override - public void scheduleAtFixedRate(TimerTask task, long delay, long period) { - // Do nothing. - } - - @Override - public void scheduleAtFixedRate(TimerTask task, Date firstTime, long period) { - // Do nothing. - } - }; - - public UrlContentCache getContentCache() { - return contentCache; - } - - public SamlServiceProviderConfigurator setContentCache(UrlContentCache contentCache) { - this.contentCache = contentCache; - return this; - } + private Set supportedNameIDs = new HashSet<>(Arrays.asList(NameIDType.EMAIL, NameIDType.PERSISTENT, NameIDType.UNSPECIFIED)); public SamlServiceProviderConfigurator() { - dummyTimer.cancel(); } public List getSamlServiceProviders() { @@ -126,13 +58,13 @@ public List getSamlServiceProviders() { public List getSamlServiceProvidersForZone(IdentityZone zone) { List result = new LinkedList<>(); - for (SamlServiceProvider provider: providerProvisioning.retrieveActive(zone.getId())) { + for (SamlServiceProvider provider : providerProvisioning.retrieveActive(zone.getId())) { try { SamlServiceProviderHolder samlServiceProviderHolder = - new SamlServiceProviderHolder(getExtendedMetadataDelegate(provider), provider); + new SamlServiceProviderHolder(getExtendedMetadataDelegate(provider), provider); result.add(samlServiceProviderHolder); - }catch(MetadataProviderException e) { - logger.error("Unable to configure SAML SP Metadata for ServiceProvider:" + provider.getEntityId(), e); + } catch (MetadataProviderException e) { + LOG.error("Unable to configure SAML SP Metadata for ServiceProvider:" + provider.getEntityId(), e); } } return Collections.unmodifiableList(result); @@ -141,17 +73,15 @@ public List getSamlServiceProvidersForZone(IdentityZo /** * adds or replaces a SAML service provider for the current zone. * - * @param provider - * - the provider to be added - * @throws MetadataProviderException - * if the system fails to fetch meta data for this provider + * @param provider - the provider to be added + * @throws MetadataProviderException if the system fails to fetch meta data for this provider */ public void validateSamlServiceProvider(SamlServiceProvider provider) throws MetadataProviderException { validateSamlServiceProvider(provider, IdentityZoneHolder.get()); } synchronized void validateSamlServiceProvider(SamlServiceProvider provider, IdentityZone zone) - throws MetadataProviderException { + throws MetadataProviderException { if (provider == null) { throw new NullPointerException(); @@ -168,25 +98,24 @@ synchronized void validateSamlServiceProvider(SamlServiceProvider provider, Iden String metadataEntityId = ((ConfigMetadataProvider) added.getDelegate()).getEntityID(); if (provider.getEntityId() == null) { provider.setEntityId(metadataEntityId); - } - else if (!metadataEntityId.equals(provider.getEntityId())) { + } else if (!metadataEntityId.equals(provider.getEntityId())) { throw new MetadataProviderException( - "Metadata entity id does not match SAML SP entity id: " + provider.getEntityId()); + "Metadata entity id does not match SAML SP entity id: " + provider.getEntityId()); } // Initializing here is necessary to access the SPSSODescriptor, otherwise an exception is thrown. added.initialize(); SPSSODescriptor spSsoDescriptor = added.getEntityDescriptor(metadataEntityId). - getSPSSODescriptor(SAMLConstants.SAML20P_NS); + getSPSSODescriptor(SAMLConstants.SAML20P_NS); if (null != spSsoDescriptor && - null != spSsoDescriptor.getNameIDFormats() && - !spSsoDescriptor.getNameIDFormats().isEmpty()) { + null != spSsoDescriptor.getNameIDFormats() && + !spSsoDescriptor.getNameIDFormats().isEmpty()) { // The SP explicitly states the NameID formats it supports, we should check that we support at least one. if (!spSsoDescriptor.getNameIDFormats().stream().anyMatch( - format -> this.supportedNameIDs.contains(format.getFormat()))) { + format -> this.supportedNameIDs.contains(format.getFormat()))) { throw new MetadataProviderException( - "UAA does not support any of the NameIDFormats specified in the metadata for entity: " - + provider.getEntityId()); + "UAA does not support any of the NameIDFormats specified in the metadata for entity: " + + provider.getEntityId()); } } List serviceProviders = getSamlServiceProvidersForZone(zone); @@ -194,28 +123,28 @@ else if (!metadataEntityId.equals(provider.getEntityId())) { } public ExtendedMetadataDelegate getExtendedMetadataDelegate(SamlServiceProvider provider) - throws MetadataProviderException { + throws MetadataProviderException { ExtendedMetadataDelegate metadata; switch (provider.getConfig().getType()) { - case DATA: { - metadata = configureXMLMetadata(provider); - break; - } - case URL: { - metadata = configureURLMetadata(provider); - break; - } - default: { - throw new MetadataProviderException("Invalid metadata type for alias[" + provider.getEntityId() + "]:" - + provider.getConfig().getMetaDataLocation()); - } + case DATA: { + metadata = configureXMLMetadata(provider); + break; + } + case URL: { + metadata = configureURLMetadata(provider); + break; + } + default: { + throw new MetadataProviderException("Invalid metadata type for alias[" + provider.getEntityId() + "]:" + + provider.getConfig().getMetaDataLocation()); + } } return metadata; } protected ExtendedMetadataDelegate configureXMLMetadata(SamlServiceProvider provider) { ConfigMetadataProvider configMetadataProvider = new ConfigMetadataProvider(provider.getIdentityZoneId(), - provider.getEntityId(), provider.getConfig().getMetaDataLocation()); + provider.getEntityId(), provider.getConfig().getMetaDataLocation()); configMetadataProvider.setParserPool(getParserPool()); ExtendedMetadata extendedMetadata = new ExtendedMetadata(); extendedMetadata.setLocal(false); @@ -227,52 +156,26 @@ protected ExtendedMetadataDelegate configureXMLMetadata(SamlServiceProvider prov } protected ExtendedMetadataDelegate configureURLMetadata(SamlServiceProvider provider) - throws MetadataProviderException { + throws MetadataProviderException { SamlServiceProviderDefinition def = provider.getConfig().clone(); ExtendedMetadata extendedMetadata = new ExtendedMetadata(); extendedMetadata.setAlias(provider.getEntityId()); - FixedHttpMetaDataProvider fixedHttpMetaDataProvider; + byte[] metadata; try { - fixedHttpMetaDataProvider = FixedHttpMetaDataProvider.buildProvider( - dummyTimer, getClientParams(), - adjustURIForPort(def.getMetaDataLocation()), - new RestTemplate(UaaHttpRequestUtils.createRequestFactory(def.isSkipSslValidation(), -1)), - this.contentCache - - ); + metadata = fixedHttpMetaDataProvider.fetchMetadata(def.getMetaDataLocation(), def.isSkipSslValidation()); } catch (URISyntaxException e) { throw new MetadataProviderException("Invalid metadata URI: " + def.getMetaDataLocation(), e); } - byte[] metadata = fixedHttpMetaDataProvider.fetchMetadata(); def.setMetaDataLocation(new String(metadata, StandardCharsets.UTF_8)); return configureXMLMetadata(provider); } - protected String adjustURIForPort(String uri) throws URISyntaxException { - URI metadataURI = new URI(uri); - if (metadataURI.getPort() < 0) { - switch (metadataURI.getScheme()) { - case "https": - return new URIBuilder(uri).setPort(443).build().toString(); - case "http": - return new URIBuilder(uri).setPort(80).build().toString(); - default: - return uri; - } - } - return uri; - } - - public SamlServiceProviderProvisioning getProviderProvisioning() { return providerProvisioning; } - - public void setProviderProvisioning(SamlServiceProviderProvisioning providerProvisioning) { this.providerProvisioning = providerProvisioning; } - - public HttpClientParams getClientParams() { - return clientParams; + public SamlServiceProviderProvisioning getProviderProvisioning() { + return providerProvisioning; } - public void setClientParams(HttpClientParams clientParams) { - this.clientParams = clientParams; + public void setProviderProvisioning(SamlServiceProviderProvisioning providerProvisioning) { + this.providerProvisioning = providerProvisioning; } public BasicParserPool getParserPool() { @@ -286,4 +189,8 @@ public void setParserPool(BasicParserPool parserPool) { public void setSupportedNameIDs(Set supportedNameIDs) { this.supportedNameIDs = supportedNameIDs; } + + public void setFixedHttpMetaDataProvider(FixedHttpMetaDataProvider fixedHttpMetaDataProvider) { + this.fixedHttpMetaDataProvider = fixedHttpMetaDataProvider; + } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfiguratorTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfiguratorTests.java index d316ecc76a7..37de3baf64f 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfiguratorTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfiguratorTests.java @@ -51,6 +51,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; @@ -59,6 +60,7 @@ public class SamlIdentityProviderConfiguratorTests { private HttpServer httpServer; + private FixedHttpMetaDataProvider fixedHttpMetaDataProvider; @BeforeClass public static void initializeOpenSAML() throws Exception { @@ -151,8 +153,9 @@ public void setUp() throws Exception { .setIconUrl("sample-icon-url") .setZoneId("uaa"); configurator.setIdentityProviderProvisioning(provisioning); - UrlContentCache urlContentCache = spy(new ExpiringUrlCache(1000 * 60 * 10, new TimeServiceImpl(), 100)); - configurator.setContentCache(urlContentCache); + fixedHttpMetaDataProvider = mock(FixedHttpMetaDataProvider.class); + + configurator.setFixedHttpMetaDataProvider(fixedHttpMetaDataProvider); } @Test(expected = NullPointerException.class) @@ -168,8 +171,6 @@ public void testAddNullProviderAlias() throws Exception { @Test public void testGetEntityID() throws Exception { - MetadataFetcher metadataFetcher = mock(MetadataFetcher.class); - configurator.setMetadataFetcher(metadataFetcher); Timer t = new Timer(); bootstrap.setIdentityProviders(BootstrapSamlIdentityProviderDataTests.parseYaml(BootstrapSamlIdentityProviderDataTests.sampleYaml)); @@ -192,7 +193,7 @@ public void testGetEntityID() throws Exception { break; } case "simplesamlphp-url": { - when(metadataFetcher.fetch(any())).thenReturn(getSimpleSamlPhpMetadata("http://simplesamlphp.somewhere.com").getBytes()); + when(fixedHttpMetaDataProvider.fetchMetadata(any(), anyBoolean())).thenReturn(getSimpleSamlPhpMetadata("http://simplesamlphp.somewhere.com").getBytes()); ComparableProvider provider = (ComparableProvider) configurator.getExtendedMetadataDelegateFromCache(def).getDelegate(); assertEquals("http://simplesamlphp.somewhere.com/saml2/idp/metadata.php", provider.getEntityID()); break; @@ -293,8 +294,7 @@ public void shouldTimeoutWhenFetchingMetadataURL() throws Exception { SamlIdentityProviderDefinition def = new SamlIdentityProviderDefinition(); def.setMetaDataLocation("https://localhost:23439"); def.setSkipSslValidation(true); - configurator.setMetadataFetcher(new MetadataFetcher()); int timeout = 100; - configurator.configureURLMetadata(def, timeout); + configurator.configureURLMetadata(def); } } \ No newline at end of file diff --git a/uaa/src/main/webapp/WEB-INF/spring/saml-idp.xml b/uaa/src/main/webapp/WEB-INF/spring/saml-idp.xml index 37295f60dbe..6dd9df4361d 100644 --- a/uaa/src/main/webapp/WEB-INF/spring/saml-idp.xml +++ b/uaa/src/main/webapp/WEB-INF/spring/saml-idp.xml @@ -8,50 +8,50 @@ LICENSE file. --> + request-matcher="ant"> + filters="samlSecurityContextPersistenceFilter,samlIdpWebSsoProcessingFilter"/> + ref="samlIdpAuthenticationProvider"/> + class="org.cloudfoundry.identity.uaa.provider.saml.idp.IdpSamlAuthenticationProvider"/> - - - + class="org.springframework.security.saml.SAMLProcessingFilter"> + + + - - + + - - + class="org.cloudfoundry.identity.uaa.provider.saml.idp.IdpSamlAuthenticationSuccessHandler"> + + - - + + @@ -66,19 +66,19 @@ - - - - + class="org.cloudfoundry.identity.uaa.provider.saml.idp.IdpWebSsoProfileImpl"> + + + + - - + + @@ -91,69 +91,75 @@ - + class="org.cloudfoundry.identity.uaa.provider.saml.idp.IdpSamlContextProviderImpl"> + - - - + class="org.cloudfoundry.identity.uaa.provider.saml.idp.IdpMetadataGeneratorFilter"> + + + - - - + class="org.cloudfoundry.identity.uaa.provider.saml.idp.IdpMetadataDisplayFilter"> + + + - + class="org.cloudfoundry.identity.uaa.provider.saml.idp.ZoneAwareIdpMetadataGenerator"> + + value="${login.saml.wantAuthnRequestSigned:false}"/> - - + value="${login.entityBaseURL:http://localhost:8080/uaa}"/> + + + value="${login.saml.idp.entityIDAlias:${login.idp.entityID:${login.saml.entityIDAlias:${login.entityID:unit-test-idp}}}}"/> - + + + + + + + + + - + class="org.cloudfoundry.identity.uaa.provider.saml.idp.SamlServiceProviderConfigurator"> - - - + + - + diff --git a/uaa/src/main/webapp/WEB-INF/spring/saml-providers.xml b/uaa/src/main/webapp/WEB-INF/spring/saml-providers.xml index 80788f6a8ba..a745fca8d9c 100644 --- a/uaa/src/main/webapp/WEB-INF/spring/saml-providers.xml +++ b/uaa/src/main/webapp/WEB-INF/spring/saml-providers.xml @@ -296,14 +296,11 @@ - - - - + diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java index 406566cbf7e..849ab81f011 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java @@ -170,7 +170,7 @@ public static boolean isMember(String userId, ScimGroup group) { public static UserInfoResponse getUserInfo(String url, String token) throws URISyntaxException { - RestTemplate rest = new RestTemplate(createRequestFactory(true)); + RestTemplate rest = new RestTemplate(createRequestFactory(true, -1)); MultiValueMap headers = new LinkedMultiValueMap<>(); headers.add(AUTHORIZATION, "Bearer "+token); headers.add(ACCEPT, APPLICATION_JSON_VALUE); @@ -179,7 +179,7 @@ public static UserInfoResponse getUserInfo(String url, String token) throws URIS } public static void deleteZone(String baseUrl, String id, String adminToken) throws URISyntaxException { - RestTemplate rest = new RestTemplate(createRequestFactory(true)); + RestTemplate rest = new RestTemplate(createRequestFactory(true, -1)); MultiValueMap headers = new LinkedMultiValueMap<>(); headers.add(AUTHORIZATION, "Bearer "+adminToken); headers.add(ACCEPT, APPLICATION_JSON_VALUE);