Skip to content

Commit cce9212

Browse files
jnmtjnmt
authored
Fix certificate and secret key version check and messages (#202)
1 parent 41728ca commit cce9212

File tree

5 files changed

+69
-13
lines changed

5 files changed

+69
-13
lines changed

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -405,10 +405,8 @@ private void load() {
405405
ENTITY_ID, CERT_HOLDER_ID));
406406

407407
// identity based on digital signature
408-
certVersion = ConfigUtils.getInt(props, DS_CERT_VERSION, 0);
409-
if (certVersion <= 0) {
410-
certVersion = ConfigUtils.getInt(props, CERT_VERSION, DEFAULT_CERT_VERSION);
411-
}
408+
certVersion = ConfigUtils.getInt(props, CERT_VERSION, DEFAULT_CERT_VERSION);
409+
certVersion = ConfigUtils.getInt(props, DS_CERT_VERSION, certVersion);
412410
cert = ConfigUtils.getString(props, DS_CERT_PEM, null);
413411
if (cert == null) {
414412
cert = ConfigUtils.getStringFromFilePath(props, DS_CERT_PATH, null);

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,7 @@ public DigitalSignatureIdentityConfig.Builder entityId(String entityId) {
8181

8282
public DigitalSignatureIdentityConfig.Builder certVersion(int certVersion) {
8383
checkArgument(
84-
certVersion >= 0,
85-
CommonError.CERT_VERSION_MUST_BE_GREATER_THAN_OR_EQUAL_TO_ZERO.buildMessage());
84+
certVersion > 0, CommonError.CERT_VERSION_MUST_BE_GREATER_THAN_ZERO.buildMessage());
8685
this.certVersion = certVersion;
8786
return this;
8887
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ public HmacIdentityConfig.Builder entityId(String entityId) {
7272

7373
public HmacIdentityConfig.Builder secretKeyVersion(int secretKeyVersion) {
7474
checkArgument(
75-
secretKeyVersion >= 0,
76-
CommonError.SECRET_VERSION_MUST_BE_GREATER_THAN_OR_EQUAL_TO_ZERO.buildMessage());
75+
secretKeyVersion > 0,
76+
CommonError.SECRET_VERSION_MUST_BE_GREATER_THAN_ZERO.buildMessage());
7777
this.secretKeyVersion = secretKeyVersion;
7878
return this;
7979
}

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

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -589,10 +589,12 @@ public void constructor_HmacAuthEnabledButNoSecretKeyGiven_ShouldCreateConfigPro
589589
throws IOException {
590590
// Arrange
591591
Properties props = new Properties();
592-
props.put(ClientConfig.ENTITY_ID, SOME_ENTITY_ID);
592+
props.put(ClientConfig.CERT_VERSION, "0");
593593
props.put(ClientConfig.CERT_HOLDER_ID, SOME_CERT_HOLDER_ID);
594594
props.put(ClientConfig.CERT_PEM, SOME_CERT_PEM);
595595
props.put(ClientConfig.PRIVATE_KEY_PEM, SOME_PRIVATE_KEY_PEM);
596+
props.put(ClientConfig.ENTITY_ID, SOME_ENTITY_ID);
597+
props.put(ClientConfig.DS_CERT_VERSION, SOME_CERT_VERSION);
596598
props.put(ClientConfig.DS_CERT_PEM, SOME_CERT_PEM + "New");
597599
props.put(ClientConfig.DS_PRIVATE_KEY_PEM, SOME_PRIVATE_KEY_PEM + "New");
598600

@@ -601,6 +603,8 @@ public void constructor_HmacAuthEnabledButNoSecretKeyGiven_ShouldCreateConfigPro
601603

602604
// Assert
603605
assertThat(config.getDigitalSignatureIdentityConfig().getEntityId()).isEqualTo(SOME_ENTITY_ID);
606+
assertThat(config.getDigitalSignatureIdentityConfig().getCertVersion())
607+
.isEqualTo(Integer.parseInt(SOME_CERT_VERSION));
604608
assertThat(config.getDigitalSignatureIdentityConfig().getCert())
605609
.isEqualTo(SOME_CERT_PEM + "New");
606610
assertThat(config.getDigitalSignatureIdentityConfig().getPrivateKey())
@@ -740,4 +744,59 @@ public void constructor_DifferentClientConfigGiven_ShouldReturnNonEqualConfigs()
740744
assertThat(config1.getLedgerTargetConfig()).isNotEqualTo(config2.getLedgerTargetConfig());
741745
assertThat(config1.getAuditorTargetConfig()).isNotEqualTo(config2.getAuditorTargetConfig());
742746
}
747+
748+
@Test
749+
public void constructor_CertVersionZeroGiven_ShouldThrowIllegalArgumentException() {
750+
// Arrange
751+
Properties props = new Properties();
752+
props.put(ClientConfig.ENTITY_ID, SOME_CERT_HOLDER_ID);
753+
props.put(ClientConfig.DS_CERT_VERSION, "0");
754+
props.put(ClientConfig.DS_CERT_PEM, SOME_CERT_PEM);
755+
props.put(ClientConfig.DS_PRIVATE_KEY_PEM, SOME_PRIVATE_KEY_PEM);
756+
757+
// Act Assert
758+
assertThatThrownBy(() -> new ClientConfig(props)).isInstanceOf(IllegalArgumentException.class);
759+
}
760+
761+
@Test
762+
public void constructor_DeprecatedCertVersionZeroGiven_ShouldThrowIllegalArgumentException() {
763+
// Arrange
764+
Properties props = new Properties();
765+
props.put(ClientConfig.CERT_HOLDER_ID, SOME_CERT_HOLDER_ID);
766+
props.put(ClientConfig.CERT_VERSION, "0");
767+
props.put(ClientConfig.CERT_PEM, SOME_CERT_PEM);
768+
props.put(ClientConfig.PRIVATE_KEY_PEM, SOME_PRIVATE_KEY_PEM);
769+
770+
// Act Assert
771+
assertThatThrownBy(() -> new ClientConfig(props)).isInstanceOf(IllegalArgumentException.class);
772+
}
773+
774+
@Test
775+
public void
776+
constructor_NewWrongCertVersionAndDeprecatedCorrectCertVersionGiven_ShouldThrowIllegalArgumentException() {
777+
// Arrange
778+
Properties props = new Properties();
779+
props.put(ClientConfig.ENTITY_ID, SOME_CERT_HOLDER_ID);
780+
props.put(ClientConfig.DS_CERT_VERSION, "0");
781+
props.put(ClientConfig.DS_CERT_PEM, SOME_CERT_PEM);
782+
props.put(ClientConfig.DS_PRIVATE_KEY_PEM, SOME_PRIVATE_KEY_PEM);
783+
props.put(ClientConfig.CERT_HOLDER_ID, SOME_CERT_HOLDER_ID);
784+
props.put(ClientConfig.CERT_VERSION, SOME_CERT_VERSION);
785+
786+
// Act Assert
787+
assertThatThrownBy(() -> new ClientConfig(props)).isInstanceOf(IllegalArgumentException.class);
788+
}
789+
790+
@Test
791+
public void constructor_SecretVersionZeroGiven_ShouldThrowIllegalArgumentException() {
792+
// Arrange
793+
Properties props = new Properties();
794+
props.put(ClientConfig.ENTITY_ID, SOME_ENTITY_ID);
795+
props.put(ClientConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod());
796+
props.put(ClientConfig.HMAC_SECRET_KEY, SOME_SECRET_KEY);
797+
props.put(ClientConfig.HMAC_SECRET_KEY_VERSION, "0");
798+
799+
// Act Assert
800+
assertThatThrownBy(() -> new ClientConfig(props)).isInstanceOf(IllegalArgumentException.class);
801+
}
743802
}

‎common/src/main/java/com/scalar/dl/ledger/error/CommonError.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,10 @@ public enum CommonError implements ScalarDlError {
163163
""),
164164
PRIVATE_KEY_AND_CERT_REQUIRED(
165165
StatusCode.INVALID_ARGUMENT, "009", "The private key and certificate are required.", "", ""),
166-
CERT_VERSION_MUST_BE_GREATER_THAN_OR_EQUAL_TO_ZERO(
166+
CERT_VERSION_MUST_BE_GREATER_THAN_ZERO(
167167
StatusCode.INVALID_ARGUMENT,
168168
"010",
169-
"The certificate version must be greater than or equal to zero.",
169+
"The certificate version must be greater than zero.",
170170
"",
171171
""),
172172
SECRET_KEY_REQUIRED(
@@ -175,10 +175,10 @@ public enum CommonError implements ScalarDlError {
175175
"A secret key is required for HMAC authentication.",
176176
"",
177177
""),
178-
SECRET_VERSION_MUST_BE_GREATER_THAN_OR_EQUAL_TO_ZERO(
178+
SECRET_VERSION_MUST_BE_GREATER_THAN_ZERO(
179179
StatusCode.INVALID_ARGUMENT,
180180
"012",
181-
"The secret version for HMAC authentication must be greater than or equal to zero.",
181+
"The secret version for HMAC authentication must be greater than zero.",
182182
"",
183183
""),
184184
GRPC_DEADLINE_DURATION_MUST_BE_GREATER_THAN_OR_EQUAL_TO_ZERO(

0 commit comments

Comments
 (0)