From ca4dfb449df224e89cef0aeea37c1dc70a603d86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20G=C3=A1lvez=20Mart=C3=ADnez?= Date: Wed, 6 Jan 2021 00:41:28 +0100 Subject: [PATCH 1/2] Add endpoint argument to decorators in order to allow to use it inside blueprints Improve performance of _check_permission method Automatically add deny rules when an allow decorator is added in order to deny access for all roles that not are in the decorator Fix tests --- flask_rbac/__init__.py | 29 +++++++++++++++++++++++----- flask_rbac/model.py | 4 ++++ test_rbac.py | 44 +++++++++++++++++++++--------------------- 3 files changed, 50 insertions(+), 27 deletions(-) diff --git a/flask_rbac/__init__.py b/flask_rbac/__init__.py index 9eafa96..e2f4a38 100644 --- a/flask_rbac/__init__.py +++ b/flask_rbac/__init__.py @@ -7,6 +7,7 @@ """ import itertools +from collections import defaultdict from flask import request, abort, _request_ctx_stack @@ -256,7 +257,7 @@ def a_view_func(): roles = _user.get_roles() return self._check_permission(roles, method, endpoint) - def allow(self, roles, methods, with_children=True): + def allow(self, roles, methods, with_children=True, endpoint=None): """This is a decorator function. You can allow roles to access the view func with it. @@ -280,12 +281,13 @@ def website_setting(): """ def decorator(view_func): _methods = [m.upper() for m in methods] - for r, m, v in itertools.product(roles, _methods, [view_func.__name__]): + resource = [endpoint or view_func.__name__] + for r, m, v in itertools.product(roles, _methods, resource): self.before_acl['allow'].append((r, m, v, with_children)) return view_func return decorator - def deny(self, roles, methods, with_children=False): + def deny(self, roles, methods, with_children=False, endpoint=None): """This is a decorator function. You can deny roles to access the view func with it. @@ -305,7 +307,8 @@ def article_post(): """ def decorator(view_func): _methods = [m.upper() for m in methods] - for r, m, v in itertools.product(roles, _methods, [view_func.__name__]): + resource = [endpoint or view_func.__name__] + for r, m, v in itertools.product(roles, _methods, resource): self.before_acl['deny'].append((r, m, v, with_children)) return view_func return decorator @@ -361,7 +364,6 @@ def _authenticate(self): (current_user, self._user_model.__class__)) resource = request.endpoint - if not resource: abort(404) @@ -378,6 +380,7 @@ def _authenticate(self): return self._deny_hook() def _check_permission(self, roles, method, resource): + if self.acl.is_exempt(resource): return True @@ -400,6 +403,7 @@ def _check_permission(self, roles, method, resource): if is_allowed != True and self.acl.is_allowed(r.get_name(), m, res): is_allowed = True + break if self.use_white: permit = (is_allowed is True) @@ -422,6 +426,21 @@ def _setup_acl(self): else: role = self._role_model.get_by_name(rn) self.acl.allow(role, method, resource, with_children) + + if not self.use_white: + to_deny_map = defaultdict(list) + all_roles = {x.get_name() if not isinstance(x, str) + else x for x in self._role_model.get_all()} + + for role, method, resource, with_children in self.before_acl['allow']: + to_deny_map[(resource, role, with_children)].append(method) + for k, methods in to_deny_map.items(): + v, role, with_children, = k + for r in all_roles - {role}: + for m in methods: + if (r, m, v, with_children) not in self.before_acl['allow']: + self.before_acl['deny'].append((r, m, v, with_children)) + for rn, method, resource, with_children in self.before_acl['deny']: role = self._role_model.get_by_name(rn) self.acl.deny(role, method, resource, with_children) diff --git a/flask_rbac/model.py b/flask_rbac/model.py index ba5d808..a8953d9 100644 --- a/flask_rbac/model.py +++ b/flask_rbac/model.py @@ -69,6 +69,10 @@ def get_by_name(name): """ return RoleMixin.roles[name] + @classmethod + def get_all(cls): + return cls.roles + class UserMixin(object): """This provides implementations for the methods that Flask-RBAC wants diff --git a/test_rbac.py b/test_rbac.py index 408dc54..6578d91 100644 --- a/test_rbac.py +++ b/test_rbac.py @@ -1,7 +1,7 @@ import unittest from flask import Flask, Response, make_response -from flask.ext.login import current_user as login_user +from flask_login import current_user as login_user from flask_rbac import RBAC, UserMixin, RoleMixin @@ -110,7 +110,7 @@ def e(): @before_decorator def f(): return Response('Hello from /f') - + @app.route('/g', methods=['GET']) @after_decorator @rbac.exempt @@ -146,18 +146,18 @@ def test_set_user_loader(self): def test_allow_get_view(self): global current_user current_user = anonymous - self.assertEqual(self.client.open('/').data, 'index') + self.assertEqual(self.client.open('/').data.decode('utf-8'), 'index') current_user = normal_user - self.assertEqual(self.client.open('/').data, 'index') - self.assertEqual(self.client.open('/b').data, 'Hello from /b') + self.assertEqual(self.client.open('/').data.decode('utf-8'), 'index') + self.assertEqual(self.client.open('/b').data.decode('utf-8'), 'Hello from /b') current_user = staff_role_user - self.assertEqual(self.client.open('/').data, 'index') - self.assertEqual(self.client.open('/b').data, 'Hello from /b') + self.assertEqual(self.client.open('/').data.decode('utf-8'), 'index') + self.assertEqual(self.client.open('/b').data.decode('utf-8'), 'Hello from /b') current_user = special_user - self.assertEqual(self.client.open('/a').data, 'Hello') + self.assertEqual(self.client.open('/a').data.decode('utf-8'), 'Hello') def test_deny_get_view(self): global current_user @@ -177,10 +177,10 @@ def test_deny_get_view(self): def test_allow_post_view(self): global current_user current_user = staff_role_user - self.assertEqual(self.client.post('/b').data, 'Hello from /b') + self.assertEqual(self.client.post('/b').data.decode('utf-8'), 'Hello from /b') current_user = special_user - self.assertEqual(self.client.post('/b').data, 'Hello from /b') + self.assertEqual(self.client.post('/b').data.decode('utf-8'), 'Hello from /b') def test_deny_post_view(self): global current_user @@ -193,20 +193,20 @@ def test_deny_post_view(self): def test_complicate_get_view(self): global current_user current_user = anonymous - self.assertEqual(self.client.open('/c').data, 'Hello from /c') + self.assertEqual(self.client.open('/c').data.decode('utf-8'), 'Hello from /c') current_user = normal_user self.assertEqual(self.client.open('/c').status_code, 403) current_user = staff_role_user - self.assertEqual(self.client.open('/c').data, 'Hello from /c') + self.assertEqual(self.client.open('/c').data.decode('utf-8'), 'Hello from /c') def test_hook(self): global current_user current_user = special_user self.rbac.set_hook(lambda: make_response('Permission Denied', 403)) self.assertEqual(self.client.open('/').status_code, 403) - self.assertEqual(self.client.open('/').data, 'Permission Denied') + self.assertEqual(self.client.open('/').data.decode('utf-8'), 'Permission Denied') def test_has_permission(self): global current_user @@ -227,20 +227,20 @@ def test_has_permission(self): current_user = None self.assertTrue(self.rbac.has_permission('GET', 'h')) - self.assertEqual(self.client.open('/h').data, 'Hello from /h') + self.assertEqual(self.client.open('/h').data.decode('utf-8'), 'Hello from /h') def test_exempt(self): global current_user current_user = anonymous - self.assertEqual(self.client.open('/g').data, 'Hello from /g') + self.assertEqual(self.client.open('/g').data.decode('utf-8'), 'Hello from /g') current_user = special_user - self.assertEqual(self.client.open('/g').data, 'Hello from /g') + self.assertEqual(self.client.open('/g').data.decode('utf-8'), 'Hello from /g') current_user = normal_user - self.assertEqual(self.client.open('/g').data, 'Hello from /g') + self.assertEqual(self.client.open('/g').data.decode('utf-8'), 'Hello from /g') class NoWhiteApplicationUnitTests(unittest.TestCase): @@ -253,10 +253,10 @@ def setUp(self): def test_allow_get_view(self): global current_user current_user = normal_user - self.assertEqual(self.client.open('/d').data, 'Hello from /d') + self.assertEqual(self.client.open('/d').data.decode('utf-8'), 'Hello from /d') current_user = staff_role_user - self.assertEqual(self.client.open('/d').data, 'Hello from /d') + self.assertEqual(self.client.open('/d').data.decode('utf-8'), 'Hello from /d') def test_deny_get_view(self): global current_user @@ -273,10 +273,10 @@ def test_deny_get_view(self): def test_allow_post_view(self): global current_user current_user = anonymous - self.assertEqual(self.client.post('/f').data, 'Hello from /f') + self.assertEqual(self.client.post('/f').data.decode('utf-8'), 'Hello from /f') current_user = staff_role_user - self.assertEqual(self.client.post('/f').data, 'Hello from /f') + self.assertEqual(self.client.post('/f').data.decode('utf-8'), 'Hello from /f') def test_deny_post_view(self): global current_user @@ -386,7 +386,7 @@ class DecoratorUnitTests(unittest.TestCase): def setUp(self): self.rbac = RBAC() - + @self.rbac.as_role_model class RoleModel(RoleMixin): pass From a56ac44f7af3a08ab0ebe40dfe58f91eb886808e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20G=C3=A1lvez=20Mart=C3=ADnez?= Date: Thu, 7 Jan 2021 10:53:51 +0100 Subject: [PATCH 2/2] Update docs Migrate Sphinx conf.py to python3 Improve perfomance of _setup_acl method --- docs/conf.py | 30 +++++++++++++++--------------- docs/quickstart.rst.inc | 32 ++++++++++++++++++++++++++------ flask_rbac/__init__.py | 12 ++++++------ flask_rbac/model.py | 2 ++ 4 files changed, 49 insertions(+), 27 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 52bc9d6..8c6b107 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -43,8 +43,8 @@ master_doc = 'index' # General information about the project. -project = u'Flask-RBAC' -copyright = u'2014, shonenada' +project = 'Flask-RBAC' +copyright = '2014, shonenada' # The version info for the project you're documenting, acts as replacement for # |version| and |release|, also used in various other places throughout the @@ -199,8 +199,8 @@ # (source start file, target name, title, # author, documentclass [howto, manual, or own class]). latex_documents = [ - ('index', 'Flask-RBAC.tex', u'Flask-RBAC Documentation', - u'shonenada', 'manual'), + ('index', 'Flask-RBAC.tex', 'Flask-RBAC Documentation', + 'shonenada', 'manual'), ] # The name of an image file (relative to this directory) to place at the top of @@ -229,8 +229,8 @@ # One entry per manual page. List of tuples # (source start file, name, description, authors, manual section). man_pages = [ - ('index', 'flask-rbac', u'Flask-RBAC Documentation', - [u'shonenada'], 1) + ('index', 'flask-rbac', 'Flask-RBAC Documentation', + ['shonenada'], 1) ] # If true, show URL addresses after external links. @@ -243,8 +243,8 @@ # (source start file, target name, title, author, # dir menu entry, description, category) texinfo_documents = [ - ('index', 'Flask-RBAC', u'Flask-RBAC Documentation', - u'shonenada', 'Flask-RBAC', 'One line description of project.', + ('index', 'Flask-RBAC', 'Flask-RBAC Documentation', + 'shonenada', 'Flask-RBAC', 'One line description of project.', 'Miscellaneous'), ] @@ -264,13 +264,13 @@ # fall back if theme is not there try: __import__('flask_theme_support') -except ImportError, e: - print '-' * 74 - print 'Warning: Flask themes unavailable. Building with default theme' - print 'If you want the Flask themes, run this command and build again:' - print - print ' git submodule update --init' - print '-' * 74 +except ImportError as e: + print('-' * 74) + print('Warning: Flask themes unavailable. Building with default theme') + print('If you want the Flask themes, run this command and build again:') + print() + print(' git submodule update --init') + print('-' * 74) pygments_style = 'tango' html_theme = 'default' diff --git a/docs/quickstart.rst.inc b/docs/quickstart.rst.inc index 400b5a7..7447575 100644 --- a/docs/quickstart.rst.inc +++ b/docs/quickstart.rst.inc @@ -26,7 +26,6 @@ or you can configuration using factory method:: return app - Mode Setting ------------ @@ -34,10 +33,13 @@ There are two modes for Flask-RBAC, `RBAC_USE_WHITE` decide whether use white list to check the permission. And it set `False` to default. ============================ ================================================ - `RBAC_USE_WHITE = True` Only allowing rules can access the resources. - This means, all deny rules and rules + `RBAC_USE_WHITE = True` Only allowing rules can access the resources. + This means, all deny rules and rules you did not add cannot access the resources. `RBAC_USE_WHITE = False` Only denying rules cannot access the resources. + In case you set an allow rule, denying rules will + also be automatically created for existing + non-added roles in this route. ============================ ================================================ Change it using:: @@ -85,7 +87,7 @@ the Role class to adapt your application, here is an example:: self.name = name def add_parent(self, parent): - # You don't need to add this role to parent's children set, + # You don't need to add this role to parent's children set, # relationship between roles would do this work automatically self.parents.append(parent) @@ -154,7 +156,7 @@ Well, if your application works under SQLAlchemy:: yield role Same as role model, you should add user model to Flask-RBAC:: - + rbac.set_user_model(User) Or using decorator:: @@ -168,7 +170,7 @@ Set User Loader --------------- Flask-RBAC need to know who is current user, so it requires you to provide a -function which tells it who is current user. +function which tells it who is current user. Flask-RBAC will load current user from `Flask-Login`_ if you have install it by default. @@ -212,10 +214,28 @@ You can use `allow` and `deny` to add rules to Flask-RBAC:: The code above adding two rules: - Allows user of *anonymous* role to *GET* /. + - Deny user of *logged_user* role to *GET* and *POST* */account/signin*. +`Flask`_ itself assumes the name of the view function as the endpoint for the +registered URL rule, that's why in rules validation by default we use the decorated function +name to check against the endpoint of the input request. But, in case you specified +a different endpoint or you use the decorators inside a blueprint or +abstracted blueprints extensions like `Flask-Admin`_ you can directly specify to the decorator +the endpoint used in your route. + +.. code-block:: python + + @app.route('/signin', methods=['GET', 'POST'], endpoint='account.signin') + @rbac.deny(['logged_user'], methods=['GET', 'POST'], + endpoint='account.signin') + def signin(): + # show sign in page or handle sign in request. + pass .. _RoleMixin: api.html#flask-ext-rbac-model-rolemixin .. _UserMixin: api.html#flask-ext-rbac-model-usermixin .. _Flask-Login: https://flask-login.readthedocs.org/en/latest/ +.. _Flask: https://flask-admin.readthedocs.io/en/latest/introduction/?highlight=endpoint#generating-urls +.. _Flask-Admin: https://flask-admin.readthedocs.io/en/latest/introduction/?highlight=endpoint#generating-urls diff --git a/flask_rbac/__init__.py b/flask_rbac/__init__.py index e2f4a38..6dfab5f 100644 --- a/flask_rbac/__init__.py +++ b/flask_rbac/__init__.py @@ -401,7 +401,7 @@ def _check_permission(self, roles, method, resource): if self.acl.is_denied(r.get_name(), m, res): return False - if is_allowed != True and self.acl.is_allowed(r.get_name(), m, res): + if not is_allowed and self.acl.is_allowed(r.get_name(), m, res): is_allowed = True break @@ -435,11 +435,11 @@ def _setup_acl(self): for role, method, resource, with_children in self.before_acl['allow']: to_deny_map[(resource, role, with_children)].append(method) for k, methods in to_deny_map.items(): - v, role, with_children, = k - for r in all_roles - {role}: - for m in methods: - if (r, m, v, with_children) not in self.before_acl['allow']: - self.before_acl['deny'].append((r, m, v, with_children)) + view, role, with_children, = k + for r, m in itertools.product(all_roles - {role}, methods): + rule = (r, m, view, with_children) + if rule not in self.before_acl['allow']: + self.before_acl['deny'].append(rule) for rn, method, resource, with_children in self.before_acl['deny']: role = self._role_model.get_by_name(rn) diff --git a/flask_rbac/model.py b/flask_rbac/model.py index a8953d9..edb5259 100644 --- a/flask_rbac/model.py +++ b/flask_rbac/model.py @@ -71,6 +71,8 @@ def get_by_name(name): @classmethod def get_all(cls): + """Return all existing roles + """ return cls.roles