remote_server: Migrate RemoteZulipServer.uuid to be UUIDField.

Given that these values are uuids, it's better to use UUIDField which is
meant for exactly that, rather than an arbitrary CharField.

This requires modifying some tests to use valid uuids.

Author: mateuszmandera

corporate/tests/test_stripe.py

@@ -4,6 +4,7 @@
 import random
 import re
 import sys
+import uuid
 from dataclasses import dataclass
 from datetime import datetime, timedelta, timezone
 from decimal import Decimal
@@ -4424,7 +4425,7 @@ def test_is_sponsored_realm(self) -> None:
         self.assertTrue(is_sponsored_realm(realm))
 
     def test_change_remote_server_plan_type(self) -> None:
-        server_uuid = "demo-1234"
+        server_uuid = str(uuid.uuid4())
         remote_server = RemoteZulipServer.objects.create(
             uuid=server_uuid,
             api_key="magic_secret_api_key",

tools/lib/capitalization.py

@@ -50,6 +50,7 @@
     r"URL",
     r"Ubuntu",
     r"Updown",
+    r"UUID",
     r"V5",
     r"Webathena",
     r"Windows",

zerver/tests/test_decorators.py

@@ -1,6 +1,7 @@
 import base64
 import os
 import re
+import uuid
 from collections import defaultdict
 from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple
 from unittest import mock, skipUnless
@@ -644,7 +645,7 @@ def test_rate_limiting_happens_in_normal_case(self) -> None:
 
     @skipUnless(settings.ZILENCER_ENABLED, "requires zilencer")
     def test_rate_limiting_happens_if_remote_server(self) -> None:
-        server_uuid = "1234-abcd"
+        server_uuid = str(uuid.uuid4())
         server = RemoteZulipServer(
             uuid=server_uuid,
             api_key="magic_secret_api_key",

zerver/tests/test_external.py

@@ -1,4 +1,5 @@
 import time
+import uuid
 from contextlib import contextmanager
 from typing import IO, Any, Callable, Iterator, Optional, Sequence
 from unittest import mock, skipUnless
@@ -411,7 +412,7 @@ def test_tor_file_not_found(self) -> None:
     @skipUnless(settings.ZILENCER_ENABLED, "requires zilencer")
     @rate_limit_rule(1, 5, domain="api_by_remote_server")
     def test_hit_ratelimits_as_remote_server(self) -> None:
-        server_uuid = "1234-abcd"
+        server_uuid = str(uuid.uuid4())
         server = RemoteZulipServer(
             uuid=server_uuid,
             api_key="magic_secret_api_key",
@@ -433,7 +434,7 @@ def test_hit_ratelimits_as_remote_server(self) -> None:
             self.assertEqual(
                 m.output,
                 [
-                    "WARNING:zerver.lib.rate_limiter:Remote server <RemoteZulipServer demo.example.com 1234-abcd> exceeded rate limits on domain api_by_remote_server"
+                    f"WARNING:zerver.lib.rate_limiter:Remote server <RemoteZulipServer demo.example.com {server_uuid[:12]}> exceeded rate limits on domain api_by_remote_server"
                 ],
             )
         finally:

zerver/tests/test_push_notifications.py

@@ -90,7 +90,7 @@
 @skipUnless(settings.ZILENCER_ENABLED, "requires zilencer")
 class BouncerTestCase(ZulipTestCase):
     def setUp(self) -> None:
-        self.server_uuid = "1234-abcd"
+        self.server_uuid = "6cde5f7a-1f7e-4978-9716-49f69ebfc9fe"
         server = RemoteZulipServer(
             uuid=self.server_uuid,
             api_key="magic_secret_api_key",
@@ -234,12 +234,15 @@ def test_register_remote_push_user_paramas(self) -> None:
             self.server_uuid, endpoint, dict(user_id=user_id, token_kind=token_kind, token=token)
         )
         self.assert_json_error(
-            result, "Zulip server auth failure: key does not match role 1234-abcd", status_code=401
+            result,
+            "Zulip server auth failure: key does not match role 6cde5f7a-1f7e-4978-9716-49f69ebfc9fe",
+            status_code=401,
         )
 
         del self.API_KEYS[self.server_uuid]
 
-        credentials = "{}:{}".format("5678-efgh", "invalid")
+        credentials_uuid = str(uuid.uuid4())
+        credentials = "{}:{}".format(credentials_uuid, "invalid")
         api_auth = "Basic " + base64.b64encode(credentials.encode()).decode()
         result = self.client_post(
             endpoint,
@@ -248,7 +251,7 @@ def test_register_remote_push_user_paramas(self) -> None:
         )
         self.assert_json_error(
             result,
-            "Zulip server auth failure: 5678-efgh is not registered -- did you run `manage.py register_server`?",
+            f"Zulip server auth failure: {credentials_uuid} is not registered -- did you run `manage.py register_server`?",
             status_code=401,
         )
 
@@ -325,7 +328,7 @@ def test_send_notification_endpoint(self) -> None:
             logger.output,
             [
                 "INFO:zilencer.views:"
-                f"Sending mobile push notifications for remote user 1234-abcd:{hamlet.id}: "
+                f"Sending mobile push notifications for remote user 6cde5f7a-1f7e-4978-9716-49f69ebfc9fe:{hamlet.id}: "
                 "2 via FCM devices, 1 via APNs devices"
             ],
         )
@@ -673,7 +676,7 @@ def check_counts(
             self.assertEqual(
                 warn_log.output,
                 [
-                    "WARNING:root:Invalid data saving zilencer_remoteinstallationcount for server demo.example.com/1234-abcd"
+                    "WARNING:root:Invalid data saving zilencer_remoteinstallationcount for server demo.example.com/6cde5f7a-1f7e-4978-9716-49f69ebfc9fe"
                 ],
             )
 
@@ -784,7 +787,7 @@ def test_realmauditlog_data_mapping(self) -> None:
         send_analytics_to_remote_server()
         remote_log_entry = RemoteRealmAuditLog.objects.order_by("id").last()
         assert remote_log_entry is not None
-        self.assertEqual(remote_log_entry.server.uuid, self.server_uuid)
+        self.assertEqual(str(remote_log_entry.server.uuid), self.server_uuid)
         self.assertEqual(remote_log_entry.remote_id, log_entry.id)
         self.assertEqual(remote_log_entry.event_time, self.TIME_ZERO)
         self.assertEqual(remote_log_entry.backfilled, True)
@@ -921,7 +924,7 @@ def test_end_to_end(self) -> None:
                 views_logger.output,
                 [
                     "INFO:zilencer.views:"
-                    f"Sending mobile push notifications for remote user 1234-abcd:{self.user_profile.id}: "
+                    f"Sending mobile push notifications for remote user 6cde5f7a-1f7e-4978-9716-49f69ebfc9fe:{self.user_profile.id}: "
                     f"{len(gcm_devices)} via FCM devices, {len(apns_devices)} via APNs devices"
                 ],
             )
@@ -982,7 +985,7 @@ def test_unregistered_client(self) -> None:
                 views_logger.output,
                 [
                     "INFO:zilencer.views:"
-                    f"Sending mobile push notifications for remote user 1234-abcd:{self.user_profile.id}: "
+                    f"Sending mobile push notifications for remote user 6cde5f7a-1f7e-4978-9716-49f69ebfc9fe:{self.user_profile.id}: "
                     f"{len(gcm_devices)} via FCM devices, {len(apns_devices)} via APNs devices"
                 ],
             )

zilencer/migrations/0021_alter_remotezulipserver_uuid.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.2.9 on 2021-12-22 10:06
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("zilencer", "0020_remotezulipserverauditlog"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="remotezulipserver",
+            name="uuid",
+            field=models.UUIDField(unique=True),
+        ),
+    ]

