From 7bfc81b21af65c91dcbd9d33deae2f020d8bbbee Mon Sep 17 00:00:00 2001 From: Jessica Tallon Date: Fri, 25 Jul 2014 18:58:57 +0100 Subject: [PATCH] Fix #923 - add allow_admin to user_has_privilege decorator --- mediagoblin/db/models.py | 29 ++++++++++--------- mediagoblin/decorators.py | 12 +++++--- mediagoblin/templates/mediagoblin/base.html | 6 ++-- .../mediagoblin/moderation/user.html | 4 +-- mediagoblin/tests/test_modelmethods.py | 17 +++++------ 5 files changed, 35 insertions(+), 33 deletions(-) diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index c2d101ac..c6424e71 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -106,25 +106,26 @@ class User(Base, UserMixin): super(User, self).delete(**kwargs) _log.info('Deleted user "{0}" account'.format(self.username)) - def has_privilege(self,*priv_names): + def has_privilege(self, privilege, allow_admin=True): """ This method checks to make sure a user has all the correct privileges to access a piece of content. - :param priv_names A variable number of unicode objects which rep- - -resent the different privileges which may give - the user access to this content. If you pass - multiple arguments, the user will be granted - access if they have ANY of the privileges - passed. + :param privilege A unicode object which represent the different + privileges which may give the user access to + content. + + :param allow_admin If this is set to True the then if the user is + an admin, then this will always return True + even if the user hasn't been given the + privilege. (defaults to True) """ - if len(priv_names) == 1: - priv = Privilege.query.filter( - Privilege.privilege_name==priv_names[0]).one() - return (priv in self.all_privileges) - elif len(priv_names) > 1: - return self.has_privilege(priv_names[0]) or \ - self.has_privilege(*priv_names[1:]) + priv = Privilege.query.filter_by(privilege_name=privilege).one() + if priv in self.all_privileges: + return True + elif allow_admin and self.has_privilege(u'admin', allow_admin=False): + return True + return False def is_banned(self): diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 90edf96b..5bf60048 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -74,7 +74,7 @@ def require_active_login(controller): return new_controller_func -def user_has_privilege(privilege_name): +def user_has_privilege(privilege_name, allow_admin=True): """ Requires that a user have a particular privilege in order to access a page. In order to require that a user have multiple privileges, use this @@ -85,14 +85,17 @@ def user_has_privilege(privilege_name): the privilege object. This object is the name of the privilege, as assigned in the Privilege.privilege_name column + + :param allow_admin If this is true then if the user is an admin + it will allow the user even if the user doesn't + have the privilage given in privilage_name. """ def user_has_privilege_decorator(controller): @wraps(controller) @require_active_login def wrapper(request, *args, **kwargs): - user_id = request.user.id - if not request.user.has_privilege(privilege_name): + if not request.user.has_privilege(privilege_name, allow_admin): raise Forbidden() return controller(request, *args, **kwargs) @@ -369,7 +372,8 @@ def require_admin_or_moderator_login(controller): @wraps(controller) def new_controller_func(request, *args, **kwargs): if request.user and \ - not request.user.has_privilege(u'admin',u'moderator'): + not (request.user.has_privilege(u'admin') + or request.user.has_privilege(u'moderator')): raise Forbidden() elif not request.user: diff --git a/mediagoblin/templates/mediagoblin/base.html b/mediagoblin/templates/mediagoblin/base.html index 63a2a6ff..13cfb47b 100644 --- a/mediagoblin/templates/mediagoblin/base.html +++ b/mediagoblin/templates/mediagoblin/base.html @@ -72,8 +72,8 @@
{%- if request.user %} - {% if request.user and - request.user.has_privilege('active') and + {% if request.user and + request.user.has_privilege('active') and not request.user.is_banned() %} {% set notification_count = get_notification_count(request.user.id) %} @@ -158,7 +158,7 @@ {%- trans %}Create new collection{% endtrans -%} {% template_hook("header_dropdown_buttons") %} - {% if request.user.has_privilege('admin','moderator') %} + {% if request.user.has_privilege('moderator') %}

{% trans %}Moderation powers:{% endtrans %} diff --git a/mediagoblin/templates/mediagoblin/moderation/user.html b/mediagoblin/templates/mediagoblin/moderation/user.html index 37e7eee9..594f845d 100644 --- a/mediagoblin/templates/mediagoblin/moderation/user.html +++ b/mediagoblin/templates/mediagoblin/moderation/user.html @@ -175,7 +175,7 @@ {% for privilege in privileges %} {{ privilege.privilege_name }} - {% if privilege in user.all_privileges %} + {% if user.has_privilege(privilege.privilege_name) %} {% trans %}Yes{% endtrans %}{% else %} @@ -183,7 +183,7 @@ {% if request.user.has_privilege('admin') %} - {% if privilege in user.all_privileges %} + {% if user.has_privilege(privilege.privilege_name) %} diff --git a/mediagoblin/tests/test_modelmethods.py b/mediagoblin/tests/test_modelmethods.py index ca436c76..32d5dce0 100644 --- a/mediagoblin/tests/test_modelmethods.py +++ b/mediagoblin/tests/test_modelmethods.py @@ -179,20 +179,17 @@ class TestUserHasPrivilege: self._setup() # then test out the user.has_privilege method for one privilege - assert not self.natalie_user.has_privilege(u'commenter') - assert self.aeva_user.has_privilege(u'active') + assert not self.aeva_user.has_privilege(u'admin') + assert self.natalie_user.has_privilege(u'active') - - def test_user_has_privileges_multiple(self, test_app): + def test_allow_admin(self, test_app): self._setup() - # when multiple args are passed to has_privilege, the method returns - # True if the user has ANY of the privileges - assert self.natalie_user.has_privilege(u'admin',u'commenter') - assert self.aeva_user.has_privilege(u'moderator',u'active') - assert not self.natalie_user.has_privilege(u'commenter',u'uploader') - + # This should work because she is an admin. + assert self.natalie_user.has_privilege(u'commenter') + # Test that we can look this out ignoring that she's an admin + assert not self.natalie_user.has_privilege(u'commenter', allow_admin=False) def test_media_data_init(test_app): Session.rollback() -- 2.25.1