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 4a68e1349..21c07c267 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -399,38 +399,47 @@ public class AccountController { @PUT @Path("/number") @Produces(MediaType.APPLICATION_JSON) - public void changeNumber(@Auth final AuthenticatedAccount authenticatedAccount, @Valid final ChangePhoneNumberRequest request) + public AccountIdentityResponse changeNumber(@Auth final AuthenticatedAccount authenticatedAccount, @Valid final ChangePhoneNumberRequest request) throws RateLimitExceededException, InterruptedException, ImpossiblePhoneNumberException, NonNormalizedPhoneNumberException { + final Account updatedAccount; + if (request.getNumber().equals(authenticatedAccount.getAccount().getNumber())) { // This may be a request that got repeated due to poor network conditions or other client error; take no action, // but report success since the account is in the desired state - return; + updatedAccount = authenticatedAccount.getAccount(); + } else { + Util.requireNormalizedNumber(request.getNumber()); + + rateLimiters.getVerifyLimiter().validate(request.getNumber()); + + final Optional storedVerificationCode = + pendingAccounts.getCodeForNumber(request.getNumber()); + + if (storedVerificationCode.isEmpty() || !storedVerificationCode.get().isValid(request.getCode())) { + throw new WebApplicationException(Response.status(403).build()); + } + + storedVerificationCode.flatMap(StoredVerificationCode::getTwilioVerificationSid) + .ifPresent(smsSender::reportVerificationSucceeded); + + final Optional existingAccount = accounts.getByE164(request.getNumber()); + + if (existingAccount.isPresent()) { + verifyRegistrationLock(existingAccount.get(), request.getRegistrationLock()); + } + + rateLimiters.getVerifyLimiter().clear(request.getNumber()); + + updatedAccount = accounts.changeNumber(authenticatedAccount.getAccount(), request.getNumber()); } - Util.requireNormalizedNumber(request.getNumber()); - - rateLimiters.getVerifyLimiter().validate(request.getNumber()); - - final Optional storedVerificationCode = - pendingAccounts.getCodeForNumber(request.getNumber()); - - if (storedVerificationCode.isEmpty() || !storedVerificationCode.get().isValid(request.getCode())) { - throw new WebApplicationException(Response.status(403).build()); - } - - storedVerificationCode.flatMap(StoredVerificationCode::getTwilioVerificationSid) - .ifPresent(smsSender::reportVerificationSucceeded); - - final Optional existingAccount = accounts.getByE164(request.getNumber()); - - if (existingAccount.isPresent()) { - verifyRegistrationLock(existingAccount.get(), request.getRegistrationLock()); - } - - rateLimiters.getVerifyLimiter().clear(request.getNumber()); - - accounts.changeNumber(authenticatedAccount.getAccount(), request.getNumber()); + return new AccountIdentityResponse( + updatedAccount.getUuid(), + updatedAccount.getNumber(), + updatedAccount.getPhoneNumberIdentifier(), + updatedAccount.getUsername().orElse(null), + updatedAccount.isStorageSupported()); } @Timed diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java index dc6c8310d..a967a1ea9 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java @@ -52,6 +52,7 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; +import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount; import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials; @@ -243,6 +244,20 @@ class AccountControllerTest { when(accountsManager.setUsername(AuthHelper.VALID_ACCOUNT, "takenusername")) .thenThrow(new UsernameNotAvailableException()); + when(accountsManager.changeNumber(any(), any())).thenAnswer((Answer) invocation -> { + final Account account = invocation.getArgument(0, Account.class); + final String number = invocation.getArgument(1, String.class); + + final UUID uuid = account.getUuid(); + + final Account updatedAccount = mock(Account.class); + when(updatedAccount.getUuid()).thenReturn(uuid); + when(updatedAccount.getNumber()).thenReturn(number); + when(updatedAccount.getPhoneNumberIdentifier()).thenReturn(UUID.randomUUID()); + + return updatedAccount; + }); + { DynamicConfiguration dynamicConfiguration = mock(DynamicConfiguration.class); when(dynamicConfigurationManager.getConfiguration()) @@ -1214,16 +1229,19 @@ class AccountControllerTest { when(pendingAccountsManager.getCodeForNumber(number)).thenReturn(Optional.of( new StoredVerificationCode(code, System.currentTimeMillis(), "push", null))); - final Response response = + 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), - MediaType.APPLICATION_JSON_TYPE)); + MediaType.APPLICATION_JSON_TYPE), AccountIdentityResponse.class); - assertThat(response.getStatus()).isEqualTo(204); verify(accountsManager).changeNumber(AuthHelper.VALID_ACCOUNT, number); + + assertThat(accountIdentityResponse.getUuid()).isEqualTo(AuthHelper.VALID_UUID); + assertThat(accountIdentityResponse.getNumber()).isEqualTo(number); + assertThat(accountIdentityResponse.getPni()).isNotEqualTo(AuthHelper.VALID_PNI); } @Test @@ -1268,16 +1286,19 @@ class AccountControllerTest { @Test void testChangePhoneNumberSameNumber() throws InterruptedException { - final Response response = + 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(AuthHelper.VALID_NUMBER, "567890", null), - MediaType.APPLICATION_JSON_TYPE)); + MediaType.APPLICATION_JSON_TYPE), AccountIdentityResponse.class); - assertThat(response.getStatus()).isEqualTo(204); verify(accountsManager, never()).changeNumber(eq(AuthHelper.VALID_ACCOUNT), any()); + + assertThat(accountIdentityResponse.getUuid()).isEqualTo(AuthHelper.VALID_UUID); + assertThat(accountIdentityResponse.getNumber()).isEqualTo(AuthHelper.VALID_NUMBER); + assertThat(accountIdentityResponse.getPni()).isEqualTo(AuthHelper.VALID_PNI); } @Test @@ -1345,7 +1366,7 @@ class AccountControllerTest { .put(Entity.entity(new ChangePhoneNumberRequest(number, code, null), MediaType.APPLICATION_JSON_TYPE)); - assertThat(response.getStatus()).isEqualTo(204); + assertThat(response.getStatus()).isEqualTo(200); verify(accountsManager).changeNumber(eq(AuthHelper.VALID_ACCOUNT), any()); } @@ -1439,7 +1460,7 @@ class AccountControllerTest { .put(Entity.entity(new ChangePhoneNumberRequest(number, code, reglock), MediaType.APPLICATION_JSON_TYPE)); - assertThat(response.getStatus()).isEqualTo(204); + assertThat(response.getStatus()).isEqualTo(200); verify(accountsManager).changeNumber(eq(AuthHelper.VALID_ACCOUNT), any()); }