Don't generate "includes e164" delivery certificates for accounts without phone numbers

service/src/main/java/org/whispersystems/textsecuregcm/auth/CertificateGenerator.java

@@ -35,6 +35,10 @@ public class CertificateGenerator {
   }
 
   public byte[] createFor(final Account account, final byte deviceId, boolean includeE164) {
+    if (includeE164 && account.getNumberOptional().isEmpty()) {
+      throw new IllegalArgumentException();
+    }
+
     SenderCertificate.Certificate.Builder builder = SenderCertificate.Certificate.newBuilder()
         .setSenderDevice(Math.toIntExact(deviceId))
         .setExpires(System.currentTimeMillis() + TimeUnit.DAYS.toMillis(expiresDays))
@@ -42,7 +46,7 @@ public class CertificateGenerator {
         .setSenderUuid(UUIDUtil.toByteString(account.getAccountIdentifier()));
 
     if (includeE164) {
-      builder.setSenderE164(account.getNumber());
+      builder.setSenderE164(account.getNumberOptional().get());
     }
 
     if (embedSigner) {

service/src/main/java/org/whispersystems/textsecuregcm/controllers/CertificateController.java

@@ -82,8 +82,11 @@ public class CertificateController {
     final Account account = accountsManager.getByAccountIdentifier(auth.accountIdentifier())
         .orElseThrow(() -> new WebApplicationException(Response.Status.UNAUTHORIZED));
 
-    return new DeliveryCertificate(
-        certificateGenerator.createFor(account, auth.deviceId(), includeE164));
+    try {
+      return new DeliveryCertificate(certificateGenerator.createFor(account, auth.deviceId(), includeE164));
+    } catch (final IllegalArgumentException _) {
+      throw new BadRequestException();
+    }
   }
 
   @GET

service/src/main/java/org/whispersystems/textsecuregcm/grpc/CredentialsGrpcService.java

@@ -93,12 +93,16 @@ public class CredentialsGrpcService extends SimpleCredentialsGrpc.CredentialsImp
     final Account account = accountsManager.getByAccountIdentifier(authenticatedDevice.accountIdentifier())
         .orElseThrow(() -> GrpcExceptions.invalidCredentials("invalid credentials"));
 
-    return GetDeliveryCertificateResponse.newBuilder()
-        .setCertificateWithE164(ByteString.copyFrom(
-            certificateGenerator.createFor(account, authenticatedDevice.deviceId(), true)))
+    final GetDeliveryCertificateResponse.Builder responseBuilder = GetDeliveryCertificateResponse.newBuilder()
         .setCertificateWithoutE164(ByteString.copyFrom(
-            certificateGenerator.createFor(account, authenticatedDevice.deviceId(), false)))
-        .build();
+            certificateGenerator.createFor(account, authenticatedDevice.deviceId(), false)));
+
+    if (account.getNumberOptional().isPresent()) {
+      responseBuilder.setCertificateWithE164(ByteString.copyFrom(
+          certificateGenerator.createFor(account, authenticatedDevice.deviceId(), true)));
+    }
+
+    return responseBuilder.build();
   }
 
   @Override

service/src/main/proto/org/signal/chat/credentials.proto

