Skip to content

Commit 7699297

Browse files
padilomaciejwalkowiakMatejNedic
authored
Relax prefix validation rules in Parameter Store and Secrets Manager (#364)
#302 backported to 2.3.x Co-authored-by: Maciej Walkowiak <[email protected]> Co-authored-by: MatejNedic <[email protected]>
1 parent 218a4d1 commit 7699297

File tree

4 files changed

+65
-53
lines changed

4 files changed

+65
-53
lines changed

‎spring-cloud-aws-parameter-store-config/src/main/java/io/awspring/cloud/paramstore/AwsParamStoreProperties.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public class AwsParamStoreProperties implements InitializingBean {
4242
/**
4343
* Pattern used for prefix validation.
4444
*/
45-
private static final Pattern PREFIX_PATTERN = Pattern.compile("(/)?([a-zA-Z0-9.\\-]+)(?:/[a-zA-Z0-9]+)*");
45+
private static final Pattern PREFIX_PATTERN = Pattern.compile("[a-zA-Z0-9.\\-/]+");
4646

4747
/**
4848
* Pattern used for profileSeparator validation.
@@ -96,11 +96,11 @@ public void afterPropertiesSet() throws Exception {
9696

9797
if (StringUtils.hasLength(prefix) && !PREFIX_PATTERN.matcher(prefix).matches()) {
9898
throw new ValidationException(CONFIG_PREFIX + ".prefix",
99-
"The prefix must have pattern of: " + PREFIX_PATTERN.toString());
99+
"The prefix value: " + prefix + " must have pattern of: " + PREFIX_PATTERN.toString());
100100
}
101101
if (!PROFILE_SEPARATOR_PATTERN.matcher(profileSeparator).matches()) {
102102
throw new ValidationException(CONFIG_PREFIX + ".profileSeparator",
103-
"The profileSeparator must have pattern of: " + PROFILE_SEPARATOR_PATTERN.toString());
103+
"The profileSeparator must have pattern of: " + PROFILE_SEPARATOR_PATTERN.toString());
104104
}
105105
}
106106

‎spring-cloud-aws-parameter-store-config/src/test/java/io/awspring/cloud/paramstore/AwsParamStorePropertiesTest.java‎

Lines changed: 35 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
import org.junit.jupiter.params.provider.Arguments;
2424
import org.junit.jupiter.params.provider.MethodSource;
2525

26+
import static org.assertj.core.api.Assertions.assertThat;
2627
import static org.assertj.core.api.Assertions.assertThatNoException;
2728
import static org.assertj.core.api.Assertions.assertThatThrownBy;
29+
import static org.assertj.core.api.Assertions.fail;
2830

2931
/**
3032
* Tests for {@link AwsParamStoreProperties}.
@@ -36,57 +38,51 @@ class AwsParamStorePropertiesTest {
3638

3739
@ParameterizedTest
3840
@MethodSource("invalidProperties")
39-
public void validationFails(AwsParamStoreProperties properties, String field, String errorCode) {
40-
assertThatThrownBy(properties::afterPropertiesSet).isInstanceOf(ValidationException.class);
41-
}
42-
43-
@Test
44-
void validationSucceeds() {
45-
AwsParamStoreProperties properties = new AwsParamStorePropertiesBuilder().withPrefix("/con")
46-
.withDefaultContext("app").withProfileSeparator("_").build();
47-
48-
assertThatNoException().isThrownBy(properties::afterPropertiesSet);
49-
}
50-
51-
@Test
52-
void validationSucceedsPrefix() {
53-
AwsParamStoreProperties properties = new AwsParamStorePropertiesBuilder().withPrefix("/con/test/bla")
54-
.withDefaultContext("app").withProfileSeparator("_").build();
55-
56-
assertThatNoException().isThrownBy(properties::afterPropertiesSet);
41+
public void validationFails(AwsParamStoreProperties properties, String field) throws Exception {
42+
try {
43+
properties.afterPropertiesSet();
44+
fail("validation should fail");
45+
}
46+
catch (ValidationException e) {
47+
assertThat(e.getField()).endsWith(field);
48+
}
5749
}
5850

59-
@Test
60-
void validationSucceedsNoPrefix() {
61-
AwsParamStoreProperties properties = new AwsParamStorePropertiesBuilder().withPrefix("")
62-
.withDefaultContext("app").withProfileSeparator("_").build();
63-
51+
@ParameterizedTest
52+
@MethodSource("validProperties")
53+
void validationSucceeds(AwsParamStoreProperties properties) {
6454
assertThatNoException().isThrownBy(properties::afterPropertiesSet);
6555
}
6656

6757
@Test
68-
void acceptsForwardSlashAsProfileSeparator() {
69-
AwsParamStoreProperties properties = new AwsParamStoreProperties();
70-
properties.setProfileSeparator("/");
71-
assertThatNoException().isThrownBy(properties::afterPropertiesSet);
58+
void checkExceptionLoggingForPrefix() {
59+
AwsParamStoreProperties properties = new AwsParamStorePropertiesBuilder().withPrefix("!.").build();
60+
assertThatThrownBy(properties::afterPropertiesSet)
61+
.hasMessage("The prefix value: !. must have pattern of: [a-zA-Z0-9.\\-/]+");
7262
}
7363

74-
@Test
75-
void acceptsBackslashAsProfileSeparator() {
76-
AwsParamStoreProperties properties = new AwsParamStoreProperties();
77-
properties.setProfileSeparator("\\");
78-
assertThatNoException().isThrownBy(properties::afterPropertiesSet);
64+
private static Stream<Arguments> validProperties() {
65+
return Stream.of(
66+
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("/con").withDefaultContext("app")
67+
.withProfileSeparator("_").build()),
68+
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("/config/someRandomValue-dev01")
69+
.withDefaultContext("app").withProfileSeparator("_").build()),
70+
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("/con/test/bla").withDefaultContext("app")
71+
.withProfileSeparator("_").build()),
72+
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("").withDefaultContext("app")
73+
.withProfileSeparator("_").build()),
74+
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("/config").withDefaultContext("app")
75+
.withProfileSeparator("/").build()),
76+
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("/config").withDefaultContext("app")
77+
.withProfileSeparator("\\").build()));
7978
}
8079

8180
private static Stream<Arguments> invalidProperties() {
82-
return Stream.of(
83-
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("!.").build(), "prefix", "Pattern"),
84-
Arguments.of(new AwsParamStorePropertiesBuilder().withDefaultContext("").build(), "defaultContext",
85-
"NotEmpty"),
86-
Arguments.of(new AwsParamStorePropertiesBuilder().withProfileSeparator("").build(), "profileSeparator",
87-
"NotEmpty"),
81+
return Stream.of(Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("!.").build(), "prefix"),
82+
Arguments.of(new AwsParamStorePropertiesBuilder().withDefaultContext("").build(), "defaultContext"),
83+
Arguments.of(new AwsParamStorePropertiesBuilder().withProfileSeparator("").build(), "profileSeparator"),
8884
Arguments.of(new AwsParamStorePropertiesBuilder().withProfileSeparator("!_").build(),
89-
"profileSeparator", "Pattern"));
85+
"profileSeparator"));
9086
}
9187

9288
private static class AwsParamStorePropertiesBuilder {

‎spring-cloud-aws-secrets-manager-config/src/main/java/io/awspring/cloud/secretsmanager/AwsSecretsManagerProperties.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public class AwsSecretsManagerProperties implements InitializingBean {
4343
/**
4444
* Pattern used for prefix validation.
4545
*/
46-
private static final Pattern PREFIX_PATTERN = Pattern.compile("(/)?([a-zA-Z0-9.\\-]+)(?:/[a-zA-Z0-9]+)*");
46+
private static final Pattern PREFIX_PATTERN = Pattern.compile("[a-zA-Z0-9.\\-/]+");
4747

4848
/**
4949
* Pattern used for profileSeparator validation.
@@ -98,11 +98,11 @@ public void afterPropertiesSet() throws Exception {
9898

9999
if (StringUtils.hasLength(prefix) && !PREFIX_PATTERN.matcher(prefix).matches()) {
100100
throw new ValidationException(CONFIG_PREFIX + ".prefix",
101-
"The prefix must have pattern of: " + PREFIX_PATTERN.toString());
101+
"The prefix value: " + prefix + " must have pattern of: " + PREFIX_PATTERN.toString());
102102
}
103103
if (!PROFILE_SEPARATOR_PATTERN.matcher(profileSeparator).matches()) {
104104
throw new ValidationException(CONFIG_PREFIX + ".profileSeparator",
105-
"The profileSeparator must have pattern of: " + PROFILE_SEPARATOR_PATTERN.toString());
105+
"The profileSeparator must have pattern of: " + PROFILE_SEPARATOR_PATTERN.toString());
106106
}
107107

108108
}

‎spring-cloud-aws-secrets-manager-config/src/test/java/io/awspring/cloud/secretsmanager/AwsSecretsManagerPropertiesTest.java‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,15 @@
1818

1919
import java.util.stream.Stream;
2020

21+
import org.junit.jupiter.api.Test;
2122
import org.junit.jupiter.params.ParameterizedTest;
2223
import org.junit.jupiter.params.provider.Arguments;
2324
import org.junit.jupiter.params.provider.MethodSource;
2425

26+
import static org.assertj.core.api.Assertions.assertThat;
2527
import static org.assertj.core.api.Assertions.assertThatNoException;
2628
import static org.assertj.core.api.Assertions.assertThatThrownBy;
29+
import static org.assertj.core.api.Assertions.fail;
2730

2831
/**
2932
* Tests for {@link AwsSecretsManagerProperties}.
@@ -35,8 +38,14 @@ class AwsSecretsManagerPropertiesTest {
3538

3639
@ParameterizedTest
3740
@MethodSource("invalidProperties")
38-
public void validationFails(AwsSecretsManagerProperties properties, String field, String errorCode) {
39-
assertThatThrownBy(properties::afterPropertiesSet).isInstanceOf(ValidationException.class);
41+
public void validationFails(AwsSecretsManagerProperties properties, String field) throws Exception {
42+
try {
43+
properties.afterPropertiesSet();
44+
fail("validation should fail");
45+
}
46+
catch (ValidationException e) {
47+
assertThat(e.getField()).endsWith(field);
48+
}
4049
}
4150

4251
@ParameterizedTest
@@ -45,10 +54,19 @@ void validationSucceeds(AwsSecretsManagerProperties properties) {
4554
assertThatNoException().isThrownBy(properties::afterPropertiesSet);
4655
}
4756

57+
@Test
58+
void checkExceptionLoggingForPrefix() {
59+
AwsSecretsManagerProperties properties = new AwsSecretsManagerPropertiesBuilder().withPrefix("!.").build();
60+
assertThatThrownBy(properties::afterPropertiesSet)
61+
.hasMessage("The prefix value: !. must have pattern of: [a-zA-Z0-9.\\-/]+");
62+
}
63+
4864
private static Stream<Arguments> validProperties() {
4965
return Stream.of(
5066
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withPrefix("").withDefaultContext("app")
5167
.withProfileSeparator("_").build()),
68+
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withPrefix("/secret/someRandomValue-dev01")
69+
.withDefaultContext("app").withProfileSeparator("_").build()),
5270
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withPrefix("/sec").withDefaultContext("app")
5371
.withProfileSeparator("_").build()),
5472
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withPrefix("/sec/test/var")
@@ -62,14 +80,12 @@ private static Stream<Arguments> validProperties() {
6280
}
6381

6482
private static Stream<Arguments> invalidProperties() {
65-
return Stream.of(
66-
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withPrefix("!.").build(), "prefix", "Pattern"),
67-
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withDefaultContext("").build(), "defaultContext",
68-
"NotEmpty"),
83+
return Stream.of(Arguments.of(new AwsSecretsManagerPropertiesBuilder().withPrefix("!.").build(), "prefix"),
84+
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withDefaultContext("").build(), "defaultContext"),
6985
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withProfileSeparator("").build(),
70-
"profileSeparator", "NotEmpty"),
86+
"profileSeparator"),
7187
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withProfileSeparator("!_").build(),
72-
"profileSeparator", "Pattern"));
88+
"profileSeparator"));
7389
}
7490

7591
private static class AwsSecretsManagerPropertiesBuilder {

0 commit comments

Comments
 (0)