push_notifications: Remove vestiges of base64 storage of tokens.

APNs tokens are provided by the client in hex, and we store them in
hex.  The existing code which attempts to "validate" them by parsing
them as base64 only works because base64 is a superset of hex.

Enforce that APNs tokens are hex, and remove all of the pieces of test
code which were incorrectly passing them in as base64 strings.

analytics/tests/test_counts.py

@@ -50,11 +50,7 @@ from zerver.actions.user_activity import update_user_activity_interval
 from zerver.actions.users import do_deactivate_user
 from zerver.lib.create_user import create_user
 from zerver.lib.exceptions import InvitationError
-from zerver.lib.push_notifications import (
+from zerver.lib.push_notifications import get_message_payload_apns, get_message_payload_gcm
-    get_message_payload_apns,
-    get_message_payload_gcm,
-    hex_to_b64,
-)
 from zerver.lib.streams import get_default_values_for_stream_permission_group_settings
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.test_helpers import activate_push_notification_service
@@ -1369,19 +1365,19 @@ class TestLoggingCountStats(AnalyticsTestCase):
 
         RemotePushDeviceToken.objects.create(
             kind=RemotePushDeviceToken.FCM,
-            token=hex_to_b64(token),
+            token=token,
             user_uuid=(hamlet.uuid),
             server=self.server,
         )
         RemotePushDeviceToken.objects.create(
             kind=RemotePushDeviceToken.FCM,
-            token=hex_to_b64(token + "aa"),
+            token=token + "aa",
             user_uuid=(hamlet.uuid),
             server=self.server,
         )
         RemotePushDeviceToken.objects.create(
             kind=RemotePushDeviceToken.APNS,
-            token=hex_to_b64(token),
+            token=token,
             user_uuid=str(hamlet.uuid),
             server=self.server,
         )

zerver/lib/push_notifications.py

@@ -1,7 +1,6 @@
 # See https://zulip.readthedocs.io/en/latest/subsystems/notifications.html
 
 import asyncio
-import base64
 import copy
 import logging
 import re
@@ -83,23 +82,13 @@ PUSH_REGISTRATION_LIVENESS_TIMEOUT = 24 * 60 * 60
 DeviceToken: TypeAlias = Union[PushDeviceToken, "RemotePushDeviceToken"]
 
 
-# We store the token as b64, but apns-client wants hex strings
-def b64_to_hex(data: str) -> str:
-    return base64.b64decode(data).hex()
-
-
-def hex_to_b64(data: str) -> str:
-    return base64.b64encode(bytes.fromhex(data)).decode()
-
-
 def validate_token(token_str: str, kind: int) -> None:
     if token_str == "" or len(token_str) > 4096:
         raise JsonableError(_("Empty or invalid length token"))
     if kind == PushDeviceToken.APNS:
-        # Validate that we can actually decode the token.
         try:
-            b64_to_hex(token_str)
+            bytes.fromhex(token_str)
-        except Exception:
+        except ValueError:
             raise JsonableError(_("Invalid APNS token"))
 
 

zerver/lib/test_classes.py

@@ -57,7 +57,7 @@ from zerver.lib.mdiff import diff_strings
 from zerver.lib.message import access_message
 from zerver.lib.notification_data import UserMessageNotificationsData
 from zerver.lib.per_request_cache import flush_per_request_caches
-from zerver.lib.push_notifications import APNsContext, hex_to_b64
+from zerver.lib.push_notifications import APNsContext
 from zerver.lib.redis_utils import bounce_redis_key_prefix_for_testing
 from zerver.lib.response import MutableJsonResponse
 from zerver.lib.sessions import get_session_dict_user
@@ -2761,7 +2761,7 @@ class PushNotificationTestCase(BouncerTestCase):
         for token, appid in self.tokens:
             PushDeviceToken.objects.create(
                 kind=PushDeviceToken.APNS,
-                token=hex_to_b64(token),
+                token=token,
                 user=self.user_profile,
                 ios_app_id=appid,
             )
