diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/provider/SamlIdentityProviderDefinition.java b/model/src/main/java/org/cloudfoundry/identity/uaa/provider/SamlIdentityProviderDefinition.java index 8a325dc0a00..761548b64e6 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/provider/SamlIdentityProviderDefinition.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/provider/SamlIdentityProviderDefinition.java @@ -57,6 +57,9 @@ public enum ExternalGroupMappingMode { private boolean skipSslValidation = false; private List authnContext; + @JsonIgnore + private String idpEntityId; + public SamlIdentityProviderDefinition() {} public SamlIdentityProviderDefinition clone() { @@ -67,6 +70,7 @@ public SamlIdentityProviderDefinition clone() { SamlIdentityProviderDefinition def = new SamlIdentityProviderDefinition(); def.setMetaDataLocation(metaDataLocation); def.setIdpEntityAlias(idpEntityAlias); + def.setIdpEntityId(idpEntityId); def.setZoneId(zoneId); def.setNameID(nameID); def.setAssertionConsumerIndex(assertionConsumerIndex); @@ -108,6 +112,16 @@ public MetadataLocation getType() { return MetadataLocation.UNKNOWN; } + @JsonIgnore + public String getIdpEntityId() { + return this.idpEntityId; + } + + @JsonIgnore + public void setIdpEntityId(final String idpEntityId) { + this.idpEntityId = idpEntityId; + } + private boolean validateXml(String xml) { if (xml==null || xml.toUpperCase().contains(" getDefaultGroups() { return defaultGroups; } @@ -78,4 +80,12 @@ public boolean isCheckOriginEnabled() { public void setCheckOriginEnabled(boolean checkOriginEnabled) { this.checkOriginEnabled = checkOriginEnabled; } + + public boolean isAllowOriginLoop() { + return this.allowOriginLoop; + } + + public void setAllowOriginLoop(final boolean allowAllOrigins) { + this.allowOriginLoop = allowAllOrigins; + } } diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java index 7ca755c87d8..b0b7cf05570 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import java.util.List; @@ -60,4 +61,19 @@ public void setMaxUsers() { userConfig.setMaxUsers(100); assertEquals(100, userConfig.getMaxUsers()); } + + @Test + public void testDefaultOrigin() { + UserConfig userConfig = new UserConfig(); + assertTrue(userConfig.isAllowOriginLoop()); + assertFalse(userConfig.isCheckOriginEnabled()); + } + + @Test + public void testOriginLoop() { + UserConfig userConfig = new UserConfig(); + assertTrue(userConfig.isAllowOriginLoop()); + userConfig.setAllowOriginLoop(false); + assertFalse(userConfig.isCheckOriginEnabled()); + } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index 8903759e243..6c664cede4d 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -138,7 +138,7 @@ public ResponseEntity createIdentityProvider(@RequestBody Iden SamlIdentityProviderDefinition definition = ObjectUtils.castInstance(body.getConfig(), SamlIdentityProviderDefinition.class); definition.setZoneId(zoneId); definition.setIdpEntityAlias(body.getOriginKey()); - samlConfigurator.validateSamlIdentityProviderDefinition(definition); + definition.setIdpEntityId(samlConfigurator.validateSamlIdentityProviderDefinition(definition, true)); body.setConfig(definition); } @@ -223,7 +223,7 @@ public ResponseEntity updateIdentityProvider(@PathVariable Str SamlIdentityProviderDefinition definition = ObjectUtils.castInstance(body.getConfig(), SamlIdentityProviderDefinition.class); definition.setZoneId(zoneId); definition.setIdpEntityAlias(body.getOriginKey()); - samlConfigurator.validateSamlIdentityProviderDefinition(definition); + samlConfigurator.validateSamlIdentityProviderDefinition(definition, false); body.setConfig(definition); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderProvisioning.java index 28c73ec1338..24833de5143 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderProvisioning.java @@ -31,6 +31,8 @@ public interface IdentityProviderProvisioning { IdentityProvider retrieveByOrigin(String origin, String zoneId); + IdentityProvider retrieveByExternId(String externId, String type, String zoneId); + default IdentityProvider retrieveByOriginIgnoreActiveFlag(String origin, String zoneId) { return retrieveByOrigin(origin, zoneId); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java index f2d1769326f..69077bab13c 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java @@ -20,6 +20,7 @@ import java.sql.Timestamp; import java.util.Date; import java.util.List; +import java.util.Optional; import java.util.UUID; @Component("identityProviderProvisioning") @@ -27,9 +28,9 @@ public class JdbcIdentityProviderProvisioning implements IdentityProviderProvisi private static Logger logger = LoggerFactory.getLogger(JdbcIdentityProviderProvisioning.class); - public static final String ID_PROVIDER_FIELDS = "id,version,created,lastmodified,name,origin_key,type,config,identity_zone_id,active,alias_id,alias_zid"; + public static final String ID_PROVIDER_FIELDS = "id,version,created,lastmodified,name,origin_key,type,config,identity_zone_id,active,alias_id,alias_zid,external_key"; - public static final String CREATE_IDENTITY_PROVIDER_SQL = "insert into identity_provider(" + ID_PROVIDER_FIELDS + ") values (?,?,?,?,?,?,?,?,?,?,?,?)"; + public static final String CREATE_IDENTITY_PROVIDER_SQL = "insert into identity_provider(" + ID_PROVIDER_FIELDS + ") values (?,?,?,?,?,?,?,?,?,?,?,?,?)"; public static final String IDENTITY_PROVIDERS_QUERY = "select " + ID_PROVIDER_FIELDS + " from identity_provider where identity_zone_id=?"; @@ -37,7 +38,7 @@ public class JdbcIdentityProviderProvisioning implements IdentityProviderProvisi public static final String IDP_WITH_ALIAS_EXISTS_QUERY = "select 1 from identity_provider idp where idp.identity_zone_id = ? and idp.alias_zid <> '' limit 1"; - public static final String ID_PROVIDER_UPDATE_FIELDS = "version,lastmodified,name,type,config,active,alias_id,alias_zid".replace(",", "=?,") + "=?"; + public static final String ID_PROVIDER_UPDATE_FIELDS = "version,lastmodified,name,type,config,active,alias_id,alias_zid,external_key".replace(",", "=?,") + "=?"; public static final String UPDATE_IDENTITY_PROVIDER_SQL = "update identity_provider set " + ID_PROVIDER_UPDATE_FIELDS + " where id=? and identity_zone_id=?"; @@ -51,6 +52,8 @@ public class JdbcIdentityProviderProvisioning implements IdentityProviderProvisi public static final String IDENTITY_PROVIDER_BY_ORIGIN_QUERY_ACTIVE = IDENTITY_PROVIDER_BY_ORIGIN_QUERY + " and active = ? "; + public static final String IDENTITY_PROVIDER_BY_EXTERNAL_QUERY = IDENTITY_PROVIDERS_QUERY + " and type=? and external_key=?"; + protected final JdbcTemplate jdbcTemplate; private final RowMapper mapper = new IdentityProviderRowMapper(); @@ -100,9 +103,14 @@ public IdentityProvider retrieveByOriginIgnoreActiveFlag(String origin, String z return jdbcTemplate.queryForObject(IDENTITY_PROVIDER_BY_ORIGIN_QUERY, mapper, origin, zoneId); } + @Override + public IdentityProvider retrieveByExternId(String externId, String type, String zoneId) { + return jdbcTemplate.queryForObject(IDENTITY_PROVIDER_BY_EXTERNAL_QUERY, mapper, zoneId, type, externId); + } + @Override public IdentityProvider create(final IdentityProvider identityProvider, String zoneId) { - validate(identityProvider); + String externId = validate(identityProvider); final String id = UUID.randomUUID().toString(); try { jdbcTemplate.update(CREATE_IDENTITY_PROVIDER_SQL, ps -> { @@ -118,7 +126,8 @@ public IdentityProvider create(final IdentityProvider identityProvider, String z ps.setString(pos++, zoneId); ps.setBoolean(pos++, identityProvider.isActive()); ps.setString(pos++, identityProvider.getAliasId()); - ps.setString(pos, identityProvider.getAliasZid()); + ps.setString(pos++, identityProvider.getAliasZid()); + ps.setString(pos, externId); }); } catch (DuplicateKeyException e) { throw new IdpAlreadyExistsException(e.getMostSpecificCause().getMessage()); @@ -128,7 +137,7 @@ public IdentityProvider create(final IdentityProvider identityProvider, String z @Override public IdentityProvider update(final IdentityProvider identityProvider, String zoneId) { - validate(identityProvider); + String externId = validate(identityProvider); jdbcTemplate.update(UPDATE_IDENTITY_PROVIDER_SQL, ps -> { int pos = 1; @@ -141,6 +150,7 @@ public IdentityProvider update(final IdentityProvider identityProvider, String z ps.setBoolean(pos++, identityProvider.isActive()); ps.setString(pos++, identityProvider.getAliasId()); ps.setString(pos++, identityProvider.getAliasZid()); + ps.setString(pos++, externId); // placeholders in WHERE ps.setString(pos++, identityProvider.getId().trim()); @@ -149,20 +159,25 @@ public IdentityProvider update(final IdentityProvider identityProvider, String z return retrieve(identityProvider.getId(), zoneId); } - protected void validate(IdentityProvider provider) { + private String validate(IdentityProvider provider) { if (provider == null) { throw new NullPointerException("Provider can not be null."); } if (!StringUtils.hasText(provider.getIdentityZoneId())) { throw new DataIntegrityViolationException("Identity zone ID must be set."); } + String externId = null; //ensure that SAML IDPs have redundant fields synchronized if (OriginKeys.SAML.equals(provider.getType()) && provider.getConfig() != null) { SamlIdentityProviderDefinition saml = ObjectUtils.castInstance(provider.getConfig(), SamlIdentityProviderDefinition.class); saml.setIdpEntityAlias(provider.getOriginKey()); saml.setZoneId(provider.getIdentityZoneId()); provider.setConfig(saml); + externId = saml.getIdpEntityId(); + } else if (provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition externalOAuthIdentityProviderDefinition) { + externId = externalOAuthIdentityProviderDefinition.getIssuer(); } + return externId; } @Override @@ -193,17 +208,25 @@ public IdentityProvider mapRow(ResultSet rs, int rowNum) throws SQLException { identityProvider.setOriginKey(rs.getString(pos++)); identityProvider.setType(rs.getString(pos++)); String config = rs.getString(pos++); + identityProvider.setIdentityZoneId(rs.getString(pos++)); + identityProvider.setActive(rs.getBoolean(pos++)); + identityProvider.setAliasId(rs.getString(pos++)); + identityProvider.setAliasZid(rs.getString(pos++)); + String externId = rs.getString(pos); if (StringUtils.hasText(config)) { AbstractIdentityProviderDefinition definition; switch (identityProvider.getType()) { case OriginKeys.SAML: definition = JsonUtils.readValue(config, SamlIdentityProviderDefinition.class); + Optional.ofNullable(definition).map(SamlIdentityProviderDefinition.class::cast).ifPresent(e -> e.setIdpEntityId(externId)); break; case OriginKeys.OAUTH20: definition = JsonUtils.readValue(config, RawExternalOAuthIdentityProviderDefinition.class); + Optional.ofNullable(definition).map(RawExternalOAuthIdentityProviderDefinition.class::cast).ifPresent(e -> e.setIssuer(externId)); break; case OriginKeys.OIDC10: definition = JsonUtils.readValue(config, OIDCIdentityProviderDefinition.class); + Optional.ofNullable(definition).map(OIDCIdentityProviderDefinition.class::cast).ifPresent(e -> e.setIssuer(externId)); break; case OriginKeys.UAA: definition = JsonUtils.readValue(config, UaaIdentityProviderDefinition.class); @@ -222,10 +245,6 @@ public IdentityProvider mapRow(ResultSet rs, int rowNum) throws SQLException { identityProvider.setConfig(definition); } } - identityProvider.setIdentityZoneId(rs.getString(pos++)); - identityProvider.setActive(rs.getBoolean(pos++)); - identityProvider.setAliasId(rs.getString(pos++)); - identityProvider.setAliasZid(rs.getString(pos)); return identityProvider; } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfigurator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfigurator.java index 2dbcec883e8..31ef18f1594 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfigurator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfigurator.java @@ -8,9 +8,14 @@ import org.cloudfoundry.identity.uaa.util.SessionUtils; import org.cloudfoundry.identity.uaa.util.UaaRandomStringUtil; import org.cloudfoundry.identity.uaa.util.UaaUrlUtils; +import org.cloudfoundry.identity.uaa.zone.IdentityZoneConfiguration; +import org.cloudfoundry.identity.uaa.zone.IdentityZoneProvisioning; +import org.cloudfoundry.identity.uaa.zone.UserConfig; +import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.cloudfoundry.identity.uaa.oauth.common.util.RandomValueStringGenerator; import org.springframework.util.CollectionUtils; @@ -23,7 +28,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; +import java.util.Optional; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; @@ -33,19 +38,26 @@ public class ExternalOAuthProviderConfigurator implements IdentityProviderProvisioning { - private static Logger LOGGER = LoggerFactory.getLogger(ExternalOAuthProviderConfigurator.class); + private static final Logger LOGGER = LoggerFactory.getLogger(ExternalOAuthProviderConfigurator.class); private final IdentityProviderProvisioning providerProvisioning; private final OidcMetadataFetcher oidcMetadataFetcher; private final UaaRandomStringUtil uaaRandomStringUtil; + private final IdentityZoneProvisioning identityZoneProvisioning; + private final IdentityZoneManager identityZoneManager; public ExternalOAuthProviderConfigurator( final @Qualifier("identityProviderProvisioning") IdentityProviderProvisioning providerProvisioning, final OidcMetadataFetcher oidcMetadataFetcher, - final UaaRandomStringUtil uaaRandomStringUtil) { + final UaaRandomStringUtil uaaRandomStringUtil, + final @Qualifier("identityZoneProvisioning") IdentityZoneProvisioning identityZoneProvisioning, + final IdentityZoneManager identityZoneManager + ) { this.providerProvisioning = providerProvisioning; this.oidcMetadataFetcher = oidcMetadataFetcher; this.uaaRandomStringUtil = uaaRandomStringUtil; + this.identityZoneProvisioning = identityZoneProvisioning; + this.identityZoneManager = identityZoneManager; } protected OIDCIdentityProviderDefinition overlay(OIDCIdentityProviderDefinition definition) { @@ -119,12 +131,25 @@ private String getCallbackUrlForIdp(String idpOriginKey, String uaaBaseUrl) { } private String getIdpUrlBase(final AbstractExternalOAuthIdentityProviderDefinition definition) { - if (definition instanceof OIDCIdentityProviderDefinition) { - return overlay((OIDCIdentityProviderDefinition) definition).getAuthUrl().toString(); + if (definition instanceof OIDCIdentityProviderDefinition oidcIdentityProviderDefinition) { + return overlay(oidcIdentityProviderDefinition).getAuthUrl().toString(); } return definition.getAuthUrl().toString(); } + private int isOriginLoopAllowed(String zoneId, int checkDone) { + if (checkDone > -1) { + return checkDone; + } + IdentityZoneConfiguration idzConfig; + if (identityZoneManager.getCurrentIdentityZoneId().equals(zoneId)) { + idzConfig = identityZoneManager.getCurrentIdentityZone().getConfig(); + } else { + idzConfig = identityZoneProvisioning.retrieve(zoneId).getConfig(); + } + return (idzConfig == null || Optional.of(idzConfig.getUserConfig()).map(UserConfig::isAllowOriginLoop).orElse(true)) ? 1 : 0; + } + @Override public IdentityProvider create(IdentityProvider identityProvider, String zoneId) { return providerProvisioning.create(identityProvider, zoneId); @@ -155,11 +180,29 @@ public List retrieveActive(String zoneId) { } public IdentityProvider retrieveByIssuer(String issuer, String zoneId) throws IncorrectResultSizeDataAccessException { + IdentityProvider issuedProvider = null; + int originLoopCheckDone = -1; + try { + issuedProvider = retrieveByExternId(issuer, OIDC10, zoneId); + if (issuedProvider != null && issuedProvider.isActive() + && issuedProvider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition oAuthIdentityProviderDefinition + && oAuthIdentityProviderDefinition.getIssuer().equals(issuer)) { + return issuedProvider; + } + } catch (EmptyResultDataAccessException e) { + originLoopCheckDone = isOriginLoopAllowed(zoneId, originLoopCheckDone); + if (originLoopCheckDone == 0) { + throw new IncorrectResultSizeDataAccessException(String.format("No provider with unique issuer[%s] found", issuer), 1, 0, e); + } + } + if (isOriginLoopAllowed(zoneId, originLoopCheckDone) == 0 && issuedProvider == null) { + throw new IncorrectResultSizeDataAccessException(String.format("Active provider with unique issuer[%s] not found", issuer), 1); + } List providers = retrieveAll(true, zoneId) .stream() .filter(p -> OIDC10.equals(p.getType()) && issuer.equals(((OIDCIdentityProviderDefinition) p.getConfig()).getIssuer())) - .collect(Collectors.toList()); + .toList(); if (providers.isEmpty()) { throw new IncorrectResultSizeDataAccessException(String.format("Active provider with issuer[%s] not found", issuer), 1); } else if (providers.size() > 1) { @@ -199,6 +242,15 @@ public IdentityProvider retrieveByOrigin(String origin, String zoneId) { return p; } + @Override + public IdentityProvider retrieveByExternId(String externId, String type, String zoneId) { + IdentityProvider p = providerProvisioning.retrieveByExternId(externId, type, zoneId); + if (p != null && OIDC10.equals(type)) { + p.setConfig(overlay((OIDCIdentityProviderDefinition) p.getConfig())); + } + return p; + } + @Override public IdentityProvider retrieveByOriginIgnoreActiveFlag(String origin, String zoneId) { IdentityProvider p = providerProvisioning.retrieveByOriginIgnoreActiveFlag(origin, zoneId); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderData.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderData.java index 96354e41b4e..d7f3f41f1f9 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderData.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderData.java @@ -27,9 +27,12 @@ import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZone; +import org.opensaml.saml2.metadata.provider.MetadataProviderException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.InitializingBean; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.security.saml.metadata.ExtendedMetadataDelegate; import static java.util.Collections.emptyList; import static java.util.Optional.ofNullable; @@ -50,8 +53,12 @@ public class BootstrapSamlIdentityProviderData implements InitializingBean { private boolean legacyShowSamlLink = true; private List> samlProviders = new LinkedList<>(); private Map> providers = null; + private final SamlIdentityProviderConfigurator samlConfigurator; - public BootstrapSamlIdentityProviderData() { + public BootstrapSamlIdentityProviderData( + final @Qualifier("metaDataProviders") SamlIdentityProviderConfigurator samlConfigurator + ) { + this.samlConfigurator = samlConfigurator; } public List getIdentityProviderDefinitions() { @@ -161,6 +168,14 @@ public void setIdentityProviders(Map> providers) { IdentityProvider provider = parseSamlProvider(def); + try { + if (def.getType() == SamlIdentityProviderDefinition.MetadataLocation.DATA) { + ExtendedMetadataDelegate metadataDelegate = samlConfigurator.getExtendedMetadataDelegate(def); + def.setIdpEntityId(((ConfigMetadataProvider) metadataDelegate.getDelegate()).getEntityID()); + } + } catch (MetadataProviderException e) { + throw new IllegalArgumentException(e.getMessage(), e); + } IdentityProviderWrapper wrapper = new IdentityProviderWrapper(provider); wrapper.setOverride(override == null ? true : override); samlProviders.add(wrapper); 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 1cedd620cfc..556113ab95b 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 @@ -4,13 +4,13 @@ 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.JdbcIdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.provider.SamlIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import org.opensaml.saml2.metadata.provider.MetadataProviderException; import org.opensaml.xml.parse.BasicParserPool; import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.security.saml.metadata.ExtendedMetadata; import org.springframework.security.saml.metadata.ExtendedMetadataDelegate; import org.springframework.stereotype.Component; @@ -71,9 +71,10 @@ public List getIdentityProviderDefinitions(List< * adds or replaces a SAML identity proviider * * @param providerDefinition - the provider to be added + * @param creation - check new created config * @throws MetadataProviderException if the system fails to fetch meta data for this provider */ - public synchronized void validateSamlIdentityProviderDefinition(SamlIdentityProviderDefinition providerDefinition) throws MetadataProviderException { + public synchronized String validateSamlIdentityProviderDefinition(SamlIdentityProviderDefinition providerDefinition, boolean creation) throws MetadataProviderException { ExtendedMetadataDelegate added, deleted = null; if (providerDefinition == null) { throw new NullPointerException(); @@ -91,26 +92,36 @@ public synchronized void validateSamlIdentityProviderDefinition(SamlIdentityProv throw new MetadataProviderException("Emtpy entityID for SAML provider with zoneId:" + providerDefinition.getZoneId() + " and origin:" + providerDefinition.getIdpEntityAlias()); } - boolean entityIDexists = false; + boolean entityIDexists = creation && entityIdExists(entityIDToBeAdded, providerDefinition.getZoneId()); - for (SamlIdentityProviderDefinition existing : getIdentityProviderDefinitions()) { - ConfigMetadataProvider existingProvider = (ConfigMetadataProvider) getExtendedMetadataDelegate(existing).getDelegate(); - if (entityIDToBeAdded.equals(existingProvider.getEntityID()) && - !(existing.getUniqueAlias().equals(clone.getUniqueAlias()))) { - entityIDexists = true; - break; + if (!entityIDexists) { + for (SamlIdentityProviderDefinition existing : getIdentityProviderDefinitions()) { + ConfigMetadataProvider existingProvider = (ConfigMetadataProvider) getExtendedMetadataDelegate(existing).getDelegate(); + if (entityIDToBeAdded.equals(existingProvider.getEntityID()) && !(existing.getUniqueAlias().equals(clone.getUniqueAlias()))) { + entityIDexists = true; + break; + } } } if (entityIDexists) { throw new MetadataProviderException("Duplicate entity ID:" + entityIDToBeAdded); } + return entityIDToBeAdded; } public ExtendedMetadataDelegate getExtendedMetadataDelegateFromCache(SamlIdentityProviderDefinition def) throws MetadataProviderException { return getExtendedMetadataDelegate(def); } + private boolean entityIdExists(String entityId, String zoneId) { + try { + return providerProvisioning.retrieveByExternId(entityId, OriginKeys.SAML, zoneId) != null; + } catch (EmptyResultDataAccessException e) { + return false; + } + } + public ExtendedMetadataDelegate getExtendedMetadataDelegate(SamlIdentityProviderDefinition def) throws MetadataProviderException { ExtendedMetadataDelegate metadata; switch (def.getType()) { diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_110__Add_ExternalKey_To_Identity_Provider.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_110__Add_ExternalKey_To_Identity_Provider.sql new file mode 100644 index 00000000000..e582bc61d16 --- /dev/null +++ b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_110__Add_ExternalKey_To_Identity_Provider.sql @@ -0,0 +1,3 @@ +-- add column external_key for oauth2,oidc,saml2 IdPs +ALTER TABLE identity_provider ADD COLUMN external_key VARCHAR(512) DEFAULT NULL; +CREATE UNIQUE INDEX IF NOT EXISTS external_key_in_zone on identity_provider (identity_zone_id,type,external_key); \ No newline at end of file diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_110__Add_ExternalKey_To_Identity_Provider.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_110__Add_ExternalKey_To_Identity_Provider.sql new file mode 100644 index 00000000000..dcb9812b4a6 --- /dev/null +++ b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_110__Add_ExternalKey_To_Identity_Provider.sql @@ -0,0 +1,3 @@ +-- add column external_key for oauth2,oidc,saml2 IdPs +ALTER TABLE identity_provider ADD COLUMN external_key VARCHAR(512) DEFAULT NULL; +CREATE UNIQUE INDEX external_key_in_zone on identity_provider (identity_zone_id,type(36),external_key) LOCK = SHARED; \ No newline at end of file diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_110__Add_ExternalKey_To_Identity_Provider.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_110__Add_ExternalKey_To_Identity_Provider.sql new file mode 100644 index 00000000000..50d03602e11 --- /dev/null +++ b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_110__Add_ExternalKey_To_Identity_Provider.sql @@ -0,0 +1,3 @@ +-- add column external_key for oauth2,oidc,saml2 IdPs +ALTER TABLE identity_provider ADD COLUMN external_key VARCHAR(512) DEFAULT NULL; +CREATE UNIQUE INDEX IF NOT EXISTS external_key_in_zone on identity_provider (identity_zone_id,type,external_key) WHERE external_key IS NOT NULL; \ No newline at end of file diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java index f6fca6c0c53..6022834edf3 100755 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java @@ -25,9 +25,12 @@ import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.cloudfoundry.identity.uaa.zone.IdentityZoneConfiguration; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; +import org.cloudfoundry.identity.uaa.zone.IdentityZoneProvisioning; import org.cloudfoundry.identity.uaa.zone.Links; import org.cloudfoundry.identity.uaa.zone.MultitenancyFixture; import org.cloudfoundry.identity.uaa.zone.MultitenantClientServices; +import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; +import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManagerImpl; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -104,6 +107,8 @@ class LoginInfoEndpointTests { private IdentityProviderProvisioning mockIdentityProviderProvisioning; private IdentityProvider uaaIdentityProvider; private IdentityZoneConfiguration originalConfiguration; + private IdentityZoneProvisioning identityZoneProvisioning; + private IdentityZoneManager identityZoneManager; private ExternalOAuthProviderConfigurator configurator; @BeforeEach @@ -119,6 +124,8 @@ void setUp() { when(mockSamlIdentityProviderConfigurator.getIdentityProviderDefinitionsForZone(any())).thenReturn(emptyList()); mockIdentityProviderProvisioning = mock(IdentityProviderProvisioning.class); uaaIdentityProvider = new IdentityProvider(); + identityZoneProvisioning = mock(IdentityZoneProvisioning.class); + identityZoneManager = new IdentityZoneManagerImpl(); when(mockIdentityProviderProvisioning.retrieveByOriginIgnoreActiveFlag(eq(OriginKeys.UAA), anyString())).thenReturn(uaaIdentityProvider); when(mockIdentityProviderProvisioning.retrieveByOrigin(eq(OriginKeys.LDAP), anyString())).thenReturn(new IdentityProvider()); idps = getIdps(); @@ -127,7 +134,7 @@ void setUp() { IdentityZoneHolder.get().setConfig(new IdentityZoneConfiguration()); UaaRandomStringUtil randomStringUtil = mock(UaaRandomStringUtil.class); when(randomStringUtil.getSecureRandom(anyInt())).thenReturn("01234567890123456789012345678901234567890123456789"); - configurator = new ExternalOAuthProviderConfigurator(mockIdentityProviderProvisioning, mockOidcMetadataFetcher, randomStringUtil); + configurator = new ExternalOAuthProviderConfigurator(mockIdentityProviderProvisioning, mockOidcMetadataFetcher, randomStringUtil, identityZoneProvisioning, identityZoneManager); extendedModelMap = new ExtendedModelMap(); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java index 754793bf8dd..0a47ca5b5d6 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java @@ -44,7 +44,10 @@ import org.cloudfoundry.identity.uaa.util.UaaRandomStringUtil; import org.cloudfoundry.identity.uaa.util.UaaTokenUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; +import org.cloudfoundry.identity.uaa.zone.IdentityZoneProvisioning; import org.cloudfoundry.identity.uaa.zone.MultitenancyFixture; +import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; +import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManagerImpl; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -142,6 +145,8 @@ class ExternalOAuthAuthenticationManagerIT { private InMemoryUaaUserDatabase userDatabase; private ExternalOAuthCodeToken xCodeToken; private ApplicationEventPublisher publisher; + private IdentityZoneProvisioning identityZoneProvisioning; + private IdentityZoneManager identityZoneManager; private static final String CODE = "the_code"; private static final String ORIGIN = "the_origin"; @@ -196,6 +201,8 @@ void setUp() throws Exception { IdentityZoneHolder.get().getConfig().getTokenPolicy().setKeys(Collections.singletonMap(keyName, PRIVATE_KEY)); provisioning = mock(IdentityProviderProvisioning.class); + identityZoneProvisioning = mock(IdentityZoneProvisioning.class); + identityZoneManager = new IdentityZoneManagerImpl(); ScimGroupExternalMembershipManager externalMembershipManager = mock(ScimGroupExternalMembershipManager.class); for (String scope : SCOPES_LIST) { @@ -219,7 +226,9 @@ void setUp() throws Exception { new ExternalOAuthProviderConfigurator( provisioning, oidcMetadataFetcher, - mock(UaaRandomStringUtil.class)) + mock(UaaRandomStringUtil.class), + identityZoneProvisioning, + identityZoneManager) ); externalOAuthAuthenticationManager = spy(new ExternalOAuthAuthenticationManager(externalOAuthProviderConfigurator, trustingRestTemplate, nonTrustingRestTemplate, tokenEndpointBuilder, new KeyInfoService(UAA_ISSUER_URL), oidcMetadataFetcher)); externalOAuthAuthenticationManager.setUserDatabase(userDatabase); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfiguratorTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfiguratorTests.java index e7d4a8327d0..1c9fc34b1d0 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfiguratorTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfiguratorTests.java @@ -9,6 +9,8 @@ import org.cloudfoundry.identity.uaa.util.AlphanumericRandomValueStringGenerator; import org.cloudfoundry.identity.uaa.util.UaaRandomStringUtil; import org.cloudfoundry.identity.uaa.zone.IdentityZone; +import org.cloudfoundry.identity.uaa.zone.IdentityZoneProvisioning; +import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; import org.hamcrest.Matchers; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -17,6 +19,7 @@ import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.web.util.UriComponentsBuilder; @@ -74,6 +77,10 @@ class ExternalOAuthProviderConfiguratorTests { private IdentityProviderProvisioning mockIdentityProviderProvisioning; @Mock private UaaRandomStringUtil mockUaaRandomStringUtil; + @Mock + private IdentityZoneProvisioning identityZoneProvisioning; + @Mock + private IdentityZoneManager identityZoneManager; private OIDCIdentityProviderDefinition config; private IdentityProvider oidcProvider; @@ -101,7 +108,9 @@ void setup() throws MalformedURLException { configurator = spy(new ExternalOAuthProviderConfigurator( mockIdentityProviderProvisioning, mockOidcMetadataFetcher, - mockUaaRandomStringUtil)); + mockUaaRandomStringUtil, + identityZoneProvisioning, + identityZoneManager)); config = new OIDCIdentityProviderDefinition(); config.setDiscoveryUrl(new URL("https://accounts.google.com/.well-known/openid-configuration")); @@ -151,6 +160,8 @@ void retrieve_by_issuer() throws Exception { when(mockIdentityProviderProvisioning.retrieveAll(eq(true), anyString())).thenReturn(Arrays.asList(oidcProvider, oauthProvider, new IdentityProvider<>().setType(LDAP))); String issuer = "https://accounts.google.com"; + when(identityZoneManager.getCurrentIdentityZoneId()).thenReturn(IdentityZone.getUaaZoneId()); + when(identityZoneManager.getCurrentIdentityZone()).thenReturn(IdentityZone.getUaa()); doAnswer(invocation -> { OIDCIdentityProviderDefinition definition = invocation.getArgument(0); definition.setIssuer(issuer); @@ -165,10 +176,89 @@ void retrieve_by_issuer() throws Exception { verify(configurator, times(1)).retrieveAll(eq(true), anyString()); } + @Test + void retrieve_by_issuer_search() throws Exception { + when(mockIdentityProviderProvisioning.retrieveByExternId(anyString(), anyString(), anyString())).thenReturn(oidcProvider); + + String issuer = "https://accounts.google.com"; + doAnswer(invocation -> { + OIDCIdentityProviderDefinition definition = invocation.getArgument(0); + definition.setIssuer(issuer); + return null; + }).when(mockOidcMetadataFetcher) + .fetchMetadataAndUpdateDefinition(any(OIDCIdentityProviderDefinition.class)); + + IdentityProvider activeExternalOAuthProvider = configurator.retrieveByIssuer(issuer, IdentityZone.getUaaZoneId()); + + assertEquals(issuer, activeExternalOAuthProvider.getConfig().getIssuer()); + verify(configurator, times(1)).overlay(config); + verify(configurator, times(1)).retrieveByExternId(anyString(), anyString(), anyString()); + } + + @Test + void retrieve_by_issuer_legacy() throws Exception { + when(mockIdentityProviderProvisioning.retrieveAll(eq(true), anyString())).thenReturn(Arrays.asList(oidcProvider, oauthProvider, new IdentityProvider<>().setType(LDAP))); + when(mockIdentityProviderProvisioning.retrieveByExternId(anyString(), anyString(), anyString())).thenThrow(new EmptyResultDataAccessException(1)); + + String issuer = "https://accounts.google.com"; + IdentityZone extraZone = IdentityZone.getUaa(); + extraZone.setId("customer"); + extraZone.setSubdomain("customer"); + when(identityZoneManager.getCurrentIdentityZoneId()).thenReturn(IdentityZone.getUaaZoneId()); + when(identityZoneProvisioning.retrieve("customer")).thenReturn(extraZone); + doAnswer(invocation -> { + OIDCIdentityProviderDefinition definition = invocation.getArgument(0); + definition.setIssuer(issuer); + return null; + }).when(mockOidcMetadataFetcher) + .fetchMetadataAndUpdateDefinition(any(OIDCIdentityProviderDefinition.class)); + + IdentityProvider activeExternalOAuthProvider = configurator.retrieveByIssuer(issuer, "customer"); + + assertEquals(issuer, activeExternalOAuthProvider.getConfig().getIssuer()); + verify(configurator, times(1)).overlay(config); + verify(configurator, times(1)).retrieveByExternId(anyString(), anyString(), anyString()); + verify(configurator, times(1)).retrieveAll(eq(true), anyString()); + } + + @Test + void retrieve_by_issuer_not_found_error() { + when(mockIdentityProviderProvisioning.retrieveByExternId(anyString(), anyString(), anyString())).thenThrow(new EmptyResultDataAccessException(1)); + + String issuer = "https://accounts.google.com"; + IdentityZone extraZone = IdentityZone.getUaa(); + extraZone.getConfig().getUserConfig().setAllowOriginLoop(false); + when(identityZoneManager.getCurrentIdentityZoneId()).thenReturn(IdentityZone.getUaaZoneId()); + when(identityZoneManager.getCurrentIdentityZone()).thenReturn(extraZone); + assertThrowsWithMessageThat( + IncorrectResultSizeDataAccessException.class, + () -> configurator.retrieveByIssuer(issuer, IdentityZone.getUaaZoneId()), + startsWith(String.format("No provider with unique issuer[%s] found", issuer)) + ); + } + + @Test + void retrieve_by_issuer_null_error() { + when(mockIdentityProviderProvisioning.retrieveByExternId(anyString(), anyString(), anyString())).thenReturn(null); + + String issuer = "https://accounts.google.com"; + IdentityZone extraZone = IdentityZone.getUaa(); + extraZone.getConfig().getUserConfig().setAllowOriginLoop(false); + when(identityZoneManager.getCurrentIdentityZoneId()).thenReturn(IdentityZone.getUaaZoneId()); + when(identityZoneManager.getCurrentIdentityZone()).thenReturn(extraZone); + assertThrowsWithMessageThat( + IncorrectResultSizeDataAccessException.class, + () -> configurator.retrieveByIssuer(issuer, IdentityZone.getUaaZoneId()), + startsWith(String.format("Active provider with unique issuer[%s] not found", issuer)) + ); + } + @Test void issuer_not_found() { String issuer = "https://accounts.google.com"; when(mockIdentityProviderProvisioning.retrieveAll(eq(true), anyString())).thenReturn(Arrays.asList(oauthProvider, new IdentityProvider<>().setType(LDAP))); + when(identityZoneManager.getCurrentIdentityZoneId()).thenReturn(IdentityZone.getUaaZoneId()); + when(identityZoneManager.getCurrentIdentityZone()).thenReturn(IdentityZone.getUaa()); assertThrowsWithMessageThat( IncorrectResultSizeDataAccessException.class, () -> configurator.retrieveByIssuer(issuer, IdentityZone.getUaaZoneId()), @@ -180,6 +270,8 @@ void issuer_not_found() { void duplicate_issuer_found() throws Exception { String issuer = "https://accounts.google.com"; when(mockIdentityProviderProvisioning.retrieveAll(eq(true), anyString())).thenReturn(Arrays.asList(oidcProvider, oidcProvider, oauthProvider, new IdentityProvider<>().setType(LDAP))); + when(identityZoneManager.getCurrentIdentityZoneId()).thenReturn(IdentityZone.getUaaZoneId()); + when(identityZoneManager.getCurrentIdentityZone()).thenReturn(IdentityZone.getUaa()); doAnswer(invocation -> { OIDCIdentityProviderDefinition definition = invocation.getArgument(0); definition.setIssuer(issuer); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderDataTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderDataTests.java index f249a0479fd..bcbe2ea2ef9 100755 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderDataTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderDataTests.java @@ -13,9 +13,13 @@ package org.cloudfoundry.identity.uaa.provider.saml; import org.cloudfoundry.identity.uaa.provider.AbstractIdentityProviderDefinition; +import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.provider.SamlIdentityProviderDefinition; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; +import org.opensaml.DefaultBootstrap; +import org.opensaml.xml.parse.BasicParserPool; import org.springframework.beans.factory.config.YamlMapFactoryBean; import org.springframework.beans.factory.config.YamlProcessor; import org.springframework.core.io.ByteArrayResource; @@ -27,6 +31,7 @@ import static java.util.Arrays.asList; import static org.hamcrest.Matchers.is; import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; public class BootstrapSamlIdentityProviderDataTests { @@ -142,9 +147,16 @@ public class BootstrapSamlIdentityProviderDataTests { " " + testXmlFileData.replace("\n","\n ") + "\n" ; + @BeforeClass + public static void initializeOpenSAML() throws Exception { + if (!org.apache.xml.security.Init.isInitialized()) { + DefaultBootstrap.bootstrap(); + } + } + @Before public void setUp() { - bootstrap = new BootstrapSamlIdentityProviderData(); + bootstrap = new BootstrapSamlIdentityProviderData(new SamlIdentityProviderConfigurator(new BasicParserPool(), mock(JdbcIdentityProviderProvisioning.class), mock(FixedHttpMetaDataProvider.class))); singleAdd = new SamlIdentityProviderDefinition() .setMetaDataLocation(String.format(BootstrapSamlIdentityProviderDataTests.xmlWithoutID, new RandomValueStringGenerator().generate())) .setIdpEntityAlias(singleAddAlias) 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 85dfbe5a505..edf6dad7330 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 @@ -19,6 +19,7 @@ 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.JdbcIdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.provider.SamlIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.provider.SlowHttpServer; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; @@ -26,6 +27,7 @@ import org.junit.jupiter.api.*; import org.junit.rules.ExpectedException; import org.opensaml.DefaultBootstrap; +import org.opensaml.saml2.metadata.provider.MetadataProviderException; import org.opensaml.xml.parse.BasicParserPool; import org.cloudfoundry.identity.uaa.oauth.common.util.RandomValueStringGenerator; import org.springframework.security.saml.trust.httpclient.TLSProtocolSocketFactory; @@ -37,6 +39,8 @@ import static java.time.Duration.ofSeconds; import static java.util.Arrays.asList; +import static org.cloudfoundry.identity.uaa.util.AssertThrowsWithMessage.assertThrowsWithMessageThat; +import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -122,7 +126,7 @@ private String getSimpleSamlPhpMetadata(String domain) { @BeforeEach public void setUp() { - bootstrap = new BootstrapSamlIdentityProviderData(); + bootstrap = new BootstrapSamlIdentityProviderData(new SamlIdentityProviderConfigurator(new BasicParserPool(), mock(JdbcIdentityProviderProvisioning.class), mock(FixedHttpMetaDataProvider.class))); singleAdd = new SamlIdentityProviderDefinition() .setMetaDataLocation(String.format(BootstrapSamlIdentityProviderDataTests.xmlWithoutID, new RandomValueStringGenerator().generate())) .setIdpEntityAlias(singleAddAlias) @@ -150,7 +154,7 @@ public void setUp() { @Test public void testAddNullProvider() { - Assertions.assertThrows(NullPointerException.class, () -> configurator.validateSamlIdentityProviderDefinition(null)); + Assertions.assertThrows(NullPointerException.class, () -> configurator.validateSamlIdentityProviderDefinition(null, true)); } @Test @@ -158,7 +162,7 @@ public void testAddNullProviderAlias() { singleAdd.setIdpEntityAlias(null); Assertions.assertThrows(NullPointerException.class, () -> { - configurator.validateSamlIdentityProviderDefinition(singleAdd); + configurator.validateSamlIdentityProviderDefinition(singleAdd, true); }); } @@ -182,6 +186,11 @@ public void testGetEntityID() throws Exception { } case "okta-local-2": { ComparableProvider provider = (ComparableProvider) configurator.getExtendedMetadataDelegateFromCache(def).getDelegate(); + IdentityProvider idp2 = mock(IdentityProvider.class); + when(idp2.getType()).thenReturn(OriginKeys.SAML); + when(idp2.getConfig()).thenReturn(def); + when(provisioning.retrieveActive(anyString())).thenReturn(asList(idp2)); + configurator.validateSamlIdentityProviderDefinition(def, true); assertEquals("http://www.okta.com/k2lw4l5bPODCMIIDBRYZ", provider.getEntityID()); break; } @@ -203,6 +212,24 @@ public void testGetEntityID() throws Exception { t.cancel(); } + @Test + void testGetEntityIDExists() { + bootstrap.setIdentityProviders(BootstrapSamlIdentityProviderDataTests.parseYaml(BootstrapSamlIdentityProviderDataTests.sampleYaml)); + bootstrap.afterPropertiesSet(); + for (SamlIdentityProviderDefinition def : bootstrap.getIdentityProviderDefinitions()) { + if ("okta-local-2".equalsIgnoreCase(def.getIdpEntityAlias())) { + IdentityProvider idp2 = mock(IdentityProvider.class); + when(idp2.getType()).thenReturn(OriginKeys.SAML); + when(idp2.getConfig()).thenReturn(def.clone().setIdpEntityAlias("okta-local-1")); + when(provisioning.retrieveActive(anyString())).thenReturn(Arrays.asList(idp2)); + assertThrowsWithMessageThat( + MetadataProviderException.class, + () -> configurator.validateSamlIdentityProviderDefinition(def, true), + startsWith("Duplicate entity ID:http://www.okta.com") + ); + } + } + } @Test public void testIdentityProviderDefinitionSocketFactoryTest() { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/test/TestUtils.java b/server/src/test/java/org/cloudfoundry/identity/uaa/test/TestUtils.java index 36348db6f0d..13ac9ef15c7 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/test/TestUtils.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/test/TestUtils.java @@ -92,7 +92,7 @@ private static void seedUaaZoneSimilarToHowTheRealFlywayMigrationDoesIt(JdbcTemp for (String origin : origins) { String identityProviderId = UUID.randomUUID().toString(); originMap.put(origin, identityProviderId); - jdbcTemplate.update("insert into identity_provider VALUES (?,?,?,0,?,?,?,?,null,?,null,null)",identityProviderId, t, t, uaa.getId(),origin,origin,origin,true); + jdbcTemplate.update("insert into identity_provider VALUES (?,?,?,0,?,?,?,?,null,?,null,null,null)",identityProviderId, t, t, uaa.getId(),origin,origin,origin,true); } jdbcTemplate.update("update oauth_client_details set identity_zone_id = ?",uaa.getId()); List clientIds = jdbcTemplate.queryForList("SELECT client_id from oauth_client_details", String.class); diff --git a/uaa/src/main/webapp/WEB-INF/spring-servlet.xml b/uaa/src/main/webapp/WEB-INF/spring-servlet.xml index 60fa3c3d283..bfadb4f3a3a 100755 --- a/uaa/src/main/webapp/WEB-INF/spring-servlet.xml +++ b/uaa/src/main/webapp/WEB-INF/spring-servlet.xml @@ -458,6 +458,7 @@ +