SONAR-8716 fix check of permissions in api/rules

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,

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<String> listTags(@Nullable String query, int size) {
   public SearchIdResult<RuleKey> 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);
-  }
 }

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);
   }
 }

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);
+  }
+}

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);
-  }
 }

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("<foo>", 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<String> tags, Set<String> systemTags) {
     dao.insert(dbSession,
       RuleTesting.newDto(key).setTags(tags).setSystemTags(systemTags));

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);
   }
 }

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);
+  }
 }