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

Improve role management with shared role check #6010

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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 @@ -73,6 +73,13 @@ private RoleConstants() {
public static final String CONSOLE_SCOPE_PREFIX = "console:";
public static final String CONSOLE_ORG_SCOPE_PREFIX = "console:org:";

// Conflict Audit Data fields
public static final String PARENT_ORG_ID = "parentOrganizationId";
public static final String SHARED_ORG_ID = "sharedOrganizationId";
public static final String EXISTING_ROLE_NAME = "existingRoleName";
public static final String NEW_ROLE_NAME = "newRoleName";
public static final String FAILURE_REASON = "failureReason";

/**
* Grouping of constants related to database table names.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,4 +473,17 @@ default List<RoleDTO> getSharedHybridRoles(String roleId, int tenantId) throws I

return null;
}

/**
* Check whether the given role is a shared role in the given tenant.
*
* @param roleId The role ID of the tenant.
* @param tenantDomain The tenant domain.
* @return True if the role is a shared role.
* @throws IdentityRoleManagementException If an error occurs while checking the shared role.
*/
default boolean isSharedRole(String roleId, String tenantDomain) throws IdentityRoleManagementException {

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.wso2.carbon.context.CarbonContext;
import org.wso2.carbon.context.PrivilegedCarbonContext;
import org.wso2.carbon.database.utils.jdbc.NamedPreparedStatement;
import org.wso2.carbon.identity.application.common.model.IdPGroup;
import org.wso2.carbon.identity.application.common.model.IdentityProvider;
import org.wso2.carbon.identity.base.IdentityConstants;
import org.wso2.carbon.identity.central.log.mgt.utils.LogConstants;
import org.wso2.carbon.identity.central.log.mgt.utils.LoggerUtils;
import org.wso2.carbon.identity.core.model.ExpressionNode;
import org.wso2.carbon.identity.core.util.IdentityConfigParser;
import org.wso2.carbon.identity.core.util.IdentityCoreConstants;
Expand Down Expand Up @@ -66,6 +69,7 @@
import org.wso2.carbon.user.core.common.UserRolesCache;
import org.wso2.carbon.user.core.service.RealmService;
import org.wso2.carbon.user.core.util.UserCoreUtil;
import org.wso2.carbon.utils.AuditLog;

import java.sql.Connection;
import java.sql.ResultSet;
Expand Down Expand Up @@ -520,7 +524,7 @@ public RoleBasicInfo getRoleBasicInfoById(String roleId, String tenantDomain)
public List<Permission> getPermissionListOfRole(String roleId, String tenantDomain)
throws IdentityRoleManagementException {

if (isOrganization(tenantDomain)) {
if (isOrganization(tenantDomain) && isSharedRole(roleId, tenantDomain)) {
return getPermissionsOfSharedRole(roleId, tenantDomain);
} else {
return getPermissions(roleId, tenantDomain);
Expand Down Expand Up @@ -803,8 +807,15 @@ public void updateRoleName(String roleId, String newRoleName, String tenantDomai
statement.setInt(RoleConstants.RoleTableColumns.UM_TENANT_ID, tenantId);
statement.executeUpdate();
}
// Update shared hybrid role names
updateSharedHybridRolesName(sharedRoles, newRoleName, connection);
// Update shared hybrid role names and return conflicting roles if there are any
List<RoleDTO> conflictingRoles = updateSharedHybridRolesNameAndReturnConflicts(sharedRoles,
roleAudience.getAudience(), newRoleName, connection);

if (!conflictingRoles.isEmpty()) {
for (RoleDTO conflictRole : conflictingRoles) {
sharedRoles.remove(conflictRole);
}
}
// Update the role name in IDN_SCIM_GROUP table.
updateSCIMRoleName(roleName, newRoleName, audienceRefId, sharedRoles, tenantDomain);

Expand All @@ -824,23 +835,72 @@ public void updateRoleName(String roleId, String newRoleName, String tenantDomai
clearUserRolesCacheByTenant(tenantId);
}

private void updateSharedHybridRolesName(List<RoleDTO> sharedRoles, String newRoleName, Connection connection)
private List<RoleDTO> updateSharedHybridRolesNameAndReturnConflicts(List<RoleDTO> sharedRoles, String roleAudience,
String newRoleName, Connection connection)
throws IdentityRoleManagementException {

List<RoleDTO> conflictingRoles = new ArrayList<>();
for (RoleDTO roleDTO : sharedRoles) {
try (NamedPreparedStatement statement = new NamedPreparedStatement(connection, UPDATE_ROLE_NAME_SQL,
RoleConstants.RoleTableColumns.UM_ID)) {
statement.setString(RoleConstants.RoleTableColumns.NEW_UM_ROLE_NAME, newRoleName);
statement.setString(RoleConstants.RoleTableColumns.UM_UUID, roleDTO.getId());
statement.setInt(RoleConstants.RoleTableColumns.UM_TENANT_ID, roleDTO.getTenantId());
statement.executeUpdate();
} catch (SQLException e) {
IdentityDatabaseUtil.rollbackUserDBTransaction(connection);
String message = "Error while updating the role name of shared role: %s";
throw new IdentityRoleManagementServerException(RoleConstants.Error.UNEXPECTED_SERVER_ERROR.getCode(),
String.format(message, roleDTO.getId()), e);
String roleSharedTenantDomain = IdentityTenantUtil.getTenantDomain(roleDTO.getTenantId());
String roleSharedOrganization = getOrganizationId(roleSharedTenantDomain);
boolean isRoleExist = false;
// If the role audience is ORGANIZATION, then we need to check whether a role is already exists with the
// new role name in the targeted organization. If the role exists, then we need to skip the role name
// update and log the conflict.
if (ORGANIZATION.equals(roleAudience)) {
isRoleExist = isExistingRoleName(newRoleName, roleAudience, roleSharedOrganization,
roleSharedTenantDomain);
}
if (!isRoleExist) {
try (NamedPreparedStatement statement = new NamedPreparedStatement(connection, UPDATE_ROLE_NAME_SQL,
RoleConstants.RoleTableColumns.UM_ID)) {
statement.setString(RoleConstants.RoleTableColumns.NEW_UM_ROLE_NAME, newRoleName);
statement.setString(RoleConstants.RoleTableColumns.UM_UUID, roleDTO.getId());
statement.setInt(RoleConstants.RoleTableColumns.UM_TENANT_ID, roleDTO.getTenantId());
statement.executeUpdate();
} catch (SQLException e) {
IdentityDatabaseUtil.rollbackUserDBTransaction(connection);
String message = "Error while updating the role name of shared role: %s";
throw new IdentityRoleManagementServerException(RoleConstants.Error.UNEXPECTED_SERVER_ERROR.
getCode(), String.format(message, roleDTO.getId()), e);
}
} else {
conflictingRoles.add(roleDTO);
if (LoggerUtils.isEnableV2AuditLogs()) {
String username = PrivilegedCarbonContext.getThreadLocalCarbonContext().getUsername();
String tenantDomain = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain();
AuditLog.AuditLogBuilder auditLogBuilder = new AuditLog.AuditLogBuilder(
IdentityUtil.getInitiatorId(username, tenantDomain), LoggerUtils.Target.User.name(),
roleDTO.getName(), LoggerUtils.Target.Role.name(),
LogConstants.UserManagement.UPDATE_ROLE_NAME_ACTION)
.data(buildAuditData(roleSharedOrganization, roleDTO.getName(), newRoleName,
"Role conflict"));
LoggerUtils.triggerAuditLogEvent(auditLogBuilder, true);
}
ShanChathusanda93 marked this conversation as resolved.
Show resolved Hide resolved
LOG.warn(String.format("Organization %s has a non shared role with name %s, ", roleSharedOrganization,
newRoleName));
}
}
return conflictingRoles;
}

private Map<String, String> buildAuditData(String sharedOrganizationId, String existingRoleName,
String newRoleName, String failureReason) throws IdentityRoleManagementException {

Map<String, String> auditData = new HashMap<>();
String parentOrganization;
try {
parentOrganization = getOrganizationId(PrivilegedCarbonContext.getThreadLocalCarbonContext()
.getTenantDomain());
} catch (IdentityRoleManagementServerException e) {
throw new IdentityRoleManagementException("Error while getting the parent organization name.", e);
}
auditData.put(RoleConstants.PARENT_ORG_ID, parentOrganization);
auditData.put(RoleConstants.SHARED_ORG_ID, sharedOrganizationId);
auditData.put(RoleConstants.EXISTING_ROLE_NAME, existingRoleName);
auditData.put(RoleConstants.NEW_ROLE_NAME, newRoleName);
auditData.put(RoleConstants.FAILURE_REASON, failureReason);
return auditData;
}

@Override
Expand Down Expand Up @@ -1534,7 +1594,7 @@ private void addRoleInfo(String roleId, String roleName, List<Permission> permis
addRoleID(roleId, roleName, audienceRefId, tenantDomain, connection);
addPermissions(roleId, permissions, tenantDomain, connection);

if (APPLICATION.equals(audience) && !isOrganization(tenantDomain)) {
if (APPLICATION.equals(audience) && !isSharedRole(roleId, tenantDomain)) {
addAppRoleAssociation(roleId, audienceId, connection);
}
IdentityDatabaseUtil.commitTransaction(connection);
Expand All @@ -1550,16 +1610,8 @@ private void addRoleInfo(String roleId, String roleName, List<Permission> permis
}
}

/**
* Check role is a shared role.
*
* @param roleId Role ID.
* @param tenantDomain Tenant Domain.
* @return is Shared role.
* @throws IdentityRoleManagementException IdentityRoleManagementException.
*/
private boolean isSharedRole(String roleId, String tenantDomain)
throws IdentityRoleManagementException {
@Override
public boolean isSharedRole(String roleId, String tenantDomain) throws IdentityRoleManagementException {

int tenantId = IdentityTenantUtil.getTenantId(tenantDomain);
boolean isShared = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ public class SQLQueries {
+ "UM_TENANT_ID=:UM_TENANT_ID; AND UM_UUID=:UM_UUID;";

public static final String GET_ROLE_TENANT_DOMAIN_BY_ID = "SELECT UM_TENANT_ID FROM UM_HYBRID_ROLE WHERE "
+ " UM_UUID=:UM_UUID;";
+ "UM_UUID=:UM_UUID;";

public static final String IS_ROLE_ID_EXIST_SQL = "SELECT COUNT(ID) FROM IDN_SCIM_GROUP WHERE "
+ "TENANT_ID=:TENANT_ID; AND ATTR_NAME=:ATTR_NAME; AND ATTR_VALUE=:ATTR_VALUE;";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,17 @@ public static int resolveAudienceRefId(String audience, String audienceId) throw

return roleDAO.getRoleAudienceRefId(audience, audienceId);
}

/**
* Checks whether the given role is a shared role in the given tenant domain.
*
* @param roleId The role ID.
* @param tenantDomain The tenant domain.
* @return Whether the role is a shared role or not.
* @throws IdentityRoleManagementException If an error occurs while checking the shared role.
*/
public static boolean isSharedRole(String roleId, String tenantDomain) throws IdentityRoleManagementException {

return roleDAO.isSharedRole(roleId, tenantDomain);
}
}
Loading
Loading