Lock account and send notification when someone passes phone verification but fails reglock

service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java

@@ -552,7 +552,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
         config.getDynamoDbTables().getSubscriptions().getTableName(), dynamoDbAsyncClient);
 
     final RegistrationLockVerificationManager registrationLockVerificationManager = new RegistrationLockVerificationManager(
-        accountsManager, clientPresenceManager, backupCredentialsGenerator, rateLimiters);
+        accountsManager, clientPresenceManager, backupCredentialsGenerator, registrationRecoveryPasswordsManager, pushNotificationManager, rateLimiters);
     final PhoneVerificationTokenManager phoneVerificationTokenManager = new PhoneVerificationTokenManager(
         registrationServiceClient, registrationRecoveryPasswordsManager);
 

service/src/main/java/org/whispersystems/textsecuregcm/auth/RegistrationLockVerificationManager.java

@@ -21,11 +21,16 @@ import org.whispersystems.textsecuregcm.entities.RegistrationLockFailure;
 import org.whispersystems.textsecuregcm.limits.RateLimiters;
 import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
 import org.whispersystems.textsecuregcm.push.ClientPresenceManager;
+import org.whispersystems.textsecuregcm.push.NotPushRegisteredException;
+import org.whispersystems.textsecuregcm.push.PushNotificationManager;
 import org.whispersystems.textsecuregcm.storage.Account;
 import org.whispersystems.textsecuregcm.storage.AccountsManager;
