From 76e937829fe593016cb4d44d5504598db4003518 Mon Sep 17 00:00:00 2001 From: Marat Mkhitaryan Date: Fri, 10 Jan 2025 02:47:59 -0800 Subject: [PATCH 1/5] fix of #1386 --- ninja/operation.py | 18 +++++++++++---- tests/test_weird_case.py | 49 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 tests/test_weird_case.py diff --git a/ninja/operation.py b/ninja/operation.py index 623c0b7d0..0b62d07b3 100644 --- a/ninja/operation.py +++ b/ninja/operation.py @@ -1,3 +1,4 @@ +import inspect from typing import ( TYPE_CHECKING, Any, @@ -187,14 +188,23 @@ def _run_authentication(self, request: HttpRequest) -> Optional[HttpResponse]: for callback in self.auth_callbacks: try: if is_async_callable(callback) or getattr(callback, "is_async", False): - result = async_to_sync(callback)(request) + # Store the initial result of the callback + initial_result = callback(request) + # Check if the initial result is a coroutine + if inspect.iscoroutine(initial_result): + # If it's a coroutine, use async_to_sync to resolve it + resolved_result = async_to_sync(callback)(request) + else: + # If it's not a coroutine, use the initial result + resolved_result = initial_result else: - result = callback(request) + # For synchronous callbacks, directly use the result + resolved_result = callback(request) except Exception as exc: return self.api.on_exception(request, exc) - if result: - request.auth = result # type: ignore + if resolved_result is not None: + request.auth = resolved_result # type: ignore return None return self.api.on_exception(request, AuthenticationError()) diff --git a/tests/test_weird_case.py b/tests/test_weird_case.py new file mode 100644 index 000000000..22e4de70d --- /dev/null +++ b/tests/test_weird_case.py @@ -0,0 +1,49 @@ + +from ninja import NinjaAPI +from ninja.security import ( + HttpBearer, +) +from ninja.testing import TestAsyncClient, TestClient + + +class SyncBearerAuth(HttpBearer): + def authenticate(self, request, token: str): + return + + +class AsyncBearerAuth(HttpBearer): + async def authenticate(self, request, token: str): + return + + +api = NinjaAPI() + + +@api.get("/sync/", auth=SyncBearerAuth()) +def sync_auth(request): + return + + +@api.get("/async/", auth=AsyncBearerAuth()) +def async_auth(request): + return + + +client = TestClient(api) +async_client = TestAsyncClient(api) + + +def test_async_bearer_invalid_bearer(): + # TypeError: object NoneType can't be used in 'await' expression + response = client.get("/async/", headers={"Authorization": "Bearer 123"}) + assert response.status_code == 401 + + +def test_async_bearer_no_bearer(): + response = client.get("/async/") + assert response.status_code == 401 + + +def test_sync_bearer_no_berer(): + response = client.get("/sync/") + assert response.status_code == 401 From 6cb8d09fb167a15051c984a839dfa811c127c5e1 Mon Sep 17 00:00:00 2001 From: Marat Mkhitaryan Date: Fri, 10 Jan 2025 02:58:50 -0800 Subject: [PATCH 2/5] add testcase --- tests/test_auth.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_auth.py b/tests/test_auth.py index 01fb64814..fae76804d 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -64,6 +64,14 @@ def authenticate(self, request, token): raise AuthorizationError +class AsyncBearerAuth(HttpBearer): + async def authenticate(self, request, token): + if token == "bearertoken": + return token + if token == "nottherightone": + raise AuthorizationError + + def demo_operation(request): return {"auth": request.auth} @@ -85,6 +93,7 @@ def on_custom_error(request, exc): ("apikeycookie", KeyCookie()), ("basic", BasicAuth()), ("bearer", BearerAuth()), + ("async_bearer", AsyncBearerAuth()), ("customexception", KeyHeaderCustomException()), ]: api.get(f"/{path}", auth=auth, operation_id=path)(demo_operation) @@ -187,6 +196,12 @@ class MockSuperUser(str): 401, BODY_UNAUTHORIZED_DEFAULT, ), + ( + "/async_bearer", + dict(headers={"Authorization": "Bearer nonexistingtoken"}), + 401, + BODY_UNAUTHORIZED_DEFAULT, + ), ( "/bearer", dict(headers={"Authorization": "Bearer nottherightone"}), @@ -215,6 +230,7 @@ def test_schema(): assert schema["components"]["securitySchemes"] == { "BasicAuth": {"scheme": "basic", "type": "http"}, "BearerAuth": {"scheme": "bearer", "type": "http"}, + "AsyncBearerAuth": {"scheme": "bearer", "type": "http"}, "KeyCookie": {"in": "cookie", "name": "key", "type": "apiKey"}, "KeyHeader": {"in": "header", "name": "key", "type": "apiKey"}, "KeyHeaderCustomException": {"in": "header", "name": "key", "type": "apiKey"}, From aa9c69cef8053a8797510e504da130e54ca5a77a Mon Sep 17 00:00:00 2001 From: Marat Mkhitaryan Date: Fri, 10 Jan 2025 03:00:33 -0800 Subject: [PATCH 3/5] rudimentary testcase --- tests/test_weird_case.py | 49 ---------------------------------------- 1 file changed, 49 deletions(-) delete mode 100644 tests/test_weird_case.py diff --git a/tests/test_weird_case.py b/tests/test_weird_case.py deleted file mode 100644 index 22e4de70d..000000000 --- a/tests/test_weird_case.py +++ /dev/null @@ -1,49 +0,0 @@ - -from ninja import NinjaAPI -from ninja.security import ( - HttpBearer, -) -from ninja.testing import TestAsyncClient, TestClient - - -class SyncBearerAuth(HttpBearer): - def authenticate(self, request, token: str): - return - - -class AsyncBearerAuth(HttpBearer): - async def authenticate(self, request, token: str): - return - - -api = NinjaAPI() - - -@api.get("/sync/", auth=SyncBearerAuth()) -def sync_auth(request): - return - - -@api.get("/async/", auth=AsyncBearerAuth()) -def async_auth(request): - return - - -client = TestClient(api) -async_client = TestAsyncClient(api) - - -def test_async_bearer_invalid_bearer(): - # TypeError: object NoneType can't be used in 'await' expression - response = client.get("/async/", headers={"Authorization": "Bearer 123"}) - assert response.status_code == 401 - - -def test_async_bearer_no_bearer(): - response = client.get("/async/") - assert response.status_code == 401 - - -def test_sync_bearer_no_berer(): - response = client.get("/sync/") - assert response.status_code == 401 From acb38424641a634e58295dc54dadbd47a212c880 Mon Sep 17 00:00:00 2001 From: Marat Mkhitaryan <57956285+mmkhitaryan@users.noreply.github.com> Date: Sun, 12 Jan 2025 20:56:57 +0000 Subject: [PATCH 4/5] simplify naming --- ninja/operation.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/ninja/operation.py b/ninja/operation.py index 0b62d07b3..0864390fb 100644 --- a/ninja/operation.py +++ b/ninja/operation.py @@ -188,23 +188,16 @@ def _run_authentication(self, request: HttpRequest) -> Optional[HttpResponse]: for callback in self.auth_callbacks: try: if is_async_callable(callback) or getattr(callback, "is_async", False): - # Store the initial result of the callback - initial_result = callback(request) - # Check if the initial result is a coroutine - if inspect.iscoroutine(initial_result): - # If it's a coroutine, use async_to_sync to resolve it - resolved_result = async_to_sync(callback)(request) - else: - # If it's not a coroutine, use the initial result - resolved_result = initial_result + result = callback(request) + if inspect.iscoroutine(result): + result = async_to_sync(callback)(request) else: - # For synchronous callbacks, directly use the result - resolved_result = callback(request) + result = callback(request) except Exception as exc: return self.api.on_exception(request, exc) - if resolved_result is not None: - request.auth = resolved_result # type: ignore + if result: + request.auth = result # type: ignore return None return self.api.on_exception(request, AuthenticationError()) From 4cd5cb059ce22a6ed0d680ab664adac1d57542d2 Mon Sep 17 00:00:00 2001 From: Marat Mkhitaryan <57956285+mmkhitaryan@users.noreply.github.com> Date: Sun, 12 Jan 2025 21:01:56 +0000 Subject: [PATCH 5/5] fix coverage missing test --- tests/test_auth.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_auth.py b/tests/test_auth.py index fae76804d..57150139b 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -202,6 +202,12 @@ class MockSuperUser(str): 401, BODY_UNAUTHORIZED_DEFAULT, ), + ( + "/async_bearer", + dict(headers={}), + 401, + BODY_UNAUTHORIZED_DEFAULT, + ), ( "/bearer", dict(headers={"Authorization": "Bearer nottherightone"}),