Opened 9 months ago

Last modified 9 months ago

#23155 new New feature

Add request attr to user_login_failed signal

Reported by: anonymous Owned by: nobody
Component: contrib.auth Version: master
Severity: Normal Keywords:
Cc: brian@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

It would be convenient to request instance in the signal user_login_failed.
This will get the users data and make limitations, such as IP address.
What do you think?

Change History (8)

comment:1 Changed 9 months ago by brian

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

The user_logged_in and user_logged_out signals both have the request parameter, so having request for user_login_failed would make sense IMHO.

comment:2 Changed 9 months ago by brian

  • Cc brian@… added

Implementing this looks not-trivial.

The authenticate() method at django.contrib.auth takes username and password parameters, but does not take a request parameter.

https://docs.djangoproject.com/en/dev/topics/auth/default/#django.contrib.auth.authenticate

This function sends the user_login_failed signal. Fixing this may require changing the API of authenticate().

comment:3 Changed 9 months ago by anonymous

Adding optional request attr to authenticate() function and according to Authentication backends would give more options and flexibility.
I believe that the framework should be improved and made ​​more flexible.

comment:4 Changed 9 months ago by aaugustin

Simply passing request to authenticate is backwards incompatible: every third-party authentication backend will be silently skipped because the arguments don't match.

comment:5 Changed 9 months ago by tomc

  • Version changed from 1.7-rc-2 to master

comment:6 Changed 9 months ago by timo

Aymeric, I think you mixed django.contrib.auth.authenticate() and the authenticate() method of each backend. We wouldn't have to pass request to the auth backends because the signal is sent in the former so it might be feasible.

comment:7 Changed 9 months ago by brian

The caller of django.contrib.auth.authenticate() would have to be updated to take this new parameter. That could be code outside Django.

We could make it an optional parameter, however then you don't actually get the benefits unless it is called correctly. New code that uses the new request parameter will get confused if used with older Django versions as the request parameter will become part of **credentials, and this will get passed through to every backend, like it or not.

The current code is:

  def authenticate(**credentials):                                                 
      for backend in get_backends():                                               
          try:                                                                     
              user = backend.authenticate(**credentials)                           
          [...]

comment:8 Changed 9 months ago by brian

Also **credentials is used to generate the user_login_failed signal.

Last edited 9 months ago by brian (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top