Permissions are verified before authentication.
Closed this issue · 15 comments
How to reproduce.
from typing import Any
from django.http import HttpRequest
from ninja_extra import permissions, NinjaExtraAPI, api_controller, http_get
from ninja_jwt.authentication import AsyncJWTAuth
from ninja_jwt.controller import NinjaJWTDefaultController
api = NinjaExtraAPI()
api.register_controllers(NinjaJWTDefaultController)
class MyAuth(AsyncJWTAuth):
async def authenticate(self, request: HttpRequest, token: str) -> Any:
# This method will be called after MyPerm.has_permission calling.
# Expected behavior: Authenticate should be called firstly.
return await self.async_jwt_authenticate(request, token)
class MyPerm(permissions.BasePermission):
def has_permission(self, request, controller):
return request.user.groups.filter(name="MyGroup").exists()
@api_controller('', tags=['My Operations'], auth=MyAuth(), permissions=[MyPerm])
class MyAPIController:
@http_get("/path")
async def get_operation(self):
return {"ok": "ok"}
api.register_controllers(MyAPIController)
Issue:
MyPerm.has_permissions
is called before AsyncJWTAuth.authenticate
so when MyPerm.has_permissions
is called, the request.user
will be always AnonymousUser
because AsyncJWTAuth.authenticate
has not been called yet.
Expected behavior
AsyncJWTAuth.authenticate
is called first to initialize request.user
, and then BasePermission.has_permission
@airddm Thanks🙏 for bringing this to my notice. It was an oversight
I have also published a new release, please let me know if the problem is resolved at your end.
Hi @eadwinCode 👋
In my case, after upgrading to the last version I have:
{
"detail": "user_id: Input should be a valid UUID, invalid character: expected an optional prefix of `urn:uuid:` followed by [0-9a-fA-F-], found `m` at 1"
}
@api_controller("/users", tags=["User"])
class UserAPI:
@http_get("/me")
def get_current_user(self, request):
return {"debug": "ok"}
from ninja.security import django_auth
api = NinjaExtraAPI(auth=django_auth)
MIDDLEWARE = [
"django.middleware.security.SecurityMiddleware",
"corsheaders.middleware.CorsMiddleware",
"django.contrib.sessions.middleware.SessionMiddleware",
"django.middleware.locale.LocaleMiddleware",
"django.middleware.common.CommonMiddleware",
"django.middleware.csrf.CsrfViewMiddleware",
"main.middlewares.auth_cached.CachedAuthenticationMiddleware",
"main.middlewares.tenant.XTenantMiddleware",
"main.middlewares.cache_control.CacheControlMiddleware",
"django.contrib.messages.middleware.MessageMiddleware",
"django.middleware.clickjacking.XFrameOptionsMiddleware",
"django_structlog.middlewares.RequestMiddleware",
]
@Tragio can you give an example source code that resulted to the error? Your current example is not showing anything that relates to user_id.
@eadwinCode interesting, so the problem it is not picking GET /me
only if I remove the other dynamic ones 🤔
import uuid
from ninja_extra import (
api_controller,
http_delete,
http_get,
status,
)
@api_controller("/users", tags=["User"])
class UserAPI:
@http_get("/me")
def get_current_user(self, request):
return {"debug": "ok"}
@http_get("/{user_id}")
def get_user(self, request, user_id: uuid.UUID):
return {"debug": "ok"}
@http_delete("/{user_id}", response={status.HTTP_204_NO_CONTENT: None})
def delete_user_from_clinic(self, request, user_id: uuid.UUID, token: str):
return {"debug": "ok"}
@Tragio you have two endpoints with there same path configuration, get_current_user
and get_user
has the same path configuration. so when you call get_current_user
, django will call get_user
cause in set up hierarchy get_user
comes last.
My solution for your
@api_controller("/users", tags=["User"])
class UserAPI:
@http_get("/")
def get_current_user(self, request):
return {"debug": "ok"}
@http_get("/{user_id}")
def get_user(self, request, user_id: uuid.UUID):
return {"debug": "ok"}
@http_delete("/{user_id}", response={status.HTTP_204_NO_CONTENT: None})
def delete_user_from_clinic(self, request, user_id: uuid.UUID, token: str):
return {"debug": "ok"}
Also, was this working before the upgrade?
I have also published a new release, please let me know if the problem is resolved at your end.
It works ! Thank you very much 👍
@eadwinCode it was working before, that is why I was confused with the behaviour 😅 I thought it respected the order that I had 🤔
@Tragio that's interesting I will look into it again, I can't remember making changes to the route models.
But what version were you on before the upgrade?
@eadwinCode I was on the version 0.21.4, when I upgraded to 0.21.6 it started with that problem 🤔
Edit: I can also change the route. My question is what is the intended behaviour, as I initially thought it was in the order I had written.
@Tragio I will create a fix for this shortly, I have seen what caused it
@Tragio You can pull the latest release in 15 minutes, that should solve it.
@eadwinCode already tested, it is working. You're amazing, thank you very much ❤️
Awesome... Thanks for reporting this