Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipWriter.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipWriter.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipWriter.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipWriter.java Wed Aug 17 14:21:07 2016 @@ -16,11 +16,16 @@ */ package org.apache.jackrabbit.oak.security.user; +import java.util.HashSet; import java.util.Iterator; +import java.util.Map; import java.util.Set; +import javax.annotation.Nonnull; import javax.jcr.RepositoryException; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import org.apache.jackrabbit.JcrConstants; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.Tree; @@ -60,23 +65,46 @@ public class MembershipWriter { * @throws RepositoryException if an error occurs */ boolean addMember(Tree groupTree, String memberContentId) throws RepositoryException { + Map<String, String> m = Maps.newHashMapWithExpectedSize(1); + m.put(memberContentId, "-"); + return addMembers(groupTree, m).isEmpty(); + } + + /** + * Adds a new member to the given {@code groupTree}. + * + * @param groupTree the group to add the member to + * @param memberIds the ids of the new members as map of 'contentId':'memberId' + * @return the set of member IDs that was not successfully processed. + * @throws RepositoryException if an error occurs + */ + Set<String> addMembers(@Nonnull Tree groupTree, @Nonnull Map<String, String> memberIds) throws RepositoryException { // check all possible rep:members properties for the new member and also find the one with the least values Tree membersList = groupTree.getChild(UserConstants.REP_MEMBERS_LIST); Iterator<Tree> trees = Iterators.concat( Iterators.singletonIterator(groupTree), membersList.getChildren().iterator() ); + + Set<String> failed = new HashSet<String>(memberIds.size()); int bestCount = membershipSizeThreshold; PropertyState bestProperty = null; Tree bestTree = null; - while (trees.hasNext()) { + + // remove existing memberIds from the map and find best-matching tree + // for the insertion of the new members. + while (trees.hasNext() && !memberIds.isEmpty()) { Tree t = trees.next(); PropertyState refs = t.getProperty(UserConstants.REP_MEMBERS); if (refs != null) { int numRefs = 0; for (String ref : refs.getValue(Type.WEAKREFERENCES)) { - if (ref.equals(memberContentId)) { - return false; + String id = memberIds.remove(ref); + if (id != null) { + failed.add(id); + if (memberIds.isEmpty()) { + break; + } } numRefs++; } @@ -88,35 +116,73 @@ public class MembershipWriter { } } - PropertyBuilder<String> propertyBuilder; - if (bestProperty == null) { - // we don't have a good candidate to store the new member. - // so there are no members at all or all are full - if (!groupTree.hasProperty(UserConstants.REP_MEMBERS)) { - bestTree = groupTree; - } else { - if (!membersList.exists()) { - membersList = groupTree.addChild(UserConstants.REP_MEMBERS_LIST); - membersList.setProperty(JcrConstants.JCR_PRIMARYTYPE, UserConstants.NT_REP_MEMBER_REFERENCES_LIST, NAME); - bestTree = membersList.addChild("0"); + // update member content structure by starting inserting new member IDs + // with the best-matching property and create new member-ref-nodes as needed. + if (!memberIds.isEmpty()) { + PropertyBuilder<String> propertyBuilder; + int propCnt; + if (bestProperty == null) { + // we don't have a good candidate to store the new members. + // so there are no members at all or all are full + if (!groupTree.hasProperty(UserConstants.REP_MEMBERS)) { + bestTree = groupTree; } else { - // keep node names linear - int i = 0; - String name = String.valueOf(i); - while (membersList.hasChild(name)) { - name = String.valueOf(++i); + bestTree = createMemberRefTree(groupTree, membersList); + } + propertyBuilder = PropertyBuilder.array(Type.WEAKREFERENCE, UserConstants.REP_MEMBERS); + propCnt = 0; + } else { + propertyBuilder = PropertyBuilder.copy(Type.WEAKREFERENCE, bestProperty); + propCnt = bestCount; + } + // if adding all new members to best-property would exceed the threshold + // the new ids need to be distributed to different member-ref-nodes + // for simplicity this is achieved by introducing new tree(s) + if ((propCnt + memberIds.size()) > membershipSizeThreshold) { + while (!memberIds.isEmpty()) { + Set s = new HashSet(); + Iterator it = memberIds.keySet().iterator(); + while (propCnt < membershipSizeThreshold && it.hasNext()) { + s.add(it.next()); + it.remove(); + propCnt++; + } + propertyBuilder.addValues(s); + bestTree.setProperty(propertyBuilder.getPropertyState()); + + if (it.hasNext()) { + // continue filling the next (new) node + propertyBuilder pair + propCnt = 0; + bestTree = createMemberRefTree(groupTree, membersList); + propertyBuilder = PropertyBuilder.array(Type.WEAKREFERENCE, UserConstants.REP_MEMBERS); } - bestTree = membersList.addChild(name); } - bestTree.setProperty(JcrConstants.JCR_PRIMARYTYPE, UserConstants.NT_REP_MEMBER_REFERENCES, NAME); + } else { + propertyBuilder.addValues(memberIds.keySet()); + bestTree.setProperty(propertyBuilder.getPropertyState()); } - propertyBuilder = PropertyBuilder.array(Type.WEAKREFERENCE, UserConstants.REP_MEMBERS); - } else { - propertyBuilder = PropertyBuilder.copy(Type.WEAKREFERENCE, bestProperty); } - propertyBuilder.addValue(memberContentId); - bestTree.setProperty(propertyBuilder.getPropertyState()); - return true; + return failed; + } + + private static Tree createMemberRefTree(@Nonnull Tree groupTree, @Nonnull Tree membersList) { + if (!membersList.exists()) { + membersList = groupTree.addChild(UserConstants.REP_MEMBERS_LIST); + membersList.setProperty(JcrConstants.JCR_PRIMARYTYPE, UserConstants.NT_REP_MEMBER_REFERENCES_LIST, NAME); + } + Tree refTree = membersList.addChild(nextRefNodeName(membersList)); + refTree.setProperty(JcrConstants.JCR_PRIMARYTYPE, UserConstants.NT_REP_MEMBER_REFERENCES, NAME); + return refTree; + } + + private static String nextRefNodeName(@Nonnull Tree membersList) { + // keep node names linear + int i = 0; + String name = String.valueOf(i); + while (membersList.hasChild(name)) { + name = String.valueOf(++i); + } + return name; } /** @@ -126,33 +192,50 @@ public class MembershipWriter { * @param memberContentId member to remove * @return {@code true} if the member was removed. */ - boolean removeMember(Tree groupTree, String memberContentId) { + boolean removeMember(@Nonnull Tree groupTree, @Nonnull String memberContentId) { + Map<String, String> m = Maps.newHashMapWithExpectedSize(1); + m.put(memberContentId, "-"); + return removeMembers(groupTree, m).isEmpty(); + } + + /** + * Removes the members from the given group. + * + * @param groupTree group to remove the member from + * @param memberIds Map of 'contentId':'memberId' of all members that need to be removed. + * @return the set of member IDs that was not successfully processed. + */ + Set<String> removeMembers(@Nonnull Tree groupTree, @Nonnull Map<String, String> memberIds) { Tree membersList = groupTree.getChild(UserConstants.REP_MEMBERS_LIST); Iterator<Tree> trees = Iterators.concat( Iterators.singletonIterator(groupTree), membersList.getChildren().iterator() ); - while (trees.hasNext()) { + while (trees.hasNext() && !memberIds.isEmpty()) { Tree t = trees.next(); PropertyState refs = t.getProperty(UserConstants.REP_MEMBERS); if (refs != null) { PropertyBuilder<String> prop = PropertyBuilder.copy(Type.WEAKREFERENCE, refs); - if (prop.hasValue(memberContentId)) { - prop.removeValue(memberContentId); - if (prop.isEmpty()) { - if (t == groupTree) { - t.removeProperty(UserConstants.REP_MEMBERS); + Iterator<Map.Entry<String,String>> memberEntries = memberIds.entrySet().iterator(); + while (memberEntries.hasNext()) { + String memberContentId = memberEntries.next().getKey(); + if (prop.hasValue(memberContentId)) { + prop.removeValue(memberContentId); + if (prop.isEmpty()) { + if (t == groupTree) { + t.removeProperty(UserConstants.REP_MEMBERS); + } else { + t.remove(); + } } else { - t.remove(); + t.setProperty(prop.getPropertyState()); } - } else { - t.setProperty(prop.getPropertyState()); + memberEntries.remove(); } - return true; } } } - return false; + return Sets.newHashSet(memberIds.values()); } /** @@ -161,7 +244,7 @@ public class MembershipWriter { * @param group node builder of group * @param members set of content ids to set */ - public void setMembers(NodeBuilder group, Set<String> members) { + public void setMembers(@Nonnull NodeBuilder group, @Nonnull Set<String> members) { group.removeProperty(UserConstants.REP_MEMBERS); if (group.hasChildNode(UserConstants.REP_MEMBERS)) { group.getChildNode(UserConstants.REP_MEMBERS).remove();
Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/SystemUserImpl.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/SystemUserImpl.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/SystemUserImpl.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/SystemUserImpl.java Wed Aug 17 14:21:07 2016 @@ -22,7 +22,6 @@ import javax.jcr.RepositoryException; import javax.jcr.UnsupportedRepositoryOperationException; import org.apache.jackrabbit.oak.api.Tree; -import org.apache.jackrabbit.oak.spi.security.principal.SystemUserPrincipal; import org.apache.jackrabbit.oak.spi.security.user.util.UserUtil; /** @@ -47,7 +46,7 @@ public class SystemUserImpl extends User if (isAdmin()) { return new AdminPrincipalImpl(getPrincipalName(), getTree(), getUserManager().getNamePathMapper()); } else { - return new SystemUserPrincipalImpl(getTree()); + return new SystemUserPrincipalImpl(getPrincipalName(), getTree(), getUserManager().getNamePathMapper()); } } @@ -66,12 +65,4 @@ public class SystemUserImpl extends User public void changePassword(String password, String oldPassword) throws RepositoryException { throw new UnsupportedRepositoryOperationException("system user"); } - - //-------------------------------------------------------------------------- - private final class SystemUserPrincipalImpl extends TreeBasedPrincipal implements SystemUserPrincipal { - - private SystemUserPrincipalImpl(Tree tree) throws RepositoryException { - super(getPrincipalName(), tree, getUserManager().getNamePathMapper()); - } - } } \ No newline at end of file Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java Wed Aug 17 14:21:07 2016 @@ -23,6 +23,7 @@ import java.util.Map; import java.util.Set; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import org.apache.felix.scr.annotations.Activate; import org.apache.felix.scr.annotations.Component; @@ -42,6 +43,7 @@ import org.apache.jackrabbit.oak.spi.sec import org.apache.jackrabbit.oak.spi.security.Context; import org.apache.jackrabbit.oak.spi.security.SecurityConfiguration; import org.apache.jackrabbit.oak.spi.security.SecurityProvider; +import org.apache.jackrabbit.oak.spi.security.principal.PrincipalProvider; import org.apache.jackrabbit.oak.spi.security.user.UserAuthenticationFactory; import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration; import org.apache.jackrabbit.oak.spi.security.user.UserConstants; @@ -182,4 +184,10 @@ public class UserConfigurationImpl exten return umgr; } } + + @Nullable + @Override + public PrincipalProvider getUserPrincipalProvider(@Nonnull Root root, @Nonnull NamePathMapper namePathMapper) { + return new UserPrincipalProvider(root, this, namePathMapper); + } } Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java Wed Aug 17 14:21:07 2016 @@ -35,6 +35,9 @@ import javax.jcr.Session; import javax.jcr.nodetype.ConstraintViolationException; import javax.jcr.nodetype.PropertyDefinition; +import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import org.apache.jackrabbit.api.JackrabbitSession; import org.apache.jackrabbit.api.security.principal.PrincipalIterator; import org.apache.jackrabbit.api.security.principal.PrincipalManager; @@ -54,6 +57,7 @@ import org.apache.jackrabbit.oak.spi.sec import org.apache.jackrabbit.oak.spi.security.SecurityProvider; import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl; import org.apache.jackrabbit.oak.spi.security.user.UserConstants; +import org.apache.jackrabbit.oak.spi.security.user.util.UserUtil; import org.apache.jackrabbit.oak.spi.xml.ImportBehavior; import org.apache.jackrabbit.oak.spi.xml.NodeInfo; import org.apache.jackrabbit.oak.spi.xml.PropInfo; @@ -160,8 +164,7 @@ class UserImporter implements ProtectedP private Map<String, Principal> principals = new HashMap<String, Principal>();; UserImporter(ConfigurationParameters config) { - String importBehaviorStr = config.getConfigValue(PARAM_IMPORT_BEHAVIOR, ImportBehavior.NAME_IGNORE); - importBehavior = ImportBehavior.valueFromString(importBehaviorStr); + importBehavior = UserUtil.getImportBehavior(config); } //----------------------------------------------< ProtectedItemImporter >--- @@ -570,8 +573,8 @@ class UserImporter implements ProtectedP toRemove.put(dm.getID(), dm); } - List<Authorizable> toAdd = new ArrayList<Authorizable>(); - Set<String> nonExisting = new HashSet<String>(); + Map<String, Authorizable> toAdd = Maps.newHashMapWithExpectedSize(members.size()); + Map<String, String> nonExisting = Maps.newHashMap(); for (String contentId : members) { String remapped = referenceTracker.get(contentId); @@ -587,26 +590,32 @@ class UserImporter implements ProtectedP } if (member != null) { if (toRemove.remove(member.getID()) == null) { - toAdd.add(member); + toAdd.put(member.getID(), member); } // else: no need to remove from rep:members } else { handleFailure("New member of " + gr + ": No such authorizable (NodeID = " + memberContentId + ')'); if (importBehavior == ImportBehavior.BESTEFFORT) { log.info("ImportBehavior.BESTEFFORT: Remember non-existing member for processing."); - nonExisting.add(contentId); + /* since we ignore the set of failed ids later on and + don't know the real memberId => use fake memberId as + value in the map */ + nonExisting.put(contentId, "-"); } } } // 2. adjust members of the group - for (Authorizable m : toRemove.values()) { - if (!gr.removeMember(m)) { - handleFailure("Failed remove existing member (" + m + ") from " + gr); + if (!toRemove.isEmpty()) { + Set<String> failed = gr.removeMembers(toRemove.keySet().toArray(new String[toRemove.size()])); + if (!failed.isEmpty()) { + handleFailure("Failed removing members " + Iterables.toString(failed) + " to " + gr); } } - for (Authorizable m : toAdd) { - if (!gr.addMember(m)) { - handleFailure("Failed add member (" + m + ") to " + gr); + + if (!toAdd.isEmpty()) { + Set<String> failed = gr.addMembers(toAdd.keySet().toArray(new String[toAdd.size()])); + if (!failed.isEmpty()) { + handleFailure("Failed add members " + Iterables.toString(failed) + " to " + gr); } } @@ -616,9 +625,12 @@ class UserImporter implements ProtectedP Tree groupTree = root.getTree(gr.getPath()); MembershipProvider membershipProvider = userManager.getMembershipProvider(); - for (String member : nonExisting) { - membershipProvider.addMember(groupTree, member); - } + + Set<String> memberContentIds = Sets.newHashSet(nonExisting.keySet()); + Set<String> failedContentIds = membershipProvider.addMembers(groupTree, nonExisting); + memberContentIds.removeAll(failedContentIds); + + userManager.onGroupUpdate(gr, false, true, memberContentIds, failedContentIds); } } } Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserInitializer.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserInitializer.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserInitializer.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserInitializer.java Wed Aug 17 14:21:07 2016 @@ -108,12 +108,25 @@ class UserInitializer implements Workspa NodeUtil index = rootTree.getOrAddChild(IndexConstants.INDEX_DEFINITIONS_NAME, JcrConstants.NT_UNSTRUCTURED); if (!index.hasChild("authorizableId")) { - IndexUtils.createIndexDefinition(index, "authorizableId", true, new String[]{REP_AUTHORIZABLE_ID}, null); + NodeUtil authorizableId = IndexUtils.createIndexDefinition(index, "authorizableId", true, new String[]{REP_AUTHORIZABLE_ID}, null); + authorizableId.setString("info", + "Oak index used by the user management " + + "to quickly search an authorizable by id."); } if (!index.hasChild("principalName")) { - IndexUtils.createIndexDefinition(index, "principalName", true, + NodeUtil principalName = IndexUtils.createIndexDefinition(index, "principalName", true, new String[]{REP_PRINCIPAL_NAME}, new String[]{NT_REP_AUTHORIZABLE}); + principalName.setString("info", + "Oak index used by the user management " + + "to quickly search a prinipal by name."); + } + if (!index.hasChild("repMembers")) { + NodeUtil members = IndexUtils.createIndexDefinition(index, "repMembers", false, + new String[]{REP_MEMBERS}, + new String[]{NT_REP_MEMBER_REFERENCES}); + members.setString("info", + "Oak index used by the user management to lookup group membership."); } ConfigurationParameters params = userConfiguration.getParameters(); @@ -140,5 +153,4 @@ class UserInitializer implements Workspa NodeState target = store.getRoot(); target.compareAgainstBaseState(base, new ApplyDiff(builder)); } - } Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java Wed Aug 17 14:21:07 2016 @@ -22,6 +22,8 @@ import java.io.UnsupportedEncodingExcept import java.security.NoSuchAlgorithmException; import java.security.Principal; import java.util.Iterator; +import java.util.List; +import java.util.Set; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; @@ -30,6 +32,7 @@ import javax.jcr.RepositoryException; import javax.jcr.UnsupportedRepositoryOperationException; import com.google.common.base.Strings; +import com.google.common.collect.Lists; import org.apache.jackrabbit.api.security.principal.PrincipalManager; import org.apache.jackrabbit.api.security.user.Authorizable; import org.apache.jackrabbit.api.security.user.AuthorizableExistsException; @@ -54,6 +57,7 @@ import org.apache.jackrabbit.oak.spi.sec import org.apache.jackrabbit.oak.spi.security.user.action.AuthorizableAction; import org.apache.jackrabbit.oak.spi.security.user.action.AuthorizableActionProvider; import org.apache.jackrabbit.oak.spi.security.user.action.DefaultAuthorizableActionProvider; +import org.apache.jackrabbit.oak.spi.security.user.action.GroupAction; import org.apache.jackrabbit.oak.spi.security.user.util.PasswordUtil; import org.apache.jackrabbit.oak.spi.security.user.util.UserUtil; import org.apache.jackrabbit.oak.util.NodeUtil; @@ -306,6 +310,52 @@ public class UserManagerImpl implements } } + /** + * Upon a group being updated (single {@code Authorizable} successfully added or removed), + * call available {@code GroupAction}s and execute the method specific to removal or addition. + * {@code GroupAction}s may then validate or modify the changes. + * + * @param group The target group. + * @param isRemove Indicates whether the member is removed or added. + * @param member The member successfully removed or added. + * @throws RepositoryException If an error occurs. + */ + void onGroupUpdate(@Nonnull Group group, boolean isRemove, @Nonnull Authorizable member) throws RepositoryException { + for (GroupAction action : selectGroupActions()) { + if (isRemove) { + action.onMemberRemoved(group, member, root, namePathMapper); + } else { + action.onMemberAdded(group, member, root, namePathMapper); + } + } + } + + /** + * Upon a group being updated (multiple {@code memberIds} added or removed), + * call available {@code GroupAction}s and execute the method specific to removal or addition. + * {@code GroupAction}s may then validate or modify the changes. + * + * @param group The target group. + * @param isRemove Indicates whether the member is removed or added. + * @param isContentId Indicates whether member ids are expressed as content-ids (UUID) or member-ids. + * @param memberIds The IDs of all members successfully removed or added. + * @param failedIds The IDs of all members whose addition or removal failed. + * @throws RepositoryException If an error occurs. + */ + void onGroupUpdate(@Nonnull Group group, boolean isRemove, boolean isContentId, @Nonnull Set<String> memberIds, @Nonnull Set<String> failedIds) throws RepositoryException { + for (GroupAction action : selectGroupActions()) { + if (isRemove) { + action.onMembersRemoved(group, memberIds, failedIds, root, namePathMapper); + } else { + if (isContentId) { + action.onMembersAddedContentId(group, memberIds, failedIds, root, namePathMapper); + } else { + action.onMembersAdded(group, memberIds, failedIds, root, namePathMapper); + } + } + } + } + //-------------------------------------------------------------------------- @CheckForNull Authorizable getAuthorizable(Tree tree) throws RepositoryException { @@ -435,4 +485,20 @@ public class UserManagerImpl implements } return queryManager; } + + /** + * Select only {@code GroupAction}s from the available {@code AuthorizableAction}s. + * + * @return A {@code List} of {@code GroupAction}s. List may be empty. + */ + @Nonnull + private List<GroupAction> selectGroupActions() { + List<GroupAction> actions = Lists.newArrayList(); + for (AuthorizableAction action : actionProvider.getAuthorizableActions(securityProvider)) { + if (action instanceof GroupAction) { + actions.add((GroupAction) action); + } + } + return actions; + } } \ No newline at end of file Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidator.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidator.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidator.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserValidator.java Wed Aug 17 14:21:07 2016 @@ -114,9 +114,9 @@ class UserValidator extends DefaultValid } if (REP_MEMBERS.equals(name)) { - Set<String> afterValues = Sets.newHashSet(after.getValue(Type.STRINGS)); - afterValues.removeAll(ImmutableSet.copyOf(before.getValue(Type.STRINGS))); - checkForCyclicMembership(afterValues); + Set<String> addedValues = Sets.newHashSet(after.getValue(Type.STRINGS)); + addedValues.removeAll(ImmutableSet.copyOf(before.getValue(Type.STRINGS))); + checkForCyclicMembership(addedValues); } } @@ -191,7 +191,7 @@ class UserValidator extends DefaultValid MembershipProvider mp = provider.getMembershipProvider(); for (String memberContentId : memberRefs) { Tree memberTree = mp.getByContentID(memberContentId, AuthorizableType.GROUP); - if (memberTree != null && mp.isMember(memberTree, groupContentId, true)) { + if (memberTree != null && mp.isMember(memberTree, parentAfter)) { throw constraintViolation(31, "Cyclic group membership detected in group" + UserUtil.getAuthorizableId(parentAfter)); } } Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/autosave/GroupImpl.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/autosave/GroupImpl.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/autosave/GroupImpl.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/autosave/GroupImpl.java Wed Aug 17 14:21:07 2016 @@ -17,6 +17,8 @@ package org.apache.jackrabbit.oak.security.user.autosave; import java.util.Iterator; +import java.util.Set; +import javax.annotation.Nonnull; import javax.jcr.RepositoryException; import org.apache.jackrabbit.api.security.user.Authorizable; @@ -74,6 +76,15 @@ class GroupImpl extends AuthorizableImpl } @Override + public Set<String> addMembers(@Nonnull String... memberIds) throws RepositoryException { + try { + return getDelegate().addMembers(memberIds); + } finally { + getMgr().autosave(); + } + } + + @Override public boolean removeMember(Authorizable authorizable) throws RepositoryException { try { if (isValid(authorizable)) { @@ -84,6 +95,15 @@ class GroupImpl extends AuthorizableImpl } finally { getMgr().autosave(); } + } + + @Override + public Set<String> removeMembers(@Nonnull String... memberIds) throws RepositoryException { + try { + return getDelegate().removeMembers(memberIds); + } finally { + getMgr().autosave(); + } } private boolean isValid(Authorizable a) { Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/QueryUtil.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/QueryUtil.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/QueryUtil.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/QueryUtil.java Wed Aug 17 14:21:07 2016 @@ -22,12 +22,12 @@ import javax.jcr.RepositoryException; import javax.jcr.Value; import org.apache.jackrabbit.api.security.user.QueryBuilder; +import org.apache.jackrabbit.oak.commons.QueryUtils; import org.apache.jackrabbit.oak.namepath.NamePathMapper; import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters; import org.apache.jackrabbit.oak.spi.security.user.AuthorizableType; import org.apache.jackrabbit.oak.spi.security.user.UserConstants; import org.apache.jackrabbit.oak.spi.security.user.util.UserUtil; -import org.apache.jackrabbit.util.Text; /** * Common utilities used for user/group queries. @@ -66,7 +66,7 @@ public final class QueryUtil { * @return The corresponding node type name. */ @Nonnull - public static String getNodeTypeName(AuthorizableType type) { + public static String getNodeTypeName(@Nonnull AuthorizableType type) { if (type == AuthorizableType.USER) { return UserConstants.NT_REP_USER; } else if (type == AuthorizableType.GROUP) { @@ -84,27 +84,7 @@ public final class QueryUtil { */ @Nonnull public static String escapeNodeName(@Nonnull String string) { - StringBuilder result = new StringBuilder(); - - int k = 0; - int j; - do { - j = string.indexOf('%', k); - if (j < 0) { - // jcr escape trail - result.append(Text.escapeIllegalJcrChars(string.substring(k))); - } else if (j > 0 && string.charAt(j - 1) == '\\') { - // literal occurrence of % -> jcr escape - result.append(Text.escapeIllegalJcrChars(string.substring(k, j) + '%')); - } else { - // wildcard occurrence of % -> jcr escape all but % - result.append(Text.escapeIllegalJcrChars(string.substring(k, j))).append('%'); - } - - k = j + 1; - } while (j >= 0); - - return result.toString(); + return QueryUtils.escapeNodeName(string); } @Nonnull @@ -133,18 +113,7 @@ public final class QueryUtil { @Nonnull public static String escapeForQuery(@Nonnull String value) { - StringBuilder ret = new StringBuilder(); - for (int i = 0; i < value.length(); i++) { - char c = value.charAt(i); - if (c == '\\') { - ret.append("\\\\"); - } else if (c == '\'') { - ret.append("''"); - } else { - ret.append(c); - } - } - return ret.toString(); + return QueryUtils.escapeForQuery(value); } @Nonnull Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/AuthorizableType.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/AuthorizableType.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/AuthorizableType.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/AuthorizableType.java Wed Aug 17 14:21:07 2016 @@ -21,6 +21,8 @@ package org.apache.jackrabbit.oak.spi.se import javax.annotation.Nonnull; import org.apache.jackrabbit.api.security.user.Authorizable; +import org.apache.jackrabbit.api.security.user.Group; +import org.apache.jackrabbit.api.security.user.User; import org.apache.jackrabbit.api.security.user.UserManager; /** @@ -66,4 +68,16 @@ public enum AuthorizableType { return true; } } + + public Class<? extends Authorizable> getAuthorizableClass() { + switch (userType) { + case UserManager.SEARCH_TYPE_GROUP: + return Group.class; + case UserManager.SEARCH_TYPE_USER: + return User.class; + default: + // TYPE_AUTHORIZABLE: + return Authorizable.class; + } + } } Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/UserConfiguration.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/UserConfiguration.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/UserConfiguration.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/UserConfiguration.java Wed Aug 17 14:21:07 2016 @@ -17,11 +17,13 @@ package org.apache.jackrabbit.oak.spi.security.user; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import org.apache.jackrabbit.api.security.user.UserManager; import org.apache.jackrabbit.oak.api.Root; import org.apache.jackrabbit.oak.namepath.NamePathMapper; import org.apache.jackrabbit.oak.spi.security.SecurityConfiguration; +import org.apache.jackrabbit.oak.spi.security.principal.PrincipalProvider; /** * Configuration interface for user management. @@ -39,4 +41,27 @@ public interface UserConfiguration exten */ @Nonnull UserManager getUserManager(Root root, NamePathMapper namePathMapper); + + /** + * Optional method that allows a given user management implementation to + * provide a specific and optimized implementation of the {@link PrincipalProvider} + * interface for the principals represented by the user/groups known to + * this implementation. + * + * If this method returns {@code null} the security setup will by default + * use a basic {@code PrincipalProvider} implementation based on public + * user management API or a combination of other {@link PrincipalProvider}s + * as configured with the repository setup. + * + * @param root The root used to read the principal information from. + * @param namePathMapper The {@code NamePathMapper} to convert oak paths to JCR paths. + * @return An implementation of {@code PrincipalProvider} or {@code null} if + * principal discovery is provided by other means of if the default principal + * provider implementation should be used that acts on public user management + * API. + * + * @see {@link org.apache.jackrabbit.oak.spi.security.principal.PrincipalConfiguration} + */ + @Nullable + PrincipalProvider getUserPrincipalProvider(@Nonnull Root root, @Nonnull NamePathMapper namePathMapper); } Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/package-info.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/package-info.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/package-info.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/package-info.java Wed Aug 17 14:21:07 2016 @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -@Version("1.0") +@Version("1.1.0") @Export(optional = "provide:=true") package org.apache.jackrabbit.oak.spi.security.user.action; Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/util/UserUtil.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/util/UserUtil.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/util/UserUtil.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/util/UserUtil.java Wed Aug 17 14:21:07 2016 @@ -27,9 +27,12 @@ import org.apache.jackrabbit.oak.api.Tre import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters; import org.apache.jackrabbit.oak.spi.security.user.AuthorizableType; import org.apache.jackrabbit.oak.spi.security.user.UserConstants; +import org.apache.jackrabbit.oak.spi.xml.ImportBehavior; +import org.apache.jackrabbit.oak.spi.xml.ProtectedItemImporter; import org.apache.jackrabbit.oak.util.TreeUtil; import org.apache.jackrabbit.util.Text; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static org.apache.jackrabbit.oak.api.Type.STRING; @@ -131,6 +134,25 @@ public final class UserUtil implements U return null; } + /** + * Retrieve the id from the given {@code authorizableTree}, which must have + * been verified for being a valid authorizable of the specified type upfront. + * + * @param authorizableTree The authorizable tree which must be of the given {@code type}/ + * @param type The type of the authorizable tree. + * @return The id retrieved from the specified {@code AuthorizableTree}. + */ + @Nonnull + public static String getAuthorizableId(@Nonnull Tree authorizableTree, @Nonnull AuthorizableType type) { + checkArgument(UserUtil.isType(authorizableTree, type)); + PropertyState idProp = authorizableTree.getProperty(UserConstants.REP_AUTHORIZABLE_ID); + if (idProp != null) { + return idProp.getValue(STRING); + } else { + return Text.unescapeIllegalJcrChars(authorizableTree.getName()); + } + } + @CheckForNull public static <T extends Authorizable> T castAuthorizable(@Nullable Authorizable authorizable, Class<T> authorizableClass) throws AuthorizableTypeException { if (authorizable == null) { @@ -143,4 +165,21 @@ public final class UserUtil implements U throw new AuthorizableTypeException("Invalid authorizable type '" + ((authorizableClass == null) ? "null" : authorizableClass) + '\''); } } + + /** + * Return the configured {@link org.apache.jackrabbit.oak.spi.xml.ImportBehavior} + * for the given {@code config}. The default behavior in case + * {@link org.apache.jackrabbit.oak.spi.xml.ProtectedItemImporter#PARAM_IMPORT_BEHAVIOR} + * is not contained in the {@code config} object is + * {@link org.apache.jackrabbit.oak.spi.xml.ImportBehavior#IGNORE} + * + * @param config The configuration parameters. + * @return The import behavior as defined by {@link org.apache.jackrabbit.oak.spi.xml.ProtectedItemImporter#PARAM_IMPORT_BEHAVIOR} + * or {@link org.apache.jackrabbit.oak.spi.xml.ImportBehavior#IGNORE} if this + * config parameter is missing. + */ + public static int getImportBehavior(@Nonnull ConfigurationParameters config) { + String importBehaviorStr = config.getConfigValue(ProtectedItemImporter.PARAM_IMPORT_BEHAVIOR, ImportBehavior.NAME_IGNORE); + return ImportBehavior.valueFromString(importBehaviorStr); + } } \ No newline at end of file Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/util/package-info.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/util/package-info.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/util/package-info.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/util/package-info.java Wed Aug 17 14:21:07 2016 @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -@Version("1.0") +@Version("1.2.0") @Export(optional = "provide:=true") package org.apache.jackrabbit.oak.spi.security.user.util; Modified: jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/AbstractSecurityTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/AbstractSecurityTest.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/AbstractSecurityTest.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/AbstractSecurityTest.java Wed Aug 17 14:21:07 2016 @@ -72,7 +72,6 @@ public abstract class AbstractSecurityTe private ContentRepository contentRepository; private UserManager userManager; private User testUser; - private PrivilegeManager privMgr; protected NamePathMapper namePathMapper = NamePathMapper.DEFAULT; protected SecurityProvider securityProvider; @@ -154,10 +153,14 @@ public abstract class AbstractSecurityTe } protected UserManager getUserManager(Root root) { - if (userManager == null) { - userManager = getConfig(UserConfiguration.class).getUserManager(root, getNamePathMapper()); + if (this.root == root) { + if (userManager == null) { + userManager = getConfig(UserConfiguration.class).getUserManager(root, getNamePathMapper()); + } + return userManager; + } else { + return getConfig(UserConfiguration.class).getUserManager(root, getNamePathMapper()); } - return userManager; } protected PrincipalManager getPrincipalManager(Root root) { @@ -187,10 +190,7 @@ public abstract class AbstractSecurityTe } protected PrivilegeManager getPrivilegeManager(Root root) { - if (privMgr == null) { - privMgr = getConfig(PrivilegeConfiguration.class).getPrivilegeManager(root, getNamePathMapper()); - } - return privMgr; + return getConfig(PrivilegeConfiguration.class).getPrivilegeManager(root, getNamePathMapper()); } protected ValueFactory getValueFactory() { Modified: jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdateTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdateTest.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdateTest.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdateTest.java Wed Aug 17 14:21:07 2016 @@ -27,7 +27,6 @@ import static org.hamcrest.CoreMatchers. 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.assertThat; import static org.junit.Assert.assertTrue; Modified: jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/principal/PrincipalProviderImplTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/principal/PrincipalProviderImplTest.java?rev=1756639&r1=1756638&r2=1756639&view=diff ============================================================================== --- jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/principal/PrincipalProviderImplTest.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/principal/PrincipalProviderImplTest.java Wed Aug 17 14:21:07 2016 @@ -17,83 +17,26 @@ package org.apache.jackrabbit.oak.security.principal; import java.security.Principal; -import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; import java.util.Iterator; -import java.util.List; -import java.util.Map; import java.util.Set; import com.google.common.collect.ImmutableSet; import org.apache.jackrabbit.api.security.principal.PrincipalManager; import org.apache.jackrabbit.api.security.user.Group; -import org.apache.jackrabbit.api.security.user.User; import org.apache.jackrabbit.api.security.user.UserManager; -import org.apache.jackrabbit.oak.AbstractSecurityTest; -import org.apache.jackrabbit.oak.spi.security.principal.AdminPrincipal; +import org.apache.jackrabbit.oak.spi.security.principal.AbstractPrincipalProviderTest; import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; import org.apache.jackrabbit.oak.spi.security.principal.PrincipalProvider; import org.junit.Test; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -/** - * PrincipalProviderImplTest... - */ -public class PrincipalProviderImplTest extends AbstractSecurityTest { +public class PrincipalProviderImplTest extends AbstractPrincipalProviderTest { - private PrincipalProvider principalProvider; - - @Override - public void before() throws Exception { - super.before(); - - principalProvider = new PrincipalProviderImpl(root, getUserConfiguration(), namePathMapper); - } - - @Test - public void testGetPrincipals() throws Exception { - String adminId = adminSession.getAuthInfo().getUserID(); - Set<? extends Principal> principals = principalProvider.getPrincipals(adminId); - - assertNotNull(principals); - assertFalse(principals.isEmpty()); - assertTrue(principals.contains(EveryonePrincipal.getInstance())); - - boolean containsAdminPrincipal = false; - for (Principal principal : principals) { - assertNotNull(principalProvider.getPrincipal(principal.getName())); - if (principal instanceof AdminPrincipal) { - containsAdminPrincipal = true; - } - } - assertTrue(containsAdminPrincipal); - } - - @Test - public void testEveryone() throws Exception { - Principal everyone = principalProvider.getPrincipal(EveryonePrincipal.NAME); - assertTrue(everyone instanceof EveryonePrincipal); - - Group everyoneGroup = null; - try { - UserManager userMgr = getUserManager(root); - everyoneGroup = userMgr.createGroup(EveryonePrincipal.NAME); - root.commit(); - - Principal ep = principalProvider.getPrincipal(EveryonePrincipal.NAME); - assertFalse(ep instanceof EveryonePrincipal); - } finally { - if (everyoneGroup != null) { - everyoneGroup.remove(); - root.commit(); - } - } + protected PrincipalProvider createPrincipalProvider() { + return new PrincipalProviderImpl(root, getUserConfiguration(), namePathMapper); } @Test @@ -127,151 +70,4 @@ public class PrincipalProviderImplTest e } } } - - @Test - public void testFindUserPrincipal() throws Exception { - User testUser = null; - try { - UserManager userMgr = getUserManager(root); - testUser = userMgr.createUser("TestUser", "pw"); - root.commit(); - - String principalName = testUser.getPrincipal().getName(); - assertNotNull(principalProvider.getPrincipal(principalName)); - - List<String> nameHints = new ArrayList<String>(); - nameHints.add("TestUser"); - nameHints.add("Test"); - nameHints.add("User"); - nameHints.add("stUs"); - - assertResult(principalProvider, nameHints, principalName, PrincipalManager.SEARCH_TYPE_NOT_GROUP, true); - assertResult(principalProvider, nameHints, principalName, PrincipalManager.SEARCH_TYPE_ALL, true); - assertResult(principalProvider, nameHints, principalName, PrincipalManager.SEARCH_TYPE_GROUP, false); - } finally { - if (testUser != null) { - testUser.remove(); - root.commit(); - } - } - } - - @Test - public void testFindGroupPrincipal() throws Exception { - Group testGroup = null; - try { - UserManager userMgr = getUserManager(root); - testGroup = userMgr.createGroup("TestGroup"); - root.commit(); - - String principalName = testGroup.getPrincipal().getName(); - assertNotNull(principalProvider.getPrincipal(principalName)); - - List<String> nameHints = new ArrayList<String>(); - nameHints.add("TestGroup"); - nameHints.add("Test"); - nameHints.add("Group"); - nameHints.add("stGr"); - - assertResult(principalProvider, nameHints, principalName, PrincipalManager.SEARCH_TYPE_GROUP, true); - assertResult(principalProvider, nameHints, principalName, PrincipalManager.SEARCH_TYPE_ALL, true); - assertResult(principalProvider, nameHints, principalName, PrincipalManager.SEARCH_TYPE_NOT_GROUP, false); - } finally { - if (testGroup != null) { - testGroup.remove(); - root.commit(); - } - } - } - - @Test - public void testFindEveryone() { - assertNotNull(principalProvider.getPrincipal(EveryonePrincipal.NAME)); - - Map<Integer, Boolean> tests = new HashMap<Integer, Boolean>(); - tests.put(PrincipalManager.SEARCH_TYPE_ALL, Boolean.TRUE); - tests.put(PrincipalManager.SEARCH_TYPE_GROUP, Boolean.TRUE); - tests.put(PrincipalManager.SEARCH_TYPE_NOT_GROUP, Boolean.FALSE); - - for (Integer searchType : tests.keySet()) { - boolean found = false; - Iterator<? extends Principal> it = principalProvider.findPrincipals(EveryonePrincipal.NAME, searchType); - while (it.hasNext()) { - Principal p = it.next(); - if (p.getName().equals(EveryonePrincipal.NAME)) { - found = true; - } - } - Boolean expected = tests.get(searchType); - assertEquals(expected.booleanValue(), found); - - } - } - - @Test - public void testFindEveryoneHint() { - assertNotNull(principalProvider.getPrincipal(EveryonePrincipal.NAME)); - - List<String> nameHints = new ArrayList<String>(); - nameHints.add("everyone"); - nameHints.add("every"); - nameHints.add("one"); - nameHints.add("very"); - - assertResult(principalProvider, nameHints, EveryonePrincipal.NAME, PrincipalManager.SEARCH_TYPE_ALL, true); - assertResult(principalProvider, nameHints, EveryonePrincipal.NAME, PrincipalManager.SEARCH_TYPE_GROUP, true); - assertResult(principalProvider, nameHints, EveryonePrincipal.NAME, PrincipalManager.SEARCH_TYPE_NOT_GROUP, false); - } - - @Test - public void testFindWithoutHint() throws Exception { - User testUser = null; - Group testGroup = null; - try { - UserManager userMgr = getUserManager(root); - testUser = userMgr.createUser("TestUser", "pw"); - testGroup = userMgr.createGroup("TestGroup"); - - root.commit(); - - Set<String> resultNames = new HashSet<String>(); - Iterator<? extends Principal> principals = principalProvider.findPrincipals(PrincipalManager.SEARCH_TYPE_ALL); - while (principals.hasNext()) { - resultNames.add(principals.next().getName()); - } - - assertTrue(resultNames.contains(EveryonePrincipal.NAME)); - assertTrue(resultNames.contains("TestUser")); - assertTrue(resultNames.contains("TestGroup")); - - } finally { - if (testUser != null) { - testUser.remove(); - } - if (testGroup != null) { - testGroup.remove(); - } - root.commit(); - } - } - - private static void assertResult(PrincipalProvider principalProvider, - List<String> nameHints, String expectedName, - int searchType, boolean toBeFound) { - for (String nameHint : nameHints) { - Iterator<? extends Principal> result = principalProvider.findPrincipals(nameHint, searchType); - boolean found = false; - while (result.hasNext()) { - Principal p = result.next(); - if (p.getName().equals(expectedName)) { - found = true; - } - } - if (toBeFound) { - assertTrue("Expected principal to be found by name hint " + expectedName, found); - } else { - assertFalse("Expected principal NOT to be found by name hint " + expectedName, found); - } - } - } } \ No newline at end of file Copied: jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractAddMembersByIdTest.java (from r1694049, jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractAddMembersByIdTest.java) URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractAddMembersByIdTest.java?p2=jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractAddMembersByIdTest.java&p1=jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractAddMembersByIdTest.java&r1=1694049&r2=1756639&rev=1756639&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractAddMembersByIdTest.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AbstractAddMembersByIdTest.java Wed Aug 17 14:21:07 2016 @@ -16,6 +16,7 @@ */ package org.apache.jackrabbit.oak.security.user; +import java.util.Iterator; import java.util.Set; import java.util.UUID; import javax.annotation.Nonnull; @@ -36,12 +37,14 @@ import org.apache.jackrabbit.oak.api.Pro import org.apache.jackrabbit.oak.api.Root; import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants; import org.apache.jackrabbit.oak.spi.security.user.UserConstants; import org.junit.Test; 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.assertTrue; @@ -185,7 +188,8 @@ public abstract class AbstractAddMembers } catch (ConstraintViolationException e) { // expected } - assertTrue(root.hasPendingChanges()); + // no modifications expected as testing for empty id is done before changes are made + assertFalse(root.hasPendingChanges()); } @Test(expected = NullPointerException.class) @@ -205,7 +209,8 @@ public abstract class AbstractAddMembers } catch (ConstraintViolationException e) { // expected } - assertTrue(root.hasPendingChanges()); + // no modifications expected as testing for null id is done before changes are made + assertFalse(root.hasPendingChanges()); } @Test @@ -252,4 +257,38 @@ public abstract class AbstractAddMembers Set<String> failed = testGroup.addMembers(getTestUser().getID(), memberGroup.getID()); assertEquals(2, failed.size()); } + + @Test + public void testEveryoneAsMember() throws Exception { + UserManagerImpl userManager = (UserManagerImpl) getUserManager(root); + Group everyone = userManager.createGroup(EveryonePrincipal.getInstance()); + try { + Set<String> failed = testGroup.addMembers(everyone.getID()); + assertFalse(failed.isEmpty()); + assertTrue(failed.contains(everyone.getID())); + root.commit(); + + assertFalse(testGroup.isDeclaredMember(everyone)); + assertFalse(testGroup.isMember(everyone)); + for (Iterator<Group> it = everyone.memberOf(); it.hasNext(); ) { + assertNotEquals(testGroup.getID(), it.next().getID()); + } + for (Iterator<Group> it = everyone.declaredMemberOf(); it.hasNext(); ) { + assertNotEquals(testGroup.getID(), it.next().getID()); + } + + boolean found = false; + MembershipProvider mp = userManager.getMembershipProvider(); + for (Iterator<String> it = mp.getMembership(root.getTree(everyone.getPath()), true); it.hasNext(); ) { + String p = it.next(); + if (testGroup.getPath().equals(p)) { + found = true; + } + } + assertFalse(found); + } finally { + everyone.remove(); + root.commit(); + } + } } \ No newline at end of file Copied: jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AddMembersByIdBestEffortTest.java (from r1694049, jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AddMembersByIdBestEffortTest.java) URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AddMembersByIdBestEffortTest.java?p2=jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AddMembersByIdBestEffortTest.java&p1=jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AddMembersByIdBestEffortTest.java&r1=1694049&r2=1756639&rev=1756639&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AddMembersByIdBestEffortTest.java (original) +++ jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AddMembersByIdBestEffortTest.java Wed Aug 17 14:21:07 2016 @@ -24,8 +24,10 @@ import java.util.Set; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import org.apache.jackrabbit.api.security.user.Authorizable; +import org.apache.jackrabbit.api.security.user.Group; import org.apache.jackrabbit.oak.api.CommitFailedException; import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters; +import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration; import org.apache.jackrabbit.oak.spi.xml.ImportBehavior; import org.apache.jackrabbit.oak.spi.xml.ProtectedItemImporter; @@ -33,6 +35,7 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -61,6 +64,44 @@ public class AddMembersByIdBestEffortTes ); } + /** + * "Oddity" when adding per id + besteffort: everyone group will not be + * dealt with separately and will end up being listed in a rep:members property. + */ + @Test + public void testEveryoneAsMember() throws Exception { + UserManagerImpl userManager = (UserManagerImpl) getUserManager(root); + Group everyone = userManager.createGroup(EveryonePrincipal.getInstance()); + try { + Set<String> failed = testGroup.addMembers(everyone.getID()); + assertTrue(failed.isEmpty()); + root.commit(); + + assertFalse(testGroup.isDeclaredMember(everyone)); + assertFalse(testGroup.isMember(everyone)); + for (Iterator<Group> it = everyone.memberOf(); it.hasNext(); ) { + assertNotEquals(testGroup.getID(), it.next().getID()); + } + for (Iterator<Group> it = everyone.declaredMemberOf(); it.hasNext(); ) { + assertNotEquals(testGroup.getID(), it.next().getID()); + } + + // oddity of the current impl that add members without testing for + boolean found = false; + MembershipProvider mp = userManager.getMembershipProvider(); + for (Iterator<String> it = mp.getMembership(root.getTree(everyone.getPath()), true); it.hasNext(); ) { + String p = it.next(); + if (testGroup.getPath().equals(p)) { + found = true; + } + } + assertTrue(found); + } finally { + everyone.remove(); + root.commit(); + } + } + @Test public void testNonExistingMember() throws Exception { Set<String> failed = addNonExistingMember(); @@ -115,6 +156,22 @@ public class AddMembersByIdBestEffortTes root.refresh(); assertFalse(testGroup.isMember(memberGroup)); } + } + @Test + public void testMemberListExistingMembers() throws Exception { + MembershipProvider mp = ((UserManagerImpl) getUserManager(root)).getMembershipProvider(); + try { + mp.setMembershipSizeThreshold(5); + for (int i = 0; i < 10; i++) { + testGroup.addMembers("member" + i); + } + + Set<String> failed = testGroup.addMembers("member2"); + assertFalse(failed.isEmpty()); + + } finally { + mp.setMembershipSizeThreshold(100); // back to default + } } } \ No newline at end of file
