Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#33231 closed New feature (wontfix)

authenticate() doesn't raise ImproperlyConfigured for incorrect authentication backends.

Reported by: JunKi Yoon Owned by: JunKi Yoon
Component: contrib.auth Version: 3.2
Severity: Normal Keywords: authenticate, AUTHENTICATION_BACKENDS
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

def authenticate doesn't raise ImproperlyConfigured for incorrect AUTHENTICATION_BACKENDS (with custom backends)

In my opinion, if an TypeError occurs, there is a problem with AUTHENTICATION_BACKENDS configure,
so raise ImproperlyConfigured should be performed.

Let's say you have a custom authentication backend like this:

AUTHENTICATION_BACKENDS = ['django.contrib.auth.backends.ModelBackend', 'myapp.backends.CustomAuth']
# myapp.backends.py
class CustomAuth(BaseBackend):
    def authenticate(self, username=None, password=None):
        # blahblah..

In this case, if request is not included as a parameter in the def authenticate function of the CustomAuth class, a TypeError occurs.

Both ModelBackend and CustomAuth fail authentication, and you can't even guess that the settings(custom backend) are wrong. So I think raise ImproperlyConfigured is necessary.

@sensitive_variables('credentials')
def authenticate(request=None, **credentials):
    """
    If the given credentials are valid, return a User object.
    """
    for backend, backend_path in _get_backends(return_tuples=True):
        backend_signature = inspect.signature(backend.authenticate)
        try:
            backend_signature.bind(request, **credentials)
        except TypeError:
            # This backend doesn't accept these credentials as arguments. Try the next one.
            continue 
        try:
            user = backend.authenticate(request, **credentials)
        except PermissionDenied:
            # This backend says to stop in our tracks - this user should not be allowed in at all.
            break
        if user is None:
            continue
        # Annotate the user object with the path of the backend.
        user.backend = backend_path
        return user

    # The credentials supplied are invalid to all backends, fire signal
    user_login_failed.send(sender=__name__, credentials=_clean_credentials(credentials), request=request)

Change History (2)

comment:1 by Mariusz Felisiak, 5 years ago

Resolution: → wontfix
Status: assigned → closed
Summary: def authenticate doesn't raise ImproperlyConfigured for incorrect AUTHENTICATION_BACKENDS (with custom backends) → authenticate() doesn't raise ImproperlyConfigured for incorrect authentication backends.

Django supports authentication backends with different set of credentials, so they can have authenticate() methods with different signatures. For example:

  • authenticate(self, request, token=None) and
  • authenticate(self, request, username=None, password=None)

once you can call it with a token and once with an username/password. Both calls are valid. That's why TypeError must be ignored (see also #31968).

in reply to:  1 comment:2 by JunKi Yoon, 5 years ago

Replying to Mariusz Felisiak:

Django supports authentication backends with different set of credentials, so they can have authenticate() methods with different signatures. For example:

  • authenticate(self, request, token=None) and
  • authenticate(self, request, username=None, password=None)

once you can call it with a token and once with an username/password. Both calls are valid. That's why TypeError must be ignored (see also #31968).

Thanks for the detailed reply. I completely understand.

Have a good day:)

Note: See TracTickets for help on using tickets.
Back to Top