Thoughts re looser coupling of django.contrib.auth and django.contrib admin

[Stephan Doliov]

Hi,

I would like to solicit some feedback regarding some existing tight coupling between django.contrib.auth and django.contrib admin.

The existing, and nominally swappable user model that ships with django assumes, for the out-of-the-box application benefit of django.contrib.admin, that there are 3 differentiable fields that should ship with any swappable user model that wishes to be swappable:

is_staff

is_superuser

and

is_active

I would like to propose that at least the "is_staff" and the "is_superuser" attributes that are part of the default user model become fallbacks to a more generic permissions (maybe even swappable permissions???) approach.

What I am envisioning is  that whereever there are checks for is_superuser, is_staff and is_active that they be wrapped in a way that if the custom user model did not offer these attributes, that perhaps some "pluggable permissioning" take their place. I enumerate the existing admin dependencies in this post, the is_superuser is relevant prolly in a different thread as it only touches the default user model (and not admin directly)

For example: from django/contrib/admin/views/decorators.py

from django.contrib.auth import REDIRECT_FIELD_NAME
from django.contrib.auth.decorators import user_passes_test




def staff_member_required(view_func=None, redirect_field_name=REDIRECT_FIELD_NAME,
                          login_url='admin:login'):
    """
    Decorator for views that checks that the user is logged in and is a staff
    member, redirecting to the login page if necessary.
    """
    actual_decorator = user_passes_test(
        lambda u: u.is_active and u.is_staff,
        login_url=login_url,
        redirect_field_name=redirect_field_name
    )
    if view_func:
        return actual_decorator(view_func)
    return actual_decorator

itself could wrap u.is_active and u.is_staff.

or, IMO, better yet, the actual decorator could be overriden by the custom user model:

e.g. the user model could:

def user_is_staff_test(self):
    if self.has_permissions(['list of perms']):
        return True
    else:
        return False

and

def user_is_superuser_test(self):
    if self.has_permissions(['list of other perms']):
        return True
    else:
        return False

and then existing decorator could be modifed to to fetch the user_passes_test method and revert to a default if it doesn't exist.

Similar mods would be required for the Admin Site class django/contrib/admin/sites.py

    def has_permission(self, request):
        """
        Return True if the given HttpRequest has permission to view
        *at least one* page in the admin site.
        """
        return request.user.is_active and request.user.is_staff

And the AdminAuthenticationForm (django/contrib/admin/forms.py):

class AdminAuthenticationForm(AuthenticationForm):
    """
    A custom authentication form used in the admin app.
    """
    error_messages = {
        'invalid_login': _(
            "Please enter the correct %(username)s and password for a staff "
            "account. Note that both fields may be case-sensitive."
        ),      
    }
    required_css_class = 'required'


    def confirm_login_allowed(self, user):
        if not user.is_active or not user.is_staff:
            raise forms.ValidationError(
                self.error_messages['invalid_login'],
                code='invalid_login',
                params={'username': self.username_field.verbose_name}
            )

I would love to hear people's thoughts on this. I am targeting an architecture where the user model as simply two meaningful columns (I don't care too much about the surrogate "id" PK), email & password.

The other attributes would be stored elsewhere. I can see carrying a convenience field of "is_active" in this table but that too could be managed in other ways (user inactivation could be done by migrating a db row to an "inactive users" table, e.g.

The way I think about last login is to provide either a user_login_history table or a more generic event table where one of the possible events would be 'user_login'.

I appreciate others' thoughts. And special thanks to the django folks who just, with release 2.02, made the last_login attribute of a custom user model no longer necessary! I had been riding with a fork of auth_user to get around that for a bit.

steve

[Collin Anderson]

Hi Steve,

If it helps at least in the short-term, those fields currently don't need to be actual database columns. I have a custom user that has these properties/methods to make work with the admin.

Collin

    @property

    def is_anonymous(self):

        return not self.is_authenticated

    @property

    def is_authenticated(self):

        return bool(self.pk)

    @property

    def is_active(self):

        return bool(self.pk)

    @property

    def is_superuser(self):

        # run some code to figure out if they have "admin" custom permission or not

        return permission_check is True

    @property

    def is_staff(self):

        return self.is_superuser

    def has_module_perms(self, app_label):

        return self.is_superuser

    def has_perm(self, perm):

        return self.is_superuser

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/84f1bc04-e6a4-4832-ab1c-1e6f4d5db62b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Stephan Doliov]

Yes, Thank you Colin, I do the same thing already as a work around, I was just thinking it would be nice if pluggable/swappable user models didn't have to do this workaround out of the box because this too, is a bit coarse to my own taste: ultimately, a "superuser" is needed. My first leanings on "superuser" are that it is different from is_staff in that it has the right to Create, Change and Delete permisions (CRUD) whereas is_staff might have selective ability to create/change/delete permissions. I am prosecuting a design point, which is if something is to be swappable, it should have the option of doing this either via settings or as a more generic has_admin_permissions hook/callback.

    @property
    def is_superuser(self):
        # run some code to figure out if they have "admin" custom permission or not
        return permission_check is True

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.