Refine `RegistrationController` logic
Local device transfer on iOS uses the `409` status code to prompt the transfer UI. This needs to happen before sending a `423` and locking an existing account, since the device transfer includes the local device database verbatim.
This commit is contained in:
parent
f9fabbedce
commit
8d1135a2a3
|
@ -130,18 +130,18 @@ public class RegistrationController {
|
||||||
REREGISTRATION_IDLE_DAYS_DISTRIBUTION.record(timeSinceLastSeen.toDays());
|
REREGISTRATION_IDLE_DAYS_DISTRIBUTION.record(timeSinceLastSeen.toDays());
|
||||||
});
|
});
|
||||||
|
|
||||||
if (existingAccount.isPresent()) {
|
|
||||||
registrationLockVerificationManager.verifyRegistrationLock(existingAccount.get(),
|
|
||||||
registrationRequest.accountAttributes().getRegistrationLock(),
|
|
||||||
userAgent, RegistrationLockVerificationManager.Flow.REGISTRATION, verificationType);
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!registrationRequest.skipDeviceTransfer() && existingAccount.map(Account::isTransferSupported).orElse(false)) {
|
if (!registrationRequest.skipDeviceTransfer() && existingAccount.map(Account::isTransferSupported).orElse(false)) {
|
||||||
// If a device transfer is possible, clients must explicitly opt out of a transfer (i.e. after prompting the user)
|
// If a device transfer is possible, clients must explicitly opt out of a transfer (i.e. after prompting the user)
|
||||||
// before we'll let them create a new account "from scratch"
|
// before we'll let them create a new account "from scratch"
|
||||||
throw new WebApplicationException(Response.status(409, "device transfer available").build());
|
throw new WebApplicationException(Response.status(409, "device transfer available").build());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (existingAccount.isPresent()) {
|
||||||
|
registrationLockVerificationManager.verifyRegistrationLock(existingAccount.get(),
|
||||||
|
registrationRequest.accountAttributes().getRegistrationLock(),
|
||||||
|
userAgent, RegistrationLockVerificationManager.Flow.REGISTRATION, verificationType);
|
||||||
|
}
|
||||||
|
|
||||||
Account account = accounts.create(number, password, signalAgent, registrationRequest.accountAttributes(),
|
Account account = accounts.create(number, password, signalAgent, registrationRequest.accountAttributes(),
|
||||||
existingAccount.map(Account::getBadges).orElseGet(ArrayList::new));
|
existingAccount.map(Account::getBadges).orElseGet(ArrayList::new));
|
||||||
|
|
||||||
|
|
|
@ -23,9 +23,12 @@ import io.dropwizard.testing.junit5.ResourceExtension;
|
||||||
import java.nio.charset.StandardCharsets;
|
import java.nio.charset.StandardCharsets;
|
||||||
import java.time.Duration;
|
import java.time.Duration;
|
||||||
import java.util.Base64;
|
import java.util.Base64;
|
||||||
|
import java.util.EnumSet;
|
||||||
import java.util.HashMap;
|
import java.util.HashMap;
|
||||||
|
import java.util.HashSet;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
|
import java.util.Set;
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
import java.util.concurrent.CompletableFuture;
|
import java.util.concurrent.CompletableFuture;
|
||||||
import java.util.function.Consumer;
|
import java.util.function.Consumer;
|
||||||
|
@ -45,9 +48,10 @@ import org.junit.jupiter.api.extension.ExtendWith;
|
||||||
import org.junit.jupiter.params.ParameterizedTest;
|
import org.junit.jupiter.params.ParameterizedTest;
|
||||||
import org.junit.jupiter.params.provider.Arguments;
|
import org.junit.jupiter.params.provider.Arguments;
|
||||||
import org.junit.jupiter.params.provider.CsvSource;
|
import org.junit.jupiter.params.provider.CsvSource;
|
||||||
import org.junit.jupiter.params.provider.EnumSource;
|
|
||||||
import org.junit.jupiter.params.provider.MethodSource;
|
import org.junit.jupiter.params.provider.MethodSource;
|
||||||
import org.junit.jupiter.params.provider.ValueSource;
|
import org.junit.jupiter.params.provider.ValueSource;
|
||||||
|
import org.junitpioneer.jupiter.cartesian.ArgumentSets;
|
||||||
|
import org.junitpioneer.jupiter.cartesian.CartesianTest;
|
||||||
import org.signal.libsignal.protocol.IdentityKey;
|
import org.signal.libsignal.protocol.IdentityKey;
|
||||||
import org.signal.libsignal.protocol.ecc.Curve;
|
import org.signal.libsignal.protocol.ecc.Curve;
|
||||||
import org.signal.libsignal.protocol.ecc.ECKeyPair;
|
import org.signal.libsignal.protocol.ecc.ECKeyPair;
|
||||||
|
@ -315,33 +319,58 @@ class RegistrationControllerTest {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ParameterizedTest
|
@CartesianTest
|
||||||
@EnumSource(RegistrationLockError.class)
|
@CartesianTest.MethodFactory("registrationLockAndDeviceTransfer")
|
||||||
void registrationLock(final RegistrationLockError error) throws Exception {
|
void registrationLockAndDeviceTransfer(
|
||||||
|
final boolean deviceTransferSupported,
|
||||||
|
@Nullable final RegistrationLockError error)
|
||||||
|
throws Exception {
|
||||||
when(registrationServiceClient.getSession(any(), any()))
|
when(registrationServiceClient.getSession(any(), any()))
|
||||||
.thenReturn(
|
.thenReturn(
|
||||||
CompletableFuture.completedFuture(
|
CompletableFuture.completedFuture(
|
||||||
Optional.of(new RegistrationServiceSession(new byte[16], NUMBER, true, null, null, null,
|
Optional.of(new RegistrationServiceSession(new byte[16], NUMBER, true, null, null, null,
|
||||||
SESSION_EXPIRATION_SECONDS))));
|
SESSION_EXPIRATION_SECONDS))));
|
||||||
|
|
||||||
when(accountsManager.getByE164(any())).thenReturn(Optional.of(mock(Account.class)));
|
final Account account = mock(Account.class);
|
||||||
|
when(accountsManager.getByE164(any())).thenReturn(Optional.of(account));
|
||||||
|
when(account.isTransferSupported()).thenReturn(deviceTransferSupported);
|
||||||
|
|
||||||
final Exception e = switch (error) {
|
final int expectedStatus;
|
||||||
|
if (deviceTransferSupported) {
|
||||||
|
expectedStatus = 409;
|
||||||
|
} else if (error != null) {
|
||||||
|
final Exception e = switch (error) {
|
||||||
case MISMATCH -> new WebApplicationException(error.getExpectedStatus());
|
case MISMATCH -> new WebApplicationException(error.getExpectedStatus());
|
||||||
case RATE_LIMITED -> new RateLimitExceededException(null, true);
|
case RATE_LIMITED -> new RateLimitExceededException(null, true);
|
||||||
};
|
};
|
||||||
doThrow(e)
|
doThrow(e)
|
||||||
.when(registrationLockVerificationManager).verifyRegistrationLock(any(), any(), any(), any(), any());
|
.when(registrationLockVerificationManager).verifyRegistrationLock(any(), any(), any(), any(), any());
|
||||||
|
expectedStatus = error.getExpectedStatus();
|
||||||
|
} else {
|
||||||
|
when(accountsManager.create(any(), any(), any(), any(), any()))
|
||||||
|
.thenReturn(mock(Account.class));
|
||||||
|
expectedStatus = 200;
|
||||||
|
}
|
||||||
|
|
||||||
final Invocation.Builder request = resources.getJerseyTest()
|
final Invocation.Builder request = resources.getJerseyTest()
|
||||||
.target("/v1/registration")
|
.target("/v1/registration")
|
||||||
.request()
|
.request()
|
||||||
.header(HttpHeaders.AUTHORIZATION, AuthHelper.getProvisioningAuthHeader(NUMBER, PASSWORD));
|
.header(HttpHeaders.AUTHORIZATION, AuthHelper.getProvisioningAuthHeader(NUMBER, PASSWORD));
|
||||||
try (Response response = request.post(Entity.json(requestJson("sessionId")))) {
|
try (Response response = request.post(Entity.json(requestJson("sessionId")))) {
|
||||||
assertEquals(error.getExpectedStatus(), response.getStatus());
|
assertEquals(expectedStatus, response.getStatus());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@SuppressWarnings("unused")
|
||||||
|
static ArgumentSets registrationLockAndDeviceTransfer() {
|
||||||
|
final Set<RegistrationLockError> registrationLockErrors = new HashSet<>(EnumSet.allOf(RegistrationLockError.class));
|
||||||
|
registrationLockErrors.add(null);
|
||||||
|
|
||||||
|
return ArgumentSets.argumentsForFirstParameter(true, false)
|
||||||
|
.argumentsForNextParameter(registrationLockErrors);
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
@ParameterizedTest
|
@ParameterizedTest
|
||||||
@CsvSource({
|
@CsvSource({
|
||||||
"false, false, false, 200",
|
"false, false, false, 200",
|
||||||
|
|
Loading…
Reference in New Issue