From fb3af7fbcb000876d94fef8f1b4c83973761543c Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 6 Nov 2023 11:05:30 -0800 Subject: [PATCH] push_notifs: Make appid required on add_apns_device_token. We're going to need to use this information, so we shouldn't just assume a value; the client should tell us the actual value. Conveniently, the Zulip mobile app does already pass this parameter and has since forever. So we can just start requiring it, with no compatibility constraint. --- api_docs/changelog.md | 7 +++++++ version.py | 2 +- zerver/tests/test_push_notifications.py | 10 ++++++++++ zerver/views/push_notifications.py | 3 +-- 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 0103e5c1c4..046aa276a2 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 8.0 +**Feature level 223** + +* `POST /users/me/apns_device_token`: + The `appid` parameter is now required. + Previously it defaulted to the server setting `ZULIP_IOS_APP_ID`, + defaulting to "org.zulip.Zulip". + **Feature level 222** * [`GET /events`](/api/get-events): When a user is deactivated or diff --git a/version.py b/version.py index 32da063ef6..676f187773 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # Changes should be accompanied by documentation explaining what the # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 222 +API_FEATURE_LEVEL = 223 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index c5dda156a8..f8ffc61206 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -780,6 +780,11 @@ class PushBouncerNotificationTest(BouncerTestCase): result = self.client_delete(endpoint, {"token": broken_token}, subdomain="zulip") self.assert_json_error(result, "Empty or invalid length token") + # Try adding without appid... + if appid: + result = self.client_post(endpoint, {"token": token}, subdomain="zulip") + self.assert_json_error(result, "Missing 'appid' argument") + # Try to remove a non-existent token... result = self.client_delete(endpoint, {"token": "abcd1234"}, subdomain="zulip") self.assert_json_error(result, "Token does not exist") @@ -2806,6 +2811,11 @@ class TestPushApi(BouncerTestCase): result = self.client_delete(endpoint, {"token": broken_token}) self.assert_json_error(result, "Empty or invalid length token") + # Try adding without appid... + if appid: + result = self.client_post(endpoint, {"token": label}) + self.assert_json_error(result, "Missing 'appid' argument") + # Try to remove a non-existent token... result = self.client_delete(endpoint, {"token": "abcd1234"}) self.assert_json_error(result, "Token does not exist") diff --git a/zerver/views/push_notifications.py b/zerver/views/push_notifications.py index 53d84e4135..2abb821184 100644 --- a/zerver/views/push_notifications.py +++ b/zerver/views/push_notifications.py @@ -1,6 +1,5 @@ from typing import Optional -from django.conf import settings from django.http import HttpRequest, HttpResponse from django.utils.translation import gettext as _ @@ -35,7 +34,7 @@ def add_apns_device_token( request: HttpRequest, user_profile: UserProfile, token: str = REQ(), - appid: str = REQ(default=settings.ZULIP_IOS_APP_ID), + appid: str = REQ(), ) -> HttpResponse: validate_token(token, PushDeviceToken.APNS) add_push_device_token(user_profile, token, PushDeviceToken.APNS, ios_app_id=appid)