From 3c7451c35cfa599c09362de51b5ab61a1c7089d1 Mon Sep 17 00:00:00 2001 From: alain <108417558+alain-kermis-sonarsource@users.noreply.github.com> Date: Fri, 14 Oct 2022 10:32:20 +0200 Subject: [PATCH] SONAR-9539 Sanitize api/rules/create web service --- .../sonar/server/rule/ws/CreateAction.java | 29 ++++--- .../server/rule/ws/CreateActionTest.java | 76 ++++++++++++++----- .../ws/client/rules/RulesService.java | 26 ++++++- 3 files changed, 104 insertions(+), 27 deletions(-) diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/rule/ws/CreateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/rule/ws/CreateAction.java index fe17a6db147..76e134b5808 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/rule/ws/CreateAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/rule/ws/CreateAction.java @@ -21,9 +21,11 @@ package org.sonar.server.rule.ws; import com.google.common.io.Resources; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; import org.sonar.api.rule.Severity; @@ -50,16 +52,16 @@ import static org.sonar.server.ws.WsUtils.writeProtobuf; public class CreateAction implements RulesWsAction { - public static final String PARAM_CUSTOM_KEY = "custom_key"; + public static final String PARAM_CUSTOM_KEY = "customKey"; public static final String PARAM_NAME = "name"; - public static final String PARAM_DESCRIPTION = "markdown_description"; + public static final String PARAM_DESCRIPTION = "markdownDescription"; public static final String PARAM_SEVERITY = "severity"; public static final String PARAM_STATUS = "status"; - public static final String PARAM_TEMPLATE_KEY = "template_key"; + public static final String PARAM_TEMPLATE_KEY = "templateKey"; public static final String PARAM_TYPE = "type"; public static final String PARAMS = "params"; - public static final String PARAM_PREVENT_REACTIVATION = "prevent_reactivation"; + public static final String PARAM_PREVENT_REACTIVATION = "preventReactivation"; static final int KEY_MAXIMUM_LENGTH = 200; static final int NAME_MAXIMUM_LENGTH = 200; @@ -93,12 +95,15 @@ public class CreateAction implements RulesWsAction { .setRequired(true) .setMaximumLength(KEY_MAXIMUM_LENGTH) .setDescription("Key of the custom rule") - .setExampleValue("Todo_should_not_be_used"); + .setExampleValue("Todo_should_not_be_used") + .setDeprecatedKey("custom_key", "9.7"); action .createParam(PARAM_TEMPLATE_KEY) + .setRequired(true) .setDescription("Key of the template rule in order to create a custom rule (mandatory for custom rule)") - .setExampleValue("java:XPath"); + .setExampleValue("java:XPath") + .setDeprecatedKey("template_key", "9.7"); action .createParam(PARAM_NAME) @@ -111,16 +116,21 @@ public class CreateAction implements RulesWsAction { .createParam(PARAM_DESCRIPTION) .setRequired(true) .setDescription("Rule description in markdown format") - .setExampleValue("Description of my custom rule"); + .setExampleValue("Description of my custom rule") + .setDeprecatedKey("markdown_description", "9.7"); action .createParam(PARAM_SEVERITY) .setPossibleValues(Severity.ALL) + .setDefaultValue(Severity.MAJOR) .setDescription("Rule severity"); action .createParam(PARAM_STATUS) - .setPossibleValues(RuleStatus.values()) + .setPossibleValues( + Arrays.stream(RuleStatus.values()) + .filter(status -> !RuleStatus.REMOVED.equals(status)) + .collect(Collectors.toList())) .setDefaultValue(RuleStatus.READY) .setDescription("Rule status"); @@ -131,7 +141,8 @@ public class CreateAction implements RulesWsAction { .createParam(PARAM_PREVENT_REACTIVATION) .setBooleanPossibleValues() .setDefaultValue(false) - .setDescription("If set to true and if the rule has been deactivated (status 'REMOVED'), a status 409 will be returned"); + .setDescription("If set to true and if the rule has been deactivated (status 'REMOVED'), a status 409 will be returned") + .setDeprecatedKey("prevent_reactivation", "9.7"); action.createParam(PARAM_TYPE) .setPossibleValues(RuleType.names()) diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/rule/ws/CreateActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/rule/ws/CreateActionTest.java index a7049f16270..0cd990bc9b0 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/rule/ws/CreateActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/rule/ws/CreateActionTest.java @@ -51,7 +51,6 @@ import static org.mockito.Mockito.mock; import static org.sonar.api.rules.RuleType.BUG; import static org.sonar.api.rules.RuleType.CODE_SMELL; import static org.sonar.db.permission.GlobalPermission.ADMINISTER_QUALITY_PROFILES; -import static org.sonar.db.rule.RuleDescriptionSectionDto.createDefaultRuleDescriptionSection; import static org.sonar.db.rule.RuleTesting.newCustomRule; import static org.sonar.db.rule.RuleTesting.newTemplateRule; import static org.sonar.server.util.TypeValidationsTesting.newFullTypeValidations; @@ -94,10 +93,10 @@ public class CreateActionTest { db.rules().insertRuleParam(templateRule, param -> param.setName("regex").setType("STRING").setDescription("Reg ex").setDefaultValue(".*")); String result = ws.newRequest() - .setParam("custom_key", "MY_CUSTOM") - .setParam("template_key", templateRule.getKey().toString()) + .setParam("customKey", "MY_CUSTOM") + .setParam("templateKey", templateRule.getKey().toString()) .setParam("name", "My custom rule") - .setParam("markdown_description", "Description") + .setParam("markdownDescription", "Description") .setParam("severity", "MAJOR") .setParam("status", "BETA") .setParam("type", BUG.name()) @@ -131,7 +130,7 @@ public class CreateActionTest { } @Test - public void create_custom_rule_with_prevent_reactivation_param_to_true() { + public void create_custom_rule_with_preventReactivation_param_to_true() { logInAsQProfileAdministrator(); RuleDto templateRule = newTemplateRule(RuleKey.of("java", "S001")); db.rules().insert(templateRule); @@ -145,12 +144,12 @@ public class CreateActionTest { db.rules().insert(customRule); TestResponse response = ws.newRequest() - .setParam("custom_key", "MY_CUSTOM") - .setParam("template_key", templateRule.getKey().toString()) + .setParam("customKey", "MY_CUSTOM") + .setParam("templateKey", templateRule.getKey().toString()) .setParam("name", "My custom rule") - .setParam("markdown_description", "Description") + .setParam("markdownDescription", "Description") .setParam("severity", "MAJOR") - .setParam("prevent_reactivation", "true") + .setParam("preventReactivation", "true") .execute(); assertThat(response.getStatus()).isEqualTo(409); @@ -172,12 +171,12 @@ public class CreateActionTest { logInAsQProfileAdministrator(); TestRequest request = ws.newRequest() - .setParam("custom_key", "MY_CUSTOM") - .setParam("template_key", "non:existing") + .setParam("customKey", "MY_CUSTOM") + .setParam("templateKey", "non:existing") .setParam("name", "My custom rule") - .setParam("markdown_description", "Description") + .setParam("markdownDescription", "Description") .setParam("severity", "MAJOR") - .setParam("prevent_reactivation", "true"); + .setParam("preventReactivation", "true"); assertThatThrownBy(request::execute) .isInstanceOf(IllegalArgumentException.class) @@ -191,18 +190,61 @@ public class CreateActionTest { RuleDto templateRule = db.rules().insert(r -> r.setIsTemplate(true).setStatus(RuleStatus.REMOVED)); TestRequest request = ws.newRequest() - .setParam("custom_key", "MY_CUSTOM") - .setParam("template_key", templateRule.getKey().toString()) + .setParam("customKey", "MY_CUSTOM") + .setParam("templateKey", templateRule.getKey().toString()) .setParam("name", "My custom rule") - .setParam("markdown_description", "Description") + .setParam("markdownDescription", "Description") .setParam("severity", "MAJOR") - .setParam("prevent_reactivation", "true"); + .setParam("preventReactivation", "true"); assertThatThrownBy(request::execute) .isInstanceOf(IllegalArgumentException.class) .hasMessage("The template key doesn't exist: " + templateRule.getKey()); } + @Test + public void throw_IllegalArgumentException_if_status_is_removed() { + logInAsQProfileAdministrator(); + + RuleDto templateRule = newTemplateRule(RuleKey.of("java", "S001")); + + TestRequest request = ws.newRequest() + .setParam("customKey", "MY_CUSTOM") + .setParam("templateKey", templateRule.getKey().toString()) + .setParam("name", "My custom rule") + .setParam("markdownDescription", "Description") + .setParam("severity", "MAJOR") + .setParam("status", "REMOVED") + .setParam("preventReactivation", "true"); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Value of parameter 'status' (REMOVED) must be one of: [BETA, DEPRECATED, READY]"); + } + + @Test + public void status_set_to_default() { + logInAsQProfileAdministrator(); + + RuleDto templateRule = newTemplateRule(RuleKey.of("java", "S001")); + db.rules().insert(templateRule); + + String result = ws.newRequest() + .setParam("customKey", "MY_CUSTOM") + .setParam("templateKey", templateRule.getKey().toString()) + .setParam("name", "My custom rule") + .setParam("markdownDescription", "Description") + .setParam("status", "BETA") + .setParam("type", BUG.name()) + .execute().getInput(); + + assertJson(result).isSimilarTo("{\n" + + " \"rule\": {\n" + + " \"severity\": \"MAJOR\"" + + " }\n" + + "}\n"); + } + @Test public void throw_ForbiddenException_if_not_profile_administrator() { userSession.logIn(); diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/rules/RulesService.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/rules/RulesService.java index 8d486db2dd7..e6571d37b4a 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/rules/RulesService.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/rules/RulesService.java @@ -60,9 +60,33 @@ public class RulesService extends BaseService { * This is part of the internal API. * This is a POST request. * @see Further information about this action online (including a response example) - * @since 4.4 + * @since 9.7 */ public void create(CreateRequest request) { + call( + new PostRequest(path("create")) + .setParam("customKey", request.getCustomKey()) + .setParam("markdownDescription", request.getMarkdownDescription()) + .setParam("name", request.getName()) + .setParam("params", request.getParams() == null ? null : request.getParams().stream().collect(Collectors.joining(","))) + .setParam("preventReactivation", request.getPreventReactivation()) + .setParam("severity", request.getSeverity()) + .setParam("status", request.getStatus()) + .setParam("templateKey", request.getTemplateKey()) + .setParam("type", request.getType()), + CreateResponse.parser()); + } + + /** + * + * This is part of the internal API. + * This is a POST request. + * This method is used specifically for sending requests using deprecated parameters for SQ before 9.7. + * @see Further information about this action online (including a response example) + * @since 4.4 + */ + @Deprecated + public void createForSQBefore97(CreateRequest request) { call( new PostRequest(path("create")) .setParam("custom_key", request.getCustomKey()) -- 2.39.5