decorator: Explicitly require req_secret in internal_notify_view.

It's hard to come up with a realistic story where this would matter:
SHARED_SECRET is generated automatically during server setup at the
same time as SECRET_KEY, which is a required setting, but it seems
preferable to be explicit that this is a required parameter for the
internal_notify authentication model.

zerver/decorator.py

@@ -828,9 +828,7 @@ def is_local_addr(addr: str) -> bool:
 # of events.  We protect them from the outside world by checking a shared
 # secret, and also the originating IP (for now).
 @has_request_variables
-def authenticate_notify(
-    request: HttpRequest, secret: Optional[str] = REQ("secret", default=None)
-) -> bool:
+def authenticate_notify(request: HttpRequest, secret: str = REQ("secret")) -> bool:
     return is_local_addr(request.META["REMOTE_ADDR"]) and secret == settings.SHARED_SECRET
 
 

zerver/tests/test_decorators.py

@@ -1584,10 +1584,14 @@ class TestInternalNotifyView(ZulipTestCase):
         )
 
         with self.settings(SHARED_SECRET="random"):
-            self.assertFalse(authenticate_notify(request))
-            with self.assertRaises(AccessDeniedError) as context:
+            with self.assertRaises(RequestVariableMissingError) as context:
                 self.internal_notify(True, request)
-            self.assertEqual(context.exception.http_status_code, 403)
+            self.assertEqual(context.exception.http_status_code, 400)
+
+        with self.settings(SHARED_SECRET=None):
+            with self.assertRaises(RequestVariableMissingError) as context:
+                self.internal_notify(True, request)
+            self.assertEqual(context.exception.http_status_code, 400)
 
         secret = "random"
         request = HostRequestMock(

zerver/tests/test_event_system.py

@@ -15,6 +15,7 @@ from zerver.actions.users import do_change_user_role
 from zerver.lib.event_schema import check_restart_event
 from zerver.lib.events import fetch_initial_state_data
 from zerver.lib.exceptions import AccessDeniedError
+from zerver.lib.request import RequestVariableMissingError
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.test_helpers import (
     HostRequestMock,
@@ -206,6 +207,14 @@ class EventsEndpointTest(ZulipTestCase):
         )
         req = HostRequestMock(post_data)
         req.META["REMOTE_ADDR"] = "127.0.0.1"
+        with self.assertRaises(RequestVariableMissingError) as context:
+            result = self.client_post_request("/notify_tornado", req)
+        self.assertEqual(str(context.exception), "Missing 'secret' argument")
+        self.assertEqual(context.exception.http_status_code, 400)
+
+        post_data["secret"] = "random"
+        req = HostRequestMock(post_data, user_profile=None)
+        req.META["REMOTE_ADDR"] = "127.0.0.1"
         with self.assertRaises(AccessDeniedError) as context:
             result = self.client_post_request("/notify_tornado", req)
         self.assertEqual(str(context.exception), "Access denied")