Skip to content

Commit

Permalink
Remove machinery for setting/storing APNs VOIP tokens
Browse files Browse the repository at this point in the history
  • Loading branch information
@jon-signal
jon-signal committed Oct 1, 2024
1 parent b693cb9 commit 92698ef
Show file tree
Hide file tree
Showing 23 changed files with 65 additions and 251 deletions.
3 changes: 0 additions & 3 deletions service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ public void setGcmRegistrationId(@Mutable @Auth AuthenticatedDevice auth,

accounts.updateDevice(account, device.getId(), d -> {
d.setApnId(null);
d.setVoipApnId(null);
d.setGcmId(registrationId.gcmRegistrationId());
d.setFetchesMessages(false);
});
Expand Down Expand Up @@ -153,7 +152,6 @@ public void setApnRegistrationId(@Mutable @Auth AuthenticatedDevice auth,
// unconditionally
accounts.updateDevice(account, device.getId(), d -> {
d.setApnId(registrationId.apnRegistrationId());
d.setVoipApnId(registrationId.voipRegistrationId());
d.setGcmId(null);
d.setFetchesMessages(false);
});
Expand All @@ -167,7 +165,6 @@ public void deleteApnRegistrationId(@Mutable @Auth AuthenticatedDevice auth) {

accounts.updateDevice(account, device.getId(), d -> {
d.setApnId(null);
d.setVoipApnId(null);
d.setFetchesMessages(false);
if (d.getId() == 1) {
d.setUserAgent("OWI");
Expand Down
3 changes: 1 addition & 2 deletions service/src/main/java/org/whispersystems/textsecuregcm/entities/ApnRegistrationId.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,5 @@
import javax.annotation.Nullable;
import javax.validation.constraints.NotEmpty;

public record ApnRegistrationId(@NotEmpty String apnRegistrationId,
@Nullable String voipRegistrationId) {
public record ApnRegistrationId(@NotEmpty String apnRegistrationId) {
}
3 changes: 1 addition & 2 deletions ...ava/org/whispersystems/textsecuregcm/experiment/IdleDevicePushNotificationExperiment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ boolean isIdle(final Device device) {

@VisibleForTesting
boolean hasPushToken(final Device device) {
// Exclude VOIP tokens since they have their own, distinct delivery mechanism
return !StringUtils.isAllBlank(device.getApnId(), device.getGcmId()) && StringUtils.isBlank(device.getVoipApnId());
return !StringUtils.isAllBlank(device.getApnId(), device.getGcmId());
}

@Override
Expand Down
12 changes: 3 additions & 9 deletions service/src/main/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,20 +110,18 @@ public Mono<SetPushTokenResponse> setPushToken(final SetPushTokenRequest request
final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice();

@Nullable final String apnsToken;
@Nullable final String apnsVoipToken;
@Nullable final String fcmToken;

switch (request.getTokenRequestCase()) {

case APNS_TOKEN_REQUEST -> {
final SetPushTokenRequest.ApnsTokenRequest apnsTokenRequest = request.getApnsTokenRequest();

if (StringUtils.isAllBlank(apnsTokenRequest.getApnsToken(), apnsTokenRequest.getApnsVoipToken())) {
throw Status.INVALID_ARGUMENT.withDescription("APNs tokens may not both be blank").asRuntimeException();
if (StringUtils.isBlank(apnsTokenRequest.getApnsToken())) {
throw Status.INVALID_ARGUMENT.withDescription("APNs token must not be blank").asRuntimeException();
}

apnsToken = StringUtils.stripToNull(apnsTokenRequest.getApnsToken());
apnsVoipToken = StringUtils.stripToNull(apnsTokenRequest.getApnsVoipToken());
fcmToken = null;
}

Expand All @@ -135,7 +133,6 @@ public Mono<SetPushTokenResponse> setPushToken(final SetPushTokenRequest request
}

apnsToken = null;
apnsVoipToken = null;
fcmToken = StringUtils.stripToNull(fcmTokenRequest.getFcmToken());
}

Expand All @@ -150,14 +147,12 @@ public Mono<SetPushTokenResponse> setPushToken(final SetPushTokenRequest request

final boolean tokenUnchanged =
Objects.equals(device.getApnId(), apnsToken) &&
Objects.equals(device.getVoipApnId(), apnsVoipToken) &&
Objects.equals(device.getGcmId(), fcmToken);

return tokenUnchanged
? Mono.empty()
: Mono.fromFuture(() -> accountsManager.updateDeviceAsync(account, authenticatedDevice.deviceId(), d -> {
d.setApnId(apnsToken);
d.setVoipApnId(apnsVoipToken);
d.setGcmId(fcmToken);
d.setFetchesMessages(false);
}));
Expand All @@ -172,14 +167,13 @@ public Mono<ClearPushTokenResponse> clearPushToken(final ClearPushTokenRequest r
return Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier()))
.map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException))
.flatMap(account -> Mono.fromFuture(() -> accountsManager.updateDeviceAsync(account, authenticatedDevice.deviceId(), device -> {
if (StringUtils.isNotBlank(device.getApnId()) || StringUtils.isNotBlank(device.getVoipApnId())) {
if (StringUtils.isNotBlank(device.getApnId())) {
device.setUserAgent(device.isPrimary() ? "OWI" : "OWP");
} else if (StringUtils.isNotBlank(device.getGcmId())) {
device.setUserAgent("OWA");
}

device.setApnId(null);
device.setVoipApnId(null);
device.setGcmId(null);
device.setFetchesMessages(true);
})))
Expand Down
38 changes: 5 additions & 33 deletions service/src/main/java/org/whispersystems/textsecuregcm/push/APNSender.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,6 @@ public class APNSender implements Managed, PushNotificationSender {
private final String bundleId;
private final ApnsClient apnsClient;

@VisibleForTesting
static final String APN_VOIP_NOTIFICATION_PAYLOAD = new SimpleApnsPayloadBuilder()
.setSound("default")
.setLocalizedAlertMessage("APN_Message")
.build();

@VisibleForTesting
static final String APN_NSE_NOTIFICATION_PAYLOAD = new SimpleApnsPayloadBuilder()
.setMutableContent(true)
Expand Down Expand Up @@ -80,22 +74,8 @@ public APNSender(ExecutorService executor, ApnsClient apnsClient, String bundleI

@Override
public CompletableFuture<SendPushNotificationResult> sendNotification(final PushNotification notification) {
final String topic = switch (notification.tokenType()) {
case APN -> bundleId;
case APN_VOIP -> bundleId + ".voip";
default -> throw new IllegalArgumentException("Unsupported token type: " + notification.tokenType());
};

final boolean isVoip = notification.tokenType() == PushNotification.TokenType.APN_VOIP;

final String payload = switch (notification.notificationType()) {
case NOTIFICATION -> {
if (isVoip) {
yield APN_VOIP_NOTIFICATION_PAYLOAD;
} else {
yield notification.urgent() ? APN_NSE_NOTIFICATION_PAYLOAD : APN_BACKGROUND_PAYLOAD;
}
}
case NOTIFICATION -> notification.urgent() ? APN_NSE_NOTIFICATION_PAYLOAD : APN_BACKGROUND_PAYLOAD;

case ATTEMPT_LOGIN_NOTIFICATION_HIGH_PRIORITY -> new SimpleApnsPayloadBuilder()
.setMutableContent(true)
Expand All @@ -115,13 +95,7 @@ public CompletableFuture<SendPushNotificationResult> sendNotification(final Push
};

final PushType pushType = switch (notification.notificationType()) {
case NOTIFICATION -> {
if (isVoip) {
yield PushType.VOIP;
} else {
yield notification.urgent() ? PushType.ALERT : PushType.BACKGROUND;
}
}
case NOTIFICATION -> notification.urgent() ? PushType.ALERT : PushType.BACKGROUND;
case ATTEMPT_LOGIN_NOTIFICATION_HIGH_PRIORITY -> PushType.ALERT;
case CHALLENGE, RATE_LIMIT_CHALLENGE -> PushType.BACKGROUND;
};
Expand All @@ -131,19 +105,17 @@ public CompletableFuture<SendPushNotificationResult> sendNotification(final Push
if (pushType == PushType.BACKGROUND) {
deliveryPriority = DeliveryPriority.CONSERVE_POWER;
} else {
deliveryPriority = (notification.urgent() || isVoip)
? DeliveryPriority.IMMEDIATE
: DeliveryPriority.CONSERVE_POWER;
deliveryPriority = notification.urgent() ? DeliveryPriority.IMMEDIATE : DeliveryPriority.CONSERVE_POWER;
}

final String collapseId =
(notification.notificationType() == PushNotification.NotificationType.NOTIFICATION && notification.urgent() && !isVoip)
(notification.notificationType() == PushNotification.NotificationType.NOTIFICATION && notification.urgent())
? "incoming-message" : null;

final Instant start = Instant.now();

return apnsClient.sendNotification(new SimpleApnsPushNotification(notification.deviceToken(),
topic,
bundleId,
payload,
MAX_EXPIRATION,
deliveryPriority,
Expand Down
3 changes: 1 addition & 2 deletions service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotification.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ public enum NotificationType {

public enum TokenType {
FCM,
APN,
APN_VOIP,
APN
}
}
8 changes: 2 additions & 6 deletions service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotificationManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ Pair<String, PushNotification.TokenType> getToken(final Device device) throws No

if (StringUtils.isNotBlank(device.getGcmId())) {
tokenAndType = new Pair<>(device.getGcmId(), PushNotification.TokenType.FCM);
} else if (StringUtils.isNotBlank(device.getVoipApnId())) {
tokenAndType = new Pair<>(device.getVoipApnId(), PushNotification.TokenType.APN_VOIP);
} else if (StringUtils.isNotBlank(device.getApnId())) {
tokenAndType = new Pair<>(device.getApnId(), PushNotification.TokenType.APN);
} else {
Expand All @@ -115,7 +113,7 @@ CompletableFuture<Optional<SendPushNotificationResult>> sendNotification(final P

final PushNotificationSender sender = switch (pushNotification.tokenType()) {
case FCM -> fcmSender;
case APN, APN_VOIP -> apnSender;
case APN -> apnSender;
};

return sender.sendNotification(pushNotification).whenComplete((result, throwable) -> {
Expand Down Expand Up @@ -171,7 +169,7 @@ private void handleDeviceUnregistered(final Account account,
tokenInvalidationTimestamp.isAfter(Instant.ofEpochMilli(device.getPushTimestamp()))).orElse(true);

if (tokenExpired) {
if (tokenType == PushNotification.TokenType.APN || tokenType == PushNotification.TokenType.APN_VOIP) {
if (tokenType == PushNotification.TokenType.APN) {
pushNotificationScheduler.cancelScheduledNotifications(account, device).whenComplete(logErrors());
}

Expand Down Expand Up @@ -203,7 +201,6 @@ private void clearPushToken(final Account account, final Device device, final Pu
switch (tokenType) {
case FCM -> d.setGcmId(null);
case APN -> d.setApnId(null);
case APN_VOIP -> d.setVoipApnId(null);
}
}
})));
Expand All @@ -213,7 +210,6 @@ private static String getPushToken(final Device device, final PushNotification.T
return switch (tokenType) {
case FCM -> device.getGcmId();
case APN -> device.getApnId();
case APN_VOIP -> device.getVoipApnId();
};
}
}
11 changes: 0 additions & 11 deletions service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ public class Device {
@JsonProperty
private String apnId;

@JsonProperty
private String voipApnId;

@JsonProperty
private long pushTimestamp;

Expand Down Expand Up @@ -89,14 +86,6 @@ public void setApnId(String apnId) {
}
}

public String getVoipApnId() {
return voipApnId;
}

public void setVoipApnId(String voipApnId) {
this.voipApnId = voipApnId;
}

public void setLastSeen(long lastSeen) {
this.lastSeen = lastSeen;
}
Expand Down
9 changes: 2 additions & 7 deletions service/src/main/java/org/whispersystems/textsecuregcm/storage/DeviceSpec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,8 @@ public Device toDevice(final byte deviceId, final Clock clock) {
device.setLastSeen(Util.todayInMillis());
device.setUserAgent(signalAgent());

apnRegistrationId().ifPresent(apnRegistrationId -> {
device.setApnId(apnRegistrationId.apnRegistrationId());
device.setVoipApnId(apnRegistrationId.voipRegistrationId());
});

gcmRegistrationId().ifPresent(gcmRegistrationId ->
device.setGcmId(gcmRegistrationId.gcmRegistrationId()));
apnRegistrationId().ifPresent(apnRegistrationId -> device.setApnId(apnRegistrationId.apnRegistrationId()));
gcmRegistrationId().ifPresent(gcmRegistrationId -> device.setGcmId(gcmRegistrationId.gcmRegistrationId()));

return device;
}
Expand Down
3 changes: 1 addition & 2 deletions service/src/main/java/org/whispersystems/textsecuregcm/workers/NotifyIdleDevicesCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ static boolean isLongIdle(final Device device, final Clock clock) {

@VisibleForTesting
static boolean hasPushToken(final Device device) {
// Exclude VOIP tokens since they have their own, distinct delivery mechanism
return !StringUtils.isAllBlank(device.getApnId(), device.getGcmId()) && StringUtils.isBlank(device.getVoipApnId());
return !StringUtils.isAllBlank(device.getApnId(), device.getGcmId());
}
}
7 changes: 0 additions & 7 deletions service/src/main/proto/org/signal/chat/device.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,6 @@ message SetPushTokenRequest {
* A "standard" APNs device token.
*/
string apns_token = 1;

/**
* A VoIP APNs device token. If present, the server will prefer to send
* message notifications to the device using this token on a VOIP APNs
* topic.
*/
string apns_voip_token = 2;
}

message FcmTokenRequest {
Expand Down
20 changes: 1 addition & 19 deletions ...ice/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,29 +308,11 @@ void testSetApnId() {
.request()
.header(HttpHeaders.AUTHORIZATION,
AuthHelper.getAuthHeader(AuthHelper.VALID_UUID_3, AuthHelper.VALID_PASSWORD_3_PRIMARY))
.put(Entity.json(new ApnRegistrationId("first", "second")))) {
.put(Entity.json(new ApnRegistrationId("first")))) {

assertThat(response.getStatus()).isEqualTo(204);

verify(AuthHelper.VALID_DEVICE_3_PRIMARY, times(1)).setApnId(eq("first"));
verify(AuthHelper.VALID_DEVICE_3_PRIMARY, times(1)).setVoipApnId(eq("second"));
verify(accountsManager, times(1)).updateDevice(eq(AuthHelper.VALID_ACCOUNT_3), anyByte(), any());
}
}

@Test
void testSetApnIdNoVoip() {
try (final Response response = resources.getJerseyTest()
.target("/v1/accounts/apn/")
.request()
.header(HttpHeaders.AUTHORIZATION,
AuthHelper.getAuthHeader(AuthHelper.VALID_UUID_3, AuthHelper.VALID_PASSWORD_3_PRIMARY))
.put(Entity.json(new ApnRegistrationId("first", null)))) {

assertThat(response.getStatus()).isEqualTo(204);

verify(AuthHelper.VALID_DEVICE_3_PRIMARY, times(1)).setApnId(eq("first"));
verify(AuthHelper.VALID_DEVICE_3_PRIMARY, times(1)).setVoipApnId(null);
verify(accountsManager, times(1)).updateDevice(eq(AuthHelper.VALID_ACCOUNT_3), anyByte(), any());
}
}
Expand Down
21 changes: 8 additions & 13 deletions service/src/test/java/org/whispersystems/textsecuregcm/controllers/DeviceControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ void linkDeviceAtomic(final boolean fetchesMessages,
final Optional<ApnRegistrationId> apnRegistrationId,
final Optional<GcmRegistrationId> gcmRegistrationId,
final Optional<String> expectedApnsToken,
final Optional<String> expectedApnsVoipToken,
final Optional<String> expectedGcmToken) {

final Device existingDevice = mock(Device.class);
Expand Down Expand Up @@ -240,9 +239,6 @@ void linkDeviceAtomic(final boolean fetchesMessages,
expectedApnsToken.ifPresentOrElse(expectedToken -> assertEquals(expectedToken, device.getApnId()),
() -> assertNull(device.getApnId()));

expectedApnsVoipToken.ifPresentOrElse(expectedToken -> assertEquals(expectedToken, device.getVoipApnId()),
() -> assertNull(device.getVoipApnId()));

expectedGcmToken.ifPresentOrElse(expectedToken -> assertEquals(expectedToken, device.getGcmId()),
() -> assertNull(device.getGcmId()));

Expand All @@ -251,14 +247,13 @@ void linkDeviceAtomic(final boolean fetchesMessages,

private static Stream<Arguments> linkDeviceAtomic() {
final String apnsToken = "apns-token";
final String apnsVoipToken = "apns-voip-token";
final String gcmToken = "gcm-token";

return Stream.of(
Arguments.of(true, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()),
Arguments.of(false, Optional.of(new ApnRegistrationId(apnsToken, null)), Optional.empty(), Optional.of(apnsToken), Optional.empty(), Optional.empty()),
Arguments.of(false, Optional.of(new ApnRegistrationId(apnsToken, apnsVoipToken)), Optional.empty(), Optional.of(apnsToken), Optional.of(apnsVoipToken), Optional.empty()),
Arguments.of(false, Optional.empty(), Optional.of(new GcmRegistrationId(gcmToken)), Optional.empty(), Optional.empty(), Optional.of(gcmToken))
Arguments.of(true, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()),
Arguments.of(false, Optional.of(new ApnRegistrationId(apnsToken)), Optional.empty(), Optional.of(apnsToken), Optional.empty()),
Arguments.of(false, Optional.of(new ApnRegistrationId(apnsToken)), Optional.empty(), Optional.of(apnsToken), Optional.empty()),
Arguments.of(false, Optional.empty(), Optional.of(new GcmRegistrationId(gcmToken)), Optional.empty(), Optional.of(gcmToken))
);
}

Expand Down Expand Up @@ -496,10 +491,10 @@ void linkDeviceAtomicConflictingChannel(final boolean fetchesMessages,

private static Stream<Arguments> linkDeviceAtomicConflictingChannel() {
return Stream.of(
Arguments.of(true, Optional.of(new ApnRegistrationId("apns-token", null)), Optional.of(new GcmRegistrationId("gcm-token"))),
Arguments.of(true, Optional.of(new ApnRegistrationId("apns-token")), Optional.of(new GcmRegistrationId("gcm-token"))),
Arguments.of(true, Optional.empty(), Optional.of(new GcmRegistrationId("gcm-token"))),
Arguments.of(true, Optional.of(new ApnRegistrationId("apns-token", null)), Optional.empty()),
Arguments.of(false, Optional.of(new ApnRegistrationId("apns-token", null)), Optional.of(new GcmRegistrationId("gcm-token")))
Arguments.of(true, Optional.of(new ApnRegistrationId("apns-token")), Optional.empty()),
Arguments.of(false, Optional.of(new ApnRegistrationId("apns-token")), Optional.of(new GcmRegistrationId("gcm-token")))
);
}

Expand Down Expand Up @@ -737,7 +732,7 @@ void linkDeviceRegistrationId(final int registrationId, final int pniRegistratio

final LinkDeviceRequest request = new LinkDeviceRequest(deviceCode.verificationCode(),
new AccountAttributes(false, registrationId, pniRegistrationId, null, null, true, new DeviceCapabilities(true, true, true, false, false)),
new DeviceActivationRequest(aciSignedPreKey, pniSignedPreKey, aciPqLastResortPreKey, pniPqLastResortPreKey, Optional.of(new ApnRegistrationId("apn", null)), Optional.empty()));
new DeviceActivationRequest(aciSignedPreKey, pniSignedPreKey, aciPqLastResortPreKey, pniPqLastResortPreKey, Optional.of(new ApnRegistrationId("apn")), Optional.empty()));

try (final Response response = resources.getJerseyTest()
.target("/v1/devices/link")
Expand Down
Loading

0 comments on commit 92698ef

Please sign in to comment.