Author: angela
Date: Thu Nov 7 17:34:16 2013
New Revision: 1539730
URL: http://svn.apache.org/r1539730
Log:
OAK-615 : UserValidator should check for cyclic group membership
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableBaseProvider.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidator.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidatorProvider.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserValidatorTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableBaseProvider.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableBaseProvider.java?rev=1539730&r1=1539729&r2=1539730&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableBaseProvider.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableBaseProvider.java
Thu Nov 7 17:34:16 2013
@@ -48,7 +48,12 @@ abstract class AuthorizableBaseProvider
@CheckForNull
Tree getByID(@Nonnull String authorizableId, @Nonnull AuthorizableType
authorizableType) {
- Tree tree = identifierManager.getTree(getContentID(authorizableId));
+ return getByContentID(getContentID(authorizableId), authorizableType);
+ }
+
+ @CheckForNull
+ Tree getByContentID(@Nonnull String contentId, @Nonnull AuthorizableType
authorizableType) {
+ Tree tree = identifierManager.getTree(contentId);
if (UserUtil.isType(tree, authorizableType)) {
return tree;
} else {
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidator.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidator.java?rev=1539730&r1=1539729&r2=1539730&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidator.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidator.java
Thu Nov 7 17:34:16 2013
@@ -16,9 +16,12 @@
*/
package org.apache.jackrabbit.oak.security.user;
+import java.util.Set;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import org.apache.jackrabbit.JcrConstants;
import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.api.PropertyState;
@@ -79,7 +82,7 @@ class UserValidator extends DefaultValid
}
if (REP_MEMBERS.equals(name)) {
- checkForCyclicMembership(after);
+ checkForCyclicMembership(after.getValue(Type.STRINGS));
}
}
@@ -107,7 +110,9 @@ class UserValidator extends DefaultValid
}
if (REP_MEMBERS.equals(name)) {
- checkForCyclicMembership(after);
+ Set<String> afterValues =
Sets.newHashSet(after.getValue(Type.STRINGS));
+
afterValues.removeAll(ImmutableSet.copyOf(before.getValue(Type.STRINGS)));
+ checkForCyclicMembership(afterValues);
}
}
@@ -173,8 +178,20 @@ class UserValidator extends DefaultValid
}
}
- private void checkForCyclicMembership(PropertyState repMembers) {
- // FIXME OAK-615
+ private void checkForCyclicMembership(@Nonnull Iterable<String>
memberRefs) throws CommitFailedException {
+ String groupContentId = TreeUtil.getString(parentAfter,
JcrConstants.JCR_UUID);
+ if (groupContentId == null) {
+ throw constraintViolation(30, "Missing content id for group " +
UserUtil.getAuthorizableId(parentAfter) + "; cannot check for cyclic group
membership.");
+ }
+ MembershipProvider mp = provider.getMembershipProvider();
+ for (String memberContentId : memberRefs) {
+ Tree memberTree = mp.getByContentID(memberContentId,
AuthorizableType.GROUP);
+ if (memberTree != null) {
+ if (mp.isCyclicMembership(memberTree, groupContentId)) {
+ throw constraintViolation(31, "Cyclic group membership
detected in group" + UserUtil.getAuthorizableId(parentAfter));
+ }
+ }
+ }
}
private static boolean isValidUUID(@Nonnull Tree parent, @Nonnull String
uuid) {
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidatorProvider.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidatorProvider.java?rev=1539730&r1=1539729&r2=1539730&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidatorProvider.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidatorProvider.java
Thu Nov 7 17:34:16 2013
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.oak.securi
import javax.annotation.Nonnull;
+import org.apache.jackrabbit.oak.core.ImmutableRoot;
import org.apache.jackrabbit.oak.core.ImmutableTree;
import org.apache.jackrabbit.oak.spi.commit.Validator;
import org.apache.jackrabbit.oak.spi.commit.ValidatorProvider;
@@ -33,6 +34,8 @@ class UserValidatorProvider extends Vali
private final ConfigurationParameters config;
+ private MembershipProvider membershipProvider;
+
UserValidatorProvider(ConfigurationParameters config) {
this.config = checkNotNull(config);
}
@@ -41,6 +44,7 @@ class UserValidatorProvider extends Vali
@Nonnull
@Override
public Validator getRootValidator(NodeState before, NodeState after) {
+ membershipProvider = new MembershipProvider(new ImmutableRoot(after),
config);
return new UserValidator(new ImmutableTree(before), new
ImmutableTree(after), this);
}
@@ -49,4 +53,9 @@ class UserValidatorProvider extends Vali
ConfigurationParameters getConfig() {
return config;
}
+
+ @Nonnull
+ MembershipProvider getMembershipProvider() {
+ return membershipProvider;
+ }
}
\ No newline at end of file
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserValidatorTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserValidatorTest.java?rev=1539730&r1=1539729&r2=1539730&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserValidatorTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserValidatorTest.java
Thu Nov 7 17:34:16 2013
@@ -38,7 +38,6 @@ import org.apache.jackrabbit.oak.spi.sec
import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
import org.apache.jackrabbit.util.Text;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import static org.junit.Assert.assertFalse;
@@ -297,7 +296,6 @@ public class UserValidatorTest extends A
* @since oak 1.0 cyclic group membership added in a single set of
transient
* modifications must be detected upon save.
*/
- @Ignore("OAK-615")
@Test
public void testDetectCyclicMembership() throws Exception {
Group group1 = null;
@@ -324,7 +322,6 @@ public class UserValidatorTest extends A
} catch (CommitFailedException e) {
// success
} finally {
- root.refresh();
if (group1 != null) group1.remove();
if (group2 != null) group2.remove();
if (group3 != null) group3.remove();
