From 0f998ecdd8916c05f73c7c3e780371572c41a622 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 3 Jan 2017 11:08:28 +0100 Subject: [PATCH] SONAR-8236 Return settings for none admin users in api/settings/values --- .../sonar/server/setting/ws/ValuesAction.java | 44 +++-- .../server/setting/ws/ValuesActionTest.java | 187 ++++++++++++------ 2 files changed, 156 insertions(+), 75 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ValuesAction.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ValuesAction.java index 3c7f1b77585..887464afbd1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ValuesAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ValuesAction.java @@ -29,6 +29,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Collectors; import org.sonar.api.config.PropertyDefinition; import org.sonar.api.config.PropertyDefinitions; @@ -49,6 +50,7 @@ import org.sonarqube.ws.client.setting.ValuesRequest; import static java.lang.String.format; import static org.apache.commons.lang.StringUtils.isEmpty; import static org.sonar.api.PropertyType.PROPERTY_SET; +import static org.sonar.api.web.UserRole.USER; import static org.sonar.server.component.ComponentFinder.ParamNames.ID_AND_KEY; import static org.sonar.server.setting.ws.SettingsWsComponentParameters.addComponentParameters; import static org.sonar.server.ws.WsUtils.writeProtobuf; @@ -61,6 +63,8 @@ public class ValuesAction implements SettingsWsAction { private static final Splitter COMMA_SPLITTER = Splitter.on(","); private static final String COMMA_ENCODED_VALUE = "%2C"; + private static final String SECURED_SUFFIX_KEY = ".secured"; + private static final String LICENSE = ".license"; private final DbClient dbClient; private final ComponentFinder componentFinder; @@ -82,11 +86,14 @@ public class ValuesAction implements SettingsWsAction { .setDescription("List settings values.
" + "If no value has been set for a setting, then the default value is returned.
" + "Either '%s' or '%s' can be provided, not both.
" + - "Requires one of the following permissions: " + - "", PARAM_COMPONENT_ID, PARAM_COMPONENT_KEY) + "Requires 'Browse' permission when a component is specified
", + "To access licensed settings, authentication is required
" + + "To access secured settings, one of the following permissions is required: " + + "", + PARAM_COMPONENT_ID, PARAM_COMPONENT_KEY) .setResponseExample(getClass().getResource("values-example.json")) .setSince("6.1") .setInternal(true) @@ -107,7 +114,6 @@ public class ValuesAction implements SettingsWsAction { try (DbSession dbSession = dbClient.openSession(true)) { ValuesRequest valuesRequest = toWsRequest(request); Optional component = loadComponent(dbSession, valuesRequest); - checkAdminPermission(component); Set keys = new HashSet<>(valuesRequest.getKeys()); Map keysToDisplayMap = getKeysToDisplayMap(keys); List settings = loadSettings(dbSession, component, keysToDisplayMap.keySet()); @@ -127,26 +133,32 @@ public class ValuesAction implements SettingsWsAction { String componentId = valuesRequest.getComponentId(); String componentKey = valuesRequest.getComponentKey(); if (componentId != null || componentKey != null) { - return Optional.of(componentFinder.getByUuidOrKey(dbSession, componentId, componentKey, ID_AND_KEY)); + ComponentDto component = componentFinder.getByUuidOrKey(dbSession, componentId, componentKey, ID_AND_KEY); + userSession.checkComponentUuidPermission(USER, component.projectUuid()); + return Optional.of(component); } return Optional.empty(); } - private void checkAdminPermission(Optional component) { - if (component.isPresent()) { - userSession.checkComponentUuidPermission(UserRole.ADMIN, component.get().uuid()); - } else { - userSession.checkPermission(GlobalPermissions.SYSTEM_ADMIN); - } - } - private List loadSettings(DbSession dbSession, Optional component, Set keys) { // List of settings must be kept in the following orders : default -> global -> component List settings = new ArrayList<>(); settings.addAll(loadDefaultSettings(keys)); settings.addAll(settingsFinder.loadGlobalSettings(dbSession, keys)); component.ifPresent(componentDto -> settings.addAll(settingsFinder.loadComponentSettings(dbSession, keys, componentDto).values())); - return settings; + return settings.stream() + .filter(isVisible(component)) + .collect(Collectors.toList()); + } + + private Predicate isVisible(Optional component) { + return setting -> !setting.getKey().endsWith(SECURED_SUFFIX_KEY) + || hasAdminPermission(component) + || (setting.getKey().contains(LICENSE) && userSession.isLoggedIn()); + } + + private boolean hasAdminPermission(Optional component) { + return component.isPresent() ? userSession.hasComponentUuidPermission(UserRole.ADMIN, component.get().uuid()) : userSession.hasPermission(GlobalPermissions.SYSTEM_ADMIN); } private List loadDefaultSettings(Set keys) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ValuesActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ValuesActionTest.java index 944783bf48a..29f274a4a70 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ValuesActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ValuesActionTest.java @@ -35,7 +35,6 @@ import org.sonar.api.config.PropertyDefinitions; import org.sonar.api.config.PropertyFieldDefinition; 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.DbTester; import org.sonar.db.component.ComponentDbTester; @@ -53,7 +52,9 @@ import org.sonarqube.ws.Settings.ValuesWsResponse; import static java.util.Arrays.asList; import static org.assertj.core.api.Java6Assertions.assertThat; +import static org.sonar.api.PropertyType.LICENSE; import static org.sonar.api.web.UserRole.ADMIN; +import static org.sonar.api.web.UserRole.CODEVIEWER; import static org.sonar.api.web.UserRole.USER; import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN; import static org.sonar.db.component.ComponentTesting.newModuleDto; @@ -92,7 +93,7 @@ public class ValuesActionTest { @Test public void return_simple_value() throws Exception { - setUserAsSystemAdmin(); + setAuthenticatedUser(); definitions.addComponent(PropertyDefinition .builder("foo") .build()); @@ -111,14 +112,12 @@ public class ValuesActionTest { @Test public void return_multi_values() throws Exception { - setUserAsSystemAdmin(); - + setAuthenticatedUser(); // Property never defined, default value is returned definitions.addComponent(PropertyDefinition.builder("default") .multiValues(true) .defaultValue("one,two") .build()); - // Property defined at global level definitions.addComponent(PropertyDefinition.builder("global") .multiValues(true) @@ -143,7 +142,7 @@ public class ValuesActionTest { @Test public void return_multi_value_with_coma() throws Exception { - setUserAsSystemAdmin(); + setAuthenticatedUser(); definitions.addComponent(PropertyDefinition.builder("global").multiValues(true).build()); propertyDb.insertProperties(newGlobalPropertyDto().setKey("global").setValue("three,four%2Cfive")); @@ -157,7 +156,7 @@ public class ValuesActionTest { @Test public void return_property_set() throws Exception { - setUserAsSystemAdmin(); + setAuthenticatedUser(); definitions.addComponent(PropertyDefinition .builder("foo") .type(PropertyType.PROPERTY_SET) @@ -179,7 +178,7 @@ public class ValuesActionTest { @Test public void return_property_set_for_component() throws Exception { - setUserAsSystemAdmin(); + setUserWithBrowsePermissionOnProject(); definitions.addComponent(PropertyDefinition .builder("foo") .type(PropertyType.PROPERTY_SET) @@ -201,7 +200,7 @@ public class ValuesActionTest { @Test public void return_default_values() throws Exception { - setUserAsSystemAdmin(); + setAuthenticatedUser(); definitions.addComponent(PropertyDefinition .builder("foo") .defaultValue("default") @@ -215,7 +214,7 @@ public class ValuesActionTest { @Test public void return_global_values() throws Exception { - setUserAsSystemAdmin(); + setAuthenticatedUser(); definitions.addComponent(PropertyDefinition.builder("property").defaultValue("default").build()); propertyDb.insertProperties( // The property is overriding default value @@ -229,7 +228,7 @@ public class ValuesActionTest { @Test public void return_project_values() throws Exception { - setUserAsSystemAdmin(); + setUserWithBrowsePermissionOnProject(); definitions.addComponent(PropertyDefinition.builder("property").defaultValue("default").build()); propertyDb.insertProperties( newGlobalPropertyDto().setKey("property").setValue("one"), @@ -244,7 +243,7 @@ public class ValuesActionTest { @Test public void return_is_inherited_to_true_when_property_is_defined_only_at_global_level() throws Exception { - setUserAsSystemAdmin(); + setUserWithBrowsePermissionOnProject(); definitions.addComponent(PropertyDefinition.builder("property").defaultValue("default").build()); // The property is not defined on project propertyDb.insertProperties(newGlobalPropertyDto().setKey("property").setValue("one")); @@ -257,7 +256,7 @@ public class ValuesActionTest { @Test public void return_values_even_if_no_property_definition() throws Exception { - setUserAsSystemAdmin(); + setAuthenticatedUser(); propertyDb.insertProperties(newGlobalPropertyDto().setKey("globalPropertyWithoutDefinition").setValue("value")); ValuesWsResponse result = executeRequestForGlobalProperties("globalPropertyWithoutDefinition"); @@ -270,7 +269,7 @@ public class ValuesActionTest { @Test public void return_empty_when_property_def_exists_but_no_value() throws Exception { - setUserAsSystemAdmin(); + setAuthenticatedUser(); definitions.addComponent(PropertyDefinition .builder("foo") .build()); @@ -282,7 +281,7 @@ public class ValuesActionTest { @Test public void return_nothing_when_unknown_keys() throws Exception { - setUserAsSystemAdmin(); + setAuthenticatedUser(); definitions.addComponent(PropertyDefinition .builder("foo") .defaultValue("default") @@ -296,7 +295,7 @@ public class ValuesActionTest { @Test public void return_module_values() throws Exception { - setUserAsSystemAdmin(); + setUserWithBrowsePermissionOnProject(); ComponentDto module = componentDb.insertComponent(newModuleDto(project)); definitions.addComponent(PropertyDefinition.builder("property").defaultValue("default").build()); propertyDb.insertProperties( @@ -312,7 +311,7 @@ public class ValuesActionTest { @Test public void return_inherited_values_on_module() throws Exception { - setUserAsSystemAdmin(); + setUserWithBrowsePermissionOnProject(); ComponentDto module = componentDb.insertComponent(newModuleDto(project)); definitions.addComponents(asList( PropertyDefinition.builder("defaultProperty").defaultValue("default").build(), @@ -335,7 +334,7 @@ public class ValuesActionTest { @Test public void return_inherited_values_on_global_setting() throws Exception { - setUserAsSystemAdmin(); + setAuthenticatedUser(); definitions.addComponents(asList( PropertyDefinition.builder("defaultProperty").defaultValue("default").build(), PropertyDefinition.builder("globalProperty").build())); @@ -351,7 +350,7 @@ public class ValuesActionTest { @Test public void return_parent_value() throws Exception { - setUserAsSystemAdmin(); + setUserWithBrowsePermissionOnProject(); ComponentDto module = componentDb.insertComponent(newModuleDto(project)); ComponentDto subModule = componentDb.insertComponent(newModuleDto(module)); definitions.addComponents(asList( @@ -369,7 +368,7 @@ public class ValuesActionTest { @Test public void return_parent_values() throws Exception { - setUserAsSystemAdmin(); + setUserWithBrowsePermissionOnProject(); ComponentDto module = componentDb.insertComponent(newModuleDto(project)); ComponentDto subModule = componentDb.insertComponent(newModuleDto(module)); definitions.addComponents(asList( @@ -387,7 +386,7 @@ public class ValuesActionTest { @Test public void return_parent_field_values() throws Exception { - setUserAsSystemAdmin(); + setUserWithBrowsePermissionOnProject(); ComponentDto module = componentDb.insertComponent(newModuleDto(project)); ComponentDto subModule = componentDb.insertComponent(newModuleDto(module)); definitions.addComponent(PropertyDefinition @@ -409,7 +408,7 @@ public class ValuesActionTest { @Test public void return_no_parent_value() throws Exception { - setUserAsSystemAdmin(); + setUserWithBrowsePermissionOnProject(); ComponentDto module = componentDb.insertComponent(newModuleDto(project)); ComponentDto subModule = componentDb.insertComponent(newModuleDto(module)); definitions.addComponents(asList( @@ -433,7 +432,7 @@ public class ValuesActionTest { @Test public void return_parent_value_when_no_definition() throws Exception { - setUserAsSystemAdmin(); + setUserWithBrowsePermissionOnProject(); ComponentDto module = componentDb.insertComponent(newModuleDto(project)); propertyDb.insertProperties( newGlobalPropertyDto().setKey("foo").setValue("global"), @@ -446,7 +445,7 @@ public class ValuesActionTest { @Test public void return_value_of_deprecated_key() throws Exception { - setUserAsSystemAdmin(); + setAuthenticatedUser(); definitions.addComponent(PropertyDefinition .builder("foo") .deprecatedKey("deprecated") @@ -462,56 +461,87 @@ public class ValuesActionTest { } @Test - public void test_example_json_response() { + public void does_not_returned_secured_settings_when_not_authenticated() throws Exception { + definitions.addComponents(asList( + PropertyDefinition.builder("foo").build(), + PropertyDefinition.builder("secret.secured").build(), + PropertyDefinition.builder("plugin.license.secured").type(LICENSE).build())); + propertyDb.insertProperties( + newGlobalPropertyDto().setKey("foo").setValue("one"), + newGlobalPropertyDto().setKey("secret.secured").setValue("password"), + newGlobalPropertyDto().setKey("plugin.license.secured").setValue("ABCD")); + + ValuesWsResponse result = executeRequestForGlobalProperties("foo", "secret.secured", "plugin.license.secured"); + + assertThat(result.getSettingsList()).extracting(Settings.Setting::getKey).containsOnly("foo"); + } + + @Test + public void return_license_with_hash_settings_when_authenticated_but_not_admin() throws Exception { + setUserWithBrowsePermissionOnProject(); + definitions.addComponents(asList( + PropertyDefinition.builder("foo").build(), + PropertyDefinition.builder("secret.secured").build(), + PropertyDefinition.builder("plugin.license.secured").type(LICENSE).build())); + propertyDb.insertProperties( + newGlobalPropertyDto().setKey("foo").setValue("one"), + newGlobalPropertyDto().setKey("secret.secured").setValue("password"), + newGlobalPropertyDto().setKey("plugin.license.secured").setValue("ABCD"), + newGlobalPropertyDto().setKey("plugin.licenseHash.secured").setValue("987654321")); + + ValuesWsResponse result = executeRequestForGlobalProperties("foo", "secret.secured", "plugin.license.secured", "plugin.licenseHash.secured"); + + assertThat(result.getSettingsList()).extracting(Settings.Setting::getKey).containsOnly("foo", "plugin.license.secured", "plugin.licenseHash.secured"); + } + + @Test + public void return_secured_and_license_settings_when_system_admin() throws Exception { setUserAsSystemAdmin(); - definitions.addComponent(PropertyDefinition - .builder("sonar.test.jira") - .defaultValue("abc") - .build()); - definitions.addComponent(PropertyDefinition - .builder("sonar.autogenerated") - .multiValues(true) - .build()); - propertyDb.insertProperties(newGlobalPropertyDto().setKey("sonar.autogenerated").setValue("val1,val2,val3")); - definitions.addComponent(PropertyDefinition - .builder("sonar.demo") - .type(PropertyType.PROPERTY_SET) - .fields(PropertyFieldDefinition.build("text").name("Text").build(), - PropertyFieldDefinition.build("boolean").name("Boolean").build()) - .build()); - propertyDb.insertPropertySet("sonar.demo", null, ImmutableMap.of("text", "foo", "boolean", "true"), ImmutableMap.of("text", "bar", "boolean", "false")); + definitions.addComponents(asList( + PropertyDefinition.builder("foo").build(), + PropertyDefinition.builder("secret.secured").build(), + PropertyDefinition.builder("plugin.license.secured").type(LICENSE).build())); + propertyDb.insertProperties( + newGlobalPropertyDto().setKey("foo").setValue("one"), + newGlobalPropertyDto().setKey("secret.secured").setValue("password"), + newGlobalPropertyDto().setKey("plugin.license.secured").setValue("ABCD"), + newGlobalPropertyDto().setKey("plugin.licenseHash.secured").setValue("987654321")); - String result = ws.newRequest() - .setParam("keys", "sonar.test.jira,sonar.autogenerated,sonar.demo") - .setMediaType(JSON) - .execute() - .getInput(); + ValuesWsResponse result = executeRequestForGlobalProperties("foo", "secret.secured", "plugin.license.secured", "plugin.licenseHash.secured"); - JsonAssert.assertJson(ws.getDef().responseExampleAsString()).isSimilarTo(result); + assertThat(result.getSettingsList()).extracting(Settings.Setting::getKey).containsOnly("foo", "secret.secured", "plugin.license.secured", "plugin.licenseHash.secured"); } @Test - public void fail_when_id_and_key_are_set() throws Exception { + public void return_secured_and_license_settings_when_project_admin() throws Exception { setUserAsProjectAdmin(); + definitions.addComponents(asList( + PropertyDefinition.builder("foo").build(), + PropertyDefinition.builder("secret.secured").build(), + PropertyDefinition.builder("plugin.license.secured").type(LICENSE).build())); + propertyDb.insertProperties( + newComponentPropertyDto(project).setKey("foo").setValue("one"), + newComponentPropertyDto(project).setKey("secret.secured").setValue("password"), + newComponentPropertyDto(project).setKey("plugin.license.secured").setValue("ABCD"), + newComponentPropertyDto(project).setKey("plugin.licenseHash.secured").setValue("987654321")); - expectedException.expect(IllegalArgumentException.class); + ValuesWsResponse result = executeRequestForProjectProperties("foo", "secret.secured", "plugin.license.secured", "plugin.licenseHash.secured"); - executeRequest(project.uuid(), project.key()); + assertThat(result.getSettingsList()).extracting(Settings.Setting::getKey).containsOnly("foo", "secret.secured", "plugin.license.secured", "plugin.licenseHash.secured"); } @Test - public void fail_when_not_system_admin() throws Exception { - userSession.login("not-admin").setGlobalPermissions(GlobalPermissions.QUALITY_GATE_ADMIN); - definitions.addComponent(PropertyDefinition.builder("foo").build()); + public void fail_when_id_and_key_are_set() throws Exception { + setAuthenticatedUser(); - expectedException.expect(ForbiddenException.class); + expectedException.expect(IllegalArgumentException.class); - executeRequestForGlobalProperties("foo"); + executeRequest(project.uuid(), project.key()); } @Test - public void fail_when_not_project_admin() throws Exception { - userSession.login("project-admin").addProjectUuidPermissions(USER, project.uuid()); + public void fail_when_user_has_not_project_browse_permission() throws Exception { + userSession.login("project-admin").addProjectUuidPermissions(CODEVIEWER, project.uuid()); definitions.addComponent(PropertyDefinition.builder("foo").build()); expectedException.expect(ForbiddenException.class); @@ -521,7 +551,7 @@ public class ValuesActionTest { @Test public void fail_when_deprecated_key_and_new_key_are_used() throws Exception { - setUserAsSystemAdmin(); + setAuthenticatedUser(); definitions.addComponent(PropertyDefinition .builder("foo") .deprecatedKey("deprecated") @@ -534,6 +564,35 @@ public class ValuesActionTest { executeRequestForGlobalProperties("foo", "deprecated"); } + @Test + public void test_example_json_response() { + setAuthenticatedUser(); + definitions.addComponent(PropertyDefinition + .builder("sonar.test.jira") + .defaultValue("abc") + .build()); + definitions.addComponent(PropertyDefinition + .builder("sonar.autogenerated") + .multiValues(true) + .build()); + propertyDb.insertProperties(newGlobalPropertyDto().setKey("sonar.autogenerated").setValue("val1,val2,val3")); + definitions.addComponent(PropertyDefinition + .builder("sonar.demo") + .type(PropertyType.PROPERTY_SET) + .fields(PropertyFieldDefinition.build("text").name("Text").build(), + PropertyFieldDefinition.build("boolean").name("Boolean").build()) + .build()); + propertyDb.insertPropertySet("sonar.demo", null, ImmutableMap.of("text", "foo", "boolean", "true"), ImmutableMap.of("text", "bar", "boolean", "false")); + + String result = ws.newRequest() + .setParam("keys", "sonar.test.jira,sonar.autogenerated,sonar.demo") + .setMediaType(JSON) + .execute() + .getInput(); + + JsonAssert.assertJson(ws.getDef().responseExampleAsString()).isSimilarTo(result); + } + @Test public void test_ws_definition() { WebService.Action action = ws.getDef(); @@ -573,12 +632,22 @@ public class ValuesActionTest { } } + private void setAuthenticatedUser() { + userSession.login("user"); + } + + private void setUserWithBrowsePermissionOnProject() { + userSession.login("user").addProjectUuidPermissions(USER, project.uuid()); + } + private void setUserAsSystemAdmin() { userSession.login("admin").setGlobalPermissions(SYSTEM_ADMIN); } private void setUserAsProjectAdmin() { - userSession.login("project-admin").addProjectUuidPermissions(ADMIN, project.uuid()); + userSession.login("project-admin") + .addProjectUuidPermissions(ADMIN, project.uuid()) + .addProjectUuidPermissions(USER, project.uuid()); } private void assertSetting(Settings.Setting setting, String expectedKey, String expectedValue, boolean expectedInherited) { -- 2.39.5