@@ -2777,14 +2777,14 @@ class PushNotificationTestCase(BouncerTestCase):
             # do their own setup.
             RemotePushDeviceToken.objects.create(
                 kind=RemotePushDeviceToken.APNS,
-                token=hex_to_b64(id_token),
+                token=id_token,
                 ios_app_id=appid,
                 user_id=self.user_profile.id,
                 server=self.server,
             )
             RemotePushDeviceToken.objects.create(
                 kind=RemotePushDeviceToken.APNS,
-                token=hex_to_b64(uuid_token),
+                token=uuid_token,
                 ios_app_id=appid,
                 user_uuid=self.user_profile.uuid,
                 server=self.server,
@@ -2803,7 +2803,7 @@ class PushNotificationTestCase(BouncerTestCase):
         for token in self.fcm_tokens:
             PushDeviceToken.objects.create(
                 kind=PushDeviceToken.FCM,
-                token=hex_to_b64(token),
+                token=token,
                 user=self.user_profile,
                 ios_app_id=None,
             )
@@ -2812,13 +2812,13 @@ class PushNotificationTestCase(BouncerTestCase):
         for id_token, uuid_token in self.remote_fcm_tokens:
             RemotePushDeviceToken.objects.create(
                 kind=RemotePushDeviceToken.FCM,
-                token=hex_to_b64(id_token),
+                token=id_token,
                 user_id=self.user_profile.id,
                 server=self.server,
             )
             RemotePushDeviceToken.objects.create(
                 kind=RemotePushDeviceToken.FCM,
-                token=hex_to_b64(uuid_token),
+                token=uuid_token,
                 user_uuid=self.user_profile.uuid,
                 server=self.server,
             )

zerver/openapi/python_examples.py

@@ -1632,7 +1632,7 @@ def get_stream_topics(client: Client, stream_id: int) -> None:
 @openapi_test_function("/users/me/apns_device_token:post")
 def add_apns_token(client: Client) -> None:
     # {code_example|start}
-    request = {"token": "apple-tokenbb", "appid": "org.zulip.Zulip"}
+    request = {"token": "c0ffee", "appid": "org.zulip.Zulip"}
     result = client.call_endpoint(url="/users/me/apns_device_token", method="POST", request=request)
     # {code_example|end}
     assert_success_response(result)
@@ -1643,7 +1643,7 @@ def add_apns_token(client: Client) -> None:
 def remove_apns_token(client: Client) -> None:
     # {code_example|start}
     request = {
-        "token": "apple-tokenbb",
+        "token": "c0ffee",
     }
     result = client.call_endpoint(
         url="/users/me/apns_device_token", method="DELETE", request=request

zerver/openapi/zulip.yaml

@@ -12670,7 +12670,7 @@ paths:
                   description: |
                     The token provided by the device.
                   type: string
-                  example: apple-tokenbb
+                  example: c0ffee
                 appid:
                   description: |
                     The ID of the Zulip app that is making the request.
@@ -12731,7 +12731,7 @@ paths:
                   description: |
                     The token provided by the device.
                   type: string
-                  example: apple-tokenbb
+                  example: c0ffee
               required:
                 - token
       responses:

zerver/tests/test_handle_push_notification.py

@@ -18,7 +18,6 @@ from zerver.actions.user_settings import do_change_user_setting
 from zerver.actions.user_topics import do_set_user_topic_visibility_policy
 from zerver.lib.push_notifications import (
     UserPushIdentityCompat,
-    b64_to_hex,
     handle_push_notification,
     handle_remove_push_notification,
 )
@@ -92,11 +91,11 @@ class HandlePushNotificationTest(PushNotificationTestCase):
             self.assertLogs("zilencer.views", level="INFO") as views_logger,
         ):
             apns_devices = [
-                (b64_to_hex(device.token), device.ios_app_id, device.token)
+                (device.token, device.ios_app_id, device.token)
                 for device in RemotePushDeviceToken.objects.filter(kind=PushDeviceToken.APNS)
             ]
             gcm_devices = [
-                (b64_to_hex(device.token), device.ios_app_id, device.token)
+                (device.token, device.ios_app_id, device.token)
                 for device in RemotePushDeviceToken.objects.filter(kind=PushDeviceToken.FCM)
             ]
             mock_fcm_messaging.send_each.return_value = self.make_fcm_success_response(
@@ -260,11 +259,11 @@ class HandlePushNotificationTest(PushNotificationTestCase):
             self.assertLogs("zilencer.views", level="INFO") as views_logger,
         ):
             apns_devices = [
-                (b64_to_hex(device.token), device.ios_app_id, device.token)
+                (device.token, device.ios_app_id, device.token)
                 for device in RemotePushDeviceToken.objects.filter(kind=PushDeviceToken.APNS)
             ]
             gcm_devices = [
-                (b64_to_hex(device.token), device.ios_app_id, device.token)
+                (device.token, device.ios_app_id, device.token)
                 for device in RemotePushDeviceToken.objects.filter(kind=PushDeviceToken.FCM)
             ]
 

zerver/tests/test_push_notifications.py

@@ -38,7 +38,6 @@ from zerver.lib.push_notifications import (
     get_message_payload_apns,
     get_message_payload_gcm,
     get_mobile_push_content,
-    hex_to_b64,
     modernize_apns_payload,
     parse_fcm_options,
     send_android_push_notification_to_user,
@@ -573,13 +572,13 @@ class PushBouncerNotificationTest(BouncerTestCase):
 
         android_token = RemotePushDeviceToken.objects.create(
             kind=RemotePushDeviceToken.FCM,
-            token=hex_to_b64("aaaa"),
+            token="aaaa",
             user_uuid=hamlet.uuid,
             server=server,
         )
         apple_token = RemotePushDeviceToken.objects.create(
             kind=RemotePushDeviceToken.APNS,
-            token=hex_to_b64("bbbb"),
+            token="bbbb",
             user_uuid=hamlet.uuid,
             server=server,
         )
@@ -626,7 +625,7 @@ class PushBouncerNotificationTest(BouncerTestCase):
             android_tokens.append(
                 RemotePushDeviceToken.objects.create(
                     kind=RemotePushDeviceToken.FCM,
-                    token=hex_to_b64(token + i),
+                    token=token + i,
                     user_id=hamlet.id,
                     server=server,
                 )
@@ -638,7 +637,7 @@ class PushBouncerNotificationTest(BouncerTestCase):
             uuid_android_tokens.append(
                 RemotePushDeviceToken.objects.create(
                     kind=RemotePushDeviceToken.FCM,
-                    token=hex_to_b64(token + i),
+                    token=token + i,
                     user_uuid=str(hamlet.uuid),
                     server=server,
                 )
@@ -646,7 +645,7 @@ class PushBouncerNotificationTest(BouncerTestCase):
 
         apple_token = RemotePushDeviceToken.objects.create(
             kind=RemotePushDeviceToken.APNS,
-            token=hex_to_b64(token),
+            token=token,
             user_id=hamlet.id,
             server=server,
         )
@@ -738,7 +737,7 @@ class PushBouncerNotificationTest(BouncerTestCase):
         remote_server = self.server
         RemotePushDeviceToken.objects.create(
             kind=RemotePushDeviceToken.FCM,
-            token=hex_to_b64("aaaaaa"),
+            token="aaaaaa",
             user_id=hamlet.id,
             server=remote_server,
         )
@@ -949,7 +948,7 @@ class PushBouncerNotificationTest(BouncerTestCase):
         hamlet = self.example_user("hamlet")
         RemotePushDeviceToken.objects.create(
             kind=RemotePushDeviceToken.FCM,
-            token=hex_to_b64("aaaaaa"),
+            token="aaaaaa",
             user_id=hamlet.id,
             server=self.server,
         )
@@ -1030,18 +1029,13 @@ class PushBouncerNotificationTest(BouncerTestCase):
         self.assert_length(remote_tokens, 0)
 
     def test_invalid_apns_token(self) -> None:
-        endpoints = [
+        payload = {
-            ("/api/v1/remotes/push/register", "apple-token"),
+            "user_id": 10,
-        ]
+            "token": "xyz uses non-hex characters",
-
+            "token_kind": PushDeviceToken.APNS,
-        for endpoint, method in endpoints:
+        }
-            payload = {
+        result = self.uuid_post(self.server_uuid, "/api/v1/remotes/push/register", payload)
-                "user_id": 10,
+        self.assert_json_error(result, "Invalid APNS token")
-                "token": "xyz uses non-hex characters",
-                "token_kind": PushDeviceToken.APNS,
-            }
-            result = self.uuid_post(self.server_uuid, endpoint, payload)
-            self.assert_json_error(result, "Invalid APNS token")
 
     def test_initialize_push_notifications(self) -> None:
         realm = get_realm("zulip")
@@ -1130,7 +1124,7 @@ class PushBouncerNotificationTest(BouncerTestCase):
         remote_realm.save()
 
         endpoint = "/json/users/me/apns_device_token"
-        token = "apple-tokenaz"
+        token = "c0ffee"
         with self.assertLogs("zilencer.views", level="WARN") as warn_log:
             result = self.client_post(
                 endpoint, {"token": token, "appid": "org.zulip.Zulip"}, subdomain="zulip"
@@ -1163,7 +1157,7 @@ class PushBouncerNotificationTest(BouncerTestCase):
         endpoints: list[tuple[str, str, int, Mapping[str, str]]] = [
             (
                 "/json/users/me/apns_device_token",
-                "apple-tokenaz",
+                "c0ffee",
                 RemotePushDeviceToken.APNS,
                 {"appid": "org.zulip.Zulip"},
             ),
@@ -2306,7 +2300,7 @@ class TestAddRemoveDeviceTokenAPI(BouncerTestCase):
         self.login_user(user)
 
         endpoints: list[tuple[str, str, Mapping[str, str]]] = [
-            ("/json/users/me/apns_device_token", "apple-tokenaz", {"appid": "org.zulip.Zulip"}),
+            ("/json/users/me/apns_device_token", "c0ffee", {"appid": "org.zulip.Zulip"}),
             ("/json/users/me/android_gcm_reg_id", "android-token", {}),
         ]
 
@@ -2317,7 +2311,7 @@ class TestAddRemoveDeviceTokenAPI(BouncerTestCase):
             result = self.client_post(endpoint, {"token": broken_token, **appid})
             self.assert_json_error(result, "Empty or invalid length token")
 
-            if label == "apple-tokenaz":
+            if "apns" in endpoint:
                 result = self.client_post(
                     endpoint, {"token": "xyz has non-hex characters", **appid}
                 )
@@ -2356,12 +2350,12 @@ class TestAddRemoveDeviceTokenAPI(BouncerTestCase):
         self.login_user(user)
 
         no_bouncer_requests: list[tuple[str, str, Mapping[str, str]]] = [
-            ("/json/users/me/apns_device_token", "apple-tokenaa", {"appid": "org.zulip.Zulip"}),
+            ("/json/users/me/apns_device_token", "c0ffee01", {"appid": "org.zulip.Zulip"}),
             ("/json/users/me/android_gcm_reg_id", "android-token-1", {}),
         ]
 
         bouncer_requests: list[tuple[str, str, Mapping[str, str]]] = [
-            ("/json/users/me/apns_device_token", "apple-tokenbb", {"appid": "org.zulip.Zulip"}),
+            ("/json/users/me/apns_device_token", "c0ffee02", {"appid": "org.zulip.Zulip"}),
             ("/json/users/me/android_gcm_reg_id", "android-token-2", {}),
         ]
 
@@ -2402,13 +2396,13 @@ class TestAddRemoveDeviceTokenAPI(BouncerTestCase):
         # PushDeviceToken will include all the device tokens.
         token_values = list(PushDeviceToken.objects.values_list("token", flat=True))
         self.assertEqual(
-            token_values, ["apple-tokenaa", "android-token-1", "apple-tokenbb", "android-token-2"]
+            token_values, ["c0ffee01", "android-token-1", "c0ffee02", "android-token-2"]
         )
 
         # RemotePushDeviceToken will only include tokens of
         # the devices using push notification bouncer.
         remote_token_values = list(RemotePushDeviceToken.objects.values_list("token", flat=True))
-        self.assertEqual(sorted(remote_token_values), ["android-token-2", "apple-tokenbb"])
+        self.assertEqual(sorted(remote_token_values), ["android-token-2", "c0ffee02"])
 
         # Test removing tokens without using push notification bouncer.
         for endpoint, token, appid in no_bouncer_requests:
@@ -2511,8 +2505,8 @@ class FCMSendTest(PushNotificationTestCase):
             send_android_push_notification_to_user(self.user_profile, data, {})
         self.assert_length(logger.output, 3)
         log_msg1 = f"INFO:zerver.lib.push_notifications:FCM: Sending notification for local user <id:{self.user_profile.id}> to 2 devices"
-        log_msg2 = f"INFO:zerver.lib.push_notifications:FCM: Sent message with ID: {response.responses[0].message_id} to {hex_to_b64(self.fcm_tokens[0])}"
+        log_msg2 = f"INFO:zerver.lib.push_notifications:FCM: Sent message with ID: {response.responses[0].message_id} to {self.fcm_tokens[0]}"
-        log_msg3 = f"INFO:zerver.lib.push_notifications:FCM: Sent message with ID: {response.responses[1].message_id} to {hex_to_b64(self.fcm_tokens[1])}"
+        log_msg3 = f"INFO:zerver.lib.push_notifications:FCM: Sent message with ID: {response.responses[1].message_id} to {self.fcm_tokens[1]}"
 
         self.assertEqual([log_msg1, log_msg2, log_msg3], logger.output)
         mock_warning.assert_not_called()
@@ -2520,14 +2514,13 @@ class FCMSendTest(PushNotificationTestCase):
     def test_not_registered(
         self, mock_fcm_messaging: mock.MagicMock, fcm_app: mock.MagicMock
     ) -> None:
-        token = hex_to_b64("1111")
+        token = "1111"
         response = self.make_fcm_error_response(
             token, firebase_messaging.UnregisteredError("Requested entity was not found.")
         )
         mock_fcm_messaging.send_each.return_value = response
 
-        def get_count(hex_token: str) -> int:
+        def get_count(token: str) -> int:
-            token = hex_to_b64(hex_token)
             return PushDeviceToken.objects.filter(token=token, kind=PushDeviceToken.FCM).count()
 
         self.assertEqual(get_count("1111"), 1)
@@ -2546,7 +2539,7 @@ class FCMSendTest(PushNotificationTestCase):
         self.assertEqual(get_count("1111"), 0)
 
     def test_failure(self, mock_fcm_messaging: mock.MagicMock, fcm_app: mock.MagicMock) -> None:
-        token = hex_to_b64("1111")
+        token = "1111"
         response = self.make_fcm_error_response(token, firebase_exceptions.UnknownError("Failed"))
         mock_fcm_messaging.send_each.return_value = response
 

zerver/tests/test_push_registration.py

@@ -27,7 +27,7 @@ class RegisterPushDeviceToBouncer(BouncerTestCase):
 
     def get_register_push_device_payload(
         self,
-        token: str = "apple-tokenaz",
+        token: str = "c0ffee",
         token_kind: str = RemotePushDevice.TokenKind.APNS,
         ios_app_id: str | None = "example.app",
         timestamp: int | None = None,
@@ -205,7 +205,7 @@ class RegisterPushDeviceToBouncer(BouncerTestCase):
 class RegisterPushDeviceToServer(BouncerTestCase):
     def get_register_push_device_payload(
         self,
-        token: str = "apple-tokenaz",
+        token: str = "c0ffee",
         token_kind: str = RemotePushDevice.TokenKind.APNS,
         ios_app_id: str | None = "example.app",
         timestamp: int | None = None,

zilencer/views.py

@@ -57,7 +57,6 @@ from zerver.lib.push_notifications import (
     HostnameAlreadyInUseBouncerError,
     InvalidRemotePushDeviceTokenError,
     UserPushIdentityCompat,
-    b64_to_hex,
     send_android_push_notification,
     send_apple_push_notification,
     send_test_push_notification_directly_to_devices,
@@ -498,10 +497,9 @@ class PushRegistration(BaseModel):
             return False
 
         if self.token_kind == RemotePushDevice.TokenKind.APNS:
-            # Validate that we can actually decode the token.
             try:
-                b64_to_hex(self.token)
+                bytes.fromhex(self.token)
-            except Exception:
+            except ValueError:
                 return False
         return True