Fix certificate and secret key version check and messages (#202)

Author: jnmt

client/src/main/java/com/scalar/dl/client/config/ClientConfig.java

@@ -403,10 +403,8 @@ private void load() {
           ENTITY_ID + " or " + CERT_HOLDER_ID + " are missing but either is required.");
 
       // identity based on digital signature
-      certVersion = ConfigUtils.getInt(props, DS_CERT_VERSION, 0);
-      if (certVersion <= 0) {
-        certVersion = ConfigUtils.getInt(props, CERT_VERSION, DEFAULT_CERT_VERSION);
-      }
+      certVersion = ConfigUtils.getInt(props, CERT_VERSION, DEFAULT_CERT_VERSION);
+      certVersion = ConfigUtils.getInt(props, DS_CERT_VERSION, certVersion);
       cert = ConfigUtils.getString(props, DS_CERT_PEM, null);
       if (cert == null) {
         cert = ConfigUtils.getStringFromFilePath(props, DS_CERT_PATH, null);

client/src/main/java/com/scalar/dl/client/config/DigitalSignatureIdentityConfig.java

@@ -79,7 +79,7 @@ public DigitalSignatureIdentityConfig.Builder entityId(String entityId) {
     }
 
     public DigitalSignatureIdentityConfig.Builder certVersion(int certVersion) {
-      checkArgument(certVersion >= 0);
+      checkArgument(certVersion > 0);
       this.certVersion = certVersion;
       return this;
     }

client/src/main/java/com/scalar/dl/client/config/HmacIdentityConfig.java

@@ -70,7 +70,7 @@ public HmacIdentityConfig.Builder entityId(String entityId) {
     }
 
     public HmacIdentityConfig.Builder secretKeyVersion(int secretKeyVersion) {
-      checkArgument(secretKeyVersion >= 0);
+      checkArgument(secretKeyVersion > 0);
       this.secretKeyVersion = secretKeyVersion;
       return this;
     }

client/src/test/java/com/scalar/dl/client/config/ClientConfigTest.java

@@ -589,10 +589,12 @@ public void constructor_HmacAuthEnabledButNoSecretKeyGiven_ShouldCreateConfigPro
           throws IOException {
     // Arrange
     Properties props = new Properties();
-    props.put(ClientConfig.ENTITY_ID, SOME_ENTITY_ID);
+    props.put(ClientConfig.CERT_VERSION, "0");
     props.put(ClientConfig.CERT_HOLDER_ID, SOME_CERT_HOLDER_ID);
     props.put(ClientConfig.CERT_PEM, SOME_CERT_PEM);
     props.put(ClientConfig.PRIVATE_KEY_PEM, SOME_PRIVATE_KEY_PEM);
+    props.put(ClientConfig.ENTITY_ID, SOME_ENTITY_ID);
+    props.put(ClientConfig.DS_CERT_VERSION, SOME_CERT_VERSION);
     props.put(ClientConfig.DS_CERT_PEM, SOME_CERT_PEM + "New");
     props.put(ClientConfig.DS_PRIVATE_KEY_PEM, SOME_PRIVATE_KEY_PEM + "New");
 
@@ -601,6 +603,8 @@ public void constructor_HmacAuthEnabledButNoSecretKeyGiven_ShouldCreateConfigPro
 
     // Assert
     assertThat(config.getDigitalSignatureIdentityConfig().getEntityId()).isEqualTo(SOME_ENTITY_ID);
+    assertThat(config.getDigitalSignatureIdentityConfig().getCertVersion())
+        .isEqualTo(Integer.parseInt(SOME_CERT_VERSION));
     assertThat(config.getDigitalSignatureIdentityConfig().getCert())
         .isEqualTo(SOME_CERT_PEM + "New");
     assertThat(config.getDigitalSignatureIdentityConfig().getPrivateKey())
@@ -740,4 +744,59 @@ public void constructor_DifferentClientConfigGiven_ShouldReturnNonEqualConfigs()
     assertThat(config1.getLedgerTargetConfig()).isNotEqualTo(config2.getLedgerTargetConfig());
     assertThat(config1.getAuditorTargetConfig()).isNotEqualTo(config2.getAuditorTargetConfig());
   }
+
+  @Test
+  public void constructor_CertVersionZeroGiven_ShouldThrowIllegalArgumentException() {
+    // Arrange
+    Properties props = new Properties();
+    props.put(ClientConfig.ENTITY_ID, SOME_CERT_HOLDER_ID);
+    props.put(ClientConfig.DS_CERT_VERSION, "0");
+    props.put(ClientConfig.DS_CERT_PEM, SOME_CERT_PEM);
+    props.put(ClientConfig.DS_PRIVATE_KEY_PEM, SOME_PRIVATE_KEY_PEM);
+
+    // Act Assert
+    assertThatThrownBy(() -> new ClientConfig(props)).isInstanceOf(IllegalArgumentException.class);
+  }
+
+  @Test
+  public void constructor_DeprecatedCertVersionZeroGiven_ShouldThrowIllegalArgumentException() {
+    // Arrange
+    Properties props = new Properties();
+    props.put(ClientConfig.CERT_HOLDER_ID, SOME_CERT_HOLDER_ID);
+    props.put(ClientConfig.CERT_VERSION, "0");
+    props.put(ClientConfig.CERT_PEM, SOME_CERT_PEM);
+    props.put(ClientConfig.PRIVATE_KEY_PEM, SOME_PRIVATE_KEY_PEM);
+
+    // Act Assert
+    assertThatThrownBy(() -> new ClientConfig(props)).isInstanceOf(IllegalArgumentException.class);
+  }
+
+  @Test
+  public void
+      constructor_NewWrongCertVersionAndDeprecatedCorrectCertVersionGiven_ShouldThrowIllegalArgumentException() {
+    // Arrange
+    Properties props = new Properties();
+    props.put(ClientConfig.ENTITY_ID, SOME_CERT_HOLDER_ID);
+    props.put(ClientConfig.DS_CERT_VERSION, "0");
+    props.put(ClientConfig.DS_CERT_PEM, SOME_CERT_PEM);
+    props.put(ClientConfig.DS_PRIVATE_KEY_PEM, SOME_PRIVATE_KEY_PEM);
+    props.put(ClientConfig.CERT_HOLDER_ID, SOME_CERT_HOLDER_ID);
+    props.put(ClientConfig.CERT_VERSION, SOME_CERT_VERSION);
+
+    // Act Assert
+    assertThatThrownBy(() -> new ClientConfig(props)).isInstanceOf(IllegalArgumentException.class);
+  }
+
+  @Test
+  public void constructor_SecretVersionZeroGiven_ShouldThrowIllegalArgumentException() {
+    // Arrange
+    Properties props = new Properties();
+    props.put(ClientConfig.ENTITY_ID, SOME_ENTITY_ID);
+    props.put(ClientConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod());
+    props.put(ClientConfig.HMAC_SECRET_KEY, SOME_SECRET_KEY);
+    props.put(ClientConfig.HMAC_SECRET_KEY_VERSION, "0");
+
+    // Act Assert
+    assertThatThrownBy(() -> new ClientConfig(props)).isInstanceOf(IllegalArgumentException.class);
+  }
 }