From 97b7b4a501d92f34f7b0e3eb95df9d94d12be65b Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Mon, 21 Sep 2020 10:26:22 -0400 Subject: [PATCH] Fix crash when receiving SMS before finishing registration. If someone has set Signal as the default SMS but has cleared data or otherwise reset the app's storage state, it can get into a weird situation. Notably, it'll crash because SmsReceiveJob.onRun() expects Recipient.self() to be available. However, it also makes it impossible to get the registration SMS, because the app won't post a notification for the code. This change will post notifications and SmsRetriever broadcasts for relevant SMS messages. --- .../securesms/jobs/SmsReceiveJob.java | 83 ++++++++++++++++--- .../notifications/NotificationIds.java | 1 + .../RegistrationNavigationActivity.java | 2 +- .../service/CodeVerificationRequest.java | 1 - .../service/RegistrationCodeRequest.java | 2 - .../service/VerificationCodeParser.java | 6 +- .../securesms/util/TextSecurePreferences.java | 9 -- .../service/VerificationCodeParserTest.java | 2 +- 8 files changed, 77 insertions(+), 29 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/SmsReceiveJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/SmsReceiveJob.java index c0190587e5..6c9b9a5291 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/SmsReceiveJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/SmsReceiveJob.java @@ -2,9 +2,19 @@ package org.thoughtcrime.securesms.jobs; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.core.app.NotificationCompat; +import androidx.core.app.Person; + +import android.app.Notification; +import android.app.NotificationManager; +import android.content.Context; +import android.content.Intent; import android.telephony.SmsMessage; -import org.thoughtcrime.securesms.database.Database; +import com.google.android.gms.auth.api.phone.SmsRetriever; +import com.google.android.gms.common.api.Status; + +import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.database.MessageDatabase; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; import org.thoughtcrime.securesms.jobmanager.Data; @@ -14,16 +24,21 @@ import org.thoughtcrime.securesms.logging.Log; import org.thoughtcrime.securesms.database.DatabaseFactory; import org.thoughtcrime.securesms.database.MessageDatabase.InsertResult; -import org.thoughtcrime.securesms.database.SmsDatabase; +import org.thoughtcrime.securesms.notifications.NotificationChannels; +import org.thoughtcrime.securesms.notifications.NotificationIds; import org.thoughtcrime.securesms.recipients.Recipient; +import org.thoughtcrime.securesms.service.VerificationCodeParser; import org.thoughtcrime.securesms.sms.IncomingTextMessage; +import org.thoughtcrime.securesms.transport.RetryLaterException; import org.thoughtcrime.securesms.util.Base64; +import org.thoughtcrime.securesms.util.ServiceUtil; import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.whispersystems.libsignal.util.guava.Optional; import java.io.IOException; import java.util.LinkedList; import java.util.List; +import java.util.concurrent.TimeUnit; public class SmsReceiveJob extends BaseJob { @@ -41,7 +56,7 @@ public class SmsReceiveJob extends BaseJob { public SmsReceiveJob(@Nullable Object[] pdus, int subscriptionId) { this(new Job.Parameters.Builder() .addConstraint(SqlCipherMigrationConstraint.KEY) - .setMaxAttempts(25) + .setLifespan(TimeUnit.DAYS.toMillis(1)) .build(), pdus, subscriptionId); @@ -72,13 +87,36 @@ public class SmsReceiveJob extends BaseJob { } @Override - public void onRun() throws MigrationPendingException { - if (TextSecurePreferences.getLocalUuid(context) == null && TextSecurePreferences.getLocalNumber(context) == null) { - throw new NotReadyException(); - } - + public void onRun() throws MigrationPendingException, RetryLaterException { Optional message = assembleMessageFragments(pdus, subscriptionId); + if (TextSecurePreferences.getLocalUuid(context) == null && TextSecurePreferences.getLocalNumber(context) == null) { + Log.i(TAG, "Received an SMS before we're registered..."); + + if (message.isPresent()) { + Optional token = VerificationCodeParser.parse(message.get().getMessageBody()); + + if (token.isPresent()) { + Log.i(TAG, "Received something that looks like a registration SMS. Posting a notification and broadcast."); + + NotificationManager manager = ServiceUtil.getNotificationManager(context); + Notification notification = buildPreRegistrationNotification(context, message.get()); + manager.notify(NotificationIds.PRE_REGISTRATION_SMS, notification); + + Intent smsRetrieverIntent = buildSmsRetrieverIntent(message.get()); + context.sendBroadcast(smsRetrieverIntent); + + return; + } else { + Log.w(TAG, "Received an SMS before registration is complete. We'll try again later."); + throw new RetryLaterException(); + } + } else { + Log.w(TAG, "Received an SMS before registration is complete, but couldn't assemble the message anyway. Ignoring."); + return; + } + } + if (message.isPresent() && !isBlocked(message.get())) { Optional insertResult = storeMessage(message.get()); @@ -99,7 +137,8 @@ public class SmsReceiveJob extends BaseJob { @Override public boolean onShouldRetry(@NonNull Exception exception) { - return exception instanceof MigrationPendingException; + return exception instanceof MigrationPendingException || + exception instanceof RetryLaterException; } private boolean isBlocked(IncomingTextMessage message) { @@ -150,7 +189,29 @@ public class SmsReceiveJob extends BaseJob { return Optional.of(new IncomingTextMessage(messages)); } - private class MigrationPendingException extends Exception { + private static Notification buildPreRegistrationNotification(@NonNull Context context, @NonNull IncomingTextMessage message) { + Recipient sender = Recipient.resolved(message.getSender()); + + return new NotificationCompat.Builder(context, NotificationChannels.getMessagesChannel(context)) + .setStyle(new NotificationCompat.MessagingStyle(new Person.Builder() + .setName(sender.getE164().or("")) + .build()) + .addMessage(new NotificationCompat.MessagingStyle.Message(message.getMessageBody(), + message.getSentTimestampMillis(), + (Person) null))) + .setSmallIcon(R.drawable.ic_notification) + .build(); + } + + /** + * @return An intent that is identical to the one the {@link SmsRetriever} API uses, so that + * we can auto-populate the SMS code on capable devices. + */ + private static Intent buildSmsRetrieverIntent(@NonNull IncomingTextMessage message) { + Intent intent = new Intent(SmsRetriever.SMS_RETRIEVED_ACTION); + intent.putExtra(SmsRetriever.EXTRA_STATUS, Status.RESULT_SUCCESS); + intent.putExtra(SmsRetriever.EXTRA_SMS_MESSAGE, message.getMessageBody()); + return intent; } public static final class Factory implements Job.Factory { @@ -172,6 +233,6 @@ public class SmsReceiveJob extends BaseJob { } } - private static class NotReadyException extends RuntimeException { + private class MigrationPendingException extends Exception { } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/notifications/NotificationIds.java b/app/src/main/java/org/thoughtcrime/securesms/notifications/NotificationIds.java index 1c91a34ab2..9a966ac7a5 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/notifications/NotificationIds.java +++ b/app/src/main/java/org/thoughtcrime/securesms/notifications/NotificationIds.java @@ -7,6 +7,7 @@ public final class NotificationIds { public static final int MESSAGE_SUMMARY = 1338; public static final int APPLICATION_MIGRATION = 4242; public static final int SMS_IMPORT_COMPLETE = 31337; + public static final int PRE_REGISTRATION_SMS = 5050; public static final int THREAD = 50000; private NotificationIds() { } diff --git a/app/src/main/java/org/thoughtcrime/securesms/registration/RegistrationNavigationActivity.java b/app/src/main/java/org/thoughtcrime/securesms/registration/RegistrationNavigationActivity.java index 0263f1408e..a1edfa6199 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/registration/RegistrationNavigationActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/registration/RegistrationNavigationActivity.java @@ -77,7 +77,7 @@ public final class RegistrationNavigationActivity extends AppCompatActivity { switch (status.getStatusCode()) { case CommonStatusCodes.SUCCESS: - Optional code = VerificationCodeParser.parse(context, (String) extras.get(SmsRetriever.EXTRA_SMS_MESSAGE)); + Optional code = VerificationCodeParser.parse((String) extras.get(SmsRetriever.EXTRA_SMS_MESSAGE)); if (code.isPresent()) { Log.i(TAG, "Received verification code."); handleVerificationCodeReceived(code.get()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/registration/service/CodeVerificationRequest.java b/app/src/main/java/org/thoughtcrime/securesms/registration/service/CodeVerificationRequest.java index 12e5483ea7..c6088ba432 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/registration/service/CodeVerificationRequest.java +++ b/app/src/main/java/org/thoughtcrime/securesms/registration/service/CodeVerificationRequest.java @@ -258,7 +258,6 @@ public final class CodeVerificationRequest { identityKey.getPublicKey(), IdentityDatabase.VerifiedStatus.VERIFIED, true, System.currentTimeMillis(), true); - TextSecurePreferences.setVerifying(context, false); TextSecurePreferences.setPushRegistered(context, true); TextSecurePreferences.setPushServerPassword(context, credentials.getPassword()); TextSecurePreferences.setSignedPreKeyRegistered(context, true); diff --git a/app/src/main/java/org/thoughtcrime/securesms/registration/service/RegistrationCodeRequest.java b/app/src/main/java/org/thoughtcrime/securesms/registration/service/RegistrationCodeRequest.java index 3be999e8c8..1cae0bf82f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/registration/service/RegistrationCodeRequest.java +++ b/app/src/main/java/org/thoughtcrime/securesms/registration/service/RegistrationCodeRequest.java @@ -77,8 +77,6 @@ public final class RegistrationCodeRequest { } private static void markAsVerifying(Context context) { - TextSecurePreferences.setVerifying(context, true); - TextSecurePreferences.setPushRegistered(context, false); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/service/VerificationCodeParser.java b/app/src/main/java/org/thoughtcrime/securesms/service/VerificationCodeParser.java index be1693b3d1..2296146047 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/service/VerificationCodeParser.java +++ b/app/src/main/java/org/thoughtcrime/securesms/service/VerificationCodeParser.java @@ -16,9 +16,7 @@ */ package org.thoughtcrime.securesms.service; -import android.content.Context; -import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.whispersystems.libsignal.util.guava.Optional; import java.util.regex.Matcher; @@ -28,14 +26,14 @@ public class VerificationCodeParser { private static final Pattern CHALLENGE_PATTERN = Pattern.compile("(.*\\D|^)([0-9]{3,4})-([0-9]{3,4}).*", Pattern.DOTALL); - public static Optional parse(Context context, String messageBody) { + public static Optional parse(String messageBody) { if (messageBody == null) { return Optional.absent(); } Matcher challengeMatcher = CHALLENGE_PATTERN.matcher(messageBody); - if (!challengeMatcher.matches() || !TextSecurePreferences.isVerifying(context)) { + if (!challengeMatcher.matches()) { return Optional.absent(); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/TextSecurePreferences.java b/app/src/main/java/org/thoughtcrime/securesms/util/TextSecurePreferences.java index 43d672b857..8c388ec075 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/TextSecurePreferences.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/TextSecurePreferences.java @@ -75,7 +75,6 @@ public class TextSecurePreferences { private static final String LOCAL_NUMBER_PREF = "pref_local_number"; private static final String LOCAL_UUID_PREF = "pref_local_uuid"; private static final String LOCAL_USERNAME_PREF = "pref_local_username"; - private static final String VERIFYING_STATE_PREF = "pref_verifying"; public static final String REGISTERED_GCM_PREF = "pref_gcm_registered"; private static final String GCM_PASSWORD_PREF = "pref_gcm_password"; private static final String SEEN_WELCOME_SCREEN_PREF = "pref_seen_welcome_screen"; @@ -840,14 +839,6 @@ public class TextSecurePreferences { return getStringPreference(context, THEME_PREF, DynamicTheme.systemThemeAvailable() ? DynamicTheme.SYSTEM : DynamicTheme.LIGHT); } - public static boolean isVerifying(Context context) { - return getBooleanPreference(context, VERIFYING_STATE_PREF, false); - } - - public static void setVerifying(Context context, boolean verifying) { - setBooleanPreference(context, VERIFYING_STATE_PREF, verifying); - } - public static boolean isPushRegistered(Context context) { return getBooleanPreference(context, REGISTERED_GCM_PREF, false); } diff --git a/app/src/test/java/org/thoughtcrime/securesms/service/VerificationCodeParserTest.java b/app/src/test/java/org/thoughtcrime/securesms/service/VerificationCodeParserTest.java index ee0ce89dd8..0b5ee75abe 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/service/VerificationCodeParserTest.java +++ b/app/src/test/java/org/thoughtcrime/securesms/service/VerificationCodeParserTest.java @@ -64,7 +64,7 @@ public class VerificationCodeParserTest extends BaseUnitTest { @Test public void testChallenges() { for (String[] challenge : challenges()) { - Optional result = VerificationCodeParser.parse(context, challenge[0]); + Optional result = VerificationCodeParser.parse(challenge[0]); assertTrue(result.isPresent()); assertEquals(challenge[1], result.get()); }