Skip to content

Commit

Permalink
Add performance index for token resolution
Browse files Browse the repository at this point in the history
* add external_key to identity_provider
* add index and optional search in retrieveByIssuer
* add external_key for OAuth/OIDC and SAML IdP
  • Loading branch information
strehle committed Jun 17, 2024
1 parent 95ad028 commit 1b58db4
Show file tree
Hide file tree
Showing 14 changed files with 176 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ public enum ExternalGroupMappingMode {
private boolean skipSslValidation = false;
private List<String> authnContext;

@JsonIgnore
private String idpEntityId;

public SamlIdentityProviderDefinition() {}

public SamlIdentityProviderDefinition clone() {
Expand All @@ -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);
Expand Down Expand Up @@ -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("<!DOCTYPE")) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public class UserConfig {

private boolean checkOriginEnabled;

private boolean allowAllOrigins = true;

public List<String> getDefaultGroups() {
return defaultGroups;
}
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public ResponseEntity<IdentityProvider> 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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=?";

Expand All @@ -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<IdentityProvider> mapper = new IdentityProviderRowMapper();
Expand Down Expand Up @@ -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 -> {
Expand All @@ -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());
Expand All @@ -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;

Expand All @@ -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());
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -150,11 +175,29 @@ public List<IdentityProvider> 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<IdentityProvider> 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) {
Expand Down Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 1b58db4

Please sign in to comment.