From 2dbc17b453d3d05b36f4e0f0e6b5cbe04e6cc97e Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Thu, 24 Jul 2025 14:28:51 +0530 Subject: [PATCH] push_notification: Revoke push notifications using encrypted payload. This commit updates 'handle_remove_push_notification' function to use the new 'send_push_notifications' function. It leads to encrypt the removal payload before sending it to bouncer. Fixes part of #35368. --- zerver/lib/push_notifications.py | 14 ++-- zerver/tests/test_e2ee_push_notifications.py | 86 +++++++++++++++++++- 2 files changed, 93 insertions(+), 7 deletions(-) diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 3cff93aff9..0529796409 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -1323,6 +1323,11 @@ def handle_remove_push_notification(user_profile_id: int, message_ids: list[int] apns_payload = get_remove_payload_apns(user_profile, truncated_message_ids) send_push_notifications_legacy(user_profile, apns_payload, gcm_payload, gcm_options) + if settings.DEVELOPMENT: + # TODO: Remove the 'settings.DEVELOPMENT' check when mobile clients start + # to offer a way to register for E2EE push notifications; otherwise it'll + # do needless DB query and logging. + send_push_notifications(user_profile, apns_payload, gcm_payload, is_removal=True) # We intentionally use the non-truncated message_ids here. We are # assuming in this very rare case that the user has manually @@ -1394,6 +1399,7 @@ def send_push_notifications( user_profile: UserProfile, apns_payload_data_to_encrypt: dict[str, Any], fcm_payload_data_to_encrypt: dict[str, Any], + is_removal: bool = False, ) -> None: # Uses 'zerver_pushdevice_user_bouncer_device_id_idx' index. push_devices = PushDevice.objects.filter(user=user_profile, bouncer_device_id__isnull=False) @@ -1425,13 +1431,11 @@ def send_push_notifications( assert push_device.bouncer_device_id is not None # for mypy device_id_to_encrypted_data[str(push_device.bouncer_device_id)] = encrypted_data - # TODO: These literals will vary when implementing notification removal. - # # Note: The "Final" qualifier serves as a shorthand # for declaring that a variable is effectively Literal. - fcm_priority: Final = "high" - apns_priority: Final = 10 - apns_push_type = PushType.ALERT + fcm_priority: Final = "normal" if is_removal else "high" + apns_priority: Final = 5 if is_removal else 10 + apns_push_type = PushType.BACKGROUND if is_removal else PushType.ALERT # Send push notification try: diff --git a/zerver/tests/test_e2ee_push_notifications.py b/zerver/tests/test_e2ee_push_notifications.py index 80bfbc1e01..98e4ccb625 100644 --- a/zerver/tests/test_e2ee_push_notifications.py +++ b/zerver/tests/test_e2ee_push_notifications.py @@ -7,10 +7,10 @@ from firebase_admin.exceptions import InternalError from firebase_admin.messaging import UnregisteredError from analytics.models import RealmCount -from zerver.lib.push_notifications import handle_push_notification +from zerver.lib.push_notifications import handle_push_notification, handle_remove_push_notification from zerver.lib.test_classes import E2EEPushNotificationTestCase from zerver.lib.test_helpers import activate_push_notification_service -from zerver.models import PushDevice +from zerver.models import PushDevice, UserMessage from zerver.models.scheduled_jobs import NotificationTriggers from zilencer.models import RemoteRealm, RemoteRealmCount @@ -455,3 +455,85 @@ class SendPushNotificationTest(E2EEPushNotificationTestCase): realm.refresh_from_db() self.assertFalse(realm.push_notifications_enabled) self.assertIsNone(realm.push_notifications_enabled_end_timestamp) + + +@activate_push_notification_service() +@mock.patch("zerver.lib.push_notifications.send_push_notifications_legacy") +class RemovePushNotificationTest(E2EEPushNotificationTestCase): + def test_success_cloud(self, unused_mock: mock.MagicMock) -> None: + hamlet = self.example_user("hamlet") + aaron = self.example_user("aaron") + + self.register_push_devices_for_notification() + message_id = self.send_personal_message( + from_user=aaron, to_user=hamlet, skip_capture_on_commit_callbacks=True + ) + user_message = UserMessage.objects.get(user_profile=hamlet, message_id=message_id) + user_message.flags.active_mobile_push_notification = True + user_message.save(update_fields=["flags"]) + + with ( + self.mock_fcm() as mock_fcm_messaging, + self.mock_apns() as send_notification, + self.assertLogs("zerver.lib.push_notifications", level="INFO") as zerver_logger, + self.assertLogs("zilencer.lib.push_notifications", level="INFO"), + ): + mock_fcm_messaging.send_each.return_value = self.make_fcm_success_response() + send_notification.return_value.is_successful = True + + handle_remove_push_notification(hamlet.id, [message_id]) + + mock_fcm_messaging.send_each.assert_called_once() + send_notification.assert_called_once() + + user_message.refresh_from_db() + self.assertFalse(user_message.flags.active_mobile_push_notification) + + self.assertEqual( + "INFO:zerver.lib.push_notifications:" + f"Sent E2EE mobile push notifications for user {hamlet.id}: 1 via FCM, 1 via APNs", + zerver_logger.output[1], + ) + + @responses.activate + @override_settings(ZILENCER_ENABLED=False) + def test_success_self_hosted(self, unused_mock: mock.MagicMock) -> None: + self.add_mock_response() + + hamlet = self.example_user("hamlet") + aaron = self.example_user("aaron") + + self.register_push_devices_for_notification(is_server_self_hosted=True) + message_id = self.send_personal_message( + from_user=aaron, to_user=hamlet, skip_capture_on_commit_callbacks=True + ) + user_message = UserMessage.objects.get(user_profile=hamlet, message_id=message_id) + user_message.flags.active_mobile_push_notification = True + user_message.save(update_fields=["flags"]) + + with ( + self.mock_fcm() as mock_fcm_messaging, + self.mock_apns() as send_notification, + mock.patch( + "corporate.lib.stripe.RemoteRealmBillingSession.current_count_for_billed_licenses", + return_value=10, + ), + self.assertLogs("zerver.lib.push_notifications", level="INFO") as zerver_logger, + self.assertLogs("zilencer.lib.push_notifications", level="INFO"), + ): + mock_fcm_messaging.send_each.return_value = self.make_fcm_success_response() + send_notification.return_value.is_successful = True + + handle_remove_push_notification(hamlet.id, [message_id]) + + mock_fcm_messaging.send_each.assert_called_once() + send_notification.assert_called_once() + + user_message.refresh_from_db() + self.assertFalse(user_message.flags.active_mobile_push_notification) + + self.assertEqual( + "INFO:zerver.lib.push_notifications:" + f"Sent E2EE mobile push notifications for user {hamlet.id}: 1 via FCM, 1 via APNs", + zerver_logger.output[1], + )