You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by an...@apache.org on 2008/04/25 17:11:57 UTC
svn commit: r651624 - in /jackrabbit/trunk:
jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/
jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/
jackrabbit-core/src/main/java/org/apache/jackrabbit/core/secu...
Author: angela
Date: Fri Apr 25 08:11:49 2008
New Revision: 651624
URL: http://svn.apache.org/viewvc?rev=651624&view=rev
Log:
JCR-1104 : JSR 283 support (security work in progress)
- user API: add distinction between declared and indirect group membership
- user Impl: move group membership to authorizable node type
- javadoc
- minor fixes/improvements
Modified:
jackrabbit/trunk/jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/Authorizable.java
jackrabbit/trunk/jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/Group.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java
jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.cnd
jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.xml
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/AuthorizableTest.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/GroupTest.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/GroupAdministratorTest.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserAdministratorTest.java
Modified: jackrabbit/trunk/jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/Authorizable.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/Authorizable.java?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/Authorizable.java (original)
+++ jackrabbit/trunk/jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/Authorizable.java Fri Apr 25 08:11:49 2008
@@ -112,7 +112,14 @@
PrincipalIterator getPrincipals() throws RepositoryException;
/**
- * @return all {@link Group}s, this Authorizable is member of
+ * @return all {@link Group}s, this Authorizable is declared member of.
+ * @throws RepositoryException
+ */
+ Iterator declaredMemberOf() throws RepositoryException;
+
+ /**
+ * @return all {@link Group}s, this Authorizable is member of included
+ * indirect group membership.
* @throws RepositoryException
*/
Iterator memberOf() throws RepositoryException;
Modified: jackrabbit/trunk/jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/Group.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/Group.java?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/Group.java (original)
+++ jackrabbit/trunk/jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/Group.java Fri Apr 25 08:11:49 2008
@@ -16,8 +16,6 @@
*/
package org.apache.jackrabbit.api.security.user;
-import org.apache.jackrabbit.api.security.user.Authorizable;
-
import javax.jcr.RepositoryException;
import java.util.Iterator;
@@ -27,14 +25,23 @@
public interface Group extends Authorizable {
/**
- * @return Iterator of <code>Authorizable</code>s which are getMembers of
- * this Group.
+ * @return Iterator of <code>Authorizable</code>s which are declared
+ * members of this Group.
+ * @throws RepositoryException
+ */
+ Iterator getDeclaredMembers() throws RepositoryException;
+
+ /**
+ * @return Iterator of <code>Authorizable</code>s which are members of
+ * this Group. This includes both declared members and all authorizables
+ * that are indirect group members.
* @throws RepositoryException
*/
Iterator getMembers() throws RepositoryException;
/**
- * @return true if the Authorizable to test is a member of this Group.
+ * @return true if the Authorizable to test is a direct or indirect member
+ * of this Group.
* @throws RepositoryException
*/
boolean isMember(Authorizable authorizable) throws RepositoryException;
@@ -45,14 +52,14 @@
*
* @return true if the <code>Authorizable</code> has successfully been added
* to this Group, false otherwise (e.g. unknown implemention
- * or if it already is a member or if the passed authorizable is the
+ * or if it already is a member or if the passed authorizable is this
* group itself or for some implementation specific constraint).
* @throws RepositoryException If an error occurs.
*/
boolean addMember(Authorizable authorizable) throws RepositoryException;
/**
- * Remove a member to this Group.<br>
+ * Remove a member from this Group.<br>
* Changes will be persisted immediately.
*
* @return true if the Authorizable was successfully removed. False otherwise.
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java Fri Apr 25 08:11:49 2008
@@ -25,6 +25,9 @@
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.api.security.user.UserManager;
import org.apache.jackrabbit.core.security.user.UserManagerImpl;
+import org.apache.jackrabbit.core.SessionImpl;
+import org.apache.jackrabbit.spi.commons.conversion.NameResolver;
+import org.apache.jackrabbit.util.Text;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -67,6 +70,8 @@
private final EveryonePrincipal everyonePrincipal;
+ private final String pGroupName;
+
/**
* Creates a new DefaultPrincipalProvider reading the principals from the
* storage below the given security root node.
@@ -81,13 +86,22 @@
everyonePrincipal = EveryonePrincipal.getInstance();
membershipCache = new LRUMap();
- // listen to any modifications for users and groups
+ // listen to modifications of group-membership
+ String[] ntNames = new String[1];
+ if (securitySession instanceof SessionImpl) {
+ NameResolver resolver = (SessionImpl) securitySession;
+ ntNames[0] = resolver.getJCRName(UserManagerImpl.NT_REP_USER);
+ pGroupName = resolver.getJCRName(UserManagerImpl.P_GROUPS);
+ } else {
+ ntNames[0] = "rep:User";
+ pGroupName = "rep:groups";
+ }
securitySession.getWorkspace().getObservationManager().addEventListener(this,
- Event.NODE_ADDED | Event.NODE_REMOVED | Event.PROPERTY_ADDED | Event.PROPERTY_CHANGED | Event.PROPERTY_REMOVED,
+ Event.NODE_REMOVED | Event.PROPERTY_ADDED | Event.PROPERTY_CHANGED | Event.PROPERTY_REMOVED,
UserManagerImpl.SECURITY_ROOT_PATH,
true,
null,
- null,
+ ntNames,
false);
}
@@ -220,10 +234,27 @@
* @see EventListener#onEvent(EventIterator)
*/
public void onEvent(EventIterator eventIterator) {
- // simple rule: flush all cached
+ // superclass: flush all cached
clearCache();
- synchronized (membershipCache) {
- membershipCache.clear();
+
+ // membership cache:
+ while (eventIterator.hasNext()) {
+ Event ev = eventIterator.nextEvent();
+ int type = ev.getType();
+ if (type == Event.PROPERTY_ADDED || type == Event.PROPERTY_CHANGED
+ || type == Event.PROPERTY_REMOVED) {
+ try {
+ if (pGroupName.equals(Text.getName(ev.getPath()))) {
+ synchronized (membershipCache) {
+ membershipCache.clear();
+ }
+ break;
+ }
+ } catch (RepositoryException e) {
+ // should never get here
+ log.warn(e.getMessage());
+ }
+ }
}
}
@@ -247,13 +278,7 @@
Iterator itr = auth.memberOf();
while (itr.hasNext()) {
Group group = (Group) itr.next();
- Principal groupPrinc = group.getPrincipal();
- if (membership.add(groupPrinc)) {
- membership.addAll(collectGroupMembership(groupPrinc, membership));
- } else {
- String msg = "Cyclic group membership detected with Group " + groupPrinc.getName();
- log.error(msg);
- }
+ membership.add(group.getPrincipal());
}
} else {
log.debug("Cannot find authorizable for principal " + princ.getName());
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java Fri Apr 25 08:11:49 2008
@@ -16,14 +16,15 @@
*/
package org.apache.jackrabbit.core.security.user;
-import org.apache.jackrabbit.core.NodeImpl;
-import org.apache.jackrabbit.core.SessionImpl;
-import org.apache.jackrabbit.core.PropertyImpl;
+import org.apache.jackrabbit.api.security.principal.PrincipalIterator;
+import org.apache.jackrabbit.api.security.principal.PrincipalManager;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.AuthorizableExistsException;
import org.apache.jackrabbit.api.security.user.Group;
-import org.apache.jackrabbit.api.security.principal.PrincipalIterator;
-import org.apache.jackrabbit.api.security.principal.PrincipalManager;
+import org.apache.jackrabbit.api.security.user.User;
+import org.apache.jackrabbit.core.NodeImpl;
+import org.apache.jackrabbit.core.PropertyImpl;
+import org.apache.jackrabbit.core.SessionImpl;
import org.apache.jackrabbit.core.security.principal.ItemBasedPrincipal;
import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
import org.apache.jackrabbit.core.security.principal.PrincipalIteratorAdapter;
@@ -39,8 +40,8 @@
import javax.jcr.nodetype.ConstraintViolationException;
import java.security.Principal;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
-import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
@@ -132,20 +133,21 @@
}
/**
+ * @see Authorizable#declaredMemberOf()
+ */
+ public Iterator declaredMemberOf() throws RepositoryException {
+ List memberShip = new ArrayList();
+ collectMembership(memberShip, false);
+ return memberShip.iterator();
+ }
+
+ /**
* @see Authorizable#memberOf()
*/
public Iterator memberOf() throws RepositoryException {
- // TODO: replace by weak-refs
- PropertyIterator itr = node.getReferences();
- Collection tmp = new HashSet((int) itr.getSize());
- while (itr.hasNext()) {
- NodeImpl groupNode = (NodeImpl) itr.nextProperty().getParent();
- if (groupNode.isNodeType(NT_REP_GROUP)) {
- Group group = GroupImpl.create(groupNode, userManager);
- tmp.add(group);
- }
- }
- return tmp.iterator();
+ List memberShip = new ArrayList();
+ collectMembership(memberShip, true);
+ return memberShip.iterator();
}
/**
@@ -252,7 +254,11 @@
* @see Authorizable#remove()
*/
public synchronized void remove() throws RepositoryException {
- // TODO: ev. remove group-memberships first?
+ // don't allow for removal of the administrator even if the executing
+ // session has all permissions.
+ if (!isGroup() && ((User) this).isAdmin()) {
+ throw new RepositoryException("The administrator cannot be removed.");
+ }
userManager.removeProtectedItem(node, node.getParent());
}
@@ -273,6 +279,77 @@
return node.getProperty(P_PRINCIPAL_NAME).getString();
}
+ boolean addToGroup(GroupImpl group) throws RepositoryException {
+ try {
+ Value[] values;
+ // TODO: replace by weak-refs
+ Value added = getSession().getValueFactory().createValue(group.getNode());
+ NodeImpl node = getNode();
+ if (node.hasProperty(P_GROUPS)) {
+ Value[] old = node.getProperty(P_GROUPS).getValues();
+ values = new Value[old.length + 1];
+ System.arraycopy(old, 0, values, 0, old.length);
+ } else {
+ values = new Value[1];
+ }
+ values[values.length - 1] = added;
+ userManager.setProtectedProperty(node, P_GROUPS, values);
+ return true;
+ } catch (RepositoryException e) {
+ // revert all pending changes and rethrow.
+ log.error("Error while editing group membership:", e.getMessage());
+ getSession().refresh(false);
+ throw e;
+ }
+ }
+
+ boolean removeFromGroup(GroupImpl group) throws RepositoryException {
+ NodeImpl node = getNode();
+ String message = "Authorizable " + getID() + " is not member of " + group.getID();
+ if (!node.hasProperty(P_GROUPS)) {
+ log.debug(message);
+ return false;
+ }
+
+ Value toRemove = getSession().getValueFactory().createValue(group.getNode());
+ PropertyImpl property = node.getProperty(P_GROUPS);
+ List valList = new ArrayList(Arrays.asList(property.getValues()));
+ if (valList.remove(toRemove)) {
+ try {
+ if (valList.isEmpty()) {
+ userManager.removeProtectedItem(property, node);
+ } else {
+ Value[] values = (Value[]) valList.toArray(new Value[valList.size()]);
+ userManager.setProtectedProperty(node, P_GROUPS, values);
+ }
+ return true;
+ } catch (RepositoryException e) {
+ // modification failed -> revert all pending changes.
+ node.refresh(false);
+ throw e;
+ }
+ } else {
+ // nothing changed
+ log.debug(message);
+ return false;
+ }
+ }
+
+ private void collectMembership(List groups, boolean includedIndirect) throws RepositoryException {
+ NodeImpl node = getNode();
+ if (!node.hasProperty(P_GROUPS)) {
+ return;
+ }
+ Value[] refs = node.getProperty(P_GROUPS).getValues();
+ for (int i = 0; i < refs.length; i++) {
+ NodeImpl groupNode = (NodeImpl) getSession().getNodeByUUID(refs[i].getString());
+ Group group = GroupImpl.create(groupNode, userManager);
+ if (groups.add(group) && includedIndirect) {
+ ((AuthorizableImpl) group).collectMembership(groups, true);
+ }
+ }
+ }
+
/**
* Test if the JCR property to be modified/removed is one of the
* following that has a special meaning and must be altered using this
@@ -282,7 +359,7 @@
* <li>rep:principalName</li>
* <li>rep:userId</li>
* <li>rep:referees</li>
- * <li>rep:members</li>
+ * <li>rep:groups</li>
* <li>rep:impersonators</li>
* </ul>
* Those properties are 'protected' in their property definition. This
@@ -296,7 +373,7 @@
private boolean isProtectedProperty(String propertyName) throws RepositoryException {
Name pName = getSession().getQName(propertyName);
if (P_PRINCIPAL_NAME.equals(pName) || P_USERID.equals(pName)
- || P_REFEREES.equals(pName) || P_MEMBERS.equals(pName)
+ || P_REFEREES.equals(pName) || P_GROUPS.equals(pName)
|| P_IMPERSONATORS.equals(pName)) {
return true;
} else {
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java Fri Apr 25 08:11:49 2008
@@ -16,30 +16,25 @@
*/
package org.apache.jackrabbit.core.security.user;
-import org.apache.jackrabbit.core.NodeImpl;
-import org.apache.jackrabbit.core.PropertyImpl;
-import org.apache.jackrabbit.api.security.principal.PrincipalManager;
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.core.NodeImpl;
import org.apache.jackrabbit.util.Text;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javax.jcr.Node;
-import javax.jcr.Property;
+import javax.jcr.PropertyIterator;
import javax.jcr.RepositoryException;
-import javax.jcr.Value;
import java.io.IOException;
import java.io.ObjectOutputStream;
import java.security.Principal;
-import java.util.ArrayList;
-import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Iterator;
-import java.util.List;
import java.util.NoSuchElementException;
import java.util.Set;
@@ -98,10 +93,17 @@
//--------------------------------------------------------------< Group >---
/**
+ * @see Group#getDeclaredMembers()
+ */
+ public Iterator getDeclaredMembers() throws RepositoryException {
+ return getMembers(false).iterator();
+ }
+
+ /**
* @see Group#getMembers()
*/
public Iterator getMembers() throws RepositoryException {
- return new MemberIterator(memberUUIDs().iterator());
+ return getMembers(true).iterator();
}
/**
@@ -111,9 +113,14 @@
if (authorizable == null || !(authorizable instanceof AuthorizableImpl)) {
return false;
} else {
+ String thisID = getID();
AuthorizableImpl impl = (AuthorizableImpl) authorizable;
- String uuid = impl.getNode().getUUID();
- return memberUUIDs().contains(uuid);
+ for (Iterator it = impl.memberOf(); it.hasNext();) {
+ if (thisID.equals(((GroupImpl) it.next()).getID())) {
+ return true;
+ }
+ }
+ return false;
}
}
@@ -130,27 +137,16 @@
return false;
}
- Node memberNode = ((AuthorizableImpl) authorizable).getNode();
+ AuthorizableImpl authImpl = ((AuthorizableImpl) authorizable);
+ Node memberNode = authImpl.getNode();
if (memberNode.isSame(getNode())) {
String msg = "Attempt to add a Group as member of itself (" + getID() + ").";
log.warn(msg);
return false;
}
- Value[] values;
- // TODO: replace by weak-refs
- Value added = getSession().getValueFactory().createValue(memberNode);
- NodeImpl node = getNode();
- if (node.hasProperty(P_MEMBERS)) {
- Value[] old = node.getProperty(P_MEMBERS).getValues();
- values = new Value[old.length + 1];
- System.arraycopy(old, 0, values, 0, old.length);
- } else {
- values = new Value[1];
- }
- values[values.length - 1] = added;
- userManager.setProtectedProperty(node, P_MEMBERS, values);
- return true;
+ // preconditions are met -> delegate to authorizableImpl
+ return authImpl.addToGroup(this);
}
/**
@@ -160,61 +156,40 @@
if (!isMember(authorizable) || !(authorizable instanceof AuthorizableImpl)) {
return false;
}
- NodeImpl node = getNode();
- if (!node.hasProperty(P_MEMBERS)) {
- log.debug("Group has no members -> cannot remove member " + authorizable.getID());
- return false;
- }
-
- Value toRemove = getSession().getValueFactory().createValue(((AuthorizableImpl)authorizable).getNode());
-
- PropertyImpl property = node.getProperty(P_MEMBERS);
- List valList = new ArrayList(Arrays.asList(property.getValues()));
-
- if (valList.remove(toRemove)) {
- try {
- if (valList.isEmpty()) {
- userManager.removeProtectedItem(property, node);
- } else {
- Value[] values = (Value[]) valList.toArray(new Value[valList.size()]);
- userManager.setProtectedProperty(node, P_MEMBERS, values);
- }
- return true;
- } catch (RepositoryException e) {
- // modification failed -> revert all pending changes.
- node.refresh(false);
- throw e;
- }
- } else {
- // nothing changed
- log.debug("Authorizable " + authorizable.getID() + " was not member of " + getID());
- return false;
- }
+ return ((AuthorizableImpl) authorizable).removeFromGroup(this);
}
//--------------------------------------------------------------------------
-
/**
*
* @return
* @throws RepositoryException
*/
- private Collection memberUUIDs() throws RepositoryException {
- Collection tmp = new HashSet();
- if (getNode().hasProperty(P_MEMBERS)) {
- Property prop = getNode().getProperty(P_MEMBERS);
- Value[] val = prop.getValues();
- for (int i = 0; i < val.length; i++) {
- tmp.add(val[i].getString());
+ private Collection getMembers(boolean includeIndirect) throws RepositoryException {
+ // TODO: replace by weak-refs
+ PropertyIterator itr = getNode().getReferences();
+ Collection members = new HashSet((int) itr.getSize());
+ while (itr.hasNext()) {
+ NodeImpl n = (NodeImpl) itr.nextProperty().getParent();
+ if (n.isNodeType(NT_REP_GROUP)) {
+ Group group = create(n, userManager);
+ // only retrieve indirect group-members if the group is not
+ // yet present (detected eventual circular membership).
+ if (members.add(group) && includeIndirect) {
+ members.addAll(((GroupImpl) group).getMembers(true));
+ }
+ } else if (n.isNodeType(NT_REP_USER)) {
+ User user = UserImpl.create(n, userManager);
+ members.add(user);
}
}
- return tmp;
+ return members;
}
/**
- * Since {@link #isMember(Authorizable)} only detects the declared
- * members of this group, this methods is used to avoid cyclic membership
- * declarations.
+ * Since {@link #isMember(Authorizable)} detects declared and inherited
+ * membership this method simply checks if the potential new member is
+ * a group that would in turn have <code>this</code> as a member.
*
* @param newMember
* @return true if the 'newMember' is a group and 'this' is an declared or
@@ -225,13 +200,6 @@
if (newMember.isGroup()) {
Group gr = (Group) newMember;
cyclic = gr.isMember(this);
- if (!cyclic) {
- PrincipalManager pmgr = getSession().getPrincipalManager();
- for (Iterator it = pmgr.getGroupMembership(getPrincipal());
- it.hasNext() && !cyclic;) {
- cyclic = newMember.getPrincipal().equals(it.next());
- }
- }
}
return cyclic;
}
@@ -370,9 +338,8 @@
try {
for (Iterator it = GroupImpl.this.getMembers(); it.hasNext();) {
Authorizable authrz = (Authorizable) it.next();
- // TODO: check again. only add main principal, since
- // 'referees' belong to a different provider and should
- // not be exposed here
+ // NOTE: only add main principal, since 'referees' belong
+ // to a different provider and should not be exposed here
members.add(authrz.getPrincipal());
}
} catch (RepositoryException e) {
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserAccessControlProvider.java Fri Apr 25 08:11:49 2008
@@ -39,6 +39,7 @@
import javax.jcr.Property;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
+import javax.jcr.Value;
import javax.jcr.observation.Event;
import javax.jcr.observation.EventListener;
import javax.jcr.observation.EventIterator;
@@ -55,9 +56,35 @@
* is used to protected the 'security workspace' containing the user and
* group data. It applies special care to make sure that modifying user data
* (e.g. password), group membership and impersonation is properly controlled.
- * The access control policy defined by this provider has the following
+ * <p/>
+ * This provider creates upon initialization the following 2 groups:
+ * <ul>
+ * <li>User administrator</li>
+ * <li>Group administrator</li>
+ * </ul>
+ *
+ * The default access control policy defined by this provider has the following
* characteristics:
- * TODO describe policy.
+ * <ul>
+ * <li>everybody has READ permission to all items,</li>
+ *
+ * <li>every known user is allowed to modify it's own properties except for
+ * her/his group membership,</li>
+ *
+ * <li>members of the 'User administrator' group are allowed to create, modify
+ * and remove those users whose node representation is within the subtree
+ * defined by the node representation of the editing user,</li>
+ *
+ * <li>members of the 'Group administrator' group are allowed to create, modify
+ * and remove groups,</li>
+ *
+ * <li>group membership can only be edited by members of the 'Group administrator'
+ * and the 'User administrator' group. The range of users that can be added
+ * as member to any Group is limited to those that are editable according to
+ * the restrictions described above for the 'User administrator'.</li>
+ * </ul>
+ *
+ * TODO: allow for editing of additional ac that extends the default permission evaluted by this provided.
*/
public class UserAccessControlProvider extends AbstractAccessControlProvider
implements UserConstants {
@@ -261,17 +288,17 @@
implements EventListener {
private final NodeImpl userNode;
- private final boolean isUserAdmin;
- private final boolean isGroupAdmin;
+
+ private boolean isUserAdmin;
+ private boolean isGroupAdmin;
protected CompiledPermissionsImpl(Set principals, NodeImpl userNode) throws RepositoryException {
this.userNode = userNode;
-
isUserAdmin = containsGroup(principals, userAdminGroup);
isGroupAdmin = containsGroup(principals, groupAdminGroup);
int events = Event.PROPERTY_CHANGED | Event.PROPERTY_ADDED | Event.PROPERTY_REMOVED;
- observationMgr.addEventListener(this, events, GROUPS_PATH, true, null, null, false);
+ observationMgr.addEventListener(this, events, USERS_PATH, true, null, null, false);
}
//------------------------------------< AbstractCompiledPermissions >---
@@ -290,7 +317,14 @@
}
Path abs2Path = path.subPath(0, 4);
+ //
if (usersPath.equals(abs2Path)) {
+ /*
+ below the user-tree
+ - determine position of target relative to the node of the editing user
+ - determine if the editing user is user/group-admin
+ - special treatment for rep:groups property
+ */
NodeImpl node = (NodeImpl) getExistingNode(path);
NodeImpl authN = null;
// seek next rep:authorizable parent
@@ -311,25 +345,40 @@
if (authN != null && authN.isNodeType(NT_REP_USER)) {
int relDepth = session.getHierarchyManager().getRelativeDepth(userNode.getNodeId(), authN.getNodeId());
+ boolean isGroupProp = P_GROUPS.equals(path.getNameElement().getName());
+ // only user-admin is allowed to modify users.
+ // for group membership (rep:groups) group-admin is required
+ // in addition.
+ boolean requiredGroups = isUserAdmin;
+ if (requiredGroups && isGroupProp) {
+ requiredGroups = isGroupAdmin;
+ }
switch (relDepth) {
case -1:
// authN is not below the userNode -> can't write anyway.
break;
case 0:
/*
- authN is same node as userNode. 2 cases to distinguish
+ authN is same node as userNode. 3 cases to distinguish
1) user is User-Admin -> R, W
2) user is NOT U-admin but nodeID is its own node.
+ 3) special treatment for rep:group property which can
+ only be modified by group-administrators
*/
- if (isUserAdmin) {
- // principals contain 'user-admin' -> user can modify
- // any item below the user-node.
+ if (requiredGroups) {
+ // principals contain 'user-admin'
+ // -> user can modify items below the user-node except rep:group.
+ // principals contains 'user-admin' + 'group-admin'
+ // -> user can modify rep:group property as well.
perms = Permission.ALL;
if (calcPrivs) {
+ // grant WRITE privilege
+ // note: ac-read/modification is not included
privs |= PrivilegeRegistry.WRITE;
}
- } else if (userNode.isSame(node)) {
+ } else if (userNode.isSame(node) && (!isGroupProp || isGroupAdmin)) {
// user can only read && write his own props
+ // except for the rep:group property.
perms |= (Permission.SET_PROPERTY | Permission.REMOVE_PROPERTY);
if (calcPrivs) {
privs |= PrivilegeRegistry.MODIFY_PROPERTIES;
@@ -342,12 +391,16 @@
1) nodeId points to an authorizable below userNode
2) nodeId points to an auth-folder below some authorizable below userNode.
- In either case user-admin group is required to have write
- permission.
+ In either case user-admin group-membership is
+ required in order to get write permission.
+ group-admin group-membership is required in addition
+ if rep:groups is the target item.
*/
- if (isUserAdmin) {
+ if (requiredGroups) {
perms = Permission.ALL;
if (calcPrivs) {
+ // grant WRITE privilege
+ // note: ac-read/modification is not included
privs |= PrivilegeRegistry.WRITE;
}
}
@@ -357,26 +410,13 @@
/*
below group-tree:
- test if the user is group-administrator.
- - in addition the following special condition must be checked:
-
- if the target id is 'rep:members' the user MUST be member of that
- the containing group in order to have WRITE privilege.
- this required in order to make sure the group-admin cannot
- modify the members of some other groups e.g. administrators.
*/
if (isGroupAdmin) {
- if (P_MEMBERS.equals(path.getNameElement().getName())) {
- if (isMember(userNode, path)) {
- perms |= (Permission.SET_PROPERTY | Permission.REMOVE_PROPERTY);
- }
- } else {
- perms = Permission.ALL;
- if (calcPrivs) {
- privs |= PrivilegeRegistry.WRITE;
- }
+ perms = Permission.ALL;
+ if (calcPrivs) {
+ privs |= PrivilegeRegistry.WRITE;
}
}
-
} // else outside of user/group tree -> read only.
return new Result(perms, privs);
@@ -426,8 +466,34 @@
Event ev = events.nextEvent();
try {
String evPath = ev.getPath();
- if ("rep:members".equals(Text.getName(evPath))) {
- // TODO: add better evaluation.
+ String repGroups = session.getJCRName(UserConstants.P_GROUPS);
+ // TODO: add better evaluation.
+ if (repGroups.equals(Text.getName(evPath)) &&
+ userNode.getPath().equals(Text.getRelativeParent(evPath, 1))) {
+ // recalculate the is...Admin flags
+ switch (ev.getType()) {
+ case Event.PROPERTY_REMOVED:
+ isUserAdmin = false;
+ isGroupAdmin = false;
+ break;
+ case Event.PROPERTY_ADDED:
+ case Event.PROPERTY_CHANGED:
+ // TODO: improve
+ Value[] vs = session.getProperty(evPath).getValues();
+ String princName = session.getJCRName(P_PRINCIPAL_NAME);
+ for (int i = 0; i < vs.length; i++) {
+ Node groupNode = session.getNodeByUUID(vs[i].getString());
+ String pName = groupNode.getProperty(princName).getString();
+ if (userAdminGroup.equals(pName)) {
+ isUserAdmin = true;
+ } else if (groupAdminGroup.equals(pName)) {
+ isGroupAdmin = true;
+ }
+ }
+ break;
+ // default: other events are not relevant.
+ }
+ // invalidate the cached results
clearCache();
// only need to clear the cache once. stop processing
break;
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java Fri Apr 25 08:11:49 2008
@@ -52,7 +52,7 @@
Name P_USERID = NF.create(Name.NS_REP_URI, "userId");
Name P_PASSWORD = NF.create(Name.NS_REP_URI, "password");
- Name P_MEMBERS = NF.create(Name.NS_REP_URI, "members");
+ Name P_GROUPS = NF.create(Name.NS_REP_URI, "groups");
/**
* Name of the user property containing the principal names of those allowed
Modified: jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.cnd
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.cnd?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.cnd (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.cnd Fri Apr 25 08:11:49 2008
@@ -205,6 +205,7 @@
+ * (rep:AuthorizableFolder) = rep:AuthorizableFolder protected version
- rep:principalName (string) protected mandatory
- rep:referees (string) protected multiple
+ - rep:groups (reference) protected multiple < 'rep:Group'
- * (undefined)
- * (undefined) multiple
@@ -217,8 +218,6 @@
- rep:password (string) mandatory
[rep:Group] > rep:Authorizable
- - rep:members (reference) protected multiple
- < 'rep:Authorizable'
[rep:AuthorizableFolder] > nt:base, mix:referenceable
+ * (rep:Authorizable) = rep:User protected version
Modified: jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.xml
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.xml?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.xml (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.xml Fri Apr 25 08:11:49 2008
@@ -476,6 +476,11 @@
</childNodeDefinition>
<propertyDefinition name="rep:principalName" requiredType="String" autoCreated="false" mandatory="true" onParentVersion="COPY" protected="true" multiple="false" />
<propertyDefinition name="rep:referees" requiredType="String" autoCreated="false" mandatory="false" onParentVersion="COPY" protected="true" multiple="true" />
+ <propertyDefinition name="rep:groups" requiredType="Reference" autoCreated="false" onParentVersion="COPY" protected="true" multiple="true">
+ <valueConstraints>
+ <valueConstraint>rep:Group</valueConstraint>
+ </valueConstraints>
+ </propertyDefinition>
<propertyDefinition name="*" requiredType="undefined" autoCreated="false" mandatory="false" onParentVersion="COPY" protected="false" multiple="false" />
<propertyDefinition name="*" requiredType="undefined" autoCreated="false" mandatory="false" onParentVersion="COPY" protected="false" multiple="true" />
</nodeType>
@@ -497,11 +502,6 @@
<supertypes>
<supertype>rep:Authorizable</supertype>
</supertypes>
- <propertyDefinition name="rep:members" requiredType="Reference" autoCreated="false" onParentVersion="COPY" protected="true" multiple="true">
- <valueConstraints>
- <valueConstraint>rep:Authorizable</valueConstraint>
- </valueConstraints>
- </propertyDefinition>
</nodeType>
<nodeType name="rep:AuthorizableFolder" isMixin="false" hasOrderableChildNodes="false" primaryItemName="">
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/AuthorizableTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/AuthorizableTest.java?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/AuthorizableTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/AuthorizableTest.java Fri Apr 25 08:11:49 2008
@@ -194,6 +194,16 @@
}
}
+ public void testDeclaredMemberOf() throws NotExecutableException, RepositoryException {
+ Authorizable auth = getTestUser(superuser);
+
+ Iterator it = auth.declaredMemberOf();
+ while (it.hasNext()) {
+ Object group = it.next();
+ assertTrue(group instanceof Group);
+ }
+ }
+
public void testAddReferee() throws NotExecutableException, RepositoryException {
Authorizable auth = getTestUser(superuser);
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/GroupTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/GroupTest.java?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/GroupTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/GroupTest.java Fri Apr 25 08:11:49 2008
@@ -22,6 +22,8 @@
import javax.jcr.RepositoryException;
import java.util.Iterator;
+import java.util.List;
+import java.util.ArrayList;
/**
* <code>GroupTest</code>...
@@ -30,9 +32,29 @@
private static Logger log = LoggerFactory.getLogger(GroupTest.class);
- private static void assertTrueMemberOfContainsGroup(Authorizable auth, Group gr) throws RepositoryException {
+ private static void assertTrueIsMember(Iterator members, Authorizable auth) throws RepositoryException {
boolean contained = false;
- for (Iterator groups = auth.memberOf(); groups.hasNext() && !contained;) {
+ while (members.hasNext() && !contained) {
+ Object next = members.next();
+ assertTrue(next instanceof Authorizable);
+ contained = ((Authorizable)next).getID().equals(auth.getID());
+ }
+ assertTrue("The given set of members must contain " + auth.getID(), contained);
+ }
+
+ private static void assertFalseIsMember(Iterator members, Authorizable auth) throws RepositoryException {
+ boolean contained = false;
+ while (members.hasNext() && !contained) {
+ Object next = members.next();
+ assertTrue(next instanceof Authorizable);
+ contained = ((Authorizable)next).getID().equals(auth.getID());
+ }
+ assertFalse("The given set of members must not contain " + auth.getID(), contained);
+ }
+
+ private static void assertTrueMemberOfContainsGroup(Iterator groups, Group gr) throws RepositoryException {
+ boolean contained = false;
+ while (groups.hasNext() && !contained) {
Object next = groups.next();
assertTrue(next instanceof Group);
contained = ((Group)next).getID().equals(gr.getID());
@@ -40,9 +62,9 @@
assertTrue("All members of a group must contain that group upon 'memberOf'.", contained);
}
- private static void assertFalseMemberOfContainsGroup(Authorizable auth, Group gr) throws RepositoryException {
+ private static void assertFalseMemberOfContainsGroup(Iterator groups, Group gr) throws RepositoryException {
boolean contained = false;
- for (Iterator groups = auth.memberOf(); groups.hasNext() && !contained;) {
+ while (groups.hasNext() && !contained) {
Object next = groups.next();
assertTrue(next instanceof Group);
contained = ((Group)next).getID().equals(gr.getID());
@@ -55,14 +77,20 @@
assertTrue(gr.isGroup());
}
- public void testGetMembersNotNull() throws NotExecutableException, RepositoryException {
+ public void testGetDeclaredMembers() throws NotExecutableException, RepositoryException {
Group gr = getTestGroup(superuser);
- assertNotNull(gr.getMembers());
+ Iterator it = gr.getDeclaredMembers();
+ assertNotNull(it);
+ while (it.hasNext()) {
+ assertTrue(it.next() instanceof Authorizable);
+ }
}
- public void testGetMembersAreAuthorizable() throws NotExecutableException, RepositoryException {
+ public void testGetMembers() throws NotExecutableException, RepositoryException {
Group gr = getTestGroup(superuser);
- for (Iterator it = gr.getMembers(); it.hasNext();) {
+ Iterator it = gr.getMembers();
+ assertNotNull(it);
+ while (it.hasNext()) {
assertTrue(it.next() instanceof Authorizable);
}
}
@@ -83,7 +111,29 @@
Iterator it = gr.getMembers();
while (it.hasNext()) {
Authorizable auth = (Authorizable) it.next();
- assertTrueMemberOfContainsGroup(auth, gr);
+ assertTrueMemberOfContainsGroup(auth.memberOf(), gr);
+ }
+ }
+
+ public void testGetDeclaredMembersAgainstDeclaredMemberOf() throws NotExecutableException, RepositoryException {
+ Group gr = getTestGroup(superuser);
+
+ Iterator it = gr.getDeclaredMembers();
+ while (it.hasNext()) {
+ Authorizable auth = (Authorizable) it.next();
+ assertTrueMemberOfContainsGroup(auth.declaredMemberOf(), gr);
+ }
+ }
+
+ public void testGetMembersContainsDeclaredMembers() throws NotExecutableException, RepositoryException {
+ Group gr = getTestGroup(superuser);
+ List l = new ArrayList();
+ for (Iterator it = gr.getMembers(); it.hasNext();) {
+ l.add(((Authorizable) it.next()).getID());
+ }
+ for (Iterator it = gr.getDeclaredMembers(); it.hasNext();) {
+ assertTrue("All declared members must also be part of the Iterator " +
+ "returned upon getMembers()",l.contains(((Authorizable) it.next()).getID()));
}
}
@@ -132,9 +182,10 @@
try {
newGroup = userMgr.createGroup(getTestPrincipal());
- assertFalseMemberOfContainsGroup(auth, newGroup);
+ assertFalseMemberOfContainsGroup(auth.memberOf(), newGroup);
assertTrue(newGroup.addMember(auth));
- assertTrueMemberOfContainsGroup(auth, newGroup);
+ assertTrueMemberOfContainsGroup(auth.declaredMemberOf(), newGroup);
+ assertTrueMemberOfContainsGroup(auth.memberOf(), newGroup);
} finally {
if (newGroup != null) {
newGroup.removeMember(auth);
@@ -143,6 +194,60 @@
}
}
+ public void testAddMemberModifiesGetMembers() throws NotExecutableException, RepositoryException {
+ User auth = getTestUser(superuser);
+ Group newGroup = null;
+ try {
+ newGroup = userMgr.createGroup(getTestPrincipal());
+
+ assertFalseIsMember(newGroup.getMembers(), auth);
+ assertFalseIsMember(newGroup.getDeclaredMembers(), auth);
+ assertTrue(newGroup.addMember(auth));
+ assertTrueIsMember(newGroup.getMembers(), auth);
+ assertTrueIsMember(newGroup.getDeclaredMembers(), auth);
+ } finally {
+ if (newGroup != null) {
+ newGroup.removeMember(auth);
+ newGroup.remove();
+ }
+ }
+ }
+
+ public void testIndirectMembers() throws NotExecutableException, RepositoryException {
+ User auth = getTestUser(superuser);
+ Group newGroup = null;
+ Group newGroup2 = null;
+ try {
+ newGroup = userMgr.createGroup(getTestPrincipal());
+ newGroup2 = userMgr.createGroup(getTestPrincipal());
+
+ newGroup.addMember(newGroup2);
+ assertTrue(newGroup.isMember(newGroup2));
+
+ newGroup2.addMember(auth);
+
+ // testuser must not be declared member of 'newGroup'
+ assertFalseIsMember(newGroup.getDeclaredMembers(), auth);
+ assertFalseMemberOfContainsGroup(auth.declaredMemberOf(), newGroup);
+
+ // testuser must however be member of 'newGroup' (indirect).
+ assertTrueIsMember(newGroup.getMembers(), auth);
+ assertTrueMemberOfContainsGroup(auth.memberOf(), newGroup);
+
+ // testuser cannot be removed from 'newGroup'
+ assertFalse(newGroup.removeMember(auth));
+ } finally {
+ if (newGroup != null) {
+ newGroup.removeMember(newGroup2);
+ newGroup.remove();
+ }
+ if (newGroup2 != null) {
+ newGroup2.removeMember(auth);
+ newGroup2.remove();
+ }
+ }
+ }
+
public void testRemoveMemberTwice() throws NotExecutableException, RepositoryException {
User auth = getTestUser(superuser);
Group newGroup = null;
@@ -174,12 +279,14 @@
}
/**
- * Removing a GroupImpl must be possible even if there are still exiting
+ * TODO: uncomment once membership-relation is stored as weak ref.
+ * Removing a GroupImpl must be possible even if there are still existing
* members present.
*
* @throws RepositoryException
* @throws NotExecutableException
*/
+ /*
public void testRemoveGroupIfMemberExist() throws RepositoryException, NotExecutableException {
User auth = getTestUser(superuser);
String newGroupId = null;
@@ -199,4 +306,5 @@
}
}
}
+ */
}
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/AuthorizableImplTest.java Fri Apr 25 08:11:49 2008
@@ -19,9 +19,12 @@
import org.apache.jackrabbit.api.security.user.AbstractUserTest;
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.api.security.user.User;
+import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.core.NodeImpl;
+import org.apache.jackrabbit.core.SessionImpl;
import org.apache.jackrabbit.test.NotExecutableException;
import org.apache.jackrabbit.value.StringValue;
+import org.apache.jackrabbit.spi.commons.conversion.NameResolver;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -29,6 +32,9 @@
import javax.jcr.RepositoryException;
import javax.jcr.Value;
import javax.jcr.nodetype.ConstraintViolationException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Iterator;
/**
* <code>AuthorizableImplTest</code>...
@@ -37,79 +43,87 @@
private static Logger log = LoggerFactory.getLogger(AuthorizableImplTest.class);
+ private List protectedUserProps = new ArrayList();
+ private List protectedGroupProps = new ArrayList();
+
+ protected void setUp() throws Exception {
+ super.setUp();
+
+ if (superuser instanceof SessionImpl) {
+ NameResolver resolver = ((SessionImpl) superuser).getNamePathResolver();
+ protectedUserProps.add(resolver.getJCRName(UserConstants.P_USERID));
+ protectedUserProps.add(resolver.getJCRName(UserConstants.P_GROUPS));
+ protectedUserProps.add(resolver.getJCRName(UserConstants.P_IMPERSONATORS));
+ protectedUserProps.add(resolver.getJCRName(UserConstants.P_PRINCIPAL_NAME));
+ protectedUserProps.add(resolver.getJCRName(UserConstants.P_REFEREES));
+
+ protectedGroupProps.add(resolver.getJCRName(UserConstants.P_PRINCIPAL_NAME));
+ protectedGroupProps.add(resolver.getJCRName(UserConstants.P_REFEREES));
+ } else {
+ throw new NotExecutableException();
+ }
+ }
+
private static void checkProtected(Property prop) throws RepositoryException {
assertTrue(prop.getDefinition().isProtected());
}
- public void testSetSpecialProperties() throws NotExecutableException, RepositoryException {
- Value v = superuser.getValueFactory().createValue("any_value");
- User u = getTestUser(superuser);
+ public void testRemoveAdmin() {
+ String adminID = superuser.getUserID();
try {
- u.setProperty("rep:userId", v);
- fail("changing the user id should fail.");
+ Authorizable admin = userMgr.getAuthorizable(adminID);
+ admin.remove();
+ fail("The admin user cannot be removed.");
} catch (RepositoryException e) {
- // success
+ // OK superuser cannot be removed. not even by the superuser itself.
}
+ }
- try {
- u.setProperty("rep:principalName", v);
- fail("changing the principalName should fail.");
- } catch (RepositoryException e) {
- // success
- }
+ public void testSetSpecialProperties() throws NotExecutableException, RepositoryException {
+ Value v = superuser.getValueFactory().createValue("any_value");
+ User u = getTestUser(superuser);
- try {
- Value[] vs = new Value[] {v};
- u.setProperty("rep:referees", vs);
- fail("changing the referees property should fail.");
- } catch (RepositoryException e) {
- // success
+ for (Iterator it = protectedUserProps.iterator(); it.hasNext();) {
+ String pName = it.next().toString();
+ try {
+ u.setProperty(pName, v);
+ fail("changing the '" +pName+ "' property on a User should fail.");
+ } catch (RepositoryException e) {
+ // success
+ }
}
-
Group g = getTestGroup(superuser);
- try {
- g.setProperty("rep:principalName", v);
- fail("changing the principalName should fail.");
- } catch (RepositoryException e) {
- // success
- }
- try {
- Value[] vs = new Value[] {v};
- g.setProperty("rep:members", vs);
- fail("changing the members property should fail.");
- } catch (RepositoryException e) {
- // success
+ for (Iterator it = protectedGroupProps.iterator(); it.hasNext();) {
+ String pName = it.next().toString();
+ try {
+ u.setProperty(pName, v);
+ fail("changing the '" +pName+ "' property on a Group should fail.");
+ } catch (RepositoryException e) {
+ // success
+ }
}
}
public void testRemoveSpecialProperties() throws NotExecutableException, RepositoryException {
User u = getTestUser(superuser);
- try {
- u.removeProperty("rep:userId");
- fail("removing the user id should fail.");
- } catch (RepositoryException e) {
- // success
- }
-
- try {
- u.removeProperty("rep:principalName");
- fail("removing the principalName should fail.");
- } catch (RepositoryException e) {
- // success
+ for (Iterator it = protectedUserProps.iterator(); it.hasNext();) {
+ String pName = it.next().toString();
+ try {
+ u.removeProperty(pName);
+ fail("removing the '" +pName+ "' property on a User should fail.");
+ } catch (RepositoryException e) {
+ // success
+ }
}
-
Group g = getTestGroup(superuser);
- try {
- g.removeProperty("rep:principalName");
- fail("removing the principalName should fail.");
- } catch (RepositoryException e) {
- // success
- }
- try {
- g.removeProperty("rep:members");
- fail("removing the members property should fail.");
- } catch (RepositoryException e) {
- // success
+ for (Iterator it = protectedGroupProps.iterator(); it.hasNext();) {
+ String pName = it.next().toString();
+ try {
+ u.removeProperty(pName);
+ fail("removing the '" +pName+ "' property on a Group should fail.");
+ } catch (RepositoryException e) {
+ // success
+ }
}
}
@@ -122,6 +136,9 @@
if (n.hasProperty(UserConstants.P_REFEREES)) {
checkProtected(n.getProperty(UserConstants.P_REFEREES));
}
+ if (n.hasProperty(UserConstants.P_GROUPS)) {
+ checkProtected(n.getProperty(UserConstants.P_GROUPS));
+ }
if (n.hasProperty(UserConstants.P_IMPERSONATORS)) {
checkProtected(n.getProperty(UserConstants.P_IMPERSONATORS));
}
@@ -132,8 +149,8 @@
NodeImpl n = gr.getNode();
checkProtected(n.getProperty(UserConstants.P_PRINCIPAL_NAME));
- if (n.hasProperty(UserConstants.P_MEMBERS)) {
- checkProtected(n.getProperty(UserConstants.P_MEMBERS));
+ if (n.hasProperty(UserConstants.P_GROUPS)) {
+ checkProtected(n.getProperty(UserConstants.P_GROUPS));
}
if (n.hasProperty(UserConstants.P_REFEREES)) {
checkProtected(n.getProperty(UserConstants.P_REFEREES));
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/GroupAdministratorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/GroupAdministratorTest.java?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/GroupAdministratorTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/GroupAdministratorTest.java Fri Apr 25 08:11:49 2008
@@ -70,16 +70,16 @@
p = getTestPrincipal();
childUID = userMgr.createUser(p.getName(), buildCredentials(p), p, uPath).getID();
- // make other user a user-administrator:
- Authorizable ua = userMgr.getAuthorizable(UserConstants.GROUP_ADMIN_GROUP_NAME);
- if (ua == null || !ua.isGroup()) {
+ // make other user a group-administrator:
+ Authorizable groupAdmin = userMgr.getAuthorizable(UserConstants.GROUP_ADMIN_GROUP_NAME);
+ if (groupAdmin == null || !groupAdmin.isGroup()) {
throw new NotExecutableException("Cannot execute test. Group-Admin name has been changed by config.");
}
- Group grAdministrators = (Group) ua;
+ Group grAdministrators = (Group) groupAdmin;
grAdministrators.addMember(u);
groupID = grAdministrators.getID();
- // create a session for the other user.
+ // create a session for the grou-admin user.
uSession = helper.getRepository().login(creds);
}
@@ -167,10 +167,71 @@
Authorizable cU = umgr.getAuthorizable(childUID);
Group gr = (Group) umgr.getAuthorizable(groupID);
- // adding and removing a child user from a group the group-admin
- // is member of must succeed.
- assertTrue(gr.addMember(cU));
- assertTrue(gr.removeMember(cU));
+ // adding and removing the child-user as member of a group not
+ // succeed as long editing session is not user-admin.
+ try {
+ assertFalse(gr.addMember(cU));
+ } catch (AccessDeniedException e) {
+ // ok
+ } finally {
+ gr.removeMember(cU);
+ }
+ }
+
+ public void testAddChildToGroup2() throws RepositoryException, NotExecutableException {
+ UserManager umgr = getUserManager(uSession);
+ Authorizable cU = umgr.getAuthorizable(childUID);
+
+ Authorizable auth = umgr.getAuthorizable(UserConstants.USER_ADMIN_GROUP_NAME);
+ if (auth == null || !auth.isGroup()) {
+ throw new NotExecutableException("Cannot execute test. User-Admin name has been changed by config.");
+ }
+ Group userAdmin = (Group)auth;
+ User self = (User) umgr.getAuthorizable(uID);
+ try {
+ assertTrue(userAdmin.addMember(self));
+
+ Group gr = (Group) umgr.getAuthorizable(groupID);
+ assertTrue(gr.addMember(cU));
+ assertTrue(gr.removeMember(cU));
+ } finally {
+ userAdmin.removeMember(self);
+ }
+ }
+
+ public void testAddMembersToCreatedGroup() throws RepositoryException, NotExecutableException {
+ UserManager umgr = getUserManager(uSession);
+ Authorizable auth = umgr.getAuthorizable(UserConstants.USER_ADMIN_GROUP_NAME);
+ if (auth == null || !auth.isGroup()) {
+ throw new NotExecutableException("Cannot execute test. User-Admin name has been changed by config.");
+ }
+ Group userAdmin = (Group) auth;
+ Group testGroup = null;
+ User self = (User) umgr.getAuthorizable(uID);
+ try {
+ // let groupadmin create a new group
+ testGroup = umgr.createGroup(getTestPrincipal(), "/a/b/c/d");
+
+ // editing session adds itself to that group -> must succeed.
+ assertTrue(testGroup.addMember(self));
+
+ // editing session adds itself to user-admin group
+ userAdmin.addMember(self);
+ assertTrue(userAdmin.isMember(self));
+
+ // add child-user to test group
+ Authorizable testUser = umgr.getAuthorizable(childUID);
+ assertFalse(testGroup.isMember(testUser));
+ assertTrue(testGroup.addMember(testUser));
+ } finally {
+ if (testGroup != null) {
+ for (Iterator it = testGroup.getDeclaredMembers(); it.hasNext();) {
+ testGroup.removeMember((Authorizable) it.next());
+ }
+ testGroup.remove();
+ }
+ userAdmin.removeMember(self);
+ }
}
public void testAddMemberToForeignGroup() throws RepositoryException, NotExecutableException {
@@ -199,10 +260,39 @@
Authorizable pU = umgr.getAuthorizable(parentUID);
Group gr = (Group) umgr.getAuthorizable(groupID);
- // adding and removing the 'parent' user to/from a group the group-admin
- // is member of must succeed.
- assertTrue(gr.addMember(pU));
- assertTrue(gr.removeMember(pU));
+ // adding and removing the parent-user as member of a group must
+ // never succeed.
+ try {
+ assertFalse(gr.addMember(pU));
+ } catch (AccessDeniedException e) {
+ // ok
+ } finally {
+ gr.removeMember(pU);
+ }
+
+ // ... even if the editing user becomes member of the user-admin group
+ Group uAdministrators = null;
+ try {
+ Authorizable userAdmin = userMgr.getAuthorizable(UserConstants.USER_ADMIN_GROUP_NAME);
+ if (userAdmin == null || !userAdmin.isGroup()) {
+ throw new NotExecutableException("Cannot execute test. User-Admin name has been changed by config.");
+ }
+ uAdministrators = (Group) userAdmin;
+ uAdministrators.addMember(userMgr.getAuthorizable(uID));
+
+ try {
+ assertFalse(gr.addMember(pU));
+ gr.removeMember(pU);
+ } catch (AccessDeniedException e) {
+ // fine as well.
+ }
+ } finally {
+ // let superuser do the clean up.
+ // remove testuser from u-admin group again.
+ if (uAdministrators != null) {
+ uAdministrators.removeMember(userMgr.getAuthorizable(uID));
+ }
+ }
}
public void testAddOwnAuthorizableAsGroupAdmin() throws RepositoryException, NotExecutableException {
@@ -235,15 +325,9 @@
throw new RepositoryException("Test user is already member -> cannot execute.");
}
- try {
- String msg = "GroupAdmin cannot add its own authorizable to a group she/he is not yet member of.";
- assertFalse(msg, uadminGr.addMember(user));
-
- // didn't throw -> clean up.
- uadminGr.removeMember(user);
- } catch (AccessDeniedException e) {
- // fine as well
- }
+ String msg = "GroupAdmin must be able to add its own authorizable to a group she/he is not yet member of.";
+ assertTrue(msg, uadminGr.addMember(user));
+ assertTrue(msg, uadminGr.removeMember(user));
}
public void testRemoveMembersOfForeignGroup() throws RepositoryException, NotExecutableException {
@@ -263,12 +347,12 @@
Group gr = (Group) getUserManager(uSession).getAuthorizable(nGr.getID());
- // removing any member must fail since uSession is not member of that group.
+ // removing any member must fail unless the testuser is user-admin
Iterator it = gr.getMembers();
if (it.hasNext()) {
Authorizable auth = (Authorizable) it.next();
- String msg = "GroupAdmin cannot remove members of a group itself is not member of.";
+ String msg = "GroupAdmin cannot remove members of other user unless he/she is user-admin.";
assertFalse(msg, gr.removeMember(auth));
} else {
throw new RepositoryException("Must contain members....");
@@ -302,11 +386,11 @@
Group gr = (Group) getUserManager(uSession).getAuthorizable(nGr.getID());
// since only 1 single member -> removal rather than modification.
- // since uSession is not member this must fail.
+ // since uSession is not user-admin this must fail.
for (Iterator it = gr.getMembers(); it.hasNext();) {
Authorizable auth = (Authorizable) it.next();
- String msg = "GroupAdmin cannot remove members of a group itself is not member of.";
+ String msg = "GroupAdmin cannot remove members of groups unless he/she is UserAdmin.";
assertFalse(gr.removeMember(auth));
}
} catch (AccessDeniedException e) {
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserAdministratorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserAdministratorTest.java?rev=651624&r1=651623&r2=651624&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserAdministratorTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserAdministratorTest.java Fri Apr 25 08:11:49 2008
@@ -143,8 +143,6 @@
}
}
- // TODO: uncomment as soon as group-members are stored as weak references
- /*
public void testRemoveHimSelf() throws RepositoryException, NotExecutableException {
UserManager umgr = getUserManager(otherSession);
@@ -156,7 +154,6 @@
// success
}
}
- */
public void testRemoveParentUser() throws RepositoryException, NotExecutableException {
UserManager umgr = getUserManager(otherSession);