diff --git a/docs/production/mobile-push-notifications.md b/docs/production/mobile-push-notifications.md index 12514d6de7..2f686acb15 100644 --- a/docs/production/mobile-push-notifications.md +++ b/docs/production/mobile-push-notifications.md @@ -226,9 +226,6 @@ push notifications through the new app is quite straightforward: `APNS_TOKEN_KEY_ID` to the corresponding 10-character key ID, and `APNS_TEAM_ID` to your 10-character Apple team ID. -- Set the `APNS_TOPIC` setting to the ID for - your app (for the official Zulip apps, it's `org.zulip.Zulip`). - - Restart the Zulip server. [apple-doc-cert]: https://developer.apple.com/documentation/usernotifications/setting_up_a_remote_notification_server/establishing_a_certificate-based_connection_to_apns diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 9f7acee4b4..fb47491209 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -178,10 +178,14 @@ def get_apns_context() -> Optional[APNsContext]: key=settings.APNS_TOKEN_KEY_FILE, key_id=settings.APNS_TOKEN_KEY_ID, team_id=settings.APNS_TEAM_ID, - topic=settings.APNS_TOPIC, max_connection_attempts=APNS_MAX_RETRIES, use_sandbox=settings.APNS_SANDBOX, err_func=err_func, + # The actual APNs topic will vary between notifications, + # so we set it there, overriding any value we put here. + # We can't just leave this out, though, because then + # the constructor attempts to guess. + topic="invalid.nonsense", ) apns = loop.run_until_complete(make_apns()) @@ -262,6 +266,7 @@ def send_apple_push_notification( payload_data = dict(modernize_apns_payload(payload_data)) message = {**payload_data.pop("custom", {}), "aps": payload_data} + have_missing_app_id = False for device in devices: if device.ios_app_id is None: # This should be present for all APNs tokens, as an invariant maintained @@ -269,13 +274,19 @@ def send_apple_push_notification( logger.error( "APNs: Missing ios_app_id for user %s device %s", user_identity, device.token ) + have_missing_app_id = True + if have_missing_app_id: + devices = [device for device in devices if device.ios_app_id is not None] async def send_all_notifications() -> Iterable[ Tuple[DeviceToken, Union[aioapns.common.NotificationResult, BaseException]] ]: requests = [ aioapns.NotificationRequest( - device_token=device.token, message=message, time_to_live=24 * 3600 + apns_topic=device.ios_app_id, + device_token=device.token, + message=message, + time_to_live=24 * 3600, ) for device in devices ] diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index 731ce5a7be..d46b676d5e 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -1491,17 +1491,20 @@ class PushNotificationTest(BouncerTestCase): apns_context.loop.close() def setup_apns_tokens(self) -> None: - self.tokens = ["aaaa", "bbbb"] - for token in self.tokens: + self.tokens = [("aaaa", "org.zulip.Zulip"), ("bbbb", "com.zulip.flutter")] + for token, appid in self.tokens: PushDeviceToken.objects.create( kind=PushDeviceToken.APNS, token=hex_to_b64(token), user=self.user_profile, - ios_app_id="org.zulip.Zulip", + ios_app_id=appid, ) - self.remote_tokens = [("cccc", "ffff")] - for id_token, uuid_token in self.remote_tokens: + self.remote_tokens = [ + ("cccc", "dddd", "org.zulip.Zulip"), + ("eeee", "ffff", "com.zulip.flutter"), + ] + for id_token, uuid_token, appid in self.remote_tokens: # We want to set up both types of RemotePushDeviceToken here: # the legacy one with user_id and the new with user_uuid. # This allows tests to work with either, without needing to @@ -1509,12 +1512,14 @@ class PushNotificationTest(BouncerTestCase): RemotePushDeviceToken.objects.create( kind=RemotePushDeviceToken.APNS, token=hex_to_b64(id_token), + ios_app_id=appid, user_id=self.user_profile.id, server=RemoteZulipServer.objects.get(uuid=self.server_uuid), ) RemotePushDeviceToken.objects.create( kind=RemotePushDeviceToken.APNS, token=hex_to_b64(uuid_token), + ios_app_id=appid, user_uuid=self.user_profile.uuid, server=RemoteZulipServer.objects.get(uuid=self.server_uuid), ) @@ -1607,6 +1612,16 @@ class HandlePushNotificationTest(PushNotificationTest): } send_notification.return_value.is_successful = True handle_push_notification(self.user_profile.id, missed_message) + self.assertEqual( + { + (args[0][0].device_token, args[0][0].apns_topic) + for args in send_notification.call_args_list + }, + { + (device.token, device.ios_app_id) + for device in RemotePushDeviceToken.objects.filter(kind=PushDeviceToken.APNS) + }, + ) self.assertEqual( views_logger.output, [ diff --git a/zproject/default_settings.py b/zproject/default_settings.py index 818e313a16..ed94392dd0 100644 --- a/zproject/default_settings.py +++ b/zproject/default_settings.py @@ -405,8 +405,8 @@ APNS_TOKEN_KEY_FILE: Optional[str] = None APNS_TOKEN_KEY_ID = get_secret("apns_token_key_id", development_only=True) APNS_TEAM_ID = get_secret("apns_team_id", development_only=True) APNS_SANDBOX = True -APNS_TOPIC = "org.zulip.Zulip" -# ZULIP_IOS_APP_ID is obsolete +# APNS_TOPIC is obsolete. Clients now pass the APNs topic to use. +# ZULIP_IOS_APP_ID is obsolete. Clients now pass the iOS app ID to use for APNs. # Limits related to the size of file uploads; last few in MB. DATA_UPLOAD_MAX_MEMORY_SIZE = 25 * 1024 * 1024