paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-10-23 04:52:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

push_notifications: Adjust APNs tokens to be case-insensitive in the database.

Browse Source 
APNs apparently treats its tokens case-insensitively; FCM does not.
Adjust the `unique_together` to instead be separate partial
constraints, keyed on the `kind` of the PushDeviceToken.
This commit is contained in:
Alex Vandiver
2025-07-02 04:47:37 +00:00
committed by Tim Abbott
parent 63f6a97f0c
commit 2f4dd72076
8 changed files with 511 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

141
zilencer/migrations/0067_remotepushdevicetoken_apns_case_insensitive.py Normal file
View File

@@ -0,0 +1,141 @@
import django.db.models.functions.text
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
("zilencer", "0066_alter_remotepushdevice_token_kind"),
]
operations = [
# This parallels zerver/migrations/0740 but must account for
# the user_id / user_uuid split.
migrations.RunSQL(
"""
WITH dups AS (
SELECT server_id, user_id, kind, LOWER(token) AS token, MAX(last_updated) AS max_last_updated
FROM zilencer_remotepushdevicetoken
WHERE kind = 1
AND user_uuid IS NULL
GROUP BY server_id, user_id, kind, LOWER(token)
HAVING COUNT(*) > 1
)
UPDATE zilencer_remotepushdevicetoken
SET last_updated = dups.max_last_updated
FROM dups
WHERE zilencer_remotepushdevicetoken.server_id = dups.server_id
AND zilencer_remotepushdevicetoken.user_id = dups.user_id
AND zilencer_remotepushdevicetoken.user_uuid IS NULL
AND zilencer_remotepushdevicetoken.kind = dups.kind
AND LOWER(zilencer_remotepushdevicetoken.token) = dups.token
"""
),
migrations.RunSQL(
"""
WITH dups AS (
SELECT server_id, user_id, kind, LOWER(token) AS token, MIN(id) AS min_id
FROM zilencer_remotepushdevicetoken
WHERE kind = 1
AND user_uuid IS NULL
GROUP BY server_id, user_id, kind, LOWER(token)
HAVING COUNT(*) > 1
)
DELETE FROM zilencer_remotepushdevicetoken
USING dups
WHERE zilencer_remotepushdevicetoken.server_id = dups.server_id
AND zilencer_remotepushdevicetoken.user_id = dups.user_id
AND zilencer_remotepushdevicetoken.user_uuid IS NULL
AND zilencer_remotepushdevicetoken.kind = dups.kind
AND LOWER(zilencer_remotepushdevicetoken.token) = dups.token
AND zilencer_remotepushdevicetoken.id != dups.min_id
"""
),
migrations.AddConstraint(
model_name="remotepushdevicetoken",
constraint=models.UniqueConstraint(
models.F("server_id"),
models.F("user_id"),
models.F("kind"),
django.db.models.functions.text.Lower(models.F("token")),
condition=models.Q(("kind", 1)),
name="zilencer_remotepushdevicetoken_apns_server_user_id_kind_token",
),
),
migrations.AddConstraint(
model_name="remotepushdevicetoken",
constraint=models.UniqueConstraint(
models.F("server_id"),
models.F("user_id"),
models.F("kind"),
models.F("token"),
condition=models.Q(("kind", 2)),
name="zilencer_remotepushdevicetoken_fcm_server_user_id_kind_token",
),
),
migrations.RunSQL(
"""
WITH dups AS (
SELECT server_id, user_uuid, kind, LOWER(token) AS token, MAX(last_updated) AS max_last_updated
FROM zilencer_remotepushdevicetoken
WHERE kind = 1
AND user_id IS NULL
GROUP BY server_id, user_uuid, kind, LOWER(token)
HAVING COUNT(*) > 1
)
UPDATE zilencer_remotepushdevicetoken
SET last_updated = dups.max_last_updated
FROM dups
WHERE zilencer_remotepushdevicetoken.server_id = dups.server_id
AND zilencer_remotepushdevicetoken.user_uuid = dups.user_uuid
AND zilencer_remotepushdevicetoken.user_id IS NULL
AND zilencer_remotepushdevicetoken.kind = dups.kind
AND LOWER(zilencer_remotepushdevicetoken.token) = dups.token
"""
),
migrations.RunSQL(
"""
WITH dups AS (
SELECT server_id, user_uuid, kind, LOWER(token) AS token, MIN(id) AS min_id
FROM zilencer_remotepushdevicetoken
WHERE kind = 1
AND user_id IS NULL
GROUP BY server_id, user_uuid, kind, LOWER(token)
HAVING COUNT(*) > 1
)
DELETE FROM zilencer_remotepushdevicetoken
USING dups
WHERE zilencer_remotepushdevicetoken.server_id = dups.server_id
AND zilencer_remotepushdevicetoken.user_uuid = dups.user_uuid
AND zilencer_remotepushdevicetoken.user_id IS NULL
AND zilencer_remotepushdevicetoken.kind = dups.kind
AND LOWER(zilencer_remotepushdevicetoken.token) = dups.token
AND zilencer_remotepushdevicetoken.id != dups.min_id
"""
),
migrations.AddConstraint(
model_name="remotepushdevicetoken",
constraint=models.UniqueConstraint(
models.F("server_id"),
models.F("user_uuid"),
models.F("kind"),
django.db.models.functions.text.Lower(models.F("token")),
condition=models.Q(("kind", 1)),
name="zilencer_remotepushdevicetoken_apns_server_uuid_kind_token",
),
),
migrations.AddConstraint(
model_name="remotepushdevicetoken",
constraint=models.UniqueConstraint(
models.F("server_id"),
models.F("user_uuid"),
models.F("kind"),
models.F("token"),
condition=models.Q(("kind", 2)),
name="zilencer_remotepushdevicetoken_fcm_server_uuid_kind_token",
),
),
migrations.AlterUniqueTogether(
name="remotepushdevicetoken",
unique_together=set(),
),
]

