diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2.java index 490db65fd..c4c09752c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2.java @@ -94,6 +94,13 @@ public class AccountControllerV2 { throw new ForbiddenException(); } + final Account account = accountsManager.getByAccountIdentifier(authenticatedDevice.accountIdentifier()) + .orElseThrow(() -> new WebApplicationException(Response.Status.UNAUTHORIZED)); + + if (account.getNumberOptional().isEmpty()) { + throw new ForbiddenException(); + } + if (!request.isSignatureValidOnEachSignedPreKey(userAgentString)) { throw new WebApplicationException("Invalid signature", 422); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java index 82d9814a7..fc1ed03f1 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java @@ -362,6 +362,12 @@ public class AccountsGrpcService extends SimpleAccountsGrpc.AccountsImplBase { final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedPrimaryDevice(); + if (getAuthenticatedAccount(authenticatedDevice).getNumberOptional().isEmpty()) { + return ChangeNumberResponse.newBuilder() + .setAccountDoesNotHavePhoneNumber(FailedPrecondition.getDefaultInstance()) + .build(); + } + final IdentityKey pniIdentityKey; try { pniIdentityKey = new IdentityKey(request.getPniIdentityKey().toByteArray()); diff --git a/service/src/main/proto/org/signal/chat/account.proto b/service/src/main/proto/org/signal/chat/account.proto index 4da06bd91..b45d4ffa6 100644 --- a/service/src/main/proto/org/signal/chat/account.proto +++ b/service/src/main/proto/org/signal/chat/account.proto @@ -407,6 +407,9 @@ message ChangeNumberResponse { errors.FailedPrecondition invalid_registration_session = 7 [(tag.reason) = "invalid_registration_session"]; errors.FailedPrecondition recovery_password_verification_failed = 8 [(tag.reason) = "recovery_password_verification_failed"]; + + // The account does not have a phone number + errors.FailedPrecondition account_does_not_have_phone_number = 9 [(tag.reason) = "account_has_no_phone_number"]; } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2Test.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2Test.java index 1e601279e..07dbadd71 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2Test.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2Test.java @@ -153,6 +153,13 @@ class AccountControllerV2Test { @ParameterizedTest @ValueSource(booleans = {true, false}) void changeNumberSuccess(final boolean useSessionVerification) throws Exception { + final Account account = mock(Account.class); + when(account.getNumberOptional()).thenReturn(Optional.of(PhoneNumberUtil.getInstance().format( + PhoneNumberUtil.getInstance().getExampleNumber("DE"), PhoneNumberUtil.PhoneNumberFormat.E164))); + + when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)) + .thenReturn(Optional.of(account)); + @Nullable final String sessionId = useSessionVerification ? encodeSessionId("session") : null; @Nullable final byte[] recoveryPassword = useSessionVerification ? null : "recovery-password".getBytes(StandardCharsets.UTF_8); @@ -244,6 +251,13 @@ class AccountControllerV2Test { @ParameterizedTest @MethodSource void invalidRegistrationId(final Integer pniRegistrationId, final int expectedStatusCode) { + final Account account = mock(Account.class); + when(account.getNumberOptional()).thenReturn(Optional.of(PhoneNumberUtil.getInstance().format( + PhoneNumberUtil.getInstance().getExampleNumber("DE"), PhoneNumberUtil.PhoneNumberFormat.E164))); + + when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)) + .thenReturn(Optional.of(account)); + final ChangeNumberRequest changeNumberRequest = new ChangeNumberRequest(encodeSessionId("session"), null, NEW_NUMBER, "123", IDENTITY_KEY, Collections.emptyList(), Map.of(Device.PRIMARY_ID, KeysHelper.signedECPreKey(1, IDENTITY_KEY_PAIR)), @@ -273,6 +287,13 @@ class AccountControllerV2Test { @Test void rateLimitedNumber() throws Exception { + final Account account = mock(Account.class); + when(account.getNumberOptional()).thenReturn(Optional.of(PhoneNumberUtil.getInstance().format( + PhoneNumberUtil.getInstance().getExampleNumber("DE"), PhoneNumberUtil.PhoneNumberFormat.E164))); + + when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)) + .thenReturn(Optional.of(account)); + doThrow(new RateLimitExceededException(null)) .when(changeNumberManager).changeNumber(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any()); @@ -291,6 +312,13 @@ class AccountControllerV2Test { @MethodSource void phoneVerificationException(final Exception exception, final int expectedStatus) throws Exception { + final Account account = mock(Account.class); + when(account.getNumberOptional()).thenReturn(Optional.of(PhoneNumberUtil.getInstance().format( + PhoneNumberUtil.getInstance().getExampleNumber("DE"), PhoneNumberUtil.PhoneNumberFormat.E164))); + + when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)) + .thenReturn(Optional.of(account)); + doThrow(exception) .when(changeNumberManager).changeNumber(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any()); @@ -316,6 +344,13 @@ class AccountControllerV2Test { @Test void deviceMessageTooLarge() throws Exception { + final Account account = mock(Account.class); + when(account.getNumberOptional()).thenReturn(Optional.of(PhoneNumberUtil.getInstance().format( + PhoneNumberUtil.getInstance().getExampleNumber("DE"), PhoneNumberUtil.PhoneNumberFormat.E164))); + + when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)) + .thenReturn(Optional.of(account)); + doThrow(MessageTooLargeException.class) .when(changeNumberManager).changeNumber(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any()); @@ -338,6 +373,13 @@ class AccountControllerV2Test { @Test void messageDeliveryNotAllowed() throws Exception { + final Account account = mock(Account.class); + when(account.getNumberOptional()).thenReturn(Optional.of(PhoneNumberUtil.getInstance().format( + PhoneNumberUtil.getInstance().getExampleNumber("DE"), PhoneNumberUtil.PhoneNumberFormat.E164))); + + when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)) + .thenReturn(Optional.of(account)); + doThrow(MessageDeliveryNotAllowedException.class) .when(changeNumberManager).changeNumber(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any()); @@ -358,6 +400,32 @@ class AccountControllerV2Test { } } + @Test + void accountHasNoNumber() throws Exception { + final Account numberlessAccount = mock(Account.class); + when(numberlessAccount.getNumberOptional()).thenReturn(Optional.empty()); + + when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)) + .thenReturn(Optional.of(numberlessAccount)); + + try (final Response response = resources.getJerseyTest() + .target("/v2/accounts/number") + .request() + .header(HttpHeaders.AUTHORIZATION, + AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.entity( + new ChangeNumberRequest(encodeSessionId("session"), null, NEW_NUMBER, "123", IDENTITY_KEY, + Collections.emptyList(), + Map.of(Device.PRIMARY_ID, KeysHelper.signedECPreKey(1, IDENTITY_KEY_PAIR)), + Map.of(Device.PRIMARY_ID, KeysHelper.signedKEMPreKey(2, IDENTITY_KEY_PAIR)), + Map.of(Device.PRIMARY_ID, 17)), + MediaType.APPLICATION_JSON_TYPE))) { + + assertEquals(403, response.getStatus()); + verify(changeNumberManager, never()).changeNumber(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any()); + } + } + /** * Valid request JSON with the give session ID and recovery password */ diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java index fb4830bb3..69ae8653d 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java @@ -836,6 +836,12 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest