SONAR-8900 Check membership when bulk assigning issue

server/sonar-server/src/main/java/org/sonar/server/issue/Action.java

@@ -29,6 +29,7 @@ import org.sonar.api.issue.condition.Condition;
 import org.sonar.api.server.ServerSide;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.IssueChangeContext;
+import org.sonar.db.component.ComponentDto;
 import org.sonar.server.user.UserSession;
 
 import static com.google.common.collect.Lists.newArrayList;
@@ -78,6 +79,8 @@ public abstract class Action {
     DefaultIssue issue();
 
     IssueChangeContext issueChangeContext();
+
+    ComponentDto project();
   }
 
 }

server/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java

@@ -20,7 +20,10 @@
 package org.sonar.server.issue;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Map;
+import java.util.Set;
+import javax.annotation.Nullable;
 import org.sonar.api.issue.condition.IsUnResolved;
 import org.sonar.api.server.ServerSide;
 import org.sonar.core.issue.DefaultIssue;
@@ -38,39 +41,55 @@ public class AssignAction extends Action {
 
   public static final String ASSIGN_KEY = "assign";
   public static final String ASSIGNEE_PARAMETER = "assignee";
-  public static final String VERIFIED_ASSIGNEE = "verifiedAssignee";
+  private static final String VERIFIED_ASSIGNEE = "verifiedAssignee";
+  private static final String ASSIGNEE_ORGANIZATIONS = "assigneeOrganizationUuids";
 
   private final DbClient dbClient;
-  private final IssueFieldsSetter issueUpdater;
+  private final IssueFieldsSetter issueFieldsSetter;
 
-  public AssignAction(DbClient dbClient, IssueFieldsSetter issueUpdater) {
+  public AssignAction(DbClient dbClient, IssueFieldsSetter issueFieldsSetter) {
     super(ASSIGN_KEY);
     this.dbClient = dbClient;
-    this.issueUpdater = issueUpdater;
+    this.issueFieldsSetter = issueFieldsSetter;
     super.setConditions(new IsUnResolved());
   }
 
   @Override
   public boolean verify(Map<String, Object> properties, Collection<DefaultIssue> issues, UserSession userSession) {
-    String assignee = getAssigneeValue(properties);
-    properties.put(VERIFIED_ASSIGNEE, isNullOrEmpty(assignee) ? null : getUser(assignee));
+    String assigneeLogin = getAssigneeValue(properties);
+    UserDto assignee = isNullOrEmpty(assigneeLogin) ? null : getUser(assigneeLogin);
+    properties.put(VERIFIED_ASSIGNEE, assignee);
+    properties.put(ASSIGNEE_ORGANIZATIONS, loadUserOrganizations(assignee));
     return true;
   }
 
-  @Override
-  public boolean execute(Map<String, Object> properties, Context context) {
-    checkArgument(properties.containsKey(VERIFIED_ASSIGNEE), "Assignee is missing from the execution parameters");
-    UserDto assignee = (UserDto) properties.get(VERIFIED_ASSIGNEE);
-    return issueUpdater.assign(context.issue(), assignee, context.issueChangeContext());
-  }
-
   private static String getAssigneeValue(Map<String, Object> properties) {
     return (String) properties.get(ASSIGNEE_PARAMETER);
   }
 
   private UserDto getUser(String assigneeKey) {
     try (DbSession dbSession = dbClient.openSession(false)) {
-      return checkFound(dbClient.userDao().selectByLogin(dbSession, assigneeKey), "Unknown user: %s", assigneeKey);
+      return checkFound(dbClient.userDao().selectActiveUserByLogin(dbSession, assigneeKey), "Unknown user: %s", assigneeKey);
     }
   }
+
+  private Set<String> loadUserOrganizations(@Nullable UserDto assignee) {
+    if (assignee == null) {
+      return Collections.emptySet();
+    }
+    try (DbSession dbSession = dbClient.openSession(false)) {
+      return dbClient.organizationMemberDao().selectOrganizationUuidsByUser(dbSession, assignee.getId());
+    }
+  }
+
+  @Override
+  public boolean execute(Map<String, Object> properties, Context context) {
+    checkArgument(properties.containsKey(VERIFIED_ASSIGNEE), "Assignee is missing from the execution parameters");
+    UserDto assignee = (UserDto) properties.get(VERIFIED_ASSIGNEE);
+    return isAssigneeMemberOfIssueOrganization(assignee, properties, context) && issueFieldsSetter.assign(context.issue(), assignee, context.issueChangeContext());
+  }
+
+  private static boolean isAssigneeMemberOfIssueOrganization(@Nullable UserDto assignee, Map<String, Object> properties, Context context) {
+    return assignee == null || ((Set<String>) properties.get(ASSIGNEE_ORGANIZATIONS)).contains(context.project().getOrganizationUuid());
+  }
 }

server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java

@@ -197,19 +197,19 @@ public class BulkChangeAction implements IssuesWsAction {
     return bulkChangeData -> {
       BulkChangeResult result = new BulkChangeResult(bulkChangeData.issues.size());
       IssueChangeContext issueChangeContext = IssueChangeContext.createUser(new Date(system2.now()), userSession.getLogin());
-      
+
       List<DefaultIssue> items = bulkChangeData.issues.stream()
         .filter(bulkChange(issueChangeContext, bulkChangeData, result))
         .collect(Collectors.toList());
       issueStorage.save(items);
-      items.stream().forEach(sendNotification(issueChangeContext, bulkChangeData));
+      items.forEach(sendNotification(issueChangeContext, bulkChangeData));
       return result;
     };
   }
 
   private Predicate<DefaultIssue> bulkChange(IssueChangeContext issueChangeContext, BulkChangeData bulkChangeData, BulkChangeResult result) {
     return issue -> {
-      ActionContext actionContext = new ActionContext(issue, issueChangeContext);
+      ActionContext actionContext = new ActionContext(issue, issueChangeContext, bulkChangeData.projectsByUuid.get(issue.projectUuid()));
       bulkChangeData.getActionsWithoutComment().forEach(applyAction(actionContext, bulkChangeData, result));
       addCommentIfNeeded(actionContext, bulkChangeData);
       return result.success.contains(issue.key());
@@ -260,10 +260,12 @@ public class BulkChangeAction implements IssuesWsAction {
   public static class ActionContext implements Action.Context {
     private final DefaultIssue issue;
     private final IssueChangeContext changeContext;
+    private final ComponentDto project;
 
-    public ActionContext(DefaultIssue issue, IssueChangeContext changeContext) {
+    public ActionContext(DefaultIssue issue, IssueChangeContext changeContext, ComponentDto project) {
       this.issue = issue;
       this.changeContext = changeContext;
+      this.project = project;
     }
 
     @Override
@@ -275,6 +277,11 @@ public class BulkChangeAction implements IssuesWsAction {
     public IssueChangeContext issueChangeContext() {
       return changeContext;
     }
+
+    @Override
+    public ComponentDto project() {
+      return project;
+    }
   }
 
   private class BulkChangeData {

server/sonar-server/src/test/java/org/sonar/server/issue/AssignActionTest.java

@@ -20,9 +20,11 @@
 package org.sonar.server.issue;
 
 import com.google.common.collect.ImmutableMap;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -30,6 +32,8 @@ import org.sonar.api.issue.Issue;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.db.DbTester;
+import org.sonar.db.component.ComponentDto;
+import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.issue.ws.BulkChangeAction;
@@ -38,10 +42,11 @@ import org.sonar.server.tester.UserSessionRule;
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.sonar.db.user.UserTesting.newUserDto;
 
 public class AssignActionTest {
 
+  private static final String ISSUE_CURRENT_ASSIGNEE = "current assignee";
+
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
 
@@ -49,45 +54,79 @@ public class AssignActionTest {
   public UserSessionRule userSession = UserSessionRule.standalone();
 
   @Rule
-  public DbTester dbTester = DbTester.create();
-
-  private IssueFieldsSetter issueUpdater = new IssueFieldsSetter();
+  public DbTester db = DbTester.create();
 
   private IssueChangeContext issueChangeContext = IssueChangeContext.createUser(new Date(), "emmerik");
-
-  private DefaultIssue issue = new DefaultIssue().setKey("ABC");
-  private Action.Context context = new BulkChangeAction.ActionContext(issue, issueChangeContext);
-
-  private AssignAction action = new AssignAction(dbTester.getDbClient(), issueUpdater);
+  private DefaultIssue issue = new DefaultIssue().setKey("ABC").setAssignee(ISSUE_CURRENT_ASSIGNEE);
+  private ComponentDto project;
+  private Action.Context context;
+  private OrganizationDto issueOrganizationDto;
+
+  private AssignAction underTest = new AssignAction(db.getDbClient(), new IssueFieldsSetter());
+
+  @Before
+  public void setUp() throws Exception {
+    issueOrganizationDto = db.organizations().insert();
+    project = db.components().insertProject(issueOrganizationDto);
+    context = new BulkChangeAction.ActionContext(issue, issueChangeContext, project);
+  }
 
   @Test
   public void assign_issue() {
-    UserDto assignee = newUserDto();
+    UserDto assignee = db.users().insertUser("john");
+    db.organizations().addMember(issueOrganizationDto, assignee);
+    Map<String, Object> properties = new HashMap<>(ImmutableMap.of("assignee", "john"));
 
-    action.execute(ImmutableMap.of("verifiedAssignee", assignee), context);
+    underTest.verify(properties, Collections.emptyList(), userSession);
+    boolean executeResult = underTest.execute(properties, context);
 
+    assertThat(executeResult).isTrue();
     assertThat(issue.assignee()).isEqualTo(assignee.getLogin());
   }
 
   @Test
-  public void fail_if_assignee_is_not_verified() {
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("Assignee is missing from the execution parameters");
+  public void unassign_issue_if_assignee_is_empty() {
+    Map<String, Object> properties = new HashMap<>(ImmutableMap.of("assignee", ""));
+
+    underTest.verify(properties, Collections.emptyList(), userSession);
+    boolean executeResult = underTest.execute(properties, context);
 
-    action.execute(emptyMap(), context);
+    assertThat(executeResult).isTrue();
+    assertThat(issue.assignee()).isNull();
   }
 
   @Test
-  public void verify_that_assignee_exists() {
-    String assignee = "arthur";
-    Map<String, Object> properties = new HashMap<>(ImmutableMap.of("assignee", assignee));
-    UserDto user = dbTester.users().insertUser(assignee);
+  public void unassign_issue_if_assignee_is_null() {
+    Map<String, Object> properties = new HashMap<>();
+    properties.put("assignee", null);
 
-    boolean verify = action.verify(properties, singletonList(issue), userSession);
+    underTest.verify(properties, Collections.emptyList(), userSession);
+    boolean executeResult = underTest.execute(properties, context);
 
-    assertThat(verify).isTrue();
-    UserDto verifiedUser = (UserDto) properties.get(AssignAction.VERIFIED_ASSIGNEE);
-    assertThat(verifiedUser.getLogin()).isEqualTo(user.getLogin());
+    assertThat(executeResult).isTrue();
+    assertThat(issue.assignee()).isNull();
+  }
+
+  @Test
+  public void does_not_assign_issue_when_assignee_is_not_member_of_project_issue_organization() {
+    OrganizationDto otherOrganizationDto = db.organizations().insert();
+    UserDto assignee = db.users().insertUser("john");
+    // User is not member of the organization of the issue
+    db.organizations().addMember(otherOrganizationDto, assignee);
+    Map<String, Object> properties = new HashMap<>(ImmutableMap.of("assignee", "john"));
+
+    underTest.verify(properties, Collections.emptyList(), userSession);
+    boolean executeResult = underTest.execute(properties, context);
+
+    assertThat(executeResult).isFalse();
+  }
+
+  @Test
+  public void fail_if_assignee_is_not_verified() {
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Assignee is missing from the execution parameters");
+
+    underTest.execute(emptyMap(), context);
   }
 
   @Test
@@ -95,24 +134,23 @@ public class AssignActionTest {
     expectedException.expect(NotFoundException.class);
     expectedException.expectMessage("Unknown user: arthur");
 
-    action.verify(ImmutableMap.of("assignee", "arthur"), singletonList(issue), userSession);
+    underTest.verify(ImmutableMap.of("assignee", "arthur"), singletonList(issue), userSession);
   }
 
   @Test
-  public void unassign_issue_if_assignee_is_empty() {
-    Map<String, Object> properties = new HashMap<>(ImmutableMap.of("assignee", ""));
+  public void fail_if_assignee_is_disabled() {
+    db.users().insertUser(user -> user.setLogin("arthur").setActive(false));
 
-    boolean verify = action.verify(properties, singletonList(issue), userSession);
+    expectedException.expect(NotFoundException.class);
+    expectedException.expectMessage("Unknown user: arthur");
 
-    assertThat(verify).isTrue();
-    assertThat(properties.containsKey(AssignAction.VERIFIED_ASSIGNEE)).isTrue();
-    assertThat(properties.get(AssignAction.VERIFIED_ASSIGNEE)).isNull();
+    underTest.verify(new HashMap<>(ImmutableMap.of("assignee", "arthur")), singletonList(issue), userSession);
   }
 
   @Test
   public void support_only_unresolved_issues() {
-    assertThat(action.supports(new DefaultIssue().setResolution(null))).isTrue();
-    assertThat(action.supports(new DefaultIssue().setResolution(Issue.RESOLUTION_FIXED))).isFalse();
+    assertThat(underTest.supports(new DefaultIssue().setResolution(null))).isTrue();
+    assertThat(underTest.supports(new DefaultIssue().setResolution(Issue.RESOLUTION_FIXED))).isFalse();
   }
 
 }

server/sonar-server/src/test/java/org/sonar/server/issue/SetSeverityActionTest.java

@@ -69,7 +69,7 @@ public class SetSeverityActionTest {
   public void set_severity() {
     DefaultIssue issue = newIssue().setSeverity(MAJOR).toDefaultIssue();
     setUserWithBrowseAndAdministerIssuePermission(issue.projectUuid());
-    BulkChangeAction.ActionContext context = new BulkChangeAction.ActionContext(issue, IssueChangeContext.createUser(NOW, userSession.getLogin()));
+    BulkChangeAction.ActionContext context = new BulkChangeAction.ActionContext(issue, IssueChangeContext.createUser(NOW, userSession.getLogin()), null);
 
     action.execute(ImmutableMap.of("severity", MINOR), context);
 

server/sonar-server/src/test/java/org/sonar/server/issue/SetTypeActionTest.java

@@ -69,7 +69,8 @@ public class SetTypeActionTest {
     DefaultIssue issue = newIssue().setType(BUG).toDefaultIssue();
     setUserWithBrowseAndAdministerIssuePermission(issue.projectUuid());
 
-    action.execute(ImmutableMap.of("type", VULNERABILITY.name()), new BulkChangeAction.ActionContext(issue, IssueChangeContext.createUser(NOW, userSession.getLogin())));
+    action.execute(ImmutableMap.of("type", VULNERABILITY.name()),
+      new BulkChangeAction.ActionContext(issue, IssueChangeContext.createUser(NOW, userSession.getLogin()), null));
 
     assertThat(issue.type()).isEqualTo(VULNERABILITY);
     assertThat(issue.isChanged()).isTrue();

server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java

@@ -214,6 +214,8 @@ public class BulkChangeActionTest {
   public void bulk_change_many_issues() throws Exception {
     setUserProjectPermissions(USER, ISSUE_ADMIN);
     UserDto userToAssign = db.users().insertUser("arthur");
+    db.organizations().addMember(organization, user);
+    db.organizations().addMember(organization, userToAssign);
     IssueDto issue1 = db.issues().insertIssue(newUnresolvedIssue().setAssignee(user.getLogin())).setType(BUG).setSeverity(MINOR);
     IssueDto issue2 = db.issues().insertIssue(newUnresolvedIssue().setAssignee(userToAssign.getLogin())).setType(BUG).setSeverity(MAJOR);
     IssueDto issue3 = db.issues().insertIssue(newUnresolvedIssue().setAssignee(null)).setType(VULNERABILITY).setSeverity(MAJOR);