[Django] #25057: Permission for view (CRUD) and correct handling of inactive users for business applications

2 views
Skip to first unread message

Django

unread,
Jul 3, 2015, 3:43:57 AM7/3/15
to django-...@googlegroups.com
#25057: Permission for view (CRUD) and correct handling of inactive users for
business applications
--------------------------------------+--------------------
Reporter: djbaldey | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
Our framework is beautiful, it is convenient to make sites, but not
business applications.
You have to write a lot of code and/or wrappers to do elementary things,
in order not to lose performance.
The most striking problem: when you need in a single view to decide what
data to display, depending on which groups the user belongs to.
For example, a simple task:


{{{
def dash(request):
ctx = {}
user = request.user

if not user.has_module_perms('myadminapp'):
return HttpResponseForbidden()

if user.has_perm('auth.view_group'):
ctx['groups'] = Group.objects.all()
if user.has_perm('auth.view_permission'):
ctx['permissions'] = Permissions.objects.all()
if user.has_perm('auth.view_user'):
ctx['users'] = User.objects.all()

# Also the external applications to which you cannot add custom
permissions:
if user.has_perm('partner.view_supplier'):
ctx['suppliers'] = Supplier.objects.all()
if user.has_perm('partner.view_org'):
ctx['orgs'] = Org.objects.all()
if user.has_perm('partner.view_address'):
ctx['addresses'] = Address.objects.all()
if user.has_perm('order.view_summaryorder'):
ctx['summary_orders'] = SummaryOrder.objects.all()
if user.has_perm('order.view_customorder'):
ctx['custom_orders'] = CustomOrder.objects.all()
if user.has_perm('order.view_document'):
ctx['docs'] = Document.objects.all()
...
# 0 additional requests in database
}}}


Today turns into:

{{{
def dash(request):
ctx = {}
user = request.user

if not user.has_module_perms('myadminapp') or not user.is_active:
return HttpResponseForbidden()

if user.groups.filter(name='Has view groups').count():
ctx['groups'] = Group.objects.all()
if user.groups.filter(name='Has view permissions').count():
ctx['permissions'] = Permissions.objects.all()
if user.groups.filter(name='Has view users').count():
ctx['users'] = User.objects.all()

# Also the external applications to which you cannot add custom
permissions:
if user.groups.filter(name='Has view suppliers').count():
ctx['suppliers'] = Supplier.objects.all()
if user.groups.filter(name='Has view orgs').count():
ctx['orgs'] = Org.objects.all()
if user.groups.filter(name='Has view addresses').count():
ctx['addresses'] = Address.objects.all()
if user.groups.filter(name='Has view summary orders').count():
ctx['summary_orders'] = SummaryOrder.objects.all()
if user.groups.filter(name='Has view custom orders').count():
ctx['custom_orders'] = CustomOrder.objects.all()
if user.groups.filter(name='Has view documents').count():
ctx['docs'] = Document.objects.all()
...
# +9 additional requests in database
}}}

Or add to the database by hand after installing the apps.
But if someone of the admins accidentally rename a group? The horror!
For large and complex applications permission to view inside framework the
only decent solution.

While all of this madness can be reduced to two simple solutions.

1. Set four default permissions in
db.models.options.Options.default_permissions = ('view', 'add', 'change',
'delete'). Also, would the names of the permission form on the language
set in settings.LANGUAGE_CODE. It's a matter of principle any
"perfectionist".
2. Inactive user should not have any permissions. Now has. For exceptional
cases of inactive user should be a special parameter for the function that
changes its behavior.

I have three patches for master branch and and can do for stable/1.8.x. I
think these changes will not affect existing projects, because the
permissions have already been recorded in the database during the
migration and shall not affect the administrative interface.

Each of the patches tested.

--
Ticket URL: <https://code.djangoproject.com/ticket/25057>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jul 3, 2015, 3:45:15 AM7/3/15
to django-...@googlegroups.com
#25057: Permission for view (CRUD) and correct handling of inactive users for
business applications
----------------------------------+----------------------------

Reporter: djbaldey | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Easy pickings: 0
UI/UX: 0 |
----------------------------------+----------------------------
Changes (by djbaldey):

* Attachment "25057_inactive_user.diff" added.

Django

unread,
Jul 3, 2015, 3:45:36 AM7/3/15
to django-...@googlegroups.com
#25057: Permission for view (CRUD) and correct handling of inactive users for
business applications
----------------------------------+----------------------------

Reporter: djbaldey | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Easy pickings: 0
UI/UX: 0 |
----------------------------------+----------------------------
Changes (by djbaldey):

* Attachment "25057_permission_for_view_with_locales.diff" added.

Django

unread,
Jul 3, 2015, 3:45:53 AM7/3/15
to django-...@googlegroups.com
#25057: Permission for view (CRUD) and correct handling of inactive users for
business applications
----------------------------------+----------------------------

Reporter: djbaldey | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Easy pickings: 0
UI/UX: 0 |
----------------------------------+----------------------------
Changes (by djbaldey):

* Attachment "25057_permission_for_view_without_locales.diff" added.

Django

unread,
Jul 3, 2015, 3:48:29 AM7/3/15
to django-...@googlegroups.com
#25057: Permission for view (CRUD) and correct handling of inactive users for
business applications
-------------------------------------+-------------------------------------
Reporter: djbaldey | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by djbaldey):

* needs_docs: => 0
* needs_tests: => 0
* needs_better_patch: => 0


Old description:

New description:


Today turns into:

P.S.: Please forgive me for my English. I write through the translator.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/25057#comment:1>

Django

unread,
Jul 3, 2015, 7:49:13 AM7/3/15
to django-...@googlegroups.com
#25057: Permission for view (CRUD) and correct handling of inactive users for
business applications
-------------------------------------+-------------------------------------
Reporter: djbaldey | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: contrib.auth | Version: master
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):

* status: new => closed
* resolution: => wontfix


Comment:

Thanks for the suggestions.

The request for a "view" permission is a duplicate of #820 which was
closed as "won't fix". If you want to reopen the discussion, please write
to the DevelopersMailingList.

I don't find your proposal for permissions checking compelling because
these ideas can be implemented with a custom user model. That seems more
elegant that requiring an application to pass `check_active=False`
everywhere it does permission checks. Also, the default `ModelBackend`
[https://github.com/django/django/blob/a570701e02e0bc09d977c8ae0b6ee987a1190039/django/contrib/auth/backends.py#L33-L40
doesn't return any permissions for inactive users] so that would need to
be overridden as well.

For future reference, please keep a ticket focused on one issue.

--
Ticket URL: <https://code.djangoproject.com/ticket/25057#comment:2>

Reply all
Reply to author
Forward
0 new messages