Opened 3 years ago

Closed 2 years ago

#26823 closed Bug (fixed)

auth signal receiver update_last_login not compatible with custom user models that have no last_login field

Reported by: Harry Percival Owned by: nobody
Component: contrib.auth Version: 1.9
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In my experiments with building a minimal custom user model, I've managed to build one that has just one field, email. Which I think is brilliant of course. One feature in the auth module is stopping it from working though, the update_last_login signal receiver from contrib.auth.models, which is wired up in auth.signals.user_logged_in.

My workaround is to do a

from django.contrib import auth
auth.signals.user_logged_in.disconnect(auth.models.update_last_login)

But we could make the signal handler more resilient. Something like this?

def update_last_login(sender, user, **kwargs):
    if not hasattr(user, 'last_login'):
        return

PR to follow...

Change History (9)

comment:1 Changed 3 years ago by Harry Percival

Component: Uncategorizedcontrib.auth
Type: UncategorizedBug

comment:2 Changed 3 years ago by Harry Percival

Has patch: set

comment:4 Changed 3 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

An alternative implementation I thought of would be to introspect the user model and see if it has a last_login field and avoid connecting the signal if not. I'm not sure it's feasible with user_logged_in.connect(update_last_login) at the module level since get_user_model() can't be called there, but maybe if the signal connecting happened in AuthConfig.ready() that could work?

comment:5 Changed 3 years ago by Claude Paroz

Patch needs improvement: set

+1 to connect the signal in ready()

comment:6 Changed 3 years ago by Florian Apolloner

Signal connection in ready() should not be done lightly, this method should be idempotent -- either just import a module from there or make sure to set dispatch_uid

comment:7 Changed 2 years ago by Linus Lewandowski

comment:8 Changed 2 years ago by Tim Graham

Patch needs improvement: unset

comment:9 Changed 2 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In eedc88bd:

Fixed #26823 -- Prevented update_last_login signal receiver from crashing if User model doesn't have last_login field.

Note: See TracTickets for help on using tickets.
Back to Top