From b31a88043e14aa048d362478f229092f41d270a2 Mon Sep 17 00:00:00 2001 From: Frederic Jacobs Date: Sun, 29 Nov 2015 16:50:11 +0100 Subject: [PATCH] Adding Signal SMS verification strings. - Changes the voice verification string. - Keeps the TextSecure SMS String for matching in Signal for Android. - Changes TextSecure to Signal for iOS, adding tap to verify link. - Added test for iOS query parameter. --- .../controllers/AccountController.java | 6 ++++-- .../textsecuregcm/sms/NexmoSmsSender.java | 15 ++++++++++++--- .../textsecuregcm/sms/SmsSender.java | 12 ++++++------ .../textsecuregcm/sms/TwilioSmsSender.java | 11 ++++++++--- .../tests/controllers/AccountControllerTest.java | 16 +++++++++++++++- .../tests/sms/DeliveryPreferenceTest.java | 8 ++++---- .../tests/sms/TwilioFallbackTest.java | 8 ++++---- 7 files changed, 53 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java index a76cd09ff..9d711298a 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -51,6 +51,7 @@ import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; @@ -103,7 +104,8 @@ public class AccountController { @GET @Path("/{transport}/code/{number}") public Response createAccount(@PathParam("transport") String transport, - @PathParam("number") String number) + @PathParam("number") String number, + @QueryParam("client") String client) throws IOException, RateLimitExceededException { if (!Util.isValidNumber(number)) { @@ -128,7 +130,7 @@ public class AccountController { if (testDevices.containsKey(number)) { // noop } else if (transport.equals("sms")) { - smsSender.deliverSmsVerification(number, verificationCode.getVerificationCodeDisplay()); + smsSender.deliverSmsVerification(number, client, verificationCode.getVerificationCodeDisplay()); } else if (transport.equals("voice")) { smsSender.deliverVoxVerification(number, verificationCode.getVerificationCodeSpeech()); } diff --git a/src/main/java/org/whispersystems/textsecuregcm/sms/NexmoSmsSender.java b/src/main/java/org/whispersystems/textsecuregcm/sms/NexmoSmsSender.java index 58e34a060..cafa7029e 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/sms/NexmoSmsSender.java +++ b/src/main/java/org/whispersystems/textsecuregcm/sms/NexmoSmsSender.java @@ -19,6 +19,7 @@ package org.whispersystems.textsecuregcm.sms; import com.codahale.metrics.Meter; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.SharedMetricRegistries; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.configuration.NexmoConfiguration; @@ -56,10 +57,18 @@ public class NexmoSmsSender { this.number = config.getNumber(); } - public void deliverSmsVerification(String destination, String verificationCode) throws IOException { - URL url = new URL(String.format(NEXMO_SMS_URL, apiKey, apiSecret, number, destination, - URLEncoder.encode(SmsSender.SMS_VERIFICATION_TEXT + verificationCode, "UTF-8"))); + public void deliverSmsVerification(String destination, String clientType, String verificationCode) throws IOException { + String verificationMsg; + if ("ios".equals(clientType)) { + verificationMsg = String.format(SmsSender.SMS_IOS_VERIFICATION_TEXT, verificationCode, verificationCode); + } else { + verificationMsg = String.format(SmsSender.SMS_VERIFICATION_TEXT, verificationCode); + } + + URL url = new URL(String.format(NEXMO_SMS_URL, apiKey, apiSecret, number, destination, + URLEncoder.encode(verificationMsg, "UTF-8"))); + URLConnection connection = url.openConnection(); connection.setDoInput(true); connection.connect(); diff --git a/src/main/java/org/whispersystems/textsecuregcm/sms/SmsSender.java b/src/main/java/org/whispersystems/textsecuregcm/sms/SmsSender.java index 9b7ed59ba..ed537a28f 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/sms/SmsSender.java +++ b/src/main/java/org/whispersystems/textsecuregcm/sms/SmsSender.java @@ -25,9 +25,9 @@ import org.slf4j.LoggerFactory; import java.io.IOException; public class SmsSender { - + static final String SMS_IOS_VERIFICATION_TEXT = "Your Signal verification code: %s\n\nOr tap: sgnl://verify/%s"; static final String SMS_VERIFICATION_TEXT = "Your TextSecure verification code: "; - static final String VOX_VERIFICATION_TEXT = "Your TextSecure verification code is: "; + static final String VOX_VERIFICATION_TEXT = "Your Signal verification code is: "; private final Logger logger = LoggerFactory.getLogger(SmsSender.class); @@ -44,7 +44,7 @@ public class SmsSender { this.nexmoSender = nexmoSender; } - public void deliverSmsVerification(String destination, String verificationCode) + public void deliverSmsVerification(String destination, String clientType, String verificationCode) throws IOException { // Fix up mexico numbers to 'mobile' format just for SMS delivery. @@ -53,14 +53,14 @@ public class SmsSender { } if (!isTwilioDestination(destination) && nexmoSender.isPresent()) { - nexmoSender.get().deliverSmsVerification(destination, verificationCode); + nexmoSender.get().deliverSmsVerification(destination, clientType, verificationCode); } else { try { - twilioSender.deliverSmsVerification(destination, verificationCode); + twilioSender.deliverSmsVerification(destination, clientType, verificationCode); } catch (TwilioRestException e) { logger.info("Twilio SMS Failed: " + e.getErrorMessage()); if (nexmoSender.isPresent()) { - nexmoSender.get().deliverSmsVerification(destination, verificationCode); + nexmoSender.get().deliverSmsVerification(destination, clientType, verificationCode); } } } diff --git a/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java b/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java index efe636bcf..a578085de 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java +++ b/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java @@ -63,7 +63,7 @@ public class TwilioSmsSender { this.random = new Random(System.currentTimeMillis()); } - public void deliverSmsVerification(String destination, String verificationCode) + public void deliverSmsVerification(String destination, String clientType, String verificationCode) throws IOException, TwilioRestException { TwilioRestClient client = new TwilioRestClient(accountId, accountToken); @@ -71,8 +71,13 @@ public class TwilioSmsSender { List messageParams = new LinkedList<>(); messageParams.add(new BasicNameValuePair("To", destination)); messageParams.add(new BasicNameValuePair("From", getRandom(random, numbers))); - messageParams.add(new BasicNameValuePair("Body", SmsSender.SMS_VERIFICATION_TEXT + verificationCode)); - + + if ("ios".equals(clientType)) { + messageParams.add(new BasicNameValuePair("Body", String.format(SmsSender.SMS_IOS_VERIFICATION_TEXT, verificationCode, verificationCode))); + } else { + messageParams.add(new BasicNameValuePair("Body", String.format(SmsSender.SMS_VERIFICATION_TEXT, verificationCode))); + } + try { messageFactory.create(messageParams); } catch (RuntimeException damnYouTwilio) { diff --git a/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java b/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java index 7c563bdd2..6450c83f1 100644 --- a/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java +++ b/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java @@ -82,7 +82,21 @@ public class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(200); - verify(smsSender).deliverSmsVerification(eq(SENDER), anyString()); + verify(smsSender).deliverSmsVerification(eq(SENDER), isNull(String.class), anyString()); + } + + @Test + public void testSendiOSCode() throws Exception { + Response response = + resources.getJerseyTest() + .target(String.format("/v1/accounts/sms/code/%s", SENDER)) + .queryParam("client", "ios") + .request() + .get(); + + assertThat(response.getStatus()).isEqualTo(200); + + verify(smsSender).deliverSmsVerification(eq(SENDER), eq("ios"), anyString()); } @Test diff --git a/src/test/java/org/whispersystems/textsecuregcm/tests/sms/DeliveryPreferenceTest.java b/src/test/java/org/whispersystems/textsecuregcm/tests/sms/DeliveryPreferenceTest.java index e5fbf9516..565408997 100644 --- a/src/test/java/org/whispersystems/textsecuregcm/tests/sms/DeliveryPreferenceTest.java +++ b/src/test/java/org/whispersystems/textsecuregcm/tests/sms/DeliveryPreferenceTest.java @@ -21,16 +21,16 @@ public class DeliveryPreferenceTest extends TestCase { public void testInternationalPreferenceOff() throws IOException, TwilioRestException { SmsSender smsSender = new SmsSender(twilioSender, Optional.of(nexmoSender), false); - smsSender.deliverSmsVerification("+441112223333", "123-456"); - verify(nexmoSender).deliverSmsVerification("+441112223333", "123-456"); + smsSender.deliverSmsVerification("+441112223333", null, "123-456"); + verify(nexmoSender).deliverSmsVerification("+441112223333", null, "123-456"); verifyNoMoreInteractions(twilioSender); } public void testInternationalPreferenceOn() throws IOException, TwilioRestException { SmsSender smsSender = new SmsSender(twilioSender, Optional.of(nexmoSender), true); - smsSender.deliverSmsVerification("+441112223333", "123-456"); - verify(twilioSender).deliverSmsVerification("+441112223333", "123-456"); + smsSender.deliverSmsVerification("+441112223333", null, "123-456"); + verify(twilioSender).deliverSmsVerification("+441112223333", null, "123-456"); verifyNoMoreInteractions(nexmoSender); } } diff --git a/src/test/java/org/whispersystems/textsecuregcm/tests/sms/TwilioFallbackTest.java b/src/test/java/org/whispersystems/textsecuregcm/tests/sms/TwilioFallbackTest.java index c27e0cf2d..d81726409 100644 --- a/src/test/java/org/whispersystems/textsecuregcm/tests/sms/TwilioFallbackTest.java +++ b/src/test/java/org/whispersystems/textsecuregcm/tests/sms/TwilioFallbackTest.java @@ -19,16 +19,16 @@ public class TwilioFallbackTest extends TestCase { @Override protected void setUp() throws IOException, TwilioRestException { - doThrow(new TwilioRestException("foo", 404)).when(twilioSender).deliverSmsVerification(anyString(), anyString()); + doThrow(new TwilioRestException("foo", 404)).when(twilioSender).deliverSmsVerification(anyString(), anyString(), anyString()); doThrow(new TwilioRestException("bar", 405)).when(twilioSender).deliverVoxVerification(anyString(), anyString()); } public void testNexmoSmsFallback() throws IOException, TwilioRestException { SmsSender smsSender = new SmsSender(twilioSender, Optional.of(nexmoSender), true); - smsSender.deliverSmsVerification("+442223334444", "123-456"); + smsSender.deliverSmsVerification("+442223334444", null, "123-456"); - verify(nexmoSender).deliverSmsVerification("+442223334444", "123-456"); - verify(twilioSender).deliverSmsVerification("+442223334444", "123-456"); + verify(nexmoSender).deliverSmsVerification("+442223334444", null, "123-456"); + verify(twilioSender).deliverSmsVerification("+442223334444", null, "123-456"); } public void testNexmoVoxFallback() throws IOException, TwilioRestException {