+import org.whispersystems.textsecuregcm.storage.Device;
+import org.whispersystems.textsecuregcm.storage.RegistrationRecoveryPasswordsManager;
 import org.whispersystems.textsecuregcm.util.Util;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.List;
 
 public class RegistrationLockVerificationManager {
   public enum Flow {
@@ -40,23 +45,31 @@ public class RegistrationLockVerificationManager {
       name(RegistrationLockVerificationManager.class, "expiredRegistrationLock");
   private static final String REQUIRED_REGISTRATION_LOCK_COUNTER_NAME =
       name(RegistrationLockVerificationManager.class, "requiredRegistrationLock");
+  private static final String CHALLENGED_DEVICE_NOT_PUSH_REGISTERED_COUNTER_NAME =
+      name(RegistrationLockVerificationManager.class, "challengedDeviceNotPushRegistered");
   private static final String ALREADY_LOCKED_TAG_NAME = "alreadyLocked";
   private static final String REGISTRATION_LOCK_VERIFICATION_FLOW_TAG_NAME = "flow";
   private static final String REGISTRATION_LOCK_MATCHES_TAG_NAME = "registrationLockMatches";
   private static final String PHONE_VERIFICATION_TYPE_TAG_NAME = "phoneVerificationType";
 
-
   private final AccountsManager accounts;
   private final ClientPresenceManager clientPresenceManager;
   private final ExternalServiceCredentialsGenerator backupServiceCredentialGenerator;
   private final RateLimiters rateLimiters;
+  private final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager;
+  private final PushNotificationManager pushNotificationManager;
 
   public RegistrationLockVerificationManager(
       final AccountsManager accounts, final ClientPresenceManager clientPresenceManager,
-      final ExternalServiceCredentialsGenerator backupServiceCredentialGenerator, final RateLimiters rateLimiters) {
+      final ExternalServiceCredentialsGenerator backupServiceCredentialGenerator,
+      final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager,
+      final PushNotificationManager pushNotificationManager,
+      final RateLimiters rateLimiters) {
     this.accounts = accounts;
     this.clientPresenceManager = clientPresenceManager;
     this.backupServiceCredentialGenerator = backupServiceCredentialGenerator;
+    this.registrationRecoveryPasswordsManager = registrationRecoveryPasswordsManager;
+    this.pushNotificationManager = pushNotificationManager;
     this.rateLimiters = rateLimiters;
   }
 
@@ -125,20 +138,29 @@ public class RegistrationLockVerificationManager {
       // Freezing the existing account credentials will definitively start the reglock timeout.
       // Until the timeout, the current reglock can still be supplied,
       // along with phone number verification, to restore access.
-      /*
+      final ExternalServiceCredentials existingBackupCredentials =
+          backupServiceCredentialGenerator.generateForUuid(account.getUuid());
 
       final Account updatedAccount;
       if (!alreadyLocked) {
-        updatedAccount = accounts.update(existingAccount, Account::lockAuthenticationCredentials);
+        updatedAccount = accounts.update(account, Account::lockAuthTokenHash);
       } else {
-        updatedAccount = existingAccount;
+        updatedAccount = account;
       }
 
-      List<Long> deviceIds = updatedAccount.getDevices().stream().map(Device::getId).toList();
+      // This will often be a no-op, since the recovery password is deleted when there's a verified session.
+      // However, this covers the case where a user re-registers with SMS bypass and then forgets their PIN.
+      registrationRecoveryPasswordsManager.removeForNumber(updatedAccount.getNumber());
+
+      final List<Long> deviceIds = updatedAccount.getDevices().stream().map(Device::getId).toList();
       clientPresenceManager.disconnectAllPresences(updatedAccount.getUuid(), deviceIds);
-      */
+
-      final ExternalServiceCredentials existingBackupCredentials =
+      try {
-          backupServiceCredentialGenerator.generateForUuid(account.getUuid());
+        // Send a push notification that prompts the client to attempt login and fail due to locked credentials
+        pushNotificationManager.sendAttemptLoginNotification(updatedAccount, "failedRegistrationLock");
+      } catch (final NotPushRegisteredException e) {
+        Metrics.counter(CHALLENGED_DEVICE_NOT_PUSH_REGISTERED_COUNTER_NAME).increment();
+      }
 
       throw new WebApplicationException(Response.status(FAILURE_HTTP_STATUS)
           .entity(new RegistrationLockFailure(existingRegistrationLock.getTimeRemaining().toMillis(),

service/src/main/java/org/whispersystems/textsecuregcm/push/APNSender.java

@@ -96,6 +96,17 @@ public class APNSender implements Managed, PushNotificationSender {
         }
       }
 
+      case ATTEMPT_LOGIN_NOTIFICATION_HIGH_PRIORITY -> new SimpleApnsPayloadBuilder()
+          .setMutableContent(true)
+          .setLocalizedAlertMessage("APN_Message")
+          .addCustomProperty("attemptLoginContext", notification.data())
+          .build();
+
+      case ATTEMPT_LOGIN_NOTIFICATION_LOW_PRIORITY -> new SimpleApnsPayloadBuilder()
+          .setContentAvailable(true)
+          .addCustomProperty("attemptLoginContext", notification.data())
+          .build();
+
       case CHALLENGE -> new SimpleApnsPayloadBuilder()
           .setSound("default")
           .setLocalizedAlertMessage("APN_Message")

service/src/main/java/org/whispersystems/textsecuregcm/push/FcmSender.java

@@ -89,6 +89,7 @@ public class FcmSender implements PushNotificationSender {
 
     final String key = switch (pushNotification.notificationType()) {
       case NOTIFICATION -> "notification";
+      case ATTEMPT_LOGIN_NOTIFICATION_HIGH_PRIORITY, ATTEMPT_LOGIN_NOTIFICATION_LOW_PRIORITY -> "attemptLoginContext";
       case CHALLENGE -> "challenge";
       case RATE_LIMIT_CHALLENGE -> "rateLimitChallenge";
     };

service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotification.java

@@ -18,7 +18,11 @@ public record PushNotification(String deviceToken,
                                boolean urgent) {
 
   public enum NotificationType {
-    NOTIFICATION, CHALLENGE, RATE_LIMIT_CHALLENGE
+    NOTIFICATION,
+    ATTEMPT_LOGIN_NOTIFICATION_HIGH_PRIORITY,
+    @Deprecated ATTEMPT_LOGIN_NOTIFICATION_LOW_PRIORITY, // Temporary support for iOS clients; can be removed after 2023-06-12
+    CHALLENGE,
+    RATE_LIMIT_CHALLENGE
   }
 
   public enum TokenType {

service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotificationManager.java

@@ -78,6 +78,22 @@ public class PushNotificationManager {
         PushNotification.NotificationType.RATE_LIMIT_CHALLENGE, challengeToken, destination, device, true));
   }
 
+  public void sendAttemptLoginNotification(final Account destination, final String context) throws NotPushRegisteredException {
+    final Device device = destination.getDevice(Device.MASTER_ID).orElseThrow(NotPushRegisteredException::new);
+    final Pair<String, PushNotification.TokenType> tokenAndType = getToken(device);
+
+    sendNotification(new PushNotification(tokenAndType.first(), tokenAndType.second(),
+        PushNotification.NotificationType.ATTEMPT_LOGIN_NOTIFICATION_HIGH_PRIORITY,
+        context, destination, device, true));
+
+    // This is a workaround for older iOS clients who need a low priority push to trigger the logout notification
+    if (tokenAndType.second() == PushNotification.TokenType.APN) {
+      sendNotification(new PushNotification(tokenAndType.first(), tokenAndType.second(),
+          PushNotification.NotificationType.ATTEMPT_LOGIN_NOTIFICATION_LOW_PRIORITY,
+          context, destination, device, false));
+    }
+  }
+
   public void handleMessagesRetrieved(final Account account, final Device device, final String userAgent) {
     RedisOperation.unchecked(() -> pushLatencyManager.recordQueueRead(account.getUuid(), device.getId(), userAgent));
     apnPushNotificationScheduler.cancelScheduledNotifications(account, device).whenComplete(logErrors());

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

@@ -14,6 +14,7 @@ import java.util.concurrent.CompletableFuture;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.whispersystems.textsecuregcm.auth.SaltedTokenHash;
+import software.amazon.awssdk.services.dynamodb.model.ResourceNotFoundException;
 
 public class RegistrationRecoveryPasswordsManager {
 
@@ -53,7 +54,10 @@ public class RegistrationRecoveryPasswordsManager {
     // there is no action to be taken on its completion
     return registrationRecoveryPasswords.removeEntry(number)
         .whenComplete((ignored, error) -> {
-          if (error != null) {
+          if (error instanceof ResourceNotFoundException) {
+            // These will naturally happen if a recovery password is already deleted. Since we can remove
+            // the recovery password through many flows, we avoid creating log messages for these exceptions
+          } else if (error != null) {
             logger.warn("Failed to remove Registration Recovery Password", error);
           }
         });

service/src/test/java/org/whispersystems/textsecuregcm/auth/RegistrationLockVerificationManagerTest.java

@@ -11,10 +11,15 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.util.List;
 import java.util.UUID;
 import java.util.function.Consumer;
 import java.util.stream.Stream;
@@ -23,15 +28,19 @@ import javax.ws.rs.WebApplicationException;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
-import org.junit.jupiter.params.provider.EnumSource;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException;
 import org.whispersystems.textsecuregcm.entities.PhoneVerificationRequest;
 import org.whispersystems.textsecuregcm.limits.RateLimiter;
 import org.whispersystems.textsecuregcm.limits.RateLimiters;
 import org.whispersystems.textsecuregcm.push.ClientPresenceManager;
+import org.whispersystems.textsecuregcm.push.NotPushRegisteredException;
+import org.whispersystems.textsecuregcm.push.PushNotificationManager;
 import org.whispersystems.textsecuregcm.storage.Account;
 import org.whispersystems.textsecuregcm.storage.AccountsManager;
+import org.whispersystems.textsecuregcm.storage.Device;
+import org.whispersystems.textsecuregcm.storage.RegistrationRecoveryPasswordsManager;
+import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
 import org.whispersystems.textsecuregcm.util.Pair;
 
 class RegistrationLockVerificationManagerTest {
@@ -40,9 +49,12 @@ class RegistrationLockVerificationManagerTest {
   private final ClientPresenceManager clientPresenceManager = mock(ClientPresenceManager.class);
   private final ExternalServiceCredentialsGenerator backupServiceCredentialsGeneraor = mock(
       ExternalServiceCredentialsGenerator.class);
+  private final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager = mock(
+      RegistrationRecoveryPasswordsManager.class);
+  private static PushNotificationManager pushNotificationManager = mock(PushNotificationManager.class);
   private final RateLimiters rateLimiters = mock(RateLimiters.class);
   private final RegistrationLockVerificationManager registrationLockVerificationManager = new RegistrationLockVerificationManager(
-      accountsManager, clientPresenceManager, backupServiceCredentialsGeneraor, rateLimiters);
+      accountsManager, clientPresenceManager, backupServiceCredentialsGeneraor, registrationRecoveryPasswordsManager, pushNotificationManager, rateLimiters);
 
   private final RateLimiter pinLimiter = mock(RateLimiter.class);
 
@@ -51,22 +63,32 @@ class RegistrationLockVerificationManagerTest {
 
   @BeforeEach
   void setUp() {
+    clearInvocations(pushNotificationManager);
     when(rateLimiters.getPinLimiter()).thenReturn(pinLimiter);
     when(backupServiceCredentialsGeneraor.generateForUuid(any()))
         .thenReturn(mock(ExternalServiceCredentials.class));
 
+    final Device device = mock(Device.class);
+    when(device.getId()).thenReturn(Device.MASTER_ID);
+
+    AccountsHelper.setupMockUpdate(accountsManager);
+
     account = mock(Account.class);
     when(account.getUuid()).thenReturn(UUID.randomUUID());
     when(account.getNumber()).thenReturn("+18005551212");
+    when(account.getDevices()).thenReturn(List.of(device));
+
     existingRegistrationLock = mock(StoredRegistrationLock.class);
     when(account.getRegistrationLock()).thenReturn(existingRegistrationLock);
   }
 
   @ParameterizedTest
-  @EnumSource
+  @MethodSource
-  void testErrors(RegistrationLockError error) throws Exception {
+  void testErrors(RegistrationLockError error, boolean alreadyLocked) throws Exception {
 
     when(existingRegistrationLock.getStatus()).thenReturn(StoredRegistrationLock.Status.REQUIRED);
+    when(account.hasLockedCredentials()).thenReturn(alreadyLocked);
+    doThrow(new NotPushRegisteredException()).when(pushNotificationManager).sendAttemptLoginNotification(any(), any());
 
     final String submittedRegistrationLock = "reglock";
 
@@ -76,6 +98,16 @@ class RegistrationLockVerificationManagerTest {
         yield new Pair<>(WebApplicationException.class, e -> {
           if (e instanceof WebApplicationException wae) {
             assertEquals(RegistrationLockVerificationManager.FAILURE_HTTP_STATUS, wae.getResponse().getStatus());
+            verify(registrationRecoveryPasswordsManager).removeForNumber(account.getNumber());
+            verify(clientPresenceManager).disconnectAllPresences(account.getUuid(), List.of(Device.MASTER_ID));
+            try {
+              verify(pushNotificationManager).sendAttemptLoginNotification(any(), eq("failedRegistrationLock"));
+            } catch (NotPushRegisteredException npre) {}
+            if (alreadyLocked) {
+              verify(account, never()).lockAuthTokenHash();
+            } else {
+              verify(account).lockAuthTokenHash();
+            }
           } else {
             fail("Exception was not of expected type");
           }
@@ -85,6 +117,12 @@ class RegistrationLockVerificationManagerTest {
         when(existingRegistrationLock.verify(any())).thenReturn(true);
         doThrow(RateLimitExceededException.class).when(pinLimiter).validate(anyString());
         yield new Pair<>(RateLimitExceededException.class, ignored -> {
+          verify(account, never()).lockAuthTokenHash();
+          try {
+            verify(pushNotificationManager, never()).sendAttemptLoginNotification(any(), eq("failedRegistrationLock"));
+          } catch (NotPushRegisteredException npre) {}
+          verify(registrationRecoveryPasswordsManager, never()).removeForNumber(account.getNumber());
+          verify(clientPresenceManager, never()).disconnectAllPresences(account.getUuid(), List.of(Device.MASTER_ID));
         });
       }
     };
@@ -97,6 +135,14 @@ class RegistrationLockVerificationManagerTest {
     exceptionType.second().accept(e);
   }
 
+  static Stream<Arguments> testErrors() {
+    return Stream.of(
+        Arguments.of(RegistrationLockError.MISMATCH, true),
+        Arguments.of(RegistrationLockError.MISMATCH, false),
+        Arguments.of(RegistrationLockError.RATE_LIMITED, false)
+    );
+  }
+
   @ParameterizedTest
   @MethodSource
   void testSuccess(final StoredRegistrationLock.Status status, @Nullable final String submittedRegistrationLock) {
@@ -109,6 +155,10 @@ class RegistrationLockVerificationManagerTest {
         () -> registrationLockVerificationManager.verifyRegistrationLock(account, submittedRegistrationLock,
             "Signal-Android/4.68.3", RegistrationLockVerificationManager.Flow.REGISTRATION,
             PhoneVerificationRequest.VerificationType.SESSION));
+
+    verify(account, never()).lockAuthTokenHash();
+    verify(registrationRecoveryPasswordsManager, never()).removeForNumber(account.getNumber());
+    verify(clientPresenceManager, never()).disconnectAllPresences(account.getUuid(), List.of(Device.MASTER_ID));
   }
 
   static Stream<Arguments> testSuccess() {

service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java

@@ -202,7 +202,8 @@ class AccountControllerTest {
       BACKUP_CFG);
 
   private static final RegistrationLockVerificationManager registrationLockVerificationManager = new RegistrationLockVerificationManager(
-      accountsManager, clientPresenceManager, backupCredentialsGenerator, rateLimiters);
+      accountsManager, clientPresenceManager, backupCredentialsGenerator, registrationRecoveryPasswordsManager,
+      pushNotificationManager, rateLimiters);
   private static final RegistrationCaptchaManager registrationCaptchaManager = new RegistrationCaptchaManager(
       captchaChecker, rateLimiters, Map.of(TEST_NUMBER, 123456), dynamicConfigurationManager);
 

service/src/test/java/org/whispersystems/textsecuregcm/push/PushNotificationManagerTest.java

@@ -9,6 +9,7 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
@@ -137,6 +138,38 @@ class PushNotificationManagerTest {
     verify(apnSender).sendNotification(new PushNotification(deviceToken, PushNotification.TokenType.APN, PushNotification.NotificationType.RATE_LIMIT_CHALLENGE, challengeToken, account, device, true));
   }
 
+  @ParameterizedTest
+  @ValueSource(booleans = {true, false})
+  void sendAttemptLoginNotification(final boolean isApn) throws NotPushRegisteredException {
+    final Account account = mock(Account.class);
+    final Device device = mock(Device.class);
+
+    final String deviceToken = "token";
+
+    when(device.getId()).thenReturn(Device.MASTER_ID);
+    if (isApn) {
+      when(device.getApnId()).thenReturn(deviceToken);
+      when(apnSender.sendNotification(any()))
+          .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, null, false)));
+    } else {
+      when(device.getGcmId()).thenReturn(deviceToken);
+      when(fcmSender.sendNotification(any()))
+          .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, null, false)));
+    }
+    when(account.getDevice(Device.MASTER_ID)).thenReturn(Optional.of(device));
+
+    pushNotificationManager.sendAttemptLoginNotification(account, "someContext");
+
+    if (isApn){
+      verify(apnSender).sendNotification(new PushNotification(deviceToken, PushNotification.TokenType.APN,
+          PushNotification.NotificationType.ATTEMPT_LOGIN_NOTIFICATION_HIGH_PRIORITY, "someContext", account, device, true));
+      verify(apnPushNotificationScheduler).scheduleBackgroundNotification(account, device);
+    } else {
+      verify(fcmSender, times(1)).sendNotification(new PushNotification(deviceToken, PushNotification.TokenType.FCM,
+          PushNotification.NotificationType.ATTEMPT_LOGIN_NOTIFICATION_HIGH_PRIORITY, "someContext", account, device, true));
+    }
+  }
+
   @ParameterizedTest
   @ValueSource(booleans = {true, false})
   void testSendNotificationFcm(final boolean urgent) {

service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java

@@ -132,6 +132,7 @@ public class AccountsHelper {
           case "getIdentityKey" -> when(updatedAccount.getIdentityKey()).thenAnswer(stubbing);
           case "getBadges" -> when(updatedAccount.getBadges()).thenAnswer(stubbing);
           case "getLastSeen" -> when(updatedAccount.getLastSeen()).thenAnswer(stubbing);
+          case "hasLockedCredentials" -> when(updatedAccount.hasLockedCredentials()).thenAnswer(stubbing);
           default -> throw new IllegalArgumentException("unsupported method: Account#" + stubbing.getInvocation().getMethod().getName());
         }
       }