Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[proposal] Authorization ( permissions ) handling #156

Closed
madEng84 opened this issue Jun 18, 2021 · 9 comments
Closed

[proposal] Authorization ( permissions ) handling #156

madEng84 opened this issue Jun 18, 2021 · 9 comments
Labels
auth enhancement New feature or request

Comments

@madEng84
Copy link

madEng84 commented Jun 18, 2021

In the docs I can see how to handle Authentication but not how to handle Authorization ( permissions )
After I have identified a logged user I want to return 403 if that user hasn't enough permission to do it
DRF has a great way to handle it, using BasePermission.has_permission and BasePermission.has_object_permission

Maybe this library could handle permissions with something like this:

from ninja import Schema
from ninja.permissions import BasePermission

class CanSeeFruitsPerm(BasePermission):
    def has_permission(self, request, view):
        return request.user.has_perm("can_see_fruits")

class AppleOut(Schema):
    color: str
    quality: str
    

@api.get("/apples", auth=BasicAuth(), perm=CanSeeFruitsPerm(), response=List[AppleOut])
def list_apples(request):
    return Apple.objects.all()

What do you think about it?

Thank you

@madEng84 madEng84 changed the title [proposal] Handle Authorization ( permissions ) [proposal] Authorization ( permissions ) handling Jun 18, 2021
@dozer133
Copy link

Great proposal!
Wouldn't this mean a lot repetition though?

Another idea would be to add an optional parameter to the __init__ of the Ninja authentication base classes, and having Ninja run a has_permission method if a perm is specified.
Then we can specify a permission when instantiating it.

For example

# inherit from base class BasicAuth
class UserAuth(BasicAuth):
    def authenticate(self, request, username, password):
        ....
    
    # Ninja runs this for you if a `perm` is specified on init
    def has_permission(self, request, perm):
        return request.user.has_perm(perm)

@api.get("/apples", auth=UserAuth(perm="can_see_fruits"), response=List[AppleOut])
def list_apples(request):
    return Apple.objects.all()

@vitalik vitalik added enhancement New feature or request auth labels Jun 25, 2021
@madEng84
Copy link
Author

I'm glad you like this proposal

I think they are different things, one permission could be used for unauthenticated users too, or the same permission could be used for different authentication types

What do you think about?

@marcosmoyano
Copy link

marcosmoyano commented Jun 9, 2022

I find myself repeating some sort of permission check against the authenticated user. It would be nice if Ninja supported permission checks like the following:

@router.get(
    url_name="resorce-get",
    path="/{user}/resource",
    response=ResourceSchema,
    auth=[auth1(), auth2(), ....].
    permissions=[perm1(), perm2(), ...]  # <- This
)
def get_resource(
    request: HttpRequest,
    user: str,
    **kwargs
):
....

This would hook nicely into Operation._run_checks

If something like this is welcome, I wouldn't mind trying to submit a PR for it.

@chrisbodon
Copy link

So what's the best method to approach different levels of permissions for users in Django ninja?

@flaeppe
Copy link
Contributor

flaeppe commented Aug 29, 2022

To throw out another idea; it would've been nice with an approach similar to Django's user_passes_test. Though it could come with the adjustment of returning a 403 forbidden response if test didn't pass.

In case django ninja could support decorating an operation, it could look like something below (e.g. login_required and user_passes_test would probably have to be reimplemented to not trigger a redirect, like Django's do)

def email_check(user):
    return user.email.endswith('@example.com')


@api.get("/apples", auth=UserAuth(), response=List[AppleOut])
@login_required
@user_passes_test(email_check)
@permission_required("can_see_fruits", raise_exception=True)
def list_apples(request):
    return Apple.objects.all()

@sebastian-philipp
Copy link

sebastian-philipp commented Jan 27, 2023

And yes, I think we have to re-implement a few decorators here.

I thought I could get around this with raise_exception=True and catching Django's PermissionDenied:

from django.contrib.auth.decorators import permission_required
from django.core.exceptions import PermissionDenied
from ninja import Router

api = NinjaAPI(auth=[some_auth()])

@api.exception_handler(PermissionDenied)
def django_permission_denied(
    request: HttpRequest, exc: PermissionDenied
) -> HttpResponse:
    return api.create_response(...)
   
...
    
@router.get("/")  # <- mypy error
@permission_required('app.some_permission', raise_exception=True)  # note: raise_exception=True
def get_something(request: HttpRequest) -> ...:
    ...

But this is not going to work out, as I'm getting this mypy error:

error: Value of type variable "_VIEW" of function cannot be "Callable[[HttpRequest], ...]"  [type-var]

due to

https://github.com/typeddjango/django-stubs/blob/b81b1bf3868eaab9fecebdd56ed817ffcb07fad8/django-stubs/contrib/auth/decorators.pyi#L8

And I actually think django-stubs is right in assuming the response to be of type HttpResponseBase.

@baseplate-admin
Copy link
Contributor

baseplate-admin commented Jun 16, 2023

I have made #776 as an alternative way to handle permissions :)

that's mostly inspied from django-rest-framework

@jtraub91
Copy link

jtraub91 commented Nov 9, 2023

Why can't this be implemented using django builtin permission_required decorator?

from django.contrib.auth.decorators import permission_required

@permission_required("app.view_model")
@api.get("/model")
def model_get(request):
    ...

This apparently doesn't work as of 0.22.2

@sebastian-philipp
Copy link

sebastian-philipp commented Nov 10, 2023

Why can't this be implemented using django builtin permission_required decorator?

cause, permission_required redirects to a login-page, which is not really the best idea for API clients

https://github.com/django/django/blob/f7389c4b07ceeb036436e065898e411b247bca78/django/contrib/auth/decorators.py#L36

@vitalik vitalik closed this as completed Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants