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 2019/08/14 08:20:35 UTC

svn commit: r1865092 - in /jackrabbit/oak/trunk/oak-authorization-principalbased/src: main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/

Author: angela
Date: Wed Aug 14 08:20:34 2019
New Revision: 1865092

URL: http://svn.apache.org/viewvc?rev=1865092&view=rev
Log:
OAK-8540 : Effective policies should implememt PrincipalAccessControlList

Added:
    jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/AbstractEntry.java   (with props)
    jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ImmutablePrincipalPolicy.java   (with props)
    jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/AbstractEntryTest.java   (with props)
    jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ImmutablePrincipalPolicyTest.java   (with props)
Modified:
    jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalBasedAccessControlManager.java
    jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalPolicyImpl.java
    jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/EffectivePolicyTest.java
    jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalBasedAccessControlManagerTest.java
    jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalPolicyImplTest.java

Added: jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/AbstractEntry.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/AbstractEntry.java?rev=1865092&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/AbstractEntry.java (added)
+++ jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/AbstractEntry.java Wed Aug 14 08:20:34 2019
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.spi.security.authorization.principalbased.impl;
+
+import com.google.common.base.Objects;
+import org.apache.jackrabbit.api.security.authorization.PrincipalAccessControlList;
+import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.ACE;
+import org.apache.jackrabbit.oak.spi.security.authorization.restriction.Restriction;
+import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBits;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import javax.jcr.security.AccessControlException;
+import java.security.Principal;
+import java.util.Set;
+
+abstract class AbstractEntry extends ACE implements PrincipalAccessControlList.Entry {
+
+    private final String oakPath;
+
+    private int hashCode;
+
+    AbstractEntry(@Nullable String oakPath, @NotNull Principal principal, @NotNull PrivilegeBits privilegeBits, @NotNull Set<Restriction> restrictions, @NotNull NamePathMapper namePathMapper) throws AccessControlException {
+        super(principal, privilegeBits, true, restrictions, namePathMapper);
+        this.oakPath = oakPath;
+    }
+
+    @Nullable
+    String getOakPath() {
+        return oakPath;
+    }
+
+    @NotNull
+    abstract NamePathMapper getNamePathMapper();
+
+    @Override
+    @Nullable
+    public String getEffectivePath() {
+        return (oakPath == null) ? null : getNamePathMapper().getJcrPath(oakPath);
+    }
+
+    @Override
+    public int hashCode() {
+        if (hashCode == 0) {
+            hashCode = Objects.hashCode(oakPath, getPrincipal().getName(), getPrivilegeBits(), Boolean.TRUE, getRestrictions());
+        }
+        return hashCode;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (obj == this) {
+            return true;
+        }
+        if (obj instanceof AbstractEntry) {
+            AbstractEntry other = (AbstractEntry) obj;
+            return equivalentPath(other.oakPath) && super.equals(obj);
+        }
+        return false;
+    }
+
+    private boolean equivalentPath(@Nullable String otherPath) {
+        return (oakPath == null) ? otherPath == null : oakPath.equals(otherPath);
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/AbstractEntry.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ImmutablePrincipalPolicy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ImmutablePrincipalPolicy.java?rev=1865092&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ImmutablePrincipalPolicy.java (added)
+++ jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ImmutablePrincipalPolicy.java Wed Aug 14 08:20:34 2019
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.spi.security.authorization.principalbased.impl;
+
+import com.google.common.base.Objects;
+import org.apache.jackrabbit.api.security.authorization.PrincipalAccessControlList;
+import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.ImmutableACL;
+import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.jcr.RepositoryException;
+import javax.jcr.Value;
+import javax.jcr.security.AccessControlException;
+import javax.jcr.security.Privilege;
+import java.security.Principal;
+import java.util.List;
+import java.util.Map;
+
+class ImmutablePrincipalPolicy extends ImmutableACL implements PrincipalAccessControlList {
+
+    private static final Logger log = LoggerFactory.getLogger(ImmutablePrincipalPolicy.class);
+
+    private final Principal principal;
+
+    private int hashCode;
+
+    public ImmutablePrincipalPolicy(@NotNull Principal principal, @NotNull String oakPath, @NotNull List<? extends PrincipalAccessControlList.Entry> entries, @NotNull RestrictionProvider restrictionProvider, @NotNull NamePathMapper namePathMapper) {
+        super(oakPath, entries, restrictionProvider, namePathMapper);
+        this.principal = principal;
+    }
+
+    public ImmutablePrincipalPolicy(@NotNull PrincipalPolicyImpl accessControlList) {
+        super(accessControlList);
+        this.principal = accessControlList.getPrincipal();
+    }
+
+    //-----------------------------------------< PrincipalAccessControlList >---
+    @Override
+    public @NotNull Principal getPrincipal() {
+        return principal;
+    }
+
+    @Override
+    public boolean addEntry(@Nullable String effectivePath, @NotNull Privilege[] privileges) throws RepositoryException {
+        throw new AccessControlException("Immutable PrincipalAccessControlList.");
+    }
+
+    @Override
+    public boolean addEntry(@Nullable String effectivePath, @NotNull Privilege[] privileges, @NotNull Map<String, Value> restrictions, @NotNull Map<String, Value[]> mvRestrictions) throws RepositoryException {
+        throw new AccessControlException("Immutable PrincipalAccessControlList.");
+    }
+
+    //-------------------------------------------------------------< Object >---
+    @Override
+    public int hashCode() {
+        if (hashCode == 0) {
+            hashCode = Objects.hashCode(principal, getOakPath(), getEntries());
+        }
+        return hashCode;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (obj == this) {
+            return true;
+        }
+        if (obj instanceof ImmutablePrincipalPolicy) {
+            ImmutablePrincipalPolicy other = (ImmutablePrincipalPolicy) obj;
+            return Objects.equal(getOakPath(), other.getOakPath())
+                    && principal.equals(other.principal)
+                    && getEntries().equals(other.getEntries());
+        }
+        return false;
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ImmutablePrincipalPolicy.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalBasedAccessControlManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalBasedAccessControlManager.java?rev=1865092&r1=1865091&r2=1865092&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalBasedAccessControlManager.java (original)
+++ jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalBasedAccessControlManager.java Wed Aug 14 08:20:34 2019
@@ -19,8 +19,8 @@ package org.apache.jackrabbit.oak.spi.se
 import com.google.common.base.Strings;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
-import org.apache.jackrabbit.api.security.JackrabbitAccessControlEntry;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlPolicy;
+import org.apache.jackrabbit.api.security.authorization.PrincipalAccessControlList;
 import org.apache.jackrabbit.api.security.principal.PrincipalManager;
 import org.apache.jackrabbit.commons.iterator.AccessControlPolicyIteratorAdapter;
 import org.apache.jackrabbit.oak.api.PropertyState;
@@ -36,17 +36,14 @@ import org.apache.jackrabbit.oak.namepat
 import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
 import org.apache.jackrabbit.oak.spi.nodetype.NodeTypeConstants;
 import org.apache.jackrabbit.oak.spi.query.QueryConstants;
-import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.ACE;
 import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AbstractAccessControlManager;
 import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;
-import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.ImmutableACL;
 import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.PolicyOwner;
 import org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions;
 import org.apache.jackrabbit.oak.spi.security.authorization.principalbased.Filter;
 import org.apache.jackrabbit.oak.spi.security.authorization.principalbased.FilterProvider;
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.Restriction;
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider;
-import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBits;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBitsProvider;
 import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
@@ -68,11 +65,11 @@ import java.security.Principal;
 import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.TreeMap;
 
 /**
  * Implementation of the {@link org.apache.jackrabbit.api.security.JackrabbitAccessControlManager}
@@ -182,15 +179,17 @@ class PrincipalBasedAccessControlManager
             QueryEngine queryEngine = getLatestRoot().getQueryEngine();
             Result result = queryEngine.executeQuery(stmt.toString(), Query.XPATH, QueryEngine.NO_BINDINGS, QueryEngine.NO_MAPPINGS);
 
-            Map<String, List<JackrabbitAccessControlEntry>> m = new TreeMap<>();
+            Map<Principal, List<AbstractEntry>> m = new HashMap<>();
             for (ResultRow row : result.getRows()) {
                 Tree entryTree = row.getTree(null);
-                String effectivePath = row.getValue(REP_EFFECTIVE_PATH).getValue(Type.STRING);
-                List<JackrabbitAccessControlEntry> entries = m.computeIfAbsent(effectivePath, s -> new ArrayList<>());
-                entries.add(createEffectiveEntry(entryTree, effectivePath));
+                AbstractEntry entry = createEffectiveEntry(entryTree);
+                if (entry != null) {
+                    List<AbstractEntry> entries = m.computeIfAbsent(entry.getPrincipal(), s -> new ArrayList<>());
+                    entries.add(entry);
+                }
             }
-            Iterable<ImmutableACL> acls = Iterables.transform(m.entrySet(), entry -> new ImmutableACL(entry.getKey(), entry.getValue(), mgrProvider.getRestrictionProvider(), getNamePathMapper()));
-            return Iterables.toArray(acls, ImmutableACL.class);
+            Iterable<PrincipalAccessControlList> acls = Iterables.transform(m.entrySet(), entry -> new ImmutablePrincipalPolicy(entry.getKey(), filter.getOakPath(entry.getKey()), entry.getValue(), mgrProvider.getRestrictionProvider(), getNamePathMapper()));
+            return Iterables.toArray(acls, PrincipalAccessControlList.class);
         } catch (ParseException e) {
             String msg = "Error while collecting effective policies at " +absPath;
             log.error(msg, e);
@@ -344,7 +343,7 @@ class PrincipalBasedAccessControlManager
             }
         }
         if (isEffectivePolicy && policy != null) {
-            return (policy.isEmpty()) ? null : new ImmutableACL(policy);
+            return (policy.isEmpty()) ? null : new ImmutablePrincipalPolicy(policy);
         } else {
             return policy;
         }
@@ -368,23 +367,28 @@ class PrincipalBasedAccessControlManager
         return paths;
     }
 
-    @NotNull
-    private JackrabbitAccessControlEntry createEffectiveEntry(@NotNull Tree entryTree, @NotNull String effectivePath) throws AccessControlException {
+    @Nullable
+    private AbstractEntry createEffectiveEntry(@NotNull Tree entryTree) throws AccessControlException {
         String principalName = TreeUtil.getString(entryTree.getParent(), AccessControlConstants.REP_PRINCIPAL_NAME);
-        PrivilegeBits bits = privilegeBitsProvider.getBits(entryTree.getProperty(Constants.REP_PRIVILEGES).getValue(Type.NAMES));
-        Set<Restriction> restrictions = mgrProvider.getRestrictionProvider().readRestrictions(effectivePath, entryTree);
-        return new EffectiveEntry(new PrincipalImpl(principalName), bits, true, restrictions, getNamePathMapper());
-    }
-
-    private final class EffectiveEntry extends ACE {
-        private EffectiveEntry(Principal principal, PrivilegeBits privilegeBits, boolean isAllow, Set<Restriction> restrictions, NamePathMapper namePathMapper) throws AccessControlException {
-            super(principal, privilegeBits, isAllow, restrictions, namePathMapper);
+        Principal principal = principalManager.getPrincipal(principalName);
+        if (principal == null || !filter.canHandle(Collections.singleton(principal))) {
+            return null;
         }
+        String oakPath = Strings.emptyToNull(TreeUtil.getString(entryTree, REP_EFFECTIVE_PATH));
+        PrivilegeBits bits = privilegeBitsProvider.getBits(entryTree.getProperty(Constants.REP_PRIVILEGES).getValue(Type.NAMES));
+        Set<Restriction> restrictions = mgrProvider.getRestrictionProvider().readRestrictions(oakPath, entryTree);
+        NamePathMapper npMapper = getNamePathMapper();
+        return new AbstractEntry(oakPath, principal, bits, restrictions, npMapper) {
+            @Override
+            @NotNull NamePathMapper getNamePathMapper() {
+                return npMapper;
+            }
 
-        @Override
-        public Privilege[] getPrivileges() {
-            Set<String> names =  privilegeBitsProvider.getPrivilegeNames(getPrivilegeBits());
-            return Utils.privilegesFromOakNames(names, mgrProvider.getPrivilegeManager(), getNamePathMapper());
-        }
+            @Override
+            public Privilege[] getPrivileges() {
+                Set<String> names =  privilegeBitsProvider.getPrivilegeNames(getPrivilegeBits());
+                return Utils.privilegesFromOakNames(names, mgrProvider.getPrivilegeManager(), getNamePathMapper());
+            }
+        };
     }
 }

Modified: jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalPolicyImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalPolicyImpl.java?rev=1865092&r1=1865091&r2=1865092&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalPolicyImpl.java (original)
+++ jackrabbit/oak/trunk/oak-authorization-principalbased/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalPolicyImpl.java Wed Aug 14 08:20:34 2019
@@ -16,7 +16,6 @@
  */
 package org.apache.jackrabbit.oak.spi.security.authorization.principalbased.impl;
 
-import com.google.common.base.Objects;
 import com.google.common.base.Strings;
 import com.google.common.collect.Maps;
 import org.apache.jackrabbit.api.security.authorization.PrincipalAccessControlList;
@@ -24,8 +23,8 @@ import org.apache.jackrabbit.api.securit
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.namepath.NamePathMapper;
 import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
-import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.ACE;
 import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AbstractAccessControlList;
 import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.Restriction;
@@ -260,26 +259,10 @@ class PrincipalPolicyImpl extends Abstra
 
     //--------------------------------------------------------------< Entry >---
 
-    final class EntryImpl extends ACE implements Entry {
-
-        private final String oakPath;
-
-        private int hashCode;
+    final class EntryImpl extends AbstractEntry {
 
         private EntryImpl(@Nullable String oakPath, @NotNull PrivilegeBits privilegeBits, @NotNull  Set<Restriction> restrictions) throws AccessControlException {
-            super(principal, privilegeBits, true, restrictions, getNamePathMapper());
-            this.oakPath = oakPath;
-        }
-
-        @Nullable
-        String getOakPath() {
-            return oakPath;
-        }
-
-        @Override
-        @Nullable
-        public String getEffectivePath() {
-            return (oakPath == null) ? null : getNamePathMapper().getJcrPath(oakPath);
+            super(oakPath, principal, privilegeBits, restrictions, PrincipalPolicyImpl.this.getNamePathMapper());
         }
 
         @Override
@@ -289,27 +272,8 @@ class PrincipalPolicyImpl extends Abstra
         }
 
         @Override
-        public int hashCode() {
-            if (hashCode == 0) {
-                hashCode = Objects.hashCode(oakPath, principal.getName(), getPrivilegeBits(), Boolean.TRUE, getRestrictions());
-            }
-            return hashCode;
-        }
-
-        @Override
-        public boolean equals(Object obj) {
-            if (obj == this) {
-                return true;
-            }
-            if (obj instanceof EntryImpl) {
-                EntryImpl other = (EntryImpl) obj;
-                return equivalentPath(other.oakPath) && super.equals(obj);
-            }
-            return false;
-        }
-
-        private boolean equivalentPath(@Nullable String otherPath) {
-            return (oakPath == null) ? otherPath == null : oakPath.equals(otherPath);
+        @NotNull NamePathMapper getNamePathMapper() {
+            return PrincipalPolicyImpl.this.getNamePathMapper();
         }
     }
 }

Added: jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/AbstractEntryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/AbstractEntryTest.java?rev=1865092&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/AbstractEntryTest.java (added)
+++ jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/AbstractEntryTest.java Wed Aug 14 08:20:34 2019
@@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.spi.security.authorization.principalbased.impl;
+
+import com.google.common.collect.ImmutableSet;
+import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.security.authorization.restriction.RestrictionProviderImpl;
+import org.apache.jackrabbit.oak.spi.security.authorization.AuthorizationConfiguration;
+import org.apache.jackrabbit.oak.spi.security.authorization.restriction.Restriction;
+import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBits;
+import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBitsProvider;
+import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
+import javax.jcr.ValueFactory;
+import javax.jcr.security.AccessControlException;
+import javax.jcr.security.Privilege;
+import java.security.Principal;
+import java.util.Collections;
+import java.util.Set;
+
+import static org.apache.jackrabbit.JcrConstants.NT_UNSTRUCTURED;
+import static org.apache.jackrabbit.oak.spi.nodetype.NodeTypeConstants.NT_OAK_UNSTRUCTURED;
+import static org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants.NT_REP_POLICY;
+import static org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants.NT_REP_RESTRICTIONS;
+import static org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants.REP_GLOB;
+import static org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants.REP_NT_NAMES;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+public class AbstractEntryTest extends AbstractPrincipalBasedTest {
+
+    private PrivilegeBitsProvider bitsProvider;
+
+    private AbstractEntry entryA;
+    private AbstractEntry entryB;
+
+    private Restriction restriction;
+
+    @Before
+    public void before() throws Exception {
+        super.before();
+
+        this.bitsProvider = new PrivilegeBitsProvider(root);
+
+        ValueFactory vf = getValueFactory(root);
+        RestrictionProvider rp = new RestrictionProviderImpl();
+        Restriction r = rp.createRestriction(TEST_OAK_PATH, REP_NT_NAMES, vf.createValue(getNamePathMapper().getJcrName(NT_OAK_UNSTRUCTURED), PropertyType.NAME));
+
+        Principal principal = getTestSystemUser().getPrincipal();
+        entryA = new TestEntry(TEST_OAK_PATH, principal, bitsProvider.getBits(PrivilegeConstants.JCR_NODE_TYPE_MANAGEMENT, PrivilegeConstants.REP_WRITE), r);
+        entryB = new TestEntry(null, principal, bitsProvider.getBits(PrivilegeConstants.JCR_NODE_TYPE_DEFINITION_MANAGEMENT));
+
+        restriction = rp.createRestriction(entryA.getOakPath(), REP_GLOB, vf.createValue("*"));
+    }
+
+    @Test
+    public void testHashCode() throws Exception {
+        assertNotEquals(entryA.hashCode(), entryB.hashCode());
+
+        // same entry -> same hash
+        assertEquals(entryA.hashCode(), entryA.hashCode());
+
+        // equivalent entry -> same hash
+        assertEquals(entryA.hashCode(), new TestEntry(entryA).hashCode());
+        assertEquals(entryB.hashCode(), new TestEntry(entryB).hashCode());
+
+        // different restrictions -> different hash
+        AbstractEntry differentRestriction = new TestEntry(entryA.getOakPath(), entryA.getPrincipal(), entryA.getPrivilegeBits(), restriction);
+        assertNotEquals(entryA.hashCode(), differentRestriction.hashCode());
+
+        // different path -> different hash
+        AbstractEntry differentPath = new TestEntry(PathUtils.ROOT_PATH, entryA.getPrincipal(), entryA.getPrivilegeBits(), entryA.getRestrictions().toArray(new Restriction[0]));
+        assertNotEquals(entryA.hashCode(), differentPath.hashCode());
+
+        // different path -> different hash
+        AbstractEntry differentPrincipal = new TestEntry(entryB.getOakPath(), EveryonePrincipal.getInstance(), entryB.getPrivilegeBits(), entryB.getRestrictions().toArray(new Restriction[0]));
+        assertNotEquals(entryB.hashCode(), differentPath.hashCode());
+
+        // different path -> different hash
+        AbstractEntry differentPrivs = new TestEntry(entryB.getOakPath(), entryB.getPrincipal(), bitsProvider.getBits(PrivilegeConstants.JCR_READ), entryB.getRestrictions().toArray(new Restriction[0]));
+        assertNotEquals(entryB.hashCode(), differentPrivs.hashCode());
+    }
+
+    @Test
+    public void testEquals() throws Exception {
+        assertNotEquals(entryA, entryB);
+        assertNotEquals(entryB, entryA);
+
+        assertEquals(entryA, entryA);
+
+        // equivalent entry -> equals
+        assertEquals(entryA, new TestEntry(entryA));
+        assertEquals(entryB, new TestEntry(entryB));
+
+        // different restrictions -> different hash
+        AbstractEntry differentRestriction = new TestEntry(entryA.getOakPath(), entryA.getPrincipal(), entryA.getPrivilegeBits(), restriction);
+        assertNotEquals(entryA, differentRestriction);
+
+        // different path -> different hash
+        AbstractEntry differentPath = new TestEntry(PathUtils.ROOT_PATH, entryA.getPrincipal(), entryA.getPrivilegeBits(), entryA.getRestrictions().toArray(new Restriction[0]));
+        assertNotEquals(entryA, differentPath);
+
+        // different path -> different hash
+        AbstractEntry differentPrincipal = new TestEntry(entryB.getOakPath(), EveryonePrincipal.getInstance(), entryB.getPrivilegeBits(), entryB.getRestrictions().toArray(new Restriction[0]));
+        assertNotEquals(entryB, differentPath);
+
+        // different path -> different hash
+        AbstractEntry differentPrivs = new TestEntry(entryB.getOakPath(), entryB.getPrincipal(), bitsProvider.getBits(PrivilegeConstants.JCR_READ), entryB.getRestrictions().toArray(new Restriction[0]));
+        assertNotEquals(entryB, differentPrivs);
+    }
+
+    private final class TestEntry extends AbstractEntry {
+
+        TestEntry(@Nullable String oakPath, @NotNull Principal principal, @NotNull PrivilegeBits privilegeBits, @NotNull Restriction... restrictions) throws AccessControlException {
+            super(oakPath, principal, privilegeBits, ImmutableSet.copyOf(restrictions), AbstractEntryTest.this.getNamePathMapper());
+        }
+
+        TestEntry(@NotNull AbstractEntry base) throws AccessControlException {
+            super(base.getOakPath(), base.getPrincipal(), base.getPrivilegeBits(), base.getRestrictions(), AbstractEntryTest.this.getNamePathMapper());
+        }
+
+        @Override
+        @NotNull NamePathMapper getNamePathMapper() {
+            return AbstractEntryTest.this.getNamePathMapper();
+        }
+
+        @Override
+        public Privilege[] getPrivileges() {
+            try {
+                return privilegesFromNames(bitsProvider.getPrivilegeNames(getPrivilegeBits()));
+            } catch (RepositoryException e) {
+                throw new RuntimeException(e);
+            }
+        }
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/AbstractEntryTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/EffectivePolicyTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/EffectivePolicyTest.java?rev=1865092&r1=1865091&r2=1865092&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/EffectivePolicyTest.java (original)
+++ jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/EffectivePolicyTest.java Wed Aug 14 08:20:34 2019
@@ -18,13 +18,15 @@ package org.apache.jackrabbit.oak.spi.se
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlEntry;
 import org.apache.jackrabbit.api.security.authorization.PrincipalAccessControlList;
+import org.apache.jackrabbit.api.security.principal.PrincipalManager;
 import org.apache.jackrabbit.oak.commons.PathUtils;
-import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.ACE;
-import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.ImmutableACL;
+import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
+import org.apache.jackrabbit.util.Text;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -34,9 +36,11 @@ import javax.jcr.security.AccessControlP
 import java.security.Principal;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import static org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants.REP_GLOB;
 import static org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants.REP_NT_NAMES;
+import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.JCR_LIFECYCLE_MANAGEMENT;
 import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.JCR_NAMESPACE_MANAGEMENT;
 import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.JCR_READ;
 import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.REP_WRITE;
@@ -44,6 +48,8 @@ import static org.junit.Assert.assertArr
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class EffectivePolicyTest extends AbstractPrincipalBasedTest {
 
@@ -65,9 +71,15 @@ public class EffectivePolicyTest extends
         acMgr = createAccessControlManager(root);
         validPrincipal = getTestSystemUser().getPrincipal();
 
+        // create 2 entries for 'validPrincipal'
+        // - jcrEffectivePath : read, write
+        // - null : namespaceMgt
         PrincipalPolicyImpl policy = setupPrincipalBasedAccessControl(validPrincipal, jcrEffectivePath, JCR_READ, REP_WRITE);
         addPrincipalBasedEntry(policy, null, JCR_NAMESPACE_MANAGEMENT);
 
+        // create 2 entries for 'validPrincipal2'
+        // - jcrEffectivePath : read
+        // - root : lifecycleMgt
         policy = (PrincipalPolicyImpl) acMgr.getApplicablePolicies(validPrincipal2)[0];
         Map<String, Value> restrictions = ImmutableMap.of(getNamePathMapper().getJcrName(REP_GLOB), getValueFactory(root).createValue("/*/glob"));
         policy.addEntry(jcrEffectivePath, privilegesFromNames(JCR_READ), restrictions, ImmutableMap.of());
@@ -85,9 +97,9 @@ public class EffectivePolicyTest extends
     public void testEffectivePolicyByPrincipal() throws Exception {
         AccessControlPolicy[] effective = acMgr.getEffectivePolicies(ImmutableSet.of(validPrincipal));
         assertEquals(1, effective.length);
-        assertTrue(effective[0] instanceof ImmutableACL);
+        assertTrue(effective[0] instanceof ImmutablePrincipalPolicy);
 
-        List<JackrabbitAccessControlEntry> entries = ((ImmutableACL)effective[0]).getEntries();
+        List<JackrabbitAccessControlEntry> entries = ((ImmutablePrincipalPolicy)effective[0]).getEntries();
         assertEquals(2, entries.size());
 
         assertTrue(entries.get(0) instanceof PrincipalAccessControlList.Entry);
@@ -99,53 +111,78 @@ public class EffectivePolicyTest extends
     }
 
     @Test
+    public void testEffectivePolicyByPrincipal2() throws Exception {
+        AccessControlPolicy[] effective = acMgr.getEffectivePolicies(ImmutableSet.of(validPrincipal2));
+        assertEquals(1, effective.length);
+        assertTrue(effective[0] instanceof ImmutablePrincipalPolicy);
+
+        List<JackrabbitAccessControlEntry> entries = ((ImmutablePrincipalPolicy)effective[0]).getEntries();
+        assertEquals(2, entries.size());
+
+        assertTrue(entries.get(0) instanceof PrincipalAccessControlList.Entry);
+        assertEquals(validPrincipal2, entries.get(0).getPrincipal());
+        assertArrayEquals(privilegesFromNames(JCR_READ), entries.get(0).getPrivileges());
+        assertEquals(jcrEffectivePath, ((PrincipalAccessControlList.Entry) entries.get(0)).getEffectivePath());
+
+        assertEquals(validPrincipal2, entries.get(1).getPrincipal());
+        assertArrayEquals(privilegesFromNames(JCR_LIFECYCLE_MANAGEMENT), entries.get(1).getPrivileges());
+        assertEquals(PathUtils.ROOT_PATH, ((PrincipalAccessControlList.Entry) entries.get(1)).getEffectivePath());
+    }
+
+    @Test
     public void testEffectivePolicyByPath() throws Exception {
-        AccessControlPolicy[] effective = acMgr.getEffectivePolicies(getNamePathMapper().getJcrPath(TEST_OAK_PATH));
+        String path = getNamePathMapper().getJcrPath(TEST_OAK_PATH);
+        AccessControlPolicy[] effective = acMgr.getEffectivePolicies(path);
         assertEquals(2, effective.length);
 
-        for (AccessControlPolicy effectivePolicy : effective) {
-            assertTrue(effectivePolicy instanceof ImmutableACL);
-
-            ImmutableACL acl = (ImmutableACL) effectivePolicy;
-            if (jcrEffectivePath.equals(acl.getPath())) {
-                List<JackrabbitAccessControlEntry> entries = acl.getEntries();
-                assertEquals(2, entries.size());
-
-                for (JackrabbitAccessControlEntry entry : entries) {
-                    if (validPrincipal.equals(entry.getPrincipal())) {
-                        assertArrayEquals(privilegesFromNames(JCR_READ, REP_WRITE), entry.getPrivileges());
-                        assertEquals(0, entry.getRestrictionNames().length);
-                    } else {
-                        assertEquals(validPrincipal2, entry.getPrincipal());
-                        assertArrayEquals(privilegesFromNames(JCR_READ), entry.getPrivileges());
-                        assertArrayEquals(new String[] {getNamePathMapper().getJcrName(REP_GLOB)}, entry.getRestrictionNames());
-                    }
-                }
-            } else {
-                assertEquals(PathUtils.ROOT_PATH, acl.getPath());
-
-                List<JackrabbitAccessControlEntry> entries = acl.getEntries();
-                assertEquals(1, entries.size());
-
-                JackrabbitAccessControlEntry entry = entries.get(0);
-                assertTrue(entry instanceof ACE);
-                assertArrayEquals(privilegesFromNames(PrivilegeConstants.JCR_LIFECYCLE_MANAGEMENT), entry.getPrivileges());
-                assertEquals(1, ((ACE) entry).getRestrictions().size());
-                assertArrayEquals(new String[] {getNamePathMapper().getJcrName(REP_NT_NAMES)}, entry.getRestrictionNames());
+        for (AccessControlPolicy policy : effective) {
+            assertTrue(policy instanceof ImmutablePrincipalPolicy);
+            ImmutablePrincipalPolicy effectivePolicy = (ImmutablePrincipalPolicy) policy;
+
+            // filter expected entries: only entries that take effect at the target path should be taken into consideration
+            ImmutablePrincipalPolicy byPrincipal = (ImmutablePrincipalPolicy) acMgr.getEffectivePolicies(ImmutableSet.of(effectivePolicy.getPrincipal()))[0];
+            Set<JackrabbitAccessControlEntry> expected = ImmutableSet.copyOf(Iterables.filter(byPrincipal.getEntries(), entry -> {
+                String effectivePath = ((PrincipalAccessControlList.Entry) entry).getEffectivePath();
+                return effectivePath != null && Text.isDescendantOrEqual(effectivePath, path);
+            }));
+
+            assertEquals(expected.size(), effectivePolicy.size());
+            List<JackrabbitAccessControlEntry> entries = effectivePolicy.getEntries();
+            for (JackrabbitAccessControlEntry entry : expected) {
+                assertTrue(entries.contains(entry));
             }
         }
     }
 
     @Test
+    public void testEffectivePolicyByPathVerifiesPrincipals() throws Exception {
+        PrincipalManager principalMgr = mock(PrincipalManager.class);
+        when(principalMgr.getPrincipal(validPrincipal.getName())).thenReturn(null);
+        when(principalMgr.getPrincipal(validPrincipal2.getName())).thenReturn(new PrincipalImpl(validPrincipal2.getName()));
+
+        MgrProvider provider = mock(MgrProvider.class);
+        when(provider.getPrincipalManager()).thenReturn(principalMgr);
+        when(provider.getRoot()).thenReturn(root);
+        when(provider.getSecurityProvider()).thenReturn(securityProvider);
+        when(provider.getNamePathMapper()).thenReturn(getNamePathMapper());
+
+        PrincipalBasedAccessControlManager acm = new PrincipalBasedAccessControlManager(provider, getFilterProvider());
+        AccessControlPolicy[] effective = acm.getEffectivePolicies(getNamePathMapper().getJcrPath(TEST_OAK_PATH));
+        assertEquals(0, effective.length);
+    }
+
+    @Test
     public void testEffectivePolicyByNullPath() throws Exception {
         AccessControlPolicy[] effective = acMgr.getEffectivePolicies((String) null);
         assertEquals(1, effective.length);
-        assertTrue(effective[0] instanceof ImmutableACL);
+        assertTrue(effective[0] instanceof ImmutablePrincipalPolicy);
+        assertEquals(validPrincipal, ((ImmutablePrincipalPolicy)effective[0]).getPrincipal());
 
-        List<JackrabbitAccessControlEntry> entries = ((ImmutableACL)effective[0]).getEntries();
+        List<JackrabbitAccessControlEntry> entries = ((ImmutablePrincipalPolicy)effective[0]).getEntries();
         assertEquals(1, entries.size());
 
-        assertTrue(entries.get(0) instanceof ACE);
+        assertTrue(entries.get(0) instanceof PrincipalAccessControlList.Entry);
+        assertNull(((PrincipalAccessControlList.Entry)entries.get(0)).getEffectivePath());
         assertEquals(validPrincipal, entries.get(0).getPrincipal());
         assertArrayEquals(privilegesFromNames(JCR_NAMESPACE_MANAGEMENT), entries.get(0).getPrivileges());
     }

Added: jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ImmutablePrincipalPolicyTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ImmutablePrincipalPolicyTest.java?rev=1865092&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ImmutablePrincipalPolicyTest.java (added)
+++ jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ImmutablePrincipalPolicyTest.java Wed Aug 14 08:20:34 2019
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.spi.security.authorization.principalbased.impl;
+
+import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.ImmutableACL;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.jcr.security.AccessControlException;
+import java.util.Collections;
+
+import static org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants.REP_GLOB;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+public class ImmutablePrincipalPolicyTest extends AbstractPrincipalBasedTest {
+
+    private PrincipalPolicyImpl policy;
+    private ImmutablePrincipalPolicy immutable;
+
+    @Before
+    @Override
+    public void before() throws Exception {
+        super.before();
+
+        policy = setupPrincipalBasedAccessControl(getTestSystemUser().getPrincipal(), testContentJcrPath, PrivilegeConstants.JCR_READ);
+        immutable = new ImmutablePrincipalPolicy(policy);
+    }
+
+    @Test
+    public void testGetPrincipal() throws Exception {
+        assertEquals(getTestSystemUser().getPrincipal(), immutable.getPrincipal());
+    }
+
+    @Test(expected = AccessControlException.class)
+    public void testAddEntry() throws Exception {
+        immutable.addEntry(PathUtils.ROOT_PATH, privilegesFromNames(PrivilegeConstants.JCR_READ));
+    }
+
+    @Test(expected = AccessControlException.class)
+    public void testAddEntryWithRestrictions() throws Exception {
+        immutable.addEntry(null, privilegesFromNames(PrivilegeConstants.JCR_ALL), Collections.singletonMap(getNamePathMapper().getJcrName(REP_GLOB), getValueFactory(root).createValue("*")), Collections.emptyMap());
+    }
+
+    @Test
+    public void testHashcode() {
+        int expectedHashCode = immutable.hashCode();
+        ImmutablePrincipalPolicy ipp = new ImmutablePrincipalPolicy(policy.getPrincipal(), policy.getOakPath(), policy.getEntries(), policy.getRestrictionProvider(), policy.getNamePathMapper());
+        assertEquals(expectedHashCode, ipp.hashCode());
+        assertEquals(expectedHashCode, new ImmutablePrincipalPolicy(policy).hashCode());
+    }
+
+    @Test
+    public void testEquals() {
+        ImmutablePrincipalPolicy ipp = new ImmutablePrincipalPolicy(policy.getPrincipal(), policy.getOakPath(), policy.getEntries(), policy.getRestrictionProvider(), policy.getNamePathMapper());
+        assertEquals(immutable, ipp);
+        assertEquals(immutable, new ImmutablePrincipalPolicy(policy));
+        assertEquals(immutable, immutable);
+    }
+
+    @Test
+    public void testNotEquals() {
+        ImmutablePrincipalPolicy differentPath = new ImmutablePrincipalPolicy(policy.getPrincipal(), "/different/path", policy.getEntries(), policy.getRestrictionProvider(), policy.getNamePathMapper());
+        ImmutablePrincipalPolicy differentPrincipal = new ImmutablePrincipalPolicy(EveryonePrincipal.getInstance(), policy.getOakPath(), policy.getEntries(), policy.getRestrictionProvider(), policy.getNamePathMapper());
+        ImmutablePrincipalPolicy differentEntries = new ImmutablePrincipalPolicy(policy.getPrincipal(), policy.getOakPath(), Collections.emptyList(), policy.getRestrictionProvider(), policy.getNamePathMapper());
+
+        assertNotEquals(immutable, policy);
+        assertNotEquals(immutable, new ImmutableACL(policy));
+        assertNotEquals(immutable, differentPath);
+        assertNotEquals(immutable, differentPrincipal);
+        assertNotEquals(immutable, differentEntries);
+
+        int hc = immutable.hashCode();
+        assertNotEquals(hc, policy.hashCode());
+        assertNotEquals(hc, new ImmutableACL(policy).hashCode());
+        assertNotEquals(hc, differentPath.hashCode());
+        assertNotEquals(hc, differentPrincipal.hashCode());
+        assertNotEquals(hc, differentEntries.hashCode());
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/ImmutablePrincipalPolicyTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalBasedAccessControlManagerTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalBasedAccessControlManagerTest.java?rev=1865092&r1=1865091&r2=1865092&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalBasedAccessControlManagerTest.java (original)
+++ jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalBasedAccessControlManagerTest.java Wed Aug 14 08:20:34 2019
@@ -26,7 +26,6 @@ import org.apache.jackrabbit.oak.api.Tre
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
 import org.apache.jackrabbit.oak.spi.nodetype.NodeTypeConstants;
-import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.ImmutableACL;
 import org.apache.jackrabbit.oak.spi.security.authorization.principalbased.FilterProvider;
 import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
 import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
@@ -87,6 +86,12 @@ public class PrincipalBasedAccessControl
         return new PrincipalPolicyImpl(validPrincipal, oakPath, getMgrProvider(root));
     }
 
+    private static void assertEffectivePolicy(@NotNull AccessControlPolicy[] effective, int size) {
+        assertEquals(1, effective.length);
+        assertTrue(effective[0] instanceof ImmutablePrincipalPolicy);
+        assertEquals(size, ((ImmutablePrincipalPolicy) effective[0]).size());
+    }
+
     @Test(expected = AccessControlException.class)
     public void testGetApplicablePoliciesNullPrincipal() throws Exception {
         acMgr.getApplicablePolicies((Principal) null);
@@ -171,7 +176,7 @@ public class PrincipalBasedAccessControl
         // after commit => effective policy present
         root.commit();
         effective = acMgr.getEffectivePolicies(ImmutableSet.of(validPrincipal));
-        assertEquals(1, effective.length);
+        assertEffectivePolicy(effective, 1);
     }
 
     @Test
@@ -236,7 +241,7 @@ public class PrincipalBasedAccessControl
         setupPrincipalBasedAccessControl(validPrincipal, testContentJcrPath, REP_WRITE);
         root.commit();
 
-        ImmutableACL effective = (ImmutableACL) acMgr.getEffectivePolicies(ImmutableSet.of(validPrincipal))[0];
+        ImmutablePrincipalPolicy effective = (ImmutablePrincipalPolicy) acMgr.getEffectivePolicies(ImmutableSet.of(validPrincipal))[0];
         acMgr.setPolicy(effective.getPath(), effective);
     }
 
@@ -334,7 +339,7 @@ public class PrincipalBasedAccessControl
         setupPrincipalBasedAccessControl(validPrincipal, testContentJcrPath, REP_WRITE);
         root.commit();
 
-        ImmutableACL effective = (ImmutableACL) acMgr.getEffectivePolicies(ImmutableSet.of(validPrincipal))[0];
+        ImmutablePrincipalPolicy effective = (ImmutablePrincipalPolicy) acMgr.getEffectivePolicies(ImmutableSet.of(validPrincipal))[0];
         acMgr.removePolicy(effective.getPath(), effective);
     }
 
@@ -440,9 +445,9 @@ public class PrincipalBasedAccessControl
         addPrincipalBasedEntry(policy, PathUtils.ROOT_PATH, JCR_READ);
         root.commit();
 
-        assertEquals(2, acMgr.getEffectivePolicies(testJcrPath).length);
-        assertEquals(2, acMgr.getEffectivePolicies(testContentJcrPath).length);
-        assertEquals(1, acMgr.getEffectivePolicies(PathUtils.ROOT_PATH).length);
+        assertEffectivePolicy(acMgr.getEffectivePolicies(testJcrPath), 2);
+        assertEffectivePolicy(acMgr.getEffectivePolicies(testContentJcrPath), 2);
+        assertEffectivePolicy(acMgr.getEffectivePolicies(PathUtils.ROOT_PATH), 1);
     }
 
     @Test

Modified: jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalPolicyImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalPolicyImplTest.java?rev=1865092&r1=1865091&r2=1865092&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalPolicyImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-authorization-principalbased/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/principalbased/impl/PrincipalPolicyImplTest.java Wed Aug 14 08:20:34 2019
@@ -67,7 +67,6 @@ import static org.apache.jackrabbit.oak.
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
@@ -584,44 +583,6 @@ public class PrincipalPolicyImplTest ext
         assertTrue(entry.isAllow());
     }
 
-    @Test
-    public void testEntryHashCode() throws Exception {
-        PrincipalPolicyImpl.EntryImpl entryA = policy.getEntries().get(0);
-        PrincipalPolicyImpl.EntryImpl entryB = policy.getEntries().get(1);
-        assertNotEquals(entryA.hashCode(), entryB.hashCode());
-
-        // same entry -> same hash
-        assertEquals(entryA.hashCode(), policy.getEntries().get(0).hashCode());
-
-        // equivalent entry on different policy -> same hash
-        emptyPolicy.addEntry(entryB.getEffectivePath(), entryB.getPrivileges(), Collections.emptyMap(), Collections.emptyMap());
-        assertEquals(entryB.hashCode(), emptyPolicy.getEntries().get(0).hashCode());
-
-        // different restrictions -> different hash
-        emptyPolicy.addEntry(entryA.getEffectivePath(), entryA.getPrivileges(), createGlobRestriction("*"), Collections.emptyMap());
-        assertNotEquals(entryA.hashCode(), emptyPolicy.getEntries().get(1).hashCode());
-    }
-
-    @Test
-    public void testEntryEquals() throws Exception {
-        PrincipalPolicyImpl.EntryImpl entryA = policy.getEntries().get(0);
-        PrincipalPolicyImpl.EntryImpl entryB = policy.getEntries().get(1);
-        assertNotEquals(entryA, entryB);
-        assertNotEquals(entryB, entryA);
-
-        assertEquals(entryA, entryA);
-        assertEquals(entryA, policy.getEntries().get(0));
-
-        // equivalent entry on different policy -> same hash
-        emptyPolicy.addEntry(entryB.getEffectivePath(), entryB.getPrivileges(), Collections.emptyMap(), Collections.emptyMap());
-        assertEquals(entryB, emptyPolicy.getEntries().get(0));
-
-        // different restrictions -> different hash
-        emptyPolicy.addEntry(entryA.getEffectivePath(), entryA.getPrivileges(), createGlobRestriction("*"), Collections.emptyMap());
-        assertNotEquals(entryA, emptyPolicy.getEntries().get(1));
-    }
-
-
     private static PrincipalAccessControlList.Entry invalidEntry(@NotNull PrincipalAccessControlList.Entry entry) {
         return new PrincipalAccessControlList.Entry() {
             @Override