39
zilencer/models.py
View File

@@ -4,7 +4,8 @@ from datetime import datetime, timedelta
from django.conf import settings
from django.core.exceptions import ValidationError
from django.db import models
from django.db.models import Max, Q, QuerySet, UniqueConstraint
from django.db.models import F, Max, Q, QuerySet, UniqueConstraint
from django.db.models.functions import Lower
from django.utils.timezone import now as timezone_now
from typing_extensions import override
@@ -116,13 +117,43 @@ class RemotePushDeviceToken(AbstractPushDeviceToken):
remote_realm = models.ForeignKey("RemoteRealm", on_delete=models.SET_NULL, null=True)
class Meta:
unique_together = [
constraints = [
# These indexes rely on the property that in Postgres,
# NULL != NULL in the context of unique indexes, so multiple
# rows with the same values in these columns can exist
# if one of them is NULL.
("server", "user_id", "kind", "token"),
("server", "user_uuid", "kind", "token"),
models.UniqueConstraint(
"server_id",
"user_id",
"kind",
Lower(F("token")),
name="zilencer_remotepushdevicetoken_apns_server_user_id_kind_token",
condition=Q(kind=1),
),
models.UniqueConstraint(
"server_id",
"user_id",
"kind",
"token",
name="zilencer_remotepushdevicetoken_fcm_server_user_id_kind_token",
condition=Q(kind=2),
),
models.UniqueConstraint(
"server_id",
"user_uuid",
"kind",
Lower(F("token")),
name="zilencer_remotepushdevicetoken_apns_server_uuid_kind_token",
condition=Q(kind=1),
),
models.UniqueConstraint(
"server_id",
"user_uuid",
"kind",
"token",
name="zilencer_remotepushdevicetoken_fcm_server_uuid_kind_token",
condition=Q(kind=2),
),
]
@override

93
zilencer/views.py
View File

@@ -12,8 +12,9 @@ from django.conf import settings
from django.core.exceptions import ValidationError
from django.core.validators import URLValidator, validate_email
from django.db import IntegrityError, transaction
from django.db.models import Model
from django.db.models import Model, QuerySet
from django.db.models.constants import OnConflict
from django.db.models.functions import Lower
from django.http import HttpRequest, HttpResponse
from django.utils.crypto import constant_time_compare, get_random_string
from django.utils.timezone import now as timezone_now
@@ -424,6 +425,26 @@ def check_transfer_challenge_response_secret_not_prepared(response: requests.Res
return secret_not_prepared
def get_remote_push_device_token(
*,
server: RemoteZulipServer,
token: str,
kind: int,
) -> QuerySet[RemotePushDeviceToken]:
if kind == RemotePushDeviceToken.APNS:
return RemotePushDeviceToken.objects.alias(lower_token=Lower("token")).filter(
server=server,
lower_token=token.lower(),
kind=kind,
)
else:
return RemotePushDeviceToken.objects.filter(
server=server,
token=token,
kind=kind,
)
@typed_endpoint
def register_remote_push_device(
request: HttpRequest,
@@ -446,9 +467,11 @@ def register_remote_push_device(
kwargs: dict[str, object] = {"user_uuid": user_uuid, "user_id": None}
# Delete pre-existing user_id registration for this user+device to avoid
# duplication. Further down, uuid registration will be created.
RemotePushDeviceToken.objects.filter(
server=server, token=token, kind=token_kind, user_id=user_id
).delete()
get_remote_push_device_token(
server=server,
token=token,
kind=token_kind,
).filter(user_id=user_id).delete()
else:
# One of these is None, so these kwargs will lead to a proper registration
# of either user_id or user_uuid type
@@ -622,9 +645,11 @@ def unregister_remote_push_device(
update_remote_realm_last_request_datetime_helper(request, server, realm_uuid, user_uuid)
(num_deleted, ignored) = RemotePushDeviceToken.objects.filter(
user_identity.filter_q(), token=token, kind=token_kind, server=server
).delete()
(num_deleted, ignored) = (
get_remote_push_device_token(token=token, kind=token_kind, server=server)
.filter(user_identity.filter_q())
.delete()
)
if num_deleted == 0:
raise JsonableError(err_("Token does not exist"))
@@ -683,7 +708,10 @@ def delete_duplicate_registrations(
assert len({registration.kind for registration in registrations}) == 1
kind = registrations[0].kind
tokens_counter = Counter(device.token for device in registrations)
if kind == RemotePushDeviceToken.APNS:
tokens_counter = Counter(device.token.lower() for device in registrations)
else:
tokens_counter = Counter(device.token for device in registrations)
tokens_to_deduplicate = []
for key in tokens_counter:
@@ -757,11 +785,12 @@ def remote_server_send_test_notification(
update_remote_realm_last_request_datetime_helper(request, server, realm_uuid, user_uuid)
try:
device = RemotePushDeviceToken.objects.get(
user_identity.filter_q(), token=token, kind=token_kind, server=server
)
except RemotePushDeviceToken.DoesNotExist:
device = (
get_remote_push_device_token(token=token, kind=token_kind, server=server)
.filter(user_identity.filter_q())
.first()
)
if device is None:
raise InvalidRemotePushDeviceTokenError
send_test_push_notification_directly_to_devices(
@@ -1046,16 +1075,38 @@ def get_deleted_devices(
kind=RemotePushDeviceToken.FCM,
server=server,
).values_list("token", flat=True)
apple_devices_we_have = RemotePushDeviceToken.objects.filter(
user_identity.filter_q(),
token__in=apple_devices,
kind=RemotePushDeviceToken.APNS,
server=server,
).values_list("token", flat=True)
# APNS tokens are case-insensitive -- but the remote server may
# not know that yet. As such, we perform our local lookups
# case-insensitively, returning the exact case the remote server
# used, and also return all-but-one of any case duplicates that
# the remote server passed us.
canonical_case = {}
apns_token_to_remove = set()
for token in apple_devices:
if token.lower() not in canonical_case:
canonical_case[token.lower()] = token
elif canonical_case[token.lower()] == token:
# Be careful to skip if identical-case tokens somehow show up more than once
pass
else:
apns_token_to_remove.add(token)
apple_devices_we_have = (
RemotePushDeviceToken.objects.annotate(lower_token=Lower("token"))
.filter(
user_identity.filter_q(),
lower_token__in=canonical_case.keys(),
kind=RemotePushDeviceToken.APNS,
server=server,
)
.values_list("lower_token", flat=True)
)
for token_to_remove in set(canonical_case.keys()) - set(apple_devices_we_have):
apns_token_to_remove.add(canonical_case[token_to_remove])
return DevicesToCleanUpDict(
android_devices=list(set(android_devices) - set(android_devices_we_have)),
apple_devices=list(set(apple_devices) - set(apple_devices_we_have)),
android_devices=sorted(set(android_devices) - set(android_devices_we_have)),
apple_devices=sorted(apns_token_to_remove),
)