SONAR-8258 check admin permission on related organization

in api/user_groups/update

diff --git a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UsersAction.java b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UsersAction.java
index 604fbc11ce18148c667f53c7ccf614778ad5901c..be4828be17e2da3a8c418f769930abc04167ab30 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UsersAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UsersAction.java
@@ -28,7 +28,6 @@ import org.sonar.api.server.ws.WebService.Param;
 import org.sonar.api.server.ws.WebService.SelectionMode;
 import org.sonar.api.utils.Paging;
 import org.sonar.api.utils.text.JsonWriter;
-import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.user.GroupMembershipQuery;
@@ -37,6 +36,7 @@ import org.sonar.db.user.UserMembershipQuery;
 import org.sonar.server.user.UserSession;
 
 import static org.sonar.api.utils.Paging.forPageIndex;
+import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN;
 import static org.sonar.server.usergroups.ws.GroupWsSupport.defineGroupWsParameters;
 
 public class UsersAction implements UserGroupsWsAction {
@@ -71,8 +71,6 @@ public class UsersAction implements UserGroupsWsAction {
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    userSession.checkLoggedIn().checkPermission(GlobalPermissions.SYSTEM_ADMIN);
-
     int pageSize = request.mandatoryParamAsInt(Param.PAGE_SIZE);
     int page = request.mandatoryParamAsInt(Param.PAGE);
     String queryString = request.param(Param.TEXT_QUERY);
@@ -80,6 +78,7 @@ public class UsersAction implements UserGroupsWsAction {
 
     try (DbSession dbSession = dbClient.openSession(false)) {
       GroupId group = support.findGroup(dbSession, request);
+      userSession.checkOrganizationPermission(group.getOrganizationUuid(), SYSTEM_ADMIN);
 
       UserMembershipQuery query = UserMembershipQuery.builder()
         .groupId(group.getId())

diff --git a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UpdateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UpdateActionTest.java
index 3d83df44ee3981ceae64ff48a63d1bd2ce67dc4a..d95a846e321917aec2db1d2479ff3e5c464f9b31 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UpdateActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UpdateActionTest.java
@@ -183,7 +183,7 @@ public class UpdateActionTest {
   }
 
   @Test
-  public void fails_if_admin_of_another_organization() throws Exception {
+  public void fails_if_admin_of_another_organization_only() throws Exception {
     OrganizationDto org1 = OrganizationTesting.insert(db, newOrganizationDto());
     OrganizationDto org2 = OrganizationTesting.insert(db, newOrganizationDto());
     GroupDto group = db.users().insertGroup(org1, "group1");

diff --git a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UsersActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UsersActionTest.java
index af0c949661b9a9bbad2142207edecf007fb76a22..0871bb192d395e4cc10a484168470a930d9068ee 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UsersActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UsersActionTest.java
@@ -22,11 +22,14 @@ package org.sonar.server.usergroups.ws;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.sonar.api.server.ws.WebService.Param;
 import org.sonar.api.server.ws.WebService.SelectionMode;
 import org.sonar.api.utils.System2;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbTester;
+import org.sonar.db.organization.OrganizationDto;
+import org.sonar.db.organization.OrganizationTesting;
 import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.exceptions.ForbiddenException;
@@ -37,11 +40,14 @@ import org.sonar.server.ws.WsTester;
 import org.sonar.server.ws.WsTester.TestRequest;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.db.organization.OrganizationTesting.newOrganizationDto;
 import static org.sonar.db.user.UserTesting.newUserDto;
-import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_GROUP_NAME;
+import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_GROUP_ID;
 
 public class UsersActionTest {
 
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
   @Rule
   public DbTester db = DbTester.create(System2.INSTANCE);
   @Rule
@@ -57,28 +63,50 @@ public class UsersActionTest {
         db.getDbClient(),
         userSession,
         groupSupport)));
-    userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
-
   }
 
-  @Test(expected = NotFoundException.class)
-  public void fail_if_unknown_user() throws Exception {
+  @Test
+  public void fail_if_unknown_group_id() throws Exception {
+    loginAsAdminOnDefaultOrganization();
+
+    expectedException.expect(NotFoundException.class);
+    expectedException.expectMessage("No group with id '42'");
+
     newUsersRequest()
       .setParam("id", "42")
       .setParam("login", "john").execute();
   }
 
-  @Test(expected = ForbiddenException.class)
-  public void fail_if_missing_permission() throws Exception {
+  @Test
+  public void fail_if_not_admin_of_organization() throws Exception {
+    GroupDto group = db.users().insertGroup();
     userSession.login("not-admin");
+
+    expectedException.expect(ForbiddenException.class);
+
     newUsersRequest()
-      .setParam("id", "42")
+      .setParam("id", group.getId().toString())
       .setParam("login", "john").execute();
   }
 
   @Test
-  public void empty_users() throws Exception {
-    GroupDto group = db.users().insertGroup(defaultOrganizationProvider.getDto(), "a group");
+  public void fail_if_admin_of_other_organization_only() throws Exception {
+    OrganizationDto org1 = OrganizationTesting.insert(db, newOrganizationDto());
+    OrganizationDto org2 = OrganizationTesting.insert(db, newOrganizationDto());
+    GroupDto group = db.users().insertGroup(org1, "the-group");
+    loginAsAdmin(org2);
+
+    expectedException.expect(ForbiddenException.class);
+
+    newUsersRequest()
+      .setParam("id", group.getId().toString())
+      .setParam("login", "john").execute();
+  }
+
+  @Test
+  public void group_has_no_users() throws Exception {
+    GroupDto group = db.users().insertGroup();
+    loginAsAdminOnDefaultOrganization();
 
     newUsersRequest()
       .setParam("login", "john")
@@ -88,11 +116,12 @@ public class UsersActionTest {
   }
 
   @Test
-  public void all_users() throws Exception {
-    GroupDto group = db.users().insertGroup(defaultOrganizationProvider.getDto(), "a group");
+  public void return_members_by_group_id() throws Exception {
+    GroupDto group = db.users().insertGroup();
     UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada").setName("Ada Lovelace"));
     db.users().insertMember(group, user1);
     db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper"));
+    loginAsAdminOnDefaultOrganization();
 
     newUsersRequest()
       .setParam("id", group.getId().toString())
@@ -102,15 +131,48 @@ public class UsersActionTest {
   }
 
   @Test
-  public void all_users_by_group_name() throws Exception {
+  public void references_group_by_its_name() throws Exception {
+    OrganizationDto org = OrganizationTesting.insert(db, newOrganizationDto());
+    GroupDto group = db.users().insertGroup(org, "the-group");
+    UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada").setName("Ada Lovelace"));
+    db.users().insertMember(group, user1);
+    db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper"));
+    loginAsAdmin(org);
+
+    newUsersRequest()
+      .setParam("organization", org.getKey())
+      .setParam("name", group.getName())
+      .setParam(Param.SELECTED, SelectionMode.ALL.value())
+      .execute()
+      .assertJson(getClass(), "all.json");
+  }
+
+  @Test
+  public void references_group_in_default_organization_by_its_name() throws Exception {
+    GroupDto group = db.users().insertGroup();
+    UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada").setName("Ada Lovelace"));
+    db.users().insertMember(group, user1);
+    db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper"));
+    loginAsAdminOnDefaultOrganization();
+
+    newUsersRequest()
+      .setParam("name", group.getName())
+      .setParam(Param.SELECTED, SelectionMode.ALL.value())
+      .execute()
+      .assertJson(getClass(), "all.json");
+  }
+
+  @Test
+  public void filter_members_by_name() throws Exception {
     GroupDto group = db.users().insertGroup(defaultOrganizationProvider.getDto(), "a group");
     UserDto adaLovelace = db.users().insertUser(newUserDto().setLogin("ada").setName("Ada Lovelace"));
     UserDto graceHopper = db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper"));
     db.users().insertMember(group, adaLovelace);
     db.users().insertMember(group, graceHopper);
+    loginAsAdminOnDefaultOrganization();
 
     String response = newUsersRequest()
-      .setParam(PARAM_GROUP_NAME, group.getName())
+      .setParam(PARAM_GROUP_ID, group.getId().toString())
       .execute().outputAsString();
 
     assertThat(response).contains("Ada Lovelace", "Grace Hopper");
@@ -122,6 +184,7 @@ public class UsersActionTest {
     UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada").setName("Ada Lovelace"));
     db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper"));
     db.users().insertMember(group, user1);
+    loginAsAdminOnDefaultOrganization();
 
     newUsersRequest()
       .setParam("id", group.getId().toString())
@@ -137,10 +200,11 @@ public class UsersActionTest {
 
   @Test
   public void deselected_users() throws Exception {
-    GroupDto group = db.users().insertGroup(defaultOrganizationProvider.getDto(), "a group");
+    GroupDto group = db.users().insertGroup();
     UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada").setName("Ada Lovelace"));
     db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper"));
     db.users().insertMember(group, user1);
+    loginAsAdminOnDefaultOrganization();
 
     newUsersRequest()
       .setParam("id", group.getId().toString())
@@ -151,10 +215,11 @@ public class UsersActionTest {
 
   @Test
   public void paging() throws Exception {
-    GroupDto group = db.users().insertGroup(defaultOrganizationProvider.getDto(), "a group");
+    GroupDto group = db.users().insertGroup();
     UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada").setName("Ada Lovelace"));
     db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper"));
     db.users().insertMember(group, user1);
+    loginAsAdminOnDefaultOrganization();
 
     newUsersRequest()
       .setParam("id", group.getId().toString())
@@ -174,10 +239,11 @@ public class UsersActionTest {
 
   @Test
   public void filtering() throws Exception {
-    GroupDto group = db.users().insertGroup(defaultOrganizationProvider.getDto(), "a group");
+    GroupDto group = db.users().insertGroup();
     UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada").setName("Ada Lovelace"));
     db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper"));
     db.users().insertMember(group, user1);
+    loginAsAdminOnDefaultOrganization();
 
     newUsersRequest()
       .setParam("id", group.getId().toString())
@@ -197,4 +263,11 @@ public class UsersActionTest {
     return wsTester.newGetRequest("api/user_groups", "users");
   }
 
+  private void loginAsAdminOnDefaultOrganization() {
+    loginAsAdmin(db.getDefaultOrganization());
+  }
+
+  private void loginAsAdmin(OrganizationDto org) {
+    userSession.login().addOrganizationPermission(org.getUuid(), GlobalPermissions.SYSTEM_ADMIN);
+  }
 }