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 2011/03/03 13:14:27 UTC
svn commit: r1076596 - in /jackrabbit/trunk/jackrabbit-core/src:
main/java/org/apache/jackrabbit/core/security/user/ImpersonationImpl.java
test/java/org/apache/jackrabbit/core/security/user/ImpersonationImplTest.java
Author: angela
Date: Thu Mar 3 12:14:26 2011
New Revision: 1076596
URL: http://svn.apache.org/viewvc?rev=1076596&view=rev
Log:
minor improvement:
- replace test for AdminPrincipal
- remove SystemPrincipal usage
Modified:
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/ImpersonationImpl.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/ImpersonationImplTest.java
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/ImpersonationImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/ImpersonationImpl.java?rev=1076596&r1=1076595&r2=1076596&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/ImpersonationImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/ImpersonationImpl.java Thu Mar 3 12:14:26 2011
@@ -17,6 +17,7 @@
package org.apache.jackrabbit.core.security.user;
import java.security.Principal;
+import java.security.acl.Group;
import java.util.HashSet;
import java.util.Set;
@@ -30,8 +31,6 @@ import org.apache.jackrabbit.api.securit
import org.apache.jackrabbit.api.security.user.Impersonation;
import org.apache.jackrabbit.core.NodeImpl;
import org.apache.jackrabbit.core.PropertyImpl;
-import org.apache.jackrabbit.core.security.SystemPrincipal;
-import org.apache.jackrabbit.core.security.principal.AdminPrincipal;
import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
import org.apache.jackrabbit.core.security.principal.PrincipalIteratorAdapter;
import org.apache.jackrabbit.value.StringValue;
@@ -82,16 +81,16 @@ class ImpersonationImpl implements Imper
* @see Impersonation#grantImpersonation(Principal)
*/
public synchronized boolean grantImpersonation(Principal principal) throws RepositoryException {
- if (principal instanceof AdminPrincipal || principal instanceof SystemPrincipal) {
- log.warn("Admin and System principal are already granted impersonation.");
- return false;
- }
-
// make sure the given principals belong to an existing authorizable
Authorizable auth = user.userManager.getAuthorizable(principal);
if (auth == null || auth.isGroup()) {
- log.warn("Cannot grant impersonation to a principal that is a Group " +
- "or an unknown Authorizable.");
+ log.warn("Cannot grant impersonation to a principal that is a Group or an unknown Authorizable.");
+ return false;
+ }
+
+ // make sure the given principal doesn't refer to the admin user.
+ if (user.userManager.isAdminId(auth.getID())) {
+ log.warn("Admin principal is already granted impersonation.");
return false;
}
@@ -115,11 +114,6 @@ class ImpersonationImpl implements Imper
* @see Impersonation#revokeImpersonation(Principal)
*/
public synchronized boolean revokeImpersonation(Principal principal) throws RepositoryException {
- if (principal instanceof AdminPrincipal || principal instanceof SystemPrincipal) {
- log.warn("Admin and System principal are always granted impersonation.");
- return false;
- }
-
boolean revoked = false;
String pName = principal.getName();
@@ -138,24 +132,31 @@ class ImpersonationImpl implements Imper
if (subject == null) {
return false;
}
- //shortcut admin/system -> always allowed
- if (!subject.getPrincipals(AdminPrincipal.class).isEmpty()
- || !subject.getPrincipals(SystemPrincipal.class).isEmpty()) {
- return true;
- }
Set<String> principalNames = new HashSet<String>();
for (Principal p : subject.getPrincipals()) {
principalNames.add(p.getName());
}
- boolean allows = false;
- try {
- Set<String> impersonators = getImpersonatorNames();
- allows = impersonators.removeAll(principalNames);
- } catch (RepositoryException e) {
- // should never get here
- log.debug(e.getMessage());
+ boolean allows;
+ Set<String> impersonators = getImpersonatorNames();
+ allows = impersonators.removeAll(principalNames);
+
+ if (!allows) {
+ // check if subject belongs to administrator user
+ for (Principal p : subject.getPrincipals()) {
+ if (p instanceof Group) {
+ continue;
+ }
+ Authorizable a = userManager.getAuthorizable(p);
+ if (user.equals(a)) {
+ allows = false;
+ break;
+ } else if (a != null && userManager.isAdminId(a.getID())) {
+ allows = true;
+ break;
+ }
+ }
}
return allows;
}
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/ImpersonationImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/ImpersonationImplTest.java?rev=1076596&r1=1076595&r2=1076596&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/ImpersonationImplTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/ImpersonationImplTest.java Thu Mar 3 12:14:26 2011
@@ -21,6 +21,8 @@ import org.apache.jackrabbit.api.securit
import org.apache.jackrabbit.api.security.user.User;
import org.apache.jackrabbit.api.security.user.UserManager;
import org.apache.jackrabbit.api.security.user.Impersonation;
+import org.apache.jackrabbit.core.security.SystemPrincipal;
+import org.apache.jackrabbit.core.security.principal.AdminPrincipal;
import org.apache.jackrabbit.test.NotExecutableException;
import javax.jcr.AccessDeniedException;
@@ -41,6 +43,7 @@ public class ImpersonationImplTest exten
private String otherUID;
+ @Override
protected void setUp() throws Exception {
super.setUp();
@@ -67,6 +70,7 @@ public class ImpersonationImplTest exten
otherUID = u2.getID();
}
+ @Override
protected void tearDown() throws Exception {
try {
uSession.logout();
@@ -120,4 +124,45 @@ public class ImpersonationImplTest exten
}
assertFalse("A simple user may not add itself as impersonator to another user.", other.getImpersonation().allows(buildSubject(p)));
}
+
+ public void testAdminPrincipalAsImpersonator() throws RepositoryException, NotExecutableException {
+ String adminId = superuser.getUserID();
+ Authorizable a = userMgr.getAuthorizable(adminId);
+ if (a == null || a.isGroup() || !((User) a).isAdmin()) {
+ throw new NotExecutableException(adminId + " is not administators ID");
+ }
+
+ Principal adminPrincipal = new AdminPrincipal(adminId);
+
+ // admin cannot be add/remove to set of impersonators of 'u' but is
+ // always allowed to impersonate that user.
+ User u = (User) userMgr.getAuthorizable(uID);
+ Impersonation impersonation = u.getImpersonation();
+
+ assertFalse(impersonation.grantImpersonation(adminPrincipal));
+ assertFalse(impersonation.revokeImpersonation(adminPrincipal));
+ assertTrue(impersonation.allows(buildSubject(adminPrincipal)));
+
+ // same if the impersonation object of the admin itself is used.
+ // however impersonation is not allowed....
+ Impersonation adminImpersonation = ((User) a).getImpersonation();
+
+ assertFalse(adminImpersonation.grantImpersonation(adminPrincipal));
+ assertFalse(adminImpersonation.revokeImpersonation(adminPrincipal));
+ assertFalse(adminImpersonation.allows(buildSubject(adminPrincipal)));
+ }
+
+ public void testSystemPrincipalAsImpersonator() throws RepositoryException {
+ Principal systemPrincipal = new SystemPrincipal();
+ assertNull(userMgr.getAuthorizable(systemPrincipal));
+
+ // system cannot be add/remove to set of impersonators of 'u' nor
+ // should it be allowed to impersonate a given user...
+ User u = (User) userMgr.getAuthorizable(uID);
+ Impersonation impersonation = u.getImpersonation();
+
+ assertFalse(impersonation.grantImpersonation(systemPrincipal));
+ assertFalse(impersonation.revokeImpersonation(systemPrincipal));
+ assertFalse(impersonation.allows(buildSubject(systemPrincipal)));
+ }
}
\ No newline at end of file