Code

Opened 3 years ago

Closed 3 years ago

#14981 closed (wontfix)

Small enhancement to User.last_login timezone handling (version 1.3.0 beta 1)

Reported by: jamercee Owned by: nobody
Component: contrib.auth Version: 1.3-alpha
Severity: Keywords: TIME_ZONE last_login UTC
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Attached is a small patch to enhance the timezone handling of django.contrib.auth to ensure that the last_login and date_joined datetime fields are stored in UTC format.

I recognize these patches make an assumption about timezone that may represent a breaking change. But after reviewing the source code, it would appear the User.last_login and User.date_joined fields were never intended to store timezone. Instead, they relied on timezone information provided by the underlying OS. This introduced the possibility for subtle development --> production system differences.

For example, we have been performing development on Windows, and host our production code on FreeBSD 8.x. Under Windows, even with settings.TIME_ZONE='UTC', the returned value from datetime.datetime.now() is always localtime. But on FreeBSD it results in the correct UTC results. The difference meant that code intended to display User.last_login on Windows was different from the coded needed to display these same values on FreeBSD. This subtle difference created a more complex debug environment.

The solution in the patch is to switch to only call datetime.datetime.utcnow() in the django.contrib.auth.models.py code.

I should mention that prior to submitting the patch, we reviewed the possibility of rolling the functionality into user profiles, and also the possibility of solving the issue by trying to somehow intercept the signal of the login() event. But each of these approaches seemed to come up short to what was essentially a simple solution.

We acknowledge this may not be the "right" way to solve this issue. But it seemed like we should start with submitting a patch, and allow the Django team to make the final decision. If you'd prefer to see a different approach to solving this in a more "django appropriate" way -- just let me know. We're new to the django world, but it's a fantatic system, and we'd be pleased to follow experienced guidance.


Code Sample:

Once this patch is applied, converting from UTC to local-timezone is a trivial affair using pytz. Here's an example of how to display last_login from a session that has already been authenticated.

from django.contrib import auth
from django.http    import HttpResponse
from django.contrib.auth.decorators import login_required

import pytz

@login_required
def show_last_login(request):
    ll_utc = pytz.utc.localize(request.user.last_login)
    ll_loc = ll_utc.astimezone(pytz.timezone(settings.TIME_ZONE))
    html = "<html><body>last login {0}</body></html>".format(ll_loc)
    return HttpResponse(html)

Attachments (1)

models.py.diff (1.9 KB) - added by jamercee 3 years ago.
Patched django.contrib.auth.models.py code to store dates in UTC format

Download all attachments as: .zip

Change History (2)

Changed 3 years ago by jamercee

Patched django.contrib.auth.models.py code to store dates in UTC format

comment:1 Changed 3 years ago by russellm

  • milestone 1.3 deleted
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to wontfix
  • Status changed from new to closed

If we're going to fix timezone handling, we're not going to do it just for the auth system. Django's handling of timezones is less than ideal, and needs to be addressed system-wide. Ticket #2626 is tracking this problem.

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.