Skip to content

Commit

Permalink
SLCORE-1067 SOQ-10-001 WP2 remediation
Browse files Browse the repository at this point in the history
  • Loading branch information
nquinquenel committed Dec 6, 2024
1 parent 26248db commit 1a73877
Show file tree
Hide file tree
Showing 10 changed files with 257 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,14 @@
package org.sonarsource.sonarlint.core.embedded.server;

import com.google.common.util.concurrent.MoreExecutors;
import java.util.Collection;
import java.util.HashSet;
import java.util.Objects;
import java.util.concurrent.CancellationException;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
import javax.annotation.PreDestroy;
Expand All @@ -32,6 +36,7 @@
import org.sonarsource.sonarlint.core.BindingCandidatesFinder;
import org.sonarsource.sonarlint.core.BindingSuggestionProvider;
import org.sonarsource.sonarlint.core.SonarCloudActiveEnvironment;
import org.sonarsource.sonarlint.core.commons.BoundScope;
import org.sonarsource.sonarlint.core.commons.log.SonarLintLogger;
import org.sonarsource.sonarlint.core.commons.progress.ExecutorServiceShutdownWatchable;
import org.sonarsource.sonarlint.core.commons.progress.SonarLintCancelMonitor;
Expand Down Expand Up @@ -78,13 +83,13 @@ public RequestHandlerBindingAssistant(BindingSuggestionProvider bindingSuggestio
}

interface Callback {
void andThen(String connectionId, @Nullable String configurationScopeId, SonarLintCancelMonitor cancelMonitor);
void andThen(String connectionId, Collection<String> boundScopes, @Nullable String configurationScopeId, SonarLintCancelMonitor cancelMonitor);
}

