Opened 4 years ago

Closed 4 years ago

#29119 closed Uncategorized (invalid)

Request for discussion / maybe features

Reported by: Stephan Doliov Owned by: nobody
Component: contrib.auth Version: 2.0
Severity: Normal Keywords: auth, admin, last_login, is_superuser, is_staff
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I would like to do some retooling of the django.contrib.auth app, in particular the user model and its swapability. I hope to generate some discussion with this ticket, it would not, IMO, make sense to consider this a single feature request. Rather, if there is support for my thoughts, we can figure out how to break up the work to make it go.

My overall goals are:
Kill both the conception of is_superuser and is_staff as direct attributes of User
Kill the last_login as a stateless field in the default User model
Kill the username field (using email as primary identifer is fine for my purposes and I don't like towing around deadspace in the backing database)
To only associate information with an authenticated (or authenticable) user in a stateless way in which it is absolutely necessary
that would be
identifier (e.g. email)
password
and I see the convenience of is_active flag but not sure if it is necessary (it could be derived by no permissions be associated with a given user)

then a login history table could take place of the the last login field (or a more generic event table, where one of the events was a 'user_login'
is_staff and is_superuser should be up to the app builder to decide. maybe there are some default permissions that are the equivalent, but those are far more things that are like roles (or Groups in django speak) than a user attribute.

Thoughts?

Face value, this is easy enough, I can create a swappable user model (following the pattern of AbstractBaseUser, AbstractUser and User) and have tinker with the last_login, is_superuser and username fields as desired.

But seeing that auth and admin apps are somewhat tightly coupled, it also means I have to set the last_login, is_superuser, and last_login attributes as properties in my concrete user class (and add getters/setters as desired, or just return False or None respectively).

So I do that and then the rubber hits the road because of:
auth/apps.py

from django.apps import AppConfig
from django.core import checks
from django.db.models.signals import post_migrate
from django.utils.translation import gettext_lazy as _

from . import get_user_model
from .checks import check_models_permissions, check_user_model
from .management import create_permissions
from .signals import user_logged_in


class AuthConfig(AppConfig):
    name = 'auth'
    label = 'auth'
    verbose_name = _("Authentication and Authorization")

    def ready(self):
        post_migrate.connect(
            create_permissions,
            dispatch_uid="auth.management.create_permissions"
        )
        if hasattr(get_user_model(), 'last_login'):
            from .models import update_last_login
            user_logged_in.connect(update_last_login, dispatch_uid='update_last_login')
        checks.register(check_user_model, checks.Tags.models)
        checks.register(check_models_permissions, checks.Tags.models)

and the update_last_login method is defined in auth.models as

def update_last_login(sender, user, **kwargs):
    """
    A signal receiver which updates the last_login date for
    the user logging in.
    """
    user.last_login = timezone.now()
    user.save(update_fields=['last_login'])

This of course generates a failure because the last_login field no longer exists in my User Model (I prefer to keep track of the entire history of login events elsewhere, as my app has a need to be able to provide the audit trail of all logins by a user).

As best I understand it, I cannot just override the AuthConfig definition -- that is, I can wholesale copy the auth app into my app and then update the config, removing or modifying the offending line. But that seems ugly to me.

It seems to me that if User models are truly swappable, then I wouldn't have to carry the last_login field and I wouldn't have to worry about django.contrib.auth hardcoding it as if it always existed.

A first order fix for this would be to test if the current user model has a last_login model field and only issuing the save() call with the last_login attribute if it exists.

Is_staff and is_superuser are not beset by the same problem, it is just that they are coupled into the logic of admin.
that would be in:
django/contrib/admin/forms.py
django/contrib/admin/sites.py
django/contrib/admin/views/decorators.py

As the auth and admin apps currently are (useful and robust for sure) I wouldn't want to undo this, but I am wondering if the very way in which Permissions are done in django should also be swappable?

Thanks for reading this long post, I look forward to some feedback.

Change History (1)

comment:1 Changed 4 years ago by Tim Graham

Resolution: invalid
Status: newclosed

Please use the DevelopersMailingList for discussion and create tickets if a consensus emerges.

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