Skip to content

Relax prefix validation rules in Parameter Store and Secrets Manager … #364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 3, 2022
Merged

Relax prefix validation rules in Parameter Store and Secrets Manager … #364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...eter-store-config/src/main/java/io/awspring/cloud/paramstore/AwsParamStoreProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class AwsParamStoreProperties implements InitializingBean {
/**
* Pattern used for prefix validation.
*/
private static final Pattern PREFIX_PATTERN = Pattern.compile("(/)?([a-zA-Z0-9.\\-]+)(?:/[a-zA-Z0-9]+)*");
private static final Pattern PREFIX_PATTERN = Pattern.compile("[a-zA-Z0-9.\\-/]+");

/**
* Pattern used for profileSeparator validation.
Expand Down Expand Up @@ -96,11 +96,11 @@ public void afterPropertiesSet() throws Exception {

if (StringUtils.hasLength(prefix) && !PREFIX_PATTERN.matcher(prefix).matches()) {
throw new ValidationException(CONFIG_PREFIX + ".prefix",
"The prefix must have pattern of: " + PREFIX_PATTERN.toString());
"The prefix value: " + prefix + " must have pattern of: " + PREFIX_PATTERN.toString());
}
if (!PROFILE_SEPARATOR_PATTERN.matcher(profileSeparator).matches()) {
throw new ValidationException(CONFIG_PREFIX + ".profileSeparator",
"The profileSeparator must have pattern of: " + PROFILE_SEPARATOR_PATTERN.toString());
"The profileSeparator must have pattern of: " + PROFILE_SEPARATOR_PATTERN.toString());
}
}

Expand Down
74 changes: 35 additions & 39 deletions ...-store-config/src/test/java/io/awspring/cloud/paramstore/AwsParamStorePropertiesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.fail;

/**
* Tests for {@link AwsParamStoreProperties}.
Expand All @@ -36,57 +38,51 @@ class AwsParamStorePropertiesTest {

@ParameterizedTest
@MethodSource("invalidProperties")
public void validationFails(AwsParamStoreProperties properties, String field, String errorCode) {
assertThatThrownBy(properties::afterPropertiesSet).isInstanceOf(ValidationException.class);
}

@Test
void validationSucceeds() {
AwsParamStoreProperties properties = new AwsParamStorePropertiesBuilder().withPrefix("/con")
.withDefaultContext("app").withProfileSeparator("_").build();

assertThatNoException().isThrownBy(properties::afterPropertiesSet);
}

@Test
void validationSucceedsPrefix() {
AwsParamStoreProperties properties = new AwsParamStorePropertiesBuilder().withPrefix("/con/test/bla")
.withDefaultContext("app").withProfileSeparator("_").build();

assertThatNoException().isThrownBy(properties::afterPropertiesSet);
public void validationFails(AwsParamStoreProperties properties, String field) throws Exception {
try {
properties.afterPropertiesSet();
fail("validation should fail");
}
catch (ValidationException e) {
assertThat(e.getField()).endsWith(field);
}
}

@Test
void validationSucceedsNoPrefix() {
AwsParamStoreProperties properties = new AwsParamStorePropertiesBuilder().withPrefix("")
.withDefaultContext("app").withProfileSeparator("_").build();

@ParameterizedTest
@MethodSource("validProperties")
void validationSucceeds(AwsParamStoreProperties properties) {
assertThatNoException().isThrownBy(properties::afterPropertiesSet);
}

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

@Test
void acceptsBackslashAsProfileSeparator() {
AwsParamStoreProperties properties = new AwsParamStoreProperties();
properties.setProfileSeparator("\\");
assertThatNoException().isThrownBy(properties::afterPropertiesSet);
private static Stream<Arguments> validProperties() {
return Stream.of(
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("/con").withDefaultContext("app")
.withProfileSeparator("_").build()),
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("/config/someRandomValue-dev01")
.withDefaultContext("app").withProfileSeparator("_").build()),
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("/con/test/bla").withDefaultContext("app")
.withProfileSeparator("_").build()),
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("").withDefaultContext("app")
.withProfileSeparator("_").build()),
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("/config").withDefaultContext("app")
.withProfileSeparator("/").build()),
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("/config").withDefaultContext("app")
.withProfileSeparator("\\").build()));
}

private static Stream<Arguments> invalidProperties() {
return Stream.of(
Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("!.").build(), "prefix", "Pattern"),
Arguments.of(new AwsParamStorePropertiesBuilder().withDefaultContext("").build(), "defaultContext",
"NotEmpty"),
Arguments.of(new AwsParamStorePropertiesBuilder().withProfileSeparator("").build(), "profileSeparator",
"NotEmpty"),
return Stream.of(Arguments.of(new AwsParamStorePropertiesBuilder().withPrefix("!.").build(), "prefix"),
Arguments.of(new AwsParamStorePropertiesBuilder().withDefaultContext("").build(), "defaultContext"),
Arguments.of(new AwsParamStorePropertiesBuilder().withProfileSeparator("").build(), "profileSeparator"),
Arguments.of(new AwsParamStorePropertiesBuilder().withProfileSeparator("!_").build(),
"profileSeparator", "Pattern"));
"profileSeparator"));
}

private static class AwsParamStorePropertiesBuilder {
Expand Down
6 changes: 3 additions & 3 deletions ...er-config/src/main/java/io/awspring/cloud/secretsmanager/AwsSecretsManagerProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public class AwsSecretsManagerProperties implements InitializingBean {
/**
* Pattern used for prefix validation.
*/
private static final Pattern PREFIX_PATTERN = Pattern.compile("(/)?([a-zA-Z0-9.\\-]+)(?:/[a-zA-Z0-9]+)*");
private static final Pattern PREFIX_PATTERN = Pattern.compile("[a-zA-Z0-9.\\-/]+");