@@ -109,7 +109,8 @@ message GetDeliveryCertificateRequest {
 // server never learns anything about the caller's intent to share their phone
 // number with their contacts.
 message GetDeliveryCertificateResponse {
-  // A delivery receipt that includes the caller's phone number
+  // A delivery receipt that includes the caller's phone number; may be empty if
+  // the authenticated account does not have a phone number
   bytes certificate_with_e164 = 1;
 
   // A delivery receipt that does not include the caller's phone number

service/src/test/java/org/whispersystems/textsecuregcm/auth/CertificateGeneratorTest.java

@@ -6,17 +6,21 @@
 package org.whispersystems.textsecuregcm.auth;
 
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import com.google.i18n.phonenumbers.PhoneNumberUtil;
+import com.google.protobuf.InvalidProtocolBufferException;
 import java.io.IOException;
 import java.util.Base64;
+import java.util.Optional;
 import java.util.UUID;
-import org.junit.jupiter.params.provider.ValueSource;
+import org.junit.jupiter.api.function.Executable;
 import org.junitpioneer.jupiter.cartesian.CartesianTest;
 import org.signal.libsignal.protocol.IdentityKey;
 import org.signal.libsignal.protocol.ecc.ECPrivateKey;
@@ -49,7 +53,6 @@ class CertificateGeneratorTest {
   }
 
   @CartesianTest
-  @ValueSource(booleans = {true, false})
   void testCreateFor(@CartesianTest.Values(booleans = {true, false}) boolean includeE164,
       @CartesianTest.Values(booleans = {true, false}) boolean embedSigner)
       throws IOException, org.signal.libsignal.protocol.InvalidKeyException {
@@ -60,7 +63,7 @@ class CertificateGeneratorTest {
 
     when(account.getIdentityKey(IdentityType.ACI)).thenReturn(IDENTITY_KEY);
     when(account.getAccountIdentifier()).thenReturn(ACI);
-    when(account.getNumber()).thenReturn(E164);
+    when(account.getNumberOptional()).thenReturn(Optional.of(E164));
 
     final byte[] contents = certificateGenerator.createFor(account, deviceId, includeE164);
     final SenderCertificate fullCertificate = SenderCertificate.parseFrom(contents);
@@ -89,4 +92,25 @@ class CertificateGeneratorTest {
     assertTrue(signingKey
         .verifySignature(fullCertificate.getCertificate().toByteArray(), fullCertificate.getSignature().toByteArray()));
   }
+
+  @CartesianTest
+  void createForNoNumber(@CartesianTest.Values(booleans = {true, false}) final boolean includeE164,
+      @CartesianTest.Values(booleans = {true, false}) final boolean embedSigner) throws InvalidProtocolBufferException {
+    final Account account = mock(Account.class);
+    final byte deviceId = 4;
+    final CertificateGenerator certificateGenerator = new CertificateGenerator(
+        SIGNING_CERTIFICATE_DATA, SIGNING_KEY, 1, embedSigner);
+
+    when(account.getIdentityKey(IdentityType.ACI)).thenReturn(IDENTITY_KEY);
+    when(account.getAccountIdentifier()).thenReturn(ACI);
+    when(account.getNumberOptional()).thenReturn(Optional.empty());
+
+    final Executable generateCertificate = () -> certificateGenerator.createFor(account, deviceId, includeE164);
+
+    if (includeE164) {
+      assertThrows(IllegalArgumentException.class, generateCertificate);
+    } else {
+      assertDoesNotThrow(generateCertificate);
+    }
+  }
 }

service/src/test/java/org/whispersystems/textsecuregcm/controllers/CertificateControllerTest.java

@@ -52,6 +52,7 @@ import org.whispersystems.textsecuregcm.entities.DeliveryCertificate;
 import org.whispersystems.textsecuregcm.entities.GroupCredentials;
 import org.whispersystems.textsecuregcm.entities.MessageProtos.SenderCertificate;
 import org.whispersystems.textsecuregcm.entities.MessageProtos.ServerCertificate;
+import org.whispersystems.textsecuregcm.storage.Account;
 import org.whispersystems.textsecuregcm.storage.AccountsManager;
 import org.whispersystems.textsecuregcm.tests.util.AuthHelper;
 import org.whispersystems.textsecuregcm.util.HeaderUtils;
@@ -224,6 +225,25 @@ class CertificateControllerTest {
     assertEquals(401, response.getStatus());
   }
 
+  @Test
+  void testValidCertificateAccountHasNoNumber() {
+    final Account numberlessAccount = mock(Account.class);
+    when(numberlessAccount.getNumberOptional()).thenReturn(Optional.empty());
+
+    when(ACCOUNTS_MANAGER.getByAccountIdentifier(AuthHelper.VALID_UUID))
+        .thenReturn(Optional.of(numberlessAccount));
+
+    final Response response = resources.getJerseyTest()
+        .target("/v1/certificate/delivery")
+        .queryParam("includeUuid", "true")
+        .queryParam("includeE164", "true")
+        .request()
+        .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
+        .get();
+
+    assertEquals(400, response.getStatus());
+  }
+
   @Test
   void testGetSingleGroupCredentialWithPniAsServiceId() {
     final Instant startOfDay = clock.instant().truncatedTo(ChronoUnit.DAYS);

service/src/test/java/org/whispersystems/textsecuregcm/grpc/CredentialsGrpcServiceTest.java

@@ -150,7 +150,7 @@ public class CredentialsGrpcServiceTest
   @BeforeEach
   void setUp() {
     when(authenticatedAccount.getAccountIdentifier()).thenReturn(AUTHENTICATED_ACI);
-    when(authenticatedAccount.getNumber()).thenReturn(PHONE_NUMBER);
+    when(authenticatedAccount.getNumberOptional()).thenReturn(Optional.of(PHONE_NUMBER));
     when(authenticatedAccount.getIdentifier(IdentityType.ACI)).thenReturn(AUTHENTICATED_ACI);
     when(authenticatedAccount.getIdentifier(IdentityType.PNI)).thenReturn(AUTHENTICATED_PNI);
     when(authenticatedAccount.getIdentityKey(IdentityType.ACI))
@@ -291,6 +291,17 @@ public class CredentialsGrpcServiceTest
         new IdentityKey(IDENTITY_KEY_PAIR.getPublicKey()).serialize());
   }
 
+  @Test
+  void getDeliveryCertificateAccountHasNoNumber() throws InvalidProtocolBufferException, InvalidKeyException {
+    when(authenticatedAccount.getNumberOptional()).thenReturn(Optional.empty());
+
+    final GetDeliveryCertificateResponse response =
+        authenticatedServiceStub().getDeliveryCertificate(GetDeliveryCertificateRequest.getDefaultInstance());
+
+    assertTrue(response.getCertificateWithE164().isEmpty());
+    checkDeliveryCertificate(response.getCertificateWithoutE164().toByteArray(), false);
+  }
+
   @ParameterizedTest
   @ValueSource(ints = {0, 1, 2, 3, 4, 5, 6, 7})
   void getGroupCredentials(final int redemptionEndOffsetDays) {