Code

Opened 5 years ago

Closed 3 years ago

#10378 closed Bug (wontfix)

authenticate() method should not continue on built-in or generic exceptions

Reported by: bendavis78 Owned by: nobody
Component: contrib.auth Version: 1.0
Severity: Normal Keywords: authenticate TypeError
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX:

Description

from django.contrib.auth, in authenticate():

    for backend in get_backends():
        try:
            user = backend.authenticate(**credentials)
        except TypeError:
            # This backend doesn't accept these credentials as arguments. Try the next one.
            continue
        if user is None:
            continue

The authenticate method makes an assumption about the meaning of a TypeError, being that "this backend doesn't accept these credentials as arguments". It should use a custom exception type where the meaning is more specific, such as AuthInvalidCredentials or something.

The reasoning behind this is that when creating your own authentication backend, it's possible to do some things that unexpectedly raise a more generic exception, such as TypeError. This can produce some very unexpected results, as this will cause your backend to be "skipped" when it shouldn't have been.

Granted, I could work around this by catching TypeError within the backend, but the backend developer shouldn't have to know that he/she needs to do that. Plus, the developer would have to go through some hoops to actually see the exception that was caught (eg, extracting traceback info from sys.exc_info())

Attachments (0)

Change History (7)

comment:1 Changed 5 years ago by jacob

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

comment:2 Changed 5 years ago by mtredinnick

Would be backwards incompatible. I know we've said "contrib" has an exception from the backwards compatibility guarantee, but we still shouldn't make changes like this without strong reason, particularly in something as heavily used as the authentication area. We fail safe here (don't let anybody in if things are raised), so it's only a diagnostic/debugging issue, not a functionality one.

I'm pretty close to -1 on making this change at the moment. It's unfortunate, but not that harmful to leave it as is.

comment:3 Changed 5 years ago by thejaswi_puthraya

  • Component changed from Uncategorized to Contrib apps

comment:4 Changed 5 years ago by thejaswi_puthraya

  • Component changed from Contrib apps to Authentication

comment:5 Changed 5 years ago by bendavis78

If anyone's curious about a workaround for this, you can wrap your authenticate to catch the TypeError. I created a custom exception called "NextBackend" that I throw if I want the authentication backend to continue, then in the wrapper if a TypeError is caught, throw it as a "_TypeError" exception so that a real TypeError is not caught by the core auth code.

class NextBackend(Exception): pass
class _TypeError(Exception): pass

class MyCustomBackend(ModelBackend):
    """ 
    Wrapper method for _authenticate(). See http://code.djangoproject.com/ticket/10378
    """
    def authenticate(self, **kwargs):
        try:
            return self._authenticate(**kwargs)
        except TypeError:
            e, args, tb = sys.exc_info()
            raise _TypeError, args, tb
        except NextBackend:
            raise TypeError
    
    def _authenticate(self, **kwargs):
        # Your auth code here...
        if authentication_fails():
            raise NextBackend

It's fugly but it works :-P

comment:6 Changed 3 years ago by SmileyChris

  • Severity set to Normal
  • Type set to Bug

comment:7 Changed 3 years ago by SmileyChris

  • Easy pickings unset
  • Resolution set to wontfix
  • Status changed from new to closed

Backing up Malcolm's -1 and closing the issue. Unfortunate, but that's about it.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.