SONAR-9022 Prevent adding/removing user on 'sonar-users' group

diff --git a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java
index 8a8eb9fab5e8e5aa78d6224e79baaa87c4a44181..179ca9b503a43fe84b6c4e7eb3289d5566ba87a1 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.usergroups.ws;
 
+import org.sonar.api.server.ws.Change;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService.NewAction;
@@ -26,6 +27,7 @@ import org.sonar.api.server.ws.WebService.NewController;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.organization.OrganizationDto;
+import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.db.user.UserGroupDto;
 import org.sonar.server.user.UserSession;
@@ -60,7 +62,8 @@ public class AddUserAction implements UserGroupsWsAction {
         "Requires the following permission: 'Administer System'.", PARAM_GROUP_ID, PARAM_GROUP_NAME))
       .setHandler(this)
       .setPost(true)
-      .setSince("5.2");
+      .setSince("5.2")
+      .setChangelog(new Change("6.4", "It's no longer possible to add a user to the default group"));
 
     defineGroupWsParameters(action);
     defineLoginWsParameter(action);
@@ -69,7 +72,7 @@ public class AddUserAction implements UserGroupsWsAction {
   @Override
   public void handle(Request request, Response response) throws Exception {
     try (DbSession dbSession = dbClient.openSession(false)) {
-      GroupId groupId = support.findGroup(dbSession, request);
+      GroupDto groupId = support.findGroupDto(dbSession, request);
       userSession.checkLoggedIn().checkPermission(ADMINISTER, groupId.getOrganizationUuid());
 
       String login = request.mandatoryParam(PARAM_LOGIN);
@@ -78,6 +81,7 @@ public class AddUserAction implements UserGroupsWsAction {
 
       OrganizationDto organization = support.findOrganizationByKey(dbSession, request.param(PARAM_ORGANIZATION_KEY));
       checkMembership(dbSession, organization, user);
+      support.checkGroupIsNotDefault(groupId);
 
       if (!isMemberOf(dbSession, user, groupId)) {
         UserGroupDto membershipDto = new UserGroupDto().setGroupId(groupId.getId()).setUserId(user.getId());
@@ -89,7 +93,7 @@ public class AddUserAction implements UserGroupsWsAction {
     }
   }
 
-  private boolean isMemberOf(DbSession dbSession, UserDto user, GroupId groupId) {
+  private boolean isMemberOf(DbSession dbSession, UserDto user, GroupDto groupId) {
     return dbClient.groupMembershipDao().selectGroupIdsByUserId(dbSession, user.getId()).contains(groupId.getId());
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/RemoveUserAction.java b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/RemoveUserAction.java
index 7282a6f886cf062315d03b9459d9334efebc81a0..568af7a7677d7a48a270348a4abb58a62fdee9b9 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/RemoveUserAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/RemoveUserAction.java
@@ -26,6 +26,7 @@ import org.sonar.api.server.ws.WebService.NewController;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.permission.OrganizationPermission;
+import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.user.UserSession;
 
@@ -70,8 +71,9 @@ public class RemoveUserAction implements UserGroupsWsAction {
     userSession.checkLoggedIn();
 
     try (DbSession dbSession = dbClient.openSession(false)) {
-      GroupId group = support.findGroup(dbSession, request);
+      GroupDto group = support.findGroupDto(dbSession, request);
       userSession.checkPermission(OrganizationPermission.ADMINISTER, group.getOrganizationUuid());
+      support.checkGroupIsNotDefault(group);
 
       String login = request.mandatoryParam(PARAM_LOGIN);
       UserDto user = getUser(dbSession, login);
@@ -88,7 +90,7 @@ public class RemoveUserAction implements UserGroupsWsAction {
   /**
    * Ensure that there are still users with admin global permission if user is removed from the group.
    */
-  private void ensureLastAdminIsNotRemoved(DbSession dbSession, GroupId group, UserDto user) {
+  private void ensureLastAdminIsNotRemoved(DbSession dbSession, GroupDto group, UserDto user) {
     int remainingAdmins = dbClient.authorizationDao().countUsersWithGlobalPermissionExcludingGroupMember(dbSession,
       group.getOrganizationUuid(), OrganizationPermission.ADMINISTER.getKey(), group.getId(), user.getId());
     checkRequest(remainingAdmins > 0, "The last administrator user cannot be removed");

diff --git a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/AddUserActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/AddUserActionTest.java
index e6968116c98405fa74e13242c89e63d2954a86df..62abd9d946730acef0b2306a2d9c1fb568ae6676 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/AddUserActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/AddUserActionTest.java
@@ -256,6 +256,23 @@ public class AddUserActionTest {
       .execute();
   }
 
+  @Test
+  public void fail_to_add_user_to_default_group() throws Exception {
+    OrganizationDto organization = db.organizations().insert();
+    UserDto user = db.users().insertUser();
+    addUserAsMemberOfOrganization(organization, user);
+    GroupDto group = db.users().insertGroup(organization, "sonar-users");
+    loginAsAdmin(organization);
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Default group 'sonar-users' cannot be used to perform this action");
+
+    newRequest()
+      .setParam("id", Integer.toString(group.getId()))
+      .setParam(PARAM_LOGIN, user.getLogin())
+      .execute();
+  }
+
   private void executeRequest(GroupDto groupDto, UserDto userDto) throws Exception {
     newRequest()
       .setParam("id", groupDto.getId().toString())

diff --git a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/RemoveUserActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/RemoveUserActionTest.java
index 1a3d41503fc7651054884d9b7ddc968a3127145e..39525e1ab0c325ca44851b50eae9452a7348db3b 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/RemoveUserActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/RemoveUserActionTest.java
@@ -225,6 +225,23 @@ public class RemoveUserActionTest {
       .execute();
   }
 
+  @Test
+  public void fail_to_remove_user_from_default_group() throws Exception {
+    OrganizationDto organization = db.organizations().insert();
+    UserDto user = db.users().insertUser();
+    GroupDto group = db.users().insertGroup(organization, "sonar-users");
+    db.users().insertMember(group, user);
+    loginAsAdmin(organization);
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Default group 'sonar-users' cannot be used to perform this action");
+
+    newRequest()
+      .setParam("id", Integer.toString(group.getId()))
+      .setParam(PARAM_LOGIN, user.getLogin())
+      .execute();
+  }
+
   private TestRequest newRequest() {
     return ws.newRequest();
   }