Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java	(revision 1788086)
+++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java	(revision )
@@ -111,8 +111,11 @@
                 log.debug(msg);
                 return false;
             }
-            // NOTE: detection of circular membership is postponed to the commit (=> UserValidator)
+            if (isCyclicMembership((Group) authorizable)) {
+                String msg = "Cyclic group membership detected for group " + getID() + " and member " + authorizable.getID();
+                throw new ConstraintViolationException(msg);
-        }
+            }
+        }
 
         boolean success = getMembershipProvider().addMember(getTree(), authorizableImpl.getTree());
 
@@ -266,18 +269,24 @@
 
             if (ImportBehavior.BESTEFFORT != importBehavior) {
                 Authorizable member = getUserManager().getAuthorizable(memberId);
+                String msg = null;
                 if (member == null) {
+                    msg = "Attempt to add or remove a non-existing member '" + memberId + "' with ImportBehavior = " + ImportBehavior.nameFromValue(importBehavior);
+                } else if (member.isGroup()) {
+                    if (((AuthorizableImpl) member).isEveryone()) {
+                        msg = "Attempt to add everyone group as member.";
+                    } else if (isCyclicMembership((Group) member)) {
+                        msg = "Cyclic group membership detected for group " + getID() + " and member " + member.getID();
+                    }
+                }
+                if (msg != null) {
                     if (ImportBehavior.ABORT == importBehavior) {
-                        throw new ConstraintViolationException("Attempt to add or remove a non-existing member " + memberId);
+                        throw new ConstraintViolationException(msg);
                     } else {
                         // ImportBehavior.IGNORE is default in UserUtil.getImportBehavior
-                        String msg = "Attempt to add or remove non-existing member '" + getID() + "' with ImportBehavior = IGNORE.";
                         log.debug(msg);
                         continue;
                     }
-                } else if (member.isGroup() && ((AuthorizableImpl) member).isEveryone()) {
-                    log.debug("Attempt to add everyone group as member.");
-                    continue;
                 }
             }
 
@@ -300,6 +309,10 @@
 
         getUserManager().onGroupUpdate(this, isRemove, false, processedIds, failedIds);
         return failedIds;
+    }
+
+    private boolean isCyclicMembership(@Nonnull Group member) throws RepositoryException {
+        return member.isMember(this);
     }
 
     /**
\ No newline at end of file
Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipProvider.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipProvider.java	(revision 1788086)
+++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipProvider.java	(revision )
@@ -220,7 +220,7 @@
      */
     @Nonnull
     Iterator<String> getMembers(@Nonnull Tree groupTree, boolean includeInherited) {
-        return getMembers(groupTree, includeInherited, new HashSet<String>());
+        return getMembers(groupTree, getContentID(groupTree), includeInherited, new HashSet<String>());
     }
 
     /**
@@ -233,11 +233,16 @@
      */
     @Nonnull
     private Iterator<String> getMembers(@Nonnull final Tree groupTree,
+                                        @Nonnull final String groupContentId,
                                         final boolean includeInherited,
                                         @Nonnull final Set<String> processedRefs) {
         MemberReferenceIterator mrit = new MemberReferenceIterator(groupTree) {
             @Override
             protected boolean hasProcessedReference(@Nonnull String value) {
+                if (groupContentId.equals(value)) {
+                    log.warn("Cyclic group membership detected for contentId " + groupContentId);
+                    return false;
+                }
                 return processedRefs.add(value);
             }
         };
@@ -261,7 +266,7 @@
             @Nonnull
             @Override
             protected Iterator<String> getNextIterator(@Nonnull Tree groupTree) {
-                return getMembers(groupTree, true, processedRefs);
+                return getMembers(groupTree, groupContentId, true, processedRefs);
             }
         };
     }
\ No newline at end of file
Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidator.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidator.java	(revision 1788086)
+++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidator.java	(revision )
@@ -16,12 +16,9 @@
  */
 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;
@@ -80,11 +77,7 @@
             String msg = "Invalid jcr:uuid for authorizable " + parentAfter.getName();
             throw constraintViolation(21, msg);
         }
-
-        if (REP_MEMBERS.equals(name)) {
-            checkForCyclicMembership(after.getValue(Type.STRINGS));
-        }
+    }
-    }
 
     @Override
     public void propertyChanged(PropertyState before, PropertyState after) throws CommitFailedException {
@@ -112,13 +105,7 @@
             String msg = "Password may not be plain text.";
             throw constraintViolation(24, msg);
         }
-
-        if (REP_MEMBERS.equals(name)) {
-            Set<String> addedValues = Sets.newHashSet(after.getValue(Type.STRINGS));
-            addedValues.removeAll(ImmutableSet.copyOf(before.getValue(Type.STRINGS)));
-            checkForCyclicMembership(addedValues);
-        }
+    }
-    }
 
 
     @Override
@@ -180,20 +167,6 @@
             return UserUtil.getAdminId(provider.getConfig()).equals(id);
         } else {
             return false;
-        }
-    }
-
-    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 && mp.isMember(memberTree, parentAfter)) {
-                throw constraintViolation(31, "Cyclic group membership detected in group" + UserUtil.getAuthorizableId(parentAfter));
-            }
         }
     }
 
\ No newline at end of file