Disallow custom authentication class to return user = `None`

[leandrodesouzadev]

Hello there! Currently it's allowed that an custom authentication class returns a tuple containing (None, None). This causes the authentication to be considered successful and sets request.user = None and this can cause errors that are hard to track where they came from. I know that this wrongly implemented by the CustomAuthentication class, the user should've raised an AuthenticationFailed exception instead. But this allows the user to shoot itself on the foot.

Example of a bad implemented authentication class:

from rest_framework.authentication import BaseAuthentication

class MyDumbAuthentication(BaseAuthentication):
  def authenticate(self, request):
    return None, None

Later if you have a permission check for example, you would see the following error:

from rest_framework.permissions import BasePermission

class MyPermCheck(BasePermission):
  def has_permission(self, request, view):
    return request.user.has_perm("foo.bar")

# raises AttributeError("'NoneType' object has no attribute 'has_perm'")

It would be nice if rest framework disallowed this totally wrong implementation.

Checklist

• [x] Raised initially as discussion
• [x] This is not a feature request suitable for implementation outside this project. Please elaborate what it is:
  • [ ] compatibility fix for new Django/Python version ...
  • [x] other type of bug fix
  • [ ] other type of improvement that does not touch existing code or change existing behavior (e.g. wrapper for new Django field)
• [x] I have reduced the issue to the simplest possible case.

[leandrodesouzadev]

I would be happy to submit a patch to disallow this!

[krishnamadhavan]

@leandrodesouzadev All implementations of permission_classes from rest_framework uses something like below:

return bool(request.user and request.user.is_staff)

So, you always verify request.user before calling any methods on request.user.

Alternatively, like here, you can raise errors before returning from the function. This way we ensure we never return None.

Does that answer your question? Happy to help otherwise.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.