From 60455ce5b39122c81c0c2bdaaba3d1ae062c66ec Mon Sep 17 00:00:00 2001 From: Nicolas QUINQUENEL Date: Fri, 6 Dec 2024 17:17:38 +0100 Subject: [PATCH] SLCORE-1079 Fix SSF-694 --- .../sonarlint/core/ServerApiProvider.java | 30 +++++++++++++---- .../ConnectionAwareHttpClientProvider.java | 4 +-- .../core/ServerApiProviderTests.java | 14 ++++---- .../core/http/ApacheHttpClientAdapter.java | 33 ++++++++++++++++--- .../core/http/HttpClientProvider.java | 8 ++--- .../core/rpc/impl/SonarLintRpcServerImpl.java | 8 ----- .../ServerVersionAndStatusChecker.java | 10 ++++++ .../fixtures/SonarLintTestRpcServer.java | 10 ------ 8 files changed, 75 insertions(+), 42 deletions(-) diff --git a/backend/core/src/main/java/org/sonarsource/sonarlint/core/ServerApiProvider.java b/backend/core/src/main/java/org/sonarsource/sonarlint/core/ServerApiProvider.java index 6502ee9bca..3ac3382e61 100644 --- a/backend/core/src/main/java/org/sonarsource/sonarlint/core/ServerApiProvider.java +++ b/backend/core/src/main/java/org/sonarsource/sonarlint/core/ServerApiProvider.java @@ -27,6 +27,7 @@ import org.eclipse.lsp4j.jsonrpc.ResponseErrorException; import org.eclipse.lsp4j.jsonrpc.messages.ResponseError; import org.sonarsource.sonarlint.core.commons.log.SonarLintLogger; +import org.sonarsource.sonarlint.core.commons.progress.SonarLintCancelMonitor; import org.sonarsource.sonarlint.core.http.ConnectionAwareHttpClientProvider; import org.sonarsource.sonarlint.core.http.HttpClient; import org.sonarsource.sonarlint.core.http.HttpClientProvider; @@ -40,6 +41,7 @@ import org.sonarsource.sonarlint.core.serverapi.EndpointParams; import org.sonarsource.sonarlint.core.serverapi.ServerApi; import org.sonarsource.sonarlint.core.serverapi.ServerApiHelper; +import org.sonarsource.sonarlint.core.serverconnection.ServerVersionAndStatusChecker; import static org.apache.commons.lang.StringUtils.removeEnd; @@ -67,12 +69,22 @@ public Optional getServerApi(String connectionId) { LOG.debug("Connection '{}' is gone", connectionId); return Optional.empty(); } - return Optional.of(new ServerApi(params.get(), awareHttpClientProvider.getHttpClient(connectionId))); + var isBearerSupported = checkIfBearerIsSupported(params.get()); + return Optional.of(new ServerApi(params.get(), awareHttpClientProvider.getHttpClient(connectionId, isBearerSupported))); + } + + private boolean checkIfBearerIsSupported(EndpointParams params) { + var httpClient = awareHttpClientProvider.getHttpClient(); + var cancelMonitor = new SonarLintCancelMonitor(); + var serverApi = new ServerApi(params, httpClient); + var status = serverApi.system().getStatus(cancelMonitor); + var serverChecker = new ServerVersionAndStatusChecker(serverApi); + return serverChecker.isSupportingBearer(status); } public ServerApi getServerApi(String baseUrl, @Nullable String organization, String token) { var params = new EndpointParams(baseUrl, removeEnd(sonarCloudUri.toString(), "/").equals(removeEnd(baseUrl, "/")), organization); - return new ServerApi(params, httpClientProvider.getHttpClientWithPreemptiveAuth(token)); + return new ServerApi(params, httpClientProvider.getHttpClientWithPreemptiveAuth(token, true)); } public ServerApi getServerApiOrThrow(String connectionId) { @@ -81,7 +93,8 @@ public ServerApi getServerApiOrThrow(String connectionId) { var error = new ResponseError(SonarLintRpcErrorCode.CONNECTION_NOT_FOUND, "Connection '" + connectionId + "' is gone", connectionId); throw new ResponseErrorException(error); } - return new ServerApi(params.get(), awareHttpClientProvider.getHttpClient(connectionId)); + var isBearerSupported = checkIfBearerIsSupported(params.get()); + return new ServerApi(params.get(), awareHttpClientProvider.getHttpClient(connectionId, isBearerSupported)); } /** @@ -89,7 +102,7 @@ public ServerApi getServerApiOrThrow(String connectionId) { */ public ServerApi getForSonarCloudNoOrg(Either credentials) { var endpointParams = new EndpointParams(sonarCloudUri.toString(), true, null); - var httpClient = getClientFor(credentials); + var httpClient = getClientFor(endpointParams, credentials); return new ServerApi(new ServerApiHelper(endpointParams, httpClient)); } @@ -97,14 +110,17 @@ public ServerApi getForTransientConnection(Either new EndpointParams(sq.getServerUrl(), false, null), sc -> new EndpointParams(sonarCloudUri.toString(), true, sc.getOrganization())); - var httpClient = getClientFor(transientConnection + var httpClient = getClientFor(endpointParams, transientConnection .map(TransientSonarQubeConnectionDto::getCredentials, TransientSonarCloudConnectionDto::getCredentials)); return new ServerApi(new ServerApiHelper(endpointParams, httpClient)); } - private HttpClient getClientFor(Either credentials) { + private HttpClient getClientFor(EndpointParams params, Either credentials) { return credentials.map( - tokenDto -> httpClientProvider.getHttpClientWithPreemptiveAuth(tokenDto.getToken()), + tokenDto -> { + var isBearerSupported = checkIfBearerIsSupported(params); + return httpClientProvider.getHttpClientWithPreemptiveAuth(tokenDto.getToken(), isBearerSupported); + }, userPass -> httpClientProvider.getHttpClientWithPreemptiveAuth(userPass.getUsername(), userPass.getPassword())); } diff --git a/backend/core/src/main/java/org/sonarsource/sonarlint/core/http/ConnectionAwareHttpClientProvider.java b/backend/core/src/main/java/org/sonarsource/sonarlint/core/http/ConnectionAwareHttpClientProvider.java index 2290933d18..2a66dc07f0 100644 --- a/backend/core/src/main/java/org/sonarsource/sonarlint/core/http/ConnectionAwareHttpClientProvider.java +++ b/backend/core/src/main/java/org/sonarsource/sonarlint/core/http/ConnectionAwareHttpClientProvider.java @@ -45,14 +45,14 @@ public HttpClient getHttpClient() { return httpClientProvider.getHttpClient(); } - public HttpClient getHttpClient(String connectionId) { + public HttpClient getHttpClient(String connectionId, boolean shouldUseBearer) { var credentials = queryClientForConnectionCredentials(connectionId); if (credentials.isEmpty()) { // Fallback on client with no authentication return httpClientProvider.getHttpClient(); } return credentials.get().map( - tokenDto -> httpClientProvider.getHttpClientWithPreemptiveAuth(tokenDto.getToken()), + tokenDto -> httpClientProvider.getHttpClientWithPreemptiveAuth(tokenDto.getToken(), shouldUseBearer), userPass -> httpClientProvider.getHttpClientWithPreemptiveAuth(userPass.getUsername(), userPass.getPassword())); } diff --git a/backend/core/src/test/java/org/sonarsource/sonarlint/core/ServerApiProviderTests.java b/backend/core/src/test/java/org/sonarsource/sonarlint/core/ServerApiProviderTests.java index 80299404df..c85a1ef13c 100644 --- a/backend/core/src/test/java/org/sonarsource/sonarlint/core/ServerApiProviderTests.java +++ b/backend/core/src/test/java/org/sonarsource/sonarlint/core/ServerApiProviderTests.java @@ -49,7 +49,7 @@ void getServerApi_for_sonarqube() { var endpointParams = mock(EndpointParams.class); when(connectionRepository.getEndpointParams("sq1")).thenReturn(Optional.of(endpointParams)); var httpClient = mock(HttpClient.class); - when(awareHttpClientProvider.getHttpClient("sq1")).thenReturn(httpClient); + when(awareHttpClientProvider.getHttpClient("sq1", false)).thenReturn(httpClient); var serverApi = underTest.getServerApi("sq1"); @@ -59,7 +59,7 @@ void getServerApi_for_sonarqube() { @Test void getServerApi_for_sonarqube_notConnected() { var httpClient = mock(HttpClient.class); - when(httpClientProvider.getHttpClientWithPreemptiveAuth("token")).thenReturn(httpClient); + when(httpClientProvider.getHttpClientWithPreemptiveAuth("token", false)).thenReturn(httpClient); var serverApi = underTest.getServerApi("sq_notConnected", null, "token"); assertThat(serverApi.isSonarCloud()).isFalse(); @@ -70,7 +70,7 @@ void getServerApi_for_sonarcloud() { var endpointParams = mock(EndpointParams.class); when(connectionRepository.getEndpointParams("sc1")).thenReturn(Optional.of(endpointParams)); var httpClient = mock(HttpClient.class); - when(awareHttpClientProvider.getHttpClient("sc1")).thenReturn(httpClient); + when(awareHttpClientProvider.getHttpClient("sc1", true)).thenReturn(httpClient); var serverApi = underTest.getServerApi("sc1"); @@ -80,7 +80,7 @@ void getServerApi_for_sonarcloud() { @Test void getServerApi_for_sonarcloud_with_trailing_slash_notConnected() { var httpClient = mock(HttpClient.class); - when(httpClientProvider.getHttpClientWithPreemptiveAuth("token")).thenReturn(httpClient); + when(httpClientProvider.getHttpClientWithPreemptiveAuth("token", true)).thenReturn(httpClient); var serverApi = underTest.getServerApi("https://sonarcloud.io/", "organization", "token"); assertThat(serverApi.isSonarCloud()).isTrue(); @@ -89,7 +89,7 @@ void getServerApi_for_sonarcloud_with_trailing_slash_notConnected() { @Test void getServerApi_for_sonarcloud_notConnected() { var httpClient = mock(HttpClient.class); - when(httpClientProvider.getHttpClientWithPreemptiveAuth("token")).thenReturn(httpClient); + when(httpClientProvider.getHttpClientWithPreemptiveAuth("token", true)).thenReturn(httpClient); var serverApi = underTest.getServerApi("https://sonarcloud.io", "organization", "token"); assertThat(serverApi.isSonarCloud()).isTrue(); @@ -99,7 +99,7 @@ void getServerApi_for_sonarcloud_notConnected() { void getServerApi_returns_empty_if_connection_doesnt_exists() { when(connectionRepository.getConnectionById("sc1")).thenReturn(null); var httpClient = mock(HttpClient.class); - when(awareHttpClientProvider.getHttpClient("sc1")).thenReturn(httpClient); + when(awareHttpClientProvider.getHttpClient("sc1", true)).thenReturn(httpClient); var serverApi = underTest.getServerApi("sc1"); @@ -109,7 +109,7 @@ void getServerApi_returns_empty_if_connection_doesnt_exists() { @Test void getServerApi_returns_empty_if_client_cant_provide_httpclient() { when(connectionRepository.getConnectionById("sc1")).thenReturn(new SonarCloudConnectionConfiguration(URI.create("http://server1"), "sc1", "myorg", true)); - when(awareHttpClientProvider.getHttpClient("sc1")).thenReturn(null); + when(awareHttpClientProvider.getHttpClient("sc1", true)).thenReturn(null); var serverApi = underTest.getServerApi("sc1"); diff --git a/backend/http/src/main/java/org/sonarsource/sonarlint/core/http/ApacheHttpClientAdapter.java b/backend/http/src/main/java/org/sonarsource/sonarlint/core/http/ApacheHttpClientAdapter.java index 3c68eeb364..6d40538c92 100644 --- a/backend/http/src/main/java/org/sonarsource/sonarlint/core/http/ApacheHttpClientAdapter.java +++ b/backend/http/src/main/java/org/sonarsource/sonarlint/core/http/ApacheHttpClientAdapter.java @@ -47,7 +47,7 @@ class ApacheHttpClientAdapter implements HttpClient { private static final SonarLintLogger LOG = SonarLintLogger.get(); - + private static final String AUTHORIZATION_HEADER = "Authorization"; private static final Timeout STREAM_CONNECTION_REQUEST_TIMEOUT = Timeout.ofSeconds(10); private static final Timeout STREAM_CONNECTION_TIMEOUT = Timeout.ofMinutes(1); private final CloseableHttpAsyncClient apacheClient; @@ -55,12 +55,14 @@ class ApacheHttpClientAdapter implements HttpClient { private final String usernameOrToken; @Nullable private final String password; + private final boolean shouldUseBearer; private boolean connected = false; - ApacheHttpClientAdapter(CloseableHttpAsyncClient apacheClient, @Nullable String usernameOrToken, @Nullable String password) { + private ApacheHttpClientAdapter(CloseableHttpAsyncClient apacheClient, @Nullable String usernameOrToken, @Nullable String password, boolean shouldUseBearer) { this.apacheClient = apacheClient; this.usernameOrToken = usernameOrToken; this.password = password; + this.shouldUseBearer = shouldUseBearer; } @Override @@ -109,7 +111,11 @@ public AsyncRequest getEventStream(String url, HttpConnectionListener connection .build()); if (usernameOrToken != null) { - request.setHeader("Authorization", basic(usernameOrToken, Objects.requireNonNullElse(password, ""))); + if (shouldUseBearer) { + request.setHeader(AUTHORIZATION_HEADER, bearer(usernameOrToken)); + } else { + request.setHeader(AUTHORIZATION_HEADER, basic(usernameOrToken, Objects.requireNonNullElse(password, ""))); + } } request.setHeader("Accept", "text/event-stream"); connected = false; @@ -246,7 +252,11 @@ public boolean cancel(boolean mayInterruptIfRunning) { private CompletableFuture executeAsync(SimpleHttpRequest httpRequest) { try { if (usernameOrToken != null) { - httpRequest.setHeader("Authorization", basic(usernameOrToken, Objects.requireNonNullElse(password, ""))); + if (shouldUseBearer) { + httpRequest.setHeader(AUTHORIZATION_HEADER, bearer(usernameOrToken)); + } else { + httpRequest.setHeader(AUTHORIZATION_HEADER, basic(usernameOrToken, Objects.requireNonNullElse(password, ""))); + } } return new CompletableFutureWrappingFuture(httpRequest); } catch (Exception e) { @@ -260,6 +270,10 @@ private static String basic(String username, String password) { return String.format("Basic %s", encoded); } + private static String bearer(String token) { + return String.format("Bearer %s", token); + } + public static class HttpAsyncRequest implements AsyncRequest { private final Future response; @@ -275,7 +289,18 @@ public void cancel() { // ignore errors } } + } + + public static ApacheHttpClientAdapter withoutCredentials(CloseableHttpAsyncClient apacheClient) { + return new ApacheHttpClientAdapter(apacheClient, null, null, false); + } + + public static ApacheHttpClientAdapter withUsernamePassword(CloseableHttpAsyncClient apacheClient, String username, @Nullable String password) { + return new ApacheHttpClientAdapter(apacheClient, username, password, false); + } + public static ApacheHttpClientAdapter withToken(CloseableHttpAsyncClient apacheClient, String token, boolean shouldUseBearer) { + return new ApacheHttpClientAdapter(apacheClient, token, null, shouldUseBearer); } } diff --git a/backend/http/src/main/java/org/sonarsource/sonarlint/core/http/HttpClientProvider.java b/backend/http/src/main/java/org/sonarsource/sonarlint/core/http/HttpClientProvider.java index 79ffa550a6..9484e2bb24 100644 --- a/backend/http/src/main/java/org/sonarsource/sonarlint/core/http/HttpClientProvider.java +++ b/backend/http/src/main/java/org/sonarsource/sonarlint/core/http/HttpClientProvider.java @@ -136,15 +136,15 @@ private static RequestConfig buildRequestConfig(@Nullable Timeout connectionRequ } public HttpClient getHttpClient() { - return new ApacheHttpClientAdapter(sharedClient, null, null); + return ApacheHttpClientAdapter.withoutCredentials(sharedClient); } public HttpClient getHttpClientWithPreemptiveAuth(String username, @Nullable String password) { - return new ApacheHttpClientAdapter(sharedClient, username, password); + return ApacheHttpClientAdapter.withUsernamePassword(sharedClient, username, password); } - public HttpClient getHttpClientWithPreemptiveAuth(String token) { - return new ApacheHttpClientAdapter(sharedClient, token, null); + public HttpClient getHttpClientWithPreemptiveAuth(String token, boolean shouldUseBearer) { + return ApacheHttpClientAdapter.withToken(sharedClient, token, shouldUseBearer); } public WebSocketClient getWebSocketClient(String token) { diff --git a/backend/rpc-impl/src/main/java/org/sonarsource/sonarlint/core/rpc/impl/SonarLintRpcServerImpl.java b/backend/rpc-impl/src/main/java/org/sonarsource/sonarlint/core/rpc/impl/SonarLintRpcServerImpl.java index 062f232968..24b2b8e917 100644 --- a/backend/rpc-impl/src/main/java/org/sonarsource/sonarlint/core/rpc/impl/SonarLintRpcServerImpl.java +++ b/backend/rpc-impl/src/main/java/org/sonarsource/sonarlint/core/rpc/impl/SonarLintRpcServerImpl.java @@ -288,14 +288,6 @@ public int getEmbeddedServerPort() { return getInitializedApplicationContext().getBean(EmbeddedServer.class).getPort(); } - public HttpClient getHttpClientNoAuth() { - return getInitializedApplicationContext().getBean(ConnectionAwareHttpClientProvider.class).getHttpClient(); - } - - public HttpClient getHttpClient(String connectionId) { - return getInitializedApplicationContext().getBean(ConnectionAwareHttpClientProvider.class).getHttpClient(connectionId); - } - public LocalOnlyIssueStorageService getLocalOnlyIssueStorageService() { return getInitializedApplicationContext().getBean(LocalOnlyIssueStorageService.class); } diff --git a/backend/server-connection/src/main/java/org/sonarsource/sonarlint/core/serverconnection/ServerVersionAndStatusChecker.java b/backend/server-connection/src/main/java/org/sonarsource/sonarlint/core/serverconnection/ServerVersionAndStatusChecker.java index a0ffe6c467..ed3962143c 100644 --- a/backend/server-connection/src/main/java/org/sonarsource/sonarlint/core/serverconnection/ServerVersionAndStatusChecker.java +++ b/backend/server-connection/src/main/java/org/sonarsource/sonarlint/core/serverconnection/ServerVersionAndStatusChecker.java @@ -29,6 +29,7 @@ public class ServerVersionAndStatusChecker { private static final String MIN_SQ_VERSION = "9.9"; + private static final String MIN_SQ_VERSION_SUPPORTING_BEARER = "10.0"; private final SystemApi systemApi; private final boolean isSonarCloud; @@ -53,6 +54,15 @@ public void checkVersionAndStatus(SonarLintCancelMonitor cancelMonitor) { } } + public boolean isSupportingBearer(ServerStatusInfo serverStatus) { + if (isSonarCloud) { + return true; + } else { + var serverVersion = Version.create(serverStatus.getVersion()); + return serverVersion.compareToIgnoreQualifier(Version.create(MIN_SQ_VERSION_SUPPORTING_BEARER)) >= 0; + } + } + private static void checkServerUp(ServerStatusInfo serverStatus) { if (!serverStatus.isUp()) { throw new IllegalStateException(serverNotReady(serverStatus)); diff --git a/medium-tests/src/test/java/mediumtest/fixtures/SonarLintTestRpcServer.java b/medium-tests/src/test/java/mediumtest/fixtures/SonarLintTestRpcServer.java index 4cac4934bd..b727e365c8 100644 --- a/medium-tests/src/test/java/mediumtest/fixtures/SonarLintTestRpcServer.java +++ b/medium-tests/src/test/java/mediumtest/fixtures/SonarLintTestRpcServer.java @@ -203,14 +203,4 @@ public int getEmbeddedServerPort() { return serverUsingJava.getEmbeddedServerPort(); } - @Deprecated(forRemoval = true) - public HttpClient getHttpClientNoAuth() { - return serverUsingJava.getHttpClientNoAuth(); - } - - @Deprecated(forRemoval = true) - public HttpClient getHttpClient(String connectionId) { - return serverUsingJava.getHttpClient(connectionId); - } - }