Skip to content

Commit

Permalink
Add registration recovery checker
Browse files Browse the repository at this point in the history
  • Loading branch information
@ameya-signal
ameya-signal committed Aug 19, 2024
1 parent 0b1ec1e commit 7cbbf73
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 14 deletions.
11 changes: 9 additions & 2 deletions service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecovery2Client;
import org.whispersystems.textsecuregcm.spam.ChallengeConstraintChecker;
import org.whispersystems.textsecuregcm.spam.RegistrationFraudChecker;
import org.whispersystems.textsecuregcm.spam.RegistrationRecoveryChecker;
import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider;
import org.whispersystems.textsecuregcm.spam.SpamChecker;
import org.whispersystems.textsecuregcm.spam.SpamFilter;
Expand Down Expand Up @@ -675,8 +676,6 @@ public void run(WhisperServerConfiguration config, Environment environment) thro
final RegistrationLockVerificationManager registrationLockVerificationManager = new RegistrationLockVerificationManager(
accountsManager, clientPresenceManager, svr2CredentialsGenerator, svr3CredentialsGenerator,
registrationRecoveryPasswordsManager, pushNotificationManager, rateLimiters);
final PhoneVerificationTokenManager phoneVerificationTokenManager = new PhoneVerificationTokenManager(
registrationServiceClient, registrationRecoveryPasswordsManager);

final ReportedMessageMetricsListener reportedMessageMetricsListener = new ReportedMessageMetricsListener(
accountsManager);
Expand Down Expand Up @@ -1059,6 +1058,12 @@ protected void configureServer(final ServerBuilder<?> serverBuilder) {
log.warn("No registration-fraud-checkers found; using default (no-op) provider as a default");
return RegistrationFraudChecker.noop();
});
final RegistrationRecoveryChecker registrationRecoveryChecker = spamFilter
.map(SpamFilter::getRegistrationRecoveryChecker)
.orElseGet(() -> {
log.warn("No registration-recovery-checkers found; using default (no-op) provider as a default");
return RegistrationRecoveryChecker.noop();
});


spamFilter.map(SpamFilter::getReportedMessageListener).ifPresent(reportMessageManager::addListener);
Expand All @@ -1072,6 +1077,8 @@ protected void configureServer(final ServerBuilder<?> serverBuilder) {
});


