[feature] Deduplicate contiguous entries in ACLs that target the same resource and access conditions

Closes eXist-db/exist#5138

Author: adamretter

exist-core/pom.xml

@@ -715,6 +715,7 @@
                                 <include>src/test/java/org/exist/xquery/functions/xmldb/XMLDBStoreTest.java</include>
                                 <include>src/test/java/org/exist/xquery/functions/xquery3/SerializeTest.java</include>
                                 <include>src/test/resources-filtered/org/exist/xquery/import-from-pkg-test.conf.xml</include>
+                                <include>src/test/xquery/securitymanager/acl.xqm</include>
                             </includes>
                         </licenseSet>
 
@@ -815,6 +816,7 @@
                                 <include>src/main/java/org/exist/scheduler/impl/QuartzSchedulerImpl.java</include>
                                 <include>src/main/java/org/exist/security/EffectiveSubject.java</include>
                                 <include>src/main/java/org/exist/security/SecurityManager.java</include>
+                                <include>src/main/java/org/exist/security/SimpleACLPermission.java</include>
                                 <include>src/main/java/org/exist/security/internal/AccountImpl.java</include>
                                 <include>src/main/java/org/exist/source/Source.java</include>
                                 <include>src/main/java/org/exist/source/SourceFactory.java</include>
@@ -1027,6 +1029,7 @@
                                 <exclude>src/main/java/org/exist/scheduler/impl/QuartzSchedulerImpl.java</exclude>
                                 <exclude>src/main/java/org/exist/security/EffectiveSubject.java</exclude>
                                 <exclude>src/main/java/org/exist/security/SecurityManager.java</exclude>
+                                <exclude>src/main/java/org/exist/security/SimpleACLPermission.java</exclude>
                                 <exclude>src/main/java/org/exist/security/internal/AccountImpl.java</exclude>
                                 <exclude>src/main/java/org/exist/source/Source.java</exclude>
                                 <exclude>src/main/java/org/exist/source/SourceFactory.java</exclude>
@@ -1214,6 +1217,7 @@
                                 <exclude>src/test/java/org/exist/xquery/value/SubSequenceRangeTest.java</exclude>
                                 <exclude>src/test/java/org/exist/xquery/value/SubSequenceTest.java</exclude>
                                 <exclude>src/test/xquery/binary-value.xqm</exclude>
+                                <exclude>src/test/xquery/securitymanager/acl.xqm</exclude>
                                 <exclude>src/test/xquery/xquery3/postfix-expr.xqm</exclude>
                                 <exclude>src/test/xquery/xquery3/serialize.xql</exclude>
 

exist-core/src/main/java/org/exist/security/SimpleACLPermission.java

@@ -1,4 +1,28 @@
 /*
+ * Elemental
+ * Copyright (C) 2024, Evolved Binary Ltd
+ *
+ * [email protected]
+ * https://www.evolvedbinary.com | https://www.elemental.xyz
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; version 2.1.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * NOTE: Parts of this file contain code from 'The eXist-db Authors'.
+ *       The original license header is included below.
+ *
+ * =====================================================================
+ *
  * eXist-db Open Source Native XML Database
  * Copyright (C) 2001 The eXist-db Authors
  *
@@ -27,6 +51,8 @@
 import org.exist.storage.io.VariableByteInput;
 import org.exist.storage.io.VariableByteOutputStream;
 
+import javax.annotation.Nullable;
+
 import static org.exist.security.PermissionRequired.IS_DBA;
 import static org.exist.security.PermissionRequired.IS_OWNER;
 import static org.exist.security.PermissionRequired.ACL_WRITE;
@@ -85,10 +111,68 @@ private void addACE(final ACE_ACCESS_TYPE access_type, final ACE_TARGET target,
             throw new PermissionDeniedException("Maximum of " + MAX_ACL_LENGTH + " ACEs has been reached.");
         }
 
-        final int[] newAcl = new int[acl.length + 1];
-        System.arraycopy(acl, 0, newAcl, 0, acl.length);
-        newAcl[newAcl.length - 1] = encodeAsACE(access_type, target, id, mode);
-        this.acl = newAcl;
+        // this is the final mode of all ACEs in the ACL for the targeted `id`
+        @Nullable Integer computedAclMode = null;
+
+        // Walk through the existing ACE and see what the permissions are for the id
+        for (int i = 0; i < acl.length; i++) {
+            final ACE_TARGET aceTarget = getACETarget(i);
+            final int aceId = getACEId(i);
+            final int aceMode = getACEMode(i);
+            final ACE_ACCESS_TYPE aceAccessType = getACEAccessType(i);
+
+            if (aceId == id && aceTarget == target) {
+                // ids match, and existing ACE is `user` and requested addition is `user`, or existing ACE is `group` and requested addition is `group`
+
+                if (aceMode == mode) {
+                    // NOTE(AR) exit - we do not want to add a duplicate mode entry for same target and id
+                    return;
+                }
+
+                computedAclMode = computeCumulativeAclMode(computedAclMode, aceAccessType, aceMode);
+            }
+        }
+
+        final int newMode;
+        if (computedAclMode == null) {
+            newMode = mode;
+        } else {
+            newMode = (computedAclMode ^ mode) & mode;
+        }
+
+        if (newMode > 0) {
+            final int[] newAcl = new int[acl.length + 1];
+            System.arraycopy(acl, 0, newAcl, 0, acl.length);
+            newAcl[newAcl.length - 1] = encodeAsACE(access_type, target, id, newMode);
+            this.acl = newAcl;
+        }
+    }
+
+    private int computeCumulativeAclMode(@Nullable final Integer existingComputedAclMode, final ACE_ACCESS_TYPE aceAccessType, final int aceMode) {
+        if (existingComputedAclMode == null) {
+            return aceMode;
+        }
+
+        if (aceAccessType == ACE_ACCESS_TYPE.ALLOWED) {
+            // ALLOWED, so we set bits
+            return existingComputedAclMode | aceMode;
+        } else {
+            int computedAclMode = existingComputedAclMode;
+            // DENIED, so we unset bits
+            if ((aceMode & READ) == READ) {
+                // unset read bit
+                computedAclMode = computedAclMode & 3;
+            }
+            if ((aceMode & WRITE) == WRITE) {
+                // unset write bit
+                computedAclMode = computedAclMode & 5;
+            }
+            if ((aceMode & EXECUTE) == EXECUTE) {
+                // unset execute bit
+                computedAclMode = computedAclMode & 6;
+            }
+            return computedAclMode;
+        }
     }
 
     public void insertUserACE(final int index, final ACE_ACCESS_TYPE access_type, final int userId, final int mode) throws PermissionDeniedException {