From: Julien Lancelot Date: Tue, 14 Mar 2017 15:55:01 +0000 (+0100) Subject: SONAR-8899 Check membership when assigning issue X-Git-Tag: 6.4-RC1~718 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=7cd770367c55ed0c350226aec556608247d2c358;p=sonarqube.git 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 9ef1a22cf34..458a9077f61 100644 --- 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 populateIssueDto) { + return insertIssue(db.getDefaultOrganization(), populateIssueDto); + } + + public IssueDto insertIssue(OrganizationDto organizationDto) { + return insertIssue(organizationDto, issueDto -> { + }); + } + + public IssueDto insertIssue(OrganizationDto organizationDto, Consumer 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 33b6b611a88..cdb9fc28346 100644 --- 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 4e0dc49ddf8..130985b9569 100644 --- 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); + } + }