From b1d6d975039a1a5d9639f8d8726da1a48d5285e1 Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Wed, 7 Sep 2016 19:20:00 +0200 Subject: [PATCH] SONAR-7986 WS settings/reset check scope and qualifier --- .../sonar/server/setting/ws/ResetAction.java | 13 +++- .../sonar/server/setting/ws/SetAction.java | 32 ++++----- .../server/setting/ws/SettingValidator.java | 70 +++++++++++++++++++ .../server/setting/ws/SettingsWsModule.java | 3 +- .../server/setting/ws/ResetActionTest.java | 35 +++++++++- .../server/setting/ws/SetActionTest.java | 3 +- .../setting/ws/SettingsWsModuleTest.java | 2 +- 7 files changed, 134 insertions(+), 24 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingValidator.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ResetAction.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ResetAction.java index 7d1c636a5dd..744b5ad8913 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ResetAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ResetAction.java @@ -20,6 +20,7 @@ package org.sonar.server.setting.ws; +import com.google.common.collect.ImmutableList; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -35,9 +36,11 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; import org.sonar.server.component.ComponentFinder; +import org.sonar.server.setting.ws.SettingValidator.SettingData; import org.sonar.server.user.UserSession; import org.sonarqube.ws.client.setting.ResetRequest; +import static org.sonar.server.setting.ws.SettingValidator.validateScope; import static org.sonar.server.setting.ws.SettingsWsComponentParameters.addComponentParameters; import static org.sonarqube.ws.client.ce.CeWsParameters.PARAM_COMPONENT_KEY; import static org.sonarqube.ws.client.setting.SettingsWsParameters.ACTION_RESET; @@ -51,13 +54,16 @@ public class ResetAction implements SettingsWsAction { private final SettingsUpdater settingsUpdater; private final UserSession userSession; private final PropertyDefinitions definitions; + private final SettingValidator settingValidator; - public ResetAction(DbClient dbClient, ComponentFinder componentFinder, SettingsUpdater settingsUpdater, UserSession userSession, PropertyDefinitions definitions) { + public ResetAction(DbClient dbClient, ComponentFinder componentFinder, SettingsUpdater settingsUpdater, UserSession userSession, PropertyDefinitions definitions, + SettingValidator settingValidator) { this.dbClient = dbClient; this.settingsUpdater = settingsUpdater; this.userSession = userSession; this.componentFinder = componentFinder; this.definitions = definitions; + this.settingValidator = settingValidator; } @Override @@ -89,6 +95,11 @@ public class ResetAction implements SettingsWsAction { ResetRequest resetRequest = toWsRequest(request); Optional component = getComponent(dbSession, resetRequest); checkPermissions(component); + resetRequest.getKeys().forEach(key -> { + SettingData data = new SettingData(key, definitions.get(key), component.orElse(null)); + ImmutableList.of(validateScope(), settingValidator.validateQualifier()) + .forEach(validation -> validation.validate(data)); + }); List keys = getKeys(resetRequest); if (component.isPresent()) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java index f2e5f9a28d9..2a9cd7337ef 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java @@ -21,6 +21,7 @@ package org.sonar.server.setting.ws; import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ListMultimap; import com.google.gson.Gson; import com.google.gson.JsonSyntaxException; @@ -55,11 +56,13 @@ import org.sonar.scanner.protocol.GsonHelper; import org.sonar.server.component.ComponentFinder; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.platform.SettingsChangeNotifier; +import org.sonar.server.setting.ws.SettingValidator.SettingData; import org.sonar.server.user.UserSession; import org.sonar.server.ws.KeyExamples; import org.sonarqube.ws.client.setting.SetRequest; import static com.google.common.base.Strings.isNullOrEmpty; +import static org.sonar.server.setting.ws.SettingValidator.validateScope; import static org.sonar.server.ws.WsUtils.checkRequest; import static org.sonarqube.ws.client.setting.SettingsWsParameters.ACTION_SET; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_COMPONENT_ID; @@ -79,9 +82,10 @@ public class SetAction implements SettingsWsAction { private final UserSession userSession; private final SettingsUpdater settingsUpdater; private final SettingsChangeNotifier settingsChangeNotifier; + private final SettingValidator settingValidator; public SetAction(PropertyDefinitions propertyDefinitions, I18n i18n, DbClient dbClient, ComponentFinder componentFinder, UserSession userSession, - SettingsUpdater settingsUpdater, SettingsChangeNotifier settingsChangeNotifier) { + SettingsUpdater settingsUpdater, SettingsChangeNotifier settingsChangeNotifier, SettingValidator settingValidator) { this.propertyDefinitions = propertyDefinitions; this.i18n = i18n; this.dbClient = dbClient; @@ -89,6 +93,7 @@ public class SetAction implements SettingsWsAction { this.userSession = userSession; this.settingsUpdater = settingsUpdater; this.settingsChangeNotifier = settingsChangeNotifier; + this.settingValidator = settingValidator; } @Override @@ -173,12 +178,6 @@ public class SetAction implements SettingsWsAction { } } - private void commonChecks(SetRequest request, @Nullable PropertyDefinition definition, Optional component) { - checkValueIsSet(request); - checkGlobalOrProject(request, definition, component); - checkComponentQualifier(request, definition, component); - } - private String doHandlePropertySet(DbSession dbSession, SetRequest request, @Nullable PropertyDefinition definition, Optional component) { validatePropertySet(request, definition); @@ -207,6 +206,13 @@ public class SetAction implements SettingsWsAction { } } + private void commonChecks(SetRequest request, @Nullable PropertyDefinition definition, Optional component) { + checkValueIsSet(request); + SettingData settingData = new SettingData(request.getKey(), definition, component.orElse(null)); + ImmutableList.of(validateScope(), settingValidator.validateQualifier()).stream() + .forEach(validation -> validation.validate(settingData)); + } + private void validatePropertySet(SetRequest request, @Nullable PropertyDefinition definition) { checkRequest(definition != null, "Setting '%s' is undefined", request.getKey()); checkRequest(PropertyType.PROPERTY_SET.equals(definition.type()), "Parameter '%s' is used for setting of property set type only", PARAM_FIELD_VALUES); @@ -252,18 +258,6 @@ public class SetAction implements SettingsWsAction { "Parameter '%s' must be used for single value setting. Parameter '%s' must be used for multi value setting.", PARAM_VALUE, PARAM_VALUES); } - private static void checkGlobalOrProject(SetRequest request, @Nullable PropertyDefinition definition, Optional component) { - checkRequest(component.isPresent() || definition == null || definition.global(), "Setting '%s' cannot be global", request.getKey()); - } - - private void checkComponentQualifier(SetRequest request, @Nullable PropertyDefinition definition, Optional component) { - String qualifier = component.isPresent() ? component.get().qualifier() : ""; - checkRequest(!component.isPresent() - || definition == null - || definition.qualifiers().contains(component.get().qualifier()), - "Setting '%s' cannot be set on a %s", request.getKey(), i18n.message(Locale.ENGLISH, "qualifier." + qualifier, null)); - } - private void checkType(SetRequest request, PropertyDefinition definition) { List values = valuesFromRequest(request); Optional failingResult = values.stream() diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingValidator.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingValidator.java new file mode 100644 index 00000000000..e0930cb112c --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingValidator.java @@ -0,0 +1,70 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.setting.ws; + +import java.util.Locale; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; +import org.sonar.api.config.PropertyDefinition; +import org.sonar.api.i18n.I18n; +import org.sonar.db.component.ComponentDto; + +import static org.sonar.server.ws.WsUtils.checkRequest; + +public class SettingValidator { + private final I18n i18n; + + public SettingValidator(I18n i18n) { + this.i18n = i18n; + } + + public static SettingValidation validateScope() { + return data -> checkRequest(data.component != null || data.definition == null || data.definition.global(), "Setting '%s' cannot be global", data.key); + } + + public SettingValidation validateQualifier() { + return data -> { + String qualifier = data.component == null ? "" : data.component.qualifier(); + checkRequest(data.component == null || data.definition == null || data.definition.qualifiers().contains(data.component.qualifier()), + "Setting '%s' cannot be set on a %s", data.key, i18n.message(Locale.ENGLISH, "qualifier." + qualifier, null)); + }; + } + + @FunctionalInterface + public interface SettingValidation { + void validate(SettingData data); + } + + public static class SettingData { + private final String key; + @CheckForNull + private final PropertyDefinition definition; + @CheckForNull + private final ComponentDto component; + + public SettingData(String key, @Nullable PropertyDefinition definition, @Nullable ComponentDto component) { + this.key = key; + this.definition = definition; + this.component = component; + } + + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsWsModule.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsWsModule.java index b81ac302d98..35288677612 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsWsModule.java +++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsWsModule.java @@ -35,6 +35,7 @@ public class SettingsWsModule extends Module { EncryptAction.class, GenerateSecretKeyAction.class, CheckSecretKeyAction.class, - SettingsUpdater.class); + SettingsUpdater.class, + SettingValidator.class); } } 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 7ba924ec5a1..f0b670b33c2 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 @@ -27,6 +27,7 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.api.config.PropertyDefinition; import org.sonar.api.config.PropertyDefinitions; +import org.sonar.api.resources.Qualifiers; import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.System2; import org.sonar.db.DbClient; @@ -39,7 +40,9 @@ import org.sonar.db.property.PropertyQuery; import org.sonar.db.user.UserDto; import org.sonar.db.user.UserTesting; import org.sonar.server.component.ComponentFinder; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.i18n.I18nRule; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.TestResponse; @@ -68,6 +71,8 @@ public class ResetActionTest { @Rule public DbTester db = DbTester.create(System2.INSTANCE); + I18nRule i18n = new I18nRule(); + PropertyDbTester propertyDb = new PropertyDbTester(db); ComponentDbTester componentDb = new ComponentDbTester(db); DbClient dbClient = db.getDbClient(); @@ -75,10 +80,11 @@ public class ResetActionTest { ComponentFinder componentFinder = new ComponentFinder(dbClient); PropertyDefinitions definitions = new PropertyDefinitions(); SettingsUpdater settingsUpdater = new SettingsUpdater(dbClient, definitions); + SettingValidator settingValidator = new SettingValidator(i18n); ComponentDto project; - ResetAction underTest = new ResetAction(dbClient, componentFinder, settingsUpdater, userSession, definitions); + ResetAction underTest = new ResetAction(dbClient, componentFinder, settingsUpdater, userSession, definitions, settingValidator); WsActionTester ws = new WsActionTester(underTest); @Before @@ -206,6 +212,33 @@ public class ResetActionTest { executeRequestOnComponentSetting("foo", project); } + @Test + public void fail_when_not_global_and_no_component() { + setUserAsSystemAdmin(); + definitions.addComponent(PropertyDefinition.builder("foo") + .onlyOnQualifiers(Qualifiers.VIEW) + .build()); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("Setting 'foo' cannot be global"); + + executeRequestOnGlobalSetting("foo"); + } + + @Test + public void fail_when_qualifier_not_included() { + setUserAsSystemAdmin(); + definitions.addComponent(PropertyDefinition.builder("foo") + .onQualifiers(Qualifiers.VIEW) + .build()); + i18n.put("qualifier." + Qualifiers.PROJECT, "project"); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("Setting 'foo' cannot be set on a project"); + + executeRequestOnComponentSetting("foo", project); + } + private void executeRequestOnGlobalSetting(String key) { executeRequest(key, null, null); } 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 d9d43e6dacf..ca51572f567 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 @@ -86,8 +86,9 @@ public class SetActionTest { PropertyDefinitions propertyDefinitions = new PropertyDefinitions(); FakeSettingsNotifier settingsChangeNotifier = new FakeSettingsNotifier(dbClient); SettingsUpdater settingsUpdater = new SettingsUpdater(dbClient, propertyDefinitions); + SettingValidator settingValidator = new SettingValidator(i18n); - SetAction underTest = new SetAction(propertyDefinitions, i18n, dbClient, componentFinder, userSession, settingsUpdater, settingsChangeNotifier); + SetAction underTest = new SetAction(propertyDefinitions, i18n, dbClient, componentFinder, userSession, settingsUpdater, settingsChangeNotifier, settingValidator); WsActionTester ws = new WsActionTester(underTest); diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SettingsWsModuleTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SettingsWsModuleTest.java index 19a40eb621b..5d24b688280 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SettingsWsModuleTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SettingsWsModuleTest.java @@ -29,6 +29,6 @@ public class SettingsWsModuleTest { public void verify_count_of_added_components() { ComponentContainer container = new ComponentContainer(); new SettingsWsModule().configure(container); - assertThat(container.size()).isEqualTo(11 + 2); + assertThat(container.size()).isEqualTo(12 + 2); } } -- 2.39.5