final PhoneVerificationTokenManager phoneVerificationTokenManager = new PhoneVerificationTokenManager(
registrationServiceClient, registrationRecoveryPasswordsManager, registrationRecoveryChecker);
final List<Object> commonControllers = Lists.newArrayList(
new AccountController(accountsManager, rateLimiters, turnTokenGenerator, registrationRecoveryPasswordsManager,
usernameHashZkProofVerifier),
Expand Down
17 changes: 13 additions & 4 deletions ...ce/src/main/java/org/whispersystems/textsecuregcm/auth/PhoneVerificationTokenManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@
import javax.ws.rs.ForbiddenException;
import javax.ws.rs.NotAuthorizedException;
import javax.ws.rs.ServerErrorException;
import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.core.Response;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.whispersystems.textsecuregcm.entities.PhoneVerificationRequest;
import org.whispersystems.textsecuregcm.entities.RegistrationServiceSession;
import org.whispersystems.textsecuregcm.registration.RegistrationServiceClient;
import org.whispersystems.textsecuregcm.spam.RegistrationRecoveryChecker;
import org.whispersystems.textsecuregcm.storage.RegistrationRecoveryPasswordsManager;

public class PhoneVerificationTokenManager {
Expand All @@ -33,17 +35,21 @@ public class PhoneVerificationTokenManager {

private final RegistrationServiceClient registrationServiceClient;
private final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager;
private final RegistrationRecoveryChecker registrationRecoveryChecker;

public PhoneVerificationTokenManager(final RegistrationServiceClient registrationServiceClient,
final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager) {
final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager,
final RegistrationRecoveryChecker registrationRecoveryChecker) {
this.registrationServiceClient = registrationServiceClient;
this.registrationRecoveryPasswordsManager = registrationRecoveryPasswordsManager;
this.registrationRecoveryChecker = registrationRecoveryChecker;
}

/**
* Checks if a {@link PhoneVerificationRequest} has a token that verifies the caller has confirmed access to the e164
* number
*
* @param requestContext the container request context
* @param number the e164 presented for verification
* @param request the request with exactly one verification token (RegistrationService sessionId or registration
* recovery password)
Expand All @@ -54,13 +60,13 @@ public PhoneVerificationTokenManager(final RegistrationServiceClient registratio
* @throws ForbiddenException if the recovery password is not valid
* @throws InterruptedException if verification did not complete before a timeout
*/
public PhoneVerificationRequest.VerificationType verify(final String number, final PhoneVerificationRequest request)
public PhoneVerificationRequest.VerificationType verify(final ContainerRequestContext requestContext, final String number, final PhoneVerificationRequest request)
throws InterruptedException {

final PhoneVerificationRequest.VerificationType verificationType = request.verificationType();
switch (verificationType) {
case SESSION -> verifyBySessionId(number, request.decodeSessionId());
case RECOVERY_PASSWORD -> verifyByRecoveryPassword(number, request.recoveryPassword());
case RECOVERY_PASSWORD -> verifyByRecoveryPassword(requestContext, number, request.recoveryPassword());
}

return verificationType;
Expand Down Expand Up @@ -97,8 +103,11 @@ private void verifyBySessionId(final String number, final byte[] sessionId) thro
}
}

private void verifyByRecoveryPassword(final String number, final byte[] recoveryPassword)
private void verifyByRecoveryPassword(final ContainerRequestContext requestContext, final String number, final byte[] recoveryPassword)
throws InterruptedException {
if (!registrationRecoveryChecker.checkRegistrationRecoveryAttempt(requestContext, number)) {
throw new ForbiddenException("recoveryPassword couldn't be verified");
}
try {
final boolean verified = registrationRecoveryPasswordsManager.verify(number, recoveryPassword)
.get(VERIFICATION_TIMEOUT_SECONDS, TimeUnit.SECONDS);
Expand Down
9 changes: 6 additions & 3 deletions service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import org.whispersystems.textsecuregcm.auth.AuthenticatedDevice;
Expand Down Expand Up @@ -98,7 +100,8 @@ public AccountControllerV2(final AccountsManager accountsManager,
name = "Retry-After",
description = "If present, an positive integer indicating the number of seconds before a subsequent attempt could succeed"))
public AccountIdentityResponse changeNumber(@Mutable @Auth final AuthenticatedDevice authenticatedDevice,
@NotNull @Valid final ChangeNumberRequest request, @HeaderParam(HttpHeaders.USER_AGENT) final String userAgentString)
@NotNull @Valid final ChangeNumberRequest request, @HeaderParam(HttpHeaders.USER_AGENT) final String userAgentString,
@Context final ContainerRequestContext requestContext)
throws RateLimitExceededException, InterruptedException {

if (!authenticatedDevice.getAuthenticatedDevice().isPrimary()) {
Expand All @@ -116,8 +119,8 @@ public AccountIdentityResponse changeNumber(@Mutable @Auth final AuthenticatedDe

rateLimiters.getRegistrationLimiter().validate(number);

final PhoneVerificationRequest.VerificationType verificationType =
phoneVerificationTokenManager.verify(number, request);
final PhoneVerificationRequest.VerificationType verificationType = phoneVerificationTokenManager.verify(
requestContext, number, request);

final Optional<Account> existingAccount = accountsManager.getByE164(number);

Expand Down
9 changes: 6 additions & 3 deletions ...ce/src/main/java/org/whispersystems/textsecuregcm/controllers/RegistrationController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import org.whispersystems.textsecuregcm.auth.BasicAuthorizationHeader;
Expand Down Expand Up @@ -102,7 +104,8 @@ public AccountIdentityResponse register(
@HeaderParam(HttpHeaders.AUTHORIZATION) @NotNull final BasicAuthorizationHeader authorizationHeader,
@HeaderParam(HeaderUtils.X_SIGNAL_AGENT) final String signalAgent,
@HeaderParam(HttpHeaders.USER_AGENT) final String userAgent,
@NotNull @Valid final RegistrationRequest registrationRequest) throws RateLimitExceededException, InterruptedException {
@NotNull @Valid final RegistrationRequest registrationRequest,
@Context final ContainerRequestContext requestContext) throws RateLimitExceededException, InterruptedException {

final String number = authorizationHeader.getUsername();
final String password = authorizationHeader.getPassword();
Expand All @@ -113,8 +116,8 @@ public AccountIdentityResponse register(

rateLimiters.getRegistrationLimiter().validate(number);

final PhoneVerificationRequest.VerificationType verificationType = phoneVerificationTokenManager.verify(number,
registrationRequest);
final PhoneVerificationRequest.VerificationType verificationType = phoneVerificationTokenManager.verify(
requestContext, number, registrationRequest);

final Optional<Account> existingAccount = accounts.getByE164(number);

Expand Down
19 changes: 19 additions & 0 deletions service/src/main/java/org/whispersystems/textsecuregcm/spam/RegistrationRecoveryChecker.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package org.whispersystems.textsecuregcm.spam;

import javax.ws.rs.container.ContainerRequestContext;

public interface RegistrationRecoveryChecker {

/**
* Determine if a registration recovery attempt should be allowed or not
*
* @param requestContext The container request context for a registration recovery attempt
* @param e164 The E164 formatted phone number of the requester
* @return true if the registration recovery attempt is allowed, false otherwise.
*/
boolean checkRegistrationRecoveryAttempt(final ContainerRequestContext requestContext, final String e164);

static RegistrationRecoveryChecker noop() {
return (ignoredCtx, ignoredE164) -> true;
}
}
8 changes: 8 additions & 0 deletions service/src/main/java/org/whispersystems/textsecuregcm/spam/SpamFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,12 @@ public interface SpamFilter extends Managed {
* @return a {@link ChallengeConstraintChecker} controlled by the spam filter
*/
ChallengeConstraintChecker getChallengeConstraintChecker();

/**
* Return a checker that will be called to determine if a user is allowed to use their
* registration recovery password to re-register
*
* @return a {@link RegistrationRecoveryChecker} controlled by the spam filter
*/
RegistrationRecoveryChecker getRegistrationRecoveryChecker();
}
41 changes: 40 additions & 1 deletion ...e/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
import org.whispersystems.textsecuregcm.mappers.NonNormalizedPhoneNumberExceptionMapper;
import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper;
import org.whispersystems.textsecuregcm.registration.RegistrationServiceClient;
import org.whispersystems.textsecuregcm.spam.RegistrationRecoveryChecker;
import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.AccountBadge;
import org.whispersystems.textsecuregcm.storage.AccountsManager;
Expand Down Expand Up @@ -111,6 +112,7 @@ class AccountControllerV2Test {
RegistrationLockVerificationManager.class);
private final RateLimiters rateLimiters = mock(RateLimiters.class);
private final RateLimiter registrationLimiter = mock(RateLimiter.class);
private final RegistrationRecoveryChecker registrationRecoveryChecker = mock(RegistrationRecoveryChecker.class);

private final ResourceExtension resources = ResourceExtension.builder()
.addProperty(ServerProperties.UNWRAP_COMPLETION_STAGE_IN_WRITER_ENABLE, Boolean.TRUE)
Expand All @@ -123,7 +125,8 @@ class AccountControllerV2Test {
.setTestContainerFactory(new GrizzlyWebTestContainerFactory())
.addResource(
new AccountControllerV2(accountsManager, changeNumberManager,
new PhoneVerificationTokenManager(registrationServiceClient, registrationRecoveryPasswordsManager),
new PhoneVerificationTokenManager(registrationServiceClient, registrationRecoveryPasswordsManager,
registrationRecoveryChecker),
registrationLockVerificationManager, rateLimiters))
.build();

Expand Down Expand Up @@ -382,6 +385,8 @@ void registrationLock(final RegistrationLockError error) throws Exception {
void recoveryPasswordManagerVerificationTrue() throws Exception {
when(registrationRecoveryPasswordsManager.verify(any(), any()))
.thenReturn(CompletableFuture.completedFuture(true));
when(registrationRecoveryChecker.checkRegistrationRecoveryAttempt(any(), any()))
.thenReturn(true);

final Invocation.Builder request = resources.getJerseyTest()
.target("/v2/accounts/number")
Expand Down Expand Up @@ -418,6 +423,40 @@ void recoveryPasswordManagerVerificationFalse() {
}
}

@Test
void registrationRecoveryCheckerAllowsAttempt() {
when(registrationRecoveryChecker.checkRegistrationRecoveryAttempt(any(), any())).thenReturn(true);
when(registrationRecoveryPasswordsManager.verify(any(), any()))
.thenReturn(CompletableFuture.completedFuture(true));

final Invocation.Builder request = resources.getJerseyTest()
.target("/v2/accounts/number")
.request()
.header(HttpHeaders.AUTHORIZATION,
AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD));
final byte[] recoveryPassword = new byte[32];
try (Response response = request.put(Entity.json(requestJsonRecoveryPassword(recoveryPassword, NEW_NUMBER)))) {
assertEquals(200, response.getStatus());
}
}

@Test
void registrationRecoveryCheckerDisallowsAttempt() {
when(registrationRecoveryChecker.checkRegistrationRecoveryAttempt(any(), any())).thenReturn(false);
when(registrationRecoveryPasswordsManager.verify(any(), any()))
.thenReturn(CompletableFuture.completedFuture(true));

final Invocation.Builder request = resources.getJerseyTest()
.target("/v2/accounts/number")
.request()
.header(HttpHeaders.AUTHORIZATION,
AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD));
final byte[] recoveryPassword = new byte[32];
try (Response response = request.put(Entity.json(requestJsonRecoveryPassword(recoveryPassword, NEW_NUMBER)))) {
assertEquals(403, response.getStatus());
}
}

/**
* Valid request JSON with the given Recovery Password
*/
Expand Down
51 changes: 50 additions & 1 deletion ...rc/test/java/org/whispersystems/textsecuregcm/controllers/RegistrationControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
import org.whispersystems.textsecuregcm.mappers.NonNormalizedPhoneNumberExceptionMapper;
import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper;
import org.whispersystems.textsecuregcm.registration.RegistrationServiceClient;
import org.whispersystems.textsecuregcm.spam.RegistrationRecoveryChecker;
import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.AccountsManager;
import org.whispersystems.textsecuregcm.storage.DeviceSpec;
Expand All @@ -97,6 +98,7 @@ class RegistrationControllerTest {
RegistrationLockVerificationManager.class);
private final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager = mock(
RegistrationRecoveryPasswordsManager.class);
private final RegistrationRecoveryChecker registrationRecoveryChecker = mock(RegistrationRecoveryChecker.class);
private final RateLimiters rateLimiters = mock(RateLimiters.class);

private final RateLimiter registrationLimiter = mock(RateLimiter.class);
Expand All @@ -110,7 +112,8 @@ class RegistrationControllerTest {
.setTestContainerFactory(new GrizzlyWebTestContainerFactory())
.addResource(
new RegistrationController(accountsManager,
new PhoneVerificationTokenManager(registrationServiceClient, registrationRecoveryPasswordsManager),
new PhoneVerificationTokenManager(registrationServiceClient, registrationRecoveryPasswordsManager,
registrationRecoveryChecker),
registrationLockVerificationManager, rateLimiters))
.build();

Expand Down Expand Up @@ -239,6 +242,7 @@ void registrationServiceTimeout() {

@Test
void recoveryPasswordManagerVerificationFailureOrTimeout() {
when(registrationRecoveryChecker.checkRegistrationRecoveryAttempt(any(), any())).thenReturn(true);
when(registrationRecoveryPasswordsManager.verify(any(), any()))
.thenReturn(CompletableFuture.failedFuture(new RuntimeException()));

Expand Down Expand Up @@ -284,6 +288,7 @@ static Stream<Arguments> registrationServiceSessionCheck() {

@Test
void recoveryPasswordManagerVerificationTrue() throws InterruptedException {
when(registrationRecoveryChecker.checkRegistrationRecoveryAttempt(any(), any())).thenReturn(true);
when(registrationRecoveryPasswordsManager.verify(any(), any()))
.thenReturn(CompletableFuture.completedFuture(true));

Expand Down Expand Up @@ -317,6 +322,50 @@ void recoveryPasswordManagerVerificationFalse() throws InterruptedException {
}
}

@Test
void registrationRecoveryCheckerAllowsAttempt() throws InterruptedException {
when(registrationRecoveryChecker.checkRegistrationRecoveryAttempt(any(), any())).thenReturn(true);
when(registrationRecoveryPasswordsManager.verify(any(), any()))
.thenReturn(CompletableFuture.completedFuture(true));

final Account account = mock(Account.class);
when(account.getPrimaryDevice()).thenReturn(mock(Device.class));

when(accountsManager.create(any(), any(), any(), any(), any(), any()))
.thenReturn(account);

final Invocation.Builder request = resources.getJerseyTest()
.target("/v1/registration")
.request()
.header(HttpHeaders.AUTHORIZATION, AuthHelper.getProvisioningAuthHeader(NUMBER, PASSWORD));
final byte[] recoveryPassword = new byte[32];
try (Response response = request.post(Entity.json(requestJsonRecoveryPassword(recoveryPassword)))) {
assertEquals(200, response.getStatus());
}
}

@Test
void registrationRecoveryCheckerDisallowsAttempt() throws InterruptedException {
when(registrationRecoveryChecker.checkRegistrationRecoveryAttempt(any(), any())).thenReturn(false);
when(registrationRecoveryPasswordsManager.verify(any(), any()))
.thenReturn(CompletableFuture.completedFuture(true));

final Account account = mock(Account.class);
when(account.getPrimaryDevice()).thenReturn(mock(Device.class));

when(accountsManager.create(any(), any(), any(), any(), any(), any()))
.thenReturn(account);

final Invocation.Builder request = resources.getJerseyTest()
.target("/v1/registration")
.request()
.header(HttpHeaders.AUTHORIZATION, AuthHelper.getProvisioningAuthHeader(NUMBER, PASSWORD));
final byte[] recoveryPassword = new byte[32];
try (Response response = request.post(Entity.json(requestJsonRecoveryPassword(recoveryPassword)))) {
assertEquals(403, response.getStatus());
}
}

@CartesianTest
@CartesianTest.MethodFactory("registrationLockAndDeviceTransfer")
void registrationLockAndDeviceTransfer(
Expand Down

0 comments on commit 7cbbf73

Please sign in to comment.