Browse Source

ref: consistently pass get_response to middleware classes (#52650)

django 3.x is noisy without this -- it's an error in django 4.x
anthony sottile 1 year ago
parent
commit
f5c113b6b1

+ 2 - 1
src/sentry/integrations/jira/views/base.py

@@ -3,6 +3,7 @@ from django.conf import settings
 from django.views.generic import View
 
 from sentry import options
+from sentry.middleware.placeholder import placeholder_get_response
 from sentry.web.helpers import render_to_response
 
 
@@ -34,7 +35,7 @@ class JiraSentryUIBaseView(View):
                 origin = "/".join(settings.STATIC_FRONTEND_APP_URL.split("/")[0:3])
                 csp_style_src.append(origin)
 
-            middleware = CSPMiddleware()
+            middleware = CSPMiddleware(placeholder_get_response)
             middleware.process_request(self.request)  # adds nonce
 
             response = render_to_response(self.html_file, context, self.request)

+ 12 - 0
src/sentry/middleware/placeholder.py

@@ -0,0 +1,12 @@
+from typing import NoReturn
+
+from django.http import HttpRequest
+
+
+def placeholder_get_response(request: HttpRequest) -> NoReturn:
+    """usage: `cls(get_response=placeholder_get_response)`
+
+    generally this is filled in by the request cycle but occasionally we use
+    middleware outside of the request cycle.
+    """
+    raise AssertionError("unreachable")

+ 2 - 2
src/sentry/services/hybrid_cloud/auth/impl.py

@@ -6,12 +6,12 @@ from typing import Any, List, Mapping
 from django.contrib.auth.models import AnonymousUser
 from django.db import connections, router, transaction
 from django.db.models import Count, F, Q
-from django.http import HttpResponse
 
 from sentry import roles
 from sentry.auth.access import get_permissions_for_user
 from sentry.auth.system import SystemToken
 from sentry.middleware.auth import RequestAuthenticationMiddleware
+from sentry.middleware.placeholder import placeholder_get_response
 from sentry.models import (
     ApiKey,
     ApiToken,
@@ -168,7 +168,7 @@ class DatabaseBackedAuthService(AuthService):
 
     def authenticate(self, *, request: AuthenticationRequest) -> MiddlewareAuthenticationResponse:
         fake_request = FakeAuthenticationRequest(request)
-        handler = RequestAuthenticationMiddleware(lambda _: HttpResponse("fake"))
+        handler = RequestAuthenticationMiddleware(placeholder_get_response)
         expired_user: User | None = None
         try:
             # Hahaha.  Yes.  You're reading this right.  I'm calling, the middleware, from the service method, that is

+ 2 - 4
src/sentry/web/frontend/base.py

@@ -24,6 +24,7 @@ from sentry.api.utils import generate_organization_url, is_member_disabled_from_
 from sentry.auth import access
 from sentry.auth.superuser import is_active_superuser
 from sentry.constants import ObjectStatus
+from sentry.middleware.placeholder import placeholder_get_response
 from sentry.models import Organization, OrganizationStatus, Project, Team, TeamStatus
 from sentry.models.avatars.base import AvatarBase
 from sentry.models.user import User
@@ -338,10 +339,7 @@ class BaseView(View, OrganizationMixin):
         return self.handle(request, *args, **kwargs)
 
     def test_csrf(self, request: HttpRequest) -> HttpResponse:
-        def _fake_get_response(request: HttpRequest) -> HttpResponse:
-            raise AssertionError("this should be unreachable")
-
-        middleware = CsrfViewMiddleware(_fake_get_response)
+        middleware = CsrfViewMiddleware(placeholder_get_response)
         return middleware.process_view(request, self.dispatch, [request], {})
 
     def get_access(self, request: HttpRequest, *args: Any, **kwargs: Any) -> access.Access:

+ 4 - 8
tests/sentry/auth/test_superuser.py

@@ -5,7 +5,6 @@ from unittest.mock import Mock, patch
 import pytest
 from django.contrib.auth.models import AnonymousUser
 from django.core import signing
-from django.http import HttpRequest, HttpResponse
 from django.utils import timezone
 from freezegun import freeze_time
 
@@ -26,6 +25,7 @@ from sentry.auth.superuser import (
     is_active_superuser,
 )
 from sentry.auth.system import SystemToken
+from sentry.middleware.placeholder import placeholder_get_response
 from sentry.middleware.superuser import SuperuserMiddleware
 from sentry.models import User
 from sentry.testutils import TestCase
@@ -44,10 +44,6 @@ INSIDE_PRIVILEGE_ACCESS_EXPIRE_TIME = timedelta(minutes=14)
 IDLE_EXPIRE_TIME = OUTSIDE_PRIVILEGE_ACCESS_EXPIRE_TIME = timedelta(hours=2)
 
 
-def _get_response(request: HttpRequest) -> HttpResponse:
-    return HttpResponse("example response")
-
-
 @freeze_time(BASETIME)
 @control_silo_test(stable=True)
 class SuperuserTestCase(TestCase):
@@ -310,7 +306,7 @@ class SuperuserTestCase(TestCase):
         delattr(request, "superuser")
         delattr(request, "is_superuser")
 
-        middleware = SuperuserMiddleware(_get_response)
+        middleware = SuperuserMiddleware(placeholder_get_response)
         middleware.process_request(request)
         assert request.superuser.is_active
         assert request.is_superuser()
@@ -334,7 +330,7 @@ class SuperuserTestCase(TestCase):
         delattr(request, "superuser")
         delattr(request, "is_superuser")
 
-        middleware = SuperuserMiddleware(_get_response)
+        middleware = SuperuserMiddleware(placeholder_get_response)
         middleware.process_request(request)
         assert not request.superuser.is_active
         assert not request.is_superuser()
@@ -350,7 +346,7 @@ class SuperuserTestCase(TestCase):
         delattr(request, "superuser")
         delattr(request, "is_superuser")
 
-        middleware = SuperuserMiddleware(_get_response)
+        middleware = SuperuserMiddleware(placeholder_get_response)
         middleware.process_request(request)
         assert not request.superuser.is_active
         assert not request.is_superuser()

+ 3 - 6
tests/sentry/sudo/test_middleware.py

@@ -1,18 +1,15 @@
 import pytest
-from django.http import HttpRequest, HttpResponse
+from django.http import HttpResponse
 
 from fixtures.sudo_testutils import BaseTestCase
+from sentry.middleware.placeholder import placeholder_get_response
 from sudo.middleware import SudoMiddleware
 from sudo.settings import COOKIE_NAME
 from sudo.utils import grant_sudo_privileges, revoke_sudo_privileges
 
 
-def get_response(request: HttpRequest) -> HttpResponse:
-    return HttpResponse("hello there")
-
-
 class SudoMiddlewareTestCase(BaseTestCase):
-    middleware = SudoMiddleware(get_response)
+    middleware = SudoMiddleware(placeholder_get_response)
 
     def assertSignedCookieEqual(self, v1, v2, reason=None):
         value, _, _ = v1.split(":")

+ 3 - 2
tests/sentry/web/test_client_config.py

@@ -6,11 +6,12 @@ from typing import Callable, Optional, Tuple
 import pytest
 from django.contrib.auth.models import AnonymousUser, User
 from django.core.cache import cache
-from django.http import HttpRequest, HttpResponse
+from django.http import HttpRequest
 from django.test import override_settings
 
 from sentry.app import env
 from sentry.middleware.auth import AuthenticationMiddleware
+from sentry.middleware.placeholder import placeholder_get_response
 from sentry.models import AuthIdentity, AuthProvider
 from sentry.silo import SiloMode
 from sentry.testutils.factories import Factories
@@ -29,7 +30,7 @@ def request_factory(f):
             request, user = result
             if not user.is_anonymous:
                 login(request, user)
-                AuthenticationMiddleware(lambda _: HttpResponse("fake")).process_request(request)
+                AuthenticationMiddleware(placeholder_get_response).process_request(request)
             else:
                 request.user = user
                 request.auth = None