Pithikos / rest-framework-roles

Role-based permissions for Django and Django REST Framework
MIT License
62 stars 4 forks source link

Question: Authentication vs Authorization with view_permissions. It return code 403 on both #11

Open MVKozlov opened 7 months ago

MVKozlov commented 7 months ago

Background: I have admin user and plain user

when I use permission_classes = [permissions.IsAdminUser] my api return

When I use view_permissions = { 'get': { 'admin': True }, 'options': { 'admin': True }, } my api return

Why don't I get 401 without authentication at all?

REST_FRAMEWORK = {
    'DEFAULT_AUTHENTICATION_CLASSES': (
        'rest_framework.authentication.BasicAuthentication',
        'rest_framework.authentication.SessionAuthentication',
    ),

This does not correspond to what is written here The first authentication class set on the view is used when determining the type of response.

Is this because the module tries all authentication methods, and not just the first or some kind of bug ?

Pithikos commented 7 months ago

This is not something that is built-in atm. Essentially 403 is the fallback for anything that is not 200.

In your case, unauthenticated users don't match any role since you don't use anon, and the fallback is PermissionDenied('Permission denied for user.').

A fast solution is to use a custom role checker for anon and then just edit your view_permissions. The caveat is that you'll get 401 for any matching anon regardless if its granted value is set to True or False. So not sure if this would be a problem or not in your case..

roles.py

from rest_framework_roles.roles import is_admin, is_user
from rest_framework.exceptions import NotAuthenticated

def is_anon(request, view):
    if request.user.is_anonymous:
        raise NotAuthenticated

ROLES = {
    'admin': is_admin,
    'user': is_user,
    'anon': is_anon,
}

views.py

class MyView:
  view_permissions = { 'get': { 'admin': True, 'anon': False}, 'options': { 'admin': True, 'anon': False}}

Another solution is to use a middleware that returns 401 for non-authenticated users.

Pithikos commented 7 months ago

FUTURE CONSIDERATION

I'm really not sure what's the best approach for this (or if it even should be addressed).. Possibly it should return 401 when no role was matched, and 403 when granting fails. The main issue is when you're treating anon as a valid user role.

Potential approaches:

  1. Fallback to 401 for unmatched roles
    ROLES = {
    'admin': is_admin,
    'user': is_user,
    }

In this case though, you wouldn't be able to use role anon which might be ok or not depending on the app.

  1. Specify default exception for unguarded views
    REST_FRAMEWORK_ROLES = {
    ..
    "DEFAULT_UNGUARDED_VIEW_EXCEPTION": NotAuthenticated,
    }

But this feels just wrong. Namely you'll probably be getting many false negatives.

  1. Using DEFAULT_VIEW_PERMISSIONS could be used
REST_FRAMEWORK_ROLES = {
  ..
  "DEFAULT_VIEW_PERMISSIONS": {
    "anon": False,
  },
}

This however will only fire on explicitly guarded views (aka via view_permissions).

  1. Using a prehook
    
    def prehook(request):
    if not request.user:
    raise NotAuthenticated

REST_FRAMEWORK_ROLES = { .. "BEFORE_ROLE_CHECKS": prehook }

MVKozlov commented 7 months ago

Thanks, while I tried the built-in is_anon(), it didn't work. Everything worked out with the custom option.

And in my case this is exactly the solution I need.

I feel like I need to dive more into the documentation :)

Maybe it will be enough to add it to rest_framework_roles.roles as

def is_unauth(request, view):
     if request.user.is_anonymous:
         raise NotAuthenticated

and have roles.py

ROLES = {
    # Django out-of-the-box
    'admin': is_admin,
    'user': is_user,
    'anon': is_anon,
    'unauth': is_unauth,
...
}

or just add it to FAQ

Although, of course, a library solution that returns an exception rather than a result is a hack

Pithikos commented 7 months ago

@MVKozlov I'm drafting a solution for this for the future. Could you please describe your specific use case?

Namely:

  1. Are you making an API where all non-authenticated (anon) requests should raise 401?
  2. Is the API using views that non-authenticated can access (e.g. tinyurl)

    It should be relateively easy to make an umbrella behaviour built-in (aka 401 for all non-authenticated). The problem is if you want your API to serve some views to the public and some not.

MVKozlov commented 7 months ago

Yes, in my case, all unauthenticated users should raise 401, and authenticated users, depending on the role, 403 or 200

It is planned that the frontend of the application will redirect the user to authorization upon response or respond that access is denied