issue #3751: Fix for group permission to be able to add/remove metada…

…ta if write permission is granted + add/remove users from group if write permission is granted (#3766)
epam · Nov 12, 2024 · 98757db · 98757db
1 parent 87a55ec
commit 98757db
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 9 deletions.
diff --git a/api/src/main/java/com/epam/pipeline/acl/user/RoleApiService.java b/api/src/main/java/com/epam/pipeline/acl/user/RoleApiService.java
@@ -86,13 +86,13 @@ public Role deleteRole(Long id) {
         return roleManager.delete(id);
     }
 
-    @PreAuthorize(ADMIN_ONLY)
+    @PreAuthorize(ADMIN_ONLY + OR + "hasPermission(#roleId, 'com.epam.pipeline.entity.user.Role', 'WRITE')")
     @AclMask
     public ExtendedRole assignRole(Long roleId, List<Long> userIds) {
         return roleManager.assignRole(roleId, userIds);
     }
 
-    @PreAuthorize(ADMIN_ONLY)
+    @PreAuthorize(ADMIN_ONLY + OR + "hasPermission(#roleId, 'com.epam.pipeline.entity.user.Role', 'WRITE')")
     public ExtendedRole removeRole(Long roleId, List<Long> userIds) {
         return roleManager.removeRole(roleId, userIds);
     }

diff --git a/api/src/main/java/com/epam/pipeline/manager/security/metadata/MetadataPermissionManager.java b/api/src/main/java/com/epam/pipeline/manager/security/metadata/MetadataPermissionManager.java
@@ -133,7 +133,7 @@ private boolean metadataPermission(final MetadataVO metadataVO, final boolean al
             return isMetadataEditAllowedForUser(metadataVO);
         }
         if (entityClass.equals(AclClass.ROLE)) {
-            return false;
+            return isMetadataEditAllowedForRole(metadataVO);
         }
         if (AclClass.TOOL.equals(entityClass) && isMetadataContainsRestrictedInstanceValues(metadataVO)) {
             return false;
@@ -143,17 +143,33 @@ private boolean metadataPermission(final MetadataVO metadataVO, final boolean al
     }
 
     private boolean isMetadataEditAllowedForUser(final MetadataVO metadataVO) {
-        final List<String> sensitiveKeys = preferenceManager.getPreference(
-                SystemPreferences.MISC_METADATA_SENSITIVE_KEYS);
-        if (MapUtils.isNotEmpty(metadataVO.getData()) && ListUtils.emptyIfNull(sensitiveKeys).stream()
-                .anyMatch(key -> metadataVO.getData().containsKey(key))) {
+        if (metadataHasSensitiveKeys(metadataVO)){
             return false;
         }
         final Long entityId = metadataVO.getEntity().getEntityId();
         return isSameUser(entityId) || permissionHelper.isAllowed("WRITE",
                 entityManager.load(AclClass.PIPELINE_USER, entityId));
     }
 
+    private boolean isMetadataEditAllowedForRole(final MetadataVO metadataVO) {
+        if (metadataHasSensitiveKeys(metadataVO)){
+            return false;
+        }
+        final Long entityId = metadataVO.getEntity().getEntityId();
+        return permissionHelper.isAllowed("WRITE",
+                entityManager.load(AclClass.ROLE, entityId));
+    }
+
+    private boolean metadataHasSensitiveKeys(MetadataVO metadataVO) {
+        final List<String> sensitiveKeys = preferenceManager.getPreference(
+                SystemPreferences.MISC_METADATA_SENSITIVE_KEYS);
+        if (MapUtils.isNotEmpty(metadataVO.getData()) && ListUtils.emptyIfNull(sensitiveKeys).stream()
+                .anyMatch(key -> metadataVO.getData().containsKey(key))) {
+            return true;
+        }
+        return false;
+    }
+
     private boolean isSameUser(final Long entityId) {
         final PipelineUser user = userManager.load(entityId);
         return permissionHelper.isOwner(user.getUserName());

diff --git a/api/src/main/java/com/epam/pipeline/security/acl/AclExpressions.java b/api/src/main/java/com/epam/pipeline/security/acl/AclExpressions.java
@@ -137,8 +137,12 @@ public final class AclExpressions {
 
     public static final String METADATA_FILTER = ADMIN_ONLY + OR +
             "@metadataPermissionManager.metadataPermission(" +
-            "filterObject.entity.entityId, filterObject.entity.entityClass, 'READ')" + OR +
-            "filterObject.entity.entityClass.name() == 'PIPELINE_USER'" + AND + "hasRole('USER_METADATA_READER')";
+            "filterObject.entity.entityId, filterObject.entity.entityClass, 'READ')" +
+            OR + "(" +
+                "filterObject.entity.entityClass.name() == 'PIPELINE_USER'" +
+                OR + "filterObject.entity.entityClass.name() == 'ROLE'" +
+            ")" +
+            AND + "hasRole('USER_METADATA_READER')";
 
     public static final String ACL_ENTITY_OWNER =
             "hasRole('ADMIN') or @grantPermissionManager.ownerPermission(#id, #aclClass)";