SONAR-9351 enforce in WS settings only for limited component types

allowed components: Project, module, view or subview
this enforces the allowed types even when property has no PropertyDefinition

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

@@ -19,8 +19,10 @@
  */
 package org.sonar.server.setting.ws;
 
+import com.google.common.collect.ImmutableSet;
 import java.util.List;
 import java.util.Locale;
+import java.util.Set;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import javax.annotation.CheckForNull;
@@ -29,6 +31,8 @@ import org.sonar.api.PropertyType;
 import org.sonar.api.config.PropertyDefinition;
 import org.sonar.api.config.PropertyDefinitions;
 import org.sonar.api.i18n.I18n;
+import org.sonar.api.resources.Qualifiers;
+import org.sonar.api.resources.Scopes;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.component.ComponentDto;
@@ -59,15 +63,31 @@ public class SettingValidations {
     };
   }
 
+  private static final Set<String> SUPPORTED_QUALIFIERS = ImmutableSet.of(Qualifiers.PROJECT, Qualifiers.VIEW, Qualifiers.MODULE, Qualifiers.SUBVIEW);
+
   public Consumer<SettingData> qualifier() {
     return data -> {
       String qualifier = data.component == null ? "" : data.component.qualifier();
       PropertyDefinition definition = definitions.get(data.key);
-      checkRequest(data.component == null || definition == null || definition.qualifiers().contains(data.component.qualifier()),
+      checkRequest(checkComponentScopeAndQualifier(data, definition),
         "Setting '%s' cannot be set on a %s", data.key, i18n.message(Locale.ENGLISH, "qualifier." + qualifier, null));
     };
   }
 
+  private static boolean checkComponentScopeAndQualifier(SettingData data, @Nullable PropertyDefinition definition) {
+    ComponentDto component = data.component;
+    if (component == null) {
+      return true;
+    }
+    if (!Scopes.PROJECT.equals(component.scope())) {
+      return false;
+    }
+    if (definition == null) {
+      return SUPPORTED_QUALIFIERS.contains(component.qualifier());
+    }
+    return definition.qualifiers().contains(component.qualifier());
+  }
+
   public Consumer<SettingData> valueType() {
     return new ValueTypeValidation();
   }
@@ -76,13 +96,13 @@ public class SettingValidations {
     return !definition.global() && definition.qualifiers().isEmpty();
   }
 
-  public static class SettingData {
+  static class SettingData {
     private final String key;
     private final List<String> values;
     @CheckForNull
     private final ComponentDto component;
 
-    public SettingData(String key, List<String> values, @Nullable ComponentDto component) {
+    SettingData(String key, List<String> values, @Nullable ComponentDto component) {
       this.key = requireNonNull(key);
       this.values = requireNonNull(values);
       this.component = component;

server/sonar-server/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java

@@ -19,6 +19,7 @@
  */
 package org.sonar.server.setting.ws;
 
+import java.util.Random;
 import javax.annotation.Nullable;
 import org.junit.Before;
 import org.junit.Rule;
@@ -289,6 +290,70 @@ public class ResetActionTest {
     executeRequestOnComponentSetting("foo", project);
   }
 
+  @Test
+  public void succeed_for_property_without_definition_when_set_on_project_component() {
+    ComponentDto project = randomPublicOrPrivateProject();
+    succeedForPropertyWithoutDefinitionAndValidComponent(project, project);
+  }
+
+  @Test
+  public void succeed_for_property_without_definition_when_set_on_module_component() {
+    ComponentDto project = randomPublicOrPrivateProject();
+    ComponentDto module = db.components().insertComponent(ComponentTesting.newModuleDto(project));
+    succeedForPropertyWithoutDefinitionAndValidComponent(project, module);
+  }
+
+  @Test
+  public void fail_for_property_without_definition_when_set_on_directory_component() {
+    ComponentDto project = randomPublicOrPrivateProject();
+    ComponentDto directory = db.components().insertComponent(ComponentTesting.newDirectory(project, "A/B"));
+    failForPropertyWithoutDefinitionOnUnsupportedComponent(project, directory);
+  }
+
+  @Test
+  public void fail_for_property_without_definition_when_set_on_file_component() {
+    ComponentDto project = randomPublicOrPrivateProject();
+    ComponentDto file = db.components().insertComponent(ComponentTesting.newFileDto(project));
+    failForPropertyWithoutDefinitionOnUnsupportedComponent(project, file);
+  }
+
+  @Test
+  public void succeed_for_property_without_definition_when_set_on_view_component() {
+    ComponentDto view = db.components().insertView();
+    succeedForPropertyWithoutDefinitionAndValidComponent(view, view);
+  }
+
+  @Test
+  public void succeed_for_property_without_definition_when_set_on_subview_component() {
+    ComponentDto view = db.components().insertView();
+    ComponentDto subview = db.components().insertComponent(ComponentTesting.newSubView(view));
+    succeedForPropertyWithoutDefinitionAndValidComponent(view, subview);
+  }
+
+  @Test
+  public void fail_for_property_without_definition_when_set_on_projectCopy_component() {
+    ComponentDto view = db.components().insertView();
+    ComponentDto projectCopy = db.components().insertComponent(ComponentTesting.newProjectCopy("a", db.components().insertPrivateProject(), view));
+
+    failForPropertyWithoutDefinitionOnUnsupportedComponent(view, projectCopy);
+  }
+
+  private void succeedForPropertyWithoutDefinitionAndValidComponent(ComponentDto root, ComponentDto module) {
+    logInAsProjectAdmin(root);
+
+    executeRequestOnComponentSetting("foo", module);
+  }
+
+  private void failForPropertyWithoutDefinitionOnUnsupportedComponent(ComponentDto root, ComponentDto component) {
+    i18n.put("qualifier." + component.qualifier(), "QualifierLabel");
+    logInAsProjectAdmin(root);
+
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("Setting 'foo' cannot be set on a QualifierLabel");
+
+    executeRequestOnComponentSetting("foo", component);
+  }
+
   private void executeRequestOnGlobalSetting(String key) {
     executeRequest(key, null);
   }
@@ -319,6 +384,10 @@ public class ResetActionTest {
     userSession.logIn().addProjectPermission(ADMIN, project);
   }
 
+  private void logInAsProjectAdmin(ComponentDto root) {
+    userSession.logIn().addProjectPermission(ADMIN, root);
+  }
+
   private void assertGlobalPropertyDoesNotExist(String key) {
     assertThat(dbClient.propertiesDao().selectGlobalProperty(dbSession, key)).isNull();
   }
@@ -343,4 +412,8 @@ public class ResetActionTest {
       dbSession)).isNotEmpty();
   }
 
+  private ComponentDto randomPublicOrPrivateProject() {
+    return new Random().nextBoolean() ? db.components().insertPrivateProject() : db.components().insertPublicProject();
+  }
+
 }

server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java

@@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.gson.Gson;
 import java.net.HttpURLConnection;
 import java.util.List;
+import java.util.Random;
 import javax.annotation.Nullable;
 import org.junit.Before;
 import org.junit.Rule;
@@ -41,6 +42,7 @@ import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.component.ComponentDto;
+import org.sonar.db.component.ComponentTesting;
 import org.sonar.db.property.PropertyDbTester;
 import org.sonar.db.property.PropertyDto;
 import org.sonar.db.property.PropertyQuery;
@@ -558,6 +560,95 @@ public class SetActionTest {
     callForProjectSettingByKey("my.key", "My Value", view.key());
   }
 
+  @Test
+  public void fail_when_property_with_definition_when_component_qualifier_does_not_match() {
+    ComponentDto project = randomPublicOrPrivateProject();
+    ComponentDto file = db.components().insertComponent(ComponentTesting.newFileDto(project));
+    definitions.addComponent(PropertyDefinition
+      .builder("my.key")
+      .name("foo")
+      .description("desc")
+      .category("cat")
+      .subCategory("subCat")
+      .type(PropertyType.STRING)
+      .defaultValue("default")
+      .onQualifiers(Qualifiers.PROJECT)
+      .build());
+    i18n.put("qualifier." + file.qualifier(), "CptLabel");
+    logInAsProjectAdministrator(project);
+
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("Setting 'my.key' cannot be set on a CptLabel");
+
+    callForProjectSettingByKey("my.key", "My Value", file.key());
+  }
+
+  @Test
+  public void succeed_for_property_without_definition_when_set_on_project_component() {
+    ComponentDto project = randomPublicOrPrivateProject();
+    succeedForPropertyWithoutDefinitionAndValidComponent(project, project);
+  }
+
+  @Test
+  public void succeed_for_property_without_definition_when_set_on_module_component() {
+    ComponentDto project = randomPublicOrPrivateProject();
+    ComponentDto module = db.components().insertComponent(ComponentTesting.newModuleDto(project));
+    succeedForPropertyWithoutDefinitionAndValidComponent(project, module);
+  }
+
+  @Test
+  public void fail_for_property_without_definition_when_set_on_directory_component() {
+    ComponentDto project = randomPublicOrPrivateProject();
+    ComponentDto directory = db.components().insertComponent(ComponentTesting.newDirectory(project, "A/B"));
+    failForPropertyWithoutDefinitionOnUnsupportedComponent(project, directory);
+  }
+
+  @Test
+  public void fail_for_property_without_definition_when_set_on_file_component() {
+    ComponentDto project = randomPublicOrPrivateProject();
+    ComponentDto file = db.components().insertComponent(ComponentTesting.newFileDto(project));
+    failForPropertyWithoutDefinitionOnUnsupportedComponent(project, file);
+  }
+
+  @Test
+  public void succeed_for_property_without_definition_when_set_on_view_component() {
+    ComponentDto view = db.components().insertView();
+    succeedForPropertyWithoutDefinitionAndValidComponent(view, view);
+  }
+
+  @Test
+  public void succeed_for_property_without_definition_when_set_on_subview_component() {
+    ComponentDto view = db.components().insertView();
+    ComponentDto subview = db.components().insertComponent(ComponentTesting.newSubView(view));
+    succeedForPropertyWithoutDefinitionAndValidComponent(view, subview);
+  }
+
+  @Test
+  public void fail_for_property_without_definition_when_set_on_projectCopy_component() {
+    ComponentDto view = db.components().insertView();
+    ComponentDto projectCopy = db.components().insertComponent(ComponentTesting.newProjectCopy("a", db.components().insertPrivateProject(), view));
+
+    failForPropertyWithoutDefinitionOnUnsupportedComponent(view, projectCopy);
+  }
+
+  private void succeedForPropertyWithoutDefinitionAndValidComponent(ComponentDto project, ComponentDto module) {
+    logInAsProjectAdministrator(project);
+
+    callForProjectSettingByKey("my.key", "My Value", module.key());
+
+    assertComponentSetting("my.key", "My Value", module.getId());
+  }
+
+  private void failForPropertyWithoutDefinitionOnUnsupportedComponent(ComponentDto root, ComponentDto component) {
+    i18n.put("qualifier." + component.qualifier(), "QualifierLabel");
+    logInAsProjectAdministrator(root);
+
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("Setting 'my.key' cannot be set on a QualifierLabel");
+
+    callForProjectSettingByKey("my.key", "My Value", component.key());
+  }
+
   @Test
   public void fail_when_single_and_multi_value_provided() {
     expectedException.expect(BadRequestException.class);
@@ -901,7 +992,12 @@ public class SetActionTest {
     }
 
   }
+
   private void logInAsProjectAdministrator(ComponentDto project) {
     userSession.logIn().addProjectPermission(UserRole.ADMIN, project);
   }
+
+  private ComponentDto randomPublicOrPrivateProject() {
+    return new Random().nextBoolean() ? db.components().insertPrivateProject() : db.components().insertPublicProject();
+  }
 }