Fix SimpleSslContextBuilderTest not testing anything by not loading keys correctly (#970)

Author: Dmitry Spikhalskiy

temporal-serviceclient/src/main/java/io/temporal/serviceclient/SimpleSslContextBuilder.java

@@ -28,6 +28,7 @@
 import java.security.KeyStore;
 import java.security.KeyStoreException;
 import java.security.NoSuchAlgorithmException;
+import javax.annotation.Nullable;
 import javax.net.ssl.*;
 
 public class SimpleSslContextBuilder {
@@ -44,9 +45,9 @@ public class SimpleSslContextBuilder {
           // gRPC requires http2 protocol.
           ApplicationProtocolNames.HTTP_2);
 
-  private final PKCS pkcs;
-  private final InputStream keyCertChain;
-  private final InputStream key;
+  private final @Nullable PKCS pkcs;
+  private final @Nullable InputStream keyCertChain;
+  private final @Nullable InputStream key;
   private TrustManager trustManager;
   private boolean useInsecureTrustManager;
   private String keyPassword;
@@ -66,20 +67,32 @@ public static SimpleSslContextBuilder newBuilder(InputStream keyCertChain, Input
     return forPKCS8(keyCertChain, key);
   }
 
+  /**
+   * Explicitly creates a builder without a client private key or certificate chain.
+   *
+   * <p>{@link #forPKCS8} and {@link #forPKCS12} support null inputs too for easier configuration
+   * API
+   */
+  public static SimpleSslContextBuilder noKeyOrCertChain() {
+    return new SimpleSslContextBuilder(null, null, null);
+  }
+
   /**
    * @param keyCertChain - an input stream for an X.509 client certificate chain in PEM format.
    * @param key - an input stream for a PKCS#8 client private key in PEM format.
    */
-  public static SimpleSslContextBuilder forPKCS8(InputStream keyCertChain, InputStream key) {
+  public static SimpleSslContextBuilder forPKCS8(
+      @Nullable InputStream keyCertChain, @Nullable InputStream key) {
     return new SimpleSslContextBuilder(PKCS.PKCS_8, keyCertChain, key);
   }
 
   /** @param pfxKeyArchive - an input stream for .pfx or .p12 PKCS12 archive file */
-  public static SimpleSslContextBuilder forPKCS12(InputStream pfxKeyArchive) {
+  public static SimpleSslContextBuilder forPKCS12(@Nullable InputStream pfxKeyArchive) {
     return new SimpleSslContextBuilder(PKCS.PKCS_12, null, pfxKeyArchive);
   }
 
-  private SimpleSslContextBuilder(PKCS pkcs, InputStream keyCertChain, InputStream key) {
+  private SimpleSslContextBuilder(
+      @Nullable PKCS pkcs, @Nullable InputStream keyCertChain, @Nullable InputStream key) {
     this.pkcs = pkcs;
     this.keyCertChain = keyCertChain;
     this.key = key;
@@ -109,16 +122,18 @@ public SslContext build() throws SSLException {
                         : getDefaultTrustManager())
             .applicationProtocolConfig(DEFAULT_APPLICATION_PROTOCOL_CONFIG);
 
-    switch (pkcs) {
-      case PKCS_8:
-        // netty by default supports PKCS8
-        sslContextBuilder.keyManager(keyCertChain, key, keyPassword);
-        break;
-      case PKCS_12:
-        sslContextBuilder.keyManager(createPKCS12KeyManager());
-        break;
-      default:
-        throw new IllegalArgumentException("PKCS " + pkcs + " is not implemented");
+    if (pkcs != null && (key != null || keyCertChain != null)) {
+      switch (pkcs) {
+        case PKCS_8:
+          // netty by default supports PKCS8
+          sslContextBuilder.keyManager(keyCertChain, key, keyPassword);
+          break;
+        case PKCS_12:
+          sslContextBuilder.keyManager(createPKCS12KeyManager());
+          break;
+        default:
+          throw new IllegalArgumentException("PKCS " + pkcs + " is not implemented");
+      }
     }
 
     return sslContextBuilder.build();

temporal-serviceclient/src/test/java/io/temporal/serviceclient/SimpleSslContextBuilderTest.java

@@ -19,26 +19,55 @@
 
 package io.temporal.serviceclient;
 
+import com.google.common.base.Preconditions;
 import java.io.IOException;
 import java.io.InputStream;
 import org.junit.Test;
 
 public class SimpleSslContextBuilderTest {
   @Test
   public void ableToLoadPKCS12Key() throws IOException {
-    try (InputStream in = SimpleSslContextBuilderTest.class.getResourceAsStream("pkcs12-key.pfx")) {
+    try (InputStream in =
+        SimpleSslContextBuilderTest.class.getClassLoader().getResourceAsStream("pkcs12-key.pfx")) {
+      Preconditions.checkState(in != null);
       SimpleSslContextBuilder builder = SimpleSslContextBuilder.forPKCS12(in);
       builder.build();
     }
   }
 
+  // to give easier API for configuration to users we allow null inputs
+  @Test
+  public void nullInputIsAcceptedForPKCS12Key() throws IOException {
+    SimpleSslContextBuilder builder = SimpleSslContextBuilder.forPKCS12(null);
+    builder.build();
+  }
+
   @Test
   public void ableToLoadPKCS8Key() throws IOException {
-    try (InputStream pkIn = SimpleSslContextBuilderTest.class.getResourceAsStream("pkcs8-pk.pem");
+    try (InputStream pkIn =
+            SimpleSslContextBuilderTest.class.getClassLoader().getResourceAsStream("pkcs8-pk.pem");
         InputStream crtIn =
-            SimpleSslContextBuilderTest.class.getResourceAsStream("pkcs8-crt-chain.pem")) {
+            SimpleSslContextBuilderTest.class
+                .getClassLoader()
+                .getResourceAsStream("pkcs8-crt-chain.pem")) {
+      Preconditions.checkState(pkIn != null);
+      Preconditions.checkState(crtIn != null);
+
       SimpleSslContextBuilder builder = SimpleSslContextBuilder.forPKCS8(crtIn, pkIn);
       builder.build();
     }
   }
+
+  // to give easier API for configuration to users we allow null inputs
+  @Test
+  public void nullInputIsAcceptedForPKCS8Key() throws IOException {
+    SimpleSslContextBuilder builder = SimpleSslContextBuilder.forPKCS8(null, null);
+    builder.build();
+  }
+
+  @Test
+  public void ableToCreateWithoutKeyOrCerts() throws IOException {
+    SimpleSslContextBuilder builder = SimpleSslContextBuilder.noKeyOrCertChain();
+    builder.build();
+  }
 }