Allow Ninja to enforce CSRF on a per-auth method basis

isic/auth.py

@@ -1,13 +1,35 @@
 from collections.abc import Callable
 
-from ninja.security import HttpBearer, django_auth
+from django.http import HttpRequest
+from ninja.errors import HttpError
+from ninja.operation import PathView
+from ninja.security import HttpBearer, SessionAuth
+from ninja.utils import check_csrf
 from oauth2_provider.oauth2_backends import get_oauthlib_core
 
 from isic.core.permissions import SessionAuthStaffUser
 
 ACCESS_PERMS = ["any", "is_authenticated", "is_staff"]
 
 
+class CsrfFixedSessionAuth(SessionAuth):
+    def _get_key(self, request: HttpRequest) -> str | None:
+        if self.csrf:
+            # Work around https://github.com/vitalik/django-ninja/issues/1068
+            # Maybe related https://github.com/vitalik/django-ninja/issues/1101
+            path_view: PathView = request.resolver_match.func.__self__
+            view_func = path_view._find_operation(request).view_func
+            # The upstream implementation doesn't send "view_func" to "check_csrf", so a
+            # "csrf_exempt" annotation can't be detected.
+            error_response = check_csrf(request, view_func)
+            if error_response:
+                raise HttpError(403, "CSRF check Failed")
+        return request.COOKIES.get(self.param_name)
+
+
+django_auth = CsrfFixedSessionAuth()
+
+
 class OAuth2AuthBearer(HttpBearer):
     def __init__(self, perm: str):
         if perm not in ACCESS_PERMS:
@@ -38,7 +60,8 @@ def authenticate(self, request, token):
             request.oauth2_error = getattr(r, "oauth2_error", {})
 
 
+# Always test OAuth2 before session auth, since OAuth2 doesn't have CSRF messiness.
 # The lambda _: True is to handle the case where a user doesn't pass any authentication.
-allow_any: list[Callable] = [django_auth, OAuth2AuthBearer("any"), lambda _: True]
-is_authenticated = [django_auth, OAuth2AuthBearer("is_authenticated")]
-is_staff = [SessionAuthStaffUser(), OAuth2AuthBearer("is_staff")]
+allow_any: list[Callable] = [OAuth2AuthBearer("any"), lambda _: True, django_auth]
+is_authenticated = [OAuth2AuthBearer("is_authenticated"), django_auth]
+is_staff = [OAuth2AuthBearer("is_staff"), SessionAuthStaffUser()]

isic/middleware.py

@@ -1,36 +1,10 @@
-from collections.abc import Callable
 import logging
-from typing import Any
 
-from django.http import HttpRequest, HttpResponseBase
-from ninja.operation import PathView
 from sentry_sdk import set_tag
 
 logger = logging.getLogger(__name__)
 
 
-# See https://github.com/vitalik/django-ninja/issues/283.
-# This exists to allow the header based authentication to be exempt from CSRF checks,
-# while still enforcing CSRF checks on the session based authentication.
-class ExemptBearerAuthFromCSRFMiddleware:
-    def __init__(self, get_response):
-        self.get_response = get_response
-
-    def __call__(self, request):
-        return self.get_response(request)
-
-    def process_view(
-        self,
-        request: HttpRequest,
-        view_func: Callable[..., HttpResponseBase],
-        view_args: tuple[Any, ...],
-        view_kwargs: dict[str, Any],
-    ) -> None:
-        klass = getattr(view_func, "__self__", None)
-        if klass and isinstance(klass, PathView):
-            request._dont_enforce_csrf_checks = True  # noqa: SLF001
-
-
 class UserTypeTagMiddleware:
     def __init__(self, get_response):
         self.get_response = get_response

isic/settings/base.py

@@ -82,8 +82,6 @@
     "isic.middleware.UserTypeTagMiddleware",
     "django.contrib.sessions.middleware.SessionMiddleware",
     "django.middleware.common.CommonMiddleware",
-    # Insert "ExemptBearerAuthFromCSRFMiddleware" just before the CsrfViewMiddleware
-    "isic.middleware.ExemptBearerAuthFromCSRFMiddleware",
     "django.middleware.csrf.CsrfViewMiddleware",
     "django.contrib.auth.middleware.AuthenticationMiddleware",
     "django.contrib.messages.middleware.MessageMiddleware",

isic/urls.py

@@ -36,7 +36,6 @@
     version="v2",
     docs_url=None,  # we want to serve the docs next to the ninja root rather than under it
     auth=allow_any,
-    csrf=True,
     urls_namespace="api",
 )
 swagger_view = partial(openapi_view, api=api)

isic/zip_download/api.py

@@ -93,8 +93,8 @@ def zip_api_auth(request: HttpRequest):
     return ZipDownloadTokenAuth()(request)
 
 
-@csrf_exempt
 @zip_router.post("/url/", response=str, include_in_schema=False)
+@csrf_exempt
 def create_zip_download_url(request: HttpRequest, payload: SearchQueryIn):
     url: ParseResult | None = settings.ISIC_ZIP_DOWNLOAD_SERVICE_URL
     if url is None:

pyproject.toml

@@ -265,7 +265,6 @@ filterwarnings = [
   "ignore:unclosed <socket\\.socket:ResourceWarning",
   # https://github.com/vitalik/django-ninja/issues/1245
   "ignore:Support for class-based `config` is deprecated:pydantic.warnings.PydanticDeprecatedSince20",
-  "ignore:csrf argument is deprecated:DeprecationWarning:isic.urls",
   "ignore:[\\w]+Factory\\._after_postgeneration will stop saving the instance:DeprecationWarning:factory.django",
   # In test_publish_copies_default_attribution
   "ignore:Unclosed file <tempfile\\.SpooledTemporaryFile:ResourceWarning",