From bc37c508122484e913e16635ba5739c9ae27aa07 Mon Sep 17 00:00:00 2001 From: Jeremy Davis Date: Thu, 30 Apr 2020 08:45:59 +0200 Subject: [PATCH] SONAR-13128 Add security review rating to security report --- .../SecurityStandardCategoryStatistics.java | 6 ++- .../sonar/server/issue/index/IssueIndex.java | 8 ++- .../index/IssueIndexSecurityReportsTest.java | 54 +++++++++---------- sonar-ws/src/main/protobuf/ws-security.proto | 12 +++-- 4 files changed, 46 insertions(+), 34 deletions(-) diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/SecurityStandardCategoryStatistics.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/SecurityStandardCategoryStatistics.java index ea54b187792..99bf1ae29ae 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/SecurityStandardCategoryStatistics.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/SecurityStandardCategoryStatistics.java @@ -30,17 +30,19 @@ public class SecurityStandardCategoryStatistics { private final OptionalInt vulnerabiliyRating; private final long toReviewSecurityHotspots; private final long reviewedSecurityHotspots; + private final Integer securityReviewRating; private final List children; private long activeRules; private long totalRules; public SecurityStandardCategoryStatistics(String category, long vulnerabilities, OptionalInt vulnerabiliyRating, long toReviewSecurityHotspots, - long reviewedSecurityHotspots, @Nullable List children) { + long reviewedSecurityHotspots, Integer securityReviewRating, @Nullable List children) { this.category = category; this.vulnerabilities = vulnerabilities; this.vulnerabiliyRating = vulnerabiliyRating; this.toReviewSecurityHotspots = toReviewSecurityHotspots; this.reviewedSecurityHotspots = reviewedSecurityHotspots; + this.securityReviewRating = securityReviewRating; this.children = children; } @@ -64,6 +66,8 @@ public class SecurityStandardCategoryStatistics { return reviewedSecurityHotspots; } + public Integer getSecurityReviewRating() { return securityReviewRating; } + public List getChildren() { return children; } diff --git a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java index 58c00a5fa8d..e076722fa16 100644 --- a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java +++ b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java @@ -86,6 +86,7 @@ import org.sonar.server.es.searchrequest.TopAggregationDefinition; import org.sonar.server.es.searchrequest.TopAggregationDefinition.SimpleFieldFilterScope; import org.sonar.server.es.searchrequest.TopAggregationHelper; import org.sonar.server.issue.index.IssueQuery.PeriodStart; +import org.sonar.server.measure.Rating; import org.sonar.server.permission.index.AuthorizationDoc; import org.sonar.server.permission.index.WebAuthorizationTypeSupport; import org.sonar.server.security.SecurityStandards; @@ -163,6 +164,8 @@ import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_TAGS import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_TYPE; import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_VULNERABILITY_PROBABILITY; import static org.sonar.server.issue.index.IssueIndexDefinition.TYPE_ISSUE; +import static org.sonar.server.security.SecurityReviewRating.computePercent; +import static org.sonar.server.security.SecurityReviewRating.computeRating; import static org.sonar.server.security.SecurityStandards.SANS_TOP_25_INSECURE_INTERACTION; import static org.sonar.server.security.SecurityStandards.SANS_TOP_25_POROUS_DEFENSES; import static org.sonar.server.security.SecurityStandards.SANS_TOP_25_RISKY_RESOURCE; @@ -1024,8 +1027,11 @@ public class IssueIndex { long reviewedSecurityHotspots = ((InternalValueCount) ((InternalFilter) categoryBucket.getAggregations().get(AGG_REVIEWED_SECURITY_HOTSPOTS)).getAggregations().get(AGG_COUNT)) .getValue(); + Optional percent = computePercent(toReviewSecurityHotspots, reviewedSecurityHotspots); + Integer securityReviewRating = computeRating(percent.orElse(null)).getIndex(); + return new SecurityStandardCategoryStatistics(categoryName, vulnerabilities, severityRating, toReviewSecurityHotspots, - reviewedSecurityHotspots, children); + reviewedSecurityHotspots, securityReviewRating, children); } private static AggregationBuilder newSecurityReportSubAggregations(AggregationBuilder categoriesAggs, boolean includeCwe, Optional> cwesInCategory) { diff --git a/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexSecurityReportsTest.java b/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexSecurityReportsTest.java index 47915fff2cf..b101d6e4a15 100644 --- a/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexSecurityReportsTest.java +++ b/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexSecurityReportsTest.java @@ -176,18 +176,18 @@ public class IssueIndexSecurityReportsTest { assertThat(cweByOwasp.get("a1")).extracting(SecurityStandardCategoryStatistics::getCategory, SecurityStandardCategoryStatistics::getVulnerabilities, SecurityStandardCategoryStatistics::getVulnerabiliyRating, SecurityStandardCategoryStatistics::getToReviewSecurityHotspots, - SecurityStandardCategoryStatistics::getReviewedSecurityHotspots) + SecurityStandardCategoryStatistics::getReviewedSecurityHotspots, SecurityStandardCategoryStatistics::getSecurityReviewRating) .containsExactlyInAnyOrder( - tuple("123", 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 0L, 0L), - tuple("456", 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 0L, 0L), - tuple("unknown", 0L, OptionalInt.empty(), 1L /* openhotspot1 */, 0L)); + tuple("123", 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 0L, 0L, 1), + tuple("456", 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 0L, 0L, 1), + tuple("unknown", 0L, OptionalInt.empty(), 1L /* openhotspot1 */, 0L, 5)); assertThat(cweByOwasp.get("a3")).extracting(SecurityStandardCategoryStatistics::getCategory, SecurityStandardCategoryStatistics::getVulnerabilities, SecurityStandardCategoryStatistics::getVulnerabiliyRating, SecurityStandardCategoryStatistics::getToReviewSecurityHotspots, - SecurityStandardCategoryStatistics::getReviewedSecurityHotspots) + SecurityStandardCategoryStatistics::getReviewedSecurityHotspots, SecurityStandardCategoryStatistics::getSecurityReviewRating) .containsExactlyInAnyOrder( - tuple("123", 2L /* openvul1, openvul2 */, OptionalInt.of(3)/* MAJOR = C */, 0L, 0L), - tuple("456", 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 0L/* toReviewHotspot */, 0L), - tuple("unknown", 0L, OptionalInt.empty(), 1L /* openhotspot1 */, 0L)); + tuple("123", 2L /* openvul1, openvul2 */, OptionalInt.of(3)/* MAJOR = C */, 0L, 0L, 1), + tuple("456", 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 0L, 0L, 1), + tuple("unknown", 0L, OptionalInt.empty(), 1L /* openhotspot1 */, 0L, 5)); } private List indexIssuesAndAssertOwaspReport(boolean includeCwe) { @@ -210,18 +210,18 @@ public class IssueIndexSecurityReportsTest { assertThat(owaspTop10Report) .extracting(SecurityStandardCategoryStatistics::getCategory, SecurityStandardCategoryStatistics::getVulnerabilities, SecurityStandardCategoryStatistics::getVulnerabiliyRating, SecurityStandardCategoryStatistics::getToReviewSecurityHotspots, - SecurityStandardCategoryStatistics::getReviewedSecurityHotspots) + SecurityStandardCategoryStatistics::getReviewedSecurityHotspots, SecurityStandardCategoryStatistics::getSecurityReviewRating) .containsExactlyInAnyOrder( - tuple("a1", 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 1L /* toreviewhotspot1 */, 0L), - tuple("a2", 0L, OptionalInt.empty(), 0L, 0L), - tuple("a3", 2L /* openvul1,openvul2 */, OptionalInt.of(3)/* MAJOR = C */, 2L/* toreviewhotspot1,toreviewhotspot2 */, 1L /* reviewedHotspot */), - tuple("a4", 0L, OptionalInt.empty(), 0L, 0L), - tuple("a5", 0L, OptionalInt.empty(), 0L, 0L), - tuple("a6", 1L /* openvul2 */, OptionalInt.of(2) /* MINOR = B */, 1L /* toreviewhotspot2 */, 0L), - tuple("a7", 0L, OptionalInt.empty(), 0L, 0L), - tuple("a8", 0L, OptionalInt.empty(), 0L, 1L /* reviewedHotspot */), - tuple("a9", 0L, OptionalInt.empty(), 0L, 0L), - tuple("a10", 0L, OptionalInt.empty(), 0L, 0L)); + tuple("a1", 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 1L /* toreviewhotspot1 */, 0L, 5), + tuple("a2", 0L, OptionalInt.empty(), 0L, 0L, 1), + tuple("a3", 2L /* openvul1,openvul2 */, OptionalInt.of(3)/* MAJOR = C */, 2L/* toreviewhotspot1,toreviewhotspot2 */, 1L /* reviewedHotspot */, 4), + tuple("a4", 0L, OptionalInt.empty(), 0L, 0L, 1), + tuple("a5", 0L, OptionalInt.empty(), 0L, 0L, 1), + tuple("a6", 1L /* openvul2 */, OptionalInt.of(2) /* MINOR = B */, 1L /* toreviewhotspot2 */, 0L, 5), + tuple("a7", 0L, OptionalInt.empty(), 0L, 0L, 1), + tuple("a8", 0L, OptionalInt.empty(), 0L, 1L /* reviewedHotspot */, 1), + tuple("a9", 0L, OptionalInt.empty(), 0L, 0L, 1), + tuple("a10", 0L, OptionalInt.empty(), 0L, 0L, 1)); return owaspTop10Report; } @@ -251,12 +251,12 @@ public class IssueIndexSecurityReportsTest { assertThat(sansTop25Report) .extracting(SecurityStandardCategoryStatistics::getCategory, SecurityStandardCategoryStatistics::getVulnerabilities, SecurityStandardCategoryStatistics::getVulnerabiliyRating, SecurityStandardCategoryStatistics::getToReviewSecurityHotspots, - SecurityStandardCategoryStatistics::getReviewedSecurityHotspots) + SecurityStandardCategoryStatistics::getReviewedSecurityHotspots, SecurityStandardCategoryStatistics::getSecurityReviewRating) .containsExactlyInAnyOrder( - tuple(SANS_TOP_25_INSECURE_INTERACTION, 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 1L /* toreviewhotspot1 */, 0L), + tuple(SANS_TOP_25_INSECURE_INTERACTION, 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 1L /* toreviewhotspot1 */, 0L, 5), tuple(SANS_TOP_25_RISKY_RESOURCE, 2L /* openvul1,openvul2 */, OptionalInt.of(3)/* MAJOR = C */, 2L/* toreviewhotspot1,toreviewhotspot2 */, - 1L /* reviewedHotspot */), - tuple(SANS_TOP_25_POROUS_DEFENSES, 1L /* openvul2 */, OptionalInt.of(2)/* MINOR = B */, 1L/* openhotspot2 */, 0L)); + 1L /* reviewedHotspot */, 4), + tuple(SANS_TOP_25_POROUS_DEFENSES, 1L /* openvul2 */, OptionalInt.of(2)/* MINOR = B */, 1L/* openhotspot2 */, 0L, 5)); assertThat(sansTop25Report).allMatch(category -> category.getChildren().isEmpty()); } @@ -292,11 +292,11 @@ public class IssueIndexSecurityReportsTest { assertThat(sansTop25Report) .extracting(SecurityStandardCategoryStatistics::getCategory, SecurityStandardCategoryStatistics::getVulnerabilities, SecurityStandardCategoryStatistics::getVulnerabiliyRating, SecurityStandardCategoryStatistics::getToReviewSecurityHotspots, - SecurityStandardCategoryStatistics::getReviewedSecurityHotspots) + SecurityStandardCategoryStatistics::getReviewedSecurityHotspots, SecurityStandardCategoryStatistics::getSecurityReviewRating) .containsExactlyInAnyOrder( - tuple(SANS_TOP_25_INSECURE_INTERACTION, 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 1L /* toreviewhotspot1 */, 0L), - tuple(SANS_TOP_25_RISKY_RESOURCE, 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 1L/* toreviewhotspot1 */, 0L), - tuple(SANS_TOP_25_POROUS_DEFENSES, 0L, OptionalInt.empty(), 0L, 0L)); + tuple(SANS_TOP_25_INSECURE_INTERACTION, 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 1L /* toreviewhotspot1 */, 0L, 5), + tuple(SANS_TOP_25_RISKY_RESOURCE, 1L /* openvul1 */, OptionalInt.of(3)/* MAJOR = C */, 1L/* toreviewhotspot1 */, 0L, 5), + tuple(SANS_TOP_25_POROUS_DEFENSES, 0L, OptionalInt.empty(), 0L, 0L, 1)); assertThat(sansTop25Report).allMatch(category -> category.getChildren().isEmpty()); } diff --git a/sonar-ws/src/main/protobuf/ws-security.proto b/sonar-ws/src/main/protobuf/ws-security.proto index 936455009cc..59bf2f9a4cd 100644 --- a/sonar-ws/src/main/protobuf/ws-security.proto +++ b/sonar-ws/src/main/protobuf/ws-security.proto @@ -37,9 +37,10 @@ message SecurityStandardCategoryStatistics { optional int64 vulnerabilityRating = 3; optional int64 toReviewSecurityHotspots = 5; optional int64 reviewedSecurityHotspots = 6; - repeated CweStatistics distribution = 7; - optional int64 activeRules = 8; - optional int64 totalRules = 9; + optional int64 securityReviewRating = 7; + repeated CweStatistics distribution = 8; + optional int64 activeRules = 9; + optional int64 totalRules = 10; } message CweStatistics { @@ -48,8 +49,9 @@ message CweStatistics { optional int64 vulnerabilityRating = 3; optional int64 toReviewSecurityHotspots = 5; optional int64 reviewedSecurityHotspots = 6; - optional int64 activeRules = 7; - optional int64 totalRules = 8; + optional int64 securityReviewRating = 7; + optional int64 activeRules = 8; + optional int64 totalRules = 9; } -- 2.39.5