mirror of https://github.com/zulip/zulip.git
APNs: Rip out the existing, broken implementation.
This code empirically doesn't work. It's not entirely clear why, even having done quite a bit of debugging; partly because the code is quite convoluted, and because it shows the symptoms of people making changes over time without really understanding how it was supposed to work. Moreover, this code targets an old version of the APNs provider API. Apple deprecated that in 2015, in favor of a shiny new one which uses HTTP/2 to meet the same needs for concurrency and scale that the old one had to do a bunch of ad-hoc protocol design for. So, rip this code out. We'll build a pathway to the new API from scratch; it's not that complicated.
This commit is contained in:
Greg Price
Tim Abbott
parent
3bceeec89f
commit
d02101a401
|
|
@ -1,4 +0,0 @@
|
|||
MAILTO=root
|
||||
|
||||
# Remove any stale apple device tokens from our list
|
||||
0 3 * * * zulip cd /home/zulip/deployments/current && ./manage.py check_apns_tokens
|
||||
|
|
@ -49,10 +49,6 @@ class zulip_ops::app_frontend {
|
|||
source => 'puppet:///modules/zulip_ops/log2zulip.zuliprc',
|
||||
}
|
||||
file { "/etc/cron.d/check-apns-tokens":
|
||||
ensure => file,
|
||||
owner => "root",
|
||||
group => "root",
|
||||
mode => 644,
|
||||
source => "puppet:///modules/zulip_ops/cron.d/check-apns-tokens",
|
||||
ensure => absent,
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -14,10 +14,8 @@ from zerver.lib.request import JsonableError
|
|||
from zerver.lib.timestamp import datetime_to_timestamp, timestamp_to_datetime
|
||||
from zerver.decorator import statsd_increment
|
||||
from zerver.lib.utils import generate_random_token
|
||||
from zerver.lib.redis_utils import get_redis_client
|
||||
from zerver.lib.queue import retry_event
|
||||
|
||||
from apns import APNs, Frame, Payload, SENT_BUFFER_QTY
|
||||
from gcm import GCM
|
||||
|
||||
from django.conf import settings
|
||||
|
|
@ -41,26 +39,6 @@ else: # nocoverage -- Not convenient to add test for this.
|
|||
|
||||
DeviceToken = Union[PushDeviceToken, RemotePushDeviceToken]
|
||||
|
||||
# APNS error codes
|
||||
ERROR_CODES = {
|
||||
1: 'Processing error',
|
||||
2: 'Missing device token', # looks like token was empty?
|
||||
3: 'Missing topic', # topic is encoded in the certificate, looks like certificate is wrong. bail out.
|
||||
4: 'Missing payload', # bail out, our message looks like empty
|
||||
5: 'Invalid token size', # current token has wrong size, skip it and retry
|
||||
6: 'Invalid topic size', # can not happen, we do not send topic, it is part of certificate. bail out.
|
||||
7: 'Invalid payload size', # our payload is probably too big. bail out.
|
||||
8: 'Invalid token', # our device token is broken, skipt it and retry
|
||||
10: 'Shutdown', # server went into maintenance mode. reported token is the last success, skip it and retry.
|
||||
None: 'Unknown', # unknown error, for sure we try again, but user should limit number of retries
|
||||
}
|
||||
|
||||
redis_client = get_redis_client()
|
||||
|
||||
# Maintain a long-lived Session object to avoid having to re-SSL-handshake
|
||||
# for each request
|
||||
connection = None
|
||||
|
||||
# `APNS_SANDBOX` should be a bool
|
||||
assert isinstance(settings.APNS_SANDBOX, bool)
|
||||
|
||||
|
|
@ -68,81 +46,6 @@ def uses_notification_bouncer():
|
|||
# type: () -> bool
|
||||
return settings.PUSH_NOTIFICATION_BOUNCER_URL is not None
|
||||
|
||||
def get_apns_key(identifer):
|
||||
# type: (SupportsInt) -> str
|
||||
return 'apns:' + str(identifer)
|
||||
|
||||
class APNsMessage(object):
|
||||
def __init__(self, user_id, tokens, alert=None, badge=None, sound=None,
|
||||
category=None, **kwargs):
|
||||
# type: (int, List[Text], Text, int, Text, Text, **Any) -> None
|
||||
self.frame = Frame()
|
||||
self.tokens = tokens
|
||||
expiry = int(time.time() + 24 * 3600)
|
||||
priority = 10
|
||||
payload = Payload(alert=alert, badge=badge, sound=sound,
|
||||
category=category, custom=kwargs)
|
||||
for token in tokens:
|
||||
data = {'token': token, 'user_id': user_id}
|
||||
identifier = random.getrandbits(32)
|
||||
key = get_apns_key(identifier)
|
||||
redis_client.hmset(key, data)
|
||||
redis_client.expire(key, expiry)
|
||||
self.frame.add_item(token, payload, identifier, expiry, priority)
|
||||
|
||||
def get_frame(self):
|
||||
# type: () -> Frame
|
||||
return self.frame
|
||||
|
||||
def response_listener(error_response):
|
||||
# type: (Dict[str, SupportsInt]) -> None
|
||||
identifier = error_response['identifier']
|
||||
key = get_apns_key(identifier)
|
||||
if not redis_client.exists(key):
|
||||
logging.warn("APNs key, {}, doesn't not exist.".format(key))
|
||||
return
|
||||
|
||||
code = error_response['status']
|
||||
assert isinstance(code, int)
|
||||
|
||||
errmsg = ERROR_CODES[code]
|
||||
data = redis_client.hgetall(key)
|
||||
token = data['token']
|
||||
user_id = int(data['user_id'])
|
||||
b64_token = hex_to_b64(token)
|
||||
|
||||
logging.warn("APNS: Failed to deliver APNS notification to %s, reason: %s" % (b64_token, errmsg))
|
||||
if code == 8:
|
||||
# Invalid Token, remove from our database
|
||||
logging.warn("APNS: Removing token from database due to above failure")
|
||||
try:
|
||||
PushDeviceToken.objects.get(user_id=user_id, token=b64_token).delete()
|
||||
return # No need to check RemotePushDeviceToken
|
||||
except PushDeviceToken.DoesNotExist:
|
||||
pass
|
||||
|
||||
if settings.ZILENCER_ENABLED:
|
||||
# Trying to delete from both models is a bit inefficient than
|
||||
# deleting from only one model but this method is very simple.
|
||||
try:
|
||||
RemotePushDeviceToken.objects.get(user_id=user_id,
|
||||
token=b64_token).delete()
|
||||
except RemotePushDeviceToken.DoesNotExist:
|
||||
pass
|
||||
|
||||
def get_connection(cert_file, key_file):
|
||||
# type: (str, str) -> APNs
|
||||
connection = APNs(use_sandbox=settings.APNS_SANDBOX,
|
||||
cert_file=cert_file,
|
||||
key_file=key_file,
|
||||
enhanced=True)
|
||||
connection.gateway_server.register_response_listener(response_listener)
|
||||
return connection
|
||||
|
||||
if settings.APNS_CERT_FILE is not None and os.path.exists(settings.APNS_CERT_FILE): # nocoverage
|
||||
connection = get_connection(settings.APNS_CERT_FILE,
|
||||
settings.APNS_KEY_FILE)
|
||||
|
||||
def num_push_devices_for_user(user_profile, kind = None):
|
||||
# type: (UserProfile, Optional[int]) -> PushDeviceToken
|
||||
if kind is None:
|
||||
|
|
@ -159,65 +62,13 @@ def hex_to_b64(data):
|
|||
# type: (Text) -> bytes
|
||||
return base64.b64encode(binascii.unhexlify(data.encode('utf-8')))
|
||||
|
||||
def _do_push_to_apns_service(user_id, message, apns_connection):
|
||||
# type: (int, APNsMessage, APNs) -> None
|
||||
if not apns_connection: # nocoverage
|
||||
logging.info("Not delivering APNS message %s to user %s due to missing connection" % (message, user_id))
|
||||
return
|
||||
|
||||
frame = message.get_frame()
|
||||
apns_connection.gateway_server.send_notification_multiple(frame)
|
||||
|
||||
def send_apple_push_notification_to_user(user, alert, **extra_data):
|
||||
# type: (UserProfile, Text, **Any) -> None
|
||||
devices = PushDeviceToken.objects.filter(user=user, kind=PushDeviceToken.APNS)
|
||||
send_apple_push_notification(user.id, devices, zulip=dict(alert=alert),
|
||||
**extra_data)
|
||||
|
||||
# Send a push notification to the desired clients
|
||||
# extra_data is a dict that will be passed to the
|
||||
# mobile app
|
||||
@statsd_increment("apple_push_notification")
|
||||
def send_apple_push_notification(user_id, devices, **extra_data):
|
||||
# type: (int, List[DeviceToken], **Any) -> None
|
||||
if not connection:
|
||||
logging.warning("Attempting to send push notification, but no connection was found. "
|
||||
"This may be because we could not find the APNS Certificate file.")
|
||||
if not devices:
|
||||
return
|
||||
|
||||
# Plain b64 token kept for debugging purposes
|
||||
tokens = [(b64_to_hex(device.token), device.ios_app_id, device.token)
|
||||
for device in devices]
|
||||
|
||||
valid_devices = [device for device in tokens if device[1] in [settings.ZULIP_IOS_APP_ID, None]]
|
||||
valid_tokens = [device[0] for device in valid_devices]
|
||||
if valid_tokens:
|
||||
logging.info("APNS: Sending apple push notification "
|
||||
"to devices: %s" % (valid_devices,))
|
||||
zulip_message = APNsMessage(user_id, valid_tokens,
|
||||
alert=extra_data['zulip']['alert'],
|
||||
**extra_data)
|
||||
_do_push_to_apns_service(user_id, zulip_message, connection)
|
||||
else: # nocoverage
|
||||
logging.warn("APNS: Not sending notification because "
|
||||
"tokens didn't match devices: %s/%s" % (tokens, settings.ZULIP_IOS_APP_ID,))
|
||||
|
||||
# NOTE: This is used by the check_apns_tokens manage.py command. Do not call it otherwise, as the
|
||||
# feedback() call can take up to 15s
|
||||
def check_apns_feedback():
|
||||
# type: () -> None
|
||||
feedback_connection = APNs(use_sandbox=settings.APNS_SANDBOX,
|
||||
cert_file=settings.APNS_CERT_FILE,
|
||||
key_file=settings.APNS_KEY_FILE)
|
||||
|
||||
for token, since in feedback_connection.feedback_server.items():
|
||||
since_date = timestamp_to_datetime(since)
|
||||
logging.info("Found unavailable token %s, unavailable since %s" % (token, since_date))
|
||||
|
||||
PushDeviceToken.objects.filter(token=hex_to_b64(token), last_updated__lt=since_date,
|
||||
kind=PushDeviceToken.APNS).delete()
|
||||
logging.info("Finished checking feedback for stale tokens")
|
||||
|
||||
logging.warn("APNs unimplemented. Dropping notification for user %d with %d devices.",
|
||||
user_id, len(devices))
|
||||
|
||||
if settings.ANDROID_GCM_API_KEY: # nocoverage
|
||||
gcm = GCM(settings.ANDROID_GCM_API_KEY)
|
||||
|
|
|
|||
|
|
@ -1,16 +0,0 @@
|
|||
from __future__ import absolute_import
|
||||
|
||||
from typing import Any
|
||||
|
||||
from django.core.management.base import BaseCommand
|
||||
from zerver.lib.push_notifications import check_apns_feedback
|
||||
|
||||
class Command(BaseCommand):
|
||||
help = """Checks the Apple Push Notifications Service for any tokens that have been
|
||||
invalidated, and removes them from the database.
|
||||
|
||||
Usage: ./manage.py check_apns_tokens"""
|
||||
|
||||
def handle(self, *args, **options):
|
||||
# type: (*Any, **Any) -> None
|
||||
check_apns_feedback()
|
||||
|
|
@ -34,30 +34,6 @@ from zerver.lib.test_classes import (
|
|||
from zilencer.models import RemoteZulipServer, RemotePushDeviceToken
|
||||
from django.utils.timezone import now
|
||||
|
||||
class MockRedis(object):
|
||||
data = {} # type: Dict[str, Any]
|
||||
|
||||
def hgetall(self, key):
|
||||
# type: (str) -> Any
|
||||
return self.data.get(key)
|
||||
|
||||
def exists(self, key):
|
||||
# type: (str) -> bool
|
||||
return key in self.data
|
||||
|
||||
def hmset(self, key, data):
|
||||
# type: (str, Dict[Any, Any]) -> None
|
||||
self.data[key] = data
|
||||
|
||||
def delete(self, key):
|
||||
# type: (str) -> None
|
||||
if self.exists(key):
|
||||
del self.data[key]
|
||||
|
||||
def expire(self, *args, **kwargs):
|
||||
# type: (*Any, **Any) -> None
|
||||
pass
|
||||
|
||||
class BouncerTestCase(ZulipTestCase):
|
||||
def setUp(self):
|
||||
# type: () -> None
|
||||
|
|
@ -255,8 +231,6 @@ class PushNotificationTest(BouncerTestCase):
|
|||
# type: () -> None
|
||||
super(PushNotificationTest, self).setUp()
|
||||
self.user_profile = self.example_user('hamlet')
|
||||
apn.connection = apn.get_connection('fake-cert', 'fake-key')
|
||||
self.redis_client = apn.redis_client = MockRedis()
|
||||
self.tokens = [u'aaaa', u'bbbb']
|
||||
for token in self.tokens:
|
||||
PushDeviceToken.objects.create(
|
||||
|
|
@ -277,12 +251,6 @@ class PushNotificationTest(BouncerTestCase):
|
|||
self.sending_client = get_client('test')
|
||||
self.sender = self.example_user('hamlet')
|
||||
|
||||
def tearDown(self):
|
||||
# type: () -> None
|
||||
super(PushNotificationTest, self).tearDown()
|
||||
for i in [100, 200]:
|
||||
self.redis_client.delete(apn.get_apns_key(i))
|
||||
|
||||
def get_message(self, type, type_id=100):
|
||||
# type: (int, int) -> Message
|
||||
recipient, _ = Recipient.objects.get_or_create(
|
||||
|
|
@ -337,9 +305,9 @@ class HandlePushNotificationTest(PushNotificationTest):
|
|||
with self.settings(PUSH_NOTIFICATION_BOUNCER_URL=''), \
|
||||
mock.patch('zerver.lib.push_notifications.requests.request',
|
||||
side_effect=self.bounce_request), \
|
||||
mock.patch('zerver.lib.push_notifications._do_push_to_apns_service'), \
|
||||
mock.patch('zerver.lib.push_notifications.gcm') as mock_gcm, \
|
||||
mock.patch('logging.info') as mock_info:
|
||||
mock.patch('logging.info') as mock_info, \
|
||||
mock.patch('logging.warn') as mock_warn:
|
||||
apns_devices = [
|
||||
(apn.b64_to_hex(device.token), device.ios_app_id, device.token)
|
||||
for device in RemotePushDeviceToken.objects.filter(
|
||||
|
|
@ -353,9 +321,9 @@ class HandlePushNotificationTest(PushNotificationTest):
|
|||
mock_gcm.json_request.return_value = {
|
||||
'success': {gcm_devices[0][2]: message.id}}
|
||||
apn.handle_push_notification(self.user_profile.id, missed_message)
|
||||
mock_info.assert_called_with(
|
||||
"APNS: Sending apple push "
|
||||
"notification to devices: %s" % (apns_devices,))
|
||||
mock_warn.assert_called_with(
|
||||
"APNs unimplemented. Dropping notification for user %d with %d devices.",
|
||||
self.user_profile.id, len(apns_devices))
|
||||
for _, _, token in gcm_devices:
|
||||
mock_info.assert_any_call(
|
||||
"GCM: Sent %s as %s" % (token, message.id))
|
||||
|
|
@ -500,96 +468,6 @@ class HandlePushNotificationTest(PushNotificationTest):
|
|||
mock_logger.assert_called_with("Could not find UserMessage with "
|
||||
"message_id 100")
|
||||
|
||||
class APNsMessageTest(PushNotificationTest):
|
||||
@mock.patch('random.getrandbits', side_effect=[100, 200])
|
||||
def test_apns_message(self, mock_getrandbits):
|
||||
# type: (mock.MagicMock) -> None
|
||||
apn.APNsMessage(self.user_profile.id, self.tokens, alert="test")
|
||||
data = self.redis_client.hgetall(apn.get_apns_key(100))
|
||||
self.assertEqual(data['token'], 'aaaa')
|
||||
self.assertEqual(int(data['user_id']), self.user_profile.id)
|
||||
data = self.redis_client.hgetall(apn.get_apns_key(200))
|
||||
self.assertEqual(data['token'], 'bbbb')
|
||||
self.assertEqual(int(data['user_id']), self.user_profile.id)
|
||||
|
||||
class ResponseListenerTest(PushNotificationTest):
|
||||
def get_error_response(self, **kwargs):
|
||||
# type: (**Any) -> Dict[str, SupportsInt]
|
||||
er = {'identifier': 0, 'status': 0} # type: Dict[str, SupportsInt]
|
||||
er.update({k: v for k, v in kwargs.items() if k in er})
|
||||
return er
|
||||
|
||||
def get_cache_value(self):
|
||||
# type: () -> Dict[str, Union[str, int]]
|
||||
return {'token': 'aaaa', 'user_id': self.user_profile.id}
|
||||
|
||||
@mock.patch('logging.warn')
|
||||
def test_cache_does_not_exist(self, mock_warn):
|
||||
# type: (mock.MagicMock) -> None
|
||||
err_rsp = self.get_error_response(identifier=100, status=1)
|
||||
apn.response_listener(err_rsp)
|
||||
msg = "APNs key, apns:100, doesn't not exist."
|
||||
mock_warn.assert_called_once_with(msg)
|
||||
|
||||
@mock.patch('logging.warn')
|
||||
def test_cache_exists(self, mock_warn):
|
||||
# type: (mock.MagicMock) -> None
|
||||
self.redis_client.hmset(apn.get_apns_key(100), self.get_cache_value())
|
||||
err_rsp = self.get_error_response(identifier=100, status=1)
|
||||
apn.response_listener(err_rsp)
|
||||
b64_token = apn.hex_to_b64('aaaa')
|
||||
errmsg = apn.ERROR_CODES[int(err_rsp['status'])]
|
||||
msg = ("APNS: Failed to deliver APNS notification to %s, "
|
||||
"reason: %s" % (b64_token, errmsg))
|
||||
mock_warn.assert_called_once_with(msg)
|
||||
|
||||
@mock.patch('logging.warn')
|
||||
def test_error_code_eight(self, mock_warn):
|
||||
# type: (mock.MagicMock) -> None
|
||||
self.redis_client.hmset(apn.get_apns_key(100), self.get_cache_value())
|
||||
err_rsp = self.get_error_response(identifier=100, status=8)
|
||||
b64_token = apn.hex_to_b64('aaaa')
|
||||
self.assertEqual(PushDeviceToken.objects.filter(
|
||||
user=self.user_profile, token=b64_token).count(), 1)
|
||||
apn.response_listener(err_rsp)
|
||||
self.assertEqual(mock_warn.call_count, 2)
|
||||
self.assertEqual(PushDeviceToken.objects.filter(
|
||||
user=self.user_profile, token=b64_token).count(), 0)
|
||||
|
||||
@mock.patch('logging.warn')
|
||||
def test_error_code_eight_when_token_doesnt_exist(self, mock_warn):
|
||||
# type: (mock.MagicMock) -> None
|
||||
cache_value = self.get_cache_value()
|
||||
cache_value['token'] = 'cccc'
|
||||
self.redis_client.hmset(apn.get_apns_key(100), cache_value)
|
||||
err_rsp = self.get_error_response(identifier=100, status=8)
|
||||
apn.response_listener(err_rsp)
|
||||
self.assertEqual(PushDeviceToken.objects.all().count(), 2)
|
||||
|
||||
@mock.patch('logging.warn')
|
||||
def test_error_code_eight_with_zilencer(self, mock_warn):
|
||||
# type: (mock.MagicMock) -> None
|
||||
cache_value = self.get_cache_value()
|
||||
cache_value['token'] = 'cccc'
|
||||
self.redis_client.hmset(apn.get_apns_key(100), cache_value)
|
||||
err_rsp = self.get_error_response(identifier=100, status=8)
|
||||
self.assertEqual(RemotePushDeviceToken.objects.all().count(), 1)
|
||||
with self.settings(ZILENCER_ENABLED=True):
|
||||
apn.response_listener(err_rsp)
|
||||
self.assertEqual(RemotePushDeviceToken.objects.all().count(), 0)
|
||||
|
||||
@mock.patch('logging.warn')
|
||||
def test_error_code_eight_with_zilencer_when_token_doesnt_exist(self, mock_warn):
|
||||
# type: (mock.MagicMock) -> None
|
||||
cache_value = self.get_cache_value()
|
||||
cache_value['token'] = 'dddd'
|
||||
self.redis_client.hmset(apn.get_apns_key(100), cache_value)
|
||||
err_rsp = self.get_error_response(identifier=100, status=8)
|
||||
self.assertEqual(RemotePushDeviceToken.objects.all().count(), 1)
|
||||
with self.settings(ZILENCER_ENABLED=True):
|
||||
apn.response_listener(err_rsp)
|
||||
self.assertEqual(RemotePushDeviceToken.objects.all().count(), 1)
|
||||
|
||||
class TestGetAlertFromMessage(PushNotificationTest):
|
||||
def test_get_alert_from_message(self):
|
||||
# type: () -> None
|
||||
|
|
@ -781,55 +659,6 @@ class TestPushApi(ZulipTestCase):
|
|||
tokens = list(PushDeviceToken.objects.filter(user=user, token=token))
|
||||
self.assertEqual(len(tokens), 0)
|
||||
|
||||
class SendNotificationTest(PushNotificationTest):
|
||||
@mock.patch('logging.warn')
|
||||
@mock.patch('logging.info')
|
||||
@mock.patch('zerver.lib.push_notifications._do_push_to_apns_service')
|
||||
def test_send_apple_push_notifiction(self, mock_send, mock_info, mock_warn):
|
||||
# type: (mock.MagicMock, mock.MagicMock, mock.MagicMock) -> None
|
||||
def test_send(user_id, message, alert):
|
||||
# type: (int, Message, str) -> None
|
||||
self.assertEqual(user_id, self.user_profile.id)
|
||||
self.assertEqual(set(message.tokens), set(self.tokens))
|
||||
|
||||
mock_send.side_effect = test_send
|
||||
apn.send_apple_push_notification_to_user(self.user_profile, "test alert")
|
||||
self.assertEqual(mock_send.call_count, 1)
|
||||
|
||||
@mock.patch('apns.GatewayConnection.send_notification_multiple')
|
||||
def test_do_push_to_apns_service(self, mock_push):
|
||||
# type: (mock.MagicMock) -> None
|
||||
msg = apn.APNsMessage(self.user_profile.id, self.tokens, alert="test")
|
||||
|
||||
def test_push(message):
|
||||
# type: (Message) -> None
|
||||
self.assertIs(message, msg.get_frame())
|
||||
|
||||
mock_push.side_effect = test_push
|
||||
apn._do_push_to_apns_service(self.user_profile.id, msg, apn.connection)
|
||||
|
||||
@mock.patch('logging.warn')
|
||||
@mock.patch('logging.info')
|
||||
@mock.patch('apns.GatewayConnection.send_notification_multiple')
|
||||
def test_connection_none(self, mock_push, mock_info, mock_warn):
|
||||
# type: (mock.MagicMock, mock.MagicMock, mock.MagicMock) -> None
|
||||
apn.connection = None
|
||||
apn.send_apple_push_notification_to_user(self.user_profile, "test alert")
|
||||
|
||||
class APNsFeedbackTest(PushNotificationTest):
|
||||
@mock.patch('logging.info')
|
||||
@mock.patch('apns.FeedbackConnection.items')
|
||||
def test_feedback(self, mock_items, mock_info):
|
||||
# type: (mock.MagicMock, mock.MagicMock) -> None
|
||||
update_time = apn.timestamp_to_datetime(int(time.time()) - 10000)
|
||||
PushDeviceToken.objects.all().update(last_updated=update_time)
|
||||
mock_items.return_value = [
|
||||
('aaaa', int(time.time())),
|
||||
]
|
||||
self.assertEqual(PushDeviceToken.objects.all().count(), 2)
|
||||
apn.check_apns_feedback()
|
||||
self.assertEqual(PushDeviceToken.objects.all().count(), 1)
|
||||
|
||||
class GCMTest(PushNotificationTest):
|
||||
def setUp(self):
|
||||
# type: () -> None
|
||||
|
|
|
|||
Loading…
Reference in New Issue