From 9a4cafe8c6fae18a5fe8cac4456c75f734d88123 Mon Sep 17 00:00:00 2001 From: Zipeng WU Date: Wed, 24 Feb 2021 13:03:12 +0100 Subject: [PATCH] SONAR-14499 Support schema validation for JSON property types --- build.gradle | 1 + .../plugins/ServerPluginRepository.java | 1 + server/sonar-webserver-webapi/build.gradle | 1 + .../server/setting/ws/SettingValidations.java | 37 +++++- .../main/resources/json-schemas/security.json | 92 ++++++++++++++ .../server/setting/ws/ResetActionTest.java | 1 + .../server/setting/ws/SetActionTest.java | 118 ++++++++++++++++++ sonar-application/build.gradle | 4 +- 8 files changed, 250 insertions(+), 5 deletions(-) create mode 100644 server/sonar-webserver-webapi/src/main/resources/json-schemas/security.json diff --git a/build.gradle b/build.gradle index e8c7f1c8083..5c264e9aa17 100644 --- a/build.gradle +++ b/build.gradle @@ -251,6 +251,7 @@ subprojects { entry 'scribejava-apis' entry 'scribejava-core' } + dependency 'com.github.everit-org.json-schema:org.everit.json.schema:1.12.2' // This project is no longer maintained and was forked // by https://github.com/java-diff-utils/java-diff-utils // (io.github.java-diff-utils:java-diff-utils). diff --git a/server/sonar-webserver-api/src/main/java/org/sonar/server/plugins/ServerPluginRepository.java b/server/sonar-webserver-api/src/main/java/org/sonar/server/plugins/ServerPluginRepository.java index c9ae2d6785a..91ec5940a75 100644 --- a/server/sonar-webserver-api/src/main/java/org/sonar/server/plugins/ServerPluginRepository.java +++ b/server/sonar-webserver-api/src/main/java/org/sonar/server/plugins/ServerPluginRepository.java @@ -27,6 +27,7 @@ import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; import javax.annotation.CheckForNull; + import org.sonar.api.Plugin; import org.sonar.core.platform.PluginInfo; import org.sonar.core.platform.PluginRepository; diff --git a/server/sonar-webserver-webapi/build.gradle b/server/sonar-webserver-webapi/build.gradle index 912ab0f758d..36c65449122 100644 --- a/server/sonar-webserver-webapi/build.gradle +++ b/server/sonar-webserver-webapi/build.gradle @@ -8,6 +8,7 @@ dependencies { // please keep the list grouped by configuration and ordered by name compile 'com.google.guava:guava' + compile 'com.github.everit-org.json-schema:org.everit.json.schema' compile project(':server:sonar-ce-common') compile project(':server:sonar-ce-task') compile project(':server:sonar-db-dao') diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java index bf2ce289c4c..42518606112 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java @@ -22,7 +22,10 @@ package org.sonar.server.setting.ws; import com.google.common.collect.ImmutableSet; import com.google.gson.Gson; import com.google.gson.JsonElement; + import java.io.IOException; +import java.io.InputStream; +import java.util.Collection; import java.util.List; import java.util.Locale; import java.util.Optional; @@ -31,6 +34,11 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import javax.annotation.CheckForNull; import javax.annotation.Nullable; + +import org.everit.json.schema.ValidationException; +import org.everit.json.schema.loader.SchemaLoader; +import org.json.JSONObject; +import org.json.JSONTokener; import org.sonar.api.PropertyType; import org.sonar.api.config.PropertyDefinition; import org.sonar.api.config.PropertyDefinitions; @@ -45,10 +53,17 @@ import org.sonar.db.user.UserDto; import org.sonar.server.exceptions.BadRequestException; import static java.lang.String.format; +import static java.util.Arrays.asList; import static java.util.Objects.requireNonNull; import static org.sonar.server.exceptions.BadRequestException.checkRequest; public class SettingValidations { + private static final Collection SECURITY_JSON_PROPERTIES = asList( + "sonar.security.config.javasecurity", + "sonar.security.config.phpsecurity", + "sonar.security.config.pythonsecurity", + "sonar.security.config.roslyn.sonaranalyzer.security.cs" + ); private final PropertyDefinitions definitions; private final DbClient dbClient; private final I18n i18n; @@ -114,6 +129,7 @@ public class SettingValidations { } private class ValueTypeValidation implements Consumer { + @Override public void accept(SettingData data) { PropertyDefinition definition = definitions.get(data.key); @@ -126,7 +142,7 @@ public class SettingValidations { } else if (definition.type() == PropertyType.USER_LOGIN) { validateLogin(data); } else if (definition.type() == PropertyType.JSON) { - validateJson(data); + validateJson(data, definition); } else { validateOtherTypes(data, definition); } @@ -159,15 +175,30 @@ public class SettingValidations { } } - private void validateJson(SettingData data) { + private void validateJson(SettingData data, PropertyDefinition definition) { Optional jsonContent = data.values.stream().findFirst(); if (jsonContent.isPresent()) { try { new Gson().getAdapter(JsonElement.class).fromJson(jsonContent.get()); - } catch (IOException e) { + validateJsonSchema(jsonContent.get(), definition); + } catch (ValidationException e) { + throw new IllegalArgumentException(String.format("Provided JSON is invalid [%s]", e.getMessage())); + } catch (IOException e){ throw new IllegalArgumentException("Provided JSON is invalid"); } } } + + private void validateJsonSchema(String json, PropertyDefinition definition) { + if(SECURITY_JSON_PROPERTIES.contains(definition.key())){ + InputStream jsonSchemaInputStream = this.getClass().getClassLoader().getResourceAsStream("json-schemas/security.json"); + if(jsonSchemaInputStream != null){ + JSONObject jsonSchema = new JSONObject(new JSONTokener(jsonSchemaInputStream)); + JSONObject jsonSubject = new JSONObject(new JSONTokener(json)); + SchemaLoader.load(jsonSchema).validate(jsonSubject); + } + } + + } } } diff --git a/server/sonar-webserver-webapi/src/main/resources/json-schemas/security.json b/server/sonar-webserver-webapi/src/main/resources/json-schemas/security.json new file mode 100644 index 00000000000..8b1cd81748c --- /dev/null +++ b/server/sonar-webserver-webapi/src/main/resources/json-schemas/security.json @@ -0,0 +1,92 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Custom configuration schema", + "description": "Schema to validate custom configuration given as input to the custom configuration properties", + "definitions": { + "Interval": { + "type": "object", + "properties": { + "fromIndex": { + "type": "integer" + } + }, + "additionalProperties": false + }, + "CommonConfiguration": { + "type": "object", + "properties": { + "args": { + "type": "array", + "items": { + "type": "integer" + } + }, + "interval": { + "$ref": "#/definitions/Interval" + }, + "isMethodPrefix": { + "type": "boolean" + }, + "isShallow": { + "type": "boolean" + }, + "isWhitelist": { + "type": "boolean" + }, + "methodId": { + "type": "string" + } + }, + "required": [ + "methodId" + ], + "additionalProperties": false + }, + "RuleConfiguration": { + "type": "object", + "properties": { + "decoders": { + "type": "array", + "items": { + "$ref": "#/definitions/CommonConfiguration" + } + }, + "encoders": { + "type": "array", + "items": { + "$ref": "#/definitions/CommonConfiguration" + } + }, + "passthroughs": { + "type": "array", + "items": { + "$ref": "#/definitions/CommonConfiguration" + } + }, + "sanitizers": { + "type": "array", + "items": { + "$ref": "#/definitions/CommonConfiguration" + } + }, + "sinks": { + "type": "array", + "items": { + "$ref": "#/definitions/CommonConfiguration" + } + }, + "sources": { + "type": "array", + "items": { + "$ref": "#/definitions/CommonConfiguration" + } + } + }, + "additionalProperties": false + } + }, + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/RuleConfiguration" + } +} \ No newline at end of file diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java index c70874e8bbd..bc7b58c8c14 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java @@ -21,6 +21,7 @@ package org.sonar.server.setting.ws; import java.util.Random; import javax.annotation.Nullable; + import org.junit.Before; import org.junit.Rule; import org.junit.Test; diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/SetActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/SetActionTest.java index 7f9e3adbb14..8e9e908bb27 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/SetActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/SetActionTest.java @@ -21,14 +21,20 @@ package org.sonar.server.setting.ws; import com.google.common.collect.ImmutableMap; import com.google.gson.Gson; +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; + import java.net.HttpURLConnection; import java.util.List; import java.util.Random; import javax.annotation.Nullable; + import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; import org.sonar.api.PropertyType; import org.sonar.api.config.PropertyDefinition; import org.sonar.api.config.PropertyDefinitions; @@ -72,6 +78,7 @@ import static org.sonar.db.property.PropertyTesting.newComponentPropertyDto; import static org.sonar.db.property.PropertyTesting.newGlobalPropertyDto; import static org.sonar.db.user.UserTesting.newUserDto; +@RunWith(DataProviderRunner.class) public class SetActionTest { private static final Gson GSON = GsonHelper.create(); @@ -103,6 +110,16 @@ public class SetActionTest { userSession.logIn().setSystemAdministrator(); } + @DataProvider + public static Object[][] securityJsonProperties() { + return new Object[][] { + {"sonar.security.config.javasecurity"}, + {"sonar.security.config.phpsecurity"}, + {"sonar.security.config.pythonsecurity"}, + {"sonar.security.config.roslyn.sonaranalyzer.security.cs"} + }; + } + @Test public void empty_204_response() { TestResponse result = ws.newRequest() @@ -429,6 +446,107 @@ public class SetActionTest { .hasMessage("Provided JSON is invalid"); } + @Test + @UseDataProvider("securityJsonProperties") + public void successfully_validate_json_schema(String securityPropertyKey) { + String security_custom_config = "{\n" + + " \"S3649\": {\n" + + " \"sources\": [\n" + + " {\n" + + " \"methodId\": \"My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery\"\n" + + " }\n" + + " ],\n" + + " \"sanitizers\": [\n" + + " {\n" + + " \"methodId\": \"str_replace\"\n" + + " }\n" + + " ],\n" + + " \"sinks\": [\n" + + " {\n" + + " \"methodId\": \"mysql_query\",\n" + + " \"args\": [1]\n" + + " }\n" + + " ]\n" + + " }\n" + + "}"; + definitions.addComponent(PropertyDefinition + .builder(securityPropertyKey) + .name("foo") + .description("desc") + .category("cat") + .subCategory("subCat") + .type(PropertyType.JSON) + .build()); + + callForGlobalSetting(securityPropertyKey, security_custom_config); + + assertGlobalSetting(securityPropertyKey, security_custom_config); + } + + @Test + @UseDataProvider("securityJsonProperties") + public void fail_json_schema_validation_when_property_has_incorrect_type(String securityPropertyKey) { + String security_custom_config = "{\n" + + " \"S3649\": {\n" + + " \"sources\": [\n" + + " {\n" + + " \"methodId\": \"My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery\"\n" + + " }\n" + + " ],\n" + + " \"sinks\": [\n" + + " {\n" + + " \"methodId\": 12345,\n" + + " \"args\": [1]\n" + + " }\n" + + " ]\n" + + " }\n" + + "}"; + definitions.addComponent(PropertyDefinition + .builder(securityPropertyKey) + .name("foo") + .description("desc") + .category("cat") + .subCategory("subCat") + .type(PropertyType.JSON) + .build()); + + assertThatThrownBy(() -> callForGlobalSetting(securityPropertyKey, security_custom_config)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("S3649/sinks/0/methodId: expected type: String, found: Integer"); + } + + @Test + @UseDataProvider("securityJsonProperties") + public void fail_json_schema_validation_when_property_has_unknown_attribute(String securityPropertyKey) { + String security_custom_config = "{\n" + + " \"S3649\": {\n" + + " \"sources\": [\n" + + " {\n" + + " \"methodId\": \"My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery\"\n" + + " }\n" + + " ],\n" + + " \"unknown\": [\n" + + " {\n" + + " \"methodId\": 12345,\n" + + " \"args\": [1]\n" + + " }\n" + + " ]\n" + + " }\n" + + "}"; + definitions.addComponent(PropertyDefinition + .builder(securityPropertyKey) + .name("foo") + .description("desc") + .category("cat") + .subCategory("subCat") + .type(PropertyType.JSON) + .build()); + + assertThatThrownBy(() -> callForGlobalSetting(securityPropertyKey, security_custom_config)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("extraneous key [unknown] is not permitted"); + } + @Test public void persist_global_setting_with_non_ascii_characters() { callForGlobalSetting("my.key", "fi±∞…"); diff --git a/sonar-application/build.gradle b/sonar-application/build.gradle index fafe614870a..d7b08781919 100644 --- a/sonar-application/build.gradle +++ b/sonar-application/build.gradle @@ -164,8 +164,8 @@ zip.doFirst { } // Check the size of the archive zip.doLast { - def minLength = 237000000 - def maxLength = 257000000 + def minLength = 238000000 + def maxLength = 258000000 def length = archiveFile.get().asFile.length() if (length < minLength) -- 2.39.5