diff --git a/service/config/sample.yml b/service/config/sample.yml index 5384075e6..c55edde1f 100644 --- a/service/config/sample.yml +++ b/service/config/sample.yml @@ -1,12 +1,6 @@ twilio: # Twilio gateway configuration accountId: accountToken: - numbers: # Numbers allocated in Twilio - - # First number - - # Second number - - # Third number - - # ... - - # Nth number nanpaMessagingServiceSid: # Twilio SID for the messaging service to use for NANPA. messagingServiceSid: # Twilio SID for the message service to use for non-NANPA. localDomain: # Domain Twilio can connect back to for calls. Should be domain of your service. diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 89abd4b12..e19e5155e 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -353,7 +353,7 @@ public class WhisperServerService extends Application numbers; - @NotEmpty private String localDomain; @@ -68,16 +63,6 @@ public class TwilioConfiguration { public void setAccountToken(String accountToken) { this.accountToken = accountToken; } - - public List getNumbers() { - return numbers; - } - - @VisibleForTesting - public void setNumbers(List numbers) { - this.numbers = numbers; - } - public String getLocalDomain() { return localDomain; } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfiguration.java index 04ab900e2..9d8d11198 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfiguration.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfiguration.java @@ -1,12 +1,12 @@ package org.whispersystems.textsecuregcm.configuration.dynamic; import com.fasterxml.jackson.annotation.JsonProperty; - -import javax.validation.Valid; +import com.google.common.annotations.VisibleForTesting; import java.util.Collections; import java.util.Map; import java.util.Optional; import java.util.Set; +import javax.validation.Valid; public class DynamicConfiguration { @@ -29,6 +29,10 @@ public class DynamicConfiguration { @JsonProperty private Set featureFlags = Collections.emptySet(); + @JsonProperty + @Valid + private DynamicTwilioConfiguration twilio = new DynamicTwilioConfiguration(); + public Optional getExperimentEnrollmentConfiguration(final String experimentName) { return Optional.ofNullable(experiments.get(experimentName)); } @@ -48,4 +52,14 @@ public class DynamicConfiguration { public Set getActiveFeatureFlags() { return featureFlags; } + + public DynamicTwilioConfiguration getTwilioConfiguration() { + return twilio; + } + + @VisibleForTesting + public void setTwilioConfiguration(DynamicTwilioConfiguration twilioConfiguration) { + this.twilio = twilioConfiguration; + } + } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicTwilioConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicTwilioConfiguration.java new file mode 100644 index 000000000..b4d4ea767 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicTwilioConfiguration.java @@ -0,0 +1,23 @@ +package org.whispersystems.textsecuregcm.configuration.dynamic; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.annotations.VisibleForTesting; +import javax.validation.constraints.NotNull; +import java.util.Collections; +import java.util.List; + +public class DynamicTwilioConfiguration { + + @JsonProperty + @NotNull + private List numbers = Collections.emptyList(); + + public List getNumbers() { + return numbers; + } + + @VisibleForTesting + public void setNumbers(List numbers) { + this.numbers = numbers; + } +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java b/service/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java index d947ce47d..52777b2b6 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java @@ -4,26 +4,14 @@ */ package org.whispersystems.textsecuregcm.sms; +import static com.codahale.metrics.MetricRegistry.name; + import com.codahale.metrics.Meter; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.SharedMetricRegistries; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.annotations.VisibleForTesting; -import org.apache.commons.lang3.StringUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.whispersystems.textsecuregcm.configuration.TwilioConfiguration; -import org.whispersystems.textsecuregcm.http.FaultTolerantHttpClient; -import org.whispersystems.textsecuregcm.http.FormDataBodyPublisher; -import org.whispersystems.textsecuregcm.util.Base64; -import org.whispersystems.textsecuregcm.util.Constants; -import org.whispersystems.textsecuregcm.util.ExecutorUtils; -import org.whispersystems.textsecuregcm.util.SystemMapper; -import org.whispersystems.textsecuregcm.util.Util; - -import javax.annotation.Nonnull; -import javax.annotation.Nullable; import java.io.IOException; import java.net.URI; import java.net.http.HttpClient; @@ -31,16 +19,27 @@ import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.nio.charset.StandardCharsets; import java.time.Duration; -import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Random; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; - -import static com.codahale.metrics.MetricRegistry.name; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.whispersystems.textsecuregcm.configuration.TwilioConfiguration; +import org.whispersystems.textsecuregcm.http.FaultTolerantHttpClient; +import org.whispersystems.textsecuregcm.http.FormDataBodyPublisher; +import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; +import org.whispersystems.textsecuregcm.util.Base64; +import org.whispersystems.textsecuregcm.util.Constants; +import org.whispersystems.textsecuregcm.util.ExecutorUtils; +import org.whispersystems.textsecuregcm.util.SystemMapper; +import org.whispersystems.textsecuregcm.util.Util; @SuppressWarnings("OptionalUsedAsFieldOrParameterType") public class TwilioSmsSender { @@ -53,7 +52,6 @@ public class TwilioSmsSender { private final String accountId; private final String accountToken; - private final ArrayList numbers; private final String messagingServiceSid; private final String nanpaMessagingServiceSid; private final String localDomain; @@ -67,13 +65,14 @@ public class TwilioSmsSender { private final URI smsUri; private final URI voxUri; + private final DynamicConfigurationManager dynamicConfigurationManager; + @VisibleForTesting - public TwilioSmsSender(String baseUri, TwilioConfiguration twilioConfiguration) { + public TwilioSmsSender(String baseUri, TwilioConfiguration twilioConfiguration, DynamicConfigurationManager dynamicConfigurationManager) { Executor executor = ExecutorUtils.newFixedThreadBoundedQueueExecutor(10, 100); this.accountId = twilioConfiguration.getAccountId(); this.accountToken = twilioConfiguration.getAccountToken(); - this.numbers = new ArrayList<>(twilioConfiguration.getNumbers()); this.localDomain = twilioConfiguration.getLocalDomain(); this.messagingServiceSid = twilioConfiguration.getMessagingServiceSid(); this.nanpaMessagingServiceSid = twilioConfiguration.getNanpaMessagingServiceSid(); @@ -93,10 +92,11 @@ public class TwilioSmsSender { .withExecutor(executor) .withName("twilio") .build(); + this.dynamicConfigurationManager = dynamicConfigurationManager; } - public TwilioSmsSender(TwilioConfiguration twilioConfiguration) { - this("https://api.twilio.com", twilioConfiguration); + public TwilioSmsSender(TwilioConfiguration twilioConfiguration, DynamicConfigurationManager dynamicConfigurationManager) { + this("https://api.twilio.com", twilioConfiguration, dynamicConfigurationManager); } public CompletableFuture deliverSmsVerification(String destination, Optional clientType, String verificationCode) { @@ -148,7 +148,7 @@ public class TwilioSmsSender { Map requestParameters = new HashMap<>(); requestParameters.put("Url", url); requestParameters.put("To", destination); - requestParameters.put("From", getRandom(random, numbers)); + requestParameters.put("From", getRandom(random, dynamicConfigurationManager.getConfiguration().getTwilioConfiguration().getNumbers())); HttpRequest request = HttpRequest.newBuilder() .uri(voxUri) @@ -164,7 +164,7 @@ public class TwilioSmsSender { .handle(this::processResponse); } - private String getRandom(Random random, ArrayList elements) { + private String getRandom(Random random, List elements) { return elements.get(random.nextInt(elements.size())); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfigurationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfigurationTest.java index 65a8fec35..51bcdb652 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfigurationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfigurationTest.java @@ -5,18 +5,21 @@ package org.whispersystems.textsecuregcm.configuration.dynamic; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + import com.fasterxml.jackson.core.JsonProcessingException; import com.vdurmont.semver4j.Semver; -import org.junit.Test; -import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; -import org.whispersystems.textsecuregcm.util.ua.ClientPlatform; - import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; - -import static org.junit.Assert.*; +import org.junit.Test; +import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; +import org.whispersystems.textsecuregcm.util.ua.ClientPlatform; public class DynamicConfigurationTest { @@ -150,4 +153,29 @@ public class DynamicConfigurationTest { assertTrue(emptyConfig.getActiveFeatureFlags().contains("testFlag")); } } + + @Test + public void testParseTwilioConfiguration() throws JsonProcessingException { + { + final String emptyConfigYaml = "test: true"; + final DynamicConfiguration emptyConfig = DynamicConfigurationManager.OBJECT_MAPPER + .readValue(emptyConfigYaml, DynamicConfiguration.class); + + assertTrue(emptyConfig.getTwilioConfiguration().getNumbers().isEmpty()); + } + + { + final String emptyConfigYaml = + "twilio:\n" + + " numbers:\n" + + " - 2135551212\n" + + " - 2135551313"; + + final DynamicTwilioConfiguration config = DynamicConfigurationManager.OBJECT_MAPPER + .readValue(emptyConfigYaml, DynamicConfiguration.class) + .getTwilioConfiguration(); + + assertEquals(List.of("2135551212", "2135551313"), config.getNumbers()); + } + } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/sms/TwilioSmsSenderTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/sms/TwilioSmsSenderTest.java index f29ada798..4ba87bb2c 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/sms/TwilioSmsSenderTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/sms/TwilioSmsSenderTest.java @@ -14,21 +14,25 @@ import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; import static com.github.tomakehurst.wiremock.client.WireMock.verify; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.mockito.Mockito.*; import com.github.tomakehurst.wiremock.junit.WireMockRule; import java.util.List; import java.util.Optional; import javax.annotation.Nonnull; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.whispersystems.textsecuregcm.configuration.TwilioConfiguration; +import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; +import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicTwilioConfiguration; import org.whispersystems.textsecuregcm.sms.TwilioSmsSender; +import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; public class TwilioSmsSenderTest { private static final String ACCOUNT_ID = "test_account_id"; private static final String ACCOUNT_TOKEN = "test_account_token"; - private static final List NUMBERS = List.of("+14151111111", "+14152222222"); private static final String MESSAGING_SERVICE_SID = "test_messaging_services_id"; private static final String NANPA_MESSAGING_SERVICE_SID = "nanpa_test_messaging_service_id"; private static final String LOCAL_DOMAIN = "test.com"; @@ -36,12 +40,24 @@ public class TwilioSmsSenderTest { @Rule public WireMockRule wireMockRule = new WireMockRule(options().dynamicPort().dynamicHttpsPort()); + private DynamicConfigurationManager dynamicConfigurationManager; + + @Before + public void setup() { + + dynamicConfigurationManager = mock(DynamicConfigurationManager.class); + DynamicConfiguration dynamicConfiguration = new DynamicConfiguration(); + DynamicTwilioConfiguration dynamicTwilioConfiguration = new DynamicTwilioConfiguration(); + dynamicConfiguration.setTwilioConfiguration(dynamicTwilioConfiguration); + dynamicTwilioConfiguration.setNumbers(List.of("+14151111111", "+14152222222")); + when(dynamicConfigurationManager.getConfiguration()).thenReturn(dynamicConfiguration); + } + @Nonnull private TwilioConfiguration createTwilioConfiguration() { TwilioConfiguration configuration = new TwilioConfiguration(); configuration.setAccountId(ACCOUNT_ID); configuration.setAccountToken(ACCOUNT_TOKEN); - configuration.setNumbers(NUMBERS); configuration.setMessagingServiceSid(MESSAGING_SERVICE_SID); configuration.setNanpaMessagingServiceSid(NANPA_MESSAGING_SERVICE_SID); configuration.setLocalDomain(LOCAL_DOMAIN); @@ -64,7 +80,7 @@ public class TwilioSmsSenderTest { public void testSendSms() { setupSuccessStubForSms(); TwilioConfiguration configuration = createTwilioConfiguration(); - TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration); + TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration, dynamicConfigurationManager); boolean success = sender.deliverSmsVerification("+14153333333", Optional.of("android-ng"), "123-456").join(); assertThat(success).isTrue(); @@ -78,7 +94,7 @@ public class TwilioSmsSenderTest { public void testSendSmsAndroid202001() { setupSuccessStubForSms(); TwilioConfiguration configuration = createTwilioConfiguration(); - TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration); + TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration, dynamicConfigurationManager); boolean success = sender.deliverSmsVerification("+14153333333", Optional.of("android-2020-01"), "123-456").join(); assertThat(success).isTrue(); @@ -93,7 +109,7 @@ public class TwilioSmsSenderTest { setupSuccessStubForSms(); TwilioConfiguration configuration = createTwilioConfiguration(); configuration.setNanpaMessagingServiceSid(NANPA_MESSAGING_SERVICE_SID); - TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration); + TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration, dynamicConfigurationManager); assertThat(sender.deliverSmsVerification("+14153333333", Optional.of("ios"), "654-321").join()).isTrue(); verify(1, postRequestedFor(urlEqualTo("/2010-04-01/Accounts/" + ACCOUNT_ID + "/Messages.json")) @@ -118,7 +134,7 @@ public class TwilioSmsSenderTest { TwilioConfiguration configuration = createTwilioConfiguration(); - TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration); + TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration, dynamicConfigurationManager); boolean success = sender.deliverVoxVerification("+14153333333", "123-456", Optional.of("en_US")).join(); assertThat(success).isTrue(); @@ -140,7 +156,7 @@ public class TwilioSmsSenderTest { TwilioConfiguration configuration = createTwilioConfiguration(); - TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration); + TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration, dynamicConfigurationManager); boolean success = sender.deliverSmsVerification("+14153333333", Optional.of("android-ng"), "123-456").join(); assertThat(success).isFalse(); @@ -161,7 +177,7 @@ public class TwilioSmsSenderTest { TwilioConfiguration configuration = createTwilioConfiguration(); - TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration); + TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration, dynamicConfigurationManager); boolean success = sender.deliverVoxVerification("+14153333333", "123-456", Optional.of("en_US")).join(); assertThat(success).isFalse(); @@ -176,7 +192,7 @@ public class TwilioSmsSenderTest { public void testSendSmsNetworkFailure() { TwilioConfiguration configuration = createTwilioConfiguration(); - TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + 39873, configuration); + TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + 39873, configuration, dynamicConfigurationManager); boolean success = sender.deliverSmsVerification("+14153333333", Optional.of("android-ng"), "123-456").join(); assertThat(success).isFalse(); @@ -193,7 +209,7 @@ public class TwilioSmsSenderTest { TwilioConfiguration configuration = createTwilioConfiguration(); - TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration); + TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration, dynamicConfigurationManager); boolean success = sender.deliverSmsVerification("+14153333333", Optional.of("android-ng"), "123-456").join(); assertThat(success).isFalse(); @@ -207,7 +223,7 @@ public class TwilioSmsSenderTest { public void testSendSmsChina() { setupSuccessStubForSms(); TwilioConfiguration configuration = createTwilioConfiguration(); - TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration); + TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration, dynamicConfigurationManager); boolean success = sender.deliverSmsVerification("+861065529988", Optional.of("android-ng"), "123-456").join(); assertThat(success).isTrue();