New auth signal: user_login_failed

[Michael]

I've implemented a new signal in Django called user_login_fail, in django.contrib.auth.

It is fired whenever an unsuccessful use of django.contrib.auth.authenticate() occurs, so will apply to all unsuccessful login attempts.

An example of use:

from django.contrib.auth.signals import user_login_fail

def login_attempt_failure_handler(sender, **kwargs):
	print "login attempt failure from %s: %r" % (sender, kwargs)

user_login_failure.connect(login_attempt_failure_handler)

This would then print on the console while running the development server:

login attempt failure from django.contrib.auth: {'credentials': {'username': u'michael', 'password': u'notmypassword'}, 'signal': <django.dispatch.dispatcher.Signal object at 0x1ba58d0>}

I'm aware at the moment that this passes back the password, however the authenticate method takes in kwargs for it's authentication, and could include some extra information needed when notifying the administrator (such as a login realm).

The other issue is that this has no idea about what is the sender or the request associated with this signal. I'm unsure of whether this could be fixed in the signal (and present some better information), or just delegate this responsibility to the use of the traceback module.

Patch / pull request is incoming.

[Michael]

Pull request with patch: ​https://github.com/django/django/pull/201

Tests pass with SQLite. I've added an additional test to make sure this signal is fired correctly.

[anonymous]

Great idea. I manually tested this to work for me too, with the minor exception that the last line of code should be

user_login_fail.connect(login_attempt_failure_handler)

[Michael]

Per the Github pull request comment by Brad Pitcher, I've changed the name of the signal to user_login_failed to match the verb tense of the other signals.

Updated the code and the tests, and checks out.

[Aymeric Augustin]

Yes, I've needed this myself, mostly for logging or security.

[Brad Pitcher]

I wrote some documentation for this in another pull request:
​https://github.com/django/django/pull/204

[Michael]

Brad, the documentation looks good, however the use of the sender parameter isn't correct there (it appears copy-pasted from the logout signal?)

The sender parameter sends the name of the auth module. Because the login failure isn't tied directly to a request (authenticate doesn't provide such a parameter), it doesn't know about where it came from.

This comes back to my note in the OP that I'm unsure if this should be something fixed in the signal or if the traceback module is a better option. Or perhaps the signal should just set the sender to None.

[Brad Pitcher]

Thanks for the catch. I updated the documentation for the sender parameter in ​https://github.com/brad/django/tree/ticket_18616_docs

[Jannis Leidel]

Seems like ​https://github.com/django/django/pull/201 is the latest code. Please make sure to always point to the correct pull request here!

[Michael]

I've updated my pull request now (201) with Brad's documentation fix mentioned.

[Michael]

I've now merged django/master into my pull request.

[Preston Holmes]

I've reviewed, merged locally, verified docs, tests and ready to land, but have asked Paul to weigh in on the idea of sending credentials in the signal. While a project author should be able to fully control what listeners register for the signal, I'd feel better with Paul's quick opinion on it.

[Paul McMillan]

I'd strongly prefer that we didn't send the password in a signal. I realize that this could be (ab)used for things like "you just tried to log in with your mother's maiden name, and we've switched to requiring the name of your first dentist!", or it could be used slightly more legitimately to chain into some other kind of system that acts kinda like a backend but not really. Those use cases should really be their own auth backend. I think this is primarily useful for logging (and acting on) failed login attempts. In that case, the actual password used probably shouldn't be passed along.

As the original poster said, since the credentials are a dict we don't know in advance which field is the password (or otherwise sensitive). Can we re-use the filtering system we already have in place to remove passwords from tracebacks?

[Preston Holmes]

It wouldn't take much to use a filtering system like that used for debug views.

django.debug.views.decorators & django.views.debug contain the pattern

If we were to do that though, it would seem that the backend should specify what the "sensitive credentials" are - but seems a bit overkill for this signal, and pretty meaningless as a general backend attribute without some clear definition of what contexts a credential would be "sensitive".

I think the best middle ground would be to use a regex along the lines of:

​https://github.com/django/django/blob/54c81a1c936f3682e3405d6737958fdffa39bed9/django/views/debug.py#L20

and strip any key from the dictionary that matches.

This along with a note in the docs seems like it should cover most foot shooting situations.

[Preston Holmes <preston@…>]

In 7cc4068c4470876c526830778cbdac2fdfd6dc26:

Fixed #18616 -- added user_login_fail signal to contrib.auth

Thanks to Brad Pitcher for documentation

[Cristiano Coelho]

Sorry for writing to such an old ticket but I couldn't find anything else related to this.
Is there any option for a signal to be triggered as well when an account that's not an admin attempts to access the admin page? As right now it 'almost' looks like a failed log in attempt, but it isn't.

[Tim Graham]

Tickets aren't an appropriate place to ask usage questions. Please use support channels instead.