From: Wouter Admiraal <45544358+wouter-admiraal-sonarsource@users.noreply.github.com>
Date: Tue, 20 Apr 2021 14:39:10 +0000 (+0200)
Subject: SONAR-14213 Do not follow redirects when interacting with gitlab.com API
X-Git-Tag: 8.9.0.43852~86
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=9c6b2db886aaa6943618573c1290aa6e422a4120;p=sonarqube.git

SONAR-14213 Do not follow redirects when interacting with gitlab.com API
---

diff --git a/server/sonar-alm-client/src/main/java/org/sonar/alm/client/gitlab/GitlabHttpClient.java b/server/sonar-alm-client/src/main/java/org/sonar/alm/client/gitlab/GitlabHttpClient.java
index 61dc5a7c0bc..36011e4e96e 100644
--- a/server/sonar-alm-client/src/main/java/org/sonar/alm/client/gitlab/GitlabHttpClient.java
+++ b/server/sonar-alm-client/src/main/java/org/sonar/alm/client/gitlab/GitlabHttpClient.java
@@ -55,6 +55,7 @@ public class GitlabHttpClient {
     client = new OkHttpClientBuilder()
       .setConnectTimeoutMs(timeoutConfiguration.getConnectTimeout())
       .setReadTimeoutMs(timeoutConfiguration.getReadTimeout())
+      .setFollowRedirects(false)
       .build();
   }
 
@@ -167,6 +168,8 @@ public class GitlabHttpClient {
         throw new IllegalArgumentException("Your GitLab token has insufficient scope");
       } else if (response.code() == HTTP_UNAUTHORIZED) {
         throw new IllegalArgumentException("Invalid personal access token");
+      } else if (response.isRedirect()) {
+        throw new IllegalArgumentException("Request was redirected, please provide the correct URL");
       } else {
         throw new IllegalArgumentException(errorMessage);
       }
diff --git a/server/sonar-alm-client/src/test/java/org/sonar/alm/client/gitlab/GitlabHttpClientTest.java b/server/sonar-alm-client/src/test/java/org/sonar/alm/client/gitlab/GitlabHttpClientTest.java
index 1a67af5818b..228d21a2349 100644
--- a/server/sonar-alm-client/src/test/java/org/sonar/alm/client/gitlab/GitlabHttpClientTest.java
+++ b/server/sonar-alm-client/src/test/java/org/sonar/alm/client/gitlab/GitlabHttpClientTest.java
@@ -99,6 +99,17 @@ public class GitlabHttpClientTest {
       .hasMessage("Invalid personal access token");
   }
 
+  @Test
+  public void should_throw_IllegalArgumentException_when_redirected() {
+    MockResponse response = new MockResponse()
+      .setResponseCode(308);
+    server.enqueue(response);
+
+    assertThatThrownBy(() -> underTest.searchProjects(gitlabUrl, "pat", "example", 1, 2))
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Request was redirected, please provide the correct URL");
+  }
+
   @Test
   public void get_project() {
     MockResponse response = new MockResponse()
diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/OkHttpClientBuilder.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/OkHttpClientBuilder.java
index ede86381037..aa3a736eb4c 100644
--- a/sonar-ws/src/main/java/org/sonarqube/ws/client/OkHttpClientBuilder.java
+++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/OkHttpClientBuilder.java
@@ -67,6 +67,7 @@ public class OkHttpClientBuilder {
   private String credentials;
   private String proxyLogin;
   private String proxyPassword;
+  private Boolean followRedirects;
   private long connectTimeoutMs = -1;
   private long readTimeoutMs = -1;
   private SSLSocketFactory sslSocketFactory = null;
@@ -159,6 +160,15 @@ public class OkHttpClientBuilder {
     return this;
   }
 
+  /**
+   * Set if redirects should be followed or not.
+   * Default is defined by OkHttp (true, follow redirects).
+   */
+  public OkHttpClientBuilder setFollowRedirects(Boolean followRedirects) {
+    this.followRedirects = followRedirects;
+    return this;
+  }
+
   public OkHttpClient build() {
     OkHttpClient.Builder builder = new OkHttpClient.Builder();
     builder.proxy(proxy);
@@ -182,6 +192,10 @@ public class OkHttpClientBuilder {
         return null;
       });
     }
+    if (followRedirects != null) {
+      builder.followRedirects(followRedirects);
+      builder.followSslRedirects(followRedirects);
+    }
 
     ConnectionSpec tls = new ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS)
       .allEnabledTlsVersions()
diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/client/OkHttpClientBuilderTest.java b/sonar-ws/src/test/java/org/sonarqube/ws/client/OkHttpClientBuilderTest.java
index 26c66de6af5..047cfbfc205 100644
--- a/sonar-ws/src/test/java/org/sonarqube/ws/client/OkHttpClientBuilderTest.java
+++ b/sonar-ws/src/test/java/org/sonarqube/ws/client/OkHttpClientBuilderTest.java
@@ -42,6 +42,8 @@ public class OkHttpClientBuilderTest {
     assertThat(okHttpClient.proxy()).isNull();
     assertThat(okHttpClient.networkInterceptors()).hasSize(1);
     assertThat(okHttpClient.sslSocketFactory()).isNotNull();
+    assertThat(okHttpClient.followRedirects()).isTrue();
+    assertThat(okHttpClient.followSslRedirects()).isTrue();
   }
 
   @Test
@@ -54,6 +56,23 @@ public class OkHttpClientBuilderTest {
     assertThat(okHttpClient.sslSocketFactory()).isEqualTo(sslSocketFactory);
   }
 
+  @Test
+  public void build_follow_redirects() {
+    OkHttpClient okHttpClientWithRedirect = underTest
+      .setFollowRedirects(true)
+      .build();
+
+    assertThat(okHttpClientWithRedirect.followRedirects()).isTrue();
+    assertThat(okHttpClientWithRedirect.followSslRedirects()).isTrue();
+
+    OkHttpClient okHttpClientWithoutRedirect = underTest
+      .setFollowRedirects(false)
+      .build();
+
+    assertThat(okHttpClientWithoutRedirect.followRedirects()).isFalse();
+    assertThat(okHttpClientWithoutRedirect.followSslRedirects()).isFalse();
+  }
+
   @Test
   public void build_throws_IAE_if_connect_timeout_is_negative() {
     expectedException.expect(IllegalArgumentException.class);