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/09/18 09:02:56 UTC
svn commit: r1867106 - in /jackrabbit/oak/trunk/oak-authorization-cug/src:
main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/
test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/
Author: angela
Date: Wed Sep 18 09:02:56 2019
New Revision: 1867106
URL: http://svn.apache.org/viewvc?rev=1867106&view=rev
Log:
OAK-8632 : rep:CugPolicy nodes containing duplicate principal names
Modified:
jackrabbit/oak/trunk/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugAccessControlManager.java
jackrabbit/oak/trunk/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImpl.java
jackrabbit/oak/trunk/oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImplTest.java
Modified: jackrabbit/oak/trunk/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugAccessControlManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugAccessControlManager.java?rev=1867106&r1=1867105&r2=1867106&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugAccessControlManager.java (original)
+++ jackrabbit/oak/trunk/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugAccessControlManager.java Wed Sep 18 09:02:56 2019
@@ -27,6 +27,7 @@ import javax.jcr.security.AccessControlP
import javax.jcr.security.AccessControlPolicyIterator;
import javax.jcr.security.Privilege;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
@@ -259,12 +260,12 @@ class CugAccessControlManager extends Ab
}
}
- private Set<Principal> getPrincipals(@NotNull Tree cugTree) {
+ private Iterable<Principal> getPrincipals(@NotNull Tree cugTree) {
PropertyState property = cugTree.getProperty(REP_PRINCIPAL_NAMES);
if (property == null) {
return Collections.emptySet();
} else {
- return ImmutableSet.copyOf(Iterables.transform(property.getValue(Type.STRINGS), principalName -> {
+ return ImmutableList.copyOf(Iterables.transform(property.getValue(Type.STRINGS), principalName -> {
Principal principal = principalManager.getPrincipal(principalName);
if (principal == null) {
log.debug("Unknown principal {}", principalName);
Modified: jackrabbit/oak/trunk/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImpl.java?rev=1867106&r1=1867105&r2=1867106&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImpl.java (original)
+++ jackrabbit/oak/trunk/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImpl.java Wed Sep 18 09:02:56 2019
@@ -16,14 +16,7 @@
*/
package org.apache.jackrabbit.oak.spi.security.authorization.cug.impl;
-import java.security.Principal;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Set;
-import javax.jcr.security.AccessControlException;
-
import com.google.common.base.Strings;
-import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import org.apache.jackrabbit.api.security.principal.PrincipalManager;
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
@@ -35,6 +28,13 @@ import org.jetbrains.annotations.Nullabl
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import javax.jcr.security.AccessControlException;
+import java.security.Principal;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Set;
+
/**
* Implementation of the {@link org.apache.jackrabbit.oak.spi.security.authorization.cug.CugPolicy}
* interface that respects the configured {@link org.apache.jackrabbit.oak.spi.xml.ImportBehavior}.
@@ -49,7 +49,7 @@ class CugPolicyImpl implements CugPolicy
private final int importBehavior;
private final CugExclude cugExclude;
- private final Set<Principal> principals = new HashSet<>();
+ private final Map<String,Principal> principals = new LinkedHashMap<>();
CugPolicyImpl(@NotNull String oakPath, @NotNull NamePathMapper namePathMapper,
@NotNull PrincipalManager principalManager, int importBehavior, @NotNull CugExclude cugExclude) {
@@ -58,28 +58,31 @@ class CugPolicyImpl implements CugPolicy
CugPolicyImpl(@NotNull String oakPath, @NotNull NamePathMapper namePathMapper,
@NotNull PrincipalManager principalManager, int importBehavior,
- @NotNull CugExclude cugExclude, @NotNull Set<Principal> principals) {
+ @NotNull CugExclude cugExclude, @NotNull Iterable<Principal> principals) {
ImportBehavior.nameFromValue(importBehavior);
this.oakPath = oakPath;
this.namePathMapper = namePathMapper;
this.principalManager = principalManager;
this.importBehavior = importBehavior;
this.cugExclude = cugExclude;
- this.principals.addAll(principals);
+ for (Principal principal : principals) {
+ this.principals.put(principal.getName(), principal);
+ }
}
@NotNull
@Override
public Set<Principal> getPrincipals() {
- return Sets.newHashSet(principals);
+ return Sets.newHashSet(principals.values());
}
@Override
public boolean addPrincipals(@NotNull Principal... principals) throws AccessControlException {
boolean modified = false;
for (Principal principal : principals) {
- if (isValidPrincipal(principal)) {
- modified |= this.principals.add(principal);
+ if (isValidPrincipal(principal) && !this.principals.containsKey(principal.getName())) {
+ this.principals.put(principal.getName(), principal);
+ modified = true;
}
}
return modified;
@@ -89,8 +92,9 @@ class CugPolicyImpl implements CugPolicy
public boolean removePrincipals(@NotNull Principal... principals) {
boolean modified = false;
for (Principal principal : principals) {
- if (principal != null) {
- modified |= this.principals.remove(principal);
+ if (principal != null && this.principals.containsKey(principal.getName())) {
+ this.principals.remove(principal.getName());
+ modified = true;
}
}
return modified;
@@ -104,7 +108,7 @@ class CugPolicyImpl implements CugPolicy
//--------------------------------------------------------------------------
Iterable<String> getPrincipalNames() {
- return Iterables.transform(principals, Principal::getName);
+ return Sets.newHashSet(principals.keySet());
}
//--------------------------------------------------------------------------
Modified: jackrabbit/oak/trunk/oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImplTest.java?rev=1867106&r1=1867105&r2=1867106&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImplTest.java Wed Sep 18 09:02:56 2019
@@ -69,11 +69,11 @@ public class CugPolicyImplTest extends A
return new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, importBehavior, exclude);
}
- private CugPolicyImpl createCugPolicy(@NotNull Set<Principal> principals) {
+ private CugPolicyImpl createCugPolicy(@NotNull Iterable<Principal> principals) {
return createCugPolicy(ImportBehavior.ABORT, principals);
}
- private CugPolicyImpl createCugPolicy(int importBehavior, @NotNull Set<Principal> principals) {
+ private CugPolicyImpl createCugPolicy(int importBehavior, @NotNull Iterable<Principal> principals) {
return new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, importBehavior, exclude, principals);
}
@@ -104,6 +104,21 @@ public class CugPolicyImplTest extends A
}
@Test
+ public void testCreateWithDuplicateName() {
+ Set<Principal> duplication = ImmutableSet.of(testPrincipal, new Principal() {
+ @Override
+ public String getName() {
+ return testPrincipal.getName();
+ }
+ });
+ assertEquals(2, duplication.size());
+
+ CugPolicyImpl cugPolicy = createCugPolicy(duplication);
+ assertEquals(1, cugPolicy.getPrincipals().size());
+ assertEquals(1, Iterables.size(cugPolicy.getPrincipalNames()));
+ }
+
+ @Test
public void testGetPrincipalNames() {
CugPolicyImpl cug = createCugPolicy(principals);
@@ -173,6 +188,13 @@ public class CugPolicyImplTest extends A
}
@Test
+ public void testAddContainedPrincipalNonEqualImpl() throws Exception {
+ CugPolicy cug = createCugPolicy(ImportBehavior.BESTEFFORT, principals);
+ assertFalse(cug.addPrincipals((Principal) () -> testPrincipal.getName()));
+ assertEquals(principals, cug.getPrincipals());
+ }
+
+ @Test
public void testAddNullPrincipal() throws Exception {
CugPolicy cug = createCugPolicy(ImportBehavior.ABORT, principals);
assertTrue(cug.addPrincipals(EveryonePrincipal.getInstance(), null));
@@ -206,7 +228,13 @@ public class CugPolicyImplTest extends A
public void testRemoveNullPrincipal() throws Exception {
CugPolicy cug = createCugPolicy(ImportBehavior.ABORT, principals);
assertTrue(cug.removePrincipals(testPrincipal, null));
+ assertTrue(cug.getPrincipals().isEmpty());
+ }
+ @Test
+ public void testRemoveContainedPrincipalNotEqual() throws Exception {
+ CugPolicy cug = createCugPolicy(ImportBehavior.BESTEFFORT, principals);
+ assertTrue(cug.removePrincipals((Principal) () -> testPrincipal.getName()));
assertTrue(cug.getPrincipals().isEmpty());
}