Opened 6 years ago
Closed 9 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 , 6 years ago
Has patch: | unset |
---|---|
Summary: | Support AUTH_GROUP_MODEL to optimize django group permission design → Add AUTH_GROUP_MODEL setting to swap the group model |
Triage Stage: | Unreviewed → Someday/Maybe |
follow-ups: 3 10 comment:2 by , 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:
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.
follow-up: 4 comment:3 by , 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.
follow-up: 11 comment:4 by , 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 , 3 years ago
Who will take a decision on the PR? Is there anyone I could try to reach out to?
comment:6 by , 3 years ago
Hi Bernd, do you think this could be something to be added in the upcoming Django 4.1 release?
comment:9 by , 3 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.
comment:10 by , 3 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
comment:11 by , 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.
comment:12 by , 2 years ago
Cc: | added |
---|
comment:13 by , 11 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
follow-up: 15 comment:14 by , 9 months ago
Has patch: | set |
---|---|
Owner: | changed from | to
I have submitted a patch: https://github.com/django/django/pull/18053
comment:15 by , 9 months ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
Triage Stage: | Someday/Maybe → Unreviewed |
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.
It's unclear if the complexity is too much to implement this. See django-developers for the discussion.