zilencer/models.py

@@ -1,5 +1,6 @@
 import datetime
 from typing import List, Tuple
+from uuid import UUID
 
 from django.conf import settings
 from django.db import models
@@ -26,7 +27,7 @@ class RemoteZulipServer(models.Model):
 
     # The unique UUID (`zulip_org_id`) and API key (`zulip_org_key`)
     # for this remote server registration.
-    uuid: str = models.CharField(max_length=UUID_LENGTH, unique=True)
+    uuid: UUID = models.UUIDField(unique=True)
     api_key: str = models.CharField(max_length=API_KEY_LENGTH)
 
     # The hostname and contact details are not verified/trusted. Thus,
@@ -44,10 +45,10 @@ class RemoteZulipServer(models.Model):
     plan_type: int = models.PositiveSmallIntegerField(default=PLAN_TYPE_SELF_HOSTED)
 
     def __str__(self) -> str:
-        return f"<RemoteZulipServer {self.hostname} {self.uuid[0:12]}>"
+        return f"<RemoteZulipServer {self.hostname} {str(self.uuid)[0:12]}>"
 
     def format_requestor_for_logs(self) -> str:
-        return "zulip-server:" + self.uuid
+        return "zulip-server:" + str(self.uuid)
 
 
 class RemotePushDeviceToken(AbstractPushDeviceToken):
@@ -137,7 +138,7 @@ def __init__(
         assert not settings.RUNNING_INSIDE_TORNADO
         assert settings.ZILENCER_ENABLED
 
-        self.uuid = remote_server.uuid
+        self.uuid = str(remote_server.uuid)
         self.domain = domain
         super().__init__()