diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioAlphaIdConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioAlphaIdConfiguration.java deleted file mode 100644 index f3ecc3093..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioAlphaIdConfiguration.java +++ /dev/null @@ -1,31 +0,0 @@ -package org.whispersystems.textsecuregcm.configuration; - -import com.google.common.annotations.VisibleForTesting; - -import javax.validation.constraints.NotEmpty; - -public class TwilioAlphaIdConfiguration { - @NotEmpty - private String prefix; - - @NotEmpty - private String value; - - public String getPrefix() { - return prefix; - } - - @VisibleForTesting - public void setPrefix(String prefix) { - this.prefix = prefix; - } - - public String getValue() { - return value; - } - - @VisibleForTesting - public void setValue(String value) { - this.value = value; - } -} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioConfiguration.java index 8d2352830..daa8fbf07 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioConfiguration.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioConfiguration.java @@ -16,34 +16,27 @@ */ package org.whispersystems.textsecuregcm.configuration; -import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.annotations.VisibleForTesting; import javax.validation.Valid; import javax.validation.constraints.NotEmpty; import javax.validation.constraints.NotNull; -import java.util.ArrayList; import java.util.List; public class TwilioConfiguration { @NotEmpty - @JsonProperty private String accountId; @NotEmpty - @JsonProperty private String accountToken; @NotNull - @JsonProperty private List numbers; @NotEmpty - @JsonProperty private String localDomain; - @JsonProperty private String messagingServicesId; @NotNull @@ -56,7 +49,7 @@ public class TwilioConfiguration { @NotNull @Valid - private List alphaId = new ArrayList<>(); + private TwilioSenderIdConfiguration senderId = new TwilioSenderIdConfiguration(); public String getAccountId() { return accountId; @@ -121,12 +114,12 @@ public class TwilioConfiguration { this.retry = retry; } - public List getAlphaId() { - return alphaId; + public TwilioSenderIdConfiguration getSenderId() { + return senderId; } @VisibleForTesting - public void setAlphaId(List alphaId) { - this.alphaId = alphaId; + public void setSenderId(TwilioSenderIdConfiguration senderId) { + this.senderId = senderId; } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioCountrySenderIdConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioCountrySenderIdConfiguration.java new file mode 100644 index 000000000..7f444cdf1 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioCountrySenderIdConfiguration.java @@ -0,0 +1,31 @@ +package org.whispersystems.textsecuregcm.configuration; + +import com.google.common.annotations.VisibleForTesting; + +import javax.validation.constraints.NotEmpty; + +public class TwilioCountrySenderIdConfiguration { + @NotEmpty + private String countryCode; + + @NotEmpty + private String senderId; + + public String getCountryCode() { + return countryCode; + } + + @VisibleForTesting + public void setCountryCode(String countryCode) { + this.countryCode = countryCode; + } + + public String getSenderId() { + return senderId; + } + + @VisibleForTesting + public void setSenderId(String senderId) { + this.senderId = senderId; + } +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioSenderIdConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioSenderIdConfiguration.java new file mode 100644 index 000000000..7dc769898 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/TwilioSenderIdConfiguration.java @@ -0,0 +1,51 @@ +package org.whispersystems.textsecuregcm.configuration; + +import com.google.common.annotations.VisibleForTesting; + +import javax.validation.Valid; +import javax.validation.constraints.NotEmpty; +import javax.validation.constraints.NotNull; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +public class TwilioSenderIdConfiguration { + @NotEmpty + private String defaultSenderId; + + @NotNull + @Valid + private List countrySpecificSenderIds = new ArrayList<>(); + + @NotNull + @Valid + private Set countryCodesWithoutSenderId = new HashSet<>(); + + public String getDefaultSenderId() { + return defaultSenderId; + } + + @VisibleForTesting + public void setDefaultSenderId(String defaultSenderId) { + this.defaultSenderId = defaultSenderId; + } + + public List getCountrySpecificSenderIds() { + return countrySpecificSenderIds; + } + + @VisibleForTesting + public void setCountrySpecificSenderIds(List countrySpecificSenderIds) { + this.countrySpecificSenderIds = countrySpecificSenderIds; + } + + public Set getCountryCodesWithoutSenderId() { + return countryCodesWithoutSenderId; + } + + @VisibleForTesting + public void setCountryCodesWithoutSenderId(Set countryCodesWithoutSenderId) { + this.countryCodesWithoutSenderId = countryCodesWithoutSenderId; + } +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/sms/SenderIdStrategy.java b/service/src/main/java/org/whispersystems/textsecuregcm/sms/SenderIdStrategy.java new file mode 100644 index 000000000..900ba6e27 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/sms/SenderIdStrategy.java @@ -0,0 +1,47 @@ +package org.whispersystems.textsecuregcm.sms; + +import com.google.common.base.Strings; +import org.whispersystems.textsecuregcm.configuration.TwilioCountrySenderIdConfiguration; +import org.whispersystems.textsecuregcm.configuration.TwilioSenderIdConfiguration; +import org.whispersystems.textsecuregcm.util.Util; + +import javax.validation.constraints.NotNull; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +public class SenderIdStrategy { + private final String defaultSenderId; + private final Map countrySpecificSenderIds; + private final Set countryCodesWithoutSenderId; + + public SenderIdStrategy(String defaultSenderId, List countrySpecificSenderIds, Set countryCodesWithoutSenderId) { + this.defaultSenderId = defaultSenderId; + this.countrySpecificSenderIds = countrySpecificSenderIds.stream().collect(Collectors.toMap( + TwilioCountrySenderIdConfiguration::getCountryCode, + TwilioCountrySenderIdConfiguration::getSenderId)); + this.countryCodesWithoutSenderId = countryCodesWithoutSenderId; + } + + public SenderIdStrategy(TwilioSenderIdConfiguration configuration) { + this(configuration.getDefaultSenderId(), + configuration.getCountrySpecificSenderIds(), + configuration.getCountryCodesWithoutSenderId()); + } + + public Optional get(@NotNull String destination) { + final String countryCode = Util.getCountryCode(destination); + if (countryCodesWithoutSenderId.contains(countryCode)) { + return Optional.empty(); + } + + final String countrySpecificSenderId = countrySpecificSenderIds.get(countryCode); + if (!Strings.isNullOrEmpty(countrySpecificSenderId)) { + return Optional.of(countrySpecificSenderId); + } + + return Optional.ofNullable(defaultSenderId); + } +} 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 193a7b3a3..a81182558 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java @@ -24,7 +24,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.whispersystems.textsecuregcm.configuration.TwilioAlphaIdConfiguration; import org.whispersystems.textsecuregcm.configuration.TwilioConfiguration; import org.whispersystems.textsecuregcm.http.FaultTolerantHttpClient; import org.whispersystems.textsecuregcm.http.FormDataBodyPublisher; @@ -42,7 +41,6 @@ import java.net.http.HttpResponse; import java.time.Duration; import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Random; @@ -62,13 +60,13 @@ public class TwilioSmsSender { private final Meter voxMeter = metricRegistry.meter(name(getClass(), "vox", "delivered")); private final Meter priceMeter = metricRegistry.meter(name(getClass(), "price")); - private final String accountId; - private final String accountToken; - private final ArrayList numbers; - private final String messagingServicesId; - private final String localDomain; - private final List alphaId; - private final Random random; + private final String accountId; + private final String accountToken; + private final ArrayList numbers; + private final String messagingServicesId; + private final String localDomain; + private final Random random; + private final SenderIdStrategy senderIdStrategy; private final FaultTolerantHttpClient httpClient; private final URI smsUri; @@ -83,7 +81,7 @@ public class TwilioSmsSender { this.numbers = new ArrayList<>(twilioConfiguration.getNumbers()); this.localDomain = twilioConfiguration.getLocalDomain(); this.messagingServicesId = twilioConfiguration.getMessagingServicesId(); - this.alphaId = twilioConfiguration.getAlphaId(); + this.senderIdStrategy = new SenderIdStrategy(twilioConfiguration.getSenderId()); this.random = new Random(System.currentTimeMillis()); this.smsUri = URI.create(baseUri + "/2010-04-01/Accounts/" + accountId + "/Messages.json"); this.voxUri = URI.create(baseUri + "/2010-04-01/Accounts/" + accountId + "/Calls.json" ); @@ -176,9 +174,9 @@ public class TwilioSmsSender { } private void setOriginationRequestParameter(String destination, Map requestParameters) { - final Optional alphaIdConfiguration = getAlphaIdConfigurationForNumber(destination); - if (alphaIdConfiguration.isPresent()) { - requestParameters.put("From", alphaIdConfiguration.get().getValue()); + final Optional senderId = senderIdStrategy.get(destination); + if (senderId.isPresent()) { + requestParameters.put("From", senderId.get()); } else if (Util.isEmpty(messagingServicesId)) { requestParameters.put("From", getRandom(random, numbers)); } else { @@ -224,21 +222,6 @@ public class TwilioSmsSender { } } - private Optional getAlphaIdConfigurationForNumber(String number) { - if (alphaId == null) { - return Optional.empty(); - } - - final String countryCode = Util.getCountryCode(number); - for (TwilioAlphaIdConfiguration twilioAlphaIdConfiguration : alphaId) { - if (twilioAlphaIdConfiguration.getPrefix().equalsIgnoreCase(countryCode)) { - return Optional.of(twilioAlphaIdConfiguration); - } - } - - return Optional.empty(); - } - public static class TwilioResponse { private TwilioSuccessResponse successResponse; 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 d76a3ebb5..84fcc6fbc 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 @@ -3,15 +3,26 @@ package org.whispersystems.textsecuregcm.tests.sms; import com.github.tomakehurst.wiremock.junit.WireMockRule; import org.junit.Rule; import org.junit.Test; -import org.whispersystems.textsecuregcm.configuration.TwilioAlphaIdConfiguration; import org.whispersystems.textsecuregcm.configuration.TwilioConfiguration; +import org.whispersystems.textsecuregcm.configuration.TwilioCountrySenderIdConfiguration; +import org.whispersystems.textsecuregcm.configuration.TwilioSenderIdConfiguration; import org.whispersystems.textsecuregcm.sms.TwilioSmsSender; import javax.annotation.Nonnull; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Optional; +import java.util.Set; +import java.util.function.Supplier; -import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; +import static com.github.tomakehurst.wiremock.client.WireMock.matching; +import static com.github.tomakehurst.wiremock.client.WireMock.post; +import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor; +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; @@ -133,8 +144,9 @@ public class TwilioSmsSenderTest { assertThat(success).isFalse(); } - @Test - public void testSendAlphaIdByCountryCode() { + private void runSenderIdTest(String destination, + String expectedSenderId, + Supplier senderIdConfigurationSupplier) { wireMockRule.stubFor(post(urlEqualTo("/2010-04-01/Accounts/" + ACCOUNT_ID + "/Messages.json")) .withBasicAuth(ACCOUNT_ID, ACCOUNT_TOKEN) .willReturn(aResponse() @@ -142,19 +154,92 @@ public class TwilioSmsSenderTest { .withBody("{\"price\": -0.00750, \"status\": \"sent\"}"))); TwilioConfiguration configuration = createTwilioConfiguration(); - TwilioAlphaIdConfiguration alphaIdConfiguration = new TwilioAlphaIdConfiguration(); - alphaIdConfiguration.setPrefix("852"); - alphaIdConfiguration.setValue("SIGNAL"); - configuration.setAlphaId(List.of(alphaIdConfiguration)); + configuration.setSenderId(senderIdConfigurationSupplier.get()); TwilioSmsSender sender = new TwilioSmsSender("http://localhost:" + wireMockRule.port(), configuration); - boolean success = sender.deliverSmsVerification("+85278675309", Optional.of("android-ng"), "987-654").join(); + boolean success = sender.deliverSmsVerification(destination, Optional.of("android-ng"), "987-654").join(); assertThat(success).isTrue(); + final String requestBodyToParam = "To=" + URLEncoder.encode(destination, StandardCharsets.UTF_8); + final String requestBodySuffix = "&Body=%3C%23%3E+Your+Signal+verification+code%3A+987-654%0A%0AdoDiFGKPO1r"; + final String expectedRequestBody; + if (expectedSenderId != null) { + expectedRequestBody = requestBodyToParam + "&From=" + expectedSenderId + requestBodySuffix; + } else { + expectedRequestBody = "MessagingServiceSid=" + MESSAGING_SERVICES_ID + "&" + requestBodyToParam + requestBodySuffix; + } verify(1, postRequestedFor(urlEqualTo("/2010-04-01/Accounts/" + ACCOUNT_ID + "/Messages.json")) .withHeader("Content-Type", equalTo("application/x-www-form-urlencoded")) - .withRequestBody(equalTo("To=%2B85278675309&From=SIGNAL&Body=%3C%23%3E+Your+Signal+verification+code%3A+987-654%0A%0AdoDiFGKPO1r"))); + .withRequestBody(equalTo(expectedRequestBody))); } + @Test + public void testSendAlphaIdByCountryCode() { + runSenderIdTest("+85278675309", "SIGNAL", () -> { + TwilioSenderIdConfiguration senderIdConfiguration = new TwilioSenderIdConfiguration(); + TwilioCountrySenderIdConfiguration twilioCountrySenderIdConfiguration = new TwilioCountrySenderIdConfiguration(); + twilioCountrySenderIdConfiguration.setCountryCode("852"); + twilioCountrySenderIdConfiguration.setSenderId("SIGNAL"); + senderIdConfiguration.setCountrySpecificSenderIds(List.of(twilioCountrySenderIdConfiguration)); + return senderIdConfiguration; + }); + } + + @Test + public void testDefaultSenderId() { + runSenderIdTest("+14098675309", "SIGNALFOO", () -> { + TwilioSenderIdConfiguration senderIdConfiguration = new TwilioSenderIdConfiguration(); + senderIdConfiguration.setDefaultSenderId("SIGNALFOO"); + return senderIdConfiguration; + }); + } + + @Test + public void testDefaultSenderIdWithDisabledCountry() { + final Supplier senderIdConfigurationSupplier = () -> { + TwilioSenderIdConfiguration senderIdConfiguration = new TwilioSenderIdConfiguration(); + senderIdConfiguration.setDefaultSenderId("SIGNALBAR"); + senderIdConfiguration.setCountryCodesWithoutSenderId(Set.of("1")); + return senderIdConfiguration; + }; + runSenderIdTest("+14098675309", null, senderIdConfigurationSupplier); + runSenderIdTest("+447911123456", "SIGNALBAR", senderIdConfigurationSupplier); + } + + @Test + public void testDefaultSenderIdWithOverriddenCountry() { + final Supplier senderIdConfigurationSupplier = () -> { + TwilioSenderIdConfiguration senderIdConfiguration = new TwilioSenderIdConfiguration(); + TwilioCountrySenderIdConfiguration twilioCountrySenderIdConfiguration = new TwilioCountrySenderIdConfiguration(); + twilioCountrySenderIdConfiguration.setCountryCode("1"); + twilioCountrySenderIdConfiguration.setSenderId("OFCOURSEISTILLLOVEYOU"); + senderIdConfiguration.setDefaultSenderId("JUSTREADTHEINSTRUCTIONS"); + senderIdConfiguration.setCountrySpecificSenderIds(List.of(twilioCountrySenderIdConfiguration)); + return senderIdConfiguration; + }; + runSenderIdTest("+15128675309", "OFCOURSEISTILLLOVEYOU", senderIdConfigurationSupplier); + runSenderIdTest("+6433456789", "JUSTREADTHEINSTRUCTIONS", senderIdConfigurationSupplier); + } + + @Test + public void testSenderIdWithAllFieldsPopulated() { + final Supplier senderIdConfigurationSupplier = () -> { + TwilioSenderIdConfiguration senderIdConfiguration = new TwilioSenderIdConfiguration(); + TwilioCountrySenderIdConfiguration twilioCountrySenderIdConfiguration1 = new TwilioCountrySenderIdConfiguration(); + TwilioCountrySenderIdConfiguration twilioCountrySenderIdConfiguration2 = new TwilioCountrySenderIdConfiguration(); + twilioCountrySenderIdConfiguration1.setCountryCode("1"); + twilioCountrySenderIdConfiguration1.setSenderId("KNOWYOUREOUTTHERE"); + twilioCountrySenderIdConfiguration2.setCountryCode("61"); + twilioCountrySenderIdConfiguration2.setSenderId("SOMEWHERE"); + senderIdConfiguration.setDefaultSenderId("SOMEHOW"); + senderIdConfiguration.setCountrySpecificSenderIds(List.of(twilioCountrySenderIdConfiguration1, twilioCountrySenderIdConfiguration2)); + senderIdConfiguration.setCountryCodesWithoutSenderId(Set.of("7")); + return senderIdConfiguration; + }; + runSenderIdTest("+15128675309", "KNOWYOUREOUTTHERE", senderIdConfigurationSupplier); + runSenderIdTest("+610998765432", "SOMEWHERE", senderIdConfigurationSupplier); + runSenderIdTest("+74991234567", null, senderIdConfigurationSupplier); + runSenderIdTest("+85278675309", "SOMEHOW", senderIdConfigurationSupplier); + } }