SONAR-8898 Check membership when adding user to a group

server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java

@@ -25,6 +25,7 @@ import org.sonar.api.server.ws.WebService.NewAction;
 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.UserDto;
 import org.sonar.db.user.UserGroupDto;
 import org.sonar.server.user.UserSession;
@@ -34,6 +35,7 @@ import static org.sonar.db.permission.OrganizationPermission.ADMINISTER;
 import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_GROUP_ID;
 import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_GROUP_NAME;
 import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_LOGIN;
+import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_ORGANIZATION_KEY;
 import static org.sonar.server.usergroups.ws.GroupWsSupport.defineGroupWsParameters;
 import static org.sonar.server.usergroups.ws.GroupWsSupport.defineLoginWsParameter;
 import static org.sonar.server.ws.WsUtils.checkFound;
@@ -66,7 +68,6 @@ 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);
       userSession.checkLoggedIn().checkPermission(ADMINISTER, groupId.getOrganizationUuid());
@@ -75,6 +76,9 @@ public class AddUserAction implements UserGroupsWsAction {
       UserDto user = dbClient.userDao().selectActiveUserByLogin(dbSession, login);
       checkFound(user, "Could not find a user with login '%s'", login);
 
+      OrganizationDto organization = support.findOrganizationByKey(dbSession, request.param(PARAM_ORGANIZATION_KEY));
+      checkMembership(dbSession, organization, user);
+
       if (!isMemberOf(dbSession, user, groupId)) {
         UserGroupDto membershipDto = new UserGroupDto().setGroupId(groupId.getId()).setUserId(user.getId());
         dbClient.userGroupDao().insert(dbSession, membershipDto);
@@ -88,4 +92,9 @@ public class AddUserAction implements UserGroupsWsAction {
   private boolean isMemberOf(DbSession dbSession, UserDto user, GroupId groupId) {
     return dbClient.groupMembershipDao().selectGroupIdsByUserId(dbSession, user.getId()).contains(groupId.getId());
   }
+
+  private void checkMembership(DbSession dbSession, OrganizationDto organization, UserDto user) {
+    dbClient.organizationMemberDao().select(dbSession, organization.getUuid(), user.getId())
+      .orElseThrow(() -> new IllegalArgumentException(format("User '%s' is not member of organization '%s'", user.getLogin(), organization.getKey())));
+  }
 }

server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/AddUserActionTest.java

@@ -19,7 +19,6 @@
  */
 package org.sonar.server.usergroups.ws;
 
-import java.util.List;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -29,8 +28,6 @@ import org.sonar.db.DbTester;
 import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
-import org.sonar.db.user.UserMembershipDto;
-import org.sonar.db.user.UserMembershipQuery;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.exceptions.UnauthorizedException;
@@ -65,6 +62,7 @@ public class AddUserActionTest {
   public void add_user_to_group_referenced_by_its_id() throws Exception {
     GroupDto group = db.users().insertGroup();
     UserDto user = db.users().insertUser();
+    addUserAsMemberOfDefaultOrganization(user);
     loginAsAdminOnDefaultOrganization();
 
     newRequest()
@@ -80,6 +78,7 @@ public class AddUserActionTest {
   public void add_user_to_group_referenced_by_its_name() throws Exception {
     GroupDto group = db.users().insertGroup();
     UserDto user = db.users().insertUser();
+    addUserAsMemberOfDefaultOrganization(user);
     loginAsAdminOnDefaultOrganization();
 
     newRequest()
@@ -96,6 +95,7 @@ public class AddUserActionTest {
     OrganizationDto org = db.organizations().insert();
     GroupDto group = db.users().insertGroup(org, "a-group");
     UserDto user = db.users().insertUser("user_login");
+    addUserAsMemberOfOrganization(org, user);
     loginAsAdmin(org);
 
     newRequest()
@@ -115,6 +115,7 @@ public class AddUserActionTest {
     GroupDto users = db.users().insertGroup(defaultOrg, "users");
     UserDto user = db.users().insertUser("my-admin");
     db.users().insertMember(users, user);
+    addUserAsMemberOfDefaultOrganization(user);
     loginAsAdminOnDefaultOrganization();
 
     newRequest()
@@ -131,6 +132,7 @@ public class AddUserActionTest {
     GroupDto users = db.users().insertGroup();
     UserDto user = db.users().insertUser();
     db.users().insertMember(users, user);
+    addUserAsMemberOfDefaultOrganization(user);
     loginAsAdminOnDefaultOrganization();
 
     newRequest()
@@ -149,6 +151,8 @@ public class AddUserActionTest {
     UserDto user1 = db.users().insertUser();
     UserDto user2 = db.users().insertUser();
     db.users().insertMember(users, user1);
+    addUserAsMemberOfDefaultOrganization(user1);
+    addUserAsMemberOfDefaultOrganization(user2);
     loginAsAdminOnDefaultOrganization();
 
     newRequest()
@@ -165,6 +169,7 @@ public class AddUserActionTest {
   public void fail_if_group_does_not_exist() throws Exception {
     UserDto user = db.users().insertUser();
     loginAsAdminOnDefaultOrganization();
+    addUserAsMemberOfDefaultOrganization(user);
 
     expectedException.expect(NotFoundException.class);
 
@@ -191,19 +196,13 @@ public class AddUserActionTest {
   public void fail_if_not_administrator() throws Exception {
     GroupDto group = db.users().insertGroup();
     UserDto user = db.users().insertUser();
+    addUserAsMemberOfDefaultOrganization(user);
 
     expectedException.expect(UnauthorizedException.class);
 
     executeRequest(group, user);
   }
 
-  private void executeRequest(GroupDto groupDto, UserDto userDto) throws Exception {
-    newRequest()
-      .setParam("id", groupDto.getId().toString())
-      .setParam("login", userDto.getLogin())
-      .execute();
-  }
-
   @Test
   public void fail_if_administrator_of_another_organization() throws Exception {
     OrganizationDto org1 = db.organizations().insert();
@@ -221,6 +220,33 @@ public class AddUserActionTest {
       .execute();
   }
 
+  @Test
+  public void fail_to_add_user_to_group_when_user_is_not_member_of_given_organization() throws Exception {
+    OrganizationDto org = db.organizations().insert(organizationDto -> organizationDto.setKey("Organization key"));
+    GroupDto group = db.users().insertGroup(org, "a-group");
+    UserDto user = db.users().insertUser("user_login");
+    OrganizationDto otherOrganization = db.organizations().insert();
+    addUserAsMemberOfOrganization(otherOrganization, user);
+    loginAsAdmin(org);
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("User 'user_login' is not member of organization 'Organization key'");
+
+    newRequest()
+      .setParam(PARAM_ORGANIZATION_KEY, org.getKey())
+      .setParam(PARAM_GROUP_NAME, group.getName())
+      .setParam(PARAM_LOGIN, user.getLogin())
+      .execute()
+      .assertNoContent();
+  }
+
+  private void executeRequest(GroupDto groupDto, UserDto userDto) throws Exception {
+    newRequest()
+      .setParam("id", groupDto.getId().toString())
+      .setParam("login", userDto.getLogin())
+      .execute();
+  }
+
   private WsTester.TestRequest newRequest() {
     return ws.newPostRequest("api/user_groups", "add_user");
   }
@@ -237,12 +263,12 @@ public class AddUserActionTest {
     return new GroupWsSupport(db.getDbClient(), defaultOrganizationProvider);
   }
 
-  private boolean isUserInGroup(UserDto userDto, GroupDto groupDto) {
-    List<UserMembershipDto> members = db.getDbClient().groupMembershipDao()
-      .selectMembers(db.getSession(), UserMembershipQuery.builder().groupId(groupDto.getId()).membership(UserMembershipQuery.IN).build(), 0, Integer.MAX_VALUE);
-    return members
-      .stream()
-      .anyMatch(dto -> dto.getLogin().equals(userDto.getLogin()));
+  private void addUserAsMemberOfDefaultOrganization(UserDto user) {
+    db.organizations().addMember(db.getDefaultOrganization(), user);
+  }
+
+  private void addUserAsMemberOfOrganization(OrganizationDto organization, UserDto user) {
+    db.organizations().addMember(organization, user);
   }
 
 }