Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add performance index for token resolution #2932

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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;
Expand Down
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, true));
body.setConfig(definition);
}

Expand Down Expand Up @@ -222,7 +222,7 @@ public ResponseEntity<IdentityProvider> 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public interface IdentityProviderProvisioning {

IdentityProvider retrieveByOrigin(String origin, String zoneId);

IdentityProvider retrieveByExternId(String externId, String type, String zoneId);

default IdentityProvider retrieveByOriginIgnoreActiveFlag(String origin, String zoneId) {
return retrieveByOrigin(origin, zoneId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,25 @@
import java.sql.Timestamp;
import java.util.Date;
import java.util.List;
import java.util.Optional;
import java.util.UUID;

@Component("identityProviderProvisioning")
public class JdbcIdentityProviderProvisioning implements IdentityProviderProvisioning, SystemDeletable {

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 IDP_WITH_ALIAS_EXISTS_QUERY = "select 1 from identity_provider idp where idp.identity_zone_id = ? and idp.alias_zid is not null and idp.alias_zid <> '' limit 1";

public static final String ID_PROVIDER_UPDATE_FIELDS = "version,lastmodified,name,type,config,active,alias_id,alias_zid".replace(",", "=?,") + "=?";
public static final String ID_PROVIDER_UPDATE_FIELDS = "version,lastmodified,name,type,config,active,alias_id,alias_zid,external_key".replace(",", "=?,") + "=?";

public static final String UPDATE_IDENTITY_PROVIDER_SQL = "update identity_provider set " + ID_PROVIDER_UPDATE_FIELDS + " where id=? and identity_zone_id=?";

Expand All @@ -51,6 +52,8 @@ public class JdbcIdentityProviderProvisioning implements IdentityProviderProvisi

public static final String IDENTITY_PROVIDER_BY_ORIGIN_QUERY_ACTIVE = IDENTITY_PROVIDER_BY_ORIGIN_QUERY + " and active = ? ";

public static final String IDENTITY_PROVIDER_BY_EXTERNAL_QUERY = IDENTITY_PROVIDERS_QUERY + " and type=? and external_key=?";

protected final JdbcTemplate jdbcTemplate;

private final RowMapper<IdentityProvider> mapper = new IdentityProviderRowMapper();
Expand Down Expand Up @@ -100,9 +103,14 @@ public IdentityProvider retrieveByOriginIgnoreActiveFlag(String origin, String z
return jdbcTemplate.queryForObject(IDENTITY_PROVIDER_BY_ORIGIN_QUERY, mapper, origin, zoneId);
}

@Override
public IdentityProvider retrieveByExternId(String externId, String type, String zoneId) {
return jdbcTemplate.queryForObject(IDENTITY_PROVIDER_BY_EXTERNAL_QUERY, mapper, zoneId, type, externId);
}

@Override
public IdentityProvider create(final IdentityProvider identityProvider, String zoneId) {
validate(identityProvider);
String externId = validate(identityProvider);
final String id = UUID.randomUUID().toString();
try {
jdbcTemplate.update(CREATE_IDENTITY_PROVIDER_SQL, ps -> {
Expand All @@ -118,7 +126,8 @@ public IdentityProvider create(final IdentityProvider identityProvider, String z
ps.setString(pos++, zoneId);
ps.setBoolean(pos++, identityProvider.isActive());
ps.setString(pos++, identityProvider.getAliasId());
ps.setString(pos, identityProvider.getAliasZid());
ps.setString(pos++, identityProvider.getAliasZid());
ps.setString(pos, externId);
});
} catch (DuplicateKeyException e) {
throw new IdpAlreadyExistsException(e.getMostSpecificCause().getMessage());
Expand All @@ -128,7 +137,7 @@ public IdentityProvider create(final IdentityProvider identityProvider, String z

@Override
public IdentityProvider update(final IdentityProvider identityProvider, String zoneId) {
validate(identityProvider);
String externId = validate(identityProvider);
jdbcTemplate.update(UPDATE_IDENTITY_PROVIDER_SQL, ps -> {
int pos = 1;

Expand All @@ -141,6 +150,7 @@ public IdentityProvider update(final IdentityProvider identityProvider, String z
ps.setBoolean(pos++, identityProvider.isActive());
ps.setString(pos++, identityProvider.getAliasId());
ps.setString(pos++, identityProvider.getAliasZid());
ps.setString(pos++, externId);

// placeholders in WHERE
ps.setString(pos++, identityProvider.getId().trim());
Expand All @@ -149,20 +159,25 @@ public IdentityProvider update(final IdentityProvider identityProvider, String z
return retrieve(identityProvider.getId(), zoneId);
}

protected void validate(IdentityProvider provider) {
private String validate(IdentityProvider provider) {
if (provider == null) {
throw new NullPointerException("Provider can not be null.");
}
if (!StringUtils.hasText(provider.getIdentityZoneId())) {
throw new DataIntegrityViolationException("Identity zone ID must be set.");
}
String externId = null;
//ensure that SAML IDPs have redundant fields synchronized
if (OriginKeys.SAML.equals(provider.getType()) && provider.getConfig() != null) {
SamlIdentityProviderDefinition saml = ObjectUtils.castInstance(provider.getConfig(), SamlIdentityProviderDefinition.class);
saml.setIdpEntityAlias(provider.getOriginKey());
saml.setZoneId(provider.getIdentityZoneId());
provider.setConfig(saml);
externId = saml.getIdpEntityId();
} else if (provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition<?> externalOAuthIdentityProviderDefinition) {
externId = externalOAuthIdentityProviderDefinition.getIssuer();
}
return externId;
}

@Override
Expand Down Expand Up @@ -193,17 +208,25 @@ public IdentityProvider mapRow(ResultSet rs, int rowNum) throws SQLException {
identityProvider.setOriginKey(rs.getString(pos++));
identityProvider.setType(rs.getString(pos++));
String config = rs.getString(pos++);
identityProvider.setIdentityZoneId(rs.getString(pos++));
identityProvider.setActive(rs.getBoolean(pos++));
identityProvider.setAliasId(rs.getString(pos++));
identityProvider.setAliasZid(rs.getString(pos++));
String externId = rs.getString(pos);
if (StringUtils.hasText(config)) {
AbstractIdentityProviderDefinition definition;
switch (identityProvider.getType()) {
case OriginKeys.SAML:
definition = JsonUtils.readValue(config, SamlIdentityProviderDefinition.class);
Optional.ofNullable(definition).map(SamlIdentityProviderDefinition.class::cast).ifPresent(e -> e.setIdpEntityId(externId));
break;
case OriginKeys.OAUTH20:
definition = JsonUtils.readValue(config, RawExternalOAuthIdentityProviderDefinition.class);
Optional.ofNullable(definition).map(RawExternalOAuthIdentityProviderDefinition.class::cast).ifPresent(e -> e.setIssuer(externId));
break;
case OriginKeys.OIDC10:
definition = JsonUtils.readValue(config, OIDCIdentityProviderDefinition.class);
Optional.ofNullable(definition).map(OIDCIdentityProviderDefinition.class::cast).ifPresent(e -> e.setIssuer(externId));
break;
case OriginKeys.UAA:
definition = JsonUtils.readValue(config, UaaIdentityProviderDefinition.class);
Expand All @@ -222,10 +245,6 @@ public IdentityProvider mapRow(ResultSet rs, int rowNum) throws SQLException {
identityProvider.setConfig(definition);
}
}
identityProvider.setIdentityZoneId(rs.getString(pos++));
identityProvider.setActive(rs.getBoolean(pos++));
identityProvider.setAliasId(rs.getString(pos++));
identityProvider.setAliasZid(rs.getString(pos));
return identityProvider;
}
}
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 int isOriginLoopAllowed(String zoneId, int checkDone) {
if (checkDone > -1) {
return checkDone;
}
IdentityZoneConfiguration idzConfig;
if (identityZoneManager.getCurrentIdentityZoneId().equals(zoneId)) {
idzConfig = identityZoneManager.getCurrentIdentityZone().getConfig();
} else {
idzConfig = identityZoneProvisioning.retrieve(zoneId).getConfig();
}
return (idzConfig == null || Optional.of(idzConfig.getUserConfig()).map(UserConfig::isAllowAllOrigins).orElse(true)) ? 1 : 0;
}

@Override
public IdentityProvider create(IdentityProvider identityProvider, String zoneId) {
return providerProvisioning.create(identityProvider, zoneId);
Expand Down Expand Up @@ -155,11 +180,29 @@ public List<IdentityProvider> retrieveActive(String zoneId) {
}

public IdentityProvider retrieveByIssuer(String issuer, String zoneId) throws IncorrectResultSizeDataAccessException {
IdentityProvider issuedProvider = null;
int originLoopCheckDone = -1;
try {
issuedProvider = retrieveByExternId(issuer, OIDC10, zoneId);
if (issuedProvider != null && issuedProvider.isActive()
&& issuedProvider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition<?> oAuthIdentityProviderDefinition
&& oAuthIdentityProviderDefinition.getIssuer().equals(issuer)) {
return issuedProvider;
}
} catch (EmptyResultDataAccessException e) {
originLoopCheckDone = isOriginLoopAllowed(zoneId, originLoopCheckDone);
if (originLoopCheckDone == 0) {
throw new IncorrectResultSizeDataAccessException(String.format("No provider with unique issuer[%s] found", issuer), 1, 0, e);
}
}
if (isOriginLoopAllowed(zoneId, originLoopCheckDone) == 0 && issuedProvider == null) {
throw new IncorrectResultSizeDataAccessException(String.format("Active provider with unique issuer[%s] not found", issuer), 1);
}
List<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 @@ -199,6 +242,15 @@ public IdentityProvider retrieveByOrigin(String origin, String zoneId) {
return p;
}

@Override
public IdentityProvider retrieveByExternId(String externId, String type, String zoneId) {
IdentityProvider p = providerProvisioning.retrieveByExternId(externId, type, zoneId);
if (p != null && OIDC10.equals(type)) {
p.setConfig(overlay((OIDCIdentityProviderDefinition) p.getConfig()));
}
return p;
}

@Override
public IdentityProvider retrieveByOriginIgnoreActiveFlag(String origin, String zoneId) {
IdentityProvider p = providerProvisioning.retrieveByOriginIgnoreActiveFlag(origin, zoneId);
Expand Down
Loading
Loading