Opened 7 years ago

Closed 6 years ago

Last modified 5 years ago

#13190 closed (fixed)

Empty settings.AUTHENTICATION_BACKENDS creates hard to trace problem

Reported by: joel3000 Owned by: Gabriel Hurley
Component: contrib.auth Version: 1.1
Severity: Keywords: easy-pickings AUTHENTICATION_BACKENDS
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

This is an obscure problem. If in settings.py the AUTHENTICATION_BACKENDS list is empty (i.e. AUTHENTICATION_BACKENDS=()),
then there will be no backends available to authenticate as it will override the django default behavior of using the default auth code.

When this occurs the auth login screen will just return a generic error about invalid username and password that is a pain to trace.

Unfortunately, I don't have time to figure out how to submit this change by the procedure but here is what I suggest to fix it.

In django/contrib/auth/init.py , add this check to get_backends():

def get_backends():
    from django.conf import settings
    backends = []
    for backend_path in settings.AUTHENTICATION_BACKENDS:
        backends.append(load_backend(backend_path))
 #### new code start####    
  if len(backends)==0:
        raise ImproperlyConfigured, 'settings.AUTHENTICATION_BACKENDS is empty.'
   #### new code end ##### 
return backends

It's an obscure problem, but it does seem to violate the "no magic" rule for django, and it tripped me up for a day
having to hunt it down.

Attachments (2)

13190_empty_auth_backends.diff (2.6 KB) - added by Gabriel Hurley 6 years ago.
13190_empty_auth_backends.diff.2 (2.6 KB) - added by Łukasz Rekucki 6 years ago.
Use new exception syntax.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 7 years ago by Gabriel Hurley

Has patch: set
milestone: 1.2
Owner: changed from nobody to Gabriel Hurley
Status: newassigned

comment:2 Changed 7 years ago by James Bennett

milestone: 1.2

This is enough of an edge case (you have to manually clear AUTHENTICATION_BACKENDS to trigger it) that it's not 1.2-critical.

comment:3 Changed 7 years ago by Russell Keith-Magee

Needs tests: set
Triage Stage: UnreviewedAccepted

comment:4 Changed 7 years ago by Gabriel Hurley

Needs tests: unset

Test added. I'll agree with James Bennett about it not being critical for 1.2. I've certainly never run into this bug myself. It seems like a pretty safe/stable change though... Either way, I've got no stake in it personally.

comment:5 Changed 6 years ago by Gabriel Hurley

milestone: 1.3

Bumping to 1.3 milestone.

Changed 6 years ago by Gabriel Hurley

comment:6 Changed 6 years ago by Gabriel Hurley

Added an updated patch which applies cleanly to trunk.

comment:7 Changed 6 years ago by Paul McMillan

Triage Stage: AcceptedReady for checkin

This patch looks good to me. Marking as RFC.

comment:8 Changed 6 years ago by Alex Gaynor

The code really should be changed to use the raise Exception("message") syntax.

Changed 6 years ago by Łukasz Rekucki

Use new exception syntax.

comment:9 Changed 6 years ago by Gabriel Hurley

Keywords: easy-pickings added

comment:10 Changed 6 years ago by Russell Keith-Magee

Resolution: fixed
Status: assignedclosed

(In [14793]) Fixed #13190 -- Improved error handling for the case where no authentication backends are defined. Thanks to Joel3000 for the report, and Łukasz Rekucki for the final patch.

comment:11 Changed 6 years ago by Russell Keith-Magee

(In [14799]) [1.2.X] Fixed #13190 -- Improved error handling for the case where no authentication backends are defined. Thanks to Joel3000 for the report, and Łukasz Rekucki for the final patch.

Backport of r14793 from trunk.

comment:12 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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