Skip to content

Commit

Permalink
CSRF automatically checked by Cookie based auth
Browse files Browse the repository at this point in the history
  • Loading branch information
vitalik committed Jul 11, 2023
1 parent 8e9b074 commit 84a31a6
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 91 deletions.
18 changes: 11 additions & 7 deletions docs/docs/reference/csrf.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,28 @@ api = NinjaAPI(csrf=True)
<span style="color: red;">Warning</span>: It is not secure to use API's with cookie-based authentication! (like `CookieKey`, or `django_auth`) when csrf is turned OFF.


**Django Ninja** will prevent you from doing this. So, if you do this:
**Django Ninja** will automatically enable csrf for Cookie based authentication


```Python hl_lines="4"
```Python hl_lines="8"
from ninja import NinjaAPI
from ninja.security import django_auth
from ninja.security import APIKeyCookie

api = NinjaAPI(auth=django_auth)
class CookieAuth(APIKeyCookie):
def authenticate(self, request, key):
return key == "test"

api = NinjaAPI(auth=CookieAuth())

```

it will raise an error. Instead, you need to set the `csrf` argument to `True` to enable CSRF checks:

or django-auth based (which is inherited from cookie based auth):

```Python hl_lines="4"
from ninja import NinjaAPI
from ninja.security import django_auth

api = NinjaAPI(auth=django_auth, csrf=True)
api = NinjaAPI(auth=django_auth)

```
```
15 changes: 1 addition & 14 deletions ninja/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -478,9 +478,7 @@ def _lookup_exception_handler(self, exc: Exc) -> Optional[ExcHandler]:
return None

def _validate(self) -> None:
from ninja.security import APIKeyCookie

