diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java index 96de80589..05bd127da 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -552,13 +552,20 @@ public class AccountController { rateLimiters.getVerifyLimiter().validate(number); - final Optional storedVerificationCode = pendingAccounts.getCodeForNumber(number); + final Optional maybeStoredVerificationCode = pendingAccounts.getCodeForNumber(number); - if (storedVerificationCode.isEmpty() || !storedVerificationCode.get().isValid(request.code())) { + final boolean codeVerified = maybeStoredVerificationCode.map(storedVerificationCode -> + storedVerificationCode.sessionId() != null ? + registrationServiceClient.checkVerificationCode(storedVerificationCode.sessionId(), + request.code(), REGISTRATION_RPC_TIMEOUT).join() : + storedVerificationCode.isValid(request.code())) + .orElse(false); + + if (!codeVerified) { throw new ForbiddenException(); } - storedVerificationCode.map(StoredVerificationCode::twilioVerificationSid) + maybeStoredVerificationCode.map(StoredVerificationCode::twilioVerificationSid) .ifPresent( verificationSid -> smsSender.reportVerificationSucceeded(verificationSid, userAgent, "changeNumber")); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java index 4bf67ff9e..8984a470d 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java @@ -1452,6 +1452,35 @@ class AccountControllerTest { assertThat(accountIdentityResponse.getPni()).isNotEqualTo(AuthHelper.VALID_PNI); } + @Test + void testChangePhoneNumberWithRegistrationService() throws Exception { + final String number = "+18005559876"; + final String code = "987654"; + final byte[] sessionId = "session".getBytes(StandardCharsets.UTF_8); + + when(pendingAccountsManager.getCodeForNumber(number)).thenReturn(Optional.of( + new StoredVerificationCode(code, System.currentTimeMillis(), "push", null, sessionId))); + + when(registrationServiceClient.checkVerificationCode(any(), any(), any())) + .thenReturn(CompletableFuture.completedFuture(true)); + + final AccountIdentityResponse accountIdentityResponse = + resources.getJerseyTest() + .target("/v1/accounts/number") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.entity(new ChangePhoneNumberRequest(number, code, null, null, null, null, null), + MediaType.APPLICATION_JSON_TYPE), AccountIdentityResponse.class); + + verify(registrationServiceClient).checkVerificationCode(sessionId, code, AccountController.REGISTRATION_RPC_TIMEOUT); + + verify(changeNumberManager).changeNumber(eq(AuthHelper.VALID_ACCOUNT), eq(number), any(), any(), any(), any()); + + assertThat(accountIdentityResponse.getUuid()).isEqualTo(AuthHelper.VALID_UUID); + assertThat(accountIdentityResponse.getNumber()).isEqualTo(number); + assertThat(accountIdentityResponse.getPni()).isNotEqualTo(AuthHelper.VALID_PNI); + } + @Test void testChangePhoneNumberImpossibleNumber() throws Exception { final String number = "This is not a real phone number"; @@ -1544,6 +1573,32 @@ class AccountControllerTest { verify(changeNumberManager, never()).changeNumber(any(), any(), any(), any(), any(), any()); } + @Test + void testChangePhoneNumberIncorrectCodeWithRegistrationService() throws Exception { + final String number = "+18005559876"; + final String code = "987654"; + final byte[] sessionId = "session-id".getBytes(StandardCharsets.UTF_8); + + when(pendingAccountsManager.getCodeForNumber(number)).thenReturn(Optional.of( + new StoredVerificationCode(code, System.currentTimeMillis(), "push", null, sessionId))); + + when(registrationServiceClient.checkVerificationCode(any(), any(), any())) + .thenReturn(CompletableFuture.completedFuture(false)); + + final Response response = + resources.getJerseyTest() + .target("/v1/accounts/number") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.entity(new ChangePhoneNumberRequest(number, code, null, null, null, null, null), + MediaType.APPLICATION_JSON_TYPE)); + + verify(registrationServiceClient).checkVerificationCode(sessionId, code, AccountController.REGISTRATION_RPC_TIMEOUT); + + assertThat(response.getStatus()).isEqualTo(403); + verify(changeNumberManager, never()).changeNumber(any(), any(), any(), any(), any(), any()); + } + @Test void testChangePhoneNumberExistingAccountReglockNotRequired() throws Exception { final String number = "+18005559876";