From 7d2641802ba60023eada16beafc3bd00a06ec9e2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 31 May 2017 10:48:27 +0200 Subject: [PATCH] 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/setting/ws/SettingValidations.java | 26 ++++- .../server/setting/ws/ResetActionTest.java | 73 ++++++++++++++ .../server/setting/ws/SetActionTest.java | 96 +++++++++++++++++++ 3 files changed, 192 insertions(+), 3 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingValidations.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingValidations.java index 0da91cde788..51076887c27 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingValidations.java +++ b/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 SUPPORTED_QUALIFIERS = ImmutableSet.of(Qualifiers.PROJECT, Qualifiers.VIEW, Qualifiers.MODULE, Qualifiers.SUBVIEW); + public Consumer 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 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 values; @CheckForNull private final ComponentDto component; - public SettingData(String key, List values, @Nullable ComponentDto component) { + SettingData(String key, List values, @Nullable ComponentDto component) { this.key = requireNonNull(key); this.values = requireNonNull(values); this.component = component; 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 dbb33021482..47a48d5963d 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 @@ -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(); + } + } 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 dbf371d3e84..0463e100451 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 @@ -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(); + } } -- 2.39.5