django-oauth-toolkit icon indicating copy to clipboard operation
django-oauth-toolkit copied to clipboard

oauth2_provider.backends.OAuth2Backend does not work when request.user is None.

Open Kraust opened this issue 2 years ago • 1 comments

Describe the bug

The oauth2_provider.backends.OAuth2Backend backend does not work when no user is provided. This would be common when the grant type is client_credentials. This causes the user_login_failed signal in django.contrib.auth.authenticate to fire if there are no other backends that can authenticate the request.

To Reproduce

Create a project with oauth2_provider.backends.OAuth2Backend as the only authentication backend, and set up a receiver for user_login_failed where you raise an exception. You will see that if you create an application and request a token with client_credentials grant type, any requests with that token will fail.

Expected behavior

I have a custom OAu2thBackend that attempts to resolve this issue, where AnonymousUser is returned if request.user is None, however, I'm unsure if this is the desired behavior.

class OAuth2Backend(backends.OAuth2Backend):
    """
        Custom class for oauth2 authentication 
        The only difference here is that we return an
        AnonymousUser when request.user is None.
    """

    def authenticate(self, request=None, **credentials):
        if request is not None:
            try:
                valid, request = OAuthLibCore.verify_request(
                    request, scopes=[])
            except ValueError as error:
                if str(error) == "Invalid hex encoding in query string.":
                    raise SuspiciousOperation(error)
                else:
                    raise
            else:
                if valid:
                    # if request.user is None, return AnonymousUser
                    if request.user is None:
                        return AnonymousUser
                    return request.user

        return None

Version

1.7.1

  • [ ] I have tested with the latest published release and it's still a problem.
  • [ ] I have tested with the master branch and it's still a problem.

Additional context

I can write up a sample demonstrating this if desired.

Kraust avatar Oct 27 '22 16:10 Kraust

@Kraust I think in theory your proposal makes sense. Can you open a PR for a more in depth discussion of the implementation?

dopry avatar Dec 12 '22 04:12 dopry