Staff (not superusers) should not manage perms of Users

[Vlada Macek]

In our project we want to let some colleagues to see and modify several tables including the Users table. Nevertheless I don't want the colleague to be able to elevate his or anybody else's privileges.

May I submit little change of UserAdmin similar to the following for consideration?

def get_readonly_fields(self, request, obj=None):
    rof = super(UserAdmin, self).get_readonly_fields(request, obj)
    if not request.user.is_superuser:
        rof += ('is_staff', 'is_superuser', 'groups', 'user_permissions')
    return rof

I rather doubt there is a use-case for current behaviour: Once the access to Users table is given, one can do anything.

In case the behavior change will get rejected, how about to add it as a tip in the doc?

[Loic Bistuer]

What about password and email both of which could be used to gain access to a superuser account; in my opinion this use-case is better served by a custom UserAdmin in your project where you whitelist the few fields that should be editable.

I tentatively accept this as a documentation issue, we could warn about the consequences of giving edit permissions to the user model.

[Vlada Macek]

It was only a sketch from me, but thanks for the security audit. :-) That's something not immediately apparent.

In my project I also add this to prevent staff user from editing other users who possibly have any permission:

def has_change_permission(self, request, obj=None):
    has = super(MyUserAdmin, self).has_change_permission(request, obj)
    if obj and not request.user.is_superuser:
        if obj != request.user:
            if obj.is_superuser or obj.groups.exists() or obj.user_permissions.exists():
                has = False

    return has

Indeed, while this depends on how particular project manages users and their perms, I still humbly think there's an idea in it worth spreading.

[Collin Anderson]

I usually have this worry too about staff members being able to change their own permissions. I usually will create a separate UserAdmin (and corresponding proxy model) for the staff users to edit the fields they need to edit. I like the readonly fields idea though.

I would certainly be in favor of a good, secure example in the documentation.

[Chris Foresman]

So is this best resolved by a change in behavior, or a documentation change?

[Collin Anderson]

I think if there's a really good, secure behavior change, we should do that, otherwise a documentation change.

[Ken Lewerentz]

I think explicit is always better than implicit and as such suggest that only superusers should be able to elevate other user's permissions unless admin is configured otherwise.

But most important is to document this, even in the tutorial for new users, as this is something I find people are generally overlooking.

[Remco]

Hi, I'm a new contributor. I would like to work on this today as part of the django sprint.

[Tim Graham <timograham@…>]

In f6b09a7f:

Refs #23559 -- warned about consequences of letting users edit User model in admin.

[Tim Graham <timograham@…>]

In 96bbade:

[1.7.x] Refs #23559 -- warned about consequences of letting users edit User model in admin.

Backport of f6b09a7f85c3b67b2011553838b079788c413432 from master

[Tim Graham <timograham@…>]

In 6f555e5:

[1.8.x] Refs #23559 -- warned about consequences of letting users edit User model in admin.

Backport of f6b09a7f85c3b67b2011553838b079788c413432 from master

[Tim Graham]

Updating ticket to contrib.auth component for investigation about making a change to the admin.