From 855e14272a100a1fc12ddadd58d7637fcac7fcda Mon Sep 17 00:00:00 2001 From: Lauryn Menard Date: Wed, 13 Jul 2022 19:11:20 +0200 Subject: [PATCH] backend: Migrate `secret` parameter to REQ framework. Instead of using request.POST to get any potential `secret` parameter used in `authenticate_notify` for `internal_notify_view` decorator, moves it to the REQ framework parameters as `req_secret`. Updates existing tests to explicitly test for a request without `secret` parameter, which defaults to `None`; this is also tested in `test_event_system.py`. --- zerver/decorator.py | 10 +++++----- zerver/tests/test_decorators.py | 11 +++++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/zerver/decorator.py b/zerver/decorator.py index a7c4bc0243..46b395d0f1 100644 --- a/zerver/decorator.py +++ b/zerver/decorator.py @@ -827,11 +827,11 @@ def is_local_addr(addr: str) -> bool: # These views are used by the main Django server to notify the Tornado server # of events. We protect them from the outside world by checking a shared # secret, and also the originating IP (for now). -def authenticate_notify(request: HttpRequest) -> bool: - return ( - is_local_addr(request.META["REMOTE_ADDR"]) - and request.POST.get("secret") == settings.SHARED_SECRET - ) +@has_request_variables +def authenticate_notify( + request: HttpRequest, secret: Optional[str] = REQ("secret", default=None) +) -> bool: + return is_local_addr(request.META["REMOTE_ADDR"]) and secret == settings.SHARED_SECRET def client_is_exempt_from_rate_limiting(request: HttpRequest) -> bool: diff --git a/zerver/tests/test_decorators.py b/zerver/tests/test_decorators.py index 66b3c39936..92f9cecf84 100644 --- a/zerver/tests/test_decorators.py +++ b/zerver/tests/test_decorators.py @@ -1578,6 +1578,17 @@ class TestInternalNotifyView(ZulipTestCase): self.internal_notify(False, request) def test_internal_requests_with_broken_secret(self) -> None: + request = HostRequestMock( + post_data=dict(data="something"), + meta_data=dict(REMOTE_ADDR="127.0.0.1"), + ) + + with self.settings(SHARED_SECRET="random"): + self.assertFalse(authenticate_notify(request)) + with self.assertRaises(AccessDeniedError) as context: + self.internal_notify(True, request) + self.assertEqual(context.exception.http_status_code, 403) + secret = "random" request = HostRequestMock( post_data=dict(secret=secret),