[ofbiz-framework] branch trunk updated: Improved: Error in user impersonation with sub permission (OFBIZ-11342)

[pgil]

This is an automated email from the ASF dual-hosted git repository.

pgil pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 8653b63  Improved: Error in user impersonation with sub permission (OFBIZ-11342)
8653b63 is described below

commit 8653b6374ed5a12acb0da41a1637faee01dd574c
Author: Gil Portenseigne <[hidden email]>
AuthorDate: Thu Feb 13 14:59:53 2020 +0100

    Improved: Error in user impersonation with sub permission
    (OFBIZ-11342)
   
    Improved javadoc
    Set 'checkMultiLevelAdminPermissionValidity' visibility to default
    Add another test verifying that hierarchy in permission is respected
   
    Thanks Mathieu for your review
---
 .../src/main/java/org/apache/ofbiz/security/SecurityUtil.java     | 6 +++---
 .../src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java | 8 ++++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/framework/security/src/main/java/org/apache/ofbiz/security/SecurityUtil.java b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityUtil.java
index 37aa15f..56f5e41 100644
--- a/framework/security/src/main/java/org/apache/ofbiz/security/SecurityUtil.java
+++ b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityUtil.java
@@ -124,14 +124,14 @@ public final class SecurityUtil {
     }
 
     /**
-     * Return if an admin permission is valid for the given list of permissions.
+     * Return {@code true} if an admin permission is valid for the given list of permissions.
      *
      * @param permissionIds List of admin permission value without "_ADMIN" suffix
      * @param permission permission to be checked with its suffix
      *
      */
-    public static boolean checkMultiLevelAdminPermissionValidity(List<String> permissionIds, String permission) {
-        while (permission.lastIndexOf("_") != -1) {
+    static boolean checkMultiLevelAdminPermissionValidity(List<String> permissionIds, String permission) {
+        while (permission.contains("_")) {
             permission = permission.substring(0, permission.lastIndexOf("_"));
             if (permissionIds.contains(permission)) return true;
         }
diff --git a/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java b/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java
index 5f9b339..47b8bb6 100644
--- a/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java
+++ b/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java
@@ -44,4 +44,12 @@ public class SecurityUtilTest {
                     adminPermissions, "EXAMPLE_WITH_MULTI_LEVEL_ADMIN"));
         assertFalse(SecurityUtil.checkMultiLevelAdminPermissionValidity(adminPermissions, "ACCTG_ADMIN"));
     }
+
+    @Test
+    public void multiLevelBadHierarchyPermissionTesting() {
+        List<String> adminPermissions = Arrays.asList("PARTYMGR", "EXAMPLE", "ACCTG_PREF");
+        assertFalse(SecurityUtil.checkMultiLevelAdminPermissionValidity(
+                    adminPermissions, "SPECIFIC_MULTI_LEVEL_EXAMPLE_VIEW"));
+        assertFalse(SecurityUtil.checkMultiLevelAdminPermissionValidity(adminPermissions, "HOTDEP_PARTYMGR_ADMIN"));
+    }
 }