SONAR-6465 Do not require admin permission, but display groups only when admin

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java
index 311b31555e59a8d33da3c1485dbbd207a7d50724..31de4ec616369d8deb69f693181cb02e96575c8f 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java
@@ -67,7 +67,7 @@ public class SearchAction implements UsersWsAction {
   @Override
   public void define(WebService.NewController controller) {
     WebService.NewAction action = controller.createAction("search")
-      .setDescription("Get a list of active users. Requires Administer System permission.")
+      .setDescription("Get a list of active users. Administer System permission is required to show the 'groups' field.")
       .setSince("3.6")
       .setHandler(this)
       .setResponseExample(getClass().getResource("example-search.json"));
@@ -81,8 +81,6 @@ public class SearchAction implements UsersWsAction {
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    userSession.checkLoggedIn().checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN);
-
     SearchOptions options = new SearchOptions()
       .setPage(request.mandatoryParamAsInt(Param.PAGE), request.mandatoryParamAsInt(Param.PAGE_SIZE));
     List<String> fields = request.paramAsStrings(Param.FIELDS);
@@ -130,7 +128,7 @@ public class SearchAction implements UsersWsAction {
   }
 
   private void writeGroupsIfNeeded(JsonWriter json, Collection<String> groups, @Nullable List<String> fields) {
-    if (fieldIsWanted(FIELD_GROUPS, fields)) {
+    if (fieldIsWanted(FIELD_GROUPS, fields) && userSession.hasGlobalPermission(GlobalPermissions.SYSTEM_ADMIN)) {
       json.name(FIELD_GROUPS).beginArray();
       for (String groupName : groups) {
         json.value(groupName);

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java
index 85b5b575321f3ae7bb965874abf84440dbccb6a6..08f2bd4a802bb5fba9ded9919c3af23e361b3a91 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java
@@ -42,7 +42,6 @@ import org.sonar.core.user.UserDto;
 import org.sonar.core.user.UserGroupDto;
 import org.sonar.server.db.DbClient;
 import org.sonar.server.es.EsTester;
-import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.user.db.GroupDao;
 import org.sonar.server.user.db.UserDao;
@@ -101,7 +100,6 @@ public class SearchActionTest {
 
   @Test
   public void search_empty() throws Exception {
-    loginAsAdmin();
     tester.newGetRequest("api/users", "search").execute().assertJson(getClass(), "empty.json");
   }
 
@@ -109,7 +107,6 @@ public class SearchActionTest {
   public void search_without_parameters() throws Exception {
     injectUsers(5);
 
-    loginAsAdmin();
     tester.newGetRequest("api/users", "search").execute().assertJson(getClass(), "five_users.json");
   }
 
@@ -117,7 +114,6 @@ public class SearchActionTest {
   public void search_with_query() throws Exception {
     injectUsers(5);
 
-    loginAsAdmin();
     tester.newGetRequest("api/users", "search").setParam("q", "user-1").execute().assertJson(getClass(), "user_one.json");
   }
 
@@ -125,7 +121,6 @@ public class SearchActionTest {
   public void search_with_paging() throws Exception {
     injectUsers(10);
 
-    loginAsAdmin();
     tester.newGetRequest("api/users", "search").setParam(Param.PAGE_SIZE, "5").execute().assertJson(getClass(), "page_one.json");
     tester.newGetRequest("api/users", "search").setParam(Param.PAGE_SIZE, "5").setParam(Param.PAGE, "2").execute().assertJson(getClass(), "page_two.json");
   }
@@ -134,21 +129,19 @@ public class SearchActionTest {
   public void search_with_fields() throws Exception {
     injectUsers(1);
 
-    loginAsAdmin();
-
     assertThat(tester.newGetRequest("api/users", "search").execute().outputAsString())
       .contains("login")
       .contains("name")
       .contains("email")
       .contains("scmAccounts")
-      .contains("groups");
+      .doesNotContain("groups");
 
     assertThat(tester.newGetRequest("api/users", "search").setParam(Param.FIELDS, "").execute().outputAsString())
       .contains("login")
       .contains("name")
       .contains("email")
       .contains("scmAccounts")
-      .contains("groups");
+      .doesNotContain("groups");
 
     assertThat(tester.newGetRequest("api/users", "search").setParam(Param.FIELDS, "login").execute().outputAsString())
       .contains("login")
@@ -164,6 +157,22 @@ public class SearchActionTest {
       .contains("scmAccounts")
       .doesNotContain("groups");
 
+    assertThat(tester.newGetRequest("api/users", "search").setParam(Param.FIELDS, "groups").execute().outputAsString())
+      .doesNotContain("login")
+      .doesNotContain("name")
+      .doesNotContain("email")
+      .doesNotContain("scmAccounts")
+      .doesNotContain("groups");
+
+    loginAsAdmin();
+
+    assertThat(tester.newGetRequest("api/users", "search").execute().outputAsString())
+      .contains("login")
+      .contains("name")
+      .contains("email")
+      .contains("scmAccounts")
+      .contains("groups");
+
     assertThat(tester.newGetRequest("api/users", "search").setParam(Param.FIELDS, "groups").execute().outputAsString())
       .doesNotContain("login")
       .doesNotContain("name")
@@ -186,12 +195,6 @@ public class SearchActionTest {
     tester.newGetRequest("api/users", "search").execute().assertJson(getClass(), "user_with_groups.json");
   }
 
-  @Test(expected = ForbiddenException.class)
-  public void fail_on_missing_permission() throws Exception {
-    userSession.login("not-admin");
-    tester.newGetRequest("api/users", "search").execute();
-  }
-
   private List<UserDto> injectUsers(int numberOfUsers) throws Exception {
     List<UserDto> userDtos = Lists.newArrayList();
     long createdAt = System.currentTimeMillis();

diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json
index 86b57c314cafbd6ec44037e0b5ecb5cb456099f4..88a6fec9ecb943b1008cbe522c7dbc802d2f7306 100644 (file)
--- a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json
+++ b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json
@@ -9,8 +9,7 @@
       "email": "user-0@mail.com",
       "scmAccounts": [
         "user-0"
-      ],
-      "groups": []
+      ]
     },
     {
       "login": "user-1",
@@ -18,8 +17,7 @@
       "email": "user-1@mail.com",
       "scmAccounts": [
         "user-1"
-      ],
-      "groups": []
+      ]
     },
     {
       "login": "user-2",
@@ -27,8 +25,7 @@
       "email": "user-2@mail.com",
       "scmAccounts": [
         "user-2"
-      ],
-      "groups": []
+      ]
     },
     {
       "login": "user-3",
@@ -36,8 +33,7 @@
       "email": "user-3@mail.com",
       "scmAccounts": [
         "user-3"
-      ],
-      "groups": []
+      ]
     },
     {
       "login": "user-4",
@@ -45,8 +41,7 @@
       "email": "user-4@mail.com",
       "scmAccounts": [
         "user-4"
-      ],
-      "groups": []
+      ]
     }
   ]
 }