From: Richard Millet Date: Thu, 24 May 2018 21:26:31 +0000 (-0700) Subject: DRYD-419: Adding code to filter out duplicate perms on lookups. X-Git-Url: https://git.aero2k.de/?a=commitdiff_plain;h=51107936655b018f777328040673dc4aeaa0bd34;p=tmp%2Fjakarta-migration.git DRYD-419: Adding code to filter out duplicate perms on lookups. --- diff --git a/services/authentication/service/src/main/java/org/collectionspace/authentication/AuthN.java b/services/authentication/service/src/main/java/org/collectionspace/authentication/AuthN.java index 81f8a5e6b..65f2bf8f8 100644 --- a/services/authentication/service/src/main/java/org/collectionspace/authentication/AuthN.java +++ b/services/authentication/service/src/main/java/org/collectionspace/authentication/AuthN.java @@ -95,6 +95,9 @@ public class AuthN { // Define a special account value for the tenantManager. Yes, this is a hack, but // less troublesome than the alternatives. public static final String TENANT_MANAGER_ACCT_ID = ALL_TENANTS_MANAGER_TENANT_ID; + + // Prefix for description of auto-generated permissions + public static final String GENERATED_STR = "Generated "; // trailing space is significant private AuthN() { //hardcoded initialization of a provider diff --git a/services/authorization-mgt/import/src/main/java/org/collectionspace/services/authorization/importer/AuthorizationGen.java b/services/authorization-mgt/import/src/main/java/org/collectionspace/services/authorization/importer/AuthorizationGen.java index a5cef4651..65054e810 100644 --- a/services/authorization-mgt/import/src/main/java/org/collectionspace/services/authorization/importer/AuthorizationGen.java +++ b/services/authorization-mgt/import/src/main/java/org/collectionspace/services/authorization/importer/AuthorizationGen.java @@ -26,27 +26,34 @@ package org.collectionspace.services.authorization.importer; import java.io.File; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + import java.util.ArrayList; import java.util.Hashtable; import java.util.List; import javax.xml.bind.JAXBContext; import javax.xml.bind.Marshaller; -import org.collectionspace.services.authorization.perms.Permission; + +import org.collectionspace.services.client.TenantClient; +import org.collectionspace.authentication.AuthN; import org.collectionspace.authentication.AuthN; + +import org.collectionspace.services.authorization.perms.Permission; import org.collectionspace.services.authorization.PermissionRole; import org.collectionspace.services.authorization.PermissionValue; import org.collectionspace.services.authorization.perms.PermissionsList; import org.collectionspace.services.authorization.PermissionsRolesList; -import org.collectionspace.services.client.TenantClient; + import org.collectionspace.services.authorization.Role; import org.collectionspace.services.authorization.RoleValue; import org.collectionspace.services.authorization.RolesList; import org.collectionspace.services.authorization.SubjectType; + import org.collectionspace.services.common.authorization_mgt.AuthorizationCommon; import org.collectionspace.services.common.config.ServicesConfigReaderImpl; import org.collectionspace.services.common.config.TenantBindingConfigReaderImpl; import org.collectionspace.services.common.security.SecurityUtils; import org.collectionspace.services.common.storage.jpa.JPATransactionContext; + import org.collectionspace.services.config.service.ServiceBindingType; import org.collectionspace.services.config.tenant.TenantBindingType; @@ -250,17 +257,17 @@ public class AuthorizationGen { } private Permission buildAdminPermission(String tenantId, String resourceName) { - String description = "Generated admin permission."; + String description = AuthN.GENERATED_STR + "admin permission."; return AuthorizationCommon.createPermission(tenantId, resourceName, description, AuthorizationCommon.ACTIONGROUP_CRUDL_NAME, true); } private Permission buildReaderPermission(String tenantId, String resourceName) { - String description = "Generated read-only (RL) permission."; + String description = AuthN.GENERATED_STR + "read-only (RL) permission."; return AuthorizationCommon.createPermission(tenantId, resourceName, description, AuthorizationCommon.ACTIONGROUP_RL_NAME, true); } private Permission buildReadWritePermission(String tenantId, String resourceName) { - String description = "Generated read-write (CRUL) permission."; + String description = AuthN.GENERATED_STR + "read-write (CRUL) permission."; return AuthorizationCommon.createPermission(tenantId, resourceName, description, AuthorizationCommon.ACTIONGROUP_CRUL_NAME, true); } diff --git a/services/common/src/main/java/org/collectionspace/services/common/authorization_mgt/PermissionRoleUtil.java b/services/common/src/main/java/org/collectionspace/services/common/authorization_mgt/PermissionRoleUtil.java index 9a66032eb..b66451c1d 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/authorization_mgt/PermissionRoleUtil.java +++ b/services/common/src/main/java/org/collectionspace/services/common/authorization_mgt/PermissionRoleUtil.java @@ -27,6 +27,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import javax.persistence.NoResultException; +import javax.persistence.NonUniqueResultException; import org.collectionspace.services.common.document.DocumentException; import org.collectionspace.services.common.document.DocumentNotFoundException; @@ -44,8 +45,10 @@ import org.collectionspace.services.authorization.perms.ActionType; import org.collectionspace.services.authorization.perms.EffectType; import org.collectionspace.services.authorization.perms.Permission; import org.collectionspace.services.authorization.perms.PermissionAction; + import org.collectionspace.services.authorization.storage.PermissionStorageConstants; import org.collectionspace.services.authorization.storage.RoleStorageConstants; + import org.collectionspace.services.authorization.PermissionResource; import org.collectionspace.services.authorization.PermissionRole; import org.collectionspace.services.authorization.PermissionRoleRel; @@ -170,56 +173,78 @@ public class PermissionRoleUtil { * Try to find a persisted Permission record using a PermissionValue instance. * */ - static private Permission lookupPermission(JPATransactionContext jpaTransactionContext, PermissionValue permissionValue, String tenantId) throws DocumentException { - Permission result = null; - - String actionGroup = permissionValue.getActionGroup() != null ? permissionValue.getActionGroup().trim() : null; - String resourceName = permissionValue.getResourceName() != null ? permissionValue.getResourceName().trim() : null; - String permissionId = permissionValue.getPermissionId() != null ? permissionValue.getPermissionId().trim() : null; - // - // If we have a permission ID, use it to try to lookup the persisted permission - // - if (permissionId != null && !permissionId.isEmpty()) { - try { - result = (Permission)JpaStorageUtils.getEntityByDualKeys( - jpaTransactionContext, - Permission.class.getName(), - PermissionStorageConstants.ID, permissionId, - PermissionStorageConstants.TENANT_ID, tenantId); - } catch (Throwable e) { - String msg = String.format("Searched for but couldn't find a permission with CSID='%s'.", - permissionId); - logger.trace(msg); - } - } else if (Tools.notBlank(resourceName) && Tools.notBlank(actionGroup)) { - // - // If there was no permission ID, then we can try to find the permission with the resource name and action group tuple - // - try { - result = (Permission)JpaStorageUtils.getEntityByDualKeys( - jpaTransactionContext, - Permission.class.getName(), - PermissionStorageConstants.RESOURCE_NAME, permissionValue.getResourceName(), - PermissionStorageConstants.ACTION_GROUP, permissionValue.getActionGroup(), - tenantId); - } catch (NoResultException e) { - String msg = String.format("Searched for but couldn't find a permission for resource='%s', action group='%s', and tenant ID='%s'.", - permissionValue.getResourceName(), permissionValue.getActionGroup(), tenantId); - logger.trace(msg); - } - } else { - String errMsg = String.format("Couldn't perform lookup of permission. Not enough information provided. Lookups requires a permission CSID or a resource name and action group tuple. The provided information was permission ID='%s', resourceName='%s', and actionGroup='%s'.", - permissionId, resourceName, actionGroup); - throw new DocumentException(errMsg); - } - - if (result == null) { - throw new DocumentNotFoundException(String.format("Could not find Permission resource with CSID='%s', actionGroup='%s', resourceName='%s'.", - permissionId, actionGroup, resourceName)); - } - - return result; - } + @SuppressWarnings("unchecked") + static private Permission lookupPermission(JPATransactionContext jpaTransactionContext, + PermissionValue permissionValue, String tenantId) throws DocumentException { + Permission result = null; + + String actionGroup = permissionValue.getActionGroup() != null ? permissionValue.getActionGroup().trim() : null; + String resourceName = permissionValue.getResourceName() != null ? permissionValue.getResourceName().trim() : null; + String permissionId = permissionValue.getPermissionId() != null ? permissionValue.getPermissionId().trim() : null; + // + // If we have a permission ID, use it to try to lookup the persisted permission + // + if (permissionId != null && !permissionId.isEmpty()) { + try { + result = (Permission) JpaStorageUtils.getEntityByDualKeys(jpaTransactionContext, + Permission.class.getName(), PermissionStorageConstants.ID, permissionId, + PermissionStorageConstants.TENANT_ID, tenantId); + } catch (Throwable e) { + String msg = String.format("Searched for but couldn't find a permission with CSID='%s'.", permissionId); + logger.trace(msg); + } + } else if (Tools.notBlank(resourceName) && Tools.notBlank(actionGroup)) { + // + // If there was no permission ID, then we can try to find the permission with + // the resource name and action group tuple + // + try { + result = (Permission) JpaStorageUtils.getEntityByDualKeys(jpaTransactionContext, + Permission.class.getName(), PermissionStorageConstants.RESOURCE_NAME, + permissionValue.getResourceName(), PermissionStorageConstants.ACTION_GROUP, + permissionValue.getActionGroup(), tenantId); + } catch (NonUniqueResultException nue) { + // + // Duplicates can happen after a CSpace instance has been upgraded from v4.x to v5.0+ + // + List resultList = (List) JpaStorageUtils.getEntityListByDualKeys( + jpaTransactionContext, Permission.class.getName(), PermissionStorageConstants.RESOURCE_NAME, + permissionValue.getResourceName(), PermissionStorageConstants.ACTION_GROUP, + permissionValue.getActionGroup(), tenantId); + logger.warn(String.format("Multiple permissions exist for resource '%s' and action group '%s'", + permissionValue.getResourceName(), permissionValue.getActionGroup())); + result = resultList.get(0); + for (Permission p : resultList) { + // + // If we find an auto-generated permission, we should use it instead. + // + if (p.getDescription() != null && p.getDescription().startsWith(AuthN.GENERATED_STR)) { + result = p; + break; + } + } + } catch (NoResultException e) { + String msg = String.format( + "Searched for but couldn't find a permission for resource='%s', action group='%s', and tenant ID='%s'.", + permissionValue.getResourceName(), permissionValue.getActionGroup(), tenantId); + logger.trace(msg); + } + } else { + String errMsg = String.format( + "Couldn't perform lookup of permission. Not enough information provided. Lookups requires a permission CSID or a resource name and action group tuple. The provided information was permission ID='%s', resourceName='%s', and actionGroup='%s'.", + permissionId, resourceName, actionGroup); + throw new DocumentException(errMsg); + } + + if (result == null) { + throw new DocumentNotFoundException(String.format( + "Could not find Permission resource with CSID='%s', actionGroup='%s', resourceName='%s'.", + permissionId, actionGroup, resourceName)); + } + + return result; + } + /** * Ensure the Role's permission relationships can be changed. * diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageUtils.java b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageUtils.java index 7fc0aff2f..109ca4dbc 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageUtils.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageUtils.java @@ -446,6 +446,32 @@ public class JpaStorageUtils { return result; } + + public static List getEntityListByDualKeys(JPATransactionContext jpaTransactionContext, String entityName, + String key1, String value1, String key2, String value2, String tenantId) throws TransactionException { + List result = null; + + boolean useTenantId = useTenantId(tenantId); + StringBuilder queryStrBldr = new StringBuilder("SELECT a FROM "); + queryStrBldr.append(entityName); + queryStrBldr.append(" a"); + queryStrBldr.append(" WHERE " + key1 + " = :" + key1); + queryStrBldr.append(" AND " + key2 + " = :" + key2); + if (useTenantId == true) { + queryStrBldr.append(" AND tenantId = :tenantId"); + } + String queryStr = queryStrBldr.toString(); // for debugging + Query q = jpaTransactionContext.createQuery(queryStr); + q.setParameter(key1, value1); + q.setParameter(key2, value2); + if (useTenantId == true) { + q.setParameter("tenantId", tenantId); + } + + result = q.getResultList(); + + return result; + } /** *