-
Notifications
You must be signed in to change notification settings - Fork 12
Description
Problem
When checking permissions that can be granted either dynamically (via django-rules) or via DB assignment (user_permissions/groups), we need a dual check pattern everywhere:
user.has_perm(perm, obj) or user.has_perm(perm)This is because Django's ModelBackend.has_perm() returns False whenever obj is passed — it skips DB-assigned permission checks entirely. So has_perm(perm, obj) only evaluates django-rules predicates, and has_perm(perm) only checks DB-assigned permissions. It doesn't look like that is changing anytime soon.
This dual check is currently used in:
Detail._has_perm()inviews/detail.pyOrganizationPermissionMixin.has_permission()inmixins.pyReviewProfileChange.post()inviews/profile.py
Proposed solution
Create a custom authentication backend that doesn't skip DB permissions when obj is passed:
from django.contrib.auth.backends import ModelBackend
class ObjectAwareModelBackend(ModelBackend):
def has_perm(self, user_obj, perm, obj=None):
# Always check DB-assigned perms, even with an object
return super().has_perm(user_obj, perm, obj=None)Replace django.contrib.auth.backends.ModelBackend with this in AUTHENTICATION_BACKENDS.
With this backend, has_perm(perm, obj) would check both:
- Rules backend: evaluates the predicate with the object
- Custom backend: checks DB-assigned permissions (ignoring obj)
This would eliminate the dual check pattern entirely. All call sites could use a single has_perm(perm, obj) call, and {% has_perm %} template tags would work correctly again without needing to pass context variables from views.
Tradeoff
This changes behavior globally — every has_perm(perm, obj) call in the project would also check DB-assigned perms. But we'd benefit from simpler template rendering contexts and it seems like this is how our permission checking ought to work.