From 9f42cc00404d94aee0769cac6dd8bd35b71ad023 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Fri, 29 Nov 2019 14:07:05 +0100 Subject: [PATCH] SONAR-12717 add SQ category and VulnerabilityProbability to response also removed Rule fields from common proto which are useless in our case (status, lang and langName) --- .../server/security/SecurityStandards.java | 60 +++++++++------ .../sonar/server/hotspot/ws/SearchAction.java | 43 +++++------ .../server/hotspot/ws/SearchActionTest.java | 76 +++---------------- sonar-ws/src/main/protobuf/ws-hotspots.proto | 9 ++- 4 files changed, 73 insertions(+), 115 deletions(-) diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java b/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java index d24cc68f8de..b99b5ec541c 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java @@ -34,6 +34,9 @@ import org.sonar.core.util.stream.MoreCollectors; import static java.util.Arrays.asList; import static java.util.Collections.singleton; +import static org.sonar.server.security.SecurityStandards.VulnerabilityProbability.HIGH; +import static org.sonar.server.security.SecurityStandards.VulnerabilityProbability.LOW; +import static org.sonar.server.security.SecurityStandards.VulnerabilityProbability.MEDIUM; @Immutable public final class SecurityStandards { @@ -54,37 +57,49 @@ public final class SecurityStandards { SANS_TOP_25_RISKY_RESOURCE, RISKY_CWE, SANS_TOP_25_POROUS_DEFENSES, POROUS_CWE); + public enum VulnerabilityProbability { + HIGH, + MEDIUM, + LOW + } + public enum SQCategory { - SQL_INJECTION("sql-injection"), - COMMAND_INJECTION("command-injection"), - PATH_TRAVERSAL_INJECTION("path-traversal-injection"), - LDAP_INJECTION("ldap-injection"), - XPATH_INJECT("xpath-injection"), - RCE("rce"), - DOS("dos"), - SSRF("ssrf"), - CSRF("csrf"), - XSS("xss"), - LOG_INJECTION("log-injection"), - HTTP_RESPONSE_SPLITTING("http-response-splitting"), - OPEN_REDIRECT("open-redirect"), - XXE("xxe"), - OBJECT_INJECTION("object-injection"), - WEAK_CRYPTOGRAPHY("weak-cryptography"), - AUTH("auth"), - INSECURE_CONF("insecure-conf"), - FILE_MANIPULATION("file-manipulation"), - OTHERS("others"); + SQL_INJECTION("sql-injection", HIGH), + COMMAND_INJECTION("command-injection", HIGH), + PATH_TRAVERSAL_INJECTION("path-traversal-injection", HIGH), + LDAP_INJECTION("ldap-injection", LOW), + XPATH_INJECTION("xpath-injection", LOW), + RCE("rce", MEDIUM), + DOS("dos", MEDIUM), + SSRF("ssrf", MEDIUM), + CSRF("csrf", HIGH), + XSS("xss", HIGH), + LOG_INJECTION("log-injection", LOW), + HTTP_RESPONSE_SPLITTING("http-response-splitting", LOW), + OPEN_REDIRECT("open-redirect", MEDIUM), + XXE("xxe", MEDIUM), + OBJECT_INJECTION("object-injection", LOW), + WEAK_CRYPTOGRAPHY("weak-cryptography", MEDIUM), + AUTH("auth", HIGH), + INSECURE_CONF("insecure-conf", LOW), + FILE_MANIPULATION("file-manipulation", LOW), + OTHERS("others", LOW); private final String key; + private final VulnerabilityProbability vulnerability; - SQCategory(String key) { + SQCategory(String key, VulnerabilityProbability vulnerability) { this.key = key; + this.vulnerability = vulnerability; } public String getKey() { return key; } + + public VulnerabilityProbability getVulnerability() { + return vulnerability; + } } public static final Map> CWES_BY_SQ_CATEGORY = ImmutableMap.>builder() @@ -92,7 +107,7 @@ public final class SecurityStandards { .put(SQCategory.COMMAND_INJECTION, ImmutableSet.of("77", "78", "88", "214")) .put(SQCategory.PATH_TRAVERSAL_INJECTION, ImmutableSet.of("22")) .put(SQCategory.LDAP_INJECTION, ImmutableSet.of("90")) - .put(SQCategory.XPATH_INJECT, ImmutableSet.of("643")) + .put(SQCategory.XPATH_INJECTION, ImmutableSet.of("643")) .put(SQCategory.RCE, ImmutableSet.of("94", "95")) .put(SQCategory.DOS, ImmutableSet.of("400", "624")) .put(SQCategory.SSRF, ImmutableSet.of("918")) @@ -108,6 +123,7 @@ public final class SecurityStandards { .put(SQCategory.INSECURE_CONF, ImmutableSet.of("102", "215", "311", "315", "346", "614", "489", "942")) .put(SQCategory.FILE_MANIPULATION, ImmutableSet.of("97", "73")) .build(); + public static final Ordering SQ_CATEGORY_ORDERING = Ordering.explicit(Arrays.stream(SQCategory.values()).collect(Collectors.toList())); public static final Ordering SQ_CATEGORY_KEYS_ORDERING = Ordering.explicit(Arrays.stream(SQCategory.values()).map(SQCategory::getKey).collect(Collectors.toList())); private final Set standards; diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java index a2dd1046c2f..149197a0eb5 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java @@ -31,8 +31,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.search.SearchHit; -import org.sonar.api.resources.Language; -import org.sonar.api.resources.Languages; import org.sonar.api.resources.Qualifiers; import org.sonar.api.resources.Scopes; import org.sonar.api.rule.RuleKey; @@ -52,8 +50,8 @@ import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.issue.index.IssueIndex; import org.sonar.server.issue.index.IssueQuery; import org.sonar.server.organization.DefaultOrganizationProvider; +import org.sonar.server.security.SecurityStandards; import org.sonar.server.user.UserSession; -import org.sonarqube.ws.Common; import org.sonarqube.ws.Hotspots; import static com.google.common.base.Strings.nullToEmpty; @@ -72,14 +70,12 @@ public class SearchAction implements HotspotsWsAction { private final UserSession userSession; private final IssueIndex issueIndex; private final DefaultOrganizationProvider defaultOrganizationProvider; - private final Languages languages; - public SearchAction(DbClient dbClient, UserSession userSession, IssueIndex issueIndex, DefaultOrganizationProvider defaultOrganizationProvider, Languages languages) { + public SearchAction(DbClient dbClient, UserSession userSession, IssueIndex issueIndex, DefaultOrganizationProvider defaultOrganizationProvider) { this.dbClient = dbClient; this.userSession = userSession; this.issueIndex = issueIndex; this.defaultOrganizationProvider = defaultOrganizationProvider; - this.languages = languages; } @Override @@ -194,15 +190,15 @@ public class SearchAction implements HotspotsWsAction { .setProject(hotspot.getProjectKey()) .setRule(hotspot.getRuleKey().toString()); ofNullable(hotspot.getStatus()).ifPresent(builder::setStatus); -// FIXME resolution field will be added later -// ofNullable(hotspot.getResolution()).ifPresent(builder::setResolution); + // FIXME resolution field will be added later + // ofNullable(hotspot.getResolution()).ifPresent(builder::setResolution); ofNullable(hotspot.getLine()).ifPresent(builder::setLine); builder.setMessage(nullToEmpty(hotspot.getMessage())); ofNullable(hotspot.getAssigneeUuid()).ifPresent(builder::setAssignee); // FIXME Filter author only if user is member of the organization (as done in issues/search WS) -// if (data.getUserOrganizationUuids().contains(component.getOrganizationUuid())) { + // if (data.getUserOrganizationUuids().contains(component.getOrganizationUuid())) { builder.setAuthor(nullToEmpty(hotspot.getAuthorLogin())); -// } + // } builder.setCreationDate(formatDateTime(hotspot.getIssueCreationDate())); builder.setUpdateDate(formatDateTime(hotspot.getIssueUpdateDate())); @@ -237,28 +233,22 @@ public class SearchAction implements HotspotsWsAction { return; } - Common.Rules.Builder rulesBuilder = Common.Rules.newBuilder(); - Common.Rule.Builder ruleBuilder = Common.Rule.newBuilder(); + Hotspots.Rule.Builder ruleBuilder = Hotspots.Rule.newBuilder(); for (RuleDefinitionDto rule : rules) { + SecurityStandards securityStandards = SecurityStandards.fromSecurityStandards(rule.getSecurityStandards()); + SecurityStandards.SQCategory sqCategory = securityStandards.getSq() + .stream() + .min(SecurityStandards.SQ_CATEGORY_ORDERING) + .orElse(SecurityStandards.SQCategory.OTHERS); ruleBuilder .clear() .setKey(rule.getKey().toString()) .setName(nullToEmpty(rule.getName())) - .setStatus(Common.RuleStatus.valueOf(rule.getStatus().name())); - - String language = rule.getLanguage(); - if (language == null) { - ruleBuilder.setLang(""); - } else { - ruleBuilder.setLang(language); - Language lang = languages.get(language); - if (lang != null) { - ruleBuilder.setLangName(lang.getName()); - } - } - rulesBuilder.addRules(ruleBuilder.build()); + .setSecurityCategory(sqCategory.getKey()) + .setVulnerabilityProbability(sqCategory.getVulnerability().name()); + + responseBuilder.addRules(ruleBuilder.build()); } - responseBuilder.setRules(rulesBuilder.build()); } private static final class SearchResponseData { @@ -293,5 +283,6 @@ public class SearchAction implements HotspotsWsAction { Set getRules() { return rules; } + } } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java index 8273a49afee..5f955e5e4e5 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java @@ -28,9 +28,6 @@ import java.util.stream.IntStream; import org.junit.Rule; import org.junit.Test; import org.sonar.api.issue.Issue; -import org.sonar.api.resources.AbstractLanguage; -import org.sonar.api.resources.Languages; -import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.System2; import org.sonar.api.web.UserRole; @@ -54,8 +51,6 @@ import org.sonar.server.permission.index.WebAuthorizationTypeSupport; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.WsActionTester; -import org.sonarqube.ws.Common; -import org.sonarqube.ws.Common.RuleStatus; import org.sonarqube.ws.Hotspots; import org.sonarqube.ws.Hotspots.Component; import org.sonarqube.ws.Hotspots.SearchWsResponse; @@ -63,8 +58,6 @@ import org.sonarqube.ws.Hotspots.SearchWsResponse; import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; import static org.sonar.api.utils.DateUtils.formatDateTime; import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; @@ -88,8 +81,7 @@ public class SearchActionTest { private IssueIndexer issueIndexer = new IssueIndexer(es.client(), dbClient, new IssueIteratorFactory(dbClient)); private StartupIndexer permissionIndexer = new PermissionIndexer(dbClient, es.client(), issueIndexer); - private Languages languages = mock(Languages.class); - private SearchAction underTest = new SearchAction(dbClient, userSessionRule, issueIndex, defaultOrganizationProvider, languages); + private SearchAction underTest = new SearchAction(dbClient, userSessionRule, issueIndex, defaultOrganizationProvider); private WsActionTester actionTester = new WsActionTester(underTest); @Test @@ -158,7 +150,7 @@ public class SearchActionTest { assertThat(response.getHotspotsList()).isEmpty(); assertThat(response.getComponentsList()).isEmpty(); - assertThat(response.getRules().getRulesList()).isEmpty(); + assertThat(response.getRulesList()).isEmpty(); } @Test @@ -173,7 +165,7 @@ public class SearchActionTest { assertThat(response.getHotspotsList()).isEmpty(); assertThat(response.getComponentsList()).isEmpty(); - assertThat(response.getRules().getRulesList()).isEmpty(); + assertThat(response.getRulesList()).isEmpty(); } @Test @@ -196,7 +188,7 @@ public class SearchActionTest { assertThat(response.getHotspotsList()).isEmpty(); assertThat(response.getComponentsList()).isEmpty(); - assertThat(response.getRules().getRulesList()).isEmpty(); + assertThat(response.getRulesList()).isEmpty(); } @Test @@ -230,8 +222,8 @@ public class SearchActionTest { assertThat(response.getComponentsList()) .extracting(Component::getKey) .containsOnly(project.getKey(), fileWithHotspot.getKey()); - assertThat(response.getRules().getRulesList()) - .extracting(Common.Rule::getKey) + assertThat(response.getRulesList()) + .extracting(Hotspots.Rule::getKey) .containsOnly(Arrays.stream(hotspots).map(t -> t.getRuleKey().toString()).toArray(String[]::new)); } @@ -262,8 +254,8 @@ public class SearchActionTest { assertThat(responseProject1.getComponentsList()) .extracting(Component::getKey) .containsOnly(project1.getKey(), file1.getKey()); - assertThat(responseProject1.getRules().getRulesList()) - .extracting(Common.Rule::getKey) + assertThat(responseProject1.getRulesList()) + .extracting(Hotspots.Rule::getKey) .containsOnly(Arrays.stream(hotspots2).map(t -> t.getRuleKey().toString()).toArray(String[]::new)); SearchWsResponse responseProject2 = actionTester.newRequest() @@ -276,8 +268,8 @@ public class SearchActionTest { assertThat(responseProject2.getComponentsList()) .extracting(Component::getKey) .containsOnly(project2.getKey(), file2.getKey()); - assertThat(responseProject2.getRules().getRulesList()) - .extracting(Common.Rule::getKey) + assertThat(responseProject2.getRulesList()) + .extracting(Hotspots.Rule::getKey) .containsOnly(Arrays.stream(hotspots2).map(t -> t.getRuleKey().toString()).toArray(String[]::new)); } @@ -423,54 +415,6 @@ public class SearchActionTest { assertThat(actualFile.getPath()).isEqualTo(file.path()); } - @Test - public void returns_details_of_rule_with_language_name_when_available() { - ComponentDto project = dbTester.components().insertPublicProject(); - userSessionRule.registerComponents(project); - indexPermissions(); - ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); - String language1 = randomAlphabetic(3); - String language2 = randomAlphabetic(2); - RuleDefinitionDto rule1a = newRule(SECURITY_HOTSPOT, t -> t.setLanguage(language1)); - RuleDefinitionDto rule1b = newRule(SECURITY_HOTSPOT, t -> t.setLanguage(language1)); - RuleDefinitionDto rule2 = newRule(SECURITY_HOTSPOT, t -> t.setLanguage(language2)); - when(languages.get(language1)).thenReturn(new AbstractLanguage(language1, language1 + "_name") { - @Override - public String[] getFileSuffixes() { - return new String[0]; - } - }); - IssueDto hotspot1a = dbTester.issues().insert(rule1a, project, file, t -> t.setType(SECURITY_HOTSPOT)); - IssueDto hotspot1b = dbTester.issues().insert(rule1b, project, file, t -> t.setType(SECURITY_HOTSPOT)); - IssueDto hotspot2 = dbTester.issues().insert(rule2, project, file, t -> t.setType(SECURITY_HOTSPOT)); - indexIssues(); - - SearchWsResponse response = actionTester.newRequest() - .setParam("projectKey", project.getKey()) - .executeProtobuf(SearchWsResponse.class); - - assertThat(response.getHotspotsList()).hasSize(3); - assertThat(response.getRules().getRulesList()).hasSize(3); - Map rulesByKey = response.getRules().getRulesList() - .stream() - .collect(uniqueIndex(t -> RuleKey.parse(t.getKey()))); - Common.Rule actualRule1a = rulesByKey.get(hotspot1a.getRuleKey()); - assertThat(actualRule1a.getName()).isEqualTo(rule1a.getName()); - assertThat(actualRule1a.getLang()).isEqualTo(rule1a.getLanguage()); - assertThat(actualRule1a.getLangName()).isEqualTo(rule1a.getLanguage() + "_name"); - assertThat(actualRule1a.getStatus()).isEqualTo(RuleStatus.valueOf(rule1a.getStatus().name())); - Common.Rule actualRule1b = rulesByKey.get(hotspot1b.getRuleKey()); - assertThat(actualRule1b.getName()).isEqualTo(rule1b.getName()); - assertThat(actualRule1b.getLang()).isEqualTo(rule1b.getLanguage()); - assertThat(actualRule1b.getLangName()).isEqualTo(rule1b.getLanguage() + "_name"); - assertThat(actualRule1b.getStatus()).isEqualTo(RuleStatus.valueOf(rule1b.getStatus().name())); - Common.Rule actualRule2 = rulesByKey.get(hotspot2.getRuleKey()); - assertThat(actualRule2.getName()).isEqualTo(rule2.getName()); - assertThat(actualRule2.getLang()).isEqualTo(rule2.getLanguage()); - assertThat(actualRule2.hasLangName()).isFalse(); - assertThat(actualRule2.getStatus()).isEqualTo(RuleStatus.valueOf(rule2.getStatus().name())); - } - private void indexPermissions() { permissionIndexer.indexOnStartup(permissionIndexer.getIndexTypes()); } diff --git a/sonar-ws/src/main/protobuf/ws-hotspots.proto b/sonar-ws/src/main/protobuf/ws-hotspots.proto index 6630860bdff..a6d71dffee1 100644 --- a/sonar-ws/src/main/protobuf/ws-hotspots.proto +++ b/sonar-ws/src/main/protobuf/ws-hotspots.proto @@ -31,7 +31,7 @@ message SearchWsResponse { optional sonarqube.ws.commons.Paging paging = 1; repeated Hotspot hotspots = 2; repeated Component components = 3; - optional sonarqube.ws.commons.Rules rules = 4; + repeated Rule rules = 4; } message Hotspot { @@ -59,3 +59,10 @@ message Component { optional string longName = 5; optional string path = 6; } + +message Rule { + optional string key = 1; + optional string name = 2; + optional string securityCategory = 3; + optional string vulnerabilityProbability = 4; +} -- 2.39.5