[Django] #18616: New auth signal: user_login_fail
Django
------------------------------+--------------------
Reporter: micolous | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/18616>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by micolous):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:1>
Django
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Comment (by anonymous):
Great idea. I manually tested this to work for me too, with the minor
exception that the last line of code should be
{{{#!python
user_login_fail.connect(login_attempt_failure_handler)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:2>
Django
------------------------------+--------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Comment (by micolous):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:3>
Django
------------------------------+--------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 1
Changes (by micolous):
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:4>
Django
------------------------------+------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Changes (by aaugustin):
* stage: Unreviewed => Accepted
Comment:
Yes, I've needed this myself, mostly for logging or security.
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:5>
Django
------------------------------+------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Comment (by bradpitcher):
I wrote some documentation for this in another pull request:
https://github.com/django/django/pull/204
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:6>
Django
------------------------------+------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Comment (by micolous):
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`.
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:7>
Django
------------------------------+------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Comment (by bradpitcher):
Thanks for the catch. I updated the documentation for the sender parameter
in https://github.com/brad/django/tree/ticket_18616_docs
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:8>
Django
------------------------------+------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Accepted
Changes (by bradpitcher):
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:9>
Django
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by jezdez):
* stage: Accepted => Ready for checkin
Comment:
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!
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:10>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by micolous):
I've updated my pull request now (201) with Brad's documentation fix
mentioned.
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:11>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* cc: bradpitcher@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:12>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by micolous):
I've now merged django/master into my pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:13>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by ptone):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:14>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by PaulM):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:15>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by ptone):
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:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:16>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"7cc4068c4470876c526830778cbdac2fdfd6dc26"]:
{{{
#!CommitTicketReference repository=""
revision="7cc4068c4470876c526830778cbdac2fdfd6dc26"
Fixed #18616 -- added user_login_fail signal to contrib.auth
Thanks to Brad Pitcher for documentation
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:17>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by cristianocca):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:18>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by timgraham):
Tickets aren't an appropriate place to ask usage questions. Please use
[wiki:TicketClosingReasons/UseSupportChannels support channels] instead.
--
Ticket URL: <https://code.djangoproject.com/ticket/18616#comment:19>