Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@Osara-B
Osara-B committed Dec 19, 2024
1 parent 60ee2d3 commit 59aab15
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 53 deletions.
12 changes: 6 additions & 6 deletions ...ntity.core/src/main/java/org/wso2/carbon/identity/core/SAMLSSOServiceProviderManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
import org.apache.commons.logging.LogFactory;
import org.wso2.carbon.identity.base.IdentityException;
import org.wso2.carbon.identity.core.dao.SAMLSSOServiceProviderDAO;
import org.wso2.carbon.identity.core.dao.SAMLSSOServiceProviderDAOImpl;
import org.wso2.carbon.identity.core.dao.SAMLSSOServiceProviderRegistryDAOImpl;
import org.wso2.carbon.identity.core.dao.JDBCSAMLSSOServiceProviderDAOImpl;
import org.wso2.carbon.identity.core.dao.RegistrySAMLSSOServiceProviderDAOImpl;
import org.wso2.carbon.identity.core.model.SAMLSSOServiceProviderDO;
import org.wso2.carbon.identity.core.util.IdentityTenantUtil;
import org.wso2.carbon.identity.core.util.IdentityUtil;
Expand Down Expand Up @@ -52,7 +52,7 @@ public class SAMLSSOServiceProviderManager {
*/
private SAMLSSOServiceProviderDAO buildSAMLSSOProvider(int tenantId) throws IdentityException {

SAMLSSOServiceProviderDAO samlSSOServiceProviderDAO = new SAMLSSOServiceProviderDAOImpl(tenantId);
SAMLSSOServiceProviderDAO samlSSOServiceProviderDAO = new JDBCSAMLSSOServiceProviderDAOImpl(tenantId);
if (StringUtils.isNotBlank(SAML_STORAGE_TYPE)) {
switch (SAML_STORAGE_TYPE) {
case HYBRID:
Expand All @@ -61,7 +61,7 @@ private SAMLSSOServiceProviderDAO buildSAMLSSOProvider(int tenantId) throws Iden
case REGISTRY:
try {
Registry registry = IdentityTenantUtil.getRegistryService().getConfigSystemRegistry(tenantId);
samlSSOServiceProviderDAO = new SAMLSSOServiceProviderRegistryDAOImpl(registry);
samlSSOServiceProviderDAO = new RegistrySAMLSSOServiceProviderDAOImpl(registry);
} catch (RegistryException e) {
LOG.error("Error while retrieving registry", e);
throw new IdentityException("Error while retrieving registry", e);
Expand Down Expand Up @@ -119,8 +119,8 @@ public boolean updateServiceProvider(SAMLSSOServiceProviderDO serviceProviderDO,
public SAMLSSOServiceProviderDO[] getServiceProviders(int tenantId)
throws IdentityException {

SAMLSSOServiceProviderDAO serviceProviderDOA = buildSAMLSSOProvider(tenantId);
return serviceProviderDOA.getServiceProviders();
SAMLSSOServiceProviderDAO serviceProviderDAO = buildSAMLSSOProvider(tenantId);
return serviceProviderDAO.getServiceProviders();
}

/**
Expand Down
20 changes: 10 additions & 10 deletions ...re/dao/SAMLSSOServiceProviderDAOImpl.java → ...ao/JDBCSAMLSSOServiceProviderDAOImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import org.wso2.carbon.identity.core.DatabaseCertificateRetriever;
import org.wso2.carbon.identity.core.IdentityRegistryResources;
import org.wso2.carbon.identity.core.KeyStoreCertificateRetriever;
import org.wso2.carbon.identity.core.model.SPProperty;
import org.wso2.carbon.identity.core.model.ServiceProviderProperty;
import org.wso2.carbon.identity.core.model.SAMLSSOServiceProviderDO;
import org.wso2.carbon.identity.core.util.IdentityTenantUtil;
import org.wso2.carbon.identity.core.util.JdbcUtils;
Expand Down Expand Up @@ -77,9 +77,9 @@
import static org.wso2.carbon.identity.core.dao.SAMLSSOServiceProviderConstants.SAML2TableColumns.PROPERTY_VALUE;
import static org.wso2.carbon.identity.core.dao.SAMLSSOServiceProviderConstants.SAML2TableColumns.SP_ID;

public class SAMLSSOServiceProviderDAOImpl implements SAMLSSOServiceProviderDAO {
public class JDBCSAMLSSOServiceProviderDAOImpl implements SAMLSSOServiceProviderDAO {

private static final Log log = LogFactory.getLog(SAMLSSOServiceProviderDAOImpl.class);
private static final Log log = LogFactory.getLog(JDBCSAMLSSOServiceProviderDAOImpl.class);
private final int tenantId;
private static final String CERTIFICATE_PROPERTY_NAME = "CERTIFICATE";
private static final String QUERY_TO_GET_APPLICATION_CERTIFICATE_ID = "SELECT " +
Expand All @@ -89,7 +89,7 @@ public class SAMLSSOServiceProviderDAOImpl implements SAMLSSOServiceProviderDAO
"META.`VALUE` FROM SP_INBOUND_AUTH INBOUND, SP_APP SP, SP_METADATA META WHERE SP.ID = INBOUND.APP_ID AND " +
"SP.ID = META.SP_ID AND META.NAME = ? AND INBOUND.INBOUND_AUTH_KEY = ? AND META.TENANT_ID = ?";

public SAMLSSOServiceProviderDAOImpl(int tenantId) {
public JDBCSAMLSSOServiceProviderDAOImpl(int tenantId) {

this.tenantId = tenantId;
}
Expand Down Expand Up @@ -366,9 +366,9 @@ private void addProperties(int serviceProviderId, SAMLSSOServiceProviderDO servi
throws DataAccessException {

NamedJdbcTemplate namedJdbcTemplate = JdbcUtils.getNewNamedJdbcTemplate();
List<SPProperty> properties =
List<ServiceProviderProperty> properties =
namedJdbcTemplate.executeQuery(SAMLSSOServiceProviderConstants.SQLQueries.GET_SAML_SSO_ATTR_BY_ID,
(resultSet, rowNumber) -> new SPProperty(resultSet.getString(PROPERTY_NAME),
(resultSet, rowNumber) -> new ServiceProviderProperty(resultSet.getString(PROPERTY_NAME),
resultSet.getString(PROPERTY_VALUE)),
namedPreparedStatement -> namedPreparedStatement.setInt(SP_ID, serviceProviderId));
serviceProviderDO.addMultiValuedProperties(properties);
Expand Down Expand Up @@ -434,14 +434,14 @@ private void processAddServiceProvider(SAMLSSOServiceProviderDO serviceProviderD

private void processAddSPProperties(SAMLSSOServiceProviderDO serviceProviderDO) throws DataAccessException {

List<SPProperty> properties = serviceProviderDO.getMultiValuedProperties();
List<ServiceProviderProperty> properties = serviceProviderDO.getMultiValuedProperties();
int serviceProviderId = processGetServiceProviderId(serviceProviderDO.getIssuer());

NamedJdbcTemplate namedJdbcTemplate = JdbcUtils.getNewNamedJdbcTemplate();

namedJdbcTemplate.executeBatchInsert(SAMLSSOServiceProviderConstants.SQLQueries.ADD_SAML_SSO_ATTR,
(namedPreparedStatement -> {
for (SPProperty property : properties) {
for (ServiceProviderProperty property : properties) {
namedPreparedStatement.setInt(SP_ID, serviceProviderId);
namedPreparedStatement.setString(PROPERTY_NAME, property.getKey());
namedPreparedStatement.setString(PROPERTY_VALUE, property.getValue());
Expand All @@ -464,15 +464,15 @@ private void processUpdateServiceProvider(SAMLSSOServiceProviderDO serviceProvid
private void processUpdateSPProperties(SAMLSSOServiceProviderDO serviceProviderDO, int serviceProviderId)
throws DataAccessException {

List<SPProperty> properties = serviceProviderDO.getMultiValuedProperties();
List<ServiceProviderProperty> properties = serviceProviderDO.getMultiValuedProperties();
NamedJdbcTemplate namedJdbcTemplate = JdbcUtils.getNewNamedJdbcTemplate();

namedJdbcTemplate.executeUpdate(SAMLSSOServiceProviderConstants.SQLQueries.DELETE_SAML_SSO_ATTR_BY_ID,
namedPreparedStatement -> namedPreparedStatement.setInt(SP_ID, serviceProviderId));

namedJdbcTemplate.executeBatchInsert(SAMLSSOServiceProviderConstants.SQLQueries.ADD_SAML_SSO_ATTR,
(namedPreparedStatement -> {
for (SPProperty property : properties) {
for (ServiceProviderProperty property : properties) {
namedPreparedStatement.setInt(SP_ID, serviceProviderId);
namedPreparedStatement.setString(PROPERTY_NAME, property.getKey());
namedPreparedStatement.setString(PROPERTY_VALUE, property.getValue());
Expand Down
4 changes: 2 additions & 2 deletions ...AMLSSOServiceProviderRegistryDAOImpl.java → ...egistrySAMLSSOServiceProviderDAOImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@

import static org.wso2.carbon.identity.core.util.JdbcUtils.isH2DB;

public class SAMLSSOServiceProviderRegistryDAOImpl extends AbstractDAO<SAMLSSOServiceProviderDO>
public class RegistrySAMLSSOServiceProviderDAOImpl extends AbstractDAO<SAMLSSOServiceProviderDO>
implements SAMLSSOServiceProviderDAO {
private static final String CERTIFICATE_PROPERTY_NAME = "CERTIFICATE";
private static final String QUERY_TO_GET_APPLICATION_CERTIFICATE_ID = "SELECT " +
Expand All @@ -65,7 +65,7 @@ public class SAMLSSOServiceProviderRegistryDAOImpl extends AbstractDAO<SAMLSSOSe

private static Log log = LogFactory.getLog(SAMLSSOServiceProviderDAO.class);

public SAMLSSOServiceProviderRegistryDAOImpl(Registry registry) {
public RegistrySAMLSSOServiceProviderDAOImpl(Registry registry) {
this.registry = registry;
}

Expand Down
21 changes: 19 additions & 2 deletions ...core/src/main/java/org/wso2/carbon/identity/core/dao/SAMLSSOServiceProviderConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,24 @@ public class SAMLSSOServiceProviderConstants {

public static final String SAML_STORAGE_CONFIG = "DataStorageType.SAML";

public enum MultiValuedPropertyKey {
ASSERTION_CONSUMER_URLS("ASSERTION_CONSUMER_URLS"),
AUDIENCES("AUDIENCES"),
RECIPIENTS("RECIPIENTS"),
SLO_RETURN_TO_URLS("SLO_RETURN_TO_URLS");

private final String value;

MultiValuedPropertyKey(String value) {
this.value = value;
}

@Override
public String toString() {
return value;
}
}

private SAMLSSOServiceProviderConstants() {

}
Expand Down Expand Up @@ -111,8 +129,7 @@ private SQLQueries() {
"ECP_ENABLED = :ECP_ENABLED;, ARTIFACT_BINDING_ENABLED = :ARTIFACT_BINDING_ENABLED;, " +
"ARTIFACT_RESOLVE_REQ_SIG_VALIDATION = :ARTIFACT_RESOLVE_REQ_SIG_VALIDATION;, " +
"IDP_ENTITY_ID_ALIAS = :IDP_ENTITY_ID_ALIAS;, ISSUER_QUALIFIER = :ISSUER_QUALIFIER;, " +
"SUPPORTED_ASSERTION_QUERY_REQUEST_TYPES = :SUPPORTED_ASSERTION_QUERY_REQUEST_TYPES;, " +
"TENANT_ID = :TENANT_ID; " +
"SUPPORTED_ASSERTION_QUERY_REQUEST_TYPES = :SUPPORTED_ASSERTION_QUERY_REQUEST_TYPES; " +
"WHERE ID = :ID; AND TENANT_ID = :TENANT_ID;";

public static final String DELETE_SAML2_SSO_CONFIG_BY_ISSUER =
Expand Down
41 changes: 19 additions & 22 deletions ...tity.core/src/main/java/org/wso2/carbon/identity/core/model/SAMLSSOServiceProviderDO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.wso2.carbon.identity.base.IdentityConstants;
import org.wso2.carbon.identity.core.util.IdentityCoreConstants;
import org.wso2.carbon.identity.core.util.IdentityUtil;
import org.wso2.carbon.identity.core.dao.SAMLSSOServiceProviderConstants.MultiValuedPropertyKey;

import java.io.Serializable;
import java.security.cert.X509Certificate;
Expand Down Expand Up @@ -81,10 +82,6 @@ public class SAMLSSOServiceProviderDO implements Serializable {
private String frontChannelLogoutBinding;

private static final String BACKCHANNEL_LOGOUT_BINDING = "BackChannel";
private static final String ASSERTION_CONSUMER_URLS = "ASSERTION_CONSUMER_URLS";
private static final String AUDIENCES = "AUDIENCES";
private static final String RECIPIENTS = "RECIPIENTS";
private static final String SLO_RETURN_TO_URLS = "SLO_RETURN_TO_URLS";

public void setDoValidateSignatureInArtifactResolve(boolean doValidateSignatureInArtifactResolve) {

Expand Down Expand Up @@ -672,24 +669,24 @@ public String getSingleLogoutMethod() {
/**
* Get configs of the SAML SSO IdP.
*
* @return List of SPProperty.
* @return List of ServiceProviderProperty.
*/
public List<SPProperty> getMultiValuedProperties() {
public List<ServiceProviderProperty> getMultiValuedProperties() {

List<SPProperty> multiValuedProperties = new ArrayList<>();
List<ServiceProviderProperty> multiValuedProperties = new ArrayList<>();

// Multi-valued attributes.
getAssertionConsumerUrlList().forEach(assertionConUrl ->
putIfNotNull(multiValuedProperties, ASSERTION_CONSUMER_URLS,
putIfNotNull(multiValuedProperties, MultiValuedPropertyKey.ASSERTION_CONSUMER_URLS.toString(),
assertionConUrl));
getRequestedRecipientsList().forEach(requestedRecipient ->
putIfNotNull(multiValuedProperties, RECIPIENTS,
putIfNotNull(multiValuedProperties, MultiValuedPropertyKey.RECIPIENTS.toString(),
requestedRecipient));
getRequestedAudiencesList().forEach(requestedAudience ->
putIfNotNull(multiValuedProperties, AUDIENCES,
putIfNotNull(multiValuedProperties, MultiValuedPropertyKey.AUDIENCES.toString(),
requestedAudience));
getIdpInitSLOReturnToURLList().forEach(idpInitSLOReturnToURL ->
putIfNotNull(multiValuedProperties, SLO_RETURN_TO_URLS,
putIfNotNull(multiValuedProperties, MultiValuedPropertyKey.SLO_RETURN_TO_URLS.toString(),
idpInitSLOReturnToURL));

return multiValuedProperties;
Expand All @@ -698,9 +695,9 @@ public List<SPProperty> getMultiValuedProperties() {
/**
* Add a list of multivalued properties.
*
* @param multiValuedProperties List of SPProperty.
* @param multiValuedProperties List of ServiceProviderProperty.
*/
public void addMultiValuedProperties(List<SPProperty> multiValuedProperties) {
public void addMultiValuedProperties(List<ServiceProviderProperty> multiValuedProperties) {

if (multiValuedProperties == null) {
return;
Expand All @@ -712,38 +709,38 @@ public void addMultiValuedProperties(List<SPProperty> multiValuedProperties) {
/**
* Add a multivalued property.
*
* @param multiValuedProperty SPProperty.
* @param multiValuedProperty ServiceProviderProperty.
*/
private void addMultiValuedProperty(SPProperty multiValuedProperty) {
private void addMultiValuedProperty(ServiceProviderProperty multiValuedProperty) {

if (multiValuedProperty == null) {
return;
}
String key = multiValuedProperty.getKey();
String value = multiValuedProperty.getValue();

if (ASSERTION_CONSUMER_URLS.equals(key)) {
if (MultiValuedPropertyKey.ASSERTION_CONSUMER_URLS.toString().equals(key)) {
List<String> attributeList = getAssertionConsumerUrlList();
if (attributeList.isEmpty()) {
attributeList = new ArrayList<>();
}
attributeList.add(value);
setAssertionConsumerUrls(attributeList);
} else if (RECIPIENTS.equals(key)) {
} else if (MultiValuedPropertyKey.RECIPIENTS.toString().equals(key)) {
List<String> attributeList = getRequestedRecipientsList();
if (attributeList.isEmpty()) {
attributeList = new ArrayList<>();
}
attributeList.add(value);
setRequestedRecipients(attributeList);
} else if (AUDIENCES.equals(key)) {
} else if (MultiValuedPropertyKey.AUDIENCES.toString().equals(key)) {
List<String> attributeList = getRequestedAudiencesList();
if (attributeList.isEmpty()) {
attributeList = new ArrayList<>();
}
attributeList.add(value);
setRequestedAudiences(attributeList);
} else if (SLO_RETURN_TO_URLS.equals(key)) {
} else if (MultiValuedPropertyKey.SLO_RETURN_TO_URLS.toString().equals(key)) {
List<String> attributeList = getIdpInitSLOReturnToURLList();
if (attributeList.isEmpty()) {
attributeList = new ArrayList<>();
Expand All @@ -756,14 +753,14 @@ private void addMultiValuedProperty(SPProperty multiValuedProperty) {
/**
* Put a key value pair to a list if the value is not null.
*
* @param list List of SPProperty.
* @param list List of ServiceProviderProperty.
* @param key Key.
* @param value Value.
*/
private void putIfNotNull(List<SPProperty> list, String key, String value) {
private void putIfNotNull(List<ServiceProviderProperty> list, String key, String value) {

if (StringUtils.isNotBlank(value)) {
list.add(new SPProperty(key, value));
list.add(new ServiceProviderProperty(key, value));
}
}

Expand Down
4 changes: 2 additions & 2 deletions ...arbon/identity/core/model/SPProperty.java → ...y/core/model/ServiceProviderProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
/**
* This class represents a tuple of key and value.
*/
public class SPProperty {
public class ServiceProviderProperty {

private String key;
private String value;

public SPProperty(String key, String value) {
public ServiceProviderProperty(String key, String value) {
this.key = key;
this.value = value;
}
Expand Down
14 changes: 7 additions & 7 deletions ...e/src/main/java/org/wso2/carbon/identity/core/persistence/IdentityPersistenceManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import org.wso2.carbon.identity.core.dao.OpenIDUserDAO;
import org.wso2.carbon.identity.core.dao.ParameterDAO;
import org.wso2.carbon.identity.core.dao.SAMLSSOServiceProviderDAO;
import org.wso2.carbon.identity.core.dao.SAMLSSOServiceProviderRegistryDAOImpl;
import org.wso2.carbon.identity.core.dao.RegistrySAMLSSOServiceProviderDAOImpl;
import org.wso2.carbon.identity.core.dao.XMPPSettingsDAO;
import org.wso2.carbon.identity.core.model.OpenIDAdminDO;
import org.wso2.carbon.identity.core.model.OpenIDUserDO;
Expand Down Expand Up @@ -236,7 +236,7 @@ public void removeOpenIDSignUp(Registry registry, UserRealm realm, String openID
*/
public boolean addServiceProvider(Registry registry, SAMLSSOServiceProviderDO serviceProviderDO)
throws IdentityException {
SAMLSSOServiceProviderDAO serviceProviderDAO = new SAMLSSOServiceProviderRegistryDAOImpl(registry);
SAMLSSOServiceProviderDAO serviceProviderDAO = new RegistrySAMLSSOServiceProviderDAOImpl(registry);
return serviceProviderDAO.addServiceProvider(serviceProviderDO);
}
/**
Expand All @@ -247,7 +247,7 @@ public boolean addServiceProvider(Registry registry, SAMLSSOServiceProviderDO se
* @throws IdentityException
*/
public SAMLSSOServiceProviderDO uploadServiceProvider(Registry registry, SAMLSSOServiceProviderDO samlssoServiceProviderDO) throws IdentityException {
SAMLSSOServiceProviderDAO serviceProviderDAO = new SAMLSSOServiceProviderRegistryDAOImpl(registry);
SAMLSSOServiceProviderDAO serviceProviderDAO = new RegistrySAMLSSOServiceProviderDAOImpl(registry);
return serviceProviderDAO.uploadServiceProvider(samlssoServiceProviderDO);
}

Expand All @@ -259,23 +259,23 @@ public SAMLSSOServiceProviderDO uploadServiceProvider(Registry registry, SAMLSSO
*/
public SAMLSSOServiceProviderDO[] getServiceProviders(Registry registry)
throws IdentityException {
SAMLSSOServiceProviderDAO serviceProviderDOA = new SAMLSSOServiceProviderRegistryDAOImpl(registry);
SAMLSSOServiceProviderDAO serviceProviderDOA = new RegistrySAMLSSOServiceProviderDAOImpl(registry);
return serviceProviderDOA.getServiceProviders();
}

public boolean removeServiceProvider(Registry registry, String issuer) throws IdentityException {
SAMLSSOServiceProviderDAO serviceProviderDAO = new SAMLSSOServiceProviderRegistryDAOImpl(registry);
SAMLSSOServiceProviderDAO serviceProviderDAO = new RegistrySAMLSSOServiceProviderDAOImpl(registry);
return serviceProviderDAO.removeServiceProvider(issuer);
}

public SAMLSSOServiceProviderDO getServiceProvider(Registry registry, String issuer)
throws IdentityException {
SAMLSSOServiceProviderDAO serviceProviderDAO = new SAMLSSOServiceProviderRegistryDAOImpl(registry);
SAMLSSOServiceProviderDAO serviceProviderDAO = new RegistrySAMLSSOServiceProviderDAOImpl(registry);
return serviceProviderDAO.getServiceProvider(issuer);
}

public boolean isServiceProviderExists(Registry registry, String issuer) throws IdentityException {
SAMLSSOServiceProviderDAO serviceProviderDAO = new SAMLSSOServiceProviderRegistryDAOImpl(registry);
SAMLSSOServiceProviderDAO serviceProviderDAO = new RegistrySAMLSSOServiceProviderDAOImpl(registry);
return serviceProviderDAO.isServiceProviderExists(issuer);
}

Expand Down
Loading

0 comments on commit 59aab15

Please sign in to comment.