SONAR-8899 Check membership when assigning issue

diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDbTester.java b/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDbTester.java
index 9ef1a22cf346222952d70d96c715e3970c8eae65..458a9077f6158b45fe908489ab16b87569290ee2 100644 (file)
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDbTester.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDbTester.java
@@ -26,6 +26,7 @@ import org.sonar.core.issue.DefaultIssueComment;
 import org.sonar.core.issue.FieldDiffs;
 import org.sonar.db.DbTester;
 import org.sonar.db.component.ComponentDto;
+import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.rule.RuleDto;
 
 import static org.sonar.db.component.ComponentTesting.newFileDto;
@@ -52,8 +53,17 @@ public class IssueDbTester {
   }
 
   public IssueDto insertIssue(Consumer<IssueDto> populateIssueDto) {
+    return insertIssue(db.getDefaultOrganization(), populateIssueDto);
+  }
+
+  public IssueDto insertIssue(OrganizationDto organizationDto) {
+    return insertIssue(organizationDto, issueDto -> {
+    });
+  }
+
+  public IssueDto insertIssue(OrganizationDto organizationDto, Consumer<IssueDto> populateIssueDto) {
     RuleDto rule = db.rules().insertRule(newRuleDto());
-    ComponentDto project = db.components().insertProject();
+    ComponentDto project = db.components().insertProject(organizationDto);
     ComponentDto file = db.components().insertComponent(newFileDto(project));
     IssueDto issueDto = newDto(rule, file, project);
     populateIssueDto.accept(issueDto);

diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/AssignAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/AssignAction.java
index 33b6b611a8819640902fec6e938b1871f5495b71..cdb9fc28346086810b4eb945e8afc88cb7babcd0 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/AssignAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/AssignAction.java
@@ -22,6 +22,7 @@ package org.sonar.server.issue.ws;
 import com.google.common.base.Strings;
 import com.google.common.io.Resources;
 import java.util.Date;
+import java.util.Optional;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.apache.commons.lang.BooleanUtils;
@@ -34,6 +35,9 @@ import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.core.util.Uuids;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
+import org.sonar.db.component.ComponentDto;
+import org.sonar.db.issue.IssueDto;
+import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.issue.IssueFieldsSetter;
 import org.sonar.server.issue.IssueFinder;
@@ -41,6 +45,8 @@ import org.sonar.server.issue.IssueUpdater;
 import org.sonar.server.user.UserSession;
 
 import static com.google.common.base.Strings.emptyToNull;
+import static java.lang.String.format;
+import static java.util.Objects.requireNonNull;
 import static org.sonar.server.ws.WsUtils.checkFound;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_ASSIGN;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_ASSIGNEE;
@@ -103,8 +109,12 @@ public class AssignAction implements IssuesWsAction {
 
   private void assign(String issueKey, @Nullable String assignee) {
     try (DbSession dbSession = dbClient.openSession(false)) {
-      DefaultIssue issue = issueFinder.getByKey(dbSession, issueKey).toDefaultIssue();
+      IssueDto issueDto = issueFinder.getByKey(dbSession, issueKey);
+      DefaultIssue issue = issueDto.toDefaultIssue();
       UserDto user = getUser(dbSession, assignee);
+      if (user != null) {
+        checkMembership(dbSession, issueDto, user);
+      }
       IssueChangeContext context = IssueChangeContext.createUser(new Date(system2.now()), userSession.getLogin());
       if (issueFieldsSetter.assign(issue, user, context)) {
         issueUpdater.saveIssue(dbSession, issue, context, null);
@@ -126,7 +136,16 @@ public class AssignAction implements IssuesWsAction {
     if (Strings.isNullOrEmpty(assignee)) {
       return null;
     }
-    UserDto user = dbClient.userDao().selectByLogin(dbSession, assignee);
-    return checkFound(user != null && user.isActive() ? user : null, "Unknown user: %s", assignee);
+    return checkFound(dbClient.userDao().selectActiveUserByLogin(dbSession, assignee), "Unknown user: %s", assignee);
+  }
+
+  private void checkMembership(DbSession dbSession, IssueDto issueDto, UserDto user) {
+    String projectUuid = requireNonNull(issueDto.getProjectUuid());
+    ComponentDto project = Optional.ofNullable(dbClient.componentDao().selectByUuid(dbSession, projectUuid).orNull())
+      .orElseThrow(() -> new IllegalStateException(format("Unknown project %s", projectUuid)));
+    OrganizationDto organizationDto = dbClient.organizationDao().selectByUuid(dbSession, project.getOrganizationUuid())
+      .orElseThrow(() -> new IllegalStateException(format("Unknown organization %s", project.getOrganizationUuid())));
+    dbClient.organizationMemberDao().select(dbSession, organizationDto.getUuid(), user.getId())
+      .orElseThrow(() -> new IllegalArgumentException(format("User '%s' is not member of organization '%s'", user.getLogin(), organizationDto.getKey())));
   }
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/AssignActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/AssignActionTest.java
index 4e0dc49ddf8e29f312e9f045c61736e9b10da859..130985b956938bab819f3ea20c4eff65401499e6 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/AssignActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/AssignActionTest.java
@@ -20,7 +20,6 @@
 package org.sonar.server.issue.ws;
 
 import javax.annotation.Nullable;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -30,6 +29,8 @@ import org.sonar.api.server.ws.Response;
 import org.sonar.api.utils.internal.TestSystem2;
 import org.sonar.db.DbTester;
 import org.sonar.db.issue.IssueDto;
+import org.sonar.db.organization.OrganizationDto;
+import org.sonar.db.user.UserDto;
 import org.sonar.server.es.EsTester;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
@@ -82,19 +83,15 @@ public class AssignActionTest {
     new IssueUpdater(db.getDbClient(),
       new ServerIssueStorage(system2, new DefaultRuleFinder(db.getDbClient()), db.getDbClient(), issueIndexer), mock(NotificationManager.class)),
     responseWriter);
-  private WsActionTester tester = new WsActionTester(underTest);
-
-  @Before
-  public void setUp() throws Exception {
-    db.users().insertUser(PREVIOUS_ASSIGNEE);
-  }
+  private WsActionTester ws = new WsActionTester(underTest);
 
   @Test
   public void assign_to_someone() throws Exception {
     IssueDto issue = newIssueWithBrowsePermission();
-    db.users().insertUser("arthur");
+    UserDto userDto = db.users().insertUser("arthur");
+    addUserAsMemberOfDefaultOrganization(userDto);
 
-    tester.newRequest()
+    ws.newRequest()
       .setParam("issue", issue.getKey())
       .setParam("assignee", "arthur")
       .execute();
@@ -107,7 +104,7 @@ public class AssignActionTest {
   public void assign_to_me() throws Exception {
     IssueDto issue = newIssueWithBrowsePermission();
 
-    tester.newRequest()
+    ws.newRequest()
       .setParam("issue", issue.getKey())
       .setParam("assignee", "_me")
       .execute();
@@ -120,7 +117,7 @@ public class AssignActionTest {
   public void assign_to_me_using_deprecated_me_param() throws Exception {
     IssueDto issue = newIssueWithBrowsePermission();
 
-    tester.newRequest()
+    ws.newRequest()
       .setParam("issue", issue.getKey())
       .setParam("me", "true")
       .execute();
@@ -133,7 +130,7 @@ public class AssignActionTest {
   public void unassign() throws Exception {
     IssueDto issue = newIssueWithBrowsePermission();
 
-    tester.newRequest()
+    ws.newRequest()
       .setParam("issue", issue.getKey())
       .execute();
 
@@ -145,7 +142,7 @@ public class AssignActionTest {
   public void unassign_with_empty_assignee_param() throws Exception {
     IssueDto issue = newIssueWithBrowsePermission();
 
-    tester.newRequest()
+    ws.newRequest()
       .setParam("issue", issue.getKey())
       .setParam("assignee", "")
       .execute();
@@ -156,10 +153,11 @@ public class AssignActionTest {
 
   @Test
   public void nothing_to_do_when_new_assignee_is_same_as_old_one() throws Exception {
-    db.users().insertUser("arthur");
     IssueDto issue = newIssueWithBrowsePermission();
+    UserDto userDto = db.users().insertUser(PREVIOUS_ASSIGNEE);
+    addUserAsMemberOfDefaultOrganization(userDto);
 
-    tester.newRequest()
+    ws.newRequest()
       .setParam("issue", issue.getKey())
       .setParam("assignee", PREVIOUS_ASSIGNEE)
       .execute();
@@ -176,7 +174,7 @@ public class AssignActionTest {
 
     expectedException.expect(NotFoundException.class);
 
-    tester.newRequest()
+    ws.newRequest()
       .setParam("issue", issue.getKey())
       .setParam("assignee", "unknown")
       .execute();
@@ -189,7 +187,7 @@ public class AssignActionTest {
 
     expectedException.expect(NotFoundException.class);
 
-    tester.newRequest()
+    ws.newRequest()
       .setParam("issue", issue.getKey())
       .setParam("assignee", "unknown")
       .execute();
@@ -202,7 +200,7 @@ public class AssignActionTest {
 
     expectedException.expect(UnauthorizedException.class);
 
-    tester.newRequest()
+    ws.newRequest()
       .setParam("issue", issue.getKey())
       .setParam("assignee", "_me")
       .execute();
@@ -211,17 +209,34 @@ public class AssignActionTest {
   @Test
   public void fail_when_missing_browse_permission() throws Exception {
     IssueDto issue = newIssue();
-    db.users().insertUser(CURRENT_USER_LOGIN);
     userSession.logIn(CURRENT_USER_LOGIN).addProjectUuidPermissions(CODEVIEWER, issue.getProjectUuid());
 
     expectedException.expect(ForbiddenException.class);
 
-    tester.newRequest()
+    ws.newRequest()
       .setParam("issue", issue.getKey())
       .setParam("assignee", "_me")
       .execute();
   }
 
+  @Test
+  public void fail_when_assignee_is_not_member_of_organization_of_project_issue() throws Exception {
+    OrganizationDto org = db.organizations().insert(organizationDto -> organizationDto.setKey("Organization key"));
+    IssueDto issueDto = db.issues().insertIssue(org);
+    setUserWithBrowsePermission(issueDto);
+    OrganizationDto otherOrganization = db.organizations().insert();
+    UserDto assignee = db.users().insertUser("arthur");
+    db.organizations().addMember(otherOrganization, assignee);
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("User 'arthur' is not member of organization 'Organization key'");
+
+    ws.newRequest()
+      .setParam("issue", issueDto.getKey())
+      .setParam("assignee", "arthur")
+      .execute();
+  }
+
   private IssueDto newIssue() {
     return db.issues().insertIssue(
       issueDto -> issueDto
@@ -230,21 +245,27 @@ public class AssignActionTest {
         .setUpdatedAt(PAST).setIssueUpdateTime(PAST));
   }
 
-  private void setUserWithBrowsePermission(IssueDto issue) {
-    db.users().insertUser(CURRENT_USER_LOGIN);
-    userSession.logIn(CURRENT_USER_LOGIN).addProjectUuidPermissions(USER, issue.getProjectUuid());
-  }
-
   private IssueDto newIssueWithBrowsePermission() {
     IssueDto issue = newIssue();
     setUserWithBrowsePermission(issue);
     return issue;
   }
 
+  private void setUserWithBrowsePermission(IssueDto issue) {
+    UserDto currentUser = db.users().insertUser(CURRENT_USER_LOGIN);
+    addUserAsMemberOfDefaultOrganization(currentUser);
+    userSession.logIn(CURRENT_USER_LOGIN).addProjectUuidPermissions(USER, issue.getProjectUuid());
+  }
+
   private void checkIssueAssignee(String issueKey, @Nullable String expectedAssignee) {
     IssueDto issueReloaded = db.getDbClient().issueDao().selectByKey(db.getSession(), issueKey).get();
     assertThat(issueReloaded.getAssignee()).isEqualTo(expectedAssignee);
     assertThat(issueReloaded.getIssueUpdateTime()).isEqualTo(NOW);
     assertThat(issueReloaded.getUpdatedAt()).isEqualTo(NOW);
   }
+
+  private void addUserAsMemberOfDefaultOrganization(UserDto user) {
+    db.organizations().addMember(db.getDefaultOrganization(), user);
+  }
+
 }