Refactor Tsunami's trust-all-certificates config so that CLI option v…

…alue takes precedence over the config file value.

PiperOrigin-RevId: 364666914
Change-Id: I74adfd61175696f1429b6ffee99a4d39079c7783

common/src/main/java/com/google/tsunami/common/net/http/HttpClientCliOptions.java

@@ -25,9 +25,8 @@ public final class HttpClientCliOptions implements CliOption {
 
   @Parameter(
       names = "--http-client-trust-all-certificates",
-      description =
-          "Whether the HTTP client should trust all certificates on HTTPS traffic. Default false.")
-  boolean trustAllCertificates = false;
+      description = "Whether the HTTP client should trust all certificates on HTTPS traffic.")
+  Boolean trustAllCertificates;
 
   @Override
   public void validate() {}

common/src/main/java/com/google/tsunami/common/net/http/HttpClientConfigProperties.java

@@ -20,5 +20,6 @@
 /** Configuration properties for {@link HttpClient}. */
 @ConfigProperties("common.net.http")
 public final class HttpClientConfigProperties {
-  boolean trustAllCertificates = false;
+  /** Whether the HTTP client should trust all certificates on HTTPS traffic. */
+  Boolean trustAllCertificates;
 }

common/src/main/java/com/google/tsunami/common/net/http/HttpClientModule.java

@@ -128,8 +128,13 @@ OkHttpClient provideOkHttpClient(
   boolean shouldTrustAllCertificates(
       HttpClientCliOptions httpClientCliOptions,
       HttpClientConfigProperties httpClientConfigProperties) {
-    return httpClientCliOptions.trustAllCertificates
-        || httpClientConfigProperties.trustAllCertificates;
+    if (httpClientCliOptions.trustAllCertificates != null) {
+      return httpClientCliOptions.trustAllCertificates;
+    }
+    if (httpClientConfigProperties.trustAllCertificates != null) {
+      return httpClientConfigProperties.trustAllCertificates;
+    }
+    return false;
   }
 
   @Qualifier

common/src/test/java/com/google/tsunami/common/net/http/HttpClientModuleTest.java

@@ -140,11 +140,28 @@ public void setTrustAllCertificates_whenFalseAndCertIsInvalid_throws()
     mockWebServer.shutdown();
   }
 
+  @Test
+  public void setTrustAllCertificates_whenBothCliAndConfigValuesAreSet_cliValueTakesPrecedence()
+      throws GeneralSecurityException, IOException {
+    cliOptions.trustAllCertificates = false;
+    configProperties.trustAllCertificates = true;
+    HttpClient httpClient =
+        Guice.createInjector(getTrustAllCertificatesTestingModule()).getInstance(HttpClient.class);
+    MockWebServer mockWebServer = startMockWebServerWithSsl();
+
+    // The certificate used in test is a self-signed one. HttpClient will reject it unless the
+    // certificate is explicitly trusted.
+    assertThrows(
+        SSLHandshakeException.class,
+        () -> httpClient.send(get(mockWebServer.url("/")).withEmptyHeaders().build()));
+
+    mockWebServer.shutdown();
+  }
+
   @Test
   public void setTrustAllCertificates_whenCliOptionEnabledAndCertIsInvalid_ignoresCertError()
       throws GeneralSecurityException, IOException {
     cliOptions.trustAllCertificates = true;
-    configProperties.trustAllCertificates = false;
     HttpClient httpClient =
         Guice.createInjector(getTrustAllCertificatesTestingModule()).getInstance(HttpClient.class);
     MockWebServer mockWebServer = startMockWebServerWithSsl();
@@ -159,7 +176,6 @@ public void setTrustAllCertificates_whenCliOptionEnabledAndCertIsInvalid_ignores
   public void
       setTrustAllCertificates_whenConfigPropropertyEnabledAndCertIsInvalid_ignoresCertError()
           throws GeneralSecurityException, IOException {
-    cliOptions.trustAllCertificates = false;
     configProperties.trustAllCertificates = true;
     HttpClient httpClient =
         Guice.createInjector(getTrustAllCertificatesTestingModule()).getInstance(HttpClient.class);