mirror of
https://github.com/signalapp/Signal-Server
synced 2026-04-21 09:38:03 +01:00
Write registration recovery passwords exclusively by PNI
This commit is contained in:
committed by
Jon Chambers
parent
8be43566a4
commit
2803c2acdb
@@ -103,9 +103,9 @@ class RegistrationLockVerificationManagerTest {
|
||||
if (e instanceof WebApplicationException wae) {
|
||||
assertEquals(RegistrationLockVerificationManager.FAILURE_HTTP_STATUS, wae.getResponse().getStatus());
|
||||
if (!verificationType.equals(PhoneVerificationRequest.VerificationType.RECOVERY_PASSWORD) || clientRegistrationLock != null) {
|
||||
verify(registrationRecoveryPasswordsManager).removeForNumber(account.getNumber());
|
||||
verify(registrationRecoveryPasswordsManager).remove(account.getIdentifier(IdentityType.PNI));
|
||||
} else {
|
||||
verify(registrationRecoveryPasswordsManager, never()).removeForNumber(any());
|
||||
verify(registrationRecoveryPasswordsManager, never()).remove(any());
|
||||
}
|
||||
verify(disconnectionRequestManager).requestDisconnection(account.getUuid(), List.of(Device.PRIMARY_ID));
|
||||
try {
|
||||
@@ -133,7 +133,7 @@ class RegistrationLockVerificationManagerTest {
|
||||
} catch (final NotPushRegisteredException ignored2) {
|
||||
}
|
||||
|
||||
verify(registrationRecoveryPasswordsManager, never()).removeForNumber(any());
|
||||
verify(registrationRecoveryPasswordsManager, never()).remove(any());
|
||||
verify(disconnectionRequestManager, never()).requestDisconnection(any(), any());
|
||||
});
|
||||
}
|
||||
@@ -171,7 +171,7 @@ class RegistrationLockVerificationManagerTest {
|
||||
PhoneVerificationRequest.VerificationType.SESSION));
|
||||
|
||||
verify(account, never()).lockAuthTokenHash();
|
||||
verify(registrationRecoveryPasswordsManager, never()).removeForNumber(any());
|
||||
verify(registrationRecoveryPasswordsManager, never()).remove(any());
|
||||
verify(disconnectionRequestManager, never()).requestDisconnection(any(), any());
|
||||
}
|
||||
|
||||
|
||||
@@ -786,7 +786,7 @@ class AccountControllerTest {
|
||||
.withRecoveryPassword(recoveryPassword)))) {
|
||||
|
||||
assertThat(response.getStatus()).isEqualTo(204);
|
||||
verify(registrationRecoveryPasswordsManager).storeForCurrentNumber(eq(AuthHelper.UNDISCOVERABLE_NUMBER), eq(recoveryPassword));
|
||||
verify(registrationRecoveryPasswordsManager).store(AuthHelper.UNDISCOVERABLE_PNI, recoveryPassword);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -582,7 +582,7 @@ class VerificationControllerTest {
|
||||
assertTrue(verificationSessionResponse.verified());
|
||||
assertTrue(verificationSessionResponse.requestedInformation().isEmpty());
|
||||
|
||||
verify(registrationRecoveryPasswordsManager).removeForNumber(registrationServiceSession.number());
|
||||
verify(registrationRecoveryPasswordsManager).remove(PNI);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -879,7 +879,7 @@ class VerificationControllerTest {
|
||||
try (Response response = request.get()) {
|
||||
assertEquals(HttpStatus.SC_OK, response.getStatus());
|
||||
|
||||
verify(registrationRecoveryPasswordsManager).removeForNumber(registrationServiceSession.number());
|
||||
verify(registrationRecoveryPasswordsManager).remove(PNI);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1204,7 +1204,7 @@ class VerificationControllerTest {
|
||||
VerificationSessionResponse.class);
|
||||
assertTrue(verificationSessionResponse.verified());
|
||||
|
||||
verify(registrationRecoveryPasswordsManager).removeForNumber(registrationServiceSession.number());
|
||||
verify(registrationRecoveryPasswordsManager).remove(PNI);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1336,7 +1336,7 @@ class VerificationControllerTest {
|
||||
|
||||
assertTrue(verificationSessionResponse.verified());
|
||||
|
||||
verify(registrationRecoveryPasswordsManager).removeForNumber(verifiedSession.number());
|
||||
verify(registrationRecoveryPasswordsManager).remove(PNI);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -114,7 +114,7 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest<AccountsGrpcService, Ac
|
||||
when(rateLimiter.validateReactive(any(UUID.class))).thenReturn(Mono.empty());
|
||||
when(rateLimiter.validateReactive(anyString())).thenReturn(Mono.empty());
|
||||
|
||||
when(registrationRecoveryPasswordsManager.storeForCurrentNumber(any(), any()))
|
||||
when(registrationRecoveryPasswordsManager.store(any(), any()))
|
||||
.thenReturn(CompletableFuture.completedFuture(null));
|
||||
|
||||
return new AccountsGrpcService(accountsManager,
|
||||
@@ -700,7 +700,7 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest<AccountsGrpcService, Ac
|
||||
.setRegistrationRecoveryPassword(ByteString.copyFrom(registrationRecoveryPassword))
|
||||
.build()));
|
||||
|
||||
verify(registrationRecoveryPasswordsManager).storeForCurrentNumber(account.getNumber(), registrationRecoveryPassword);
|
||||
verify(registrationRecoveryPasswordsManager).store(account.getIdentifier(IdentityType.PNI), registrationRecoveryPassword);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -137,7 +137,7 @@ public class AccountCreationDeletionIntegrationTest {
|
||||
final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager =
|
||||
mock(RegistrationRecoveryPasswordsManager.class);
|
||||
|
||||
when(registrationRecoveryPasswordsManager.removeForNumber(any()))
|
||||
when(registrationRecoveryPasswordsManager.remove(any()))
|
||||
.thenReturn(CompletableFuture.completedFuture(null));
|
||||
|
||||
disconnectionRequestManager = mock(DisconnectionRequestManager.class);
|
||||
|
||||
@@ -130,7 +130,7 @@ class AccountsManagerChangeNumberIntegrationTest {
|
||||
final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager =
|
||||
mock(RegistrationRecoveryPasswordsManager.class);
|
||||
|
||||
when(registrationRecoveryPasswordsManager.removeForNumber(any()))
|
||||
when(registrationRecoveryPasswordsManager.remove(any()))
|
||||
.thenReturn(CompletableFuture.completedFuture(null));
|
||||
|
||||
accountsManager = new AccountsManager(
|
||||
|
||||
@@ -224,7 +224,7 @@ class AccountsManagerTest {
|
||||
final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager =
|
||||
mock(RegistrationRecoveryPasswordsManager.class);
|
||||
|
||||
when(registrationRecoveryPasswordsManager.removeForNumber(any())).thenReturn(CompletableFuture.completedFuture(null));
|
||||
when(registrationRecoveryPasswordsManager.remove(any())).thenReturn(CompletableFuture.completedFuture(null));
|
||||
|
||||
when(keysManager.deleteSingleUsePreKeys(any())).thenReturn(CompletableFuture.completedFuture(null));
|
||||
when(messagesManager.clear(any())).thenReturn(CompletableFuture.completedFuture(null));
|
||||
|
||||
@@ -137,7 +137,7 @@ public class AddRemoveDeviceIntegrationTest {
|
||||
final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager =
|
||||
mock(RegistrationRecoveryPasswordsManager.class);
|
||||
|
||||
when(registrationRecoveryPasswordsManager.removeForNumber(any()))
|
||||
when(registrationRecoveryPasswordsManager.remove(any()))
|
||||
.thenReturn(CompletableFuture.completedFuture(null));
|
||||
|
||||
PUBSUB_SERVER_EXTENSION.getRedisClient().useConnection(connection -> {
|
||||
|
||||
@@ -33,16 +33,14 @@ public class RegistrationRecoveryTest {
|
||||
|
||||
private static final MutableClock CLOCK = MockUtils.mutableClock(0);
|
||||
private static final Duration EXPIRATION = Duration.ofSeconds(1000);
|
||||
private static final String NUMBER = "+18005555555";
|
||||
private static final UUID PNI = UUID.randomUUID();
|
||||
|
||||
private static final SaltedTokenHash ORIGINAL_HASH = SaltedTokenHash.generateFor("pass1");
|
||||
private static final SaltedTokenHash ANOTHER_HASH = SaltedTokenHash.generateFor("pass2");
|
||||
|
||||
@RegisterExtension
|
||||
private static final DynamoDbExtension DYNAMO_DB_EXTENSION =
|
||||
new DynamoDbExtension(Tables.PNI, Tables.REGISTRATION_RECOVERY_PASSWORDS);
|
||||
|
||||
private UUID pni;
|
||||
new DynamoDbExtension(Tables.REGISTRATION_RECOVERY_PASSWORDS);
|
||||
|
||||
private RegistrationRecoveryPasswords registrationRecoveryPasswords;
|
||||
|
||||
@@ -57,22 +55,18 @@ public class RegistrationRecoveryTest {
|
||||
DYNAMO_DB_EXTENSION.getDynamoDbAsyncClient(),
|
||||
CLOCK
|
||||
);
|
||||
final PhoneNumberIdentifiers phoneNumberIdentifiers =
|
||||
new PhoneNumberIdentifiers(DYNAMO_DB_EXTENSION.getDynamoDbAsyncClient(), Tables.PNI.tableName());
|
||||
|
||||
manager = new RegistrationRecoveryPasswordsManager(registrationRecoveryPasswords, phoneNumberIdentifiers);
|
||||
|
||||
pni = phoneNumberIdentifiers.getPhoneNumberIdentifier(NUMBER).join();
|
||||
manager = new RegistrationRecoveryPasswordsManager(registrationRecoveryPasswords);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLookupAfterWrite() throws Exception {
|
||||
registrationRecoveryPasswords.addOrReplace(NUMBER, pni, ORIGINAL_HASH).get();
|
||||
final long initialExp = fetchTimestamp(NUMBER);
|
||||
registrationRecoveryPasswords.addOrReplace(PNI, ORIGINAL_HASH).get();
|
||||
final long initialExp = fetchTimestamp(PNI);
|
||||
final long expectedExpiration = CLOCK.instant().getEpochSecond() + EXPIRATION.getSeconds();
|
||||
assertEquals(expectedExpiration, initialExp);
|
||||
|
||||
final Optional<SaltedTokenHash> saltedTokenHashByPni = registrationRecoveryPasswords.lookup(pni).get();
|
||||
final Optional<SaltedTokenHash> saltedTokenHashByPni = registrationRecoveryPasswords.lookup(PNI).get();
|
||||
assertTrue(saltedTokenHashByPni.isPresent());
|
||||
assertEquals(ORIGINAL_HASH.salt(), saltedTokenHashByPni.get().salt());
|
||||
assertEquals(ORIGINAL_HASH.hash(), saltedTokenHashByPni.get().hash());
|
||||
@@ -80,15 +74,15 @@ public class RegistrationRecoveryTest {
|
||||
|
||||
@Test
|
||||
public void testLookupAfterRefresh() throws Exception {
|
||||
registrationRecoveryPasswords.addOrReplace(NUMBER, pni, ORIGINAL_HASH).get();
|
||||
registrationRecoveryPasswords.addOrReplace(PNI, ORIGINAL_HASH).get();
|
||||
|
||||
CLOCK.increment(50, TimeUnit.SECONDS);
|
||||
registrationRecoveryPasswords.addOrReplace(NUMBER, pni, ORIGINAL_HASH).get();
|
||||
final long updatedExp = fetchTimestamp(NUMBER);
|
||||
registrationRecoveryPasswords.addOrReplace(PNI, ORIGINAL_HASH).get();
|
||||
final long updatedExp = fetchTimestamp(PNI);
|
||||
final long expectedExp = CLOCK.instant().getEpochSecond() + EXPIRATION.getSeconds();
|
||||
assertEquals(expectedExp, updatedExp);
|
||||
|
||||
final Optional<SaltedTokenHash> saltedTokenHashByPni = registrationRecoveryPasswords.lookup(pni).get();
|
||||
final Optional<SaltedTokenHash> saltedTokenHashByPni = registrationRecoveryPasswords.lookup(PNI).get();
|
||||
assertTrue(saltedTokenHashByPni.isPresent());
|
||||
assertEquals(ORIGINAL_HASH.salt(), saltedTokenHashByPni.get().salt());
|
||||
assertEquals(ORIGINAL_HASH.hash(), saltedTokenHashByPni.get().hash());
|
||||
@@ -96,10 +90,10 @@ public class RegistrationRecoveryTest {
|
||||
|
||||
@Test
|
||||
public void testReplace() throws Exception {
|
||||
registrationRecoveryPasswords.addOrReplace(NUMBER, pni, ORIGINAL_HASH).get();
|
||||
registrationRecoveryPasswords.addOrReplace(NUMBER, pni, ANOTHER_HASH).get();
|
||||
registrationRecoveryPasswords.addOrReplace(PNI, ORIGINAL_HASH).get();
|
||||
registrationRecoveryPasswords.addOrReplace(PNI, ANOTHER_HASH).get();
|
||||
|
||||
final Optional<SaltedTokenHash> saltedTokenHashByPni = registrationRecoveryPasswords.lookup(pni).get();
|
||||
final Optional<SaltedTokenHash> saltedTokenHashByPni = registrationRecoveryPasswords.lookup(PNI).get();
|
||||
assertTrue(saltedTokenHashByPni.isPresent());
|
||||
assertEquals(ANOTHER_HASH.salt(), saltedTokenHashByPni.get().salt());
|
||||
assertEquals(ANOTHER_HASH.hash(), saltedTokenHashByPni.get().hash());
|
||||
@@ -107,13 +101,13 @@ public class RegistrationRecoveryTest {
|
||||
|
||||
@Test
|
||||
public void testRemove() throws Exception {
|
||||
assertDoesNotThrow(() -> registrationRecoveryPasswords.removeEntry(NUMBER, pni).join());
|
||||
assertDoesNotThrow(() -> registrationRecoveryPasswords.removeEntry(PNI).join());
|
||||
|
||||
registrationRecoveryPasswords.addOrReplace(NUMBER, pni, ORIGINAL_HASH).get();
|
||||
assertTrue(registrationRecoveryPasswords.lookup(pni).get().isPresent());
|
||||
registrationRecoveryPasswords.addOrReplace(PNI, ORIGINAL_HASH).get();
|
||||
assertTrue(registrationRecoveryPasswords.lookup(PNI).get().isPresent());
|
||||
|
||||
registrationRecoveryPasswords.removeEntry(NUMBER, pni).get();
|
||||
assertTrue(registrationRecoveryPasswords.lookup(pni).get().isEmpty());
|
||||
registrationRecoveryPasswords.removeEntry(PNI).get();
|
||||
assertTrue(registrationRecoveryPasswords.lookup(PNI).get().isEmpty());
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -123,31 +117,31 @@ public class RegistrationRecoveryTest {
|
||||
final byte[] wrongPassword = "qwerty123".getBytes(StandardCharsets.UTF_8);
|
||||
|
||||
// initial store
|
||||
manager.storeForCurrentNumber(NUMBER, password).get();
|
||||
assertTrue(manager.verify(pni, password).get());
|
||||
assertFalse(manager.verify(pni, wrongPassword).get());
|
||||
manager.store(PNI, password).get();
|
||||
assertTrue(manager.verify(PNI, password).get());
|
||||
assertFalse(manager.verify(PNI, wrongPassword).get());
|
||||
|
||||
// update
|
||||
manager.storeForCurrentNumber(NUMBER, password).get();
|
||||
assertTrue(manager.verify(pni, password).get());
|
||||
assertFalse(manager.verify(pni, wrongPassword).get());
|
||||
manager.store(PNI, password).get();
|
||||
assertTrue(manager.verify(PNI, password).get());
|
||||
assertFalse(manager.verify(PNI, wrongPassword).get());
|
||||
|
||||
// replace
|
||||
manager.storeForCurrentNumber(NUMBER, updatedPassword).get();
|
||||
assertTrue(manager.verify(pni, updatedPassword).get());
|
||||
assertFalse(manager.verify(pni, password).get());
|
||||
assertFalse(manager.verify(pni, wrongPassword).get());
|
||||
manager.store(PNI, updatedPassword).get();
|
||||
assertTrue(manager.verify(PNI, updatedPassword).get());
|
||||
assertFalse(manager.verify(PNI, password).get());
|
||||
assertFalse(manager.verify(PNI, wrongPassword).get());
|
||||
|
||||
manager.removeForNumber(NUMBER).get();
|
||||
assertFalse(manager.verify(pni, updatedPassword).get());
|
||||
assertFalse(manager.verify(pni, password).get());
|
||||
assertFalse(manager.verify(pni, wrongPassword).get());
|
||||
manager.remove(PNI).get();
|
||||
assertFalse(manager.verify(PNI, updatedPassword).get());
|
||||
assertFalse(manager.verify(PNI, password).get());
|
||||
assertFalse(manager.verify(PNI, wrongPassword).get());
|
||||
}
|
||||
|
||||
private static long fetchTimestamp(final String number) throws ExecutionException, InterruptedException {
|
||||
private static long fetchTimestamp(final UUID phoneNumberIdentifier) throws ExecutionException, InterruptedException {
|
||||
return DYNAMO_DB_EXTENSION.getDynamoDbAsyncClient().getItem(GetItemRequest.builder()
|
||||
.tableName(Tables.REGISTRATION_RECOVERY_PASSWORDS.tableName())
|
||||
.key(Map.of(RegistrationRecoveryPasswords.KEY_PNI, AttributeValues.fromString(number)))
|
||||
.key(Map.of(RegistrationRecoveryPasswords.KEY_PNI, AttributeValues.fromString(phoneNumberIdentifier.toString())))
|
||||
.build())
|
||||
.thenApply(getItemResponse -> {
|
||||
final Map<String, AttributeValue> item = getItemResponse.item();
|
||||
|
||||
Reference in New Issue
Block a user