From f28d9d39eb8faf26edbcb5b03c5df95d63c407f9 Mon Sep 17 00:00:00 2001 From: antonrh Date: Fri, 10 May 2024 13:19:36 +0300 Subject: [PATCH 1/9] Improve swagger csrf detection --- ninja/openapi/docs.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/ninja/openapi/docs.py b/ninja/openapi/docs.py index c70fadd1..b217db2a 100644 --- a/ninja/openapi/docs.py +++ b/ninja/openapi/docs.py @@ -1,7 +1,7 @@ import json from abc import ABC, abstractmethod from pathlib import Path -from typing import TYPE_CHECKING, Any, Optional +from typing import TYPE_CHECKING, Any, Optional, Union from django.conf import settings from django.http import HttpRequest, HttpResponse @@ -13,7 +13,8 @@ if TYPE_CHECKING: # if anyone knows a cleaner way to make mypy happy - welcome - from ninja import NinjaAPI # pragma: no cover + from ninja import NinjaAPI, Router # pragma: no cover + from ninja.operation import Operation ABS_TPL_PATH = Path(__file__).parent.parent / "templates/ninja/" @@ -103,9 +104,21 @@ def _render_cdn_template( def _csrf_needed(api: "NinjaAPI") -> bool: - if api.csrf: - return True - if not api.auth or api.auth == NOT_SET: - return False - - return any(getattr(a, "csrf", False) for a in api.auth) # type: ignore + if not hasattr(api, "_csrf_cache"): + for operation in _iter_operations(api): + for auth_callback in operation.auth_callbacks: + if getattr(auth_callback, "csrf", False): + api._csrf_cache = True + return True + continue + api._csrf_cache = False + return api._csrf_cache + + +def _iter_operations(api_or_router: Union["NinjaAPI", "Router"]) -> Iterator["Operation"]: + """this is helper to iterate over all operations in api or router""" + if isinstance(api_or_router, Router): + for _, path_view in api_or_router.path_operations.items(): + yield from path_view.operations + for _, router in api_or_router._routers: # noqa + yield from _iter_operations(router) From 6831f585373557e6897886d998ebe67026822948 Mon Sep 17 00:00:00 2001 From: antonrh Date: Fri, 10 May 2024 13:20:50 +0300 Subject: [PATCH 2/9] Improve swagger csrf detection --- ninja/openapi/docs.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/ninja/openapi/docs.py b/ninja/openapi/docs.py index b217db2a..49a2db9e 100644 --- a/ninja/openapi/docs.py +++ b/ninja/openapi/docs.py @@ -103,6 +103,15 @@ def _render_cdn_template( return HttpResponse(html) +def _iter_operations(api_or_router: Union["NinjaAPI", "Router"]) -> Iterator["Operation"]: + """this is helper to iterate over all operations in api or router""" + if isinstance(api_or_router, Router): + for _, path_view in api_or_router.path_operations.items(): + yield from path_view.operations + for _, router in api_or_router._routers: # noqa + yield from _iter_operations(router) + + def _csrf_needed(api: "NinjaAPI") -> bool: if not hasattr(api, "_csrf_cache"): for operation in _iter_operations(api): @@ -113,12 +122,3 @@ def _csrf_needed(api: "NinjaAPI") -> bool: continue api._csrf_cache = False return api._csrf_cache - - -def _iter_operations(api_or_router: Union["NinjaAPI", "Router"]) -> Iterator["Operation"]: - """this is helper to iterate over all operations in api or router""" - if isinstance(api_or_router, Router): - for _, path_view in api_or_router.path_operations.items(): - yield from path_view.operations - for _, router in api_or_router._routers: # noqa - yield from _iter_operations(router) From 4efb8b79728783da9a90c5c9334eb5351c8d98a9 Mon Sep 17 00:00:00 2001 From: antonrh Date: Fri, 10 May 2024 13:22:43 +0300 Subject: [PATCH 3/9] Add missing Iterator type --- ninja/openapi/docs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ninja/openapi/docs.py b/ninja/openapi/docs.py index 49a2db9e..0064ed85 100644 --- a/ninja/openapi/docs.py +++ b/ninja/openapi/docs.py @@ -1,7 +1,7 @@ import json from abc import ABC, abstractmethod from pathlib import Path -from typing import TYPE_CHECKING, Any, Optional, Union +from typing import TYPE_CHECKING, Any, Iterator, Optional, Union from django.conf import settings from django.http import HttpRequest, HttpResponse From f5879919c1f7fdf2cbbe65c651f0a220a6c74a0e Mon Sep 17 00:00:00 2001 From: antonrh Date: Fri, 10 May 2024 13:35:25 +0300 Subject: [PATCH 4/9] Fix linters errors --- ninja/openapi/docs.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/ninja/openapi/docs.py b/ninja/openapi/docs.py index 0064ed85..0f58bb4c 100644 --- a/ninja/openapi/docs.py +++ b/ninja/openapi/docs.py @@ -8,12 +8,12 @@ from django.shortcuts import render from django.urls import reverse -from ninja.constants import NOT_SET +from ninja.router import Router from ninja.types import DictStrAny if TYPE_CHECKING: # if anyone knows a cleaner way to make mypy happy - welcome - from ninja import NinjaAPI, Router # pragma: no cover + from ninja import NinjaAPI # pragma: no cover from ninja.operation import Operation ABS_TPL_PATH = Path(__file__).parent.parent / "templates/ninja/" @@ -103,7 +103,7 @@ def _render_cdn_template( return HttpResponse(html) -def _iter_operations(api_or_router: Union["NinjaAPI", "Router"]) -> Iterator["Operation"]: +def _iter_operations(api_or_router: Union["NinjaAPI", Router]) -> Iterator["Operation"]: """this is helper to iterate over all operations in api or router""" if isinstance(api_or_router, Router): for _, path_view in api_or_router.path_operations.items(): @@ -113,12 +113,14 @@ def _iter_operations(api_or_router: Union["NinjaAPI", "Router"]) -> Iterator["Op def _csrf_needed(api: "NinjaAPI") -> bool: - if not hasattr(api, "_csrf_cache"): + add_csrf = getattr(api, "_add_csrf", None) + if add_csrf is None: for operation in _iter_operations(api): for auth_callback in operation.auth_callbacks: if getattr(auth_callback, "csrf", False): - api._csrf_cache = True + api._add_csrf = True # type: ignore[attr-defined] return True continue - api._csrf_cache = False - return api._csrf_cache + add_csrf = False + api._add_csrf = add_csrf # type: ignore[attr-defined] + return add_csrf From 8c432a216617ea534726eb40a6f2f444f3020658 Mon Sep 17 00:00:00 2001 From: antonrh Date: Fri, 10 May 2024 14:15:28 +0300 Subject: [PATCH 5/9] Add swagger add_csrf tests --- tests/test_csrf.py | 47 +++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/tests/test_csrf.py b/tests/test_csrf.py index eeab4884..aea44c5b 100644 --- a/tests/test_csrf.py +++ b/tests/test_csrf.py @@ -98,10 +98,18 @@ def test_view(request): assert response.status_code == 200, response.content -def test_docs(): +def test_docs_add_csrf_globally(): "Testing that docs are initializing csrf headers correctly" - api = NinjaAPI(csrf=True) + class CookieAuth(APIKeyCookie): + def authenticate(self, request, key): + return key == "test" + + api = NinjaAPI(csrf=False, auth=CookieAuth()) # `csrf=False` should be ignored + + @api.get("/test1") + def endpoint1(request): + return {"success": True} client = TestClient(api) resp = client.get("/docs") @@ -109,29 +117,42 @@ def test_docs(): 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_add_csrf_by_operation(): + "Testing that docs are initializing csrf headers correctly" -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) # `csrf=False` should be ignored + + @api.get("/test1", auth=CookieAuth()) + def endpoint1(request): + return {"success": True} + + @api.get("/test2") + def endpoint1(request): + return {"success": True} - api = NinjaAPI(csrf=False, auth=CookieAuth()) 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 = NinjaAPI(csrf=False, auth=HeaderAuth()) + +def test_docs_do_not_add_csrf(): + class HeaderAuth(APIKeyHeader): + def authenticate(self, request, key): + return key == "test" + + api = NinjaAPI(csrf=True, auth=HeaderAuth()) # `csrf=True` should be ignored + + @api.get("/test1") + def endpoint1(request): + return {"success": True} + client = TestClient(api) resp = client.get("/docs") csrf_token = re.findall(r'data-csrf-token="(.*?)"', resp.content.decode("utf8"))[0] From f429c09b132e29a1972b56045f83beae94211513 Mon Sep 17 00:00:00 2001 From: antonrh Date: Fri, 10 May 2024 14:20:07 +0300 Subject: [PATCH 6/9] Update swagger _csrf_needed helper --- ninja/openapi/docs.py | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/ninja/openapi/docs.py b/ninja/openapi/docs.py index 0f58bb4c..92a5e2ca 100644 --- a/ninja/openapi/docs.py +++ b/ninja/openapi/docs.py @@ -1,7 +1,7 @@ import json from abc import ABC, abstractmethod from pathlib import Path -from typing import TYPE_CHECKING, Any, Iterator, Optional, Union +from typing import Callable, TYPE_CHECKING, Any, Iterator, Optional, Union from django.conf import settings from django.http import HttpRequest, HttpResponse @@ -14,7 +14,6 @@ if TYPE_CHECKING: # if anyone knows a cleaner way to make mypy happy - welcome from ninja import NinjaAPI # pragma: no cover - from ninja.operation import Operation ABS_TPL_PATH = Path(__file__).parent.parent / "templates/ninja/" @@ -103,24 +102,26 @@ def _render_cdn_template( return HttpResponse(html) -def _iter_operations(api_or_router: Union["NinjaAPI", Router]) -> Iterator["Operation"]: +def _iter_auth_callbacks( + api_or_router: Union["NinjaAPI", Router], +) -> Iterator[Callable[..., Any]]: """this is helper to iterate over all operations in api or router""" if isinstance(api_or_router, Router): for _, path_view in api_or_router.path_operations.items(): - yield from path_view.operations + for operation in path_view.operations: + yield from operation.auth_callbacks for _, router in api_or_router._routers: # noqa - yield from _iter_operations(router) + yield from _iter_auth_callbacks(router) def _csrf_needed(api: "NinjaAPI") -> bool: - add_csrf = getattr(api, "_add_csrf", None) - if add_csrf is None: - for operation in _iter_operations(api): - for auth_callback in operation.auth_callbacks: - if getattr(auth_callback, "csrf", False): - api._add_csrf = True # type: ignore[attr-defined] - return True - continue - add_csrf = False - api._add_csrf = add_csrf # type: ignore[attr-defined] - return add_csrf + add_csrf: Optional[bool] = getattr(api, "_add_csrf", None) + if add_csrf is not None: + return add_csrf + for auth_callback in _iter_auth_callbacks(api): + if getattr(auth_callback, "csrf", False): + api._add_csrf = True # type: ignore[attr-defined] + return True + continue + api._add_csrf = False # type: ignore[attr-defined] + return False From d917e624ccd6305374d78fed5986f25596a61cfe Mon Sep 17 00:00:00 2001 From: antonrh Date: Fri, 10 May 2024 14:30:35 +0300 Subject: [PATCH 7/9] Fix linter errors --- ninja/openapi/docs.py | 2 +- tests/test_csrf.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ninja/openapi/docs.py b/ninja/openapi/docs.py index 92a5e2ca..756b8a6e 100644 --- a/ninja/openapi/docs.py +++ b/ninja/openapi/docs.py @@ -1,7 +1,7 @@ import json from abc import ABC, abstractmethod from pathlib import Path -from typing import Callable, TYPE_CHECKING, Any, Iterator, Optional, Union +from typing import TYPE_CHECKING, Any, Callable, Iterator, Optional, Union from django.conf import settings from django.http import HttpRequest, HttpResponse diff --git a/tests/test_csrf.py b/tests/test_csrf.py index aea44c5b..ffbfe470 100644 --- a/tests/test_csrf.py +++ b/tests/test_csrf.py @@ -107,8 +107,8 @@ def authenticate(self, request, key): api = NinjaAPI(csrf=False, auth=CookieAuth()) # `csrf=False` should be ignored - @api.get("/test1") - def endpoint1(request): + @api.get("/test") + def test_view(request): return {"success": True} client = TestClient(api) @@ -128,11 +128,11 @@ def authenticate(self, request, key): api = NinjaAPI(csrf=False) # `csrf=False` should be ignored @api.get("/test1", auth=CookieAuth()) - def endpoint1(request): + def test_view1(request): return {"success": True} @api.get("/test2") - def endpoint1(request): + def test_view2(request): return {"success": True} client = TestClient(api) @@ -149,8 +149,8 @@ def authenticate(self, request, key): api = NinjaAPI(csrf=True, auth=HeaderAuth()) # `csrf=True` should be ignored - @api.get("/test1") - def endpoint1(request): + @api.get("/test") + def test_view(request): return {"success": True} client = TestClient(api) From 71f5c76163b4ec14d88cb30a029086bcfe76bf8b Mon Sep 17 00:00:00 2001 From: antonrh Date: Fri, 10 May 2024 14:36:39 +0300 Subject: [PATCH 8/9] Improve code coverage --- tests/test_csrf.py | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/tests/test_csrf.py b/tests/test_csrf.py index ffbfe470..17d5198a 100644 --- a/tests/test_csrf.py +++ b/tests/test_csrf.py @@ -3,7 +3,7 @@ from django.conf import settings from django.views.decorators.csrf import csrf_exempt -from ninja import NinjaAPI +from ninja import NinjaAPI, Router from ninja.security import APIKeyCookie, APIKeyHeader from ninja.testing import TestClient as BaseTestClient @@ -98,7 +98,7 @@ def test_view(request): assert response.status_code == 200, response.content -def test_docs_add_csrf_globally(): +def test_docs_add_csrf(): "Testing that docs are initializing csrf headers correctly" class CookieAuth(APIKeyCookie): @@ -112,6 +112,14 @@ def test_view(request): return {"success": 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 + + assert hasattr(api, "_add_csrf") # `api._add_csrf` should be set as cache + resp = client.get("/docs") assert resp.status_code == 200 csrf_token = re.findall(r'data-csrf-token="(.*?)"', resp.content.decode("utf8"))[0] @@ -142,6 +150,34 @@ def test_view2(request): assert len(csrf_token) > 0 +def test_docs_add_csrf_by_sub_router(): + "Testing that docs are initializing csrf headers correctly" + + class CookieAuth(APIKeyCookie): + def authenticate(self, request, key): + return key == "test" + + api = NinjaAPI(csrf=False) # `csrf=False` should be ignored + + @api.get("/test1", auth=CookieAuth()) + def test_view1(request): + return {"success": True} + + router = Router() + + @router.get("/test2") + def test_view2(request): + return {"success": True} + + api.add_router("/router", router) + + 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 + + def test_docs_do_not_add_csrf(): class HeaderAuth(APIKeyHeader): def authenticate(self, request, key): From d3e3304d1a0cbfdc4c8c1b294c871dc004104919 Mon Sep 17 00:00:00 2001 From: antonrh Date: Fri, 10 May 2024 14:39:11 +0300 Subject: [PATCH 9/9] Improve code coverage --- ninja/openapi/docs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ninja/openapi/docs.py b/ninja/openapi/docs.py index 756b8a6e..6b32c9bc 100644 --- a/ninja/openapi/docs.py +++ b/ninja/openapi/docs.py @@ -122,6 +122,5 @@ def _csrf_needed(api: "NinjaAPI") -> bool: if getattr(auth_callback, "csrf", False): api._add_csrf = True # type: ignore[attr-defined] return True - continue api._add_csrf = False # type: ignore[attr-defined] return False