diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java index e21f40ff1617..2381c52ee716 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java @@ -170,6 +170,7 @@ import org.sonar.server.rule.ws.RepositoriesAction; import org.sonar.server.rule.ws.RuleMapper; import org.sonar.server.rule.ws.RuleQueryFactory; +import org.sonar.server.rule.ws.RuleWsSupport; import org.sonar.server.rule.ws.RulesWs; import org.sonar.server.rule.ws.TagsAction; import org.sonar.server.serverid.ws.ServerIdWsModule; @@ -290,6 +291,7 @@ protected void configureLevel() { RuleDeleter.class, org.sonar.server.rule.ws.UpdateAction.class, RulesWs.class, + RuleWsSupport.class, org.sonar.server.rule.ws.SearchAction.class, org.sonar.server.rule.ws.ShowAction.class, org.sonar.server.rule.ws.CreateAction.class, diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java b/server/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java index 48e703743188..581d37977b7b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java @@ -23,13 +23,11 @@ import javax.annotation.Nullable; import org.sonar.api.rule.RuleKey; import org.sonar.api.server.ServerSide; -import org.sonar.core.permission.GlobalPermissions; import org.sonar.server.es.SearchIdResult; import org.sonar.server.es.SearchOptions; import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.rule.index.RuleIndexDefinition; import org.sonar.server.rule.index.RuleQuery; -import org.sonar.server.user.UserSession; /** * @since 4.4 @@ -38,13 +36,9 @@ public class RuleService { private final RuleIndex index; - private final RuleDeleter ruleDeleter; - private final UserSession userSession; - public RuleService(RuleIndex index, RuleDeleter ruleDeleter, UserSession userSession) { + public RuleService(RuleIndex index) { this.index = index; - this.ruleDeleter = ruleDeleter; - this.userSession = userSession; } public RuleQuery newRuleQuery() { @@ -70,15 +64,4 @@ public Set listTags(@Nullable String query, int size) { public SearchIdResult search(RuleQuery query, SearchOptions options) { return index.search(query, options); } - - public void delete(RuleKey ruleKey) { - checkPermission(); - ruleDeleter.delete(ruleKey); - } - - private void checkPermission() { - userSession - .checkLoggedIn() - .checkPermission(GlobalPermissions.QUALITY_PROFILE_ADMIN); - } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/DeleteAction.java b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/DeleteAction.java index d27696e63741..3800ca40c67d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/DeleteAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/DeleteAction.java @@ -23,19 +23,18 @@ import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; -import org.sonar.server.rule.RuleService; +import org.sonar.server.rule.RuleDeleter; -/** - * @since 4.4 - */ public class DeleteAction implements RulesWsAction { public static final String PARAM_KEY = "key"; - private final RuleService service; + private final RuleDeleter ruleDeleter; + private final RuleWsSupport ruleWsSupport; - public DeleteAction(RuleService service) { - this.service = service; + public DeleteAction(RuleDeleter ruleDeleter, RuleWsSupport ruleWsSupport) { + this.ruleDeleter = ruleDeleter; + this.ruleWsSupport = ruleWsSupport; } @Override @@ -56,7 +55,8 @@ public void define(WebService.NewController controller) { @Override public void handle(Request request, Response response) { + ruleWsSupport.checkQProfileAdminPermission(); RuleKey key = RuleKey.parse(request.mandatoryParam(PARAM_KEY)); - service.delete(key); + ruleDeleter.delete(key); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/RuleWsSupport.java b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/RuleWsSupport.java new file mode 100644 index 000000000000..4ae15277d583 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/RuleWsSupport.java @@ -0,0 +1,42 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.rule.ws; + +import org.sonar.api.server.ServerSide; +import org.sonar.core.permission.GlobalPermissions; +import org.sonar.server.organization.DefaultOrganizationProvider; +import org.sonar.server.user.UserSession; + +@ServerSide +public class RuleWsSupport { + private final UserSession userSession; + private final DefaultOrganizationProvider defaultOrganizationProvider; + + public RuleWsSupport(UserSession userSession, DefaultOrganizationProvider defaultOrganizationProvider) { + this.userSession = userSession; + this.defaultOrganizationProvider = defaultOrganizationProvider; + } + + public void checkQProfileAdminPermission() { + userSession + .checkLoggedIn() + .checkOrganizationPermission(defaultOrganizationProvider.get().getUuid(), GlobalPermissions.QUALITY_PROFILE_ADMIN); + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/UpdateAction.java b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/UpdateAction.java index 1e8980aa1ff2..15023cfa77b4 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/UpdateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/UpdateAction.java @@ -35,7 +35,6 @@ import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.KeyValueFormat; -import org.sonar.core.permission.GlobalPermissions; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.rule.RuleDto; @@ -71,12 +70,15 @@ public class UpdateAction implements RulesWsAction { private final RuleUpdater ruleUpdater; private final RuleMapper mapper; private final UserSession userSession; + private final RuleWsSupport ruleWsSupport; - public UpdateAction(DbClient dbClient, RuleUpdater ruleUpdater, RuleMapper mapper, UserSession userSession) { + public UpdateAction(DbClient dbClient, RuleUpdater ruleUpdater, RuleMapper mapper, UserSession userSession, + RuleWsSupport ruleWsSupport) { this.dbClient = dbClient; this.ruleUpdater = ruleUpdater; this.mapper = mapper; this.userSession = userSession; + this.ruleWsSupport = ruleWsSupport; } @Override @@ -158,16 +160,14 @@ public void define(WebService.NewController controller) { @Override public void handle(Request request, Response response) throws Exception { - checkPermission(); - DbSession dbSession = dbClient.openSession(false); - try { + ruleWsSupport.checkQProfileAdminPermission(); + + try (DbSession dbSession = dbClient.openSession(false)) { RuleUpdate update = readRequest(dbSession, request); ruleUpdater.update(dbSession, update, userSession); UpdateResponse updateResponse = buildResponse(dbSession, update.getRuleKey()); writeProtobuf(updateResponse, request, response); - } finally { - dbClient.closeSession(dbSession); } } @@ -269,9 +269,4 @@ private UpdateResponse buildResponse(DbSession dbSession, RuleKey key) { return responseBuilder.build(); } - - private void checkPermission() { - userSession.checkLoggedIn(); - userSession.checkPermission(GlobalPermissions.QUALITY_PROFILE_ADMIN); - } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java index 7c63882908eb..1694ec2477c9 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java @@ -27,13 +27,10 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.api.rule.RuleKey; -import org.sonar.core.permission.GlobalPermissions; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.rule.RuleDao; import org.sonar.db.rule.RuleTesting; -import org.sonar.server.exceptions.ForbiddenException; -import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.rule.index.RuleIndexer; import org.sonar.server.tester.ServerTester; import org.sonar.server.tester.UserSessionRule; @@ -111,24 +108,6 @@ public void listTags_returns_empty_results_if_filter_contains_regexp_special_cha assertThat(service.listTags("", 10)).isEmpty(); } - @Test - public void delete_throws_UnauthorizedException_if_not_logged_in() { - expectedException.expect(UnauthorizedException.class); - expectedException.expectMessage("Authentication is required"); - - service.delete(RuleKey.of("java", "S001")); - } - - @Test - public void delete_throws_ForbiddenException_if_not_administrator() { - userSessionRule.logIn().setGlobalPermissions(GlobalPermissions.SCAN_EXECUTION); - - expectedException.expect(ForbiddenException.class); - expectedException.expectMessage("Insufficient privileges"); - - service.delete(RuleKey.of("java", "S001")); - } - private void insertRule(RuleKey key, Set tags, Set systemTags) { dao.insert(dbSession, RuleTesting.newDto(key).setTags(tags).setSystemTags(systemTags)); diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/ws/DeleteActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/ws/DeleteActionTest.java index 4bf09d52c528..70f4df1cf9a7 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/ws/DeleteActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/ws/DeleteActionTest.java @@ -19,35 +19,64 @@ */ package org.sonar.server.rule.ws; -import org.junit.Before; +import org.junit.Rule; import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.junit.rules.ExpectedException; import org.sonar.api.rule.RuleKey; -import org.sonar.server.rule.RuleService; +import org.sonar.core.permission.GlobalPermissions; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.UnauthorizedException; +import org.sonar.server.organization.DefaultOrganizationProvider; +import org.sonar.server.organization.TestDefaultOrganizationProvider; +import org.sonar.server.rule.RuleDeleter; +import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.WsTester; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -@RunWith(MockitoJUnitRunner.class) public class DeleteActionTest { - WsTester tester; + @Rule + public UserSessionRule userSession = UserSessionRule.standalone(); + @Rule + public ExpectedException expectedException = ExpectedException.none(); - @Mock - RuleService ruleService; + private RuleDeleter ruleDeleter = mock(RuleDeleter.class); + private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.fromUuid("ORG1"); + private RuleWsSupport ruleWsSupport = new RuleWsSupport(userSession, defaultOrganizationProvider); + private WsTester tester = new WsTester(new RulesWs(new DeleteAction(ruleDeleter, ruleWsSupport))); - @Before - public void setUp() { - tester = new WsTester(new RulesWs(new DeleteAction(ruleService))); - } @Test public void delete_custom_rule() throws Exception { + logInAsQProfileAdministrator(); + WsTester.TestRequest request = tester.newPostRequest("api/rules", "delete").setParam("key", "squid:XPath_1402065390816"); request.execute(); - verify(ruleService).delete(RuleKey.of("squid", "XPath_1402065390816")); + verify(ruleDeleter).delete(RuleKey.of("squid", "XPath_1402065390816")); + } + + @Test + public void throw_ForbiddenException_if_not_profile_administrator() throws Exception { + userSession.logIn(); + + expectedException.expect(ForbiddenException.class); + + tester.newPostRequest("api/rules", "delete").execute(); + } + + @Test + public void throw_UnauthorizedException_if_not_logged_in() throws Exception { + expectedException.expect(UnauthorizedException.class); + + tester.newPostRequest("api/rules", "delete").execute(); + } + + private void logInAsQProfileAdministrator() { + userSession + .logIn() + .addOrganizationPermission(defaultOrganizationProvider.get().getUuid(), GlobalPermissions.QUALITY_PROFILE_ADMIN); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/ws/UpdateActionMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/ws/UpdateActionMediumTest.java index 7669fb675d4a..fbda66d8067c 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/ws/UpdateActionMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/ws/UpdateActionMediumTest.java @@ -35,6 +35,7 @@ import org.sonar.db.rule.RuleDto; import org.sonar.db.rule.RuleParamDto; import org.sonar.db.rule.RuleTesting; +import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.rule.NewCustomRule; import org.sonar.server.rule.RuleCreator; import org.sonar.server.rule.RuleService; @@ -56,7 +57,7 @@ public class UpdateActionMediumTest { public static ServerTester tester = new ServerTester().withEsIndexes(); @Rule - public UserSessionRule userSessionRule = UserSessionRule.forServerTester(tester).logIn().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN); + public UserSessionRule userSessionRule = UserSessionRule.forServerTester(tester); WsTester wsTester; @@ -71,6 +72,7 @@ public void setUp() { ruleService = tester.get(RuleService.class); ruleDao = tester.get(RuleDao.class); session = tester.get(DbClient.class).openSession(false); + logInAsQProfileAdministrator(); } @After @@ -173,4 +175,9 @@ public void fail_to_update_custom_when_description_is_empty() { } } + private void logInAsQProfileAdministrator() { + userSessionRule + .logIn() + .addOrganizationPermission(tester.get(DefaultOrganizationProvider.class).get().getUuid(), GlobalPermissions.QUALITY_PROFILE_ADMIN); + } }