/**
* Pattern used for profileSeparator validation.
Expand Down Expand Up @@ -98,11 +98,11 @@ public void afterPropertiesSet() throws Exception {

if (StringUtils.hasLength(prefix) && !PREFIX_PATTERN.matcher(prefix).matches()) {
throw new ValidationException(CONFIG_PREFIX + ".prefix",
"The prefix must have pattern of: " + PREFIX_PATTERN.toString());
"The prefix value: " + prefix + " must have pattern of: " + PREFIX_PATTERN.toString());
}
if (!PROFILE_SEPARATOR_PATTERN.matcher(profileSeparator).matches()) {
throw new ValidationException(CONFIG_PREFIX + ".profileSeparator",
"The profileSeparator must have pattern of: " + PROFILE_SEPARATOR_PATTERN.toString());
"The profileSeparator must have pattern of: " + PROFILE_SEPARATOR_PATTERN.toString());
}

}
Expand Down
32 changes: 24 additions & 8 deletions ...onfig/src/test/java/io/awspring/cloud/secretsmanager/AwsSecretsManagerPropertiesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@

import java.util.stream.Stream;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.fail;

/**
* Tests for {@link AwsSecretsManagerProperties}.
Expand All @@ -35,8 +38,14 @@ class AwsSecretsManagerPropertiesTest {

@ParameterizedTest
@MethodSource("invalidProperties")
public void validationFails(AwsSecretsManagerProperties properties, String field, String errorCode) {
assertThatThrownBy(properties::afterPropertiesSet).isInstanceOf(ValidationException.class);
public void validationFails(AwsSecretsManagerProperties properties, String field) throws Exception {
try {
properties.afterPropertiesSet();
fail("validation should fail");
}
catch (ValidationException e) {
assertThat(e.getField()).endsWith(field);
}
}

@ParameterizedTest
Expand All @@ -45,10 +54,19 @@ void validationSucceeds(AwsSecretsManagerProperties properties) {
assertThatNoException().isThrownBy(properties::afterPropertiesSet);
}

@Test
void checkExceptionLoggingForPrefix() {
AwsSecretsManagerProperties properties = new AwsSecretsManagerPropertiesBuilder().withPrefix("!.").build();
assertThatThrownBy(properties::afterPropertiesSet)
.hasMessage("The prefix value: !. must have pattern of: [a-zA-Z0-9.\\-/]+");
}

private static Stream<Arguments> validProperties() {
return Stream.of(
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withPrefix("").withDefaultContext("app")
.withProfileSeparator("_").build()),
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withPrefix("/secret/someRandomValue-dev01")
.withDefaultContext("app").withProfileSeparator("_").build()),
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withPrefix("/sec").withDefaultContext("app")
.withProfileSeparator("_").build()),
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withPrefix("/sec/test/var")
Expand All @@ -62,14 +80,12 @@ private static Stream<Arguments> validProperties() {
}

private static Stream<Arguments> invalidProperties() {
return Stream.of(
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withPrefix("!.").build(), "prefix", "Pattern"),
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withDefaultContext("").build(), "defaultContext",
"NotEmpty"),
return Stream.of(Arguments.of(new AwsSecretsManagerPropertiesBuilder().withPrefix("!.").build(), "prefix"),
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withDefaultContext("").build(), "defaultContext"),
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withProfileSeparator("").build(),
"profileSeparator", "NotEmpty"),
"profileSeparator"),
Arguments.of(new AwsSecretsManagerPropertiesBuilder().withProfileSeparator("!_").build(),
"profileSeparator", "Pattern"));
"profileSeparator"));
}

private static class AwsSecretsManagerPropertiesBuilder {
Expand Down