SONAR-8716 fix check of permissions in api/settings

6 files changed, 59 insertions, 73 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/GenerateSecretKeyAction.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/GenerateSecretKeyAction.java
index 8c2999f6d7f..e897e57ed02 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/GenerateSecretKeyAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/GenerateSecretKeyAction.java
@@ -27,7 +27,6 @@ import org.sonar.api.server.ws.WebService;
 import org.sonar.server.user.UserSession;
 import org.sonarqube.ws.Settings.GenerateSecretKeyWsResponse;
 
-import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN;
 import static org.sonar.server.ws.WsUtils.writeProtobuf;
 
 public class GenerateSecretKeyAction implements SettingsWsAction {
@@ -52,7 +51,7 @@ public class GenerateSecretKeyAction implements SettingsWsAction {
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    userSession.checkPermission(SYSTEM_ADMIN);
+    userSession.checkIsRoot();
 
     writeProtobuf(GenerateSecretKeyWsResponse.newBuilder().setSecretKey(settings.getEncryption().generateRandomSecretKey()).build(), request, response);
   }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ResetAction.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ResetAction.java
index 10e76bb1583..5c92dfc2322 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ResetAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ResetAction.java
@@ -30,7 +30,6 @@ import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.web.UserRole;
-import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.core.util.stream.Collectors;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
@@ -90,8 +89,7 @@ public class ResetAction implements SettingsWsAction {
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    DbSession dbSession = dbClient.openSession(false);
-    try {
+    try (DbSession dbSession = dbClient.openSession(false)) {
       ResetRequest resetRequest = toWsRequest(request);
       Optional<ComponentDto> component = getComponent(dbSession, resetRequest);
       checkPermissions(component);
@@ -109,8 +107,6 @@ public class ResetAction implements SettingsWsAction {
       }
       dbSession.commit();
       response.noContent();
-    } finally {
-      dbClient.closeSession(dbSession);
     }
   }
 
@@ -142,7 +138,7 @@ public class ResetAction implements SettingsWsAction {
     if (component.isPresent()) {
       userSession.checkComponentPermission(UserRole.ADMIN, component.get());
     } else {
-      userSession.checkPermission(GlobalPermissions.SYSTEM_ADMIN);
+      userSession.checkIsRoot();
     }
   }
 }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java
index 7ac07f678af..3f6161ffc46 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java
@@ -45,7 +45,6 @@ import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.web.UserRole;
-import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.component.ComponentDto;
@@ -130,13 +129,9 @@ public class SetAction implements SettingsWsAction {
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    DbSession dbSession = dbClient.openSession(false);
-    try {
+    try (DbSession dbSession = dbClient.openSession(false)) {
       doHandle(dbSession, toWsRequest(request));
-    } finally {
-      dbClient.closeSession(dbSession);
     }
-
     response.noContent();
   }
 
@@ -273,7 +268,7 @@ public class SetAction implements SettingsWsAction {
     if (component.isPresent()) {
       userSession.checkComponentPermission(UserRole.ADMIN, component.get());
     } else {
-      userSession.checkPermission(GlobalPermissions.SYSTEM_ADMIN);
+      userSession.checkIsRoot();
     }
   }
 
diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/GenerateSecretKeyActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/GenerateSecretKeyActionTest.java
index 1214adadbc8..27e8dce948a 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/GenerateSecretKeyActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/GenerateSecretKeyActionTest.java
@@ -40,23 +40,19 @@ import org.sonarqube.ws.MediaTypes;
 import org.sonarqube.ws.Settings.GenerateSecretKeyWsResponse;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.sonar.core.permission.GlobalPermissions.QUALITY_PROFILE_ADMIN;
-import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN;
 
 public class GenerateSecretKeyActionTest {
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
   @Rule
-  public UserSessionRule userSession = UserSessionRule.standalone().setGlobalPermissions(SYSTEM_ADMIN);
+  public UserSessionRule userSession = UserSessionRule.standalone().login().setRoot();
   @Rule
   public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
-  Settings settings = new MapSettings();
-  Encryption encryption = settings.getEncryption();
-
-  GenerateSecretKeyAction underTest = new GenerateSecretKeyAction(settings, userSession);
-
-  WsActionTester ws = new WsActionTester(underTest);
+  private Settings settings = new MapSettings();
+  private Encryption encryption = settings.getEncryption();
+  private GenerateSecretKeyAction underTest = new GenerateSecretKeyAction(settings, userSession);
+  private WsActionTester ws = new WsActionTester(underTest);
 
   @Test
   public void generate_valid_secret_key() throws IOException {
@@ -83,14 +79,16 @@ public class GenerateSecretKeyActionTest {
   }
 
   @Test
-  public void fail_if_insufficient_permissions() {
-    expectedException.expect(ForbiddenException.class);
+  public void throw_ForbiddenException_if_not_root() {
+    userSession.login();
 
-    userSession.anonymous().setGlobalPermissions(QUALITY_PROFILE_ADMIN);
+    expectedException.expect(ForbiddenException.class);
+    expectedException.expectMessage("Insufficient privileges");
 
     call();
   }
 
+
   private GenerateSecretKeyWsResponse call() {
     TestRequest request = ws.newRequest()
       .setMediaType(MediaTypes.PROTOBUF)
diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java
index 21a0910f947..b0d418329e6 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java
@@ -29,7 +29,6 @@ import org.sonar.api.config.PropertyDefinition;
 import org.sonar.api.config.PropertyDefinitions;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.System2;
-import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
@@ -55,7 +54,6 @@ import static org.sonar.api.resources.Qualifiers.PROJECT;
 import static org.sonar.api.resources.Qualifiers.VIEW;
 import static org.sonar.api.web.UserRole.ADMIN;
 import static org.sonar.api.web.UserRole.USER;
-import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN;
 import static org.sonar.db.component.ComponentTesting.newProjectDto;
 import static org.sonar.db.property.PropertyTesting.newComponentPropertyDto;
 import static org.sonar.db.property.PropertyTesting.newGlobalPropertyDto;
@@ -72,21 +70,18 @@ public class ResetActionTest {
   @Rule
   public DbTester db = DbTester.create(System2.INSTANCE);
 
-  I18nRule i18n = new I18nRule();
-
-  PropertyDbTester propertyDb = new PropertyDbTester(db);
-  ComponentDbTester componentDb = new ComponentDbTester(db);
-  DbClient dbClient = db.getDbClient();
-  DbSession dbSession = db.getSession();
-  ComponentFinder componentFinder = new ComponentFinder(dbClient);
-  PropertyDefinitions definitions = new PropertyDefinitions();
-  SettingsUpdater settingsUpdater = new SettingsUpdater(dbClient, definitions);
-  SettingValidations settingValidations = new SettingValidations(definitions, dbClient, i18n);
-
-  ComponentDto project;
-
-  ResetAction underTest = new ResetAction(dbClient, componentFinder, settingsUpdater, userSession, definitions, settingValidations);
-  WsActionTester ws = new WsActionTester(underTest);
+  private I18nRule i18n = new I18nRule();
+  private PropertyDbTester propertyDb = new PropertyDbTester(db);
+  private ComponentDbTester componentDb = new ComponentDbTester(db);
+  private DbClient dbClient = db.getDbClient();
+  private DbSession dbSession = db.getSession();
+  private ComponentFinder componentFinder = new ComponentFinder(dbClient);
+  private PropertyDefinitions definitions = new PropertyDefinitions();
+  private SettingsUpdater settingsUpdater = new SettingsUpdater(dbClient, definitions);
+  private SettingValidations settingValidations = new SettingValidations(definitions, dbClient, i18n);
+  private ComponentDto project;
+  private ResetAction underTest = new ResetAction(dbClient, componentFinder, settingsUpdater, userSession, definitions, settingValidations);
+  private WsActionTester ws = new WsActionTester(underTest);
 
   @Before
   public void setUp() throws Exception {
@@ -95,7 +90,7 @@ public class ResetActionTest {
 
   @Test
   public void remove_global_setting() throws Exception {
-    setUserAsSystemAdmin();
+    logInAsRoot();
     definitions.addComponent(PropertyDefinition.builder("foo").build());
     propertyDb.insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one"));
 
@@ -105,7 +100,7 @@ public class ResetActionTest {
 
   @Test
   public void remove_global_setting_even_if_not_defined() throws Exception {
-    setUserAsSystemAdmin();
+    logInAsRoot();
     propertyDb.insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one"));
 
     executeRequestOnGlobalSetting("foo");
@@ -114,7 +109,7 @@ public class ResetActionTest {
 
   @Test
   public void remove_component_setting() throws Exception {
-    setUserAsProjectAdmin();
+    logInAsProjectAdmin();
     definitions.addComponent(PropertyDefinition.builder("foo").onQualifiers(PROJECT).build());
     propertyDb.insertProperties(newComponentPropertyDto(project).setKey("foo").setValue("value"));
 
@@ -124,7 +119,7 @@ public class ResetActionTest {
 
   @Test
   public void remove_component_setting_even_if_not_defined() throws Exception {
-    setUserAsProjectAdmin();
+    logInAsProjectAdmin();
     propertyDb.insertProperties(newComponentPropertyDto(project).setKey("foo").setValue("value"));
 
     executeRequestOnProjectSetting("foo");
@@ -133,7 +128,7 @@ public class ResetActionTest {
 
   @Test
   public void remove_hidden_setting() throws Exception {
-    setUserAsSystemAdmin();
+    logInAsRoot();
     definitions.addComponent(PropertyDefinition.builder("foo").hidden().build());
     propertyDb.insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one"));
 
@@ -143,7 +138,7 @@ public class ResetActionTest {
 
   @Test
   public void ignore_project_setting_when_removing_global_setting() throws Exception {
-    setUserAsSystemAdmin();
+    logInAsRoot();
     propertyDb.insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one"));
     propertyDb.insertProperties(newComponentPropertyDto(project).setKey("foo").setValue("value"));
 
@@ -155,7 +150,7 @@ public class ResetActionTest {
 
   @Test
   public void ignore_global_setting_when_removing_project_setting() throws Exception {
-    setUserAsProjectAdmin();
+    logInAsProjectAdmin();
     propertyDb.insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one"));
     propertyDb.insertProperties(newComponentPropertyDto(project).setKey("foo").setValue("value"));
 
@@ -167,7 +162,7 @@ public class ResetActionTest {
 
   @Test
   public void ignore_user_setting_when_removing_global_setting() throws Exception {
-    setUserAsSystemAdmin();
+    logInAsRoot();
     UserDto user = dbClient.userDao().insert(dbSession, UserTesting.newUserDto());
     propertyDb.insertProperties(newUserPropertyDto("foo", "one", user));
 
@@ -177,7 +172,7 @@ public class ResetActionTest {
 
   @Test
   public void ignore_user_setting_when_removing_project_setting() throws Exception {
-    setUserAsProjectAdmin();
+    logInAsProjectAdmin();
     UserDto user = dbClient.userDao().insert(dbSession, UserTesting.newUserDto());
     propertyDb.insertProperties(newUserPropertyDto("foo", "one", user));
 
@@ -187,14 +182,14 @@ public class ResetActionTest {
 
   @Test
   public void ignore_unknown_setting_key() throws Exception {
-    setUserAsSystemAdmin();
+    logInAsRoot();
 
     executeRequestOnGlobalSetting("unknown");
   }
 
   @Test
   public void remove_setting_by_deprecated_key() throws Exception {
-    setUserAsSystemAdmin();
+    logInAsRoot();
     definitions.addComponent(PropertyDefinition.builder("foo").deprecatedKey("old").build());
     propertyDb.insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one"));
 
@@ -204,7 +199,7 @@ public class ResetActionTest {
 
   @Test
   public void empty_204_response() {
-    setUserAsSystemAdmin();
+    logInAsRoot();
     TestResponse result = ws.newRequest()
       .setParam("keys", "my.key")
       .execute();
@@ -224,28 +219,30 @@ public class ResetActionTest {
   }
 
   @Test
-  public void fail_when_not_system_admin() throws Exception {
-    userSession.logIn("not-admin").setGlobalPermissions(GlobalPermissions.QUALITY_GATE_ADMIN);
+  public void throw_ForbiddenException_if_global_setting_and_not_root() throws Exception {
+    userSession.logIn();
     definitions.addComponent(PropertyDefinition.builder("foo").build());
 
     expectedException.expect(ForbiddenException.class);
+    expectedException.expectMessage("Insufficient privileges");
 
     executeRequestOnGlobalSetting("foo");
   }
 
   @Test
-  public void fail_when_not_project_admin() throws Exception {
-    userSession.logIn("project-admin").addProjectUuidPermissions(USER, project.uuid());
+  public void throw_ForbiddenException_if_project_setting_and_not_project_administrator() throws Exception {
+    userSession.logIn().addProjectUuidPermissions(USER, project.uuid());
     definitions.addComponent(PropertyDefinition.builder("foo").build());
 
     expectedException.expect(ForbiddenException.class);
+    expectedException.expectMessage("Insufficient privileges");
 
     executeRequestOnComponentSetting("foo", project);
   }
 
   @Test
   public void fail_when_not_global_and_no_component() {
-    setUserAsSystemAdmin();
+    logInAsRoot();
     definitions.addComponent(PropertyDefinition.builder("foo")
       .onlyOnQualifiers(VIEW)
       .build());
@@ -258,7 +255,7 @@ public class ResetActionTest {
 
   @Test
   public void fail_when_qualifier_not_included() {
-    setUserAsSystemAdmin();
+    logInAsRoot();
     definitions.addComponent(PropertyDefinition.builder("foo")
       .onQualifiers(VIEW)
       .build());
@@ -272,7 +269,7 @@ public class ResetActionTest {
 
   @Test
   public void fail_to_reset_setting_component_when_setting_is_global() {
-    setUserAsSystemAdmin();
+    logInAsRoot();
     definitions.addComponent(PropertyDefinition.builder("foo").build());
     i18n.put("qualifier." + PROJECT, "project");
 
@@ -304,12 +301,12 @@ public class ResetActionTest {
     request.execute();
   }
 
-  private void setUserAsSystemAdmin() {
-    userSession.logIn("admin").setGlobalPermissions(SYSTEM_ADMIN);
+  private void logInAsRoot() {
+    userSession.logIn().setRoot();
   }
 
-  private void setUserAsProjectAdmin() {
-    userSession.logIn("project-admin").addProjectUuidPermissions(ADMIN, project.uuid());
+  private void logInAsProjectAdmin() {
+    userSession.logIn().addProjectUuidPermissions(ADMIN, project.uuid());
   }
 
   private void assertGlobalPropertyDoesNotExist(String key) {
diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java
index 8ab347473f9..38bc520db37 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java
@@ -37,7 +37,6 @@ import org.sonar.api.server.ws.WebService;
 import org.sonar.api.server.ws.WebService.Param;
 import org.sonar.api.utils.System2;
 import org.sonar.api.web.UserRole;
-import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
@@ -73,8 +72,8 @@ public class SetActionTest {
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
   @Rule
-  public UserSessionRule userSession = UserSessionRule.standalone()
-    .setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
+  public UserSessionRule userSession = UserSessionRule.standalone().login().setRoot();
+
   @Rule
   public DbTester db = DbTester.create(System2.INSTANCE);
   private PropertyDbTester propertyDb = new PropertyDbTester(db);
@@ -136,7 +135,7 @@ public class SetActionTest {
   @Test
   public void persist_project_property_with_project_admin_permission() {
     ComponentDto project = db.components().insertProject();
-    userSession.anonymous().addProjectUuidPermissions(UserRole.ADMIN, project.uuid());
+    userSession.login().addProjectUuidPermissions(UserRole.ADMIN, project.uuid());
 
     callForProjectSettingByKey("my.key", "my value", project.key());
 
@@ -423,9 +422,11 @@ public class SetActionTest {
   }
 
   @Test
-  public void fail_when_insufficient_privileges() {
-    userSession.anonymous().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN);
+  public void throw_ForbiddenException_if_not_root() {
+    userSession.login();
+
     expectedException.expect(ForbiddenException.class);
+    expectedException.expectMessage("Insufficient privileges");
 
     callForGlobalSetting("my.key", "my value");
   }