Opened 3 days ago
Last modified 30 hours ago
#35792 new Cleanup/optimization
Enhance _get_group_permissions method of ModelBackend
Reported by: | Bona Fide IT GmbH | Owned by: | |
---|---|---|---|
Component: | contrib.auth | Version: | 4.2 |
Severity: | Normal | Keywords: | Backend |
Cc: | Bona Fide IT GmbH | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
We would like to suggest a change to the _get_group_permissions
method in the django ModelBackend
.
The current code gets the registered User
model and from it the groups
field to get the related query name to compose a filter query for the Permission
model at the end.
Current version of django/contrib/auth/backends.py
(https://github.com/django/django/blob/main/django/contrib/auth/backends.py#L61-L64) :
from django.contrib.auth import get_user_model from django.contrib.auth.models import Permission from django.db.models import Exists, OuterRef, Q UserModel = get_user_model() ... class ModelBackend(BaseBackend): ... 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}) ...
Since in this method both the group
field of the Permission
model and the reverse relation field groups
of the [custom] User
model are fixed anyway, we suggest the following change.
class ModelBackend(BaseBackend): ... def _get_group_permissions(self, user_obj): return Permission.objects.filter(group__in=user_obj.groups.all()) ...
We are not sure if this will fundamentally change the performance of the query, but it seems to us to be a simpler variant than going via _meta
and then constructing a query string that is fixed anyway.
A further argument from our side is that this would probably also make it possible to use a custom Group
model and to solve problems such as those mentioned here: https://github.com/django-guardian/django-guardian/pull/504#issuecomment-429810401 without introducing something like an AUTH_GROUP_MODEL
setting.
Attachments (1)
Change History (3)
by , 3 days ago
Attachment: | django.patch added |
---|
comment:1 by , 2 days ago
comment:2 by , 30 hours ago
We added a pull request on GitHub, https://github.com/django/django/pull/18633.
Thanks for the suggestion. It would be nice to submit your patch as a pull request on GitHub so that we may see the outcome of the change in the test suite.