Allow registration via recovery password for reglock enabled accounts

This commit is contained in:
Katherine Yen 2023-04-20 09:21:04 -07:00 committed by GitHub
parent 6dfdbeb7bb
commit 4fb89360ce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 26 additions and 13 deletions

View File

@ -148,9 +148,15 @@ public class RegistrationLockVerificationManager {
updatedAccount = account; updatedAccount = account;
} }
// This will often be a no-op, since the recovery password is deleted when there's a verified session. // The client often sends an empty registration lock token on the first request
// However, this covers the case where a user re-registers with SMS bypass and then forgets their PIN. // and sends an actual token if the server returns a 423 indicating that one is required.
registrationRecoveryPasswordsManager.removeForNumber(updatedAccount.getNumber()); // This logic accounts for that behavior by not deleting the registration recovery password
// if the user verified correctly via registration recovery password and sent an empty token.
// This allows users to re-register via registration recovery password
// instead of always being forced to fall back to SMS verification.
if (!phoneVerificationType.equals(PhoneVerificationRequest.VerificationType.RECOVERY_PASSWORD) || clientRegistrationLock != null) {
registrationRecoveryPasswordsManager.removeForNumber(updatedAccount.getNumber());
}
final List<Long> deviceIds = updatedAccount.getDevices().stream().map(Device::getId).toList(); final List<Long> deviceIds = updatedAccount.getDevices().stream().map(Device::getId).toList();
clientPresenceManager.disconnectAllPresences(updatedAccount.getUuid(), deviceIds); clientPresenceManager.disconnectAllPresences(updatedAccount.getUuid(), deviceIds);

View File

@ -84,21 +84,26 @@ class RegistrationLockVerificationManagerTest {
@ParameterizedTest @ParameterizedTest
@MethodSource @MethodSource
void testErrors(RegistrationLockError error, boolean alreadyLocked) throws Exception { void testErrors(RegistrationLockError error,
PhoneVerificationRequest.VerificationType verificationType,
@Nullable String clientRegistrationLock,
boolean alreadyLocked) throws Exception {
when(existingRegistrationLock.getStatus()).thenReturn(StoredRegistrationLock.Status.REQUIRED); when(existingRegistrationLock.getStatus()).thenReturn(StoredRegistrationLock.Status.REQUIRED);
when(account.hasLockedCredentials()).thenReturn(alreadyLocked); when(account.hasLockedCredentials()).thenReturn(alreadyLocked);
doThrow(new NotPushRegisteredException()).when(pushNotificationManager).sendAttemptLoginNotification(any(), any()); doThrow(new NotPushRegisteredException()).when(pushNotificationManager).sendAttemptLoginNotification(any(), any());
final String submittedRegistrationLock = "reglock";
final Pair<Class<? extends Exception>, Consumer<Exception>> exceptionType = switch (error) { final Pair<Class<? extends Exception>, Consumer<Exception>> exceptionType = switch (error) {
case MISMATCH -> { case MISMATCH -> {
when(existingRegistrationLock.verify(submittedRegistrationLock)).thenReturn(false); when(existingRegistrationLock.verify(clientRegistrationLock)).thenReturn(false);
yield new Pair<>(WebApplicationException.class, e -> { yield new Pair<>(WebApplicationException.class, e -> {
if (e instanceof WebApplicationException wae) { if (e instanceof WebApplicationException wae) {
assertEquals(RegistrationLockVerificationManager.FAILURE_HTTP_STATUS, wae.getResponse().getStatus()); assertEquals(RegistrationLockVerificationManager.FAILURE_HTTP_STATUS, wae.getResponse().getStatus());
verify(registrationRecoveryPasswordsManager).removeForNumber(account.getNumber()); if (!verificationType.equals(PhoneVerificationRequest.VerificationType.RECOVERY_PASSWORD) || clientRegistrationLock != null) {
verify(registrationRecoveryPasswordsManager).removeForNumber(account.getNumber());
} else {
verify(registrationRecoveryPasswordsManager, never()).removeForNumber(account.getNumber());
}
verify(clientPresenceManager).disconnectAllPresences(account.getUuid(), List.of(Device.MASTER_ID)); verify(clientPresenceManager).disconnectAllPresences(account.getUuid(), List.of(Device.MASTER_ID));
try { try {
verify(pushNotificationManager).sendAttemptLoginNotification(any(), eq("failedRegistrationLock")); verify(pushNotificationManager).sendAttemptLoginNotification(any(), eq("failedRegistrationLock"));
@ -128,18 +133,20 @@ class RegistrationLockVerificationManagerTest {
}; };
final Exception e = assertThrows(exceptionType.first(), () -> final Exception e = assertThrows(exceptionType.first(), () ->
registrationLockVerificationManager.verifyRegistrationLock(account, submittedRegistrationLock, registrationLockVerificationManager.verifyRegistrationLock(account, clientRegistrationLock,
"Signal-Android/4.68.3", RegistrationLockVerificationManager.Flow.REGISTRATION, "Signal-Android/4.68.3", RegistrationLockVerificationManager.Flow.REGISTRATION,
PhoneVerificationRequest.VerificationType.SESSION)); verificationType));
exceptionType.second().accept(e); exceptionType.second().accept(e);
} }
static Stream<Arguments> testErrors() { static Stream<Arguments> testErrors() {
return Stream.of( return Stream.of(
Arguments.of(RegistrationLockError.MISMATCH, true), Arguments.of(RegistrationLockError.MISMATCH, PhoneVerificationRequest.VerificationType.SESSION, "reglock", true),
Arguments.of(RegistrationLockError.MISMATCH, false), Arguments.of(RegistrationLockError.MISMATCH, PhoneVerificationRequest.VerificationType.SESSION, "reglock", false),
Arguments.of(RegistrationLockError.RATE_LIMITED, false) Arguments.of(RegistrationLockError.MISMATCH, PhoneVerificationRequest.VerificationType.RECOVERY_PASSWORD, "reglock", false),
Arguments.of(RegistrationLockError.MISMATCH, PhoneVerificationRequest.VerificationType.RECOVERY_PASSWORD, null, false),
Arguments.of(RegistrationLockError.RATE_LIMITED, PhoneVerificationRequest.VerificationType.SESSION, "reglock", false)
); );
} }