From d972bb1ca95b62d0f85ef6d9f99105f104537c5f Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Thu, 24 Jul 2025 19:41:11 +0530 Subject: [PATCH] push_notifications: Redact content for older clients if E2EE required. This commit replaces the `PUSH_NOTIFICATION_REDACT_CONTENT` server setting with `require_e2ee_push_notifications` realm setting. If `require_e2ee_push_notifications` set to True: * Older clients: Content redacted * Updated clients: Encrypted content If `require_e2ee_push_notifications` set to False: * Older clients: Content NOT redacted * Updated clients: Encrypted content Note: Older clients refers to clients that don't support E2EE. Fixes part of #35370. --- zerver/lib/push_notifications.py | 15 +++- zerver/tests/test_e2ee_push_notifications.py | 80 ++++++++++++++++++++ zerver/tests/test_push_notifications.py | 80 -------------------- zproject/default_settings.py | 2 - zproject/prod_settings_template.py | 6 -- 5 files changed, 92 insertions(+), 91 deletions(-) diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 90e88aa13f..6eeb42f655 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -971,9 +971,6 @@ def get_mobile_push_content(rendered_content: str) -> str: else: child.getparent().replace(child, collapse_element) - if settings.PUSH_NOTIFICATION_REDACT_CONTENT: - return _("New message") - elem = lxml.html.fragment_fromstring(rendered_content, create_parent=True) change_katex_to_raw_latex(elem) potentially_collapse_quotes(elem) @@ -1324,6 +1321,18 @@ def send_push_notifications_legacy( ) return + # While sending push notifications for new messages to older clients + # (which don't support E2EE), if `require_e2ee_push_notifications` + # realm setting is set to `true`, we redact the content. + if gcm_payload.get("event") != "remove" and user_profile.realm.require_e2ee_push_notifications: + # Make deep copies so redaction doesn't affect the original dicts + apns_payload = copy.deepcopy(apns_payload) + gcm_payload = copy.deepcopy(gcm_payload) + + placeholder_content = _("New message") + apns_payload["alert"]["body"] = placeholder_content + gcm_payload["content"] = placeholder_content + if uses_notification_bouncer(): send_notifications_to_bouncer( user_profile, apns_payload, gcm_payload, gcm_options, android_devices, apple_devices diff --git a/zerver/tests/test_e2ee_push_notifications.py b/zerver/tests/test_e2ee_push_notifications.py index cb514328ca..ca8a40a0fe 100644 --- a/zerver/tests/test_e2ee_push_notifications.py +++ b/zerver/tests/test_e2ee_push_notifications.py @@ -678,3 +678,83 @@ class RemovePushNotificationTest(E2EEPushNotificationTestCase): f"Sent E2EE mobile push notifications for user {hamlet.id}: 1 via FCM, 1 via APNs", zerver_logger.output[2], ) + + +class RequireE2EEPushNotificationsSettingTest(E2EEPushNotificationTestCase): + def test_content_redacted(self) -> None: + hamlet = self.example_user("hamlet") + aaron = self.example_user("aaron") + realm = hamlet.realm + + self.register_old_push_devices_for_notification() + self.register_push_devices_for_notification() + + message_id = self.send_personal_message( + from_user=aaron, + to_user=hamlet, + content="not-redacted", + skip_capture_on_commit_callbacks=True, + ) + missed_message = { + "message_id": message_id, + "trigger": NotificationTriggers.DIRECT_MESSAGE, + } + + realm.require_e2ee_push_notifications = True + realm.save(update_fields=["require_e2ee_push_notifications"]) + + # Verify that the content is redacted in payloads supplied to + # 'send_notifications_to_bouncer' - payloads supplied to bouncer (legacy codepath). + # + # Verify that the content is not redacted in payloads supplied to + # 'send_push_notifications' - payloads which get encrypted. + with ( + activate_push_notification_service(), + mock.patch( + "zerver.lib.push_notifications.send_notifications_to_bouncer" + ) as mock_legacy, + mock.patch("zerver.lib.push_notifications.send_push_notifications") as mock_e2ee, + ): + handle_push_notification(hamlet.id, missed_message) + + mock_legacy.assert_called_once() + self.assertEqual(mock_legacy.call_args.args[1]["alert"]["body"], "New message") + self.assertEqual(mock_legacy.call_args.args[2]["content"], "New message") + + mock_e2ee.assert_called_once() + self.assertEqual(mock_e2ee.call_args.args[1]["alert"]["body"], "not-redacted") + self.assertEqual(mock_e2ee.call_args.args[2]["content"], "not-redacted") + + message_id = self.send_personal_message( + from_user=aaron, to_user=hamlet, skip_capture_on_commit_callbacks=True + ) + missed_message = { + "message_id": message_id, + "trigger": NotificationTriggers.DIRECT_MESSAGE, + } + + # Verify that the content is redacted in payloads supplied to + # to functions for sending it through APNs and FCM directly. + with ( + mock.patch("zerver.lib.push_notifications.has_apns_credentials", return_value=True), + mock.patch("zerver.lib.push_notifications.has_fcm_credentials", return_value=True), + mock.patch( + "zerver.lib.push_notifications.send_notifications_to_bouncer" + ) as send_bouncer, + mock.patch( + "zerver.lib.push_notifications.send_apple_push_notification", return_value=0 + ) as send_apple, + mock.patch( + "zerver.lib.push_notifications.send_android_push_notification", return_value=0 + ) as send_android, + # We have already asserted the payloads passed to E2EE codepath above. + mock.patch("zerver.lib.push_notifications.send_push_notifications"), + ): + handle_push_notification(hamlet.id, missed_message) + + send_bouncer.assert_not_called() + send_apple.assert_called_once() + send_android.assert_called_once() + + self.assertEqual(send_apple.call_args.args[2]["alert"]["body"], "New message") + self.assertEqual(send_android.call_args.args[2]["content"], "New message") diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index 69dc610549..b734e9a237 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -1836,49 +1836,6 @@ class TestGetAPNsPayload(PushNotificationTestCase): NotificationTriggers.STREAM_WILDCARD_MENTION ) - @override_settings(PUSH_NOTIFICATION_REDACT_CONTENT=True) - def test_get_message_payload_apns_redacted_content(self) -> None: - user_profile = self.example_user("othello") - message_id = self.send_group_direct_message( - self.sender, [self.example_user("othello"), self.example_user("cordelia")] - ) - message = Message.objects.get(id=message_id) - payload = get_message_payload_apns( - user_profile, message, NotificationTriggers.DIRECT_MESSAGE - ) - expected = { - "alert": { - "title": "Cordelia, Lear's daughter, King Hamlet, Othello, the Moor of Venice", - "subtitle": "King Hamlet:", - "body": "New message", - }, - "sound": "default", - "badge": 0, - "custom": { - "zulip": { - "message_ids": [message.id], - "recipient_type": "private", - "pm_users": ",".join( - str(user_profile_id) - for user_profile_id in sorted( - s.user_profile_id - for s in Subscription.objects.filter(recipient=message.recipient) - ) - ), - "sender_email": self.sender.email, - "sender_id": self.sender.id, - "server": settings.EXTERNAL_HOST, - "realm_id": self.sender.realm.id, - "realm_name": self.sender.realm.name, - "realm_uri": self.sender.realm.url, - "realm_url": self.sender.realm.url, - "user_id": user_profile.id, - "time": datetime_to_timestamp(message.date_sent), - }, - }, - } - self.assertDictEqual(payload, expected) - def test_get_message_payload_apns_stream_message_from_inaccessible_user(self) -> None: self.set_up_db_for_testing_user_access() @@ -2048,43 +2005,6 @@ class TestGetGCMPayload(PushNotificationTestCase): }, ) - @override_settings(PUSH_NOTIFICATION_REDACT_CONTENT=True) - def test_get_message_payload_gcm_redacted_content(self) -> None: - stream = Stream.objects.get(name="Denmark") - message = self.get_message(Recipient.STREAM, stream.id, stream.realm_id) - hamlet = self.example_user("hamlet") - payload, gcm_options = get_message_payload_gcm(hamlet, message) - self.assertDictEqual( - payload, - { - "user_id": hamlet.id, - "event": "message", - "zulip_message_id": message.id, - "time": datetime_to_timestamp(message.date_sent), - "content": "New message", - "content_truncated": False, - "server": settings.EXTERNAL_HOST, - "realm_id": hamlet.realm.id, - "realm_name": hamlet.realm.name, - "realm_uri": hamlet.realm.url, - "realm_url": hamlet.realm.url, - "sender_id": hamlet.id, - "sender_email": hamlet.email, - "sender_full_name": "King Hamlet", - "sender_avatar_url": absolute_avatar_url(message.sender), - "recipient_type": "stream", - "topic": "Test topic", - "stream": "Denmark", - "stream_id": stream.id, - }, - ) - self.assertDictEqual( - gcm_options, - { - "priority": "high", - }, - ) - def test_get_message_payload_gcm_stream_message_from_inaccessible_user(self) -> None: self.set_up_db_for_testing_user_access() diff --git a/zproject/default_settings.py b/zproject/default_settings.py index a9d9a8f93f..f6e2414c8c 100644 --- a/zproject/default_settings.py +++ b/zproject/default_settings.py @@ -243,8 +243,6 @@ ZULIP_SERVICE_PUSH_NOTIFICATIONS = False ZULIP_SERVICE_SUBMIT_USAGE_STATISTICS: bool | None = None ZULIP_SERVICE_SECURITY_ALERTS = False -PUSH_NOTIFICATION_REDACT_CONTENT = False - # Old setting kept around for backwards compatibility. Some old servers # may have it in their settings.py. PUSH_NOTIFICATION_BOUNCER_URL: str | None = None diff --git a/zproject/prod_settings_template.py b/zproject/prod_settings_template.py index f05c1694fd..76fbf99cc2 100644 --- a/zproject/prod_settings_template.py +++ b/zproject/prod_settings_template.py @@ -779,12 +779,6 @@ SOCIAL_AUTH_SAML_SUPPORT_CONTACT = { ## can disable submitting usage statistics here. # ZULIP_SERVICE_SUBMIT_USAGE_STATISTICS = False -## Whether to redact the content of push notifications. This is less -## usable, but avoids sending message content over the wire. In the -## future, we're likely to replace this with an end-to-end push -## notification encryption feature. -# PUSH_NOTIFICATION_REDACT_CONTENT = False - ## Whether to lightly advertise sponsoring Zulip in the gear menu. # PROMOTE_SPONSORING_ZULIP = True