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.

Sign up for GitHub

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

Jump to bottom

Fix issue with disabled IDP getting picked for fidp parameter #4991

Merged
merged 2 commits into from
Oct 17, 2023
Merged

Fix issue with disabled IDP getting picked for fidp parameter #4991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34ef5cb
Get only an enabled IDP when queried using realm id
Lakshan-Banneheke Oct 14, 2023
72fa451
Add function for getting enabled file based IDPs
Lakshan-Banneheke Oct 17, 2023
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
12 changes: 8 additions & 4 deletions ...rg.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/IdentityProviderManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1673,11 +1673,15 @@ public IdentityProvider getIdPByRealmId(String realmId, String tenantDomain)
public IdentityProvider getEnabledIdPByRealmId(String realmId, String tenantDomain)
throws IdentityProviderManagementException {

IdentityProvider idp = getIdPByRealmId(realmId, tenantDomain);
if (idp != null && idp.isEnable()) {
return idp;
int tenantId = IdentityTenantUtil.getTenantId(tenantDomain);
if (StringUtils.isEmpty(realmId)) {
throw new IdentityProviderManagementException("Invalid argument: Identity Provider Home Realm Identifier value is empty.");
}
return null;
IdentityProvider identityProvider = dao.getEnabledIdPByRealmId(realmId, tenantId, tenantDomain);
if (identityProvider == null) {
identityProvider = new FileBasedIdPMgtDAO().getEnabledIdPByRealmId(realmId);
}
return identityProvider;
}

/**
Expand Down
52 changes: 52 additions & 0 deletions ...g.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/CacheBackedIdPMgtDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,58 @@ public IdentityProvider getIdPByRealmId(String realmId, int tenantId,
return identityProvider;
}

/**
* Get the enabled IDP of the given realm id.
*
* @param realmId Realm ID of the required identity provider.
* @param tenantId Tenant ID of the required identity provider.
* @param tenantDomain Tenant domain of the required identity provider.
* @return Enabled identity provider of the given realm id.
* @throws IdentityProviderManagementException Error when getting the identity provider.
*/
public IdentityProvider getEnabledIdPByRealmId(String realmId, int tenantId,
String tenantDomain) throws IdentityProviderManagementException {

IdPHomeRealmIdCacheKey cacheKey = new IdPHomeRealmIdCacheKey(realmId);
IdPCacheEntry entry = idPCacheByHRI.getValueFromCache(cacheKey, tenantDomain);
if (entry != null) {
Lakshan-Banneheke marked this conversation as resolved.
Show resolved Hide resolved
if (log.isDebugEnabled()) {
log.debug("Cache entry found for Identity Provider with Home Realm ID " + realmId);
}
// Check whether the idp in the cache is enabled.
if (entry.getIdentityProvider().isEnable()) {
return entry.getIdentityProvider();
}
if (log.isDebugEnabled()) {
Lakshan-Banneheke marked this conversation as resolved.
Show resolved Hide resolved
log.debug("Identity Provider with Home Realm ID " + realmId + " available in the cache is disabled. " +
"Fetching entry from DB.");
}
} else {
if (log.isDebugEnabled()) {
log.debug("Cache entry not found for Identity Provider with Home Realm ID " + realmId
+ ". Fetching entry from DB.");
}
}

IdentityProvider identityProvider = idPMgtDAO.getEnabledIdPByRealmId(realmId, tenantId, tenantDomain);

if (identityProvider != null) {
if (log.isDebugEnabled()) {
log.debug("Entry fetched from DB for Identity Provider with Home Realm ID " + realmId
+ ". Updating cache.");
}
idPCacheByHRI.addToCache(cacheKey, new IdPCacheEntry(identityProvider), tenantDomain);
IdPNameCacheKey idPNameCacheKey = new IdPNameCacheKey(identityProvider.getIdentityProviderName());
idPCacheByName.addToCache(idPNameCacheKey, new IdPCacheEntry(identityProvider), tenantDomain);
} else {
if (log.isDebugEnabled()) {
log.debug("Entry for Identity Provider with Home Realm ID " + realmId
+ " not found in cache or DB.");
}
}
return identityProvider;
}

/**
* Adds a new Identity Provider and cache it.
*
Expand Down
21 changes: 21 additions & 0 deletions ...org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/FileBasedIdPMgtDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,27 @@ public IdentityProvider getIdPByRealmId(String realmId, String tenantDomain) {
return null;
}

/**
* Get the enabled IDP of the given realm id.
*
* @param realmId Realm ID of the required identity provider.
* @return Enabled identity provider of the given realm id.
*/
public IdentityProvider getEnabledIdPByRealmId(String realmId) {

Map<String, IdentityProvider> map = IdPManagementServiceComponent.getFileBasedIdPs();
for (Iterator<Entry<String, IdentityProvider>> iterator = map.entrySet().iterator(); iterator
.hasNext(); ) {
Entry<String, IdentityProvider> entry = iterator.next();
if (entry.getValue().getHomeRealmId() != null
&& entry.getValue().getHomeRealmId().equals(realmId)
&& entry.getValue().isEnable()) {
return entry.getValue();
}
}
return null;
}

/**
* Retrieves the first matching IDP for the given metadata property.
* Intended to ony be used to retrieve IDP name based on a unique metadata property.
Expand Down
32 changes: 32 additions & 0 deletions ...t/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2803,6 +2803,38 @@ public IdentityProvider getIdPByRealmId(String realmId, int tenantId, String ten

}

/**
Lakshan-Banneheke marked this conversation as resolved.
Show resolved Hide resolved
* Get the enabled IDP of the given realm id.
*
* @param realmId Realm ID of the required identity provider.
* @param tenantId Tenant ID of the required identity provider.
* @param tenantDomain Tenant domain of the required identity provider.
* @return Enabled identity provider of the given realm id.
* @throws IdentityProviderManagementException Error when getting the identity provider.
* @throws SQLException Error when executing SQL query.
*/
public IdentityProvider getEnabledIdPByRealmId(String realmId, int tenantId, String tenantDomain)
throws IdentityProviderManagementException {

try (Connection dbConnection = IdentityDatabaseUtil.getDBConnection(false)) {
String idPName = null;
String sqlStmt = IdPManagementConstants.SQLQueries.GET_ENABLED_IDP_NAME_BY_REALM_ID_SQL;
PreparedStatement prepStmt = dbConnection.prepareStatement(sqlStmt);
prepStmt.setInt(1, tenantId);
prepStmt.setInt(2, MultitenantConstants.SUPER_TENANT_ID);
prepStmt.setString(3, realmId);
prepStmt.setString(4, IdPManagementConstants.IS_TRUE_VALUE);
ResultSet rs = prepStmt.executeQuery();
if (rs.next()) {
idPName = rs.getString(IdPManagementConstants.NAME);
}
return getIdPByName(dbConnection, idPName, tenantId, tenantDomain);
} catch (SQLException e) {
throw new IdentityProviderManagementException("Error while retrieving Identity Provider by realm " +
realmId, e);
}
}

/**
* @param identityProvider
* @param tenantId
Expand Down
3 changes: 3 additions & 0 deletions ...so2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/util/IdPManagementConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ public static class SQLQueries {
public static final String GET_IDP_NAME_BY_REALM_ID_SQL = "SELECT NAME FROM IDP WHERE (TENANT_ID = ? OR " +
"(TENANT_ID = ? AND NAME LIKE '" + SHARED_IDP_PREFIX + "%')) AND HOME_REALM_ID=?";

public static final String GET_ENABLED_IDP_NAME_BY_REALM_ID_SQL = "SELECT NAME FROM IDP WHERE (TENANT_ID = ? OR " +
"(TENANT_ID = ? AND NAME LIKE '" + SHARED_IDP_PREFIX + "%')) AND HOME_REALM_ID = ? AND IS_ENABLED = ?";

public static final String GET_IDP_CLAIM_MAPPINGS_SQL = "SELECT IDP_CLAIM.CLAIM," +
" IDP_CLAIM_MAPPING.LOCAL_CLAIM, IDP_CLAIM_MAPPING.DEFAULT_VALUE, IDP_CLAIM_MAPPING.IS_REQUESTED "
+ "FROM IDP_CLAIM_MAPPING INNER JOIN IDP_CLAIM ON IDP_CLAIM_MAPPING.IDP_CLAIM_ID= IDP_CLAIM.ID " +
Expand Down
65 changes: 64 additions & 1 deletion ...o2.carbon.idp.mgt/src/test/java/org/wso2/carbon/idp/mgt/dao/CacheBackedIdPMgtDAOTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ public class CacheBackedIdPMgtDAOTest extends PowerMockTestCase {

private static final Integer SAMPLE_TENANT_ID2 = 1;

private static final Integer SAMPLE_TENANT_ID3 = 2;

private static final Integer NOT_EXISTING_TENANT_ID = 4;

private static final String TENANT_DOMAIN = "carbon.super";
Expand Down Expand Up @@ -620,7 +622,7 @@ public Object[][] getIdPByRealmIdData() {
return new Object[][]{
{"testIdP1", "1", SAMPLE_TENANT_ID1, true},
{"testIdP2", "2", SAMPLE_TENANT_ID1, true},
{"notExist", "4", SAMPLE_TENANT_ID2, false},
{"notExist", "99", SAMPLE_TENANT_ID2, false},
};
}

Expand Down Expand Up @@ -654,6 +656,47 @@ public void testGetIdPRealmId(String idpName, String realmId, int tenantId, bool
}
}

@DataProvider
public Object[][] getEnabledIdPByRealmIdData() {

return new Object[][]{
{"testIdP4", "4", SAMPLE_TENANT_ID3, true, true},
{"testIdP5", "5", SAMPLE_TENANT_ID3, true, false},
{"notExist", "99", SAMPLE_TENANT_ID3, false, false},
};
}

@Test(dataProvider = "getEnabledIdPByRealmIdData")
public void testGetEnabledIdPRealmId(String idpName, String realmId, int tenantId, boolean isExist, boolean isEnabled) throws Exception {

mockStatic(IdentityDatabaseUtil.class);
try (Connection connection = getConnection(DB_NAME)) {
when(IdentityDatabaseUtil.getDBConnection(anyBoolean())).thenReturn(connection);
when(IdentityDatabaseUtil.getDBConnection()).thenReturn(connection);
when(IdentityDatabaseUtil.getDataSource()).thenReturn(dataSourceMap.get(DB_NAME));
addTestIdps(connection);
// Retrieving IDP from DB.
IdentityProvider idpResult = cacheBackedIdPMgtDAO.getEnabledIdPByRealmId(realmId, tenantId, TENANT_DOMAIN);

IdentityProvider idpFromCache = null;
if (isExist && isEnabled) {
// Retrieving IDP from cache using realmID as cache key.
IdPCacheByHRI idPCacheByHRI = IdPCacheByHRI.getInstance();
IdPHomeRealmIdCacheKey cacheKey = new IdPHomeRealmIdCacheKey(realmId);
IdPCacheEntry entry = idPCacheByHRI.getValueFromCache(cacheKey, TENANT_DOMAIN);
idpFromCache = entry.getIdentityProvider();
}
if (isExist && isEnabled) {
assertEquals(idpFromCache.getIdentityProviderName(), idpName,
"'getEnabledIdPByRealmId' method fails");
assertEquals(idpResult, idpFromCache, "'getEnabledIdPByRealmId' method fails");
} else {
assertNull(idpResult, "'getEnabledIdPByRealmId' method fails");
assertNull(idpFromCache, "'getEnabledIdPByRealmId' method fails");
}
}
}

@DataProvider
public Object[][] addIdPData() {

Expand Down Expand Up @@ -1452,6 +1495,26 @@ private void addTestIdps() throws IdentityProviderManagementException {
idPManagementDAO.addIdP(idp3, SAMPLE_TENANT_ID2);
}

private void addTestIdps(Connection connection) throws IdentityProviderManagementException {
// Initialize Test Identity Provider 4.
IdentityProvider idp4 = new IdentityProvider();
idp4.setIdentityProviderName("testIdP4");
idp4.setHomeRealmId("4");

// Initialize Test Identity Provider 5.
IdentityProvider idp5 = new IdentityProvider();
idp5.setIdentityProviderName("testIdP5");
idp5.setHomeRealmId("5");

// IDP with Only name.
idPManagementDAO.addIdP(idp4, SAMPLE_TENANT_ID3);
// Disabled IDP.
idPManagementDAO.addIdP(idp5, SAMPLE_TENANT_ID3);
IdentityProvider tempIdP = idPManagementDAO.getIdPByName(connection, "testIdP5", SAMPLE_TENANT_ID3, TENANT_DOMAIN);
tempIdP.setEnable(false);
idPManagementDAO.updateIdP(tempIdP, idp5, SAMPLE_TENANT_ID3);
}

private int getIdPCount(Connection connection, String idpName, int tenantId) throws SQLException {

String query = IdPManagementConstants.SQLQueries.GET_IDP_BY_NAME_SQL;
Expand Down
54 changes: 53 additions & 1 deletion ...g.wso2.carbon.idp.mgt/src/test/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAOTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ public class IdPManagementDAOTest extends PowerMockTestCase {
private static final String DB_NAME = "test";
private static final Integer SAMPLE_TENANT_ID = -1234;
private static final Integer SAMPLE_TENANT_ID2 = 1;
private static final Integer SAMPLE_TENANT_ID3 = 2;

private static final String TENANT_DOMAIN = "carbon.super";
private static final String IDP_GROUP1 = "idpGroup1";
private static final String IDP_GROUP2 = "idpGroup2";
Expand Down Expand Up @@ -917,7 +919,7 @@ public Object[][] getIdPByRealmIdData() {
return new Object[][]{
{"testIdP1", "1", SAMPLE_TENANT_ID, true},
{"testIdP2", "2", SAMPLE_TENANT_ID, true},
{"notExist", "4", SAMPLE_TENANT_ID2, false},
{"notExist", "99", SAMPLE_TENANT_ID2, false},
};
}

Expand All @@ -941,6 +943,36 @@ public void testGetIdPByRealmId(String idpName, String realmId, int tenantId, bo
}
}

@DataProvider
public Object[][] getEnabledIdPByRealmIdData() {

return new Object[][]{
{"testIdP4", "4", SAMPLE_TENANT_ID3, true, true},
{"testIdP5", "5", SAMPLE_TENANT_ID3, true, false},
{"notExist", "99", SAMPLE_TENANT_ID3, false, false},
};
}

@Test(dataProvider = "getEnabledIdPByRealmIdData")
public void testGetEnabledIdPRealmId(String idpName, String realmId, int tenantId, boolean isExist, boolean isEnabled) throws Exception {

mockStatic(IdentityDatabaseUtil.class);

try (Connection connection = getConnection(DB_NAME)) {
when(IdentityDatabaseUtil.getDBConnection(anyBoolean())).thenReturn(connection);
when(IdentityDatabaseUtil.getDBConnection()).thenReturn(connection);
when(IdentityDatabaseUtil.getDataSource()).thenReturn(dataSourceMap.get(DB_NAME));
addTestIdps(connection);

IdentityProvider idpResult = idPManagementDAO.getEnabledIdPByRealmId(realmId, tenantId, TENANT_DOMAIN);
if (isExist && isEnabled) {
assertEquals(idpResult.getIdentityProviderName(), idpName, "'getEnabledIdPByRealmId' method fails");
} else {
assertNull(idpResult, "'getEnabledIdPByRealmId' method fails");
}
}
}

@Test(dataProvider = "getIdPByNameData")
public void testGetIDPNameByResourceId(String idpName, int tenantId, boolean isExist) throws Exception {

Expand Down Expand Up @@ -1818,6 +1850,26 @@ private void addTestIdps() throws IdentityProviderManagementException {
idPManagementDAO.addIdP(idp3, SAMPLE_TENANT_ID2);
}

private void addTestIdps(Connection connection) throws IdentityProviderManagementException {
Lakshan-Banneheke marked this conversation as resolved.
Show resolved Hide resolved
// Initialize Test Identity Provider 4.
IdentityProvider idp4 = new IdentityProvider();
idp4.setIdentityProviderName("testIdP4");
idp4.setHomeRealmId("4");

// Initialize Test Identity Provider 5.
IdentityProvider idp5 = new IdentityProvider();
idp5.setIdentityProviderName("testIdP5");
idp5.setHomeRealmId("5");

// IDP with Only name.
idPManagementDAO.addIdP(idp4, SAMPLE_TENANT_ID3);
// Disabled IDP.
idPManagementDAO.addIdP(idp5, SAMPLE_TENANT_ID3);
IdentityProvider tempIdP = idPManagementDAO.getIdPByName(connection, "testIdP5", SAMPLE_TENANT_ID3, TENANT_DOMAIN);
tempIdP.setEnable(false);
idPManagementDAO.updateIdP(tempIdP, idp5, SAMPLE_TENANT_ID3);
}

private void addTestTrustedTokenIssuers() throws IdentityProviderManagementException {

// Initialize Test Identity Provider 1.
Expand Down
Loading