From f4013ed54001cca10dfcb1c9973013f54d063ed5 Mon Sep 17 00:00:00 2001 From: Daniel Schwarz Date: Thu, 23 Mar 2017 12:05:48 +0100 Subject: [PATCH] SONAR-8857 make api/qualityprofiles/deactivate_rule organization aware --- .../java/org/sonar/db/rule/RuleTesting.java | 3 + .../qualityprofile/QProfileService.java | 12 -- .../qualityprofile/ws/CreateAction.java | 7 +- .../ws/DeactivateRuleAction.java | 36 ++++-- .../qualityprofile/ws/QProfileWsSupport.java | 9 +- .../ws/DeactivateRuleActionTest.java | 109 +++++++++++++++++- .../ws/QProfilesWsMediumTest.java | 11 +- 7 files changed, 153 insertions(+), 34 deletions(-) diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleTesting.java b/server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleTesting.java index a35add1f3c5..e4265a20095 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleTesting.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleTesting.java @@ -163,4 +163,7 @@ public class RuleTesting { .setType(templateRule.getType()); } + public static RuleKey randomRuleKey() { + return RuleKey.of(randomAlphanumeric(5), randomAlphanumeric(5)); + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java index a258811f493..b62c919fa9d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java @@ -24,7 +24,6 @@ import javax.annotation.Nullable; import org.sonar.api.server.ServerSide; import org.sonar.db.DbClient; import org.sonar.db.DbSession; -import org.sonar.db.qualityprofile.ActiveRuleKey; import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.rule.index.RuleQuery; @@ -67,17 +66,6 @@ public class QProfileService { } } - /** - * Deactivate a rule on a Quality profile. Does nothing if the rule is not activated, but - * fails if the rule or the profile does not exist. - */ - public List deactivate(ActiveRuleKey key) { - verifyAdminPermission(); - try (DbSession dbSession = db.openSession(false)) { - return ruleActivator.deactivateAndUpdateIndex(dbSession, key); - } - } - public BulkChangeResult bulkActivate(RuleQuery ruleQuery, String profile, @Nullable String severity) { verifyAdminPermission(); return ruleActivator.bulkActivate(ruleQuery, profile, severity); diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/CreateAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/CreateAction.java index 4d1813112b7..09b4cd83999 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/CreateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/CreateAction.java @@ -133,10 +133,9 @@ public class CreateAction implements QProfileWsAction { result.add(exporters.importXml(profile, importerKey, contentToImport, dbSession)); } } - String organizationKey = qProfileWsSupport.getOrganizationKey(dbSession, result.profile()); dbSession.commit(); activeRuleIndexer.index(result.getChanges()); - return buildResponse(result, organizationKey); + return buildResponse(result, organization); } private static CreateRequest toRequest(Request request, OrganizationDto organization) { @@ -147,10 +146,10 @@ public class CreateAction implements QProfileWsAction { return builder.build(); } - private CreateWsResponse buildResponse(QProfileResult result, String organizationKey) { + private CreateWsResponse buildResponse(QProfileResult result, OrganizationDto organization) { String language = result.profile().getLanguage(); CreateWsResponse.QualityProfile.Builder builder = CreateWsResponse.QualityProfile.newBuilder() - .setOrganization(organizationKey) + .setOrganization(organization.getKey()) .setKey(result.profile().getKey()) .setName(result.profile().getName()) .setLanguage(language) diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRuleAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRuleAction.java index 600c90bcd5d..e1173d8d632 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRuleAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRuleAction.java @@ -25,8 +25,14 @@ import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.core.util.Uuids; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.organization.OrganizationDto; +import org.sonar.db.permission.OrganizationPermission; import org.sonar.db.qualityprofile.ActiveRuleKey; -import org.sonar.server.qualityprofile.QProfileService; +import org.sonar.db.qualityprofile.QualityProfileDto; +import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.user.UserSession; import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.ACTION_DEACTIVATE_RULE; import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.ActivateActionParameters.PARAM_PROFILE_KEY; @@ -35,10 +41,16 @@ import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters. @ServerSide public class DeactivateRuleAction implements QProfileWsAction { - private final QProfileService service; + private final DbClient dbClient; + private final RuleActivator ruleActivator; + private final UserSession userSession; + private final QProfileWsSupport wsSupport; - public DeactivateRuleAction(QProfileService service) { - this.service = service; + public DeactivateRuleAction(DbClient dbClient, RuleActivator ruleActivator, UserSession userSession, QProfileWsSupport wsSupport) { + this.dbClient = dbClient; + this.ruleActivator = ruleActivator; + this.userSession = userSession; + this.wsSupport = wsSupport; } public void define(WebService.NewController controller) { @@ -62,11 +74,19 @@ public class DeactivateRuleAction implements QProfileWsAction { @Override public void handle(Request request, Response response) throws Exception { - RuleKey ruleKey = readRuleKey(request); - service.deactivate(ActiveRuleKey.of(request.mandatoryParam(PARAM_PROFILE_KEY), ruleKey)); + RuleKey ruleKey = RuleKey.parse(request.mandatoryParam(PARAM_RULE_KEY)); + String qualityProfileKey = request.mandatoryParam(PARAM_PROFILE_KEY); + userSession.checkLoggedIn(); + try (DbSession dbSession = dbClient.openSession(false)) { + checkPermission(qualityProfileKey, dbSession); + ActiveRuleKey activeRuleKey = ActiveRuleKey.of(qualityProfileKey, ruleKey); + ruleActivator.deactivateAndUpdateIndex(dbSession, activeRuleKey); + } } - private static RuleKey readRuleKey(Request request) { - return RuleKey.parse(request.mandatoryParam(PARAM_RULE_KEY)); + private void checkPermission(String qualityProfileKey, DbSession dbSession) { + QualityProfileDto qualityProfile = dbClient.qualityProfileDao().selectByKey(dbSession, qualityProfileKey); + OrganizationDto organization = wsSupport.getOrganization(dbSession, qualityProfile); + userSession.checkPermission(OrganizationPermission.ADMINISTER_QUALITY_PROFILES, organization); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java index 0bbad55e5cc..2047a20a3a7 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java @@ -32,6 +32,7 @@ import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.user.UserSession; import org.sonar.server.ws.WsUtils; +import static java.util.Objects.requireNonNull; import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_PROFILES; import static org.sonar.server.ws.WsUtils.checkFound; import static org.sonarqube.ws.client.component.ComponentsWsParameters.PARAM_ORGANIZATION; @@ -49,11 +50,11 @@ public class QProfileWsSupport { this.defaultOrganizationProvider = defaultOrganizationProvider; } - public String getOrganizationKey(DbSession dbSession, QualityProfileDto profile) { + public OrganizationDto getOrganization(DbSession dbSession, QualityProfileDto profile) { + requireNonNull(profile); String organizationUuid = profile.getOrganizationUuid(); return dbClient.organizationDao().selectByUuid(dbSession, organizationUuid) - .orElseThrow(() -> new IllegalStateException("Cannot load organization with uuid=" + organizationUuid)) - .getKey(); + .orElseThrow(() -> new IllegalStateException("Cannot load organization with uuid=" + organizationUuid)); } public OrganizationDto getOrganizationByKey(DbSession dbSession, @Nullable String organizationKey) { @@ -114,7 +115,7 @@ public class QProfileWsSupport { OrganizationDto org = getOrganizationByKey(dbSession, ref.getOrganizationKey().orElse(null)); profile = dbClient.qualityProfileDao().selectByNameAndLanguage(org, ref.getName(), ref.getLanguage(), dbSession); checkFound(profile, "Quality Profile for language '%s' and name '%s' does not exist%s", ref.getLanguage(), ref.getName(), - ref.getOrganizationKey().map(o -> " in organization '"+o+"'").orElse("")); + ref.getOrganizationKey().map(o -> " in organization '" + o + "'").orElse("")); } return profile; } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java index 4b6107fb5ce..f42507a197f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java @@ -19,22 +19,125 @@ */ package org.sonar.server.qualityprofile.ws; +import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; +import org.sonar.api.rule.RuleKey; import org.sonar.api.server.ws.WebService; -import org.sonar.server.qualityprofile.QProfileService; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.DbTester; +import org.sonar.db.organization.OrganizationDto; +import org.sonar.db.permission.OrganizationPermission; +import org.sonar.db.qualityprofile.ActiveRuleKey; +import org.sonar.db.qualityprofile.QualityProfileDto; +import org.sonar.db.rule.RuleTesting; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.UnauthorizedException; +import org.sonar.server.organization.TestDefaultOrganizationProvider; +import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.tester.UserSessionRule; +import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.WsActionTester; +import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.sonar.server.platform.db.migration.def.VarcharColumnDef.UUID_SIZE; public class DeactivateRuleActionTest { + @Rule + public DbTester dbTester = DbTester.create(); + @Rule + public UserSessionRule userSession = UserSessionRule.standalone(); + @Rule + public ExpectedException thrown = ExpectedException.none(); + + private DbClient dbClient = dbTester.getDbClient(); + private RuleActivator ruleActivator = mock(RuleActivator.class); + private QProfileWsSupport wsSupport = new QProfileWsSupport(dbClient, userSession, TestDefaultOrganizationProvider.from(dbTester)); + private DeactivateRuleAction underTest = new DeactivateRuleAction(dbClient, ruleActivator, userSession, wsSupport); + private WsActionTester wsActionTester = new WsActionTester(underTest); + private OrganizationDto defaultOrganization; + private OrganizationDto organization; + + @Before + public void before() { + defaultOrganization = dbTester.getDefaultOrganization(); + organization = dbTester.organizations().insert(); + } + @Test public void define_deactivate_rule_action() { - DeactivateRuleAction action = new DeactivateRuleAction(mock(QProfileService.class)); - WebService.Action definition = new WsActionTester(action).getDef(); + WebService.Action definition = wsActionTester.getDef(); assertThat(definition).isNotNull(); assertThat(definition.isPost()).isTrue(); assertThat(definition.params()).extracting(WebService.Param::key).containsExactlyInAnyOrder("profile_key", "rule_key"); } + + @Test + public void should_fail_if_not_logged_in() { + TestRequest request = wsActionTester.newRequest() + .setMethod("POST") + .setParam("rule_key", RuleTesting.newRuleDto().getKey().toString()) + .setParam("profile_key", randomAlphanumeric(UUID_SIZE)); + + thrown.expect(UnauthorizedException.class); + request.execute(); + } + + @Test + public void should_fail_if_not_organization_quality_profile_administrator() { + userSession.logIn().addPermission(OrganizationPermission.ADMINISTER_QUALITY_PROFILES, defaultOrganization); + QualityProfileDto qualityProfile = dbTester.qualityProfiles().insert(organization); + TestRequest request = wsActionTester.newRequest() + .setMethod("POST") + .setParam("rule_key", RuleTesting.newRuleDto().getKey().toString()) + .setParam("profile_key", qualityProfile.getKey()); + + thrown.expect(ForbiddenException.class); + request.execute(); + } + + @Test + public void deactivate_rule_in_default_organization() { + userSession.logIn().addPermission(OrganizationPermission.ADMINISTER_QUALITY_PROFILES, defaultOrganization); + QualityProfileDto qualityProfile = dbTester.qualityProfiles().insert(defaultOrganization); + RuleKey ruleKey = RuleTesting.randomRuleKey(); + TestRequest request = wsActionTester.newRequest() + .setMethod("POST") + .setParam("rule_key", ruleKey.toString()) + .setParam("profile_key", qualityProfile.getKey()); + + request.execute(); + + ArgumentCaptor captor = ArgumentCaptor.forClass(ActiveRuleKey.class); + Mockito.verify(ruleActivator).deactivateAndUpdateIndex(any(DbSession.class), captor.capture()); + assertThat(captor.getValue().ruleKey()).isEqualTo(ruleKey); + assertThat(captor.getValue().qProfile()).isEqualTo(qualityProfile.getKey()); + } + + @Test + public void deactivate_rule_in_specific_organization() { + userSession.logIn().addPermission(OrganizationPermission.ADMINISTER_QUALITY_PROFILES, organization); + QualityProfileDto qualityProfile = dbTester.qualityProfiles().insert(organization); + RuleKey ruleKey = RuleTesting.randomRuleKey(); + TestRequest request = wsActionTester.newRequest() + .setMethod("POST") + .setParam("organization", organization.getKey()) + .setParam("rule_key", ruleKey.toString()) + .setParam("profile_key", qualityProfile.getKey()); + + request.execute(); + + ArgumentCaptor captor = ArgumentCaptor.forClass(ActiveRuleKey.class); + Mockito.verify(ruleActivator).deactivateAndUpdateIndex(any(DbSession.class), captor.capture()); + assertThat(captor.getValue().ruleKey()).isEqualTo(ruleKey); + assertThat(captor.getValue().qProfile()).isEqualTo(qualityProfile.getKey()); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java index 7e2f33839c9..75d37b07279 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java @@ -32,6 +32,8 @@ import org.sonar.api.rule.Severity; import org.sonar.api.server.ws.WebService; import org.sonar.db.DbClient; import org.sonar.db.DbSession; +import org.sonar.db.organization.OrganizationDto; +import org.sonar.db.organization.OrganizationTesting; import org.sonar.db.qualityprofile.ActiveRuleDao; import org.sonar.db.qualityprofile.ActiveRuleDto; import org.sonar.db.qualityprofile.ActiveRuleKey; @@ -75,6 +77,7 @@ public class QProfilesWsMediumTest { private WsTester wsTester; private RuleIndexer ruIndexer = tester.get(RuleIndexer.class); private ActiveRuleIndexer activeRuIndexer = tester.get(ActiveRuleIndexer.class); + private OrganizationDto organization; @Before public void setUp() { @@ -85,6 +88,8 @@ public class QProfilesWsMediumTest { ruIndexer = tester.get(RuleIndexer.class); activeRuIndexer = tester.get(ActiveRuleIndexer.class); + organization = OrganizationTesting.newOrganizationDto().setKey("org-123"); + db.organizationDao().insert(session, organization); } @After @@ -405,8 +410,8 @@ public class QProfilesWsMediumTest { @Test public void reset() throws Exception { - QualityProfileDto profile = QProfileTesting.newXooP1("org-123"); - QualityProfileDto subProfile = QProfileTesting.newXooP2("org-123").setParentKee(QProfileTesting.XOO_P1_KEY); + QualityProfileDto profile = QProfileTesting.newXooP1(organization); + QualityProfileDto subProfile = QProfileTesting.newXooP2(organization).setParentKee(QProfileTesting.XOO_P1_KEY); db.qualityProfileDao().insert(session, profile, subProfile); RuleDefinitionDto rule = createRule(profile.getLanguage(), "rule"); @@ -437,7 +442,7 @@ public class QProfilesWsMediumTest { } private QualityProfileDto createProfile(String lang) { - QualityProfileDto profile = QProfileTesting.newQProfileDto("org-123", new QProfileName(lang, "P" + lang), "p" + lang); + QualityProfileDto profile = QProfileTesting.newQProfileDto(organization, new QProfileName(lang, "P" + lang), "p" + lang); db.qualityProfileDao().insert(session, profile); return profile; } -- 2.39.5