diff --git a/isic/auth.py b/isic/auth.py index b3588086..d91b861b 100644 --- a/isic/auth.py +++ b/isic/auth.py @@ -1,6 +1,10 @@ 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 @@ -8,6 +12,24 @@ 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()] diff --git a/isic/middleware.py b/isic/middleware.py index 3588c21c..b63e0291 100644 --- a/isic/middleware.py +++ b/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 diff --git a/isic/settings/base.py b/isic/settings/base.py index ecb347de..29a5c4df 100644 --- a/isic/settings/base.py +++ b/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", diff --git a/isic/urls.py b/isic/urls.py index fcd0f189..bcbaac7e 100644 --- a/isic/urls.py +++ b/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) diff --git a/isic/zip_download/api.py b/isic/zip_download/api.py index 1e8cade0..d4b0128b 100644 --- a/isic/zip_download/api.py +++ b/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: diff --git a/pyproject.toml b/pyproject.toml index fe5df7ed..20081954 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -265,7 +265,6 @@ filterwarnings = [ "ignore:unclosed