Code

Opened 7 years ago

Closed 7 years ago

#3029 closed defect (fixed)

[patch] Erroneous values stored in contrib.auth fields: date_joined, last_login

Reported by: Tim Goh Owned by: nobody
Component: Contrib apps Version: master
Severity: normal Keywords:
Cc: gary.wilson@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

Problem:

The date_joined and last_login fields in the contrib.auth package are both declared as "default=models.LazyDate()", which results in the following incorrect behavior:

  1. date_joined and last_login are updated to the current date/time whenever the User model is saved.
  2. last_login is updated when the user logs into the admin interface, but any application front-ends hooking on to contrib.auth will have to update it themselves upon authenticating. This is not what the name "last_login" implies.

Patch description:

(patched against r4077)

The attached patch defines date_joined and last_login as auto_now_add=True, so they are timestamped upon creation of the user object. Subsequently, upon successful authentication, last_login is updated with the current time.

Attachments (2)

contrib_auth_date_fields_errors.patch (2.0 KB) - added by Tim Goh <timgoh@…> 7 years ago.
contrib_auth_date_fields_errors_v2.patch (2.0 KB) - added by Tim Goh <timgoh@…> 7 years ago.
against r4085

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by Tim Goh <timgoh@…>

comment:1 Changed 7 years ago by SmileyChris

You can't assume that user will have a last_login property - you're assuming it will be django.auth.models.User, when it could be any model passed from an alternate auth backend.

Changed 7 years ago by Tim Goh <timgoh@…>

against r4085

comment:2 Changed 7 years ago by Tim Goh <timgoh@…>

You're right. The last_login update code should go in ModelBackend instead. Updated diff attached.

comment:3 Changed 7 years ago by Gary Wilson <gary.wilson@…>

Updating last_login in the backend's authenticate method is not quite right either. The AuthenticationForm manipulator can still raise errors after authenticate() has been called, in which case last_login gets updated when it should not.

comment:4 Changed 7 years ago by Gary Wilson <gary.wilson@…>

  • Cc gary.wilson@… added

comment:5 Changed 7 years ago by Simon G. <dev@…>

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

comment:6 Changed 7 years ago by ubernostrum

  • Resolution set to fixed
  • Status changed from new to closed

LazyDate is gone and those fields now default to datetime.datetime.now which only happens on the initial save, so this is fixed.

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.