Enforce validation constraints for dynamic configuration objects.

service/src/main/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManager.java

@@ -18,8 +18,12 @@ import org.slf4j.LoggerFactory;
 import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
 import org.whispersystems.textsecuregcm.util.Util;
 
+import javax.validation.ConstraintViolation;
+import javax.validation.Validation;
+import javax.validation.Validator;
 import java.nio.charset.StandardCharsets;
 import java.util.Optional;
+import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -32,16 +36,19 @@ public class DynamicConfigurationManager {
   private final AmazonAppConfig appConfigClient;
 
   private final AtomicReference<DynamicConfiguration>   configuration    = new AtomicReference<>();
-  private final Logger                                  logger           = LoggerFactory.getLogger(DynamicConfigurationManager.class);
 
   private GetConfigurationResult lastConfigResult;
 
   private boolean initialized = false;
 
-  public static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(new YAMLFactory())
+  private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(new YAMLFactory())
       .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
       .registerModule(new JavaTimeModule());
 
+  private static final Validator VALIDATOR = Validation.buildDefaultValidatorFactory().getValidator();
+
+  private static final Logger logger = LoggerFactory.getLogger(DynamicConfigurationManager.class);
+
   public DynamicConfigurationManager(String application, String environment, String configurationName) {
     this(AmazonAppConfigClient.builder()
                               .withClientConfiguration(new ClientConfiguration().withClientExecutionTimeout(10000).withRequestTimeout(10000))
@@ -104,7 +111,9 @@ public class DynamicConfigurationManager {
 
     if (!StringUtils.equals(lastConfigResult.getConfigurationVersion(), previousVersion)) {
       logger.info("Received new config version: {}", lastConfigResult.getConfigurationVersion());
-      maybeDynamicConfiguration = Optional.of(OBJECT_MAPPER.readValue(StandardCharsets.UTF_8.decode(lastConfigResult.getContent().asReadOnlyBuffer()).toString(), DynamicConfiguration.class));
+
+      maybeDynamicConfiguration =
+          parseConfiguration(StandardCharsets.UTF_8.decode(lastConfigResult.getContent().asReadOnlyBuffer()).toString());
     } else {
       // No change since last version
       maybeDynamicConfiguration = Optional.empty();
@@ -113,6 +122,23 @@ public class DynamicConfigurationManager {
     return maybeDynamicConfiguration;
   }
 
+  @VisibleForTesting
+  public static Optional<DynamicConfiguration> parseConfiguration(final String configurationYaml) throws JsonProcessingException {
+    final DynamicConfiguration configuration = OBJECT_MAPPER.readValue(configurationYaml, DynamicConfiguration.class);
+    final Set<ConstraintViolation<DynamicConfiguration>> violations = VALIDATOR.validate(configuration);
+
+    final Optional<DynamicConfiguration> maybeDynamicConfiguration;
+
+    if (violations.isEmpty()) {
+      maybeDynamicConfiguration = Optional.of(configuration);
+    } else {
+      logger.warn("Failed to validate configuration: {}", violations);
+      maybeDynamicConfiguration = Optional.empty();
+    }
+
+    return maybeDynamicConfiguration;
+  }
+
   private DynamicConfiguration retrieveInitialDynamicConfiguration() {
     for (;;) {
       try {

service/src/test/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfigurationTest.java

@@ -31,8 +31,8 @@ class DynamicConfigurationTest {
   void testParseExperimentConfig() throws JsonProcessingException {
     {
       final String emptyConfigYaml = "test: true";
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration emptyConfig =
-          .readValue(emptyConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(emptyConfigYaml).orElseThrow();
 
       assertFalse(emptyConfig.getExperimentEnrollmentConfiguration("test").isPresent());
     }
@@ -51,8 +51,8 @@ class DynamicConfigurationTest {
               "    enrolledUuids:\n" +
               "      - 71618739-114c-4b1f-bb0d-6478a44eb600";
 
-      final DynamicConfiguration config = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration config =
-          .readValue(experimentConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(experimentConfigYaml).orElseThrow();
 
       assertFalse(config.getExperimentEnrollmentConfiguration("unconfigured").isPresent());
 
@@ -79,8 +79,8 @@ class DynamicConfigurationTest {
   void testParsePreRegistrationExperiments() throws JsonProcessingException {
     {
       final String emptyConfigYaml = "test: true";
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration emptyConfig =
-          .readValue(emptyConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(emptyConfigYaml).orElseThrow();
 
       assertFalse(emptyConfig.getPreRegistrationEnrollmentConfiguration("test").isPresent());
     }
@@ -105,8 +105,8 @@ class DynamicConfigurationTest {
               "    excludedCountryCodes:\n" +
               "      - 47";
 
-      final DynamicConfiguration config = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration config =
-          .readValue(experimentConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(experimentConfigYaml).orElseThrow();
 
       assertFalse(config.getPreRegistrationEnrollmentConfiguration("unconfigured").isPresent());
 
@@ -152,14 +152,14 @@ class DynamicConfigurationTest {
   void testParseRemoteDeprecationConfig() throws JsonProcessingException {
     {
       final String emptyConfigYaml = "test: true";
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration emptyConfig =
-          .readValue(emptyConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(emptyConfigYaml).orElseThrow();
 
       assertNotNull(emptyConfig.getRemoteDeprecationConfiguration());
     }
 
     {
-      final String experimentConfigYaml =
+      final String remoteDeprecationConfig =
           "remoteDeprecation:\n" +
               "  minimumVersions:\n" +
               "    IOS: 1.2.3\n" +
@@ -172,8 +172,9 @@ class DynamicConfigurationTest {
               "    DESKTOP:\n" +
               "      - 1.4.0-beta.2";
 
-      final DynamicConfiguration config = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration config =
-          .readValue(experimentConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(remoteDeprecationConfig).orElseThrow();
+
       final DynamicRemoteDeprecationConfiguration remoteDeprecationConfiguration = config
           .getRemoteDeprecationConfiguration();
 
@@ -191,8 +192,8 @@ class DynamicConfigurationTest {
   void testParseMessageRateConfiguration() throws JsonProcessingException {
     {
       final String emptyConfigYaml = "test: true";
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration emptyConfig =
-          .readValue(emptyConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(emptyConfigYaml).orElseThrow();
 
       assertFalse(emptyConfig.getMessageRateConfiguration().isEnforceUnsealedSenderRateLimit());
     }
@@ -202,8 +203,8 @@ class DynamicConfigurationTest {
           "messageRate:\n" +
               "  enforceUnsealedSenderRateLimit: true";
 
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration emptyConfig =
-          .readValue(messageRateConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(messageRateConfigYaml).orElseThrow();
 
       assertTrue(emptyConfig.getMessageRateConfiguration().isEnforceUnsealedSenderRateLimit());
     }
@@ -213,19 +214,19 @@ class DynamicConfigurationTest {
   void testParseFeatureFlags() throws JsonProcessingException {
     {
       final String emptyConfigYaml = "test: true";
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration emptyConfig =
-          .readValue(emptyConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(emptyConfigYaml).orElseThrow();
 
       assertTrue(emptyConfig.getActiveFeatureFlags().isEmpty());
     }
 
     {
-      final String emptyConfigYaml =
+      final String featureFlagYaml =
           "featureFlags:\n"
               + "  - testFlag";
 
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration emptyConfig =
-          .readValue(emptyConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(featureFlagYaml).orElseThrow();
 
       assertTrue(emptyConfig.getActiveFeatureFlags().contains("testFlag"));
     }
@@ -235,22 +236,22 @@ class DynamicConfigurationTest {
   void testParseTwilioConfiguration() throws JsonProcessingException {
     {
       final String emptyConfigYaml = "test: true";
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration emptyConfig =
-          .readValue(emptyConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(emptyConfigYaml).orElseThrow();
 
       assertTrue(emptyConfig.getTwilioConfiguration().getNumbers().isEmpty());
     }
 
     {
-      final String emptyConfigYaml =
+      final String twilioConfigYaml =
           "twilio:\n"
               + "  numbers:\n"
               + "    - 2135551212\n"
               + "    - 2135551313";
 
-      final DynamicTwilioConfiguration config = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicTwilioConfiguration config =
-          .readValue(emptyConfigYaml, DynamicConfiguration.class)
+          DynamicConfigurationManager.parseConfiguration(twilioConfigYaml).orElseThrow()
-          .getTwilioConfiguration();
+              .getTwilioConfiguration();
 
       assertEquals(List.of("2135551212", "2135551313"), config.getNumbers());
     }
@@ -260,8 +261,8 @@ class DynamicConfigurationTest {
   void testParsePaymentsConfiguration() throws JsonProcessingException {
     {
       final String emptyConfigYaml = "test: true";
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration emptyConfig =
-          .readValue(emptyConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(emptyConfigYaml).orElseThrow();
 
       assertTrue(emptyConfig.getPaymentsConfiguration().getAllowedCountryCodes().isEmpty());
     }
@@ -272,9 +273,9 @@ class DynamicConfigurationTest {
               + "  allowedCountryCodes:\n"
               + "    - 44";
 
-      final DynamicPaymentsConfiguration config = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicPaymentsConfiguration config =
-          .readValue(paymentsConfigYaml, DynamicConfiguration.class)
+          DynamicConfigurationManager.parseConfiguration(paymentsConfigYaml).orElseThrow()
-          .getPaymentsConfiguration();
+              .getPaymentsConfiguration();
 
       assertEquals(Set.of("44"), config.getAllowedCountryCodes());
     }
@@ -284,8 +285,8 @@ class DynamicConfigurationTest {
   void testParseSignupCaptchaConfiguration() throws JsonProcessingException {
     {
       final String emptyConfigYaml = "test: true";
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration emptyConfig =
-          .readValue(emptyConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(emptyConfigYaml).orElseThrow();
 
       assertTrue(emptyConfig.getSignupCaptchaConfiguration().getCountryCodes().isEmpty());
     }
@@ -296,9 +297,9 @@ class DynamicConfigurationTest {
               + "  countryCodes:\n"
               + "    - 1";
 
-      final DynamicSignupCaptchaConfiguration config = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicSignupCaptchaConfiguration config =
-          .readValue(signupCaptchaConfig, DynamicConfiguration.class)
+          DynamicConfigurationManager.parseConfiguration(signupCaptchaConfig).orElseThrow()
-          .getSignupCaptchaConfiguration();
+              .getSignupCaptchaConfiguration();
 
       assertEquals(Set.of("1"), config.getCountryCodes());
     }
@@ -308,8 +309,8 @@ class DynamicConfigurationTest {
   void testParseAccountsDynamoDbMigrationConfiguration() throws JsonProcessingException {
     {
       final String emptyConfigYaml = "test: true";
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicConfiguration emptyConfig =
-          .readValue(emptyConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(emptyConfigYaml).orElseThrow();
 
       assertFalse(emptyConfig.getAccountsDynamoDbMigrationConfiguration().isBackgroundMigrationEnabled());
       assertFalse(emptyConfig.getAccountsDynamoDbMigrationConfiguration().isDeleteEnabled());
@@ -326,9 +327,9 @@ class DynamicConfigurationTest {
               + "  readEnabled: true\n"
               + "  writeEnabled: true";
 
-      final DynamicAccountsDynamoDbMigrationConfiguration config = DynamicConfigurationManager.OBJECT_MAPPER
+      final DynamicAccountsDynamoDbMigrationConfiguration config =
-          .readValue(accountsDynamoDbMigrationConfig, DynamicConfiguration.class)
+          DynamicConfigurationManager.parseConfiguration(accountsDynamoDbMigrationConfig).orElseThrow()
-          .getAccountsDynamoDbMigrationConfiguration();
+              .getAccountsDynamoDbMigrationConfiguration();
 
       assertTrue(config.isBackgroundMigrationEnabled());
       assertEquals(100, config.getBackgroundMigrationExecutorThreads());
@@ -342,8 +343,8 @@ class DynamicConfigurationTest {
   void testParseLimits() throws JsonProcessingException {
     {
       final String emptyConfigYaml = "test: true";
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER.readValue(
+      final DynamicConfiguration emptyConfig =
-          emptyConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(emptyConfigYaml).orElseThrow();
 
       assertThat(emptyConfig.getLimits().getUnsealedSenderNumber().getMaxCardinality()).isEqualTo(100);
       assertThat(emptyConfig.getLimits().getUnsealedSenderNumber().getTtl()).isEqualTo(Duration.ofDays(1));
@@ -355,9 +356,10 @@ class DynamicConfigurationTest {
           + "  unsealedSenderNumber:\n"
           + "    maxCardinality: 99\n"
           + "    ttl: PT23H";
-      final CardinalityRateLimitConfiguration unsealedSenderNumber = DynamicConfigurationManager.OBJECT_MAPPER
+
-          .readValue(limitsConfig, DynamicConfiguration.class)
+      final CardinalityRateLimitConfiguration unsealedSenderNumber =
-          .getLimits().getUnsealedSenderNumber();
+          DynamicConfigurationManager.parseConfiguration(limitsConfig).orElseThrow()
+              .getLimits().getUnsealedSenderNumber();
 
       assertThat(unsealedSenderNumber.getMaxCardinality()).isEqualTo(99);
       assertThat(unsealedSenderNumber.getTtl()).isEqualTo(Duration.ofHours(23));
@@ -368,8 +370,8 @@ class DynamicConfigurationTest {
   void testParseRateLimitReset() throws JsonProcessingException {
     {
       final String emptyConfigYaml = "test: true";
-      final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER.readValue(
+      final DynamicConfiguration emptyConfig =
-          emptyConfigYaml, DynamicConfiguration.class);
+          DynamicConfigurationManager.parseConfiguration(emptyConfigYaml).orElseThrow();
 
       assertThat(emptyConfig.getRateLimitChallengeConfiguration().getClientSupportedVersions()).isEmpty();
       assertThat(emptyConfig.getRateLimitChallengeConfiguration().isPreKeyLimitEnforced()).isFalse();
@@ -384,9 +386,11 @@ class DynamicConfigurationTest {
               + "    IOS: 5.1.0\n"
               + "    ANDROID: 5.2.0\n"
               + "    DESKTOP: 5.0.0";
-      DynamicRateLimitChallengeConfiguration rateLimitChallengeConfiguration = DynamicConfigurationManager.OBJECT_MAPPER
+
-          .readValue(rateLimitChallengeConfig, DynamicConfiguration.class)
+      DynamicRateLimitChallengeConfiguration rateLimitChallengeConfiguration =
-          .getRateLimitChallengeConfiguration();
+          DynamicConfigurationManager.parseConfiguration(rateLimitChallengeConfig).orElseThrow()
+              .getRateLimitChallengeConfiguration();
+
       final Map<ClientPlatform, Semver> clientSupportedVersions = rateLimitChallengeConfiguration.getClientSupportedVersions();
 
       assertThat(clientSupportedVersions.get(ClientPlatform.IOS)).isEqualTo(new Semver("5.1.0"));