void assistConnectionAndBindingIfNeededAsync(AssistCreatingConnectionParams connectionParams, String projectKey, Callback callback) {
void assistConnectionAndBindingIfNeededAsync(AssistCreatingConnectionParams connectionParams, String projectKey, String origin, Callback callback) {
var cancelMonitor = new SonarLintCancelMonitor();
cancelMonitor.watchForShutdown(executorService);
executorService.submit(() -> assistConnectionAndBindingIfNeeded(connectionParams, projectKey, callback, cancelMonitor));
executorService.submit(() -> assistConnectionAndBindingIfNeeded(connectionParams, projectKey, origin, callback, cancelMonitor));
}

private void assistConnectionAndBindingIfNeeded(AssistCreatingConnectionParams connectionParams, String projectKey, String origin,
Expand All @@ -102,19 +107,22 @@ private void assistConnectionAndBindingIfNeeded(AssistCreatingConnectionParams c
var assistNewConnectionResult = assistCreatingConnectionAndWaitForRepositoryUpdate(connectionParams, cancelMonitor);
var assistNewBindingResult = assistBindingAndWaitForRepositoryUpdate(assistNewConnectionResult.getNewConnectionId(), isSonarCloud,
projectKey, cancelMonitor);
callback.andThen(assistNewConnectionResult.getNewConnectionId(), assistNewBindingResult.getConfigurationScopeId(), cancelMonitor);
var boundScopes = new HashSet<String>();
if (assistNewBindingResult.getConfigurationScopeId() != null) {
boundScopes.add(assistNewBindingResult.getConfigurationScopeId());
}
callback.andThen(assistNewConnectionResult.getNewConnectionId(), boundScopes, assistNewBindingResult.getConfigurationScopeId(), cancelMonitor);
} finally {
endFullBindingProcess();
}
} else {
// Should we check that the origin is matching the serverUrl URI?
var isOriginTrusted = repository.hasConnectionWithOrigin(origin);
if (isOriginTrusted) {
// we pick the first connection but this could lead to issues later if there were several matches (make the user select the right
// one?)
assistBindingIfNeeded(connectionsMatchingOrigin.get(0).getConnectionId(), isSonarCloud, projectKey, callback, cancelMonitor);
} else {
LOG.warn("The origin " + origin + " is not trusted, this could be a malicious request");
LOG.warn("The origin '" + origin + "' is not trusted, this could be a malicious request");
client.showMessage(new ShowMessageParams(MessageType.ERROR, "SonarQube for IDE received a non-trusted request and could not proceed with it. " +
"See logs for more details."));
}
Expand Down Expand Up @@ -153,10 +161,15 @@ private void assistBindingIfNeeded(String connectionId, boolean isSonarCloud, St
var scopes = configurationRepository.getBoundScopesToConnectionAndSonarProject(connectionId, projectKey);
if (scopes.isEmpty()) {
var assistNewBindingResult = assistBindingAndWaitForRepositoryUpdate(connectionId, isSonarCloud, projectKey, cancelMonitor);
callback.andThen(connectionId, assistNewBindingResult.getConfigurationScopeId(), cancelMonitor);
var boundScopes = new HashSet<String>();
if (assistNewBindingResult.getConfigurationScopeId() != null) {
boundScopes.add(assistNewBindingResult.getConfigurationScopeId());
}
callback.andThen(connectionId, boundScopes, assistNewBindingResult.getConfigurationScopeId(), cancelMonitor);
} else {
var boundScopes = scopes.stream().map(BoundScope::getConfigScopeId).filter(Objects::nonNull).collect(Collectors.toSet());
// we pick the first bound scope but this could lead to issues later if there were several matches (make the user select the right one?)
callback.andThen(connectionId, scopes.iterator().next().getConfigScopeId(), cancelMonitor);
callback.andThen(connectionId, boundScopes, scopes.iterator().next().getConfigScopeId(), cancelMonitor);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
import java.io.IOException;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Optional;
Expand All @@ -37,9 +39,7 @@
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.Method;
import org.apache.hc.core5.http.NameValuePair;
import org.apache.hc.core5.http.ParseException;
import org.apache.hc.core5.http.ProtocolException;
import org.apache.hc.core5.http.io.HttpRequestHandler;
import org.apache.hc.core5.http.io.entity.EntityUtils;
import org.apache.hc.core5.http.io.entity.StringEntity;
Expand All @@ -48,6 +48,7 @@
import org.sonarsource.sonarlint.core.SonarCloudActiveEnvironment;
import org.sonarsource.sonarlint.core.commons.log.SonarLintLogger;
import org.sonarsource.sonarlint.core.file.PathTranslationService;
import org.sonarsource.sonarlint.core.fs.ClientFileSystemService;
import org.sonarsource.sonarlint.core.rpc.protocol.SonarLintRpcClient;
import org.sonarsource.sonarlint.core.rpc.protocol.backend.initialize.InitializeParams;
import org.sonarsource.sonarlint.core.rpc.protocol.client.branch.MatchProjectBranchParams;
Expand Down Expand Up @@ -82,24 +83,29 @@ public class ShowFixSuggestionRequestHandler implements HttpRequestHandler {
private final PathTranslationService pathTranslationService;
private final String sonarCloudUrl;
private final SonarProjectBranchesSynchronizationService sonarProjectBranchesSynchronizationService;
private final ClientFileSystemService clientFs;

public ShowFixSuggestionRequestHandler(SonarLintRpcClient client, TelemetryService telemetryService,
InitializeParams params, RequestHandlerBindingAssistant requestHandlerBindingAssistant,
PathTranslationService pathTranslationService, SonarCloudActiveEnvironment sonarCloudActiveEnvironment,
SonarProjectBranchesSynchronizationService sonarProjectBranchesSynchronizationService) {
SonarProjectBranchesSynchronizationService sonarProjectBranchesSynchronizationService, ClientFileSystemService clientFs) {
this.client = client;
this.telemetryService = telemetryService;
this.canOpenFixSuggestion = params.getFeatureFlags().canOpenFixSuggestion();
this.requestHandlerBindingAssistant = requestHandlerBindingAssistant;
this.pathTranslationService = pathTranslationService;
this.sonarCloudUrl = sonarCloudActiveEnvironment.getUri().toString();
this.sonarProjectBranchesSynchronizationService = sonarProjectBranchesSynchronizationService;
this.clientFs = clientFs;
}

@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException, IOException {
var showFixSuggestionQuery = extractQuery(request);
if (!canOpenFixSuggestion || !Method.POST.isSame(request.getMethod()) || !showFixSuggestionQuery.isValid()) {
var originHeader = request.getHeader("Origin");
var origin = originHeader != null ? originHeader.getValue() : null;
var showFixSuggestionQuery = extractQuery(request, origin);

if (origin == null || !canOpenFixSuggestion || !Method.POST.isSame(request.getMethod()) || !showFixSuggestionQuery.isValid()) {
response.setCode(HttpStatus.SC_BAD_REQUEST);
return;
}
Expand All @@ -111,27 +117,52 @@ public void handle(ClassicHttpRequest request, ClassicHttpResponse response, Htt

requestHandlerBindingAssistant.assistConnectionAndBindingIfNeededAsync(
serverConnectionParams,
showFixSuggestionQuery.projectKey,
(connectionId, configScopeId, cancelMonitor) -> {
showFixSuggestionQuery.projectKey, origin,
(connectionId, boundScopes, configScopeId, cancelMonitor) -> {
if (configScopeId != null) {
var branchToMatch = showFixSuggestionQuery.branch;
if (branchToMatch == null) {
branchToMatch = sonarProjectBranchesSynchronizationService.findMainBranch(connectionId, showFixSuggestionQuery.projectKey, cancelMonitor);
branchToMatch = sonarProjectBranchesSynchronizationService.findMainBranch(connectionId, showFixSuggestionQuery.projectKey,
cancelMonitor);
}
var localBranchMatchesRequesting = client.matchProjectBranch(new MatchProjectBranchParams(configScopeId, branchToMatch)).join().isBranchMatched();
var localBranchMatchesRequesting =
client.matchProjectBranch(new MatchProjectBranchParams(configScopeId, branchToMatch)).join().isBranchMatched();
if (!localBranchMatchesRequesting) {
client.showMessage(new ShowMessageParams(MessageType.ERROR, "Attempted to show a fix suggestion from branch '" + branchToMatch + "', " +
"which is different from the currently checked-out branch.\nPlease switch to the correct branch and try again."));
client.showMessage(new ShowMessageParams(MessageType.ERROR,
"Attempted to show a fix suggestion from branch '" + branchToMatch + "', which is different from the currently " +
"checked-out branch.\nPlease switch to the correct branch and try again."));
return;
}
showFixSuggestionForScope(configScopeId, showFixSuggestionQuery.issueKey, showFixSuggestionQuery.fixSuggestion);

if (doesClientFileExists(configScopeId, showFixSuggestionQuery.fixSuggestion.fileEdit.path, boundScopes)) {
showFixSuggestionForScope(configScopeId, showFixSuggestionQuery.issueKey, showFixSuggestionQuery.fixSuggestion);
} else {
client.showMessage(new ShowMessageParams(MessageType.ERROR, "Attempted to show a fix suggestion for a file that is " +
"not known by SonarQube for IDE"));
}
}
});

response.setCode(HttpStatus.SC_OK);
response.setEntity(new StringEntity("OK"));
}

private boolean doesClientFileExists(String configScopeId, String filePath, Collection<String> boundScopes) {
var optTranslation = pathTranslationService.getOrComputePathTranslation(configScopeId);
if (optTranslation.isPresent()) {
var translation = optTranslation.get();
var idePath = translation.serverToIdePath(Paths.get(filePath));
for (var scope: boundScopes) {
for (var file: clientFs.getFiles(scope)) {
if (Path.of(file.getUri()).endsWith(idePath)) {
return true;
}
}
}
}
return false;
}

private static AssistCreatingConnectionParams createAssistServerConnectionParams(ShowFixSuggestionQuery query) {
String tokenName = query.getTokenName();
String tokenValue = query.getTokenValue();
Expand All @@ -140,9 +171,8 @@ private static AssistCreatingConnectionParams createAssistServerConnectionParams
: new AssistCreatingConnectionParams(new SonarQubeConnectionParams(query.getServerUrl(), tokenName, tokenValue));
}

private boolean isSonarCloud(ClassicHttpRequest request) throws ProtocolException {
return Optional.ofNullable(request.getHeader("Origin"))
.map(NameValuePair::getValue)
private boolean isSonarCloud(@Nullable String origin) {
return Optional.ofNullable(origin)
.map(sonarCloudUrl::equals)
.orElse(false);
}
Expand All @@ -167,7 +197,7 @@ private void showFixSuggestionForScope(String configScopeId, String issueKey, Fi
}

@VisibleForTesting
ShowFixSuggestionQuery extractQuery(ClassicHttpRequest request) throws HttpException, IOException {
ShowFixSuggestionQuery extractQuery(ClassicHttpRequest request, @Nullable String origin) throws HttpException, IOException {
var params = new HashMap<String, String>();
try {
new URIBuilder(request.getUri(), StandardCharsets.UTF_8)
Expand All @@ -177,7 +207,7 @@ ShowFixSuggestionQuery extractQuery(ClassicHttpRequest request) throws HttpExcep
// Ignored
}
var payload = extractAndValidatePayload(request);
boolean isSonarCloud = isSonarCloud(request);
boolean isSonarCloud = isSonarCloud(origin);
var serverUrl = isSonarCloud ? sonarCloudUrl : params.get("server");
return new ShowFixSuggestionQuery(serverUrl, params.get("project"), params.get("issue"), params.get("branch"),
params.get("tokenName"), params.get("tokenValue"), params.get("organizationKey"), isSonarCloud, payload);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,18 @@ public ShowHotspotRequestHandler(SonarLintRpcClient client, ServerApiProvider se

@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException, IOException {
var originHeader = request.getHeader("Origin");
var origin = originHeader != null ? originHeader.getValue() : null;
var showHotspotQuery = extractQuery(request);
if (!Method.GET.isSame(request.getMethod()) || !showHotspotQuery.isValid()) {
if (origin == null || !Method.GET.isSame(request.getMethod()) || !showHotspotQuery.isValid()) {
response.setCode(HttpStatus.SC_BAD_REQUEST);
return;
}
telemetryService.showHotspotRequestReceived();
var sonarQubeConnectionParams = new SonarQubeConnectionParams(showHotspotQuery.serverUrl, null, null);
var connectionParams = new AssistCreatingConnectionParams(sonarQubeConnectionParams);
requestHandlerBindingAssistant.assistConnectionAndBindingIfNeededAsync(connectionParams, showHotspotQuery.projectKey,
(connectionId, configScopeId, cancelMonitor) -> {
requestHandlerBindingAssistant.assistConnectionAndBindingIfNeededAsync(connectionParams, showHotspotQuery.projectKey, origin,
(connectionId, boundScopes, configScopeId, cancelMonitor) -> {
if (configScopeId != null) {
showHotspotForScope(connectionId, configScopeId, showHotspotQuery.hotspotKey, cancelMonitor);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,10 @@ public ShowIssueRequestHandler(SonarLintRpcClient client, ServerApiProvider serv

@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException, IOException {
var originHeader = request.getHeader("Origin");
var origin = originHeader != null ? originHeader.getValue() : null;
var showIssueQuery = extractQuery(request);
if (!Method.GET.isSame(request.getMethod()) || !showIssueQuery.isValid()) {
if (origin == null || !Method.GET.isSame(request.getMethod()) || !showIssueQuery.isValid()) {
response.setCode(HttpStatus.SC_BAD_REQUEST);
return;
}
Expand All @@ -106,7 +108,8 @@ public void handle(ClassicHttpRequest request, ClassicHttpResponse response, Htt
requestHandlerBindingAssistant.assistConnectionAndBindingIfNeededAsync(
serverConnectionParams,
showIssueQuery.projectKey,
(connectionId, configScopeId, cancelMonitor) -> {
origin,
(connectionId, boundScopes, configScopeId, cancelMonitor) -> {
if (configScopeId != null) {
var branchToMatch = showIssueQuery.branch;
if (branchToMatch == null) {
Expand Down
Loading

0 comments on commit 1a73877

Please sign in to comment.