[CST-5306] Added security check in EPersonAuthority

dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java

@@ -7,6 +7,9 @@
  */
 package org.dspace.authorize;
 
+import static org.dspace.app.util.AuthorizeUtil.canCollectionAdminManageAccounts;
+import static org.dspace.app.util.AuthorizeUtil.canCommunityAdminManageAccounts;
+
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -900,6 +903,16 @@ public class AuthorizeServiceImpl implements AuthorizeService {
         return discoverResult.getTotalSearchResults();
     }
 
+    @Override
+    public boolean isAccountManager(Context context) {
+        try {
+            return (canCommunityAdminManageAccounts() && isCommunityAdmin(context)
+                || canCollectionAdminManageAccounts() && isCollectionAdmin(context));
+        } catch (SQLException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
     private boolean performCheck(Context context, String query) throws SQLException {
         if (context.getCurrentUser() == null) {
             return false;

dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java

@@ -592,4 +592,12 @@ public interface AuthorizeService {
      */
     long countAdminAuthorizedCollection(Context context, String query)
         throws SearchServiceException, SQLException;
+
+    /**
+     * Returns true if the current user can manage accounts.
+     *
+     * @param  context context with the current user
+     * @return         true if the current user can manage accounts
+     */
+    boolean isAccountManager(Context context);
 }

dspace-api/src/main/java/org/dspace/content/authority/EPersonAuthority.java

@@ -9,16 +9,20 @@ package org.dspace.content.authority;
 
 import java.sql.SQLException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.UUID;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.dspace.authorize.factory.AuthorizeServiceFactory;
+import org.dspace.authorize.service.AuthorizeService;
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.factory.EPersonServiceFactory;
 import org.dspace.eperson.service.EPersonService;
 import org.dspace.util.UUIDUtils;
+import org.dspace.web.ContextUtil;
 
 /**
  * Implementation of {@link ChoiceAuthority} based on EPerson. Allows you to set
@@ -38,6 +42,8 @@ public class EPersonAuthority implements ChoiceAuthority {
 
     private EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService();
 
+    private AuthorizeService authorizeService = AuthorizeServiceFactory.getInstance().getAuthorizeService();
+
     @Override
     public Choices getBestMatch(String text, String locale) {
         return getMatches(text, 0, 2, locale);
@@ -45,18 +51,14 @@ public class EPersonAuthority implements ChoiceAuthority {
 
     @Override
     public Choices getMatches(String text, int start, int limit, String locale) {
-        Context context = null;
         if (limit <= 0) {
             limit = 20;
         }
-        context = new Context();
+
-        List<EPerson> ePersons = null;
+        Context context = getContext();
-        try {
+
-            ePersons = ePersonService.search(context, text, start, limit);
+        List<EPerson> ePersons = searchEPersons(context, text, start, limit);
-        } catch (SQLException e) {
+
-            log.error(e.getMessage(), e);
-            throw new RuntimeException(e.getMessage(), e);
-        }
         List<Choice> choiceList = new ArrayList<Choice>();
         for (EPerson eperson : ePersons) {
             choiceList.add(new Choice(eperson.getID().toString(), eperson.getFullName(), eperson.getFullName()));
@@ -74,7 +76,7 @@ public class EPersonAuthority implements ChoiceAuthority {
             return null;
         }
 
-        Context context = new Context();
+        Context context = getContext();
         try {
             EPerson ePerson = ePersonService.find(context, uuid);
             return ePerson != null ? ePerson.getFullName() : null;
@@ -85,6 +87,34 @@ public class EPersonAuthority implements ChoiceAuthority {
 
     }
 
+    private List<EPerson> searchEPersons(Context context, String text, int start, int limit) {
+
+        if (!isCurrentUserAdminOrAccessGroupManager(context)) {
+            return Collections.emptyList();
+        }
+
+        try {
+            return ePersonService.search(context, text, start, limit);
+        } catch (SQLException e) {
+            log.error(e.getMessage(), e);
+            throw new RuntimeException(e.getMessage(), e);
+        }
+
+    }
+
+    private Context getContext() {
+        Context context = ContextUtil.obtainCurrentRequestContext();
+        return context != null ? context : new Context();
+    }
+
+    private boolean isCurrentUserAdminOrAccessGroupManager(Context context) {
+        try {
+            return authorizeService.isAdmin(context) || authorizeService.isAccountManager(context);
+        } catch (SQLException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
     @Override
     public String getPluginInstanceName() {
         return authorityName;

dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java

@@ -21,7 +21,6 @@ import javax.servlet.http.HttpServletRequest;
 import org.apache.commons.lang3.StringUtils;
 import org.dspace.app.rest.login.PostLoggedInAction;
 import org.dspace.app.rest.utils.ContextUtil;
-import org.dspace.app.util.AuthorizeUtil;
 import org.dspace.authenticate.AuthenticationMethod;
 import org.dspace.authenticate.service.AuthenticationService;
 import org.dspace.authorize.service.AuthorizeService;
@@ -198,20 +197,15 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider
         EPerson eperson = context.getCurrentUser();
         if (eperson != null) {
             boolean isAdmin = false;
-            boolean isCommunityAdmin = false;
-            boolean isCollectionAdmin = false;
             try {
                 isAdmin = authorizeService.isAdmin(context, eperson);
-                isCommunityAdmin = authorizeService.isCommunityAdmin(context);
-                isCollectionAdmin = authorizeService.isCollectionAdmin(context);
             } catch (SQLException e) {
                 log.error("SQL error while checking for admin rights", e);
             }
 
             if (isAdmin) {
                 authorities.add(new SimpleGrantedAuthority(ADMIN_GRANT));
-            } else if ((isCommunityAdmin && AuthorizeUtil.canCommunityAdminManageAccounts())
+            } else if (authorizeService.isAccountManager(context)) {
-                       || (isCollectionAdmin && AuthorizeUtil.canCollectionAdminManageAccounts())) {
                 authorities.add(new SimpleGrantedAuthority(MANAGE_ACCESS_GROUP));
             }
 

dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonAuthorityIT.java

@@ -9,6 +9,7 @@ package org.dspace.app.rest;
 
 import static org.dspace.app.rest.matcher.VocabularyMatcher.matchVocabularyEntry;
 import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.empty;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@@ -38,7 +39,7 @@ public class EPersonAuthorityIT extends AbstractControllerIntegrationTest {
         String thirdEPersonId = createEPerson("Luca", "Bollini");
         context.restoreAuthSystemState();
 
-        String token = getAuthToken(eperson.getEmail(), password);
+        String token = getAuthToken(admin.getEmail(), password);
         getClient(token).perform(get("/api/submission/vocabularies/EPersonAuthority/entries")
             .param("filter", "Luca"))
             .andExpect(status().isOk())
@@ -65,7 +66,7 @@ public class EPersonAuthorityIT extends AbstractControllerIntegrationTest {
         String thirdEPersonId = createEPerson("Luca", "Bollini");
         context.restoreAuthSystemState();
 
-        String token = getAuthToken(eperson.getEmail(), password);
+        String token = getAuthToken(admin.getEmail(), password);
         getClient(token).perform(get("/api/submission/vocabularies/EPersonAuthority/entries")
             .param("filter", "Giamminonni"))
             .andExpect(status().isOk())
@@ -91,7 +92,7 @@ public class EPersonAuthorityIT extends AbstractControllerIntegrationTest {
         String secondEPersonId = createEPerson("Andrea", "Bollini");
         context.restoreAuthSystemState();
 
-        String token = getAuthToken(eperson.getEmail(), password);
+        String token = getAuthToken(admin.getEmail(), password);
         getClient(token).perform(get("/api/submission/vocabularies/EPersonAuthority/entries")
             .param("filter", firstEPersonId))
             .andExpect(status().isOk())
@@ -108,6 +109,38 @@ public class EPersonAuthorityIT extends AbstractControllerIntegrationTest {
 
     }
 
+    @Test
+    public void testEPersonAuthorityWithAnonymousUser() throws Exception {
+
+        context.turnOffAuthorisationSystem();
+        createEPerson("Luca", "Giamminonni");
+        createEPerson("Andrea", "Bollini");
+        context.restoreAuthSystemState();
+
+        getClient().perform(get("/api/submission/vocabularies/EPersonAuthority/entries")
+            .param("filter", "Luca"))
+            .andExpect(status().isUnauthorized());
+
+    }
+
+    @Test
+    public void testEPersonAuthorityWithNotAdminUser() throws Exception {
+
+        context.turnOffAuthorisationSystem();
+        createEPerson("Luca", "Giamminonni");
+        createEPerson("Andrea", "Bollini");
+        createEPerson("Luca", "Bollini");
+        context.restoreAuthSystemState();
+
+        String token = getAuthToken(eperson.getEmail(), password);
+        getClient(token).perform(get("/api/submission/vocabularies/EPersonAuthority/entries")
+            .param("filter", "Luca"))
+            .andExpect(status().isOk())
+            .andExpect(jsonPath("$._embedded.entries", empty()))
+            .andExpect(jsonPath("$.page.totalElements", Matchers.is(0)));
+
+    }
+
     private String createEPerson(String firstName, String lastName) throws SQLException {
         return EPersonBuilder.createEPerson(context)
             .withNameInMetadata(firstName, lastName)