From 6407d0b1f96623760094c604be2e8d1243d06142 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Mon, 18 Nov 2019 18:12:54 -0800 Subject: [PATCH] push_notifications: Clear PushDeviceToken on API key change. This includes adding a new endpoint to the push notification bouncer interface, and code to call it appropriately after resetting a user's personal API key. When we add support for a user having multiple API keys, we may need to add an additional key here to support removing keys associated with just one client. --- zerver/lib/actions.py | 4 +++ zerver/lib/push_notifications.py | 12 ++++++++ zerver/tests/test_push_notifications.py | 39 ++++++++++++++++++++++++- zerver/worker/queue_processors.py | 4 ++- zilencer/urls.py | 2 ++ zilencer/views.py | 8 +++++ 6 files changed, 67 insertions(+), 2 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index a5c5399755..e405b84543 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -3336,6 +3336,10 @@ def do_regenerate_api_key(user_profile: UserProfile, acting_user: UserProfile) - )), bot_owner_user_ids(user_profile)) + event = {'type': 'clear_push_device_tokens', + 'user_profile_id': user_profile.id} + queue_json_publish("deferred_work", event) + return new_api_key def notify_avatar_url_change(user_profile: UserProfile) -> None: diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 086c7f252f..9d9712ddae 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -420,6 +420,18 @@ def remove_push_device_token(user_profile: UserProfile, token_str: str, kind: in except PushDeviceToken.DoesNotExist: raise JsonableError(_("Token does not exist")) +def clear_push_device_tokens(user_profile_id: int) -> None: + # Deletes all of a user's PushDeviceTokens. + if uses_notification_bouncer(): + post_data = { + 'server_uuid': settings.ZULIP_ORG_ID, + 'user_id': user_profile_id, + } + send_to_push_bouncer("POST", "push/unregister/all", post_data) + return + + PushDeviceToken.objects.filter(user_id=user_profile_id).delete() + # # Push notifications in general # diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index bc8d692a6e..a059466408 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -37,7 +37,11 @@ from zerver.models import ( Stream, Subscription, ) -from zerver.lib.actions import do_delete_messages, do_mark_stream_messages_as_read +from zerver.lib.actions import ( + do_delete_messages, + do_mark_stream_messages_as_read, + do_regenerate_api_key, +) from zerver.lib.soft_deactivation import do_soft_deactivate_users from zerver.lib.push_notifications import ( absolute_avatar_url, @@ -225,6 +229,24 @@ class PushBouncerNotificationTest(BouncerTestCase): result = self.api_post(self.server_uuid, endpoint, payload) self.assert_json_error(result, 'Empty or invalid length token') + def test_remote_push_unregister_all(self) -> None: + payload = self.get_generic_payload('register') + + # Verify correct results are success + result = self.api_post(self.server_uuid, + '/api/v1/remotes/push/register', payload) + self.assert_json_success(result) + + remote_tokens = RemotePushDeviceToken.objects.filter(token=payload['token']) + self.assertEqual(len(remote_tokens), 1) + result = self.api_post(self.server_uuid, + '/api/v1/remotes/push/unregister/all', + dict(user_id=10)) + self.assert_json_success(result) + + remote_tokens = RemotePushDeviceToken.objects.filter(token=payload['token']) + self.assertEqual(len(remote_tokens), 0) + def test_invalid_apns_token(self) -> None: endpoints = [ ('/api/v1/remotes/push/register', 'apple-token'), @@ -307,6 +329,21 @@ class PushBouncerNotificationTest(BouncerTestCase): server=server)) self.assertEqual(len(tokens), 0) + # Re-add copies of those tokens + for endpoint, token, kind in endpoints: + result = self.client_post(endpoint, {'token': token}, + subdomain="zulip") + self.assert_json_success(result) + tokens = list(RemotePushDeviceToken.objects.filter(user_id=user.id, + server=server)) + self.assertEqual(len(tokens), 2) + + # Remove it using the bouncer after an API key change + do_regenerate_api_key(user, user) + tokens = list(RemotePushDeviceToken.objects.filter(user_id=user.id, + server=server)) + self.assertEqual(len(tokens), 0) + class AnalyticsBouncerTest(BouncerTestCase): TIME_ZERO = datetime.datetime(1988, 3, 14).replace(tzinfo=timezone_utc) diff --git a/zerver/worker/queue_processors.py b/zerver/worker/queue_processors.py index 078af416bd..7b19561f66 100644 --- a/zerver/worker/queue_processors.py +++ b/zerver/worker/queue_processors.py @@ -25,7 +25,7 @@ from zerver.lib.queue import SimpleQueueClient, queue_json_publish, retry_event from zerver.lib.timestamp import timestamp_to_datetime from zerver.lib.email_notifications import handle_missedmessage_emails from zerver.lib.push_notifications import handle_push_notification, handle_remove_push_notification, \ - initialize_push_notifications + initialize_push_notifications, clear_push_device_tokens from zerver.lib.actions import do_send_confirmation_email, \ do_update_user_activity, do_update_user_activity_interval, do_update_user_presence, \ internal_send_message, internal_send_private_message, notify_realm_export, \ @@ -667,6 +667,8 @@ class DeferredWorker(QueueProcessingWorker): (stream, recipient, sub) = access_stream_by_id(user_profile, stream_id, require_active=False) do_mark_stream_messages_as_read(user_profile, client, stream) + elif event['type'] == 'clear_push_device_tokens': + clear_push_device_tokens(event["user_profile_id"]) elif event['type'] == 'realm_export': start = time.time() realm = Realm.objects.get(id=event['realm_id']) diff --git a/zilencer/urls.py b/zilencer/urls.py index 8337a28ef6..19dd1816a3 100644 --- a/zilencer/urls.py +++ b/zilencer/urls.py @@ -13,6 +13,8 @@ v1_api_and_json_patterns = [ {'POST': 'zilencer.views.register_remote_push_device'}), url('^remotes/push/unregister$', rest_dispatch, {'POST': 'zilencer.views.unregister_remote_push_device'}), + url('^remotes/push/unregister/all$', rest_dispatch, + {'POST': 'zilencer.views.unregister_all_remote_push_devices'}), url('^remotes/push/notify$', rest_dispatch, {'POST': 'zilencer.views.remote_server_notify_push'}), diff --git a/zilencer/views.py b/zilencer/views.py index 7a896ffd77..21f848bee6 100644 --- a/zilencer/views.py +++ b/zilencer/views.py @@ -120,6 +120,14 @@ def unregister_remote_push_device(request: HttpRequest, entity: Union[UserProfil return json_success() +@has_request_variables +def unregister_all_remote_push_devices(request: HttpRequest, entity: Union[UserProfile, RemoteZulipServer], + user_id: int=REQ(validator=check_int)) -> HttpResponse: + server = validate_entity(entity) + RemotePushDeviceToken.objects.filter(user_id=user_id, + server=server).delete() + return json_success() + @has_request_variables def remote_server_notify_push(request: HttpRequest, entity: Union[UserProfile, RemoteZulipServer], payload: Dict[str, Any]=REQ(argument_type='body')) -> HttpResponse: