push_notifications: Store tokens locally even when bouncer is used.

This makes the system store and track PushDeviceToken objects on
the local Zulip server when using the push notifications bouncer
and includes tests for this.

This is something we need to implement end-to-end encryption for
push notifications. We'll add the encryption key as an additional
property on the local PushDeviceToken object.

It also likely adds some value in the case that a server were to
switch between using the bouncer service and sending notifications
directly, though in practice that's unlikely to happen.

zerver/lib/push_notifications.py

@@ -371,6 +371,23 @@ def add_push_device_token(user_profile: UserProfile,
     logger.info("Registering push device: %d %r %d %r",
                 user_profile.id, token_str, kind, ios_app_id)
 
+    # Regardless of whether we're using the push notifications
+    # bouncer, we want to store a PushDeviceToken record locally.
+    # These can be used to discern whether the user has any mobile
+    # devices configured, and is also where we will store encryption
+    # keys for mobile push notifications.
+    try:
+        with transaction.atomic():
+            PushDeviceToken.objects.create(
+                user_id=user_profile.id,
+                kind=kind,
+                token=token_str,
+                ios_app_id=ios_app_id,
+                # last_updated is to be renamed to date_created.
+                last_updated=timezone_now())
+    except IntegrityError:
+        pass
+
     # If we're sending things to the push notification bouncer
     # register this user with them here
     if uses_notification_bouncer():
@@ -387,21 +404,19 @@ def add_push_device_token(user_profile: UserProfile,
         logger.info("Sending new push device to bouncer: %r", post_data)
         # Calls zilencer.views.register_remote_push_device
         send_to_push_bouncer('POST', 'push/register', post_data)
-        return
-
-    try:
-        with transaction.atomic():
-            PushDeviceToken.objects.create(
-                user_id=user_profile.id,
-                kind=kind,
-                token=token_str,
-                ios_app_id=ios_app_id,
-                # last_updated is to be renamed to date_created.
-                last_updated=timezone_now())
-    except IntegrityError:
-        pass
 
 def remove_push_device_token(user_profile: UserProfile, token_str: str, kind: int) -> None:
+    try:
+        token = PushDeviceToken.objects.get(token=token_str, kind=kind, user=user_profile)
+        token.delete()
+    except PushDeviceToken.DoesNotExist:
+        # If we are using bouncer, don't raise the exception. It will
+        # be raised by the code below eventually. This is important
+        # during the transition period after upgrading to a version
+        # that stores local PushDeviceToken objects even when using
+        # the push notifications bouncer.
+        if not uses_notification_bouncer():
+            raise JsonableError(_("Token does not exist"))
 
     # If we're sending things to the push notification bouncer
     # unregister this user with them here
@@ -415,13 +430,6 @@ def remove_push_device_token(user_profile: UserProfile, token_str: str, kind: in
         }
         # Calls zilencer.views.unregister_remote_push_device
         send_to_push_bouncer("POST", "push/unregister", post_data)
-        return
-
-    try:
-        token = PushDeviceToken.objects.get(token=token_str, kind=kind, user=user_profile)
-        token.delete()
-    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.

zerver/tests/test_push_notifications.py

@@ -1615,8 +1615,8 @@ class TestNumPushDevicesForUser(PushNotificationTest):
                                           kind=PushDeviceToken.APNS)
         self.assertEqual(count, 2)
 
-class TestPushApi(ZulipTestCase):
+class TestPushApi(BouncerTestCase):
-    def test_push_api(self) -> None:
+    def test_push_api_error_handling(self) -> None:
         user = self.example_user('cordelia')
         self.login_user(user)
 
@@ -1643,9 +1643,31 @@ class TestPushApi(ZulipTestCase):
             result = self.client_delete(endpoint, {'token': 'abcd1234'})
             self.assert_json_error(result, 'Token does not exist')
 
-        # Add tokens
+            # Use push notification bouncer and try to remove non-existing tokens.
-        for endpoint, token in endpoints:
+            with self.settings(PUSH_NOTIFICATION_BOUNCER_URL='https://push.zulip.org.example.com'), \
-            # Test that we can push twice
+                mock.patch('zerver.lib.remote_server.requests.request',
+                           side_effect=self.bounce_request) as remote_server_request:
+                result = self.client_delete(endpoint, {'token': 'abcd1234'})
+                self.assert_json_error(result, 'Token does not exist')
+                remote_server_request.assert_called_once()
+
+    def test_push_api_add_and_remove_device_tokens(self) -> None:
+        user = self.example_user('cordelia')
+        self.login_user(user)
+
+        no_bouncer_requests = [
+            ('/json/users/me/apns_device_token', 'apple-tokenaa'),
+            ('/json/users/me/android_gcm_reg_id', 'android-token-1'),
+        ]
+
+        bouncer_requests = [
+            ('/json/users/me/apns_device_token', 'apple-tokenbb'),
+            ('/json/users/me/android_gcm_reg_id', 'android-token-2'),
+        ]
+
+        # Add tokens without using push notification bouncer.
+        for endpoint, token in no_bouncer_requests:
+            # Test that we can push twice.
             result = self.client_post(endpoint, {'token': token})
             self.assert_json_success(result)
 
@@ -1656,17 +1678,59 @@ class TestPushApi(ZulipTestCase):
             self.assertEqual(len(tokens), 1)
             self.assertEqual(tokens[0].token, token)
 
-        # User should have tokens for both devices now.
+        with self.settings(PUSH_NOTIFICATION_BOUNCER_URL='https://push.zulip.org.example.com'), \
-        tokens = list(PushDeviceToken.objects.filter(user=user))
+            mock.patch('zerver.lib.remote_server.requests.request',
-        self.assertEqual(len(tokens), 2)
+                       side_effect=self.bounce_request):
+            # Enable push notification bouncer and add tokens.
+            for endpoint, token in bouncer_requests:
+                # Test that we can push twice.
+                result = self.client_post(endpoint, {'token': token})
+                self.assert_json_success(result)
 
-        # Remove tokens
+                result = self.client_post(endpoint, {'token': token})
-        for endpoint, token in endpoints:
+                self.assert_json_success(result)
+
+                tokens = list(PushDeviceToken.objects.filter(user=user, token=token))
+                self.assertEqual(len(tokens), 1)
+                self.assertEqual(tokens[0].token, token)
+
+                tokens = list(RemotePushDeviceToken.objects.filter(user_id=user.id, token=token))
+                self.assertEqual(len(tokens), 1)
+                self.assertEqual(tokens[0].token, token)
+
+        # PushDeviceToken will include all the device tokens.
+        tokens = list(PushDeviceToken.objects.values_list('token', flat=True))
+        self.assertEqual(tokens, ['apple-tokenaa', 'android-token-1', 'apple-tokenbb', 'android-token-2'])
+
+        # RemotePushDeviceToken will only include tokens of
+        # the devices using push notification bouncer.
+        remote_tokens = list(RemotePushDeviceToken.objects.values_list('token', flat=True))
+        self.assertEqual(remote_tokens, ['apple-tokenbb', 'android-token-2'])
+
+        # Test removing tokens without using push notification bouncer.
+        for endpoint, token in no_bouncer_requests:
             result = self.client_delete(endpoint, {'token': token})
             self.assert_json_success(result)
             tokens = list(PushDeviceToken.objects.filter(user=user, token=token))
             self.assertEqual(len(tokens), 0)
 
+        # Use push notification bouncer and test removing device tokens.
+        # Tokens will be removed both locally and remotely.
+        with self.settings(PUSH_NOTIFICATION_BOUNCER_URL='https://push.zulip.org.example.com'), \
+            mock.patch('zerver.lib.remote_server.requests.request',
+                       side_effect=self.bounce_request):
+            for endpoint, token in bouncer_requests:
+                result = self.client_delete(endpoint, {'token': token})
+                self.assert_json_success(result)
+                tokens = list(PushDeviceToken.objects.filter(user=user, token=token))
+                remote_tokens = list(RemotePushDeviceToken.objects.filter(user_id=user.id, token=token))
+                self.assertEqual(len(tokens), 0)
+                self.assertEqual(len(remote_tokens), 0)
+
+        # Verify that the above process indeed removed all the tokens we created.
+        self.assertEqual(RemotePushDeviceToken.objects.all().count(), 0)
+        self.assertEqual(PushDeviceToken.objects.all().count(), 0)
+
 class GCMParseOptionsTest(TestCase):
     def test_invalid_option(self) -> None:
         with self.assertRaises(JsonableError):