SONAR-7300 Does not return secured value in property set

server/sonar-server/src/main/java/org/sonar/server/setting/ws/Setting.java

@@ -100,9 +100,10 @@ public class Setting {
     }
     ImmutableTable.Builder<String, String, String> tableBuilder = new ImmutableTable.Builder<>();
     propertySets.forEach(property -> {
-      List<String> setIdWithFieldKey = DOT_SPLITTER.splitToList(property.getKey().replace(propertyKey + ".", ""));
+      String keyWithoutSettingKey = property.getKey().replace(propertyKey + ".", "");
+      List<String> 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<String, String, String> table = tableBuilder.build();

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<ComponentDto> component) {
+  boolean isVisible(String key, @Nullable PropertyDefinition definition, Optional<ComponentDto> component) {
     return verifySecuredSetting(key, definition, component) && (verifyLicenseSetting(key, definition));
   }
 

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<Setting> settings;
     private final Optional<ComponentDto> 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<String> 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<Map<String, String>> fieldValues) {
+    private Settings.FieldValues.Builder createFieldValuesBuilder(List<Map<String, String>> fieldValues) {
       Settings.FieldValues.Builder builder = Settings.FieldValues.newBuilder();
       for (Map<String, String> propertySetMap : fieldValues) {
         builder.addFieldValuesBuilder().putAllValue(propertySetMap);
       }
       return builder;
     }
+
+    private List<Map<String, String>> filterVisiblePropertySets(List<Map<String, String>> propertySets) {
+      List<Map<String, String>> filteredPropertySets = new ArrayList<>();
+      propertySets.forEach(map -> {
+        Map<String, String> 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;
+    }
   }
 
 }

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<Setting> 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

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