From 751fd7ffdb00dcf77e6fa982e6d3428dd7a6c564 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 3 Dec 2019 16:50:08 +0100 Subject: [PATCH] SONAR-12718 add WS api/hotspots/show --- .../hotspot/ws/HotspotRuleDescription.java | 123 ++++++ .../ws/HotspotWsResponseFormatter.java | 47 +++ .../server/hotspot/ws/HotspotsWsModule.java | 5 +- .../sonar/server/hotspot/ws/SearchAction.java | 34 +- .../sonar/server/hotspot/ws/ShowAction.java | 123 +++++- .../issue/TextRangeResponseFormatter.java | 83 ++++ .../sonar/server/issue/ws/IssueWsModule.java | 2 + .../server/issue/ws/SearchResponseData.java | 4 + .../server/issue/ws/SearchResponseFormat.java | 56 +-- .../ws/HotspotRuleDescriptionTest.java | 352 +++++++++++++++++ .../hotspot/ws/HotspotsWsModuleTest.java | 2 +- .../server/hotspot/ws/SearchActionTest.java | 35 +- .../server/hotspot/ws/ShowActionTest.java | 364 ++++++++++++++++++ .../server/issue/ws/IssueWsModuleTest.java | 2 +- .../issue/ws/SearchActionComponentsTest.java | 3 +- .../issue/ws/SearchActionFacetsTest.java | 3 +- .../server/issue/ws/SearchActionTest.java | 3 +- .../ws/SearchActionTestOnSonarCloud.java | 3 +- sonar-ws/src/main/protobuf/ws-hotspots.proto | 55 ++- 19 files changed, 1187 insertions(+), 112 deletions(-) create mode 100644 server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotRuleDescription.java create mode 100644 server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsResponseFormatter.java create mode 100644 server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/TextRangeResponseFormatter.java create mode 100644 server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotRuleDescriptionTest.java create mode 100644 server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotRuleDescription.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotRuleDescription.java new file mode 100644 index 00000000000..205bc723dbc --- /dev/null +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotRuleDescription.java @@ -0,0 +1,123 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info 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.hotspot.ws; + +import java.util.Optional; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; +import org.sonar.db.rule.RuleDefinitionDto; + +import static java.lang.Character.isWhitespace; +import static java.util.Optional.ofNullable; + +public class HotspotRuleDescription { + private static final HotspotRuleDescription NO_DESCRIPTION = new HotspotRuleDescription(null, null, null); + + @CheckForNull + private final String risk; + @CheckForNull + private final String vulnerable; + @CheckForNull + private final String fixIt; + + private HotspotRuleDescription(@Nullable String risk, @Nullable String vulnerable, @Nullable String fixIt) { + this.risk = risk; + this.vulnerable = vulnerable; + this.fixIt = fixIt; + } + + public static HotspotRuleDescription from(RuleDefinitionDto dto) { + String description = dto.getDescription(); + if (description == null) { + return NO_DESCRIPTION; + } + + String vulnerableTitle = "

Ask Yourself Whether

"; + String fixItTitle = "

Recommended Secure Coding Practices

"; + int vulnerableTitlePosition = description.indexOf(vulnerableTitle); + int fixItTitlePosition = description.indexOf(fixItTitle); + if (vulnerableTitlePosition == -1 && fixItTitlePosition == -1) { + return NO_DESCRIPTION; + } + + if (vulnerableTitlePosition == -1) { + return new HotspotRuleDescription( + trimingSubstring(description, 0, fixItTitlePosition), + null, + trimingSubstring(description, fixItTitlePosition, description.length()) + ); + } + if (fixItTitlePosition == -1) { + return new HotspotRuleDescription( + trimingSubstring(description, 0, vulnerableTitlePosition), + trimingSubstring(description, vulnerableTitlePosition, description.length()), + null + ); + } + return new HotspotRuleDescription( + trimingSubstring(description, 0, vulnerableTitlePosition), + trimingSubstring(description, vulnerableTitlePosition, fixItTitlePosition), + trimingSubstring(description, fixItTitlePosition, description.length()) + ); + } + + @CheckForNull + private static String trimingSubstring(String description, int beginIndex, int endIndex) { + if (beginIndex == endIndex) { + return null; + } + + int trimmedBeginIndex = beginIndex; + while (trimmedBeginIndex < endIndex && isWhitespace(description.charAt(trimmedBeginIndex))) { + trimmedBeginIndex++; + } + int trimmedEndIndex = endIndex; + while (trimmedEndIndex > 0 && trimmedEndIndex > trimmedBeginIndex && isWhitespace(description.charAt(trimmedEndIndex - 1))) { + trimmedEndIndex--; + } + if (trimmedBeginIndex == trimmedEndIndex) { + return null; + } + + return description.substring(trimmedBeginIndex, trimmedEndIndex); + } + + public Optional getRisk() { + return ofNullable(risk); + } + + public Optional getVulnerable() { + return ofNullable(vulnerable); + } + + public Optional getFixIt() { + return ofNullable(fixIt); + } + + @Override + public String toString() { + return "HotspotRuleDescription{" + + "risk='" + risk + '\'' + + ", vulnerable='" + vulnerable + '\'' + + ", fixIt='" + fixIt + '\'' + + '}'; + } + +} diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsResponseFormatter.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsResponseFormatter.java new file mode 100644 index 00000000000..0ac88897685 --- /dev/null +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsResponseFormatter.java @@ -0,0 +1,47 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info 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.hotspot.ws; + +import org.sonar.db.component.ComponentDto; +import org.sonar.server.organization.DefaultOrganizationProvider; +import org.sonarqube.ws.Hotspots; + +import static java.util.Optional.ofNullable; + +public class HotspotWsResponseFormatter { + private final DefaultOrganizationProvider defaultOrganizationProvider; + + public HotspotWsResponseFormatter(DefaultOrganizationProvider defaultOrganizationProvider) { + this.defaultOrganizationProvider = defaultOrganizationProvider; + } + + Hotspots.Component formatComponent(Hotspots.Component.Builder builder, ComponentDto component) { + builder + .clear() + .setOrganization(defaultOrganizationProvider.get().getKey()) + .setKey(component.getKey()) + .setQualifier(component.qualifier()) + .setName(component.name()) + .setLongName(component.longName()); + ofNullable(component.path()).ifPresent(builder::setPath); + return builder.build(); + } + +} diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotsWsModule.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotsWsModule.java index e04dd6d3cce..2a437420b81 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotsWsModule.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotsWsModule.java @@ -25,8 +25,9 @@ public class HotspotsWsModule extends Module { @Override protected void configureModule() { add( + HotspotWsResponseFormatter.class, SearchAction.class, - HotspotsWs.class - ); + ShowAction.class, + HotspotsWs.class); } } 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 021901007cf..250278316d8 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 @@ -51,13 +51,12 @@ import org.sonar.server.es.SearchOptions; 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 org.sonarqube.ws.Hotspots.SearchWsResponse; -import static com.google.common.base.Strings.nullToEmpty; import static java.util.Optional.ofNullable; import static org.sonar.api.server.ws.WebService.Param.PAGE; import static org.sonar.api.server.ws.WebService.Param.PAGE_SIZE; @@ -68,6 +67,7 @@ import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; import static org.sonar.server.security.SecurityStandards.fromSecurityStandards; import static org.sonar.server.ws.KeyExamples.KEY_PROJECT_EXAMPLE_001; import static org.sonar.server.ws.WsUtils.writeProtobuf; +import static org.sonarqube.ws.WsUtils.nullToEmpty; public class SearchAction implements HotspotsWsAction { private static final String PARAM_PROJECT_KEY = "projectKey"; @@ -75,13 +75,14 @@ public class SearchAction implements HotspotsWsAction { private final DbClient dbClient; private final UserSession userSession; private final IssueIndex issueIndex; - private final DefaultOrganizationProvider defaultOrganizationProvider; + private final HotspotWsResponseFormatter responseFormatter; - public SearchAction(DbClient dbClient, UserSession userSession, IssueIndex issueIndex, DefaultOrganizationProvider defaultOrganizationProvider) { + public SearchAction(DbClient dbClient, UserSession userSession, IssueIndex issueIndex, + HotspotWsResponseFormatter responseFormatter) { this.dbClient = dbClient; this.userSession = userSession; this.issueIndex = issueIndex; - this.defaultOrganizationProvider = defaultOrganizationProvider; + this.responseFormatter = responseFormatter; } @Override @@ -187,8 +188,8 @@ public class SearchAction implements HotspotsWsAction { } } - private Hotspots.SearchWsResponse formatResponse(SearchResponseData searchResponseData) { - Hotspots.SearchWsResponse.Builder responseBuilder = Hotspots.SearchWsResponse.newBuilder(); + private SearchWsResponse formatResponse(SearchResponseData searchResponseData) { + SearchWsResponse.Builder responseBuilder = SearchWsResponse.newBuilder(); formatPaging(searchResponseData, responseBuilder); if (!searchResponseData.isEmpty()) { formatHotspots(searchResponseData, responseBuilder); @@ -197,7 +198,7 @@ public class SearchAction implements HotspotsWsAction { return responseBuilder.build(); } - private void formatPaging(SearchResponseData searchResponseData, Hotspots.SearchWsResponse.Builder responseBuilder) { + private void formatPaging(SearchResponseData searchResponseData, SearchWsResponse.Builder responseBuilder) { Paging paging = searchResponseData.getPaging(); Common.Paging.Builder pagingBuilder = Common.Paging.newBuilder() .setPageIndex(paging.pageIndex()) @@ -207,13 +208,13 @@ public class SearchAction implements HotspotsWsAction { responseBuilder.setPaging(pagingBuilder.build()); } - private static void formatHotspots(SearchResponseData searchResponseData, Hotspots.SearchWsResponse.Builder responseBuilder) { + private void formatHotspots(SearchResponseData searchResponseData, SearchWsResponse.Builder responseBuilder) { List orderedHotspots = searchResponseData.getOrderedHotspots(); if (orderedHotspots.isEmpty()) { return; } - Hotspots.Hotspot.Builder builder = Hotspots.Hotspot.newBuilder(); + SearchWsResponse.Hotspot.Builder builder = SearchWsResponse.Hotspot.newBuilder(); for (IssueDto hotspot : orderedHotspots) { RuleDefinitionDto rule = searchResponseData.getRule(hotspot.getRuleKey()) // due to join with table Rule when retrieving data from Issues, this can't happen @@ -244,7 +245,7 @@ public class SearchAction implements HotspotsWsAction { } } - private void formatComponents(SearchResponseData searchResponseData, Hotspots.SearchWsResponse.Builder responseBuilder) { + private void formatComponents(SearchResponseData searchResponseData, SearchWsResponse.Builder responseBuilder) { Set components = searchResponseData.getComponents(); if (components.isEmpty()) { return; @@ -252,16 +253,7 @@ public class SearchAction implements HotspotsWsAction { Hotspots.Component.Builder builder = Hotspots.Component.newBuilder(); for (ComponentDto component : components) { - builder - .clear() - .setOrganization(defaultOrganizationProvider.get().getKey()) - .setKey(component.getKey()) - .setQualifier(component.qualifier()) - .setName(component.name()) - .setLongName(component.longName()); - ofNullable(component.path()).ifPresent(builder::setPath); - - responseBuilder.addComponents(builder.build()); + responseBuilder.addComponents(responseFormatter.formatComponent(builder, component)); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java index f384e4c2872..862f73f53ac 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java @@ -19,6 +19,9 @@ */ package org.sonar.server.hotspot.ws; +import java.util.Objects; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.RuleType; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; @@ -28,10 +31,20 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; import org.sonar.db.issue.IssueDto; +import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.issue.TextRangeResponseFormatter; +import org.sonar.server.security.SecurityStandards; import org.sonar.server.user.UserSession; +import org.sonarqube.ws.Hotspots; +import org.sonarqube.ws.Hotspots.ShowWsResponse; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Strings.nullToEmpty; import static java.lang.String.format; +import static java.util.Optional.ofNullable; +import static org.sonar.api.utils.DateUtils.formatDateTime; +import static org.sonar.server.ws.WsUtils.writeProtobuf; public class ShowAction implements HotspotsWsAction { @@ -39,10 +52,14 @@ public class ShowAction implements HotspotsWsAction { private final DbClient dbClient; private final UserSession userSession; + private final HotspotWsResponseFormatter responseFormatter; + private final TextRangeResponseFormatter textRangeFormatter; - public ShowAction(DbClient dbClient, UserSession userSession) { + public ShowAction(DbClient dbClient, UserSession userSession, HotspotWsResponseFormatter responseFormatter, TextRangeResponseFormatter textRangeFormatter) { this.dbClient = dbClient; this.userSession = userSession; + this.responseFormatter = responseFormatter; + this.textRangeFormatter = textRangeFormatter; } @Override @@ -50,8 +67,9 @@ public class ShowAction implements HotspotsWsAction { WebService.NewAction action = controller .createAction("show") .setHandler(this) - .setDescription("Provides the details of a Security Hotpot.") - .setSince("8.1"); + .setDescription("Provides the details of a Security Hotspot.") + .setSince("8.1") + .setInternal(true); action.createParam(PARAM_HOTSPOT_KEY) .setDescription("Key of the Security Hotspot") @@ -66,12 +84,103 @@ public class ShowAction implements HotspotsWsAction { String hotspotKey = request.mandatoryParam(PARAM_HOTSPOT_KEY); try (DbSession dbSession = dbClient.openSession(false)) { IssueDto hotspot = dbClient.issueDao().selectByKey(dbSession, hotspotKey) - .orElseThrow(() -> new NotFoundException(format("Hotspot with key '%s' does not exist", hotspotKey))); - ComponentDto project = dbClient.componentDao().selectByUuid(dbSession, hotspot.getProjectUuid()) - .orElseThrow(() -> new NotFoundException(format("Project with uuid '%s' does not exist", hotspot.getProjectUuid()))); - userSession.checkComponentPermission(UserRole.USER, project); + .filter(t -> t.getType() == RuleType.SECURITY_HOTSPOT.getDbConstant()) + .orElseThrow(() -> new NotFoundException(format("Hotspot '%s' does not exist", hotspotKey))); + Components components = loadComponents(dbSession, hotspot); + RuleDefinitionDto rule = loadRule(dbSession, hotspot); + ShowWsResponse.Builder responseBuilder = ShowWsResponse.newBuilder(); + formatHotspot(responseBuilder, hotspot); + formatComponents(components, responseBuilder); + formatRule(responseBuilder, rule); + formatTextRange(hotspot, responseBuilder); + + writeProtobuf(responseBuilder.build(), request, response); } } + + private void formatHotspot(ShowWsResponse.Builder builder, IssueDto hotspot) { + builder.setKey(hotspot.getKey()); + ofNullable(hotspot.getStatus()).ifPresent(builder::setStatus); + // 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())) { + builder.setAuthor(nullToEmpty(hotspot.getAuthorLogin())); + // } + builder.setCreationDate(formatDateTime(hotspot.getIssueCreationDate())); + builder.setUpdateDate(formatDateTime(hotspot.getIssueUpdateDate())); + } + + private void formatComponents(Components components, ShowWsResponse.Builder responseBuilder) { + responseBuilder + .setProject(responseFormatter.formatComponent(Hotspots.Component.newBuilder(), components.getProject())) + .setComponent(responseFormatter.formatComponent(Hotspots.Component.newBuilder(), components.getComponent())); + } + + private void formatRule(ShowWsResponse.Builder responseBuilder, RuleDefinitionDto ruleDefinitionDto) { + SecurityStandards securityStandards = SecurityStandards.fromSecurityStandards(ruleDefinitionDto.getSecurityStandards()); + SecurityStandards.SQCategory sqCategory = securityStandards.getSqCategory(); + HotspotRuleDescription hotspotRuleDescription = HotspotRuleDescription.from(ruleDefinitionDto); + Hotspots.Rule.Builder ruleBuilder = Hotspots.Rule.newBuilder() + .setKey(ruleDefinitionDto.getKey().toString()) + .setName(nullToEmpty(ruleDefinitionDto.getName())) + .setSecurityCategory(sqCategory.getKey()) + .setVulnerabilityProbability(sqCategory.getVulnerability().name()); + hotspotRuleDescription.getVulnerable().ifPresent(ruleBuilder::setVulnerabilityDescription); + hotspotRuleDescription.getRisk().ifPresent(ruleBuilder::setRiskDescription); + hotspotRuleDescription.getFixIt().ifPresent(ruleBuilder::setFixRecommendations); + responseBuilder.setRule(ruleBuilder.build()); + } + + private void formatTextRange(IssueDto hotspot, ShowWsResponse.Builder responseBuilder) { + textRangeFormatter.formatTextRange(hotspot, responseBuilder::setTextRange); + } + + private RuleDefinitionDto loadRule(DbSession dbSession, IssueDto hotspot) { + RuleKey ruleKey = hotspot.getRuleKey(); + return dbClient.ruleDao().selectDefinitionByKey(dbSession, ruleKey) + .orElseThrow(() -> new NotFoundException(format("Rule '%s' does not exist", ruleKey))); + } + + private Components loadComponents(DbSession dbSession, IssueDto hotspot) { + String projectUuid = hotspot.getProjectUuid(); + String componentUuid = hotspot.getComponentUuid(); + checkArgument(projectUuid != null, "Hotspot '%s' has no project", hotspot.getKee()); + checkArgument(componentUuid != null, "Hotspot '%s' has no component", hotspot.getKee()); + + ComponentDto project = dbClient.componentDao().selectByUuid(dbSession, projectUuid) + .orElseThrow(() -> new NotFoundException(format("Project with uuid '%s' does not exist", projectUuid))); + userSession.checkComponentPermission(UserRole.USER, project); + + boolean hotspotOnProject = Objects.equals(projectUuid, componentUuid); + ComponentDto component = hotspotOnProject ? project + : dbClient.componentDao().selectByUuid(dbSession, componentUuid) + .orElseThrow(() -> new NotFoundException(format("Component with uuid '%s' does not exist", componentUuid))); + + return new Components(project, component); + } + + private static final class Components { + private final ComponentDto project; + private final ComponentDto component; + + private Components(ComponentDto project, ComponentDto component) { + this.project = project; + this.component = component; + } + + public ComponentDto getProject() { + return project; + } + + public ComponentDto getComponent() { + return component; + } + } + } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/TextRangeResponseFormatter.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/TextRangeResponseFormatter.java new file mode 100644 index 00000000000..b1eacc1d724 --- /dev/null +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/TextRangeResponseFormatter.java @@ -0,0 +1,83 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info 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.issue; + +import java.util.Map; +import java.util.function.Consumer; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.issue.IssueDto; +import org.sonar.db.protobuf.DbCommons; +import org.sonar.db.protobuf.DbIssues; +import org.sonarqube.ws.Common; + +import static java.util.Optional.ofNullable; + +public class TextRangeResponseFormatter { + + public void formatTextRange(IssueDto dto, Consumer rangeConsumer) { + DbIssues.Locations locations = dto.parseLocations(); + if (locations == null) { + return; + } + formatTextRange(locations, rangeConsumer); + } + + public void formatTextRange(DbIssues.Locations locations, Consumer rangeConsumer) { + if (locations.hasTextRange()) { + DbCommons.TextRange textRange = locations.getTextRange(); + rangeConsumer.accept(convertTextRange(textRange).build()); + } + } + + public Common.Location formatLocation(DbIssues.Location source, String issueComponent, Map componentsByUuid) { + Common.Location.Builder target = Common.Location.newBuilder(); + if (source.hasMsg()) { + target.setMsg(source.getMsg()); + } + if (source.hasTextRange()) { + DbCommons.TextRange sourceRange = source.getTextRange(); + Common.TextRange.Builder targetRange = convertTextRange(sourceRange); + target.setTextRange(targetRange); + } + if (source.hasComponentId()) { + ofNullable(componentsByUuid.get(source.getComponentId())).ifPresent(c -> target.setComponent(c.getKey())); + } else { + target.setComponent(issueComponent); + } + return target.build(); + } + + private static Common.TextRange.Builder convertTextRange(DbCommons.TextRange sourceRange) { + Common.TextRange.Builder targetRange = Common.TextRange.newBuilder(); + if (sourceRange.hasStartLine()) { + targetRange.setStartLine(sourceRange.getStartLine()); + } + if (sourceRange.hasStartOffset()) { + targetRange.setStartOffset(sourceRange.getStartOffset()); + } + if (sourceRange.hasEndLine()) { + targetRange.setEndLine(sourceRange.getEndLine()); + } + if (sourceRange.hasEndOffset()) { + targetRange.setEndOffset(sourceRange.getEndOffset()); + } + return targetRange; + } +} diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java index b21e4728d6f..1103eee6efa 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java @@ -21,6 +21,7 @@ package org.sonar.server.issue.ws; import org.sonar.core.platform.Module; import org.sonar.server.issue.AvatarResolverImpl; +import org.sonar.server.issue.TextRangeResponseFormatter; import org.sonar.server.issue.IssueFieldsSetter; import org.sonar.server.issue.IssueFinder; import org.sonar.server.issue.TransitionService; @@ -45,6 +46,7 @@ public class IssueWsModule extends Module { IssuesWs.class, AvatarResolverImpl.class, SearchResponseLoader.class, + TextRangeResponseFormatter.class, SearchResponseFormat.class, OperationResponseWriter.class, AddCommentAction.class, diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchResponseData.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchResponseData.java index fad0639372c..fff99e6c79c 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchResponseData.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchResponseData.java @@ -82,6 +82,10 @@ public class SearchResponseData { return componentsByUuid.get(uuid); } + public Map getComponentsByUuid() { + return componentsByUuid; + } + public List getUsers() { return new ArrayList<>(usersByUuid.values()); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java index 362fe7f428d..8b628b17185 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java @@ -37,14 +37,13 @@ import org.sonar.api.utils.Paging; import org.sonar.db.component.ComponentDto; import org.sonar.db.issue.IssueChangeDto; import org.sonar.db.issue.IssueDto; -import org.sonar.db.protobuf.DbCommons; import org.sonar.db.protobuf.DbIssues; -import org.sonar.db.protobuf.DbIssues.Location; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.user.UserDto; import org.sonar.markdown.Markdown; import org.sonar.server.es.Facets; import org.sonar.server.issue.AvatarResolver; +import org.sonar.server.issue.TextRangeResponseFormatter; import org.sonar.server.issue.workflow.Transition; import org.sonarqube.ws.Common; import org.sonarqube.ws.Issues; @@ -78,11 +77,14 @@ public class SearchResponseFormat { private final Durations durations; private final Languages languages; private final AvatarResolver avatarFactory; + private final TextRangeResponseFormatter textRangeFormatter; - public SearchResponseFormat(Durations durations, Languages languages, AvatarResolver avatarFactory) { + public SearchResponseFormat(Durations durations, Languages languages, AvatarResolver avatarFactory, + TextRangeResponseFormatter textRangeFormatter) { this.durations = durations; this.languages = languages; this.avatarFactory = avatarFactory; + this.textRangeFormatter = textRangeFormatter; } SearchWsResponse formatSearch(Set fields, SearchResponseData data, Paging paging, Facets facets) { @@ -219,57 +221,19 @@ public class SearchResponseFormat { return ruleKey.repository().replace(EXTERNAL_RULE_REPO_PREFIX, ""); } - private static void completeIssueLocations(IssueDto dto, Issue.Builder issueBuilder, SearchResponseData data) { + private void completeIssueLocations(IssueDto dto, Issue.Builder issueBuilder, SearchResponseData data) { DbIssues.Locations locations = dto.parseLocations(); if (locations == null) { return; } - if (locations.hasTextRange()) { - DbCommons.TextRange textRange = locations.getTextRange(); - issueBuilder.setTextRange(convertTextRange(textRange)); - } + textRangeFormatter.formatTextRange(locations, issueBuilder::setTextRange); for (DbIssues.Flow flow : locations.getFlowList()) { Common.Flow.Builder targetFlow = Common.Flow.newBuilder(); - for (Location flowLocation : flow.getLocationList()) { - targetFlow.addLocations(convertLocation(issueBuilder, flowLocation, data)); + for (DbIssues.Location flowLocation : flow.getLocationList()) { + targetFlow.addLocations(textRangeFormatter.formatLocation(flowLocation, issueBuilder.getComponent(), data.getComponentsByUuid())); } - issueBuilder.addFlows(targetFlow); - } - } - - private static Common.Location convertLocation(Issue.Builder issueBuilder, Location source, SearchResponseData data) { - Common.Location.Builder target = Common.Location.newBuilder(); - if (source.hasMsg()) { - target.setMsg(source.getMsg()); - } - if (source.hasTextRange()) { - DbCommons.TextRange sourceRange = source.getTextRange(); - Common.TextRange.Builder targetRange = convertTextRange(sourceRange); - target.setTextRange(targetRange); - } - if (source.hasComponentId()) { - ofNullable(data.getComponentByUuid(source.getComponentId())).ifPresent(c -> target.setComponent(c.getKey())); - } else { - target.setComponent(issueBuilder.getComponent()); - } - return target.build(); - } - - private static Common.TextRange.Builder convertTextRange(DbCommons.TextRange sourceRange) { - Common.TextRange.Builder targetRange = Common.TextRange.newBuilder(); - if (sourceRange.hasStartLine()) { - targetRange.setStartLine(sourceRange.getStartLine()); - } - if (sourceRange.hasStartOffset()) { - targetRange.setStartOffset(sourceRange.getStartOffset()); - } - if (sourceRange.hasEndLine()) { - targetRange.setEndLine(sourceRange.getEndLine()); - } - if (sourceRange.hasEndOffset()) { - targetRange.setEndOffset(sourceRange.getEndOffset()); + issueBuilder.addFlows(targetFlow.build()); } - return targetRange; } private static void formatIssueTransitions(SearchResponseData data, Issue.Builder wsIssue, IssueDto dto) { diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotRuleDescriptionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotRuleDescriptionTest.java new file mode 100644 index 00000000000..1a35b069c1b --- /dev/null +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotRuleDescriptionTest.java @@ -0,0 +1,352 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info 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.hotspot.ws; + +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.db.rule.RuleTesting; + +import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; +import static org.assertj.core.api.Assertions.assertThat; + +@RunWith(DataProviderRunner.class) +public class HotspotRuleDescriptionTest { + @Test + public void parse_returns_all_empty_fields_when_no_description() { + RuleDefinitionDto dto = RuleTesting.newRule().setDescription(null); + + HotspotRuleDescription result = HotspotRuleDescription.from(dto); + + assertThat(result.getRisk()).isEmpty(); + assertThat(result.getVulnerable()).isEmpty(); + assertThat(result.getFixIt()).isEmpty(); + } + + @Test + @UseDataProvider("noContentVariants") + public void parse_returns_all_empty_fields_when_empty_description(String noContent) { + RuleDefinitionDto dto = RuleTesting.newRule().setDescription(""); + + HotspotRuleDescription result = HotspotRuleDescription.from(dto); + + assertThat(result.getRisk()).isEmpty(); + assertThat(result.getVulnerable()).isEmpty(); + assertThat(result.getFixIt()).isEmpty(); + } + + @Test + public void parse_ignores_titles_if_not_h2() { + RuleDefinitionDto dto = RuleTesting.newRule().setDescription( + "acme\" +" + + "

Ask Yourself Whether

\n" + + "bar\n" + + "

Recommended Secure Coding Practices

\n" + + "foo"); + + HotspotRuleDescription result = HotspotRuleDescription.from(dto); + + assertThat(result.getRisk()).isEmpty(); + assertThat(result.getVulnerable()).isEmpty(); + assertThat(result.getFixIt()).isEmpty(); + } + + @Test + @UseDataProvider("whiteSpaceBeforeAndAfterCombinations") + public void parse_does_not_trim_content_of_h2_titles(String whiteSpaceBefore, String whiteSpaceAfter) { + RuleDefinitionDto dto = RuleTesting.newRule().setDescription( + "acme\" +" + + "

" + whiteSpaceBefore + "Ask Yourself Whether" + whiteSpaceAfter + "

\n" + + "bar\n" + + "

" + whiteSpaceBefore + "Recommended Secure Coding Practices\" + whiteSpaceAfter + \"

\n" + + "foo"); + + HotspotRuleDescription result = HotspotRuleDescription.from(dto); + + assertThat(result.getRisk()).isEmpty(); + assertThat(result.getVulnerable()).isEmpty(); + assertThat(result.getFixIt()).isEmpty(); + } + + @DataProvider + public static Object[][] whiteSpaceBeforeAndAfterCombinations() { + String whiteSpace = " "; + String noWithSpace = ""; + return new Object[][] { + {noWithSpace, whiteSpace}, + {whiteSpace, noWithSpace}, + {whiteSpace, whiteSpace} + }; + } + + @Test + @UseDataProvider("descriptionsWithoutTitles") + public void parse_return_null_fields_when_desc_contains_neither_title(String description) { + RuleDefinitionDto dto = RuleTesting.newRule().setDescription(description); + + HotspotRuleDescription result = HotspotRuleDescription.from(dto); + + assertThat(result.getRisk()).isEmpty(); + assertThat(result.getVulnerable()).isEmpty(); + assertThat(result.getFixIt()).isEmpty(); + } + + @DataProvider + public static Object[][] descriptionsWithoutTitles() { + return new Object[][] { + {""}, + {randomAlphabetic(123)}, + {"bar\n" + + "acme\n" + + "foo"} + }; + } + + @Test + public void parse_return_null_risk_when_desc_starts_with_ask_yourself_title() { + RuleDefinitionDto dto = RuleTesting.newRule().setDescription( + "

Ask Yourself Whether

\n" + + "bar\n" + + "

Recommended Secure Coding Practices

\n" + + "foo"); + + HotspotRuleDescription result = HotspotRuleDescription.from(dto); + + assertThat(result.getRisk()).isEmpty(); + assertThat(result.getVulnerable().get()).isEqualTo("

Ask Yourself Whether

\nbar"); + assertThat(result.getFixIt().get()).isEqualTo("

Recommended Secure Coding Practices

\nfoo"); + } + + @Test + public void parse_return_null_vulnerable_when_no_ask_yourself_whether_title() { + RuleDefinitionDto dto = RuleTesting.newRule().setDescription( + "bar\n" + + "

Recommended Secure Coding Practices

\n" + + "foo"); + + HotspotRuleDescription result = HotspotRuleDescription.from(dto); + + assertThat(result.getRisk().get()).isEqualTo("bar"); + assertThat(result.getVulnerable()).isEmpty(); + assertThat(result.getFixIt().get()).isEqualTo("

Recommended Secure Coding Practices

\nfoo"); + } + + @Test + @UseDataProvider("noContentVariants") + public void parse_returns_vulnerable_with_only_title_when_no_content_between_titles(String noContent) { + RuleDefinitionDto dto = RuleTesting.newRule().setDescription( + "bar\n" + + "

Ask Yourself Whether

\n" + + noContent + + "

Recommended Secure Coding Practices

\n" + + "foo"); + + HotspotRuleDescription result = HotspotRuleDescription.from(dto); + + assertThat(result.getRisk().get()).isEqualTo("bar"); + assertThat(result.getVulnerable().get()).isEqualTo("

Ask Yourself Whether

"); + assertThat(result.getFixIt().get()).isEqualTo("

Recommended Secure Coding Practices

\nfoo"); + } + + @Test + @UseDataProvider("noContentVariants") + public void parse_returns_fixIt_with_only_title_when_no_content_after_Recommended_Secure_Coding_Practices_title(String noContent) { + RuleDefinitionDto dto = RuleTesting.newRule().setDescription( + "bar\n" + + "

Ask Yourself Whether

\n" + + "bar" + + "

Recommended Secure Coding Practices

\n" + + noContent); + + HotspotRuleDescription result = HotspotRuleDescription.from(dto); + + assertThat(result.getRisk().get()).isEqualTo("bar"); + assertThat(result.getVulnerable().get()).isEqualTo("

Ask Yourself Whether

\nbar"); + assertThat(result.getFixIt().get()).isEqualTo("

Recommended Secure Coding Practices

"); + } + + @DataProvider + public static Object[][] noContentVariants() { + return new Object[][] { + {""}, + {"\n"}, + {" \n "}, + {"\t\n \n"}, + }; + } + + @Test + public void parse_return_null_fixIt_when_desc_has_no_Recommended_Secure_Coding_Practices_title() { + RuleDefinitionDto dto = RuleTesting.newRule().setDescription( + "bar\n" + + "

Ask Yourself Whether

\n" + + "foo"); + + HotspotRuleDescription result = HotspotRuleDescription.from(dto); + + assertThat(result.getRisk().get()).isEqualTo("bar"); + assertThat(result.getVulnerable().get()).isEqualTo("

Ask Yourself Whether

\nfoo"); + assertThat(result.getFixIt()).isEmpty(); + } + + @Test + public void parse_returns_regular_description() { + RuleDefinitionDto dto = RuleTesting.newRule().setDescription( + "

Enabling Cross-Origin Resource Sharing (CORS) is security-sensitive. For example, it has led in the past to the following vulnerabilities:

\n" + + "\n" + + "

Applications that enable CORS will effectively relax the same-origin policy in browsers, which is in place to prevent AJAX requests to hosts other\n" + + "than the one showing in the browser address bar. Being too permissive, CORS can potentially allow an attacker to gain access to sensitive\n" + + "information.

\n" + + "

This rule flags code that enables CORS or specifies any HTTP response headers associated with CORS. The goal is to guide security code reviews.

\n" + + "

Ask Yourself Whether

\n" + + "
    \n" + + "
  • Any URLs responding with Access-Control-Allow-Origin: * include sensitive content.
    • \n" + + "
  • Any domains specified in Access-Control-Allow-Origin headers are checked against a whitelist.
    • \n" + + "
\n" + + "

Recommended Secure Coding Practices

\n" + + "
    \n" + + "
  • The Access-Control-Allow-Origin header should be set only on specific URLs that require access from other domains. Don't enable\n" + + " the header on the entire domain.
    • \n" + + "
  • Don't rely on the Origin header blindly without validation as it could be spoofed by an attacker. Use a whitelist to check that\n" + + " the Origin domain (including protocol) is allowed before returning it back in the Access-Control-Allow-Origin header.\n" + + "
    • \n" + + "
  • Use Access-Control-Allow-Origin: * only if your application absolutely requires it, for example in the case of an open/public API.\n" + + " For such endpoints, make sure that there is no sensitive content or information included in the response.
    • \n" + + "
\n" + + "

Sensitive Code Example

\n" + + "
\n" +
+        "// === Java Servlet ===\n" +
+        "@Override\n" +
+        "protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {\n" +
+        "  resp.setHeader(\"Content-Type\", \"text/plain; charset=utf-8\");\n" +
+        "  resp.setHeader(\"Access-Control-Allow-Origin\", \"http://localhost:8080\"); // Questionable\n" +
+        "  resp.setHeader(\"Access-Control-Allow-Credentials\", \"true\"); // Questionable\n" +
+        "  resp.setHeader(\"Access-Control-Allow-Methods\", \"GET\"); // Questionable\n" +
+        "  resp.getWriter().write(\"response\");\n" +
+        "}\n" +
+        "
\n" + + "
\n" +
+        "// === Spring MVC Controller annotation ===\n" +
+        "@CrossOrigin(origins = \"http://domain1.com\") // Questionable\n" +
+        "@RequestMapping(\"\")\n" +
+        "public class TestController {\n" +
+        "    public String home(ModelMap model) {\n" +
+        "        model.addAttribute(\"message\", \"ok \");\n" +
+        "        return \"view\";\n" +
+        "    }\n" +
+        "\n" +
+        "    @CrossOrigin(origins = \"http://domain2.com\") // Questionable\n" +
+        "    @RequestMapping(value = \"/test1\")\n" +
+        "    public ResponseEntity<String> test1() {\n" +
+        "        return ResponseEntity.ok().body(\"ok\");\n" +
+        "    }\n" +
+        "}\n" +
+        "
\n" + + "

See

\n" + + ""); + + HotspotRuleDescription result = HotspotRuleDescription.from(dto); + + assertThat(result.getRisk().get()).isEqualTo( + "

Enabling Cross-Origin Resource Sharing (CORS) is security-sensitive. For example, it has led in the past to the following vulnerabilities:

\n" + + "\n" + + "

Applications that enable CORS will effectively relax the same-origin policy in browsers, which is in place to prevent AJAX requests to hosts other\n" + + "than the one showing in the browser address bar. Being too permissive, CORS can potentially allow an attacker to gain access to sensitive\n" + + "information.

\n" + + "

This rule flags code that enables CORS or specifies any HTTP response headers associated with CORS. The goal is to guide security code reviews.

"); + assertThat(result.getVulnerable().get()).isEqualTo( + "

Ask Yourself Whether

\n" + + "
    \n" + + "
  • Any URLs responding with Access-Control-Allow-Origin: * include sensitive content.
    • \n" + + "
  • Any domains specified in Access-Control-Allow-Origin headers are checked against a whitelist.
    • \n" + + "
"); + assertThat(result.getFixIt().get()).isEqualTo( + "

Recommended Secure Coding Practices

\n" + + "
    \n" + + "
  • The Access-Control-Allow-Origin header should be set only on specific URLs that require access from other domains. Don't enable\n" + + " the header on the entire domain.
    • \n" + + "
  • Don't rely on the Origin header blindly without validation as it could be spoofed by an attacker. Use a whitelist to check that\n" + + " the Origin domain (including protocol) is allowed before returning it back in the Access-Control-Allow-Origin header.\n" + + "
    • \n" + + "
  • Use Access-Control-Allow-Origin: * only if your application absolutely requires it, for example in the case of an open/public API.\n" + + " For such endpoints, make sure that there is no sensitive content or information included in the response.
    • \n" + + "
\n" + + "

Sensitive Code Example

\n" + + "
\n" +
+        "// === Java Servlet ===\n" +
+        "@Override\n" +
+        "protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {\n" +
+        "  resp.setHeader(\"Content-Type\", \"text/plain; charset=utf-8\");\n" +
+        "  resp.setHeader(\"Access-Control-Allow-Origin\", \"http://localhost:8080\"); // Questionable\n" +
+        "  resp.setHeader(\"Access-Control-Allow-Credentials\", \"true\"); // Questionable\n" +
+        "  resp.setHeader(\"Access-Control-Allow-Methods\", \"GET\"); // Questionable\n" +
+        "  resp.getWriter().write(\"response\");\n" +
+        "}\n" +
+        "
\n" + + "
\n" +
+        "// === Spring MVC Controller annotation ===\n" +
+        "@CrossOrigin(origins = \"http://domain1.com\") // Questionable\n" +
+        "@RequestMapping(\"\")\n" +
+        "public class TestController {\n" +
+        "    public String home(ModelMap model) {\n" +
+        "        model.addAttribute(\"message\", \"ok \");\n" +
+        "        return \"view\";\n" +
+        "    }\n" +
+        "\n" +
+        "    @CrossOrigin(origins = \"http://domain2.com\") // Questionable\n" +
+        "    @RequestMapping(value = \"/test1\")\n" +
+        "    public ResponseEntity<String> test1() {\n" +
+        "        return ResponseEntity.ok().body(\"ok\");\n" +
+        "    }\n" +
+        "}\n" +
+        "
\n" + + "

See

\n" + + ""); + } +} diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotsWsModuleTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotsWsModuleTest.java index 197753a60a4..a9c839b56df 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotsWsModuleTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotsWsModuleTest.java @@ -30,7 +30,7 @@ public class HotspotsWsModuleTest { public void verify_count_of_added_components() { ComponentContainer container = new ComponentContainer(); new HotspotsWsModule().configure(container); - assertThat(container.size()).isEqualTo(COMPONENTS_IN_EMPTY_COMPONENT_CONTAINER + 2); + assertThat(container.size()).isEqualTo(COMPONENTS_IN_EMPTY_COMPONENT_CONTAINER + 4); } } 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 2e173802409..d1a1fc45ef9 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 @@ -97,8 +97,9 @@ public class SearchActionTest { private IssueIndex issueIndex = new IssueIndex(es.client(), System2.INSTANCE, userSessionRule, new WebAuthorizationTypeSupport(userSessionRule)); private IssueIndexer issueIndexer = new IssueIndexer(es.client(), dbClient, new IssueIteratorFactory(dbClient)); private StartupIndexer permissionIndexer = new PermissionIndexer(dbClient, es.client(), issueIndexer); + private HotspotWsResponseFormatter responseFormatter = new HotspotWsResponseFormatter(defaultOrganizationProvider); - private SearchAction underTest = new SearchAction(dbClient, userSessionRule, issueIndex, defaultOrganizationProvider); + private SearchAction underTest = new SearchAction(dbClient, userSessionRule, issueIndex, responseFormatter); private WsActionTester actionTester = new WsActionTester(underTest); @Test @@ -200,7 +201,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(Hotspots.SearchWsResponse.Hotspot::getKey) .containsOnly(Arrays.stream(hotspots) .filter(t -> !t.getKey().equals(hotspotWithoutRule.getKey())) .map(IssueDto::getKey) @@ -250,7 +251,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(SearchWsResponse.Hotspot::getKey) .containsOnly(Arrays.stream(hotspots).map(IssueDto::getKey).toArray(String[]::new)); assertThat(response.getComponentsList()) .extracting(Component::getKey) @@ -274,7 +275,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(Hotspots.SearchWsResponse.Hotspot::getKey) .containsOnly(Arrays.stream(hotspots).map(IssueDto::getKey).toArray(String[]::new)); assertThat(response.getComponentsList()) .extracting(Component::getKey) @@ -302,7 +303,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(responseProject1.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(SearchWsResponse.Hotspot::getKey) .doesNotContainAnyElementsOf(Arrays.stream(hotspots2).map(IssueDto::getKey).collect(Collectors.toList())); assertThat(responseProject1.getComponentsList()) .extracting(Component::getKey) @@ -312,7 +313,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(responseProject2.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(SearchWsResponse.Hotspot::getKey) .containsOnly(Arrays.stream(hotspots2).map(IssueDto::getKey).toArray(String[]::new)); assertThat(responseProject2.getComponentsList()) .extracting(Component::getKey) @@ -339,7 +340,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(SearchWsResponse.Hotspot::getKey) .containsOnly(unresolvedHotspot.getKey(), badlyClosedHotspot.getKey()); } @@ -363,7 +364,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()).hasSize(1); - Hotspots.Hotspot actual = response.getHotspots(0); + SearchWsResponse.Hotspot actual = response.getHotspots(0); assertThat(actual.getComponent()).isEqualTo(file.getKey()); assertThat(actual.getProject()).isEqualTo(project.getKey()); assertThat(actual.getStatus()).isEqualTo(hotspot.getStatus()); @@ -392,7 +393,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()).hasSize(1); - Hotspots.Hotspot actual = response.getHotspots(0); + Hotspots.SearchWsResponse.Hotspot actual = response.getHotspots(0); assertThat(actual.getSecurityCategory()).isEqualTo(expected.getKey()); assertThat(actual.getVulnerabilityProbability()).isEqualTo(expected.getVulnerability().name()); } @@ -434,7 +435,7 @@ public class SearchActionTest { assertThat(response.getHotspotsList()) .hasSize(1); - Hotspots.Hotspot actual = response.getHotspots(0); + SearchWsResponse.Hotspot actual = response.getHotspots(0); assertThat(actual.hasStatus()).isFalse(); // FIXME resolution field will be added later // assertThat(actual.hasResolution()).isFalse(); @@ -463,7 +464,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(SearchWsResponse.Hotspot::getKey) .containsOnly(fileHotspot.getKey(), dirHotspot.getKey(), projectHotspot.getKey()); assertThat(response.getComponentsList()).hasSize(3); assertThat(response.getComponentsList()) @@ -523,7 +524,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(SearchWsResponse.Hotspot::getKey) .containsExactly(expectedHotspotKeys); } @@ -556,7 +557,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(SearchWsResponse.Hotspot::getKey) .containsExactly(expectedHotspotKeys); } @@ -579,7 +580,7 @@ public class SearchActionTest { SearchWsResponse response = request.executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(SearchWsResponse.Hotspot::getKey) .containsExactly(hotspots.stream().limit(100).map(IssueDto::getKey).toArray(String[]::new)); assertThat(response.getPaging().getTotal()).isEqualTo(hotspots.size()); assertThat(response.getPaging().getPageIndex()).isEqualTo(1); @@ -623,7 +624,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(SearchWsResponse.Hotspot::getKey) .containsExactly(hotspots.stream().skip(2 * pageSize).limit(pageSize).map(IssueDto::getKey).toArray(String[]::new)); assertThat(response.getPaging().getTotal()).isEqualTo(hotspots.size()); assertThat(response.getPaging().getPageIndex()).isEqualTo(3); @@ -635,7 +636,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(SearchWsResponse.Hotspot::getKey) .containsExactly(hotspots.stream().skip(3 * pageSize).limit(pageSize).map(IssueDto::getKey).toArray(String[]::new)); assertThat(response.getPaging().getTotal()).isEqualTo(hotspots.size()); assertThat(response.getPaging().getPageIndex()).isEqualTo(4); @@ -648,7 +649,7 @@ public class SearchActionTest { .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) - .extracting(Hotspots.Hotspot::getKey) + .extracting(SearchWsResponse.Hotspot::getKey) .isEmpty(); assertThat(response.getPaging().getTotal()).isEqualTo(hotspots.size()); assertThat(response.getPaging().getPageIndex()).isEqualTo(emptyPage); diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java new file mode 100644 index 00000000000..9f394b96e9e --- /dev/null +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java @@ -0,0 +1,364 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info 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.hotspot.ws; + +import com.google.common.collect.ImmutableSet; +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; +import java.util.Arrays; +import java.util.Collections; +import java.util.Random; +import java.util.Set; +import java.util.function.Consumer; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.sonar.api.rules.RuleType; +import org.sonar.api.utils.System2; +import org.sonar.api.web.UserRole; +import org.sonar.db.DbClient; +import org.sonar.db.DbTester; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.issue.IssueDto; +import org.sonar.db.issue.IssueTesting; +import org.sonar.db.protobuf.DbCommons; +import org.sonar.db.protobuf.DbIssues; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.db.rule.RuleTesting; +import org.sonar.server.issue.TextRangeResponseFormatter; +import org.sonar.server.es.EsTester; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.organization.TestDefaultOrganizationProvider; +import org.sonar.server.security.SecurityStandards; +import org.sonar.server.security.SecurityStandards.SQCategory; +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.Hotspots; + +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.sonar.api.rules.RuleType.SECURITY_HOTSPOT; +import static org.sonar.db.component.ComponentTesting.newFileDto; + +@RunWith(DataProviderRunner.class) +public class ShowActionTest { + private static final Random RANDOM = new Random(); + + @Rule + public DbTester dbTester = DbTester.create(System2.INSTANCE); + @Rule + public EsTester es = EsTester.create(); + @Rule + public UserSessionRule userSessionRule = UserSessionRule.standalone(); + + private DbClient dbClient = dbTester.getDbClient(); + private TestDefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(dbTester); + + private TextRangeResponseFormatter commonFormatter = new TextRangeResponseFormatter(); + private HotspotWsResponseFormatter responseFormatter = new HotspotWsResponseFormatter(defaultOrganizationProvider); + + private ShowAction underTest = new ShowAction(dbClient, userSessionRule, responseFormatter, commonFormatter); + private WsActionTester actionTester = new WsActionTester(underTest); + + @Test + public void ws_is_internal() { + assertThat(actionTester.getDef().isInternal()).isTrue(); + } + + @Test + public void fails_with_IAE_if_parameter_hotspot_is_missing() { + TestRequest request = actionTester.newRequest(); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("The 'hotspot' parameter is missing"); + } + + @Test + public void fails_with_NotFoundException_if_hotspot_does_not_exist() { + String key = randomAlphabetic(12); + TestRequest request = actionTester.newRequest() + .setParam("hotspot", key); + + assertThatThrownBy(request::execute) + .isInstanceOf(NotFoundException.class) + .hasMessage("Hotspot '%s' does not exist", key); + } + + @Test + @UseDataProvider("ruleTypesButHotspot") + public void fails_with_NotFoundException_if_issue_is_not_a_hotspot(RuleType ruleType) { + ComponentDto project = dbTester.components().insertPublicProject(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(ruleType); + IssueDto notAHotspot = dbTester.issues().insertIssue(IssueTesting.newIssue(rule, project, file).setType(ruleType)); + TestRequest request = newRequest(notAHotspot); + + assertThatThrownBy(request::execute) + .isInstanceOf(NotFoundException.class) + .hasMessage("Hotspot '%s' does not exist", notAHotspot.getKey()); + } + + @DataProvider + public static Object[][] ruleTypesButHotspot() { + return Arrays.stream(RuleType.values()) + .filter(t -> t != SECURITY_HOTSPOT) + .map(t -> new Object[] {t}) + .toArray(Object[][]::new); + } + + @Test + public void fails_with_ForbiddenException_if_project_is_private_and_not_allowed() { + ComponentDto project = dbTester.components().insertPrivateProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule)); + TestRequest request = newRequest(hotspot); + + assertThatThrownBy(request::execute) + .isInstanceOf(ForbiddenException.class) + .hasMessage("Insufficient privileges"); + } + + @Test + public void succeeds_on_public_project() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule)); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + assertThat(response.getKey()).isEqualTo(hotspot.getKey()); + } + + @Test + public void succeeds_on_private_project_with_permission() { + ComponentDto project = dbTester.components().insertPrivateProject(); + userSessionRule.registerComponents(project); + userSessionRule.logIn().addProjectPermission(UserRole.USER, project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule)); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + assertThat(response.getKey()).isEqualTo(hotspot.getKey()); + } + + @Test + public void returns_hotspot_component_and_rule() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule)); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + assertThat(response.getKey()).isEqualTo(hotspot.getKey()); + verifyComponent(response.getComponent(), file); + verifyComponent(response.getProject(), project); + verifyRule(response.getRule(), rule); + assertThat(response.hasTextRange()).isFalse(); + } + + @Test + public void returns_no_textrange_when_locations_have_none() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule) + .setLocations(DbIssues.Locations.newBuilder().build())); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + assertThat(response.hasTextRange()).isFalse(); + } + + @Test + @UseDataProvider("randomTextRangeValues") + public void returns_textRange(int startLine, int endLine, int startOffset, int endOffset) { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule) + .setLocations(DbIssues.Locations.newBuilder() + .setTextRange(DbCommons.TextRange.newBuilder() + .setStartLine(startLine) + .setEndLine(endLine) + .setStartOffset(startOffset) + .setEndOffset(endOffset) + .build()) + .build())); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + assertThat(response.hasTextRange()).isTrue(); + Common.TextRange textRange = response.getTextRange(); + assertThat(textRange.getStartLine()).isEqualTo(startLine); + assertThat(textRange.getEndLine()).isEqualTo(endLine); + assertThat(textRange.getStartOffset()).isEqualTo(startOffset); + assertThat(textRange.getEndOffset()).isEqualTo(endOffset); + } + + @DataProvider + public static Object[][] randomTextRangeValues() { + int startLine = RANDOM.nextInt(200); + int endLine = RANDOM.nextInt(200); + int startOffset = RANDOM.nextInt(200); + int endOffset = RANDOM.nextInt(200); + return new Object[][] { + {startLine, endLine, startOffset, endOffset} + }; + } + + @Test + public void returns_textRange_missing_fields() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule) + .setLocations(DbIssues.Locations.newBuilder() + .setTextRange(DbCommons.TextRange.newBuilder().build()) + .build())); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + assertThat(response.hasTextRange()).isTrue(); + Common.TextRange textRange = response.getTextRange(); + assertThat(textRange.hasStartLine()).isFalse(); + assertThat(textRange.hasEndLine()).isFalse(); + assertThat(textRange.hasStartOffset()).isFalse(); + assertThat(textRange.hasEndOffset()).isFalse(); + } + + @Test + @UseDataProvider("allSQCategoryAndVulnerabilityProbability") + public void returns_securityCategory_and_vulnerabilityProbability_of_rule(Set standards, + SQCategory expected) { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT, t -> t.setSecurityStandards(standards)); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule) + .setLocations(DbIssues.Locations.newBuilder() + .setTextRange(DbCommons.TextRange.newBuilder().build()) + .build())); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + Hotspots.Rule wsRule = response.getRule(); + assertThat(wsRule.getSecurityCategory()).isEqualTo(expected.getKey()); + assertThat(wsRule.getVulnerabilityProbability()).isEqualTo(expected.getVulnerability().name()); + } + + @DataProvider + public static Object[][] allSQCategoryAndVulnerabilityProbability() { + Stream allButOthers = SecurityStandards.CWES_BY_SQ_CATEGORY + .entrySet() + .stream() + .map(t -> new Object[] { + t.getValue().stream().map(s -> "cwe:" + s).collect(Collectors.toSet()), + t.getKey() + }); + Stream others = Stream.of( + new Object[] {Collections.emptySet(), SQCategory.OTHERS}, + new Object[] {ImmutableSet.of("foo", "bar", "acme"), SQCategory.OTHERS}); + return Stream.concat(allButOthers, others) + .toArray(Object[][]::new); + } + + @Test + public void returns_project_twice_when_hotspot_on_project() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, project, rule) + .setLocations(DbIssues.Locations.newBuilder() + .setTextRange(DbCommons.TextRange.newBuilder().build()) + .build())); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + verifyComponent(response.getProject(), project); + verifyComponent(response.getComponent(), project); + } + + public void verifyRule(Hotspots.Rule wsRule, RuleDefinitionDto dto) { + assertThat(wsRule.getKey()).isEqualTo(dto.getKey().toString()); + assertThat(wsRule.getName()).isEqualTo(dto.getName()); + assertThat(wsRule.getSecurityCategory()).isEqualTo(SQCategory.OTHERS.getKey()); + assertThat(wsRule.getVulnerabilityProbability()).isEqualTo(SQCategory.OTHERS.getVulnerability().name()); + } + + private static void verifyComponent(Hotspots.Component wsComponent, ComponentDto dto) { + assertThat(wsComponent.getKey()).isEqualTo(dto.getKey()); + if (dto.path() == null) { + assertThat(wsComponent.hasPath()).isFalse(); + } else { + assertThat(wsComponent.getPath()).isEqualTo(dto.path()); + } + assertThat(wsComponent.getQualifier()).isEqualTo(dto.qualifier()); + assertThat(wsComponent.getName()).isEqualTo(dto.name()); + assertThat(wsComponent.getLongName()).isEqualTo(dto.longName()); + } + + private static IssueDto newHotspot(ComponentDto project, ComponentDto file, RuleDefinitionDto rule) { + return IssueTesting.newIssue(rule, project, file).setType(SECURITY_HOTSPOT); + } + + private TestRequest newRequest(IssueDto hotspot) { + return actionTester.newRequest() + .setParam("hotspot", hotspot.getKey()); + } + + private RuleDefinitionDto newRule(RuleType ruleType) { + return newRule(ruleType, t -> { + }); + } + + private RuleDefinitionDto newRule(RuleType ruleType, Consumer populate) { + RuleDefinitionDto ruleDefinition = RuleTesting.newRule() + .setType(ruleType); + populate.accept(ruleDefinition); + dbTester.rules().insert(ruleDefinition); + return ruleDefinition; + } + +} diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/IssueWsModuleTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/IssueWsModuleTest.java index bb67997b4d0..4e41efcfd6f 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/IssueWsModuleTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/IssueWsModuleTest.java @@ -30,7 +30,7 @@ public class IssueWsModuleTest { public void verify_count_of_added_components() { ComponentContainer container = new ComponentContainer(); new IssueWsModule().configure(container); - assertThat(container.size()).isEqualTo(COMPONENTS_IN_EMPTY_COMPONENT_CONTAINER + 28); + assertThat(container.size()).isEqualTo(COMPONENTS_IN_EMPTY_COMPONENT_CONTAINER + 29); } } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionComponentsTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionComponentsTest.java index f4e85478c72..9902e74ee9b 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionComponentsTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionComponentsTest.java @@ -37,6 +37,7 @@ import org.sonar.db.component.ComponentDto; import org.sonar.db.issue.IssueDto; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.server.issue.TextRangeResponseFormatter; import org.sonar.server.es.EsTester; import org.sonar.server.issue.AvatarResolverImpl; import org.sonar.server.issue.IssueFieldsSetter; @@ -105,7 +106,7 @@ public class SearchActionComponentsTest { private IssueWorkflow issueWorkflow = new IssueWorkflow(new FunctionExecutor(issueFieldsSetter), issueFieldsSetter); private SearchResponseLoader searchResponseLoader = new SearchResponseLoader(userSession, dbClient, new TransitionService(userSession, issueWorkflow)); private Languages languages = new Languages(); - private SearchResponseFormat searchResponseFormat = new SearchResponseFormat(new Durations(), languages, new AvatarResolverImpl()); + private SearchResponseFormat searchResponseFormat = new SearchResponseFormat(new Durations(), languages, new AvatarResolverImpl(), new TextRangeResponseFormatter()); private PermissionIndexerTester permissionIndexer = new PermissionIndexerTester(es, issueIndexer); private WsActionTester ws = new WsActionTester(new SearchAction(userSession, issueIndex, issueQueryFactory, searchResponseLoader, searchResponseFormat, diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionFacetsTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionFacetsTest.java index 0981dccbb84..4cc89cf1b10 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionFacetsTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionFacetsTest.java @@ -37,6 +37,7 @@ import org.sonar.db.component.ComponentDto; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.user.UserDto; +import org.sonar.server.issue.TextRangeResponseFormatter; import org.sonar.server.es.EsTester; import org.sonar.server.es.StartupIndexer; import org.sonar.server.issue.AvatarResolverImpl; @@ -87,7 +88,7 @@ public class SearchActionFacetsTest { private IssueQueryFactory issueQueryFactory = new IssueQueryFactory(db.getDbClient(), Clock.systemUTC(), userSession); private SearchResponseLoader searchResponseLoader = new SearchResponseLoader(userSession, db.getDbClient(), new TransitionService(userSession, null)); private Languages languages = new Languages(); - private SearchResponseFormat searchResponseFormat = new SearchResponseFormat(new Durations(), languages, new AvatarResolverImpl()); + private SearchResponseFormat searchResponseFormat = new SearchResponseFormat(new Durations(), languages, new AvatarResolverImpl(), new TextRangeResponseFormatter()); private WsActionTester ws = new WsActionTester( new SearchAction(userSession, issueIndex, issueQueryFactory, searchResponseLoader, searchResponseFormat, diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java index daa6ab4e4c2..3d1b890861b 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java @@ -51,6 +51,7 @@ import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleDto; import org.sonar.db.rule.RuleTesting; import org.sonar.db.user.UserDto; +import org.sonar.server.issue.TextRangeResponseFormatter; import org.sonar.server.es.EsTester; import org.sonar.server.es.SearchOptions; import org.sonar.server.es.StartupIndexer; @@ -120,7 +121,7 @@ public class SearchActionTest { private IssueWorkflow issueWorkflow = new IssueWorkflow(new FunctionExecutor(issueFieldsSetter), issueFieldsSetter); private SearchResponseLoader searchResponseLoader = new SearchResponseLoader(userSession, dbClient, new TransitionService(userSession, issueWorkflow)); private Languages languages = new Languages(); - private SearchResponseFormat searchResponseFormat = new SearchResponseFormat(new Durations(), languages, new AvatarResolverImpl()); + private SearchResponseFormat searchResponseFormat = new SearchResponseFormat(new Durations(), languages, new AvatarResolverImpl(), new TextRangeResponseFormatter()); private WsActionTester ws = new WsActionTester(new SearchAction(userSession, issueIndex, issueQueryFactory, searchResponseLoader, searchResponseFormat, new MapSettings().asConfig(), System2.INSTANCE, dbClient)); private StartupIndexer permissionIndexer = new PermissionIndexer(dbClient, es.client(), issueIndexer); diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionTestOnSonarCloud.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionTestOnSonarCloud.java index 9443753ccdb..8d71785e450 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionTestOnSonarCloud.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionTestOnSonarCloud.java @@ -37,6 +37,7 @@ import org.sonar.db.component.ComponentDto; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.user.UserDto; +import org.sonar.server.issue.TextRangeResponseFormatter; import org.sonar.server.es.EsTester; import org.sonar.server.issue.AvatarResolverImpl; import org.sonar.server.issue.IssueFieldsSetter; @@ -77,7 +78,7 @@ public class SearchActionTestOnSonarCloud { private IssueWorkflow issueWorkflow = new IssueWorkflow(new FunctionExecutor(issueFieldsSetter), issueFieldsSetter); private SearchResponseLoader searchResponseLoader = new SearchResponseLoader(userSession, dbClient, new TransitionService(userSession, issueWorkflow)); private Languages languages = new Languages(); - private SearchResponseFormat searchResponseFormat = new SearchResponseFormat(new Durations(), languages, new AvatarResolverImpl()); + private SearchResponseFormat searchResponseFormat = new SearchResponseFormat(new Durations(), languages, new AvatarResolverImpl(), new TextRangeResponseFormatter()); private PermissionIndexerTester permissionIndexer = new PermissionIndexerTester(es, issueIndexer); private SearchAction underTest = new SearchAction(userSession, issueIndex, issueQueryFactory, searchResponseLoader, searchResponseFormat, diff --git a/sonar-ws/src/main/protobuf/ws-hotspots.proto b/sonar-ws/src/main/protobuf/ws-hotspots.proto index e6650472d04..cef59629a8e 100644 --- a/sonar-ws/src/main/protobuf/ws-hotspots.proto +++ b/sonar-ws/src/main/protobuf/ws-hotspots.proto @@ -31,23 +31,42 @@ message SearchWsResponse { optional sonarqube.ws.commons.Paging paging = 1; repeated Hotspot hotspots = 2; repeated Component components = 3; + + message Hotspot { + optional string key = 1; + optional string component = 2; + optional string project = 3; + optional string securityCategory = 4; + optional string vulnerabilityProbability = 5; + optional string status = 6; + // FIXME resolution field will be added later + // optional string resolution = 7; + optional int32 line = 8; + optional string message = 9; + optional string assignee = 10; + optional string author = 11; + optional string creationDate = 12; + optional string updateDate = 13; + } } -message Hotspot { +// Response of GET api/hotspots/show +message ShowWsResponse { optional string key = 1; - optional string component = 2; - optional string project = 3; - optional string securityCategory = 4; - optional string vulnerabilityProbability = 5; - optional string status = 6; + optional Component component = 2; + optional Component project = 3; + optional Rule rule = 4; + optional string status = 5; // FIXME resolution field will be added later -// optional string resolution = 7; - optional int32 line = 8; - optional string message = 9; - optional string assignee = 10; - optional string author = 11; - optional string creationDate = 12; - optional string updateDate = 13; +// optional string resolution = 6; + optional int32 line = 7; + optional string message = 8; + optional string assignee = 9; + // SCM login of the committer who introduced the issue + optional string author = 10; + optional string creationDate = 11; + optional string updateDate = 12; + optional sonarqube.ws.commons.TextRange textRange = 13; } message Component { @@ -58,3 +77,13 @@ 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; + optional string riskDescription = 5; + optional string vulnerabilityDescription = 6; + optional string fixRecommendations = 7; +} -- 2.39.5