# 1) urls namespacing validation
# urls namespacing validation
skip_registry = os.environ.get("NINJA_SKIP_REGISTRY", False)
if (
not skip_registry
Expand All @@ -497,17 +495,6 @@ def _validate(self) -> None:
raise ConfigError(msg.strip())
NinjaAPI._registry.append(self.urls_namespace)

# 2) csrf
if self.csrf is False:
for _prefix, router in self._routers:
for path_operation in router.path_operations.values():
for operation in path_operation.operations:
for auth in operation.auth_callbacks:
if isinstance(auth, APIKeyCookie):
raise ConfigError(
"Cookie Authentication must be used with CSRF. Please use NinjaAPI(csrf=True)"
)


_imported_while_running_in_debug_server = is_debug_server()

Expand Down
5 changes: 5 additions & 0 deletions ninja/openapi/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django.urls import reverse

from ninja.conf import settings as ninja_settings
from ninja.constants import NOT_SET
from ninja.responses import Response
from ninja.types import DictStrAny

Expand Down Expand Up @@ -42,8 +43,12 @@ def openapi_view(request: HttpRequest, api: "NinjaAPI") -> HttpResponse:
so we automatically detect - if ninja is in INSTALLED_APPS - then we render with django.shortcuts.render
otherwise - rendering custom html with swagger js from cdn
"""
add_csrf = api.csrf
if not add_csrf and api.auth != NOT_SET:
add_csrf = any(getattr(a, "csrf", False) for a in api.auth) # type: ignore
context = {
"api": api,
"add_csrf": add_csrf,
"openapi_json_url": reverse(f"{api.urls_namespace}:openapi-json"),
}
if "ninja" in settings.INSTALLED_APPS:
Expand Down
12 changes: 11 additions & 1 deletion ninja/security/apikey.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
from django.http import HttpRequest

from ninja.compatibility.request import get_headers
from ninja.errors import HttpError
from ninja.security.base import AuthBase
from ninja.utils import check_csrf

__all__ = ["APIKeyBase", "APIKeyQuery", "APIKeyCookie", "APIKeyHeader"]

Expand All @@ -14,7 +16,7 @@ class APIKeyBase(AuthBase, ABC):
param_name: str = "key"

def __init__(self) -> None:
self.openapi_name = self.param_name
self.openapi_name = self.param_name # this sets the name of the security schema
super().__init__()

def __call__(self, request: HttpRequest) -> Optional[Any]:
Expand All @@ -40,7 +42,15 @@ def _get_key(self, request: HttpRequest) -> Optional[str]:
class APIKeyCookie(APIKeyBase, ABC):
openapi_in: str = "cookie"

def __init__(self, csrf: bool = True) -> None:
self.csrf = csrf
super().__init__()

def _get_key(self, request: HttpRequest) -> Optional[str]:
if self.csrf:
error_response = check_csrf(request)
if error_response:
raise HttpError(403, "CSRF check Failed")
return request.COOKIES.get(self.param_name)


Expand Down
4 changes: 2 additions & 2 deletions ninja/templates/ninja/swagger.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
</head>
<body
data-openapi-url="{{ openapi_json_url }}"
data-csrf-token="{% if api.csrf %}{{ csrf_token }}{% endif %}"
data-api-csrf="{{ api.csrf|default:"" }}">
data-csrf-token="{% if add_csrf %}{{ csrf_token }}{% endif %}"
data-api-csrf="{% if add_csrf %}true{% endif %}">

<div id="swagger-ui"></div>

Expand Down
2 changes: 1 addition & 1 deletion ninja/templates/ninja/swagger_cdn.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
SwaggerUIBundle.SwaggerUIStandalonePreset
],
layout: "BaseLayout",
{% if api.csrf and csrf_token %}
{% if add_csrf %}
requestInterceptor: (req) => {
req.headers['X-CSRFToken'] = "{{csrf_token}}"
return req;
Expand Down
6 changes: 5 additions & 1 deletion ninja/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ def normalize_path(path: str) -> str:
return path


def _no_view() -> None:
pass # pragma: no cover


def check_csrf(
request: HttpRequest, callback: Callable
request: HttpRequest, callback: Callable = _no_view
) -> Optional[HttpResponseForbidden]:
mware = CsrfViewMiddleware(lambda x: HttpResponseForbidden()) # pragma: no cover
request.csrf_processing_done = False # type: ignore
Expand Down
93 changes: 67 additions & 26 deletions tests/test_csrf.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import re
from unittest import mock

import pytest
Expand All @@ -6,7 +7,7 @@

from ninja import NinjaAPI
from ninja.errors import ConfigError
from ninja.security import APIKeyCookie
from ninja.security import APIKeyCookie, APIKeyHeader
from ninja.testing import TestClient as BaseTestClient


Expand Down Expand Up @@ -63,38 +64,78 @@ def test_csrf_on():
assert client.post("/post/csrf_exempt", COOKIES=COOKIES).status_code == 200


def test_raises_on_cookie_auth():
"It should raise if user picked Cookie based auth and csrf=False"
def test_csrf_cookie_auth():
"Cookie based authtentication should have csrf check by default"

class Auth(APIKeyCookie):
class CookieAuth(APIKeyCookie):
def authenticate(self, request, key):
return request.COOKIES[key] == "foo"
return key == "test"

api = NinjaAPI(auth=Auth(), csrf=False)
cookie_auth = CookieAuth()
api = NinjaAPI(auth=cookie_auth)

@api.get("/some")
def some_method(request):
pass
@api.post("/test")
def test_view(request):
return {"success": True}

with pytest.raises(ConfigError):
api._validate()
client = TestClient(api)

try:
import os
# No auth - access denied
assert client.post("/test").status_code == 403

os.environ["NINJA_SKIP_REGISTRY"] = ""
# Cookie auth + valid csrf
cookies = {"key": "test"}
cookies.update(COOKIES)
response = client.post("/test", COOKIES=cookies, headers={"X-CSRFTOKEN": TOKEN})
assert response.status_code == 200, response.content

# Check for wrong error reported
match = "Looks like you created multiple NinjaAPIs"
with pytest.raises(ConfigError, match=match):
_urls = api.urls
# Cookie auth + INVALID csrf
response = client.post(
"/test", COOKIES=cookies, headers={"X-CSRFTOKEN": TOKEN + "invalid"}
)
assert response.status_code == 403, response.content

# django debug server can attempt to import the urls twice when errors exist
# verify we get the correct error reported
match = "Cookie Authentication must be used with CSRF"
with pytest.raises(ConfigError, match=match):
with mock.patch("ninja.main._imported_while_running_in_debug_server", True):
_urls = api.urls
# Turning off csrf on cookie, valid key, no csrf passed
cookie_auth.csrf = False
response = client.post("/test", COOKIES={"key": "test"})
assert response.status_code == 200, response.content

finally:
os.environ["NINJA_SKIP_REGISTRY"] = "yes"

def test_docs():
"Testing that docs are initializing csrf headers correctly"

api = NinjaAPI(csrf=True)

client = TestClient(api)
resp = client.get("/docs")
assert resp.status_code == 200
csrf_token = re.findall(r'data-csrf-token="(.*?)"', resp.content.decode("utf8"))[0]
assert len(csrf_token) > 0

api.csrf = False
resp = client.get("/docs")
assert resp.status_code == 200
csrf_token = re.findall(r'data-csrf-token="(.*?)"', resp.content.decode("utf8"))[0]
assert len(csrf_token) == 0


def test_docs_cookie_auth():
class CookieAuth(APIKeyCookie):
def authenticate(self, request, key):
return key == "test"

class HeaderAuth(APIKeyHeader):
def authenticate(self, request, key):
return key == "test"

api = NinjaAPI(csrf=False, auth=CookieAuth())
client = TestClient(api)
resp = client.get("/docs")
csrf_token = re.findall(r'data-csrf-token="(.*?)"', resp.content.decode("utf8"))[0]
assert len(csrf_token) > 0

api = NinjaAPI(csrf=False, auth=HeaderAuth())
client = TestClient(api)
resp = client.get("/docs")
csrf_token = re.findall(r'data-csrf-token="(.*?)"', resp.content.decode("utf8"))[0]
assert len(csrf_token) == 0
39 changes: 0 additions & 39 deletions tests/test_csrf_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,42 +60,3 @@ async def post_on(request):
"/post", COOKIES=COOKIES, headers={"X-CSRFTOKEN": TOKEN}
)
assert response.status_code == 200


@pytest.mark.skipif(django.VERSION < (3, 1), reason="requires django 3.1 or higher")
@pytest.mark.asyncio
async def test_raises_on_cookie_auth():
"It should raise if user picked Cookie based auth and csrf=False"

class Auth(APIKeyCookie):
def authenticate(self, request, key):
return request.COOKIES[key] == "foo"

api = NinjaAPI(auth=Auth(), csrf=False)

@api.get("/some")
async def some_method(request):
pass

with pytest.raises(ConfigError):
api._validate()

try:
import os

os.environ["NINJA_SKIP_REGISTRY"] = ""

# Check for wrong error reported
match = "Looks like you created multiple NinjaAPIs"
with pytest.raises(ConfigError, match=match):
_urls = api.urls

# django debug server can attempt to import the urls twice when errors exist
# verify we get the correct error reported
match = "Cookie Authentication must be used with CSRF"
with pytest.raises(ConfigError, match=match):
with mock.patch("ninja.main._imported_while_running_in_debug_server", True):
_urls = api.urls

finally:
os.environ["NINJA_SKIP_REGISTRY"] = "yes"

0 comments on commit 84a31a6

Please sign in to comment.