You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2021/06/04 12:17:49 UTC

svn commit: r1890460 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/ oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/

Author: angela
Date: Fri Jun  4 12:17:49 2021
New Revision: 1890460

URL: http://svn.apache.org/viewvc?rev=1890460&view=rev
Log:
OAK-9452 : minor improvements to privilege management

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/JcrAllCommitHook.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeConfigurationImpl.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeDefinitionWriter.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeManagerImpl.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeValidator.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeValidatorProvider.java
    jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinition.java
    jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBits.java
    jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeConfiguration.java
    jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeUtil.java
    jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/package-info.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/JcrAllCommitHook.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/JcrAllCommitHook.java?rev=1890460&r1=1890459&r2=1890460&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/JcrAllCommitHook.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/JcrAllCommitHook.java Fri Jun  4 12:17:49 2021
@@ -38,22 +38,26 @@ class JcrAllCommitHook implements PostVa
 
     @NotNull
     @Override
-    public NodeState processCommit(
-            NodeState before, NodeState after, CommitInfo info) {
+    public NodeState processCommit(NodeState before, NodeState after, CommitInfo info) {
         NodeBuilder builder = after.builder();
-        after.compareAgainstBaseState(before, new PrivilegeDiff(null, null, builder));
+        after.compareAgainstBaseState(before, new PrivilegeDiff(builder));
         return builder.getNodeState();
     }
 
-    private final class PrivilegeDiff extends DefaultNodeStateDiff {
+    private static final class PrivilegeDiff extends DefaultNodeStateDiff {
 
         private static final String ROOT_PATH = "";
 
         private final String path;
         private final NodeBuilder nodeBuilder;
 
-        private PrivilegeDiff(PrivilegeDiff parentDiff, String nodeName, NodeBuilder nodeBuilder) {
-            this.path = (nodeName == null) ? ROOT_PATH : parentDiff.path + '/' + nodeName;
+        private PrivilegeDiff(@NotNull NodeBuilder nodeBuilder) {
+            this.path = ROOT_PATH;
+            this.nodeBuilder = nodeBuilder;
+        }
+        
+        private PrivilegeDiff(@NotNull PrivilegeDiff parentDiff, @NotNull String nodeName, @NotNull NodeBuilder nodeBuilder) {
+            this.path = parentDiff.path + '/' + nodeName;
             this.nodeBuilder = nodeBuilder;
         }
 
@@ -62,27 +66,7 @@ class JcrAllCommitHook implements PostVa
             if (PRIVILEGES_PATH.equals(path)) {
                 if (!JCR_ALL.equals(name)) {
                     // a new privilege was registered -> update the jcr:all privilege
-                    NodeBuilder jcrAll = nodeBuilder.child(JCR_ALL);
-                    PropertyState aggregates = jcrAll.getProperty(REP_AGGREGATES);
-
-                    PropertyBuilder<String> propertyBuilder;
-                    if (aggregates == null) {
-                        propertyBuilder = PropertyBuilder.array(Type.NAME, REP_AGGREGATES);
-                    } else {
-                        propertyBuilder = PropertyBuilder.copy(Type.NAME, aggregates);
-                    }
-                    if (!propertyBuilder.hasValue(name)) {
-                        propertyBuilder.addValue(name);
-                        jcrAll.setProperty(propertyBuilder.getPropertyState());
-                    }
-
-                    // update the privilege bits of the jcr:all in case the new
-                    // privilege isn't an aggregate
-                    if (!after.hasProperty(REP_AGGREGATES)) {
-                        PrivilegeBits bits = PrivilegeBits.getInstance(after.getProperty(REP_BITS));
-                        PrivilegeBits all = PrivilegeBits.getInstance(jcrAll.getProperty(REP_BITS));
-                        jcrAll.setProperty(PrivilegeBits.getInstance(all).add(bits).asPropertyState(REP_BITS));
-                    }
+                    updateJcrAll(name, after);
                 } // else: jcr:all privilege has been added -> ignore
             } else {
                 String p = path  + '/' + name;
@@ -92,6 +76,30 @@ class JcrAllCommitHook implements PostVa
             }
             return true;
         }
+        
+        private void updateJcrAll(@NotNull String name, @NotNull NodeState after) {
+            NodeBuilder jcrAll = nodeBuilder.child(JCR_ALL);
+            PropertyState aggregates = jcrAll.getProperty(REP_AGGREGATES);
+
+            PropertyBuilder<String> propertyBuilder;
+            if (aggregates == null) {
+                propertyBuilder = PropertyBuilder.array(Type.NAME, REP_AGGREGATES);
+            } else {
+                propertyBuilder = PropertyBuilder.copy(Type.NAME, aggregates);
+            }
+            if (!propertyBuilder.hasValue(name)) {
+                propertyBuilder.addValue(name);
+                jcrAll.setProperty(propertyBuilder.getPropertyState());
+            }
+
+            // update the privilege bits of the jcr:all in case the new
+            // privilege isn't an aggregate
+            if (!after.hasProperty(REP_AGGREGATES)) {
+                PrivilegeBits bits = PrivilegeBits.getInstance(after.getProperty(REP_BITS));
+                PrivilegeBits all = PrivilegeBits.getInstance(jcrAll.getProperty(REP_BITS));
+                jcrAll.setProperty(PrivilegeBits.getInstance(all).add(bits).asPropertyState(REP_BITS));
+            }
+        }
 
         @Override
         public boolean childNodeChanged(String name, NodeState before, NodeState after) {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeConfigurationImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeConfigurationImpl.java?rev=1890460&r1=1890459&r2=1890460&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeConfigurationImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeConfigurationImpl.java Fri Jun  4 12:17:49 2021
@@ -46,7 +46,7 @@ public class PrivilegeConfigurationImpl
     //---------------------------------------------< PrivilegeConfiguration >---
     @NotNull
     @Override
-    public PrivilegeManager getPrivilegeManager(Root root, NamePathMapper namePathMapper) {
+    public PrivilegeManager getPrivilegeManager(@NotNull Root root, @NotNull NamePathMapper namePathMapper) {
         return new PrivilegeManagerImpl(root, namePathMapper);
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeDefinitionWriter.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeDefinitionWriter.java?rev=1890460&r1=1890459&r2=1890460&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeDefinitionWriter.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeDefinitionWriter.java Fri Jun  4 12:17:49 2021
@@ -125,14 +125,14 @@ class PrivilegeDefinitionWriter implemen
 
     private void writePrivilegeNode(@NotNull Tree privilegesTree, @NotNull PrivilegeDefinition definition) throws RepositoryException {
         String name = definition.getName();
-        Tree privNode = TreeUtil.addChild(privilegesTree, name, NT_REP_PRIVILEGE);
+        Tree privilegeNode = TreeUtil.addChild(privilegesTree, name, NT_REP_PRIVILEGE);
         if (definition.isAbstract()) {
-            privNode.setProperty(REP_IS_ABSTRACT, true);
+            privilegeNode.setProperty(REP_IS_ABSTRACT, true);
         }
         Set<String> declAggrNames = definition.getDeclaredAggregateNames();
         boolean isAggregate = !declAggrNames.isEmpty();
         if (isAggregate) {
-            privNode.setProperty(REP_AGGREGATES, declAggrNames, Type.NAMES);
+            privilegeNode.setProperty(REP_AGGREGATES, declAggrNames, Type.NAMES);
         }
 
         PrivilegeBits bits;
@@ -141,18 +141,18 @@ class PrivilegeDefinitionWriter implemen
         } else if (isAggregate) {
             bits = bitsMgr.getBits(declAggrNames);
             if (bits.isEmpty()) {
-                throw new RepositoryException("Illegal aggregation of non-exising privileges on '" + name + "'.");
+                throw new RepositoryException("Illegal aggregation of non-existing privileges on '" + name + "'.");
             }
         } else {
             bits = next();
         }
-        bits.writeTo(privNode);
+        bits.writeTo(privilegeNode);
     }
 
     @NotNull
     private static Collection<PrivilegeDefinition> getBuiltInDefinitions() {
         Map<String, PrivilegeDefinition> definitions = new LinkedHashMap<>();
-        NON_AGGREGATE_PRIVILEGES.forEach((privilegeName) -> {
+        NON_AGGREGATE_PRIVILEGES.forEach(privilegeName -> {
             PrivilegeDefinition def = new ImmutablePrivilegeDefinition(privilegeName, false, null);
             definitions.put(privilegeName, def);
         });

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeManagerImpl.java?rev=1890460&r1=1890459&r2=1890460&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeManagerImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeManagerImpl.java Fri Jun  4 12:17:49 2021
@@ -26,6 +26,7 @@ import javax.jcr.RepositoryException;
 import javax.jcr.security.AccessControlException;
 import javax.jcr.security.Privilege;
 
+import com.google.common.base.Strings;
 import org.apache.jackrabbit.api.security.authorization.PrivilegeManager;
 import org.apache.jackrabbit.oak.api.Root;
 import org.apache.jackrabbit.oak.namepath.NamePathMapper;
@@ -81,7 +82,7 @@ class PrivilegeManagerImpl implements Pr
         if (root.hasPendingChanges()) {
             throw new InvalidItemStateException("Attempt to register a new privilege while there are pending changes.");
         }
-        if (privilegeName == null || privilegeName.isEmpty()) {
+        if (Strings.isNullOrEmpty(privilegeName)) {
             throw new RepositoryException("Invalid privilege name " + privilegeName);
         }
         PrivilegeDefinition definition = new ImmutablePrivilegeDefinition(getOakName(privilegeName), isAbstract, getOakNames(declaredAggregateNames));

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeValidator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeValidator.java?rev=1890460&r1=1890459&r2=1890460&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeValidator.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeValidator.java Fri Jun  4 12:17:49 2021
@@ -125,7 +125,7 @@ class PrivilegeValidator extends Default
     }
 
     //------------------------------------------------------------< private >---
-    private void validateNext(PrivilegeBits bits) throws CommitFailedException {
+    private void validateNext(@NotNull PrivilegeBits bits) throws CommitFailedException {
         PrivilegeBits next = PrivilegeBits.getInstance(getPrivilegesTree(rootAfter).getProperty(REP_NEXT));
         if (!next.equals(bits.nextBits())) {
             throw new CommitFailedException(CONSTRAINT, 43, "Next bits not updated");
@@ -133,7 +133,7 @@ class PrivilegeValidator extends Default
     }
 
     @NotNull
-    private Tree getPrivilegesTree(Root root) throws CommitFailedException {
+    private static Tree getPrivilegesTree(@NotNull Root root) throws CommitFailedException {
         Tree privilegesTree = root.getTree(PRIVILEGES_PATH);
         if (!privilegesTree.exists()) {
             throw new CommitFailedException(CONSTRAINT, 44, "Privilege store not initialized.");
@@ -155,19 +155,19 @@ class PrivilegeValidator extends Default
      *          If any of
      *          the checks listed above fails.
      */
-    private void validateDefinition(Tree definitionTree) throws CommitFailedException {
+    private void validateDefinition(@NotNull Tree definitionTree) throws CommitFailedException {
         PrivilegeBits newBits = PrivilegeBits.getInstance(definitionTree);
         if (newBits.isEmpty()) {
             throw new CommitFailedException(CONSTRAINT, 48, "PrivilegeBits are missing.");
         }
 
-        Set<String> privNames = bitsProvider.getPrivilegeNames(newBits);
+        Set<String> privilegeNames = bitsProvider.getPrivilegeNames(newBits);
         PrivilegeDefinition definition = PrivilegeUtil.readDefinition(definitionTree);
         Set<String> declaredNames = definition.getDeclaredAggregateNames();
 
         // non-aggregate privilege
         if (declaredNames.isEmpty()) {
-            if (!privNames.isEmpty()) {
+            if (!privilegeNames.isEmpty()) {
                 throw new CommitFailedException(CONSTRAINT, 49, "PrivilegeBits already in used.");
             }
             validateNext(newBits);
@@ -180,7 +180,16 @@ class PrivilegeValidator extends Default
         }
 
         // aggregation of >1 privileges
-        Map<String, PrivilegeDefinition> definitions = new PrivilegeDefinitionReader(rootBefore).readDefinitions();
+        validateAggregation(definition.getName(), declaredNames, new PrivilegeDefinitionReader(rootBefore).readDefinitions());
+
+        PrivilegeBits aggrBits = bitsProvider.getBits(declaredNames.toArray(new String[0]));
+        if (!newBits.equals(aggrBits)) {
+            throw new CommitFailedException(CONSTRAINT, 53, "Invalid privilege bits for aggregated privilege definition.");
+        }
+    }
+    
+    private static void validateAggregation(@NotNull String definitionName, @NotNull Set<String> declaredNames , 
+                                            @NotNull Map<String, PrivilegeDefinition> definitions) throws CommitFailedException {
         for (String aggrName : declaredNames) {
             // aggregated privilege not registered
             if (!definitions.containsKey(aggrName)) {
@@ -188,7 +197,7 @@ class PrivilegeValidator extends Default
             }
 
             // check for circular aggregation
-            if (isCircularAggregation(definition.getName(), aggrName, definitions)) {
+            if (isCircularAggregation(definitionName, aggrName, definitions)) {
                 String msg = "Detected circular aggregation within custom privilege caused by " + aggrName;
                 throw new CommitFailedException(CONSTRAINT, 52, msg);
             }
@@ -203,29 +212,24 @@ class PrivilegeValidator extends Default
 
             // test for exact same aggregation or aggregation with the same net effect
             if (declaredNames.equals(existingDeclared) || aggregateNames.equals(resolveAggregates(existingDeclared, definitions))) {
-                String msg = "Custom aggregate privilege '" + definition.getName() + "' is already covered by '" + existing.getName() + '\'';
+                String msg = "Custom aggregate privilege '" + definitionName + "' is already covered by '" + existing.getName() + '\'';
                 throw new CommitFailedException(CONSTRAINT, 53, msg);
             }
         }
-
-        PrivilegeBits aggrBits = bitsProvider.getBits(declaredNames.toArray(new String[0]));
-        if (!newBits.equals(aggrBits)) {
-            throw new CommitFailedException(CONSTRAINT, 53, "Invalid privilege bits for aggregated privilege definition.");
-        }
     }
 
-    private static boolean isCircularAggregation(String privilegeName, String aggregateName,
-                                                 Map<String, PrivilegeDefinition> definitions) {
+    private static boolean isCircularAggregation(@NotNull String privilegeName, @NotNull String aggregateName,
+                                                 @NotNull Map<String, PrivilegeDefinition> definitions) {
         if (privilegeName.equals(aggregateName)) {
             return true;
         }
 
-        PrivilegeDefinition aggrPriv = definitions.get(aggregateName);
-        if (aggrPriv.getDeclaredAggregateNames().isEmpty()) {
+        PrivilegeDefinition aggrDefinition = definitions.get(aggregateName);
+        if (aggrDefinition.getDeclaredAggregateNames().isEmpty()) {
             return false;
         } else {
             boolean isCircular = false;
-            for (String name : aggrPriv.getDeclaredAggregateNames()) {
+            for (String name : aggrDefinition.getDeclaredAggregateNames()) {
                 if (privilegeName.equals(name)) {
                     return true;
                 }
@@ -237,7 +241,8 @@ class PrivilegeValidator extends Default
         }
     }
 
-    private static Set<String> resolveAggregates(Set<String> declared, Map<String, PrivilegeDefinition> definitions) throws CommitFailedException {
+    @NotNull
+    private static Set<String> resolveAggregates(@NotNull Set<String> declared, @NotNull Map<String, PrivilegeDefinition> definitions) throws CommitFailedException {
         Set<String> aggregateNames = new HashSet<>();
         for (String name : declared) {
             PrivilegeDefinition d = definitions.get(name);

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeValidatorProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeValidatorProvider.java?rev=1890460&r1=1890459&r2=1890460&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeValidatorProvider.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeValidatorProvider.java Fri Jun  4 12:17:49 2021
@@ -46,10 +46,8 @@ class PrivilegeValidatorProvider extends
 
     @NotNull
     @Override
-    public Validator getRootValidator(
-            NodeState before, NodeState after, CommitInfo info) {
-        return new SubtreeValidator(new PrivilegeValidator(createRoot(before), createRoot(after), treeProvider),
-                JCR_SYSTEM, REP_PRIVILEGES);
+    public Validator getRootValidator(NodeState before, NodeState after, CommitInfo info) {
+        return new SubtreeValidator(new PrivilegeValidator(createRoot(before), createRoot(after), treeProvider), JCR_SYSTEM, REP_PRIVILEGES);
     }
 
     private Root createRoot(NodeState nodeState) {

Modified: jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinition.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinition.java?rev=1890460&r1=1890459&r2=1890460&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinition.java (original)
+++ jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinition.java Fri Jun  4 12:17:49 2021
@@ -31,6 +31,7 @@ public final class ImmutablePrivilegeDef
     private final String name;
     private final boolean isAbstract;
     private final Set<String> declaredAggregateNames;
+    private final int hashcode;
 
     public ImmutablePrivilegeDefinition(@NotNull String name, boolean isAbstract, @Nullable Iterable<String> declaredAggregateNames) {
         this.name = name;
@@ -40,6 +41,7 @@ public final class ImmutablePrivilegeDef
             builder.addAll(declaredAggregateNames);
         }
         this.declaredAggregateNames = builder.build();
+        hashcode = Objects.hashCode(this.name, this.isAbstract, this.declaredAggregateNames);
     }
 
     //------------------------------------------------< PrivilegeDefinition >---
@@ -63,7 +65,7 @@ public final class ImmutablePrivilegeDef
     //-------------------------------------------------------------< Object >---
     @Override
     public int hashCode() {
-        return Objects.hashCode(name, isAbstract(),  declaredAggregateNames);
+        return hashcode;
     }
 
     @Override

Modified: jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBits.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBits.java?rev=1890460&r1=1890459&r2=1890460&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBits.java (original)
+++ jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBits.java Fri Jun  4 12:17:49 2021
@@ -265,7 +265,6 @@ public final class PrivilegeBits impleme
                                             @NotNull PrivilegeBits parentBits,
                                             boolean isAllow) {
         long privs = bits.d.longValue();
-        long parentPrivs = parentBits.d.longValue();
         long perm = Permissions.NO_PERMISSION;
         if ((privs & READ) == READ) {
             perm |= Permissions.READ;
@@ -289,7 +288,16 @@ public final class PrivilegeBits impleme
                 perm |= Permissions.REMOVE_PROPERTY;
             }
         }
+        perm |= calculateWritePermissions(privs, parentBits.d.longValue(), isAllow);
 
+        // the remaining (special) permissions are simply defined on the node
+        perm |= calculateOtherPermissions(privs);
+        return perm;
+    }
+    
+    private static long calculateWritePermissions(long privs, long parentPrivs, boolean isAllow) {
+        long perm = Permissions.NO_PERMISSION;
+        
         // add_node permission is granted through privilege on the parent.
         if ((parentPrivs & ADD_CHILD_NODES) == ADD_CHILD_NODES) {
             perm |= Permissions.ADD_NODE;
@@ -319,8 +327,11 @@ public final class PrivilegeBits impleme
                 (privs & REMOVE_CHILD_NODES) == REMOVE_CHILD_NODES) {
             perm |= Permissions.MODIFY_CHILD_NODE_COLLECTION;
         }
-
-        // the remaining (special) permissions are simply defined on the node
+        return perm;
+    }
+    
+    private static long calculateOtherPermissions(long privs) {
+        long perm = Permissions.NO_PERMISSION;
         if ((privs & READ_AC) == READ_AC) {
             perm |= Permissions.READ_ACCESS_CONTROL;
         }
@@ -637,17 +648,20 @@ public final class PrivilegeBits impleme
         private final long bits;
         private final long[] bitsArr;
         private final boolean isSimple;
+        private final int hashcode;
 
         private UnmodifiableData(long bits) {
             this.bits = bits;
             bitsArr = null;
             isSimple = true;
+            hashcode = Long.valueOf(this.bits).hashCode();
         }
 
         private UnmodifiableData(long[] bitsArr) {
             bits = NO_PRIVILEGE;
             this.bitsArr = bitsArr;
             isSimple = false;
+            hashcode = Arrays.hashCode(this.bitsArr);
         }
 
         @Override
@@ -711,7 +725,7 @@ public final class PrivilegeBits impleme
         //---------------------------------------------------------< Object >---
         @Override
         public int hashCode() {
-            return (isSimple) ? Long.valueOf(bits).hashCode() : Arrays.hashCode(bitsArr);
+            return hashcode;
         }
 
         @Override
@@ -863,7 +877,7 @@ public final class PrivilegeBits impleme
 
         private static long[] diff(long[] a, long[] b) {
             int index = -1;
-            long[] res = new long[((a.length > b.length) ? a.length : b.length)];
+            long[] res = new long[(Math.max(a.length, b.length))];
             for (int i = 0; i < res.length; i++) {
                 if (i < a.length && i < b.length) {
                     res[i] = a[i] & ~b[i];

Modified: jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeConfiguration.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeConfiguration.java?rev=1890460&r1=1890459&r2=1890460&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeConfiguration.java (original)
+++ jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeConfiguration.java Fri Jun  4 12:17:49 2021
@@ -37,5 +37,5 @@ public interface PrivilegeConfiguration
      * @return A new {@code PrivilegeManager}.
      */
     @NotNull
-    PrivilegeManager getPrivilegeManager(Root root, NamePathMapper namePathMapper);
+    PrivilegeManager getPrivilegeManager(@NotNull Root root, @NotNull NamePathMapper namePathMapper);
 }

Modified: jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeUtil.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeUtil.java?rev=1890460&r1=1890459&r2=1890460&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeUtil.java (original)
+++ jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeUtil.java Fri Jun  4 12:17:49 2021
@@ -37,7 +37,7 @@ public final class PrivilegeUtil impleme
      * @return The privileges root.
      */
     @NotNull
-    public static Tree getPrivilegesTree(Root root) {
+    public static Tree getPrivilegesTree(@NotNull Root root) {
         return root.getTree(PRIVILEGES_PATH);
     }
 

Modified: jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/package-info.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/package-info.java?rev=1890460&r1=1890459&r2=1890460&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/package-info.java (original)
+++ jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/package-info.java Fri Jun  4 12:17:49 2021
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-@Version("1.4.1")
+@Version("1.4.2")
 package org.apache.jackrabbit.oak.spi.security.privilege;
 
 import org.osgi.annotation.versioning.Version;