diff --git a/client/src/main/java/com/scalar/dl/client/config/ClientConfig.java b/client/src/main/java/com/scalar/dl/client/config/ClientConfig.java index 875db0aa..c1e41fc8 100644 --- a/client/src/main/java/com/scalar/dl/client/config/ClientConfig.java +++ b/client/src/main/java/com/scalar/dl/client/config/ClientConfig.java @@ -405,10 +405,8 @@ private void load() { ENTITY_ID, CERT_HOLDER_ID)); // 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); diff --git a/client/src/main/java/com/scalar/dl/client/config/DigitalSignatureIdentityConfig.java b/client/src/main/java/com/scalar/dl/client/config/DigitalSignatureIdentityConfig.java index 6c6da032..4de9c7de 100644 --- a/client/src/main/java/com/scalar/dl/client/config/DigitalSignatureIdentityConfig.java +++ b/client/src/main/java/com/scalar/dl/client/config/DigitalSignatureIdentityConfig.java @@ -81,8 +81,7 @@ public DigitalSignatureIdentityConfig.Builder entityId(String entityId) { public DigitalSignatureIdentityConfig.Builder certVersion(int certVersion) { checkArgument( - certVersion >= 0, - CommonError.CERT_VERSION_MUST_BE_GREATER_THAN_OR_EQUAL_TO_ZERO.buildMessage()); + certVersion > 0, CommonError.CERT_VERSION_MUST_BE_GREATER_THAN_ZERO.buildMessage()); this.certVersion = certVersion; return this; } diff --git a/client/src/main/java/com/scalar/dl/client/config/HmacIdentityConfig.java b/client/src/main/java/com/scalar/dl/client/config/HmacIdentityConfig.java index 92b4a5e9..57d7272c 100644 --- a/client/src/main/java/com/scalar/dl/client/config/HmacIdentityConfig.java +++ b/client/src/main/java/com/scalar/dl/client/config/HmacIdentityConfig.java @@ -72,8 +72,8 @@ public HmacIdentityConfig.Builder entityId(String entityId) { public HmacIdentityConfig.Builder secretKeyVersion(int secretKeyVersion) { checkArgument( - secretKeyVersion >= 0, - CommonError.SECRET_VERSION_MUST_BE_GREATER_THAN_OR_EQUAL_TO_ZERO.buildMessage()); + secretKeyVersion > 0, + CommonError.SECRET_VERSION_MUST_BE_GREATER_THAN_ZERO.buildMessage()); this.secretKeyVersion = secretKeyVersion; return this; } diff --git a/client/src/test/java/com/scalar/dl/client/config/ClientConfigTest.java b/client/src/test/java/com/scalar/dl/client/config/ClientConfigTest.java index a3784d54..b822f8bb 100644 --- a/client/src/test/java/com/scalar/dl/client/config/ClientConfigTest.java +++ b/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); + } } diff --git a/common/src/main/java/com/scalar/dl/ledger/error/CommonError.java b/common/src/main/java/com/scalar/dl/ledger/error/CommonError.java index 4c3d40f6..f015bc24 100644 --- a/common/src/main/java/com/scalar/dl/ledger/error/CommonError.java +++ b/common/src/main/java/com/scalar/dl/ledger/error/CommonError.java @@ -163,10 +163,10 @@ public enum CommonError implements ScalarDlError { ""), PRIVATE_KEY_AND_CERT_REQUIRED( StatusCode.INVALID_ARGUMENT, "009", "The private key and certificate are required.", "", ""), - CERT_VERSION_MUST_BE_GREATER_THAN_OR_EQUAL_TO_ZERO( + CERT_VERSION_MUST_BE_GREATER_THAN_ZERO( StatusCode.INVALID_ARGUMENT, "010", - "The certificate version must be greater than or equal to zero.", + "The certificate version must be greater than zero.", "", ""), SECRET_KEY_REQUIRED( @@ -175,10 +175,10 @@ public enum CommonError implements ScalarDlError { "A secret key is required for HMAC authentication.", "", ""), - SECRET_VERSION_MUST_BE_GREATER_THAN_OR_EQUAL_TO_ZERO( + SECRET_VERSION_MUST_BE_GREATER_THAN_ZERO( StatusCode.INVALID_ARGUMENT, "012", - "The secret version for HMAC authentication must be greater than or equal to zero.", + "The secret version for HMAC authentication must be greater than zero.", "", ""), GRPC_DEADLINE_DURATION_MUST_BE_GREATER_THAN_OR_EQUAL_TO_ZERO(