From 143c3858fcee059a20afbd9d79add4ccd5e52bc9 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 5 Jan 2017 12:02:50 +0100 Subject: [PATCH] SONAR-7300 Does not return secured value in property set --- .../org/sonar/server/setting/ws/Setting.java | 5 +-- .../ws/SettingsPermissionPredicates.java | 2 +- .../sonar/server/setting/ws/ValuesAction.java | 25 +++++++++---- .../server/setting/ws/SettingsFinderTest.java | 6 ++-- .../server/setting/ws/ValuesActionTest.java | 35 +++++++++++++++++++ 5 files changed, 61 insertions(+), 12 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/Setting.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/Setting.java index 7260b1ce55b..0da8b900eb3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/Setting.java +++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/Setting.java @@ -100,9 +100,10 @@ public class Setting { } ImmutableTable.Builder tableBuilder = new ImmutableTable.Builder<>(); propertySets.forEach(property -> { - List setIdWithFieldKey = DOT_SPLITTER.splitToList(property.getKey().replace(propertyKey + ".", "")); + String keyWithoutSettingKey = property.getKey().replace(propertyKey + ".", ""); + List setIdWithFieldKey = DOT_SPLITTER.splitToList(keyWithoutSettingKey); String setId = setIdWithFieldKey.get(0); - String fieldKey = setIdWithFieldKey.get(1); + String fieldKey = keyWithoutSettingKey.replaceFirst(setId + ".", ""); tableBuilder.put(setId, fieldKey, property.getValue()); }); ImmutableTable table = tableBuilder.build(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsPermissionPredicates.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsPermissionPredicates.java index 73b7bb40a6e..53eb10147b7 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsPermissionPredicates.java +++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsPermissionPredicates.java @@ -50,7 +50,7 @@ public class SettingsPermissionPredicates { return propertyDefinition -> isVisible(propertyDefinition.key(), propertyDefinition, component); } - private boolean isVisible(String key, @Nullable PropertyDefinition definition, Optional component) { + boolean isVisible(String key, @Nullable PropertyDefinition definition, Optional component) { return verifySecuredSetting(key, definition, component) && (verifyLicenseSetting(key, definition)); } 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 f14d1866f53..804f74291d0 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 @@ -191,7 +191,7 @@ public class ValuesAction implements SettingsWsAction { })); } - private static class ValuesResponseBuilder { + private class ValuesResponseBuilder { private final List settings; private final Optional requestedComponent; @@ -233,7 +233,7 @@ public class ValuesAction implements SettingsWsAction { valueBuilder.setInherited(isDefault || !isSet); } - private static void setValue(Setting setting, Settings.Setting.Builder valueBuilder) { + private void setValue(Setting setting, Settings.Setting.Builder valueBuilder) { PropertyDefinition definition = setting.getDefinition(); String value = setting.getValue(); if (definition == null) { @@ -241,7 +241,7 @@ public class ValuesAction implements SettingsWsAction { return; } if (definition.type().equals(PROPERTY_SET)) { - valueBuilder.setFieldValues(createFieldValuesBuilder(setting.getPropertySets())); + valueBuilder.setFieldValues(createFieldValuesBuilder(filterVisiblePropertySets(setting.getPropertySets()))); } else if (definition.multiValues()) { valueBuilder.setValues(createValuesBuilder(value)); } else { @@ -260,7 +260,8 @@ public class ValuesAction implements SettingsWsAction { } if (definition.type().equals(PROPERTY_SET)) { - valueBuilder.setParentFieldValues(createFieldValuesBuilder(valueBuilder.getInherited() ? setting.getPropertySets() : parent.getPropertySets())); + valueBuilder.setParentFieldValues( + createFieldValuesBuilder(valueBuilder.getInherited() ? filterVisiblePropertySets(setting.getPropertySets()) : filterVisiblePropertySets(parent.getPropertySets()))); } else if (definition.multiValues()) { valueBuilder.setParentValues(createValuesBuilder(value)); } else { @@ -270,18 +271,30 @@ public class ValuesAction implements SettingsWsAction { settingsByParentKey.put(setting.getKey(), setting); } - private static Settings.Values.Builder createValuesBuilder(String value) { + private Settings.Values.Builder createValuesBuilder(String value) { List values = COMMA_SPLITTER.splitToList(value).stream().map(v -> v.replace(COMMA_ENCODED_VALUE, ",")).collect(Collectors.toList()); return Settings.Values.newBuilder().addAllValues(values); } - private static Settings.FieldValues.Builder createFieldValuesBuilder(List> fieldValues) { + private Settings.FieldValues.Builder createFieldValuesBuilder(List> fieldValues) { Settings.FieldValues.Builder builder = Settings.FieldValues.newBuilder(); for (Map propertySetMap : fieldValues) { builder.addFieldValuesBuilder().putAllValue(propertySetMap); } return builder; } + + private List> filterVisiblePropertySets(List> propertySets) { + List> filteredPropertySets = new ArrayList<>(); + propertySets.forEach(map -> { + Map set = new HashMap<>(); + map.entrySet().stream() + .filter(entry -> settingsPermissionPredicates.isVisible(entry.getKey(), null, requestedComponent)) + .forEach(entry -> set.put(entry.getKey(), entry.getValue())); + filteredPropertySets.add(set); + }); + return filteredPropertySets; + } } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SettingsFinderTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SettingsFinderTest.java index 83e7fe29a4f..a5f75dc3bea 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SettingsFinderTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SettingsFinderTest.java @@ -90,7 +90,7 @@ public class SettingsFinderTest { .type(PROPERTY_SET) .fields(asList( PropertyFieldDefinition.build("key").name("Key").build(), - PropertyFieldDefinition.build("size").name("Size").build())) + PropertyFieldDefinition.build("size.value").name("Size").build())) .build(), PropertyDefinition.builder("another") .type(PROPERTY_SET) @@ -99,14 +99,14 @@ public class SettingsFinderTest { insertProperties( newGlobalPropertyDto().setKey("set1").setValue("1,2"), newGlobalPropertyDto().setKey("set1.1.key").setValue("key1"), - newGlobalPropertyDto().setKey("set1.1.size").setValue("size1"), + newGlobalPropertyDto().setKey("set1.1.size.value").setValue("size1"), newGlobalPropertyDto().setKey("set1.2.key").setValue("key2"), newGlobalPropertyDto().setKey("set2").setValue("1"), newGlobalPropertyDto().setKey("another.1.key").setValue("key1")); List settings = underTest.loadGlobalSettings(dbSession, newHashSet("set1")); assertThat(settings).hasSize(1); - assertSetting(settings.get(0), "set1", "1,2", null, true, ImmutableMap.of("key", "key1", "size", "size1"), ImmutableMap.of("key", "key2")); + assertSetting(settings.get(0), "set1", "1,2", null, true, ImmutableMap.of("key", "key1", "size.value", "size1"), ImmutableMap.of("key", "key2")); } @Test 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 a6591d4334a..1207a090542 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 @@ -470,6 +470,23 @@ public class ValuesActionTest { assertThat(result.getSettingsList()).extracting(Settings.Setting::getKey).containsOnly("foo"); } + @Test + public void does_not_returned_secured_and_license_settings_in_property_set_when_not_authenticated() throws Exception { + definitions.addComponent(PropertyDefinition + .builder("foo") + .type(PropertyType.PROPERTY_SET) + .fields(asList( + PropertyFieldDefinition.build("key").name("Key").build(), + PropertyFieldDefinition.build("plugin.license.secured").name("License").type(LICENSE).build(), + PropertyFieldDefinition.build("secret.secured").name("Secured").build())) + .build()); + propertyDb.insertPropertySet("foo", null, ImmutableMap.of("key", "key1", "plugin.license.secured", "ABCD", "secret.secured", "123456")); + + ValuesWsResponse result = executeRequestForGlobalProperties("foo"); + + assertFieldValues(result.getSettings(0), ImmutableMap.of("key", "key1")); + } + @Test public void return_license_with_hash_settings_when_authenticated_but_not_admin() throws Exception { setUserWithBrowsePermissionOnProject(); @@ -526,6 +543,24 @@ public class ValuesActionTest { assertThat(result.getSettingsList()).extracting(Settings.Setting::getKey).containsOnly("foo", "secret.secured", "plugin.license.secured", "plugin.licenseHash.secured"); } + @Test + public void return_secured_and_license_settings_in_property_set_when_system_admin() throws Exception { + setUserAsSystemAdmin(); + definitions.addComponent(PropertyDefinition + .builder("foo") + .type(PropertyType.PROPERTY_SET) + .fields(asList( + PropertyFieldDefinition.build("key").name("Key").build(), + PropertyFieldDefinition.build("plugin.license.secured").name("License").type(LICENSE).build(), + PropertyFieldDefinition.build("secret.secured").name("Secured").build())) + .build()); + propertyDb.insertPropertySet("foo", null, ImmutableMap.of("key", "key1", "plugin.license.secured", "ABCD", "secret.secured", "123456")); + + ValuesWsResponse result = executeRequestForGlobalProperties("foo"); + + assertFieldValues(result.getSettings(0), ImmutableMap.of("key", "key1", "plugin.license.secured", "ABCD", "secret.secured", "123456")); + } + @Test public void return_global_settings_from_definitions_when_no_component_and_no_keys() throws Exception { setUserAsSystemAdmin(); -- 2.39.5