Code

Opened 8 months ago

Closed 8 months ago

#20921 closed New feature (wontfix)

Creating a decorator any_permission_required

Reported by: fabiobatalha@… Owned by: fabiobatalha
Component: contrib.auth Version: master
Severity: Normal Keywords: auth perms required_permission has_perms
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When decorating a view with required_permission the developer can authorize a user to execute a view only if the user have all the perms informed is the decorator.

In some situations it is necessary to allow the user to execute a view if the user have at least one of the allowed permissions. It could happen when some authorizations are controlled in the template level.

For example:

views.py
@any_permission_required(['journalmanager.list_journal', 'journalmanager.list_editor_journal'], login_url=AUTHZ_REDIRECT_URL)
def dash_journal(request, journal_id=None):

journal = get_object_or_404(models.Journal, id=journal_id)
return render_to_response('journalmanager/journal_dash.html', {

'journal': journal,
}, context_instance=RequestContext(request))

template:

{% if perms.journalmanager.list_journal %}

INTERFACE FEATURES FOR USERS WITH SIMPLE PERMS

{% endif %}
{% if perms.journalmanager.list_editor_journal %}

INTERFACE FEATURES FOR USERS WITH EDITORS PERMS

{% endif %}

Attachments (0)

Change History (4)

comment:1 Changed 8 months ago by fabiobatalha@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Suggested solution:

Add this method at django.auth.contrib.models.User

def has_any_perms(self, perm_list, obj=None):

"""
Returns True if the user has any of the specified permissions. If
object is passed, it checks if the user has at least one of the required
perms for thisobject.
"""
for perm in perm_list:

if self.has_perm(perm, obj):

return True

return False

Add this method at django.contrib.auth.decorators

def any_permission_required(perm, login_url=None, raise_exception=False):

"""
Decorator for views that checks whether a user has any particular permission
enabled, redirecting to the log-in page if neccesary.
If the raise_exception parameter is given the PermissionDenied exception
is raised.
"""
def check_perms(user):

if not isinstance(perm, (list, tuple)):

perms = (perm, )

else:

perms = perm

# First check if the user has the permission (even anon users)
if user.has_any_perms(perms):

return True

# In case the 403 handler should be called raise the exception
if raise_exception:

raise PermissionDenied

# As the last resort, show the login form
return False

return user_passes_test(check_perms, login_url=login_url)

comment:2 Changed 8 months ago by wim@…

  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.5 to master

Would be handy. Flask has one as well.

If you are willing and like to contribute, could you provide a patch, or even better: a pull request containing your change? Thanks!

comment:3 Changed 8 months ago by aaugustin

I'm not sure this is a good idea. This API naturally leads to designs with badly-defined access control, which is dangerously close to a security vulnerability.

In fact, "controlling authorizations at the template level" is akin to client-side access control. If you don't have two paths in your view, one for users with permission list_journal and the other for list_editors_journal, nothing prevents "simple" users from performing "editor" actions by crafting requests. If you have to separate paths, then why not just make two views? Finally, if you have lots of logic in common between the two views, you can factor it out by writing two class based views, where one inherits the other.

And if having one permission or the other only matters for display, not for access control, then the canonical way to implement this in Django is:

  • to give editors both list_journal and list_editors_journal
  • to use permission_required(journalmanager.list_journal) on the view
  • to use {% if perms.journalmanager.list_editor_journal %} ... {% else %} ... {% endif %} in the template.

I'm -0; if another core dev wants it, why not; but the use case shown above doesn't convince me and this new API may prove harmful to the security of Django websites.

comment:4 Changed 8 months ago by timo

  • Resolution set to wontfix
  • Status changed from new to closed

Agreed, I don't see a particular need to include this with Django. It can always be implemented in your project if you need it.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.