From 1b58db42687a8fbc8271f12b3cbbf01222dd7c3a Mon Sep 17 00:00:00 2001 From: d036670 Date: Mon, 17 Jun 2024 16:03:56 +0200 Subject: [PATCH 01/14] Add performance index for token resolution * add external_key to identity_provider * add index and optional search in retrieveByIssuer * add external_key for OAuth/OIDC and SAML IdP --- .../SamlIdentityProviderDefinition.java | 14 ++++ .../identity/uaa/zone/UserConfig.java | 10 +++ .../provider/IdentityProviderEndpoints.java | 2 +- .../IdentityProviderProvisioning.java | 2 + .../JdbcIdentityProviderProvisioning.java | 41 ++++++++---- .../ExternalOAuthProviderConfigurator.java | 64 +++++++++++++++++-- .../SamlIdentityProviderConfigurator.java | 28 +++++--- ...__Add_ExternalKey_To_Identity_Provider.sql | 3 + ...__Add_ExternalKey_To_Identity_Provider.sql | 3 + ...__Add_ExternalKey_To_Identity_Provider.sql | 3 + .../uaa/login/LoginInfoEndpointTests.java | 9 ++- .../ExternalOAuthAuthenticationManagerIT.java | 11 +++- ...xternalOAuthProviderConfiguratorTests.java | 16 ++++- .../identity/uaa/test/TestUtils.java | 2 +- 14 files changed, 176 insertions(+), 32 deletions(-) create mode 100644 server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_110__Add_ExternalKey_To_Identity_Provider.sql create mode 100644 server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_110__Add_ExternalKey_To_Identity_Provider.sql create mode 100644 server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_110__Add_ExternalKey_To_Identity_Provider.sql 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 isAllowAllOrigins() { + return this.allowAllOrigins; + } + + public void setAllowAllOrigins(final boolean allowAllOrigins) { + this.allowAllOrigins = allowAllOrigins; + } } 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 240ff723407..6c01aefed77 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 @@ -137,7 +137,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)); 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 0349fe729f6..2686d764468 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 @@ -29,6 +29,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 89f8eedf1de..0295131013d 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 @@ -11,7 +11,6 @@ import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; import org.springframework.stereotype.Component; -import org.springframework.util.Assert; import org.springframework.util.StringUtils; import java.sql.ResultSet; @@ -26,15 +25,15 @@ 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=?"; public static final String IDENTITY_ACTIVE_PROVIDERS_QUERY = IDENTITY_PROVIDERS_QUERY + " and active=?"; - 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=?"; @@ -48,6 +47,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(); @@ -85,9 +86,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 -> { @@ -103,7 +109,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()); @@ -113,7 +120,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; @@ -126,6 +133,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()); @@ -134,20 +142,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; } /** @@ -181,17 +194,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); + ((SamlIdentityProviderDefinition) definition).setIdpEntityId(externId); break; case OriginKeys.OAUTH20: definition = JsonUtils.readValue(config, RawExternalOAuthIdentityProviderDefinition.class); + ((RawExternalOAuthIdentityProviderDefinition) definition).setIssuer(externId); break; case OriginKeys.OIDC10: definition = JsonUtils.readValue(config, OIDCIdentityProviderDefinition.class); + ((OIDCIdentityProviderDefinition) definition).setIssuer(externId); break; case OriginKeys.UAA: definition = JsonUtils.readValue(config, UaaIdentityProviderDefinition.class); @@ -210,10 +231,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 7ae1f6ec846..8cbb33ddb4e 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 boolean isOriginLoopAllowed(String zoneId, boolean allowed) { + if (!allowed) { + return false; + } + 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::isAllowAllOrigins).orElse(true); + } + @Override public IdentityProvider create(IdentityProvider identityProvider, String zoneId) { return providerProvisioning.create(identityProvider, zoneId); @@ -150,11 +175,29 @@ public List retrieveActive(String zoneId) { } public IdentityProvider retrieveByIssuer(String issuer, String zoneId) throws IncorrectResultSizeDataAccessException { + IdentityProvider issuedProvider = null; + boolean originLoopAllowed = true; + 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) { + originLoopAllowed = isOriginLoopAllowed(zoneId, true); + if (!isOriginLoopAllowed(zoneId, originLoopAllowed)) { + throw new IncorrectResultSizeDataAccessException(String.format("No provider with unique issuer[%s] found", issuer), 1, 0, e); + } + } + if (!isOriginLoopAllowed(zoneId, originLoopAllowed) && 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) { @@ -194,6 +237,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/SamlIdentityProviderConfigurator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfigurator.java index 1cedd620cfc..856ce3f3fef 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; @@ -73,7 +73,7 @@ public List getIdentityProviderDefinitions(List< * @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 { + public synchronized String validateSamlIdentityProviderDefinition(SamlIdentityProviderDefinition providerDefinition) throws MetadataProviderException { ExtendedMetadataDelegate added, deleted = null; if (providerDefinition == null) { throw new NullPointerException(); @@ -91,26 +91,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 = 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..990b30d9338 --- /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 CLOB DEFAULT NULL; +CREATE UNIQUE INDEX identity_provider_ext_key_zid__idx 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..cc1d1ac5363 --- /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 TEXT DEFAULT NULL; +CREATE UNIQUE INDEX identity_provider_ext_key_zid__idx 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/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..cc1d1ac5363 --- /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 TEXT DEFAULT NULL; +CREATE UNIQUE INDEX identity_provider_ext_key_zid__idx on identity_provider (identity_zone_id,type,external_key); \ 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 6600d4e69af..63a501a3d01 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 @@ -43,7 +43,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; @@ -141,6 +144,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"; @@ -195,6 +200,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) { @@ -218,7 +225,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 e9624285b05..00a9d3ce09b 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 @@ -8,6 +8,8 @@ import org.cloudfoundry.identity.uaa.provider.RawExternalOAuthIdentityProviderDefinition; 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; @@ -69,6 +71,10 @@ class ExternalOAuthProviderConfiguratorTests { private IdentityProviderProvisioning mockIdentityProviderProvisioning; @Mock private UaaRandomStringUtil mockUaaRandomStringUtil; + @Mock + private IdentityZoneProvisioning identityZoneProvisioning; + @Mock + private IdentityZoneManager identityZoneManager; private OIDCIdentityProviderDefinition config; private IdentityProvider oidcProvider; @@ -96,7 +102,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")); @@ -146,6 +154,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); @@ -164,6 +174,8 @@ void retrieve_by_issuer() throws Exception { 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()), @@ -175,6 +187,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/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); From 823413a5d2bd6cb93fdbb39b433aa71d163d1414 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 18 Jun 2024 07:35:28 +0200 Subject: [PATCH 02/14] Change index for mysql --- .../db/mysql/V4_110__Add_ExternalKey_To_Identity_Provider.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index cc1d1ac5363..4c4b03b26de 100644 --- 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 @@ -1,3 +1,3 @@ -- add column external_key for oauth2,oidc,saml2 IdPs ALTER TABLE identity_provider ADD COLUMN external_key TEXT DEFAULT NULL; -CREATE UNIQUE INDEX identity_provider_ext_key_zid__idx on identity_provider (identity_zone_id,type,external_key); \ No newline at end of file +CREATE UNIQUE INDEX identity_provider_ext_key_zid__idx on identity_provider (identity_zone_id(36),type(8),external_key(724)); \ No newline at end of file From 0aeed8eaf9e8dd3f99f52c054e60e5cb7dfe6be7 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 18 Jun 2024 07:39:09 +0200 Subject: [PATCH 03/14] Add tests --- .../ExternalOAuthProviderConfigurator.java | 16 ++-- ...xternalOAuthProviderConfiguratorTests.java | 78 +++++++++++++++++++ 2 files changed, 86 insertions(+), 8 deletions(-) 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 8cbb33ddb4e..a1f0621a37a 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 @@ -137,9 +137,9 @@ private String getIdpUrlBase(final AbstractExternalOAuthIdentityProviderDefiniti return definition.getAuthUrl().toString(); } - private boolean isOriginLoopAllowed(String zoneId, boolean allowed) { - if (!allowed) { - return false; + private int isOriginLoopAllowed(String zoneId, int checkDone) { + if (checkDone > -1) { + return checkDone; } IdentityZoneConfiguration idzConfig; if (identityZoneManager.getCurrentIdentityZoneId().equals(zoneId)) { @@ -147,7 +147,7 @@ private boolean isOriginLoopAllowed(String zoneId, boolean allowed) { } else { idzConfig = identityZoneProvisioning.retrieve(zoneId).getConfig(); } - return idzConfig == null || Optional.of(idzConfig.getUserConfig()).map(UserConfig::isAllowAllOrigins).orElse(true); + return (idzConfig == null || Optional.of(idzConfig.getUserConfig()).map(UserConfig::isAllowAllOrigins).orElse(true)) ? 1 : 0; } @Override @@ -176,7 +176,7 @@ public List retrieveActive(String zoneId) { public IdentityProvider retrieveByIssuer(String issuer, String zoneId) throws IncorrectResultSizeDataAccessException { IdentityProvider issuedProvider = null; - boolean originLoopAllowed = true; + int originLoopCheckDone = -1; try { issuedProvider = retrieveByExternId(issuer, OIDC10, zoneId); if (issuedProvider != null && issuedProvider.isActive() @@ -185,12 +185,12 @@ public IdentityProvider retrieveByIssuer(String issuer, String zoneId) throws In return issuedProvider; } } catch (EmptyResultDataAccessException e) { - originLoopAllowed = isOriginLoopAllowed(zoneId, true); - if (!isOriginLoopAllowed(zoneId, originLoopAllowed)) { + 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, originLoopAllowed) && issuedProvider == null) { + 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) 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 00a9d3ce09b..50a51a9185c 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 @@ -16,6 +16,7 @@ import org.junit.jupiter.api.extension.ExtendWith; 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; @@ -170,6 +171,83 @@ 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(eq(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(eq(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() throws Exception { + when(mockIdentityProviderProvisioning.retrieveByExternId(anyString(), anyString(), anyString())).thenThrow(new EmptyResultDataAccessException(1)); + + String issuer = "https://accounts.google.com"; + IdentityZone extraZone = IdentityZone.getUaa(); + extraZone.getConfig().getUserConfig().setAllowAllOrigins(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() throws Exception { + when(mockIdentityProviderProvisioning.retrieveByExternId(anyString(), anyString(), anyString())).thenReturn(null); + + String issuer = "https://accounts.google.com"; + IdentityZone extraZone = IdentityZone.getUaa(); + extraZone.getConfig().getUserConfig().setAllowAllOrigins(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"; From 0bed30f20cb8129515993ecb787fd58e11521df4 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 18 Jun 2024 10:20:59 +0200 Subject: [PATCH 04/14] Set entityId during YAML configuration --- .../saml/BootstrapSamlIdentityProviderData.java | 17 ++++++++++++++++- .../BootstrapSamlIdentityProviderDataTests.java | 14 +++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) 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/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) From c294a770e9f15df6e3f2cbbe1bf31fe93b281346 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 18 Jun 2024 10:21:14 +0200 Subject: [PATCH 05/14] more tests --- .../config/IdentityProviderBootstrap.java | 1 - ...SamlIdentityProviderConfiguratorTests.java | 34 ++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/IdentityProviderBootstrap.java b/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/IdentityProviderBootstrap.java index 8d00dc21484..88fb8907566 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/IdentityProviderBootstrap.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/IdentityProviderBootstrap.java @@ -13,7 +13,6 @@ package org.cloudfoundry.identity.uaa.impl.config; -import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; 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..be476fa76e9 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) @@ -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(Arrays.asList(idp2)); + configurator.validateSamlIdentityProviderDefinition(def); assertEquals("http://www.okta.com/k2lw4l5bPODCMIIDBRYZ", provider.getEntityID()); break; } @@ -203,6 +212,29 @@ public void testGetEntityID() throws Exception { t.cancel(); } + @Test + public void testGetEntityIDExists() throws Exception { + + Timer t = new Timer(); + bootstrap.setIdentityProviders(BootstrapSamlIdentityProviderDataTests.parseYaml(BootstrapSamlIdentityProviderDataTests.sampleYaml)); + bootstrap.afterPropertiesSet(); + for (SamlIdentityProviderDefinition def : bootstrap.getIdentityProviderDefinitions()) { + switch (def.getIdpEntityAlias()) { + 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.clone().setIdpEntityAlias("okta-local-1")); + when(provisioning.retrieveActive(anyString())).thenReturn(Arrays.asList(idp2)); + assertThrowsWithMessageThat( + MetadataProviderException.class, + () -> configurator.validateSamlIdentityProviderDefinition(def), + startsWith("Duplicate entity ID:http://www.okta.com") + ); + } + } + } + } @Test public void testIdentityProviderDefinitionSocketFactoryTest() { From 94e35d405908331da0e119abea94b19b57eb09eb Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 18 Jun 2024 11:27:53 +0200 Subject: [PATCH 06/14] test fix --- .../uaa/provider/JdbcIdentityProviderProvisioning.java | 7 ++++--- .../saml/SamlIdentityProviderConfiguratorTests.java | 7 ++----- 2 files changed, 6 insertions(+), 8 deletions(-) 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 aa759a1efad..9520806f1cb 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") @@ -217,15 +218,15 @@ public IdentityProvider mapRow(ResultSet rs, int rowNum) throws SQLException { switch (identityProvider.getType()) { case OriginKeys.SAML: definition = JsonUtils.readValue(config, SamlIdentityProviderDefinition.class); - ((SamlIdentityProviderDefinition) definition).setIdpEntityId(externId); + Optional.ofNullable(definition).map(SamlIdentityProviderDefinition.class::cast).ifPresent(e -> e.setIdpEntityId(externId)); break; case OriginKeys.OAUTH20: definition = JsonUtils.readValue(config, RawExternalOAuthIdentityProviderDefinition.class); - ((RawExternalOAuthIdentityProviderDefinition) definition).setIssuer(externId); + Optional.ofNullable(definition).map(RawExternalOAuthIdentityProviderDefinition.class::cast).ifPresent(e -> e.setIssuer(externId)); break; case OriginKeys.OIDC10: definition = JsonUtils.readValue(config, OIDCIdentityProviderDefinition.class); - ((OIDCIdentityProviderDefinition) definition).setIssuer(externId); + Optional.ofNullable(definition).map(OIDCIdentityProviderDefinition.class::cast).ifPresent(e -> e.setIssuer(externId)); break; case OriginKeys.UAA: definition = JsonUtils.readValue(config, UaaIdentityProviderDefinition.class); 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 be476fa76e9..86a64bab598 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 @@ -189,7 +189,7 @@ public void testGetEntityID() throws Exception { IdentityProvider idp2 = mock(IdentityProvider.class); when(idp2.getType()).thenReturn(OriginKeys.SAML); when(idp2.getConfig()).thenReturn(def); - when(provisioning.retrieveActive(anyString())).thenReturn(Arrays.asList(idp2)); + when(provisioning.retrieveActive(anyString())).thenReturn(asList(idp2)); configurator.validateSamlIdentityProviderDefinition(def); assertEquals("http://www.okta.com/k2lw4l5bPODCMIIDBRYZ", provider.getEntityID()); break; @@ -213,15 +213,12 @@ public void testGetEntityID() throws Exception { } @Test - public void testGetEntityIDExists() throws Exception { - - Timer t = new Timer(); + void testGetEntityIDExists() throws Exception { bootstrap.setIdentityProviders(BootstrapSamlIdentityProviderDataTests.parseYaml(BootstrapSamlIdentityProviderDataTests.sampleYaml)); bootstrap.afterPropertiesSet(); for (SamlIdentityProviderDefinition def : bootstrap.getIdentityProviderDefinitions()) { switch (def.getIdpEntityAlias()) { 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.clone().setIdpEntityAlias("okta-local-1")); From 3e2a35ec55f719507bfea46c763c9ab35df43a8e Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 18 Jun 2024 13:51:32 +0200 Subject: [PATCH 07/14] Add documentation --- .../identity/uaa/mock/zones/IdentityZoneEndpointDocs.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java index 3895ca60425..91dbaf854b3 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java @@ -127,6 +127,7 @@ class IdentityZoneEndpointDocs extends EndpointDocs { private static final String USER_CONFIG_USER_LIMIT_DESCRIPTION = "Number of users in the zone. If more than 0, it limits the amount of users in the zone. (defaults to -1, no limit)."; private static final String USER_CONFIG_USER_LIMIT_CONSTRAINT = "Optional number, default -1, no limit."; private static final String USER_CONFIG_CHECK_ORIGIN_ENABLED = "Flag for switching on the check if origin is valid when creating or updating users (defaults to false)"; + private static final String USER_CONFIG_ALLOW_ORIGIN_LOOP = "Flag for switching off the loop over all origins in a zone (defaults to true)"; private static final String SERVICE_PROVIDER_KEY = "-----BEGIN RSA PRIVATE KEY-----\n" + @@ -311,6 +312,7 @@ void createIdentityZone() throws Exception { fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")).optional().type(ARRAY), fieldWithPath("config.userConfig.maxUsers").description(USER_CONFIG_USER_LIMIT_DESCRIPTION).attributes(key("constraints").value(USER_CONFIG_USER_LIMIT_CONSTRAINT)).optional().type(NUMBER), fieldWithPath("config.userConfig.checkOriginEnabled").description(USER_CONFIG_CHECK_ORIGIN_ENABLED).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), + fieldWithPath("config.userConfig.allowAllOrigins").description(USER_CONFIG_ALLOW_ORIGIN_LOOP).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), fieldWithPath("created").ignored(), fieldWithPath("last_modified").ignored() @@ -473,6 +475,7 @@ void getAllIdentityZones() throws Exception { fieldWithPath("[].config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")).optional().type(ARRAY), fieldWithPath("[].config.userConfig.maxUsers").description(USER_CONFIG_USER_LIMIT_DESCRIPTION).attributes(key("constraints").value(USER_CONFIG_USER_LIMIT_CONSTRAINT)).optional().type(NUMBER), fieldWithPath("[].config.userConfig.checkOriginEnabled").description(USER_CONFIG_CHECK_ORIGIN_ENABLED).attributes(key("constraints").value("optional")).optional().type(BOOLEAN), + fieldWithPath("[].config.userConfig.allowAllOrigins").description(USER_CONFIG_ALLOW_ORIGIN_LOOP).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), fieldWithPath("[].created").ignored(), fieldWithPath("[].last_modified").ignored() @@ -619,6 +622,7 @@ void updateIdentityZone() throws Exception { fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")).optional().type(ARRAY), fieldWithPath("config.userConfig.maxUsers").description(USER_CONFIG_USER_LIMIT_DESCRIPTION).attributes(key("constraints").value(USER_CONFIG_USER_LIMIT_CONSTRAINT)).optional().type(NUMBER), fieldWithPath("config.userConfig.checkOriginEnabled").description(USER_CONFIG_CHECK_ORIGIN_ENABLED).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), + fieldWithPath("config.userConfig.allowAllOrigins").description(USER_CONFIG_ALLOW_ORIGIN_LOOP).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), fieldWithPath("created").ignored(), fieldWithPath("last_modified").ignored() @@ -802,6 +806,7 @@ private Snippet getResponseFields() { fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).optional().type(ARRAY), fieldWithPath("config.userConfig.maxUsers").description(USER_CONFIG_USER_LIMIT_DESCRIPTION), fieldWithPath("config.userConfig.checkOriginEnabled").description(USER_CONFIG_CHECK_ORIGIN_ENABLED).optional().type(BOOLEAN), + fieldWithPath("config.userConfig.allowAllOrigins").description(USER_CONFIG_ALLOW_ORIGIN_LOOP).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), fieldWithPath("created").ignored(), fieldWithPath("last_modified").ignored() From 2c3862360da24dbe20cee92c671f672dd2f7b178 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 18 Jun 2024 13:52:06 +0200 Subject: [PATCH 08/14] Check entityID only in creation --- .../identity/uaa/provider/IdentityProviderEndpoints.java | 4 ++-- .../provider/saml/SamlIdentityProviderConfigurator.java | 5 +++-- .../saml/SamlIdentityProviderConfiguratorTests.java | 8 ++++---- 3 files changed, 9 insertions(+), 8 deletions(-) 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 6c01aefed77..cd8af02e541 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 @@ -137,7 +137,7 @@ public ResponseEntity createIdentityProvider(@RequestBody Iden SamlIdentityProviderDefinition definition = ObjectUtils.castInstance(body.getConfig(), SamlIdentityProviderDefinition.class); definition.setZoneId(zoneId); definition.setIdpEntityAlias(body.getOriginKey()); - definition.setIdpEntityId(samlConfigurator.validateSamlIdentityProviderDefinition(definition)); + definition.setIdpEntityId(samlConfigurator.validateSamlIdentityProviderDefinition(definition, true)); body.setConfig(definition); } @@ -222,7 +222,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/saml/SamlIdentityProviderConfigurator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfigurator.java index 856ce3f3fef..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 @@ -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 String 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,7 +92,7 @@ public synchronized String validateSamlIdentityProviderDefinition(SamlIdentityPr throw new MetadataProviderException("Emtpy entityID for SAML provider with zoneId:" + providerDefinition.getZoneId() + " and origin:" + providerDefinition.getIdpEntityAlias()); } - boolean entityIDexists = entityIdExists(entityIDToBeAdded, providerDefinition.getZoneId()); + boolean entityIDexists = creation && entityIdExists(entityIDToBeAdded, providerDefinition.getZoneId()); if (!entityIDexists) { for (SamlIdentityProviderDefinition existing : getIdentityProviderDefinitions()) { 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 86a64bab598..777da58a87f 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 @@ -154,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 @@ -162,7 +162,7 @@ public void testAddNullProviderAlias() { singleAdd.setIdpEntityAlias(null); Assertions.assertThrows(NullPointerException.class, () -> { - configurator.validateSamlIdentityProviderDefinition(singleAdd); + configurator.validateSamlIdentityProviderDefinition(singleAdd, true); }); } @@ -190,7 +190,7 @@ public void testGetEntityID() throws Exception { when(idp2.getType()).thenReturn(OriginKeys.SAML); when(idp2.getConfig()).thenReturn(def); when(provisioning.retrieveActive(anyString())).thenReturn(asList(idp2)); - configurator.validateSamlIdentityProviderDefinition(def); + configurator.validateSamlIdentityProviderDefinition(def, true); assertEquals("http://www.okta.com/k2lw4l5bPODCMIIDBRYZ", provider.getEntityID()); break; } @@ -225,7 +225,7 @@ void testGetEntityIDExists() throws Exception { when(provisioning.retrieveActive(anyString())).thenReturn(Arrays.asList(idp2)); assertThrowsWithMessageThat( MetadataProviderException.class, - () -> configurator.validateSamlIdentityProviderDefinition(def), + () -> configurator.validateSamlIdentityProviderDefinition(def, true), startsWith("Duplicate entity ID:http://www.okta.com") ); } From d6f0ce52fff94a8ecc82d74ab965b229dca687b2 Mon Sep 17 00:00:00 2001 From: d036670 Date: Wed, 26 Jun 2024 08:35:00 +0200 Subject: [PATCH 09/14] Use partial index on postgresql only postgres supports them --- .../postgresql/V4_110__Add_ExternalKey_To_Identity_Provider.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index cc1d1ac5363..ce5baec0f6f 100644 --- 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 @@ -1,3 +1,3 @@ -- add column external_key for oauth2,oidc,saml2 IdPs ALTER TABLE identity_provider ADD COLUMN external_key TEXT DEFAULT NULL; -CREATE UNIQUE INDEX identity_provider_ext_key_zid__idx on identity_provider (identity_zone_id,type,external_key); \ No newline at end of file +CREATE UNIQUE INDEX identity_provider_ext_key_zid__idx on identity_provider (identity_zone_id,type,external_key) WHERE external_key IS NOT NULL; \ No newline at end of file From 7feee63f502a5006442f81fbd288197590e0b051 Mon Sep 17 00:00:00 2001 From: d036670 Date: Wed, 26 Jun 2024 11:21:37 +0200 Subject: [PATCH 10/14] Refactorings - shorten the index name. Similar name pattern then existing --- .../db/hsqldb/V4_110__Add_ExternalKey_To_Identity_Provider.sql | 2 +- .../db/mysql/V4_110__Add_ExternalKey_To_Identity_Provider.sql | 2 +- .../postgresql/V4_110__Add_ExternalKey_To_Identity_Provider.sql | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 index 990b30d9338..59f464d43be 100644 --- 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 @@ -1,3 +1,3 @@ -- add column external_key for oauth2,oidc,saml2 IdPs ALTER TABLE identity_provider ADD COLUMN external_key CLOB DEFAULT NULL; -CREATE UNIQUE INDEX identity_provider_ext_key_zid__idx on identity_provider (identity_zone_id,type,external_key); \ No newline at end of file +CREATE UNIQUE INDEX 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 index 4c4b03b26de..cba304263fb 100644 --- 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 @@ -1,3 +1,3 @@ -- add column external_key for oauth2,oidc,saml2 IdPs ALTER TABLE identity_provider ADD COLUMN external_key TEXT DEFAULT NULL; -CREATE UNIQUE INDEX identity_provider_ext_key_zid__idx on identity_provider (identity_zone_id(36),type(8),external_key(724)); \ No newline at end of file +CREATE UNIQUE INDEX external_key_in_zone on identity_provider (identity_zone_id(36),type(8),external_key(724)); \ 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 index ce5baec0f6f..7791283a409 100644 --- 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 @@ -1,3 +1,3 @@ -- add column external_key for oauth2,oidc,saml2 IdPs ALTER TABLE identity_provider ADD COLUMN external_key TEXT DEFAULT NULL; -CREATE UNIQUE INDEX identity_provider_ext_key_zid__idx on identity_provider (identity_zone_id,type,external_key) WHERE external_key IS NOT NULL; \ No newline at end of file +CREATE UNIQUE INDEX 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 From bcf65dbad4bdd5d9c71b1b7a63db1d2b0f53b331 Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 12 Jul 2024 16:05:47 +0200 Subject: [PATCH 11/14] Change external_key to VARCHAR Adopt index --- .../hsqldb/V4_110__Add_ExternalKey_To_Identity_Provider.sql | 4 ++-- .../db/mysql/V4_110__Add_ExternalKey_To_Identity_Provider.sql | 4 ++-- .../V4_110__Add_ExternalKey_To_Identity_Provider.sql | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) 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 index 59f464d43be..e582bc61d16 100644 --- 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 @@ -1,3 +1,3 @@ -- add column external_key for oauth2,oidc,saml2 IdPs -ALTER TABLE identity_provider ADD COLUMN external_key CLOB DEFAULT NULL; -CREATE UNIQUE INDEX external_key_in_zone on identity_provider (identity_zone_id,type,external_key); \ No newline at end of file +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 index cba304263fb..dcb9812b4a6 100644 --- 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 @@ -1,3 +1,3 @@ -- add column external_key for oauth2,oidc,saml2 IdPs -ALTER TABLE identity_provider ADD COLUMN external_key TEXT DEFAULT NULL; -CREATE UNIQUE INDEX external_key_in_zone on identity_provider (identity_zone_id(36),type(8),external_key(724)); \ No newline at end of file +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 index 7791283a409..50d03602e11 100644 --- 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 @@ -1,3 +1,3 @@ -- add column external_key for oauth2,oidc,saml2 IdPs -ALTER TABLE identity_provider ADD COLUMN external_key TEXT DEFAULT NULL; -CREATE UNIQUE INDEX 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 +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 From 341bb3d55d1d924b5025efeea94562ea22d43f00 Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 12 Jul 2024 16:06:13 +0200 Subject: [PATCH 12/14] Add allowOriginLoop yaml configuration option --- .../org/cloudfoundry/identity/uaa/zone/UserConfig.java | 10 +++++----- .../oauth/ExternalOAuthProviderConfigurator.java | 2 +- .../oauth/ExternalOAuthProviderConfiguratorTests.java | 4 ++-- uaa/src/main/webapp/WEB-INF/spring-servlet.xml | 1 + 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java b/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java index 60f5718dfad..19d55f84c89 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java @@ -31,7 +31,7 @@ public class UserConfig { private boolean checkOriginEnabled; - private boolean allowAllOrigins = true; + private boolean allowOriginLoop = true; public List getDefaultGroups() { return defaultGroups; @@ -81,11 +81,11 @@ public void setCheckOriginEnabled(boolean checkOriginEnabled) { this.checkOriginEnabled = checkOriginEnabled; } - public boolean isAllowAllOrigins() { - return this.allowAllOrigins; + public boolean isAllowOriginLoop() { + return this.allowOriginLoop; } - public void setAllowAllOrigins(final boolean allowAllOrigins) { - this.allowAllOrigins = allowAllOrigins; + public void setAllowOriginLoop(final boolean allowAllOrigins) { + this.allowOriginLoop = allowAllOrigins; } } 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 578c2264091..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 @@ -147,7 +147,7 @@ private int isOriginLoopAllowed(String zoneId, int checkDone) { } else { idzConfig = identityZoneProvisioning.retrieve(zoneId).getConfig(); } - return (idzConfig == null || Optional.of(idzConfig.getUserConfig()).map(UserConfig::isAllowAllOrigins).orElse(true)) ? 1 : 0; + return (idzConfig == null || Optional.of(idzConfig.getUserConfig()).map(UserConfig::isAllowOriginLoop).orElse(true)) ? 1 : 0; } @Override 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 e6ffcd72b01..890054e04d2 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 @@ -227,7 +227,7 @@ void retrieve_by_issuer_not_found_error() throws Exception { String issuer = "https://accounts.google.com"; IdentityZone extraZone = IdentityZone.getUaa(); - extraZone.getConfig().getUserConfig().setAllowAllOrigins(false); + extraZone.getConfig().getUserConfig().setAllowOriginLoop(false); when(identityZoneManager.getCurrentIdentityZoneId()).thenReturn(IdentityZone.getUaaZoneId()); when(identityZoneManager.getCurrentIdentityZone()).thenReturn(extraZone); assertThrowsWithMessageThat( @@ -243,7 +243,7 @@ void retrieve_by_issuer_null_error() throws Exception { String issuer = "https://accounts.google.com"; IdentityZone extraZone = IdentityZone.getUaa(); - extraZone.getConfig().getUserConfig().setAllowAllOrigins(false); + extraZone.getConfig().getUserConfig().setAllowOriginLoop(false); when(identityZoneManager.getCurrentIdentityZoneId()).thenReturn(IdentityZone.getUaaZoneId()); when(identityZoneManager.getCurrentIdentityZone()).thenReturn(extraZone); assertThrowsWithMessageThat( 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 @@ + Date: Sat, 13 Jul 2024 09:26:36 +0200 Subject: [PATCH 13/14] adopt rename in docs --- .../identity/uaa/mock/zones/IdentityZoneEndpointDocs.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java index 91dbaf854b3..37bf12c1249 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java @@ -312,7 +312,7 @@ void createIdentityZone() throws Exception { fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")).optional().type(ARRAY), fieldWithPath("config.userConfig.maxUsers").description(USER_CONFIG_USER_LIMIT_DESCRIPTION).attributes(key("constraints").value(USER_CONFIG_USER_LIMIT_CONSTRAINT)).optional().type(NUMBER), fieldWithPath("config.userConfig.checkOriginEnabled").description(USER_CONFIG_CHECK_ORIGIN_ENABLED).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), - fieldWithPath("config.userConfig.allowAllOrigins").description(USER_CONFIG_ALLOW_ORIGIN_LOOP).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), + fieldWithPath("config.userConfig.allowOriginLoop").description(USER_CONFIG_ALLOW_ORIGIN_LOOP).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), fieldWithPath("created").ignored(), fieldWithPath("last_modified").ignored() @@ -475,7 +475,7 @@ void getAllIdentityZones() throws Exception { fieldWithPath("[].config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")).optional().type(ARRAY), fieldWithPath("[].config.userConfig.maxUsers").description(USER_CONFIG_USER_LIMIT_DESCRIPTION).attributes(key("constraints").value(USER_CONFIG_USER_LIMIT_CONSTRAINT)).optional().type(NUMBER), fieldWithPath("[].config.userConfig.checkOriginEnabled").description(USER_CONFIG_CHECK_ORIGIN_ENABLED).attributes(key("constraints").value("optional")).optional().type(BOOLEAN), - fieldWithPath("[].config.userConfig.allowAllOrigins").description(USER_CONFIG_ALLOW_ORIGIN_LOOP).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), + fieldWithPath("[].config.userConfig.allowOriginLoop").description(USER_CONFIG_ALLOW_ORIGIN_LOOP).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), fieldWithPath("[].created").ignored(), fieldWithPath("[].last_modified").ignored() @@ -622,7 +622,7 @@ void updateIdentityZone() throws Exception { fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")).optional().type(ARRAY), fieldWithPath("config.userConfig.maxUsers").description(USER_CONFIG_USER_LIMIT_DESCRIPTION).attributes(key("constraints").value(USER_CONFIG_USER_LIMIT_CONSTRAINT)).optional().type(NUMBER), fieldWithPath("config.userConfig.checkOriginEnabled").description(USER_CONFIG_CHECK_ORIGIN_ENABLED).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), - fieldWithPath("config.userConfig.allowAllOrigins").description(USER_CONFIG_ALLOW_ORIGIN_LOOP).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), + fieldWithPath("config.userConfig.allowOriginLoop").description(USER_CONFIG_ALLOW_ORIGIN_LOOP).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), fieldWithPath("created").ignored(), fieldWithPath("last_modified").ignored() @@ -806,7 +806,7 @@ private Snippet getResponseFields() { fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).optional().type(ARRAY), fieldWithPath("config.userConfig.maxUsers").description(USER_CONFIG_USER_LIMIT_DESCRIPTION), fieldWithPath("config.userConfig.checkOriginEnabled").description(USER_CONFIG_CHECK_ORIGIN_ENABLED).optional().type(BOOLEAN), - fieldWithPath("config.userConfig.allowAllOrigins").description(USER_CONFIG_ALLOW_ORIGIN_LOOP).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), + fieldWithPath("config.userConfig.allowOriginLoop").description(USER_CONFIG_ALLOW_ORIGIN_LOOP).attributes(key("constraints").value("Optional")).optional().type(BOOLEAN), fieldWithPath("created").ignored(), fieldWithPath("last_modified").ignored() From a6f467edf6a284dcfce558907c10e354804835ec Mon Sep 17 00:00:00 2001 From: d036670 Date: Sat, 13 Jul 2024 09:44:47 +0200 Subject: [PATCH 14/14] test improvements --- .../identity/uaa/zone/UserConfigTest.java | 16 +++++++++++++ ...xternalOAuthProviderConfiguratorTests.java | 8 +++---- ...SamlIdentityProviderConfiguratorTests.java | 24 +++++++++---------- 3 files changed, 31 insertions(+), 17 deletions(-) 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/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfiguratorTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfiguratorTests.java index 890054e04d2..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 @@ -191,7 +191,7 @@ void retrieve_by_issuer_search() throws Exception { IdentityProvider activeExternalOAuthProvider = configurator.retrieveByIssuer(issuer, IdentityZone.getUaaZoneId()); assertEquals(issuer, activeExternalOAuthProvider.getConfig().getIssuer()); - verify(configurator, times(1)).overlay(eq(config)); + verify(configurator, times(1)).overlay(config); verify(configurator, times(1)).retrieveByExternId(anyString(), anyString(), anyString()); } @@ -216,13 +216,13 @@ void retrieve_by_issuer_legacy() throws Exception { IdentityProvider activeExternalOAuthProvider = configurator.retrieveByIssuer(issuer, "customer"); assertEquals(issuer, activeExternalOAuthProvider.getConfig().getIssuer()); - verify(configurator, times(1)).overlay(eq(config)); + 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() throws Exception { + void retrieve_by_issuer_not_found_error() { when(mockIdentityProviderProvisioning.retrieveByExternId(anyString(), anyString(), anyString())).thenThrow(new EmptyResultDataAccessException(1)); String issuer = "https://accounts.google.com"; @@ -238,7 +238,7 @@ void retrieve_by_issuer_not_found_error() throws Exception { } @Test - void retrieve_by_issuer_null_error() throws Exception { + void retrieve_by_issuer_null_error() { when(mockIdentityProviderProvisioning.retrieveByExternId(anyString(), anyString(), anyString())).thenReturn(null); String issuer = "https://accounts.google.com"; 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 777da58a87f..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 @@ -213,22 +213,20 @@ public void testGetEntityID() throws Exception { } @Test - void testGetEntityIDExists() throws Exception { + void testGetEntityIDExists() { bootstrap.setIdentityProviders(BootstrapSamlIdentityProviderDataTests.parseYaml(BootstrapSamlIdentityProviderDataTests.sampleYaml)); bootstrap.afterPropertiesSet(); for (SamlIdentityProviderDefinition def : bootstrap.getIdentityProviderDefinitions()) { - switch (def.getIdpEntityAlias()) { - case "okta-local-2": { - 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") - ); - } + 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") + ); } } }