Opened 6 years ago

Closed 3 months ago

#29748 closed New feature (wontfix)

Add AUTH_GROUP_MODEL setting to swap the group model

Reported by: damoncheng Owned by: Csirmaz Bendegúz
Component: contrib.auth Version: dev
Severity: Normal Keywords:
Cc: JM Barbier Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The information is not enough for the most project in django.contrib.auth.models.User and django.contrib.auth.models.Group. The project can only custom User and Team model.

The auth User can be changed in the django settings by AUTH_USER_MODEL. But the Group cannot.

Further, There are many group permissions problem presented when project use django permissions.

Firstly, In django.contrib.auth.ModelBackend, Django currently bind group permissions with django.contrib.auth.models.Group


def _get_group_permissions(self, user_obj):
    user_groups_field = get_user_model()._meta.get_field('groups')
    user_groups_query = 'group__%s' % user_groups_field.related_query_name()
    return Permission.objects.filter(**{user_groups_query: user_obj})

If the project want to relate the group permission to custom team. the project have to custom backend that extend django.contrib.auth.ModelBackend and modify group to team.

Secondly, the bad design limit the other app design, many app about django permission , only support group permission based on django.contrib.auth.models.Group, like django-guardian.

In the conversation with Adam Johnson(https://groups.google.com/forum/#!topic/django-developers/llQJZUKejXg). Admin suggestion me to contribute it or to use OneToOneField to solve the basic problem. But the OneToOneField is not elegant and it have problem in some scene. For example, A project is a cloud item, It use SCIM to manage user and team identities in cloud. it transfer resource from sender to many receiver.

Before the project use OneToOneField, The project only transfer user and team from sender to receiver. It spend 2 hour.

After the project use OneToOneField. The project need transfer user and team from sender to receiver, then create group and make team OneToOneField to Group in the receiver. It spend 6 hour.

it degrade the transform performence.

So please make me to support AUTH_GROUP_MODEL that optimize django group permission design

Change History (13)

comment:1 by Tim Graham, 6 years ago

Has patch: unset
Summary: Support AUTH_GROUP_MODEL to optimize django group permission designAdd AUTH_GROUP_MODEL setting to swap the group model
Triage Stage: UnreviewedSomeday/Maybe

It's unclear if the complexity is too much to implement this. See django-developers for the discussion.

comment:2 by Bernd Wechner, 3 years ago

See: https://github.com/shacker/django-todo/pull/130

If Django core developers express a willingness to see this coded, I'm willing to code it up and submit a PR.

It's not hard and is needed for consistency with the settings.AUTH_USER_MODEL setting.

It's not 100% trivial as it means we'd need to add not only add:

def get_group_model():
    """
    Return the Group model that is active in this project.
    """
    try:
        return django_apps.get_model(settings.AUTH_GROUP_MODEL, require_ready=False)
    except ValueError:
        raise ImproperlyConfigured("AUTH_GROUP_MODEL must be of the form 'app_label.model_name'")
    except LookupError:
        raise ImproperlyConfigured(
            "AUTH_GROUP_MODEL refers to model '%s' that has not been installed" % settings.AUTH_GROUP_MODEL
        )

around about here:

https://github.com/django/django/blob/4ff500f2948bfc332b3f4159021cad06e91943d3/django/contrib/auth/__init__.py#L155

but also we confront the fact that with configurable USER and GROUP models the relation between them is also perforce, configurable. We need only know that one exists (and can stipulate that it is to-many from USER to GROUP (by definition), and thus implement:

def user_group_field():
    find a field in get_user_model() that is a 2many relation to get_group_model() and return it

from that field the reverse field can also be fetched and we need:

def group_user_field():
    return the related_name for user_group_field()

So it's not nothing but it is rather trivial all the same, and I'm very to code it up, run the tests, PR it. But don't want to waste my time if it's not welcome. I could infer it's welcome from the triage three years ago to Someday / Maybe. But will fish here for some encouragement first.

Last edited 3 years ago by Bernd Wechner (previous) (diff)

in reply to:  2 ; comment:3 by Gian Luca Decurtins, 3 years ago

Replying to Bernd Wechner:

So it's not nothing but it is rather trivial all the same, and I'm very to code it up, run the tests, PR it. But don't want to waste my time if it's not welcome. I could infer it's welcome from the triage three years ago to Someday / Maybe. But will fish here for some encouragement first.

Definitely interested in such a feature - in my current project I now face a similar challenge. Create a Team model with a OneToOne field or wait for some elegant solution. Sadly I can't contribute to this one as I think this area is too risky for a beginner.

in reply to:  3 ; comment:4 by Bernd Wechner, 3 years ago

Definitely interested in such a feature - in my current project I now face a similar challenge. Create a Team model with a OneToOne field or wait for some elegant solution. Sadly I can't contribute to this one as I think this area is too risky for a beginner.

I'm happy to implement it and PR it. It's not a biggie. Just a waste of time if the PR is likely to bounce for unforeseen reasons, that is the whole idea meets with opposition at the triage level. Given this ticket exists and isn't closed as "won't do" that is a positive sign and your interest contributes to that, meaning I might risk doing it some time ;-).. It's probably one evenings work for me one day after the kids are in bed by time I've merged in the latest master, designed and implemented the best patch I can imagine and then crafted a PR with description and reference to this ticket.

comment:5 by Gian Luca Decurtins, 2 years ago

Who will take a decision on the PR? Is there anyone I could try to reach out to?

comment:6 by Gian Luca Decurtins, 2 years ago

Hi Bernd, do you think this could be something to be added in the upcoming Django 4.1 release?

comment:9 by Michael, 2 years ago

I was just about to open ticket for this exact same thing. The most common use case I have come across when searching for a solution is to add a description field to the Group model. There seems to be no complicated/hacky solution to this. Most solution end up creating custom migrations within:

site-packages/django/contrib/auth/migrations/

Which lives outside ones project, and can't be version controlled.

in reply to:  2 comment:10 by Michael, 2 years ago

So it's not nothing but it is rather trivial all the same, and I'm very to code it up, run the tests, PR it. But don't want to waste my time if it's not welcome. I could infer it's welcome from the triage three years ago to Someday / Maybe. But will fish here for some encouragement first.

Please submit the PR, I think this feature will add a lot of benefit to people who are in the same boat.

Just this single post on customizing django group has 11K views:
https://stackoverflow.com/questions/33068424/customize-django-group-model

in reply to:  4 comment:11 by Vasanth, 2 years ago

Replying to Bernd Wechner:

It's probably one evenings work for me one day after the kids are in bed by time I've merged in the latest master, designed and implemented the best patch I can imagine and then crafted a PR with description and reference to this ticket.

Hello there Bernd. If you have a branch or repo with changes feel free to share it here. I'll help with finishing up the PR as much as I can to get this ticket into prod within the next couple of releases.

Last edited 2 years ago by Vasanth (previous) (diff)

comment:12 by JM Barbier, 2 years ago

Cc: JM Barbier added

comment:13 by Hossein , 6 months ago

defining AUTH_GROUP_MODEL in settings was a good solution, but apparently it needs some work to get prioritized.

developers face problem when making a custom group model, as they want to add fields and methods to Group model

https://stackoverflow.com/questions/2181039/how-do-i-extend-the-django-group-model

Meanwhile, I suggest to make Group model get inherited from AbstractGroup. In this way, one can make a CustomGroup (AbstarctGroup) in their code.

I made a PR to handel this:
https://github.com/django/django/pull/17820

Last edited 6 months ago by Hossein (previous) (diff)

comment:14 by Csirmaz Bendegúz, 4 months ago

Has patch: set
Owner: changed from damoncheng to Csirmaz Bendegúz

in reply to:  14 comment:15 by Natalia Bidart, 3 months ago

Resolution: wontfix
Status: assignedclosed
Triage Stage: Someday/MaybeUnreviewed

Replying to Csirmaz Bendegúz:

I have submitted a patch: https://github.com/django/django/pull/18053

As said in the forum post, thank you so much Csirmaz for your work on this. Nevertheless, following the forum and previous conversations, I'll be closing this as wontfix.

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