From 1cf174a61374061b66924e8cbd4a5609ea161757 Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Fri, 21 Jun 2024 16:56:57 -0400 Subject: [PATCH] Include "token invalidation timestamp" in push notification responses --- .../textsecuregcm/push/APNSender.java | 12 ++++++----- .../textsecuregcm/push/FcmSender.java | 5 +++-- .../push/PushNotificationManager.java | 4 ++-- .../push/SendPushNotificationResult.java | 8 ++++++-- .../textsecuregcm/push/APNSenderTest.java | 8 ++++---- .../textsecuregcm/push/FcmSenderTest.java | 8 ++++---- .../push/PushNotificationManagerTest.java | 20 +++++++++---------- 7 files changed, 36 insertions(+), 29 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/APNSender.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/APNSender.java index 457cc80a5..d7fed0202 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/APNSender.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/APNSender.java @@ -23,6 +23,7 @@ import java.security.InvalidKeyException; import java.security.NoSuchAlgorithmException; import java.time.Duration; import java.time.Instant; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import org.whispersystems.textsecuregcm.configuration.ApnConfiguration; @@ -156,20 +157,21 @@ public class APNSender implements Managed, PushNotificationSender { }) .thenApplyAsync(response -> { final boolean accepted; - final String rejectionReason; + final Optional rejectionReason; final boolean unregistered; if (response.isAccepted()) { accepted = true; - rejectionReason = null; + rejectionReason = Optional.empty(); unregistered = false; } else { accepted = false; - rejectionReason = response.getRejectionReason().orElse("unknown"); - unregistered = ("Unregistered".equals(rejectionReason) || "BadDeviceToken".equals(rejectionReason)); + rejectionReason = response.getRejectionReason(); + unregistered = response.getRejectionReason().map(reason -> "Unregistered".equals(reason) || "BadDeviceToken".equals(reason)) + .orElse(false); } - return new SendPushNotificationResult(accepted, rejectionReason, unregistered); + return new SendPushNotificationResult(accepted, rejectionReason, unregistered, response.getTokenInvalidationTimestamp()); }, executor); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/FcmSender.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/FcmSender.java index fb7444bb9..8074803ba 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/FcmSender.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/FcmSender.java @@ -23,6 +23,7 @@ import io.micrometer.core.instrument.Timer; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.ThreadFactory; @@ -96,7 +97,7 @@ public class FcmSender implements PushNotificationSender { return GoogleApiUtil.toCompletableFuture(firebaseMessagingClient.sendAsync(builder.build()), executor) .whenComplete((ignored, throwable) -> sample.stop(SEND_NOTIFICATION_TIMER)) - .thenApply(ignored -> new SendPushNotificationResult(true, null, false)) + .thenApply(ignored -> new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty())) .exceptionally(throwable -> { if (ExceptionUtils.unwrap(throwable) instanceof final FirebaseMessagingException firebaseMessagingException) { final String errorCode; @@ -111,7 +112,7 @@ public class FcmSender implements PushNotificationSender { final boolean unregistered = firebaseMessagingException.getMessagingErrorCode() == MessagingErrorCode.UNREGISTERED; - return new SendPushNotificationResult(false, errorCode, unregistered); + return new SendPushNotificationResult(false, Optional.of(errorCode), unregistered, Optional.empty()); } else { throw ExceptionUtils.wrap(throwable); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotificationManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotificationManager.java index 0437289a7..22680a266 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotificationManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotificationManager.java @@ -123,8 +123,8 @@ public class PushNotificationManager { "accepted", String.valueOf(result.accepted()), "unregistered", String.valueOf(result.unregistered())); - if (StringUtils.isNotBlank(result.errorCode())) { - tags = tags.and("errorCode", result.errorCode()); + if (result.errorCode().isPresent()) { + tags = tags.and("errorCode", result.errorCode().get()); } Metrics.counter(SENT_NOTIFICATION_COUNTER_NAME, tags).increment(); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/SendPushNotificationResult.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/SendPushNotificationResult.java index fc7abdd85..62fe62c91 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/SendPushNotificationResult.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/SendPushNotificationResult.java @@ -5,7 +5,11 @@ package org.whispersystems.textsecuregcm.push; -import javax.annotation.Nullable; +import java.time.Instant; +import java.util.Optional; -public record SendPushNotificationResult(boolean accepted, @Nullable String errorCode, boolean unregistered) { +public record SendPushNotificationResult(boolean accepted, + Optional errorCode, + boolean unregistered, + Optional unregisteredTimestamp) { } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/push/APNSenderTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/push/APNSenderTest.java index 75df6e714..2fe9ad4ed 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/push/APNSenderTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/push/APNSenderTest.java @@ -84,7 +84,7 @@ class APNSenderTest { assertThat(notification.getValue().getTopic()).isEqualTo(BUNDLE_ID + ".voip"); assertThat(result.accepted()).isTrue(); - assertThat(result.errorCode()).isNull(); + assertThat(result.errorCode()).isEmpty(); assertThat(result.unregistered()).isFalse(); verifyNoMoreInteractions(apnsClient); @@ -127,7 +127,7 @@ class APNSenderTest { } assertThat(result.accepted()).isTrue(); - assertThat(result.errorCode()).isNull(); + assertThat(result.errorCode()).isEmpty(); assertThat(result.unregistered()).isFalse(); verifyNoMoreInteractions(apnsClient); @@ -160,7 +160,7 @@ class APNSenderTest { assertThat(notification.getValue().getPriority()).isEqualTo(DeliveryPriority.IMMEDIATE); assertThat(result.accepted()).isFalse(); - assertThat(result.errorCode()).isEqualTo("Unregistered"); + assertThat(result.errorCode()).hasValue("Unregistered"); assertThat(result.unregistered()).isTrue(); } @@ -188,7 +188,7 @@ class APNSenderTest { assertThat(notification.getValue().getPriority()).isEqualTo(DeliveryPriority.IMMEDIATE); assertThat(result.accepted()).isFalse(); - assertThat(result.errorCode()).isEqualTo("BadTopic"); + assertThat(result.errorCode()).hasValue("BadTopic"); assertThat(result.unregistered()).isFalse(); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/push/FcmSenderTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/push/FcmSenderTest.java index 7b337cfcf..c6b5088c9 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/push/FcmSenderTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/push/FcmSenderTest.java @@ -7,7 +7,6 @@ package org.whispersystems.textsecuregcm.push; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -21,6 +20,7 @@ import com.google.firebase.messaging.FirebaseMessagingException; import com.google.firebase.messaging.Message; import com.google.firebase.messaging.MessagingErrorCode; import java.io.IOException; +import java.util.Optional; import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; @@ -65,7 +65,7 @@ class FcmSenderTest { verify(firebaseMessaging).sendAsync(any(Message.class)); assertTrue(result.accepted()); - assertNull(result.errorCode()); + assertTrue(result.errorCode().isEmpty()); assertFalse(result.unregistered()); } @@ -85,7 +85,7 @@ class FcmSenderTest { verify(firebaseMessaging).sendAsync(any(Message.class)); assertFalse(result.accepted()); - assertEquals("INVALID_ARGUMENT", result.errorCode()); + assertEquals(Optional.of("INVALID_ARGUMENT"), result.errorCode()); assertFalse(result.unregistered()); } @@ -105,7 +105,7 @@ class FcmSenderTest { verify(firebaseMessaging).sendAsync(any(Message.class)); assertFalse(result.accepted()); - assertEquals("UNREGISTERED", result.errorCode()); + assertEquals(Optional.of("UNREGISTERED"), result.errorCode()); assertTrue(result.unregistered()); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/push/PushNotificationManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/push/PushNotificationManagerTest.java index 5b2665002..e64adf5b7 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/push/PushNotificationManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/push/PushNotificationManagerTest.java @@ -64,7 +64,7 @@ class PushNotificationManagerTest { when(account.getDevice(Device.PRIMARY_ID)).thenReturn(Optional.of(device)); when(fcmSender.sendNotification(any())) - .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, null, false))); + .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty()))); pushNotificationManager.sendNewMessageNotification(account, Device.PRIMARY_ID, urgent); verify(fcmSender).sendNotification(new PushNotification(deviceToken, PushNotification.TokenType.FCM, PushNotification.NotificationType.NOTIFICATION, null, account, device, urgent)); @@ -76,7 +76,7 @@ class PushNotificationManagerTest { final String challengeToken = "challenge"; when(apnSender.sendNotification(any())) - .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, null, false))); + .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty()))); pushNotificationManager.sendRegistrationChallengeNotification(deviceToken, PushNotification.TokenType.APN_VOIP, challengeToken); verify(apnSender).sendNotification(new PushNotification(deviceToken, PushNotification.TokenType.APN_VOIP, PushNotification.NotificationType.CHALLENGE, challengeToken, null, null, true)); @@ -95,7 +95,7 @@ class PushNotificationManagerTest { when(account.getPrimaryDevice()).thenReturn(device); when(apnSender.sendNotification(any())) - .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, null, false))); + .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty()))); pushNotificationManager.sendRateLimitChallengeNotification(account, challengeToken); verify(apnSender).sendNotification(new PushNotification(deviceToken, PushNotification.TokenType.APN, PushNotification.NotificationType.RATE_LIMIT_CHALLENGE, challengeToken, account, device, true)); @@ -113,11 +113,11 @@ class PushNotificationManagerTest { if (isApn) { when(device.getApnId()).thenReturn(deviceToken); when(apnSender.sendNotification(any())) - .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, null, false))); + .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty()))); } else { when(device.getGcmId()).thenReturn(deviceToken); when(fcmSender.sendNotification(any())) - .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, null, false))); + .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty()))); } when(account.getDevice(Device.PRIMARY_ID)).thenReturn(Optional.of(device)); @@ -145,7 +145,7 @@ class PushNotificationManagerTest { "token", PushNotification.TokenType.FCM, PushNotification.NotificationType.NOTIFICATION, null, account, device, urgent); when(fcmSender.sendNotification(pushNotification)) - .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, null, false))); + .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty()))); pushNotificationManager.sendNotification(pushNotification); @@ -169,7 +169,7 @@ class PushNotificationManagerTest { "token", PushNotification.TokenType.APN, PushNotification.NotificationType.NOTIFICATION, null, account, device, urgent); when(apnSender.sendNotification(pushNotification)) - .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, null, false))); + .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty()))); if (!urgent) { when(apnPushNotificationScheduler.scheduleBackgroundNotification(account, device)) @@ -202,7 +202,7 @@ class PushNotificationManagerTest { "token", PushNotification.TokenType.APN_VOIP, PushNotification.NotificationType.NOTIFICATION, null, account, device, urgent); when(apnSender.sendNotification(pushNotification)) - .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, null, false))); + .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty()))); pushNotificationManager.sendNotification(pushNotification); @@ -230,7 +230,7 @@ class PushNotificationManagerTest { "token", PushNotification.TokenType.FCM, PushNotification.NotificationType.NOTIFICATION, null, account, device, true); when(fcmSender.sendNotification(pushNotification)) - .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(false, null, true))); + .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(false, Optional.empty(), true, Optional.empty()))); pushNotificationManager.sendNotification(pushNotification); @@ -252,7 +252,7 @@ class PushNotificationManagerTest { "token", PushNotification.TokenType.APN_VOIP, PushNotification.NotificationType.NOTIFICATION, null, account, device, true); when(apnSender.sendNotification(pushNotification)) - .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(false, null, true))); + .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(false, Optional.empty(), true, Optional.empty()))); when(apnPushNotificationScheduler.cancelScheduledNotifications(account, device)) .thenReturn(CompletableFuture.completedFuture(null));