diff --git a/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java b/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java index e20353a402..bacc60f14e 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java @@ -260,7 +260,7 @@ public class AuthorizeServiceImpl implements AuthorizeService // if user is an Admin on this object DSpaceObject adminObject = useInheritance ? serviceFactory.getDSpaceObjectService(o).getAdminObject(c, o, action) : null; - if (isAdmin(c, adminObject)) + if (isAdmin(c, e, adminObject)) { c.cacheAuthorizedAction(o, action, e, true, null); return true; @@ -322,7 +322,7 @@ public class AuthorizeServiceImpl implements AuthorizeService } if ((rp.getGroup() != null) - && (groupService.isMember(c, rp.getGroup()))) + && (groupService.isMember(c, e, rp.getGroup()))) { // group was set, and eperson is a member // of that group @@ -370,9 +370,14 @@ public class AuthorizeServiceImpl implements AuthorizeService @Override public boolean isAdmin(Context c, DSpaceObject o) throws SQLException { + return this.isAdmin(c, c.getCurrentUser(), o); + } + @Override + public boolean isAdmin(Context c, EPerson e, DSpaceObject o) throws SQLException + { // return true if user is an Administrator - if (isAdmin(c)) + if (isAdmin(c, e)) { return true; } @@ -382,7 +387,7 @@ public class AuthorizeServiceImpl implements AuthorizeService return false; } - Boolean cachedResult = c.getCachedAuthorizationResult(o, Constants.ADMIN, c.getCurrentUser()); + Boolean cachedResult = c.getCachedAuthorizationResult(o, Constants.ADMIN, e); if (cachedResult != null) { return cachedResult.booleanValue(); } @@ -397,18 +402,18 @@ public class AuthorizeServiceImpl implements AuthorizeService // check policies for date validity if (resourcePolicyService.isDateValid(rp)) { - if (rp.getEPerson() != null && rp.getEPerson().equals(c.getCurrentUser())) + if (rp.getEPerson() != null && rp.getEPerson().equals(e)) { - c.cacheAuthorizedAction(o, Constants.ADMIN, c.getCurrentUser(), true, rp); + c.cacheAuthorizedAction(o, Constants.ADMIN, e, true, rp); return true; // match } if ((rp.getGroup() != null) - && (groupService.isMember(c, rp.getGroup()))) + && (groupService.isMember(c, e, rp.getGroup()))) { // group was set, and eperson is a member // of that group - c.cacheAuthorizedAction(o, Constants.ADMIN, c.getCurrentUser(), true, rp); + c.cacheAuthorizedAction(o, Constants.ADMIN, e, true, rp); return true; } } @@ -427,12 +432,12 @@ public class AuthorizeServiceImpl implements AuthorizeService DSpaceObject parent = serviceFactory.getDSpaceObjectService(o).getParentObject(c, o); if (parent != null) { - boolean admin = isAdmin(c, parent); - c.cacheAuthorizedAction(o, Constants.ADMIN, c.getCurrentUser(), admin, null); + boolean admin = isAdmin(c, e, parent); + c.cacheAuthorizedAction(o, Constants.ADMIN, e, admin, null); return admin; } - c.cacheAuthorizedAction(o, Constants.ADMIN, c.getCurrentUser(), false, null); + c.cacheAuthorizedAction(o, Constants.ADMIN, e, false, null); return false; } @@ -455,6 +460,24 @@ public class AuthorizeServiceImpl implements AuthorizeService return groupService.isMember(c, Group.ADMIN); } } + + @Override + public boolean isAdmin(Context c, EPerson e) throws SQLException + { + // if we're ignoring authorization, user is member of admin + if (c.ignoreAuthorization()) + { + return true; + } + + if (e == null) + { + return false; // anonymous users can't be admins.... + } else + { + return groupService.isMember(c, e, Group.ADMIN); + } + } public boolean isCommunityAdmin(Context c) throws SQLException { diff --git a/dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java b/dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java index dfffab8c0b..9b5d30ebe1 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java +++ b/dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java @@ -181,11 +181,28 @@ public interface AuthorizeService { */ public boolean isAdmin(Context c, DSpaceObject o) throws SQLException; + /** + * Check to see if a specific user is an Administrator of a given object + * within DSpace. Always return {@code true} if the user is a System + * Admin + * + * @param c current context + * @param e the user to check + * @param o current DSpace Object, if null the call will be + * equivalent to a call to the isAdmin(Context c) + * method + * @return {@code true} if the user has administrative privileges on the + * given DSpace object + * @throws SQLException if database error + */ + public boolean isAdmin(Context c, EPerson e, DSpaceObject o) throws SQLException; + /** * Check to see if the current user is a System Admin. Always return - * {@code true} if c.ignoreAuthorization is set. Anonymous users - * can't be Admins (EPerson set to NULL) + * {@code true} if c.ignoreAuthorization is set. If no EPerson is + * logged in and context.getCurrentUser() returns null, this method + * returns false as anonymous users can never be administrators. * * @param c current context * @return {@code true} if user is an admin or ignore authorization @@ -194,6 +211,17 @@ public interface AuthorizeService { * An exception that provides information on a database access error or other errors. */ public boolean isAdmin(Context c) throws SQLException; + + /** + * Check to see if a specific user is system admin. Always return + * {@code true} if c.ignoreAuthorization is set. + * + * @param c current context + * @return {@code true} if user is an admin or ignore authorization + * flag set + * @throws SQLException if database error + */ + public boolean isAdmin(Context c, EPerson e) throws SQLException; public boolean isCommunityAdmin(Context c) throws SQLException; diff --git a/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java b/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java index 4737f56e98..07c0261bb7 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java @@ -158,44 +158,55 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl implements @Override public boolean isMember(Context context, Group group) throws SQLException { - EPerson currentUser = context.getCurrentUser(); + return isMember(context, context.getCurrentUser(), group); + } - if(group == null) { - return false; - - // special, everyone is member of group 0 (anonymous) - } else if (StringUtils.equals(group.getName(), Group.ANONYMOUS)) { - return true; - } else if(currentUser != null) { - - Boolean cachedGroupMembership = context.getCachedGroupMembership(group, currentUser); - if(cachedGroupMembership != null) { - return cachedGroupMembership.booleanValue(); - - } else if(CollectionUtils.isNotEmpty(context.getSpecialGroups())) { - Set allMemberGroups = allMemberGroupsSet(context, currentUser); - boolean result = allMemberGroups.contains(group); - - context.cacheGroupMembership(group, currentUser, result); - return result; - } else { - //lookup eperson in normal groups and subgroups - boolean result = epersonInGroup(context, group.getName(), currentUser); - context.cacheGroupMembership(group, currentUser, result); - return result; - } - } else { - // Check also for anonymous users if IP authentication used - List specialGroups = context.getSpecialGroups(); - if(CollectionUtils.isNotEmpty(specialGroups)) { - for(Group specialGroup : specialGroups){ - if (StringUtils.equals(specialGroup.getName(), group.getName())) { - return true; - } - } - } + @Override + public boolean isMember(Context context, EPerson ePerson, Group group) + throws SQLException + { + if (group == null) + { return false; } + + // special, everyone is member of the anonymous group + if (StringUtils.equals(group.getName(), Group.ANONYMOUS)) { + return true; + } + + Boolean cachedGroupMembership = context.getCachedGroupMembership(group, ePerson); + if(cachedGroupMembership != null) + { + return cachedGroupMembership.booleanValue(); + } + + // if we have special groups they may be set by IP authentication + // check special groups for anonymous users and if the user is currently + // logged in + if (CollectionUtils.isNotEmpty(context.getSpecialGroups()) + && (ePerson == null || ePerson.equals(context.getCurrentUser()))) + { + for (Group specialGroup : context.getSpecialGroups()) + { + if (specialGroup.equals(group) || group2GroupCacheDAO.find(context, group, specialGroup) != null) + { + context.cacheGroupMembership(group, ePerson, true); + return true; + } + } + } + + // we checked all possible memberships for anonymous users + if (ePerson == null) + { + return false; + } + + //lookup eperson in normal groups and subgroups + boolean result = epersonInGroup(context, group.getName(), ePerson); + context.cacheGroupMembership(group, ePerson, result); + return result; } @Override @@ -203,6 +214,11 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl implements return isMember(context, findByName(context, groupName)); } + @Override + public boolean isMember(final Context context, EPerson eperson, final String groupName) throws SQLException { + return isMember(context, eperson, findByName(context, groupName)); + } + @Override public List allMemberGroups(Context context, EPerson ePerson) throws SQLException { return new ArrayList<>(allMemberGroupsSet(context, ePerson)); diff --git a/dspace-api/src/main/java/org/dspace/eperson/dao/Group2GroupCacheDAO.java b/dspace-api/src/main/java/org/dspace/eperson/dao/Group2GroupCacheDAO.java index 9fc0422dca..2c39aae5d5 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/dao/Group2GroupCacheDAO.java +++ b/dspace-api/src/main/java/org/dspace/eperson/dao/Group2GroupCacheDAO.java @@ -14,7 +14,6 @@ import org.dspace.eperson.Group2GroupCache; import java.sql.SQLException; import java.util.List; -import java.util.Set; /** * Database Access Object interface class for the Group2GroupCache object. @@ -27,7 +26,7 @@ public interface Group2GroupCacheDAO extends GenericDAO { public List findByParent(Context context, Group group) throws SQLException; - public List findByChildren(Context context, Set groups) throws SQLException; + public List findByChildren(Context context, Iterable groups) throws SQLException; public Group2GroupCache find(Context context, Group parent, Group child) throws SQLException; diff --git a/dspace-api/src/main/java/org/dspace/eperson/dao/impl/Group2GroupCacheDAOImpl.java b/dspace-api/src/main/java/org/dspace/eperson/dao/impl/Group2GroupCacheDAOImpl.java index a27689d75e..6e0a6c34d1 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/dao/impl/Group2GroupCacheDAOImpl.java +++ b/dspace-api/src/main/java/org/dspace/eperson/dao/impl/Group2GroupCacheDAOImpl.java @@ -44,7 +44,7 @@ public class Group2GroupCacheDAOImpl extends AbstractHibernateDAO findByChildren(Context context, Set groups) throws SQLException { + public List findByChildren(Context context, Iterable groups) throws SQLException { Criteria criteria = createCriteria(context, Group2GroupCache.class); Disjunction orDisjunction = Restrictions.or(); diff --git a/dspace-api/src/main/java/org/dspace/eperson/service/GroupService.java b/dspace-api/src/main/java/org/dspace/eperson/service/GroupService.java index 0f5b617de1..74e894926f 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/service/GroupService.java +++ b/dspace-api/src/main/java/org/dspace/eperson/service/GroupService.java @@ -134,7 +134,8 @@ public interface GroupService extends DSpaceObjectService, DSpaceObjectLe /** * fast check to see if an eperson is a member called with eperson id, does * database lookup without instantiating all of the epeople objects and is - * thus a static method + * thus a static method. This method uses context.getCurrentUser() as + * eperson whos membership should be checked. * * @param context * context @@ -145,6 +146,34 @@ public interface GroupService extends DSpaceObjectService, DSpaceObjectLe */ public boolean isMember(Context context, String groupName) throws SQLException; + /** + * fast check to see if an eperson is a member called with eperson id, does + * database lookup without instantiating all of the epeople objects and is + * thus a static method. The eperson whos membership should be checked must + * be defined as method attribute. + * + * @param context + * context + * @param groupName + * the name of the group to check + * @return true or false + * @throws SQLException if database error + */ + public boolean isMember(Context context, EPerson epersonToCheck, String groupName) throws SQLException; + + /** + * fast check to see if an eperson is a member called with eperson id, does + * database lookup without instantiating all of the epeople objects and is + * thus a static method + * + * @param context DSpace context object. + * @param eperson EPerson whos membership should be checked. + * @param group The group to check against. + * @return true or false + * @throws SQLException if database error + */ + public boolean isMember(Context context, EPerson eperson, Group group) throws SQLException; + /** * Get all of the groups that an eperson is a member of. * diff --git a/dspace-api/src/test/java/org/dspace/content/InstallItemTest.java b/dspace-api/src/test/java/org/dspace/content/InstallItemTest.java index bc5eecae95..2ff28bc599 100644 --- a/dspace-api/src/test/java/org/dspace/content/InstallItemTest.java +++ b/dspace-api/src/test/java/org/dspace/content/InstallItemTest.java @@ -15,6 +15,7 @@ import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.*; import org.dspace.core.Constants; import org.dspace.core.Context; +import org.dspace.eperson.EPerson; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -147,6 +148,7 @@ public class InstallItemTest extends AbstractUnitTest Constants.ADD); result = false; // Allow full Admin perms authorizeService.isAdmin((Context) any); result = true; + authorizeService.isAdmin((Context) any, (EPerson) any); result = true; }}; String handle = "123456789/56789";