From e0786ec5e6f7d919d024883107208d55716ec602 Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Wed, 18 Sep 2013 17:01:58 +0200 Subject: [PATCH 01/17] Match operators on Permissions on the set needs - Also matches the initial premise on the permission objects. --- src/flask_principal.py | 4 ++-- tests/test_principal.py | 18 ++++++++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/flask_principal.py b/src/flask_principal.py index c60a6eb..73d3e0c 100644 --- a/src/flask_principal.py +++ b/src/flask_principal.py @@ -236,12 +236,12 @@ def __bool__(self): """ return self._bool() - def __and__(self, other): + def __or__(self, other): """Does the same thing as ``self.union(other)`` """ return self.union(other) - def __or__(self, other): + def __sub__(self, other): """Does the same thing as ``self.difference(other)`` """ return self.difference(other) diff --git a/tests/test_principal.py b/tests/test_principal.py index 120f95a..5becff6 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -187,23 +187,33 @@ def test_reverse_permission(self): d = p.reverse() assert ('a', 'b') in d.excludes - def test_permission_and(self): + def test_permission_difference(self): p1 = Permission(RoleNeed('boss')) p2 = Permission(RoleNeed('lackey')) - p3 = p1 & p2 - p4 = p1.union(p2) + p3 = p1 - p2 + p4 = p1.difference(p2) + + # parity with set operations + p3needs = p1.needs - p2.needs assert p3.needs == p4.needs + assert p3.needs == p3needs def test_permission_or(self): p1 = Permission(RoleNeed('boss'), RoleNeed('lackey')) p2 = Permission(RoleNeed('lackey'), RoleNeed('underling')) p3 = p1 | p2 - p4 = p1.difference(p2) + p4 = p1.union(p2) + + # Ensure that an `or` between sets also result in the expected + # behavior. As expected, as "any of which must be present to + # access a resource". + p3needs = p1.needs | p2.needs assert p3.needs == p4.needs + assert p3.needs == p3needs def test_contains(self): p1 = Permission(RoleNeed('boss'), RoleNeed('lackey')) From d706d3b9ef22bbe188edd3c97c5b47006e1072b3 Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Wed, 18 Sep 2013 17:06:15 +0200 Subject: [PATCH 02/17] Also test the operation on exclusion. --- tests/test_principal.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_principal.py b/tests/test_principal.py index 5becff6..4e997a1 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -200,6 +200,19 @@ def test_permission_difference(self): assert p3.needs == p4.needs assert p3.needs == p3needs + def test_permission_difference_excludes(self): + p1 = Permission(RoleNeed('boss')).reverse() + p2 = Permission(RoleNeed('lackey')).reverse() + + p3 = p1 - p2 + p4 = p1.difference(p2) + + # parity with set operations + p3excludes = p1.excludes - p2.excludes + + assert p3.excludes == p4.excludes + assert p3.excludes == p3excludes + def test_permission_or(self): p1 = Permission(RoleNeed('boss'), RoleNeed('lackey')) p2 = Permission(RoleNeed('lackey'), RoleNeed('underling')) @@ -215,6 +228,21 @@ def test_permission_or(self): assert p3.needs == p4.needs assert p3.needs == p3needs + def test_permission_or_excludes(self): + p1 = Permission(RoleNeed('boss'), RoleNeed('lackey')).reverse() + p2 = Permission(RoleNeed('lackey'), RoleNeed('underling')).reverse() + + p3 = p1 | p2 + p4 = p1.union(p2) + + # Ensure that an `or` between sets also result in the expected + # behavior. As expected, as "any of which must be present to + # access a resource". + p3excludes = p1.excludes | p2.excludes + + assert p3.excludes == p4.excludes + assert p3.excludes == p3excludes + def test_contains(self): p1 = Permission(RoleNeed('boss'), RoleNeed('lackey')) p2 = Permission(RoleNeed('lackey')) From e8e84b8e0584de87a3e338efdf1a43ec8f09b6e4 Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Wed, 18 Sep 2013 17:36:20 +0200 Subject: [PATCH 03/17] Demonstrate further boolean operations. --- tests/test_principal.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/test_principal.py b/tests/test_principal.py index 4e997a1..5261bc9 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -108,12 +108,16 @@ def k(): def l(): s = [] if not admin_or_editor: - s.append("not admin") + s.append("not admin_or_editor") + if not (admin_permission or editor_permission): + s.append("not (admin or editor)") i = Identity('ali') identity_changed.send(app, identity=i) if admin_or_editor: - s.append("now admin") + s.append("now admin_or_editor") + if admin_permission or editor_permission: + s.append("now admin or editor") return Response('\n'.join(s)) @app.route("/m") @@ -137,6 +141,11 @@ def o(): admin_or_editor.test() return Response("OK") + @app.route("/o2") + def o2(): + (admin_permission | editor_permission).test() + return Response("OK") + @app.route("/p") def p(): admin_or_editor.test(404) @@ -317,8 +326,10 @@ def handle_permission_denied(error): def test_permission_bool(self): response = self.client.open('/l') assert response.status_code == 200 - assert b'not admin' in response.data - assert b'now admin' in response.data + assert b'not admin_or_editor' in response.data + assert b'not (admin or editor)' in response.data + assert b'now admin_or_editor' in response.data + assert b'now admin or editor' in response.data def test_denied_passes(self): response = self.client.open("/m") @@ -329,6 +340,7 @@ def test_denied_fails(self): def test_permission_test(self): self.assertRaises(PermissionDenied, self.client.open, '/o') + self.assertRaises(PermissionDenied, self.client.open, '/o2') def test_permission_test_with_http_exc(self): response = self.client.open("/p") From e1c7222bdddc313c01b371435d60fe576d372787 Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Wed, 18 Sep 2013 17:40:46 +0200 Subject: [PATCH 04/17] Further operator tests. --- tests/test_principal.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_principal.py b/tests/test_principal.py index 5261bc9..57d8202 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -146,6 +146,13 @@ def o2(): (admin_permission | editor_permission).test() return Response("OK") + @app.route("/o3") + def o3(): + i = mkadmin() + identity_changed.send(app, identity=i) + (admin_permission | editor_permission).test() + return Response("OK") + @app.route("/p") def p(): admin_or_editor.test(404) @@ -340,8 +347,14 @@ def test_denied_fails(self): def test_permission_test(self): self.assertRaises(PermissionDenied, self.client.open, '/o') + + def test_permission_operator_test(self): self.assertRaises(PermissionDenied, self.client.open, '/o2') + response = self.client.open('/o3') + assert response.status_code == 200 + assert response.data == 'OK' + def test_permission_test_with_http_exc(self): response = self.client.open("/p") assert response.status_code == 404 From e09568936f9494acbc50c011ca9d8dc40dceac87 Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Wed, 18 Sep 2013 22:54:06 +0200 Subject: [PATCH 05/17] Fix test case for python-3.3 --- tests/test_principal.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_principal.py b/tests/test_principal.py index 57d8202..e8899af 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -353,7 +353,7 @@ def test_permission_operator_test(self): response = self.client.open('/o3') assert response.status_code == 200 - assert response.data == 'OK' + assert response.data == b'OK' def test_permission_test_with_http_exc(self): response = self.client.open("/p") From 29ccd2b5f9b127183a4c3711ca17df752b17f903 Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Mon, 7 Oct 2013 22:36:07 +1300 Subject: [PATCH 06/17] Split out a BasePermission from Permission. - Have this class contain all the basic methods we need currently that are still needed by the Permission class. --- src/flask_principal.py | 106 +++++++++++++++++++++++------------------ 1 file changed, 59 insertions(+), 47 deletions(-) diff --git a/src/flask_principal.py b/src/flask_principal.py index 73d3e0c..5d9b6c4 100644 --- a/src/flask_principal.py +++ b/src/flask_principal.py @@ -211,17 +211,8 @@ def __exit__(self, *args): return False -class Permission(object): - """Represents needs, any of which must be present to access a resource - - :param needs: The needs for this permission - """ - def __init__(self, *needs): - """A set of needs, any of which must be present in an identity to have - access. - """ - - self.perms = {n: True for n in needs} +class BasePermission(object): + """The Base Permission.""" def _bool(self): return bool(self.can()) @@ -236,34 +227,6 @@ def __bool__(self): """ return self._bool() - def __or__(self, other): - """Does the same thing as ``self.union(other)`` - """ - return self.union(other) - - def __sub__(self, other): - """Does the same thing as ``self.difference(other)`` - """ - return self.difference(other) - - def __contains__(self, other): - """Does the same thing as ``other.issubset(self)``. - """ - return other.issubset(self) - - def __repr__(self): - return '<{0} needs={1} excludes={2}>'.format( - self.__class__.__name__, self.needs, self.excludes - ) - - @property - def needs(self): - return set(n for n in self.perms if self.perms[n]) - - @property - def excludes(self): - return set(n for n in self.perms if not self.perms[n]) - def require(self, http_exception=None): """Create a principal for this permission. @@ -293,6 +256,63 @@ def test(self, http_exception=None): with self.require(http_exception): pass + def allows(self, identity): + """Whether the identity can access this permission. + + :param identity: The identity + """ + + raise NotImplementedError + + def can(self): + """Whether the required context for this permission has access + + This creates an identity context and tests whether it can access this + permission + """ + return self.require().can() + + +class Permission(BasePermission): + """Represents needs, any of which must be present to access a resource + + :param needs: The needs for this permission + """ + def __init__(self, *needs): + """A set of needs, any of which must be present in an identity to have + access. + """ + + self.perms = {n: True for n in needs} + + def __or__(self, other): + """Does the same thing as ``self.union(other)`` + """ + return self.union(other) + + def __sub__(self, other): + """Does the same thing as ``self.difference(other)`` + """ + return self.difference(other) + + def __contains__(self, other): + """Does the same thing as ``other.issubset(self)``. + """ + return other.issubset(self) + + def __repr__(self): + return '<{0} needs={1} excludes={2}>'.format( + self.__class__.__name__, self.needs, self.excludes + ) + + @property + def needs(self): + return set(n for n in self.perms if self.perms[n]) + + @property + def excludes(self): + return set(n for n in self.perms if not self.perms[n]) + def reverse(self): """ Returns reverse of current state (needs->excludes, excludes->needs) @@ -354,14 +374,6 @@ def allows(self, identity): return True - def can(self): - """Whether the required context for this permission has access - - This creates an identity context and tests whether it can access this - permission - """ - return self.require().can() - class Denial(Permission): """ From 02c4171d1a3aecb40bf53d3610ae04537db8f4de Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Mon, 7 Oct 2013 22:56:38 +1300 Subject: [PATCH 07/17] Allow Permission class to know the error code --- src/flask_principal.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/flask_principal.py b/src/flask_principal.py index 5d9b6c4..409d97d 100644 --- a/src/flask_principal.py +++ b/src/flask_principal.py @@ -214,6 +214,8 @@ def __exit__(self, *args): class BasePermission(object): """The Base Permission.""" + http_exception = None + def _bool(self): return bool(self.can()) @@ -239,6 +241,10 @@ def require(self, http_exception=None): :param http_exception: the HTTP exception code (403, 401 etc) """ + + if http_exception is None: + http_exception = self.http_exception + return IdentityContext(self, http_exception) def test(self, http_exception=None): From eed9fe936a6a4f56bd4b5fa61c76ea1f3c009891 Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Mon, 7 Oct 2013 23:19:19 +1300 Subject: [PATCH 08/17] Better id manager - Can't just test one role... --- tests/test_principal.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/test_principal.py b/tests/test_principal.py index e8899af..96702d7 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -16,8 +16,17 @@ def _on_principal_init(sender, identity): - if identity.id == 'ali': - identity.provides.add(RoleNeed('admin')) + role_map = { + 'ali': (RoleNeed('admin'),), + 'admin': (RoleNeed('admin'),), + 'editor': (RoleNeed('editor'),), + 'admin_editor': (RoleNeed('editor'), RoleNeed('admin')), + } + + roles = role_map.get(identity.id) + if roles: + for role in roles: + identity.provides.add(role) class ReraiseException(Exception): From 299f5b891b50e8354bc6ab520c2b6130b2154e9b Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Mon, 7 Oct 2013 23:23:34 +1300 Subject: [PATCH 09/17] Implement the bitwise ``or`` of permissions - Allows the creation of permission objects that check for any of the nested permissions. --- src/flask_principal.py | 32 ++++++++++++++++++- tests/test_principal.py | 69 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/flask_principal.py b/src/flask_principal.py index 409d97d..6cad72c 100644 --- a/src/flask_principal.py +++ b/src/flask_principal.py @@ -229,6 +229,11 @@ def __bool__(self): """ return self._bool() + def __or__(self, other): + """See ``OrPermission``. + """ + return OrPermission(self, other) + def require(self, http_exception=None): """Create a principal for this permission. @@ -279,6 +284,29 @@ def can(self): return self.require().can() +class _NaryOperatorPermission(BasePermission): + + def __init__(self, *permissions): + self.permissions = set(permissions) + + +class OrPermission(_NaryOperatorPermission): + """Result of bitwise ``or`` of BasePermission""" + + def allows(self, identity): + """ + Checks for any of the nested permission instances that allow the + identity and return True, else return False. + + :param identity: The identity. + """ + + for p in self.permissions: + if p.allows(identity): + return True + return False + + class Permission(BasePermission): """Represents needs, any of which must be present to access a resource @@ -294,7 +322,9 @@ def __init__(self, *needs): def __or__(self, other): """Does the same thing as ``self.union(other)`` """ - return self.union(other) + if isinstance(other, Permission): + return self.union(other) + return super(Permission, self).__or__(other) def __sub__(self, other): """Does the same thing as ``self.difference(other)`` diff --git a/tests/test_principal.py b/tests/test_principal.py index 96702d7..e699054 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -5,6 +5,7 @@ from flask import Flask, Response +from flask_principal import BasePermission, OrPermission from flask_principal import Principal, Permission, Denial, RoleNeed, \ PermissionDenied, identity_changed, Identity, identity_loaded @@ -15,6 +16,16 @@ admin_denied = Denial(RoleNeed('admin')) +class RolenamePermission(BasePermission): + def __init__(self, role): + self.role = role + def allows(self, identity): + return RoleNeed(self.role) in identity.provides + +admin_role_permission = RolenamePermission('admin') +editor_role_permission = RolenamePermission('editor') + + def _on_principal_init(sender, identity): role_map = { 'ali': (RoleNeed('admin'),), @@ -86,6 +97,48 @@ def f(): with admin_or_editor.require(): return Response('hello') + @app.route('/or_base') + def or_base(): + i = mkadmin() + admin_or_editor_rp = (admin_role_permission | editor_role_permission) + identity_changed.send(app, identity=i) + with admin_or_editor_rp.require(): + return Response('hello') + + @app.route('/or_mixed1') + def or_mixed1(): + result = [] + admin_or_editor_mixed = (admin_role_permission | editor_permission) + + i = Identity('admin') + identity_changed.send(app, identity=i) + with admin_or_editor_mixed.require(): + result.append('good') + + i = Identity('editor') + identity_changed.send(app, identity=i) + with admin_or_editor_mixed.require(): + result.append('good') + + return Response(''.join(result)) + + @app.route('/or_mixed2') # reversed type of the above. + def or_mixed2(): + result = [] + admin_or_editor_mixed = (admin_permission | editor_role_permission) + + i = Identity('admin') + identity_changed.send(app, identity=i) + with admin_or_editor_mixed.require(): + result.append('good') + + i = Identity('editor') + identity_changed.send(app, identity=i) + with admin_or_editor_mixed.require(): + result.append('good') + + return Response(''.join(result)) + @app.route('/g') @admin_permission.require() @editor_permission.require() @@ -175,6 +228,15 @@ def mkadmin(): return i +class BasePermissionUnitTests(unittest.TestCase): + + def test_or_permission(self): + admin_or_editor_rp = (admin_role_permission | editor_role_permission) + self.assertTrue(isinstance(admin_or_editor_rp, OrPermission)) + self.assertEqual(admin_or_editor_rp.permissions, + set(admin_role_permission, editor_role_permission)) + + class PrincipalUnitTests(unittest.TestCase): def test_permission_union(self): @@ -307,6 +369,13 @@ def test_or_permissions(self): assert self.client.open('/e').data == b'hello' assert self.client.open('/f').data == b'hello' + def test_base_or_permissions(self): + assert self.client.open('/or_base').data == b'hello' + + def test_mixed_or_permissions(self): + assert self.client.open('/or_mixed1').data == b'goodgood' + assert self.client.open('/or_mixed2').data == b'goodgood' + def test_and_permissions_view_denied(self): self.assertRaises(PermissionDenied, self.client.open, '/g') From 7172e2ea1fcce8304e9f0e88495a919655923cfe Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Tue, 8 Oct 2013 00:04:03 +1300 Subject: [PATCH 10/17] Implement the bitwise ``and`` of permissions - Have the ``AndPermission`` similar to the ``OrPermission``. --- src/flask_principal.py | 22 +++++++++++++++++ tests/test_principal.py | 53 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/flask_principal.py b/src/flask_principal.py index 6cad72c..3528235 100644 --- a/src/flask_principal.py +++ b/src/flask_principal.py @@ -234,6 +234,11 @@ def __or__(self, other): """ return OrPermission(self, other) + def __and__(self, other): + """See ``AndPermission``. + """ + return AndPermission(self, other) + def require(self, http_exception=None): """Create a principal for this permission. @@ -307,6 +312,23 @@ def allows(self, identity): return False +class AndPermission(_NaryOperatorPermission): + """Result of bitwise ``and`` of BasePermission""" + + def allows(self, identity): + """ + Checks for any of the nested permission instances that disallow + the identity and return False, else return True. + + :param identity: The identity. + """ + + for p in self.permissions: + if not p.allows(identity): + return False + return True + + class Permission(BasePermission): """Represents needs, any of which must be present to access a resource diff --git a/tests/test_principal.py b/tests/test_principal.py index e699054..7d86615 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -5,7 +5,7 @@ from flask import Flask, Response -from flask_principal import BasePermission, OrPermission +from flask_principal import BasePermission, OrPermission, AndPermission from flask_principal import Principal, Permission, Denial, RoleNeed, \ PermissionDenied, identity_changed, Identity, identity_loaded @@ -97,6 +97,39 @@ def f(): with admin_or_editor.require(): return Response('hello') + @app.route('/and_base_fail') + def and_base_fail(): + i = mkadmin() + admin_and_editor_rp = (admin_role_permission & editor_role_permission) + identity_changed.send(app, identity=i) + with admin_and_editor_rp.require(): + return Response('fail') + + @app.route('/and_base_success') + def and_base_success(): + i = Identity('admin_editor') + identity_changed.send(app, identity=i) + # using both formerly default, calling parent __and__ + admin_and_editor_rp = (admin_permission & editor_permission) + with admin_and_editor_rp.require(): + return Response('good') + + @app.route('/and_mixed1') + def and_mixed1(): + admin_and_editor_mixed = (admin_role_permission & editor_permission) + i = Identity('editor') + identity_changed.send(app, identity=i) + with admin_and_editor_mixed.require(): + return Response('fail') + + @app.route('/and_mixed2') # reversed type of the above. + def and_mixed2(): + admin_and_editor_mixed = (admin_permission & editor_role_permission) + i = Identity('admin_editor') + identity_changed.send(app, identity=i) + with admin_and_editor_mixed.require(): + return Response('good') + @app.route('/or_base') def or_base(): i = mkadmin() @@ -234,7 +267,15 @@ def test_or_permission(self): admin_or_editor_rp = (admin_role_permission | editor_role_permission) self.assertTrue(isinstance(admin_or_editor_rp, OrPermission)) self.assertEqual(admin_or_editor_rp.permissions, - set(admin_role_permission, editor_role_permission)) + set([admin_role_permission, editor_role_permission])) + + def test_and_permission(self): + admin_and_editor_rp = (admin_role_permission & editor_role_permission) + self.assertTrue(isinstance(admin_and_editor_rp, AndPermission)) + self.assertEqual(admin_and_editor_rp.permissions, + set([admin_role_permission, editor_role_permission])) + + # TODO test manual construction class PrincipalUnitTests(unittest.TestCase): @@ -376,6 +417,14 @@ def test_mixed_or_permissions(self): assert self.client.open('/or_mixed1').data == b'goodgood' assert self.client.open('/or_mixed2').data == b'goodgood' + def test_base_and_permissions(self): + self.assertRaises(PermissionDenied, self.client.open, '/and_base_fail') + self.assertEqual(self.client.open('/and_base_success').data, b'good') + + def test_mixed_and_permissions(self): + self.assertRaises(PermissionDenied, self.client.open, '/and_mixed1') + self.assertEqual(self.client.open('/and_mixed2').data, b'good') + def test_and_permissions_view_denied(self): self.assertRaises(PermissionDenied, self.client.open, '/g') From 48331d6c8012d5ee5077acbb03af4eab3a500381 Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Tue, 8 Oct 2013 00:34:32 +1300 Subject: [PATCH 11/17] Testing the mixed usage of ``and``, ``or``. --- tests/test_principal.py | 68 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/test_principal.py b/tests/test_principal.py index 7d86615..9c05ca9 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -13,6 +13,8 @@ admin_permission = Permission(RoleNeed('admin')) admin_or_editor = Permission(RoleNeed('admin'), RoleNeed('editor')) editor_permission = Permission(RoleNeed('editor')) +manager_permission = Permission(RoleNeed('manager')) + admin_denied = Denial(RoleNeed('admin')) @@ -24,6 +26,8 @@ def allows(self, identity): admin_role_permission = RolenamePermission('admin') editor_role_permission = RolenamePermission('editor') +manager_role_permission = RolenamePermission('manager') +reviewer_role_permission = RolenamePermission('reviewer') def _on_principal_init(sender, identity): @@ -31,7 +35,11 @@ def _on_principal_init(sender, identity): 'ali': (RoleNeed('admin'),), 'admin': (RoleNeed('admin'),), 'editor': (RoleNeed('editor'),), + 'reviewer': (RoleNeed('reviewer'),), 'admin_editor': (RoleNeed('editor'), RoleNeed('admin')), + 'manager': (RoleNeed('manager'),), + 'manager_editor': (RoleNeed('editor'), RoleNeed('manager')), + 'reviewer_editor': (RoleNeed('editor'), RoleNeed('reviewer')), } roles = role_map.get(identity.id) @@ -172,6 +180,58 @@ def or_mixed2(): return Response(''.join(result)) + @app.route('/mixed_ops_fail') + def mixed_ops_fail(): + result = [] + mixed_perms = (admin_permission | manager_permission | + (reviewer_role_permission & editor_role_permission)) + + i = Identity('editor') + identity_changed.send(app, identity=i) + with mixed_perms.require(): + result.append('fail') + + @app.route('/mixed_ops1') + def mixed_ops1(): + result = [] + mixed_perms = (admin_permission | manager_permission | + (reviewer_role_permission & editor_role_permission)) + + i = Identity('reviewer_editor') + identity_changed.send(app, identity=i) + with mixed_perms.require(): + result.append('good') + + i = Identity('manager') + identity_changed.send(app, identity=i) + with mixed_perms.require(): + result.append('good') + + i = Identity('admin') + identity_changed.send(app, identity=i) + with mixed_perms.require(): + result.append('good') + + return Response(''.join(result)) + + @app.route('/mixed_ops2') + def mixed_ops2(): + result = [] + mixed_perms = ((admin_permission & editor_permission) | + (manager_role_permission & editor_role_permission)) + + i = Identity('manager_editor') + identity_changed.send(app, identity=i) + with mixed_perms.require(): + result.append('good') + + i = Identity('admin_editor') + identity_changed.send(app, identity=i) + with mixed_perms.require(): + result.append('good') + + return Response(''.join(result)) + @app.route('/g') @admin_permission.require() @editor_permission.require() @@ -425,6 +485,14 @@ def test_mixed_and_permissions(self): self.assertRaises(PermissionDenied, self.client.open, '/and_mixed1') self.assertEqual(self.client.open('/and_mixed2').data, b'good') + def test_mixed_and_or_permissions_fail(self): + self.assertRaises(PermissionDenied, + self.client.open, '/mixed_ops_fail') + + def test_mixed_and_or_permissions(self): + self.assertEqual(self.client.open('/mixed_ops1').data, b'goodgoodgood') + self.assertEqual(self.client.open('/mixed_ops2').data, b'goodgood') + def test_and_permissions_view_denied(self): self.assertRaises(PermissionDenied, self.client.open, '/g') From 5a62b76edca56ae16ba6aa6408acef6aff11e466 Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Sat, 19 Oct 2013 14:48:04 +1300 Subject: [PATCH 12/17] Include alternative permissions for fail tests. - Done by testing that those singular bits of permission don't result in accessing of private bad data. --- tests/test_principal.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tests/test_principal.py b/tests/test_principal.py index 9c05ca9..6cc6604 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -222,14 +222,29 @@ def mixed_ops2(): i = Identity('manager_editor') identity_changed.send(app, identity=i) - with mixed_perms.require(): + if mixed_perms.can(): result.append('good') + i = Identity('manager') + identity_changed.send(app, identity=i) + if mixed_perms.can(): + result.append('bad') + + i = Identity('editor') + identity_changed.send(app, identity=i) + if mixed_perms.can(): + result.append('bad') + i = Identity('admin_editor') identity_changed.send(app, identity=i) - with mixed_perms.require(): + if mixed_perms.can(): result.append('good') + i = Identity('admin') + identity_changed.send(app, identity=i) + if mixed_perms.can(): + result.append('bad') + return Response(''.join(result)) @app.route('/g') From 4a6f66fd9249ed48ad574754d2c636795960a9e6 Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Sat, 19 Oct 2013 15:14:16 +1300 Subject: [PATCH 13/17] Implement invert (not) operation. --- src/flask_principal.py | 27 ++++++++++++++++ tests/test_principal.py | 69 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/src/flask_principal.py b/src/flask_principal.py index 3528235..f9cbb05 100644 --- a/src/flask_principal.py +++ b/src/flask_principal.py @@ -239,6 +239,11 @@ def __and__(self, other): """ return AndPermission(self, other) + def __invert__(self): + """See ``NotPermission``. + """ + return NotPermission(self) + def require(self, http_exception=None): """Create a principal for this permission. @@ -295,6 +300,9 @@ def __init__(self, *permissions): self.permissions = set(permissions) +# These classes would be unnecessary if we have predicate calculus +# primatives of some kind. + class OrPermission(_NaryOperatorPermission): """Result of bitwise ``or`` of BasePermission""" @@ -329,6 +337,25 @@ def allows(self, identity): return True +class NotPermission(BasePermission): + """ + Result of bitwise ``not`` of BasePermission + + Really could be implemented by returning a transformed result of the + source class of itself, but for the sake of clear presentation I am + not doing that. + """ + + def __init__(self, permission): + self.permission = permission + + def __invert__(self): + return self.permission + + def allows(self, identity): + return not self.permission.allows(identity) + + class Permission(BasePermission): """Represents needs, any of which must be present to access a resource diff --git a/tests/test_principal.py b/tests/test_principal.py index 6cc6604..1c4e1fb 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -14,6 +14,8 @@ admin_or_editor = Permission(RoleNeed('admin'), RoleNeed('editor')) editor_permission = Permission(RoleNeed('editor')) manager_permission = Permission(RoleNeed('manager')) +admin_or_editor_or_manager = Permission( + RoleNeed('admin'), RoleNeed('editor'), RoleNeed('manager')) admin_denied = Denial(RoleNeed('admin')) @@ -40,6 +42,7 @@ def _on_principal_init(sender, identity): 'manager': (RoleNeed('manager'),), 'manager_editor': (RoleNeed('editor'), RoleNeed('manager')), 'reviewer_editor': (RoleNeed('editor'), RoleNeed('reviewer')), + 'admin_manager': (RoleNeed('admin'), RoleNeed('manager')), } roles = role_map.get(identity.id) @@ -180,6 +183,25 @@ def or_mixed2(): return Response(''.join(result)) + @app.route('/not_base') + def not_base(): + result = [] + not_admin_perm = ~admin_role_permission + + identity_changed.send(app, identity=Identity('admin')) + if not_admin_perm.can(): + result.append('admin') + + identity_changed.send(app, identity=Identity('editor')) + if not_admin_perm.can(): + result.append('editor') + + identity_changed.send(app, identity=Identity('admin_manager')) + if not_admin_perm.can(): + result.append('admin_manager') + + return Response(''.join(result)) + @app.route('/mixed_ops_fail') def mixed_ops_fail(): result = [] @@ -247,6 +269,43 @@ def mixed_ops2(): return Response(''.join(result)) + @app.route('/mixed_ops3') + def mixed_ops3(): + result = [] + mixed_perms = ( + ((admin_permission & editor_permission) | + (manager_role_permission & editor_role_permission)) & + ~(manager_role_permission & admin_permission) & + ~reviewer_role_permission + ) + + i = Identity('manager_editor') + identity_changed.send(app, identity=i) + if mixed_perms.can(): + result.append('good') + + i = Identity('admin_editor') + identity_changed.send(app, identity=i) + if mixed_perms.can(): + result.append('good') + + i = Identity('admin_manager') + identity_changed.send(app, identity=i) + if mixed_perms.can(): + result.append('bad') + + i = Identity('manager_editor_admin') + identity_changed.send(app, identity=i) + if mixed_perms.can(): + result.append('bad') + + i = Identity('reviewer') + identity_changed.send(app, identity=i) + if mixed_perms.can(): + result.append('bad') + + return Response(''.join(result)) + @app.route('/g') @admin_permission.require() @editor_permission.require() @@ -446,6 +505,12 @@ def test_permission_or_excludes(self): assert p3.excludes == p4.excludes assert p3.excludes == p3excludes + def test_permission_not(self): + p1 = Permission(RoleNeed('boss'), RoleNeed('lackey')) + p2 = ~p1 + p3 = ~p2 + assert p3 == p1 + def test_contains(self): p1 = Permission(RoleNeed('boss'), RoleNeed('lackey')) p2 = Permission(RoleNeed('lackey')) @@ -488,6 +553,9 @@ def test_or_permissions(self): def test_base_or_permissions(self): assert self.client.open('/or_base').data == b'hello' + def test_base_not_permissions(self): + self.assertEqual(self.client.open('/not_base').data, b'editor') + def test_mixed_or_permissions(self): assert self.client.open('/or_mixed1').data == b'goodgood' assert self.client.open('/or_mixed2').data == b'goodgood' @@ -507,6 +575,7 @@ def test_mixed_and_or_permissions_fail(self): def test_mixed_and_or_permissions(self): self.assertEqual(self.client.open('/mixed_ops1').data, b'goodgoodgood') self.assertEqual(self.client.open('/mixed_ops2').data, b'goodgood') + self.assertEqual(self.client.open('/mixed_ops3').data, b'goodgood') def test_and_permissions_view_denied(self): self.assertRaises(PermissionDenied, self.client.open, '/g') From dc08f24cb11726079ef5f8fb9ac4b52bd9f6be3d Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Sat, 19 Oct 2013 15:41:53 +1300 Subject: [PATCH 14/17] Expose those bitwise implementation methods - Could keep them outside not behind those special override methods and have those call the actual implementation. --- src/flask_principal.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/flask_principal.py b/src/flask_principal.py index f9cbb05..85c26f4 100644 --- a/src/flask_principal.py +++ b/src/flask_principal.py @@ -232,16 +232,25 @@ def __bool__(self): def __or__(self, other): """See ``OrPermission``. """ + return self.or_(other) + + def or_(self, other): return OrPermission(self, other) def __and__(self, other): """See ``AndPermission``. """ + return self.and_(other) + + def and_(self, other): return AndPermission(self, other) def __invert__(self): """See ``NotPermission``. """ + return self.invert() + + def invert(self): return NotPermission(self) def require(self, http_exception=None): @@ -349,7 +358,7 @@ class NotPermission(BasePermission): def __init__(self, permission): self.permission = permission - def __invert__(self): + def invert(self): return self.permission def allows(self, identity): From f8e4bc4d449ba0f49e4ae2d350bac218cb8fa401 Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Sat, 19 Oct 2013 15:54:43 +1300 Subject: [PATCH 15/17] Test out operator combination using constructor --- tests/test_principal.py | 62 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/test_principal.py b/tests/test_principal.py index 1c4e1fb..e31c52a 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -6,6 +6,7 @@ from flask import Flask, Response from flask_principal import BasePermission, OrPermission, AndPermission +from flask_principal import NotPermission from flask_principal import Principal, Permission, Denial, RoleNeed, \ PermissionDenied, identity_changed, Identity, identity_loaded @@ -43,6 +44,8 @@ def _on_principal_init(sender, identity): 'manager_editor': (RoleNeed('editor'), RoleNeed('manager')), 'reviewer_editor': (RoleNeed('editor'), RoleNeed('reviewer')), 'admin_manager': (RoleNeed('admin'), RoleNeed('manager')), + 'admin_editor_manager': ( + RoleNeed('admin'), RoleNeed('editor'), RoleNeed('manager')), } roles = role_map.get(identity.id) @@ -125,6 +128,34 @@ def and_base_success(): with admin_and_editor_rp.require(): return Response('good') + @app.route('/and_bunch') + def and_bunch(): + result = [] + + bunch = AndPermission( + admin_role_permission, + editor_role_permission, + manager_role_permission, + ) + + identity_changed.send(app, identity=Identity('admin')) + if bunch.can(): + result.append('bad') + + identity_changed.send(app, identity=Identity('manager')) + if bunch.can(): + result.append('bad') + + identity_changed.send(app, identity=Identity('reviewer')) + if bunch.can(): + result.append('bad') + + identity_changed.send(app, identity=Identity('admin_editor_manager')) + if bunch.can(): + result.append('good') + + return ''.join(result) + @app.route('/and_mixed1') def and_mixed1(): admin_and_editor_mixed = (admin_role_permission & editor_permission) @@ -149,6 +180,31 @@ def or_base(): with admin_or_editor_rp.require(): return Response('hello') + @app.route('/or_bunch') + def or_bunch(): + result = [] + + bunch = OrPermission( + admin_role_permission, + editor_role_permission, + manager_role_permission, + reviewer_role_permission, + ) + + identity_changed.send(app, identity=Identity('admin')) + if bunch.can(): + result.append('good') + + identity_changed.send(app, identity=Identity('manager')) + if bunch.can(): + result.append('good') + + identity_changed.send(app, identity=Identity('reviewer')) + if bunch.can(): + result.append('good') + + return ''.join(result) + @app.route('/or_mixed1') def or_mixed1(): result = [] @@ -553,6 +609,9 @@ def test_or_permissions(self): def test_base_or_permissions(self): assert self.client.open('/or_base').data == b'hello' + def test_or_permissions_bunch(self): + self.assertEqual(self.client.open('/or_bunch').data, b'goodgoodgood') + def test_base_not_permissions(self): self.assertEqual(self.client.open('/not_base').data, b'editor') @@ -597,6 +656,9 @@ def test_and_permissions_view_with_http_exc_decorated(self): response = self.client.open("/k") assert response.status_code == 403 + def test_and_permissions_bunch(self): + self.assertEqual(self.client.open('/and_bunch').data, b'good') + def test_and_permissions_view_with_custom_errhandler(self): app = mkapp() From a65768387fee327a85477e7eadaedca15cb4d09e Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Sat, 19 Oct 2013 16:01:52 +1300 Subject: [PATCH 16/17] First attempt at explaining the bitwise operators. --- docs/index.rst | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/docs/index.rst b/docs/index.rst index df1ee3d..dee389a 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -314,6 +314,54 @@ of the resource, in this case the blog post:: abort(403) # HTTP Forbidden +Combining Permissions +--------------------- + +While the core constructs provided are sufficient for the common simple use +cases, for more complex cases it is possible to combine existing permissions +through the use of bitwise operators to result in a new permission object that +can do the complex validations. For instance, it is to create a permission +where the identity has the both the `"blog_poster"` or `"blog_reviewer"` role +but not the `"under_probation"` role. Example:: + + blog_admin = Permission(RoleNeed('blog_admin')) + blog_poster = Permission(RoleNeed('blog_poster')) + blog_reviewer = Permission(RoleNeed('blog_reviewer')) + under_probation = Permission(RoleNeed('under_probation')) + + prize_permission = ((blog_poster | blog_reviewer) & ~under_probation) + + @app.route('/blog/prizes') + @prize_permission.require() + def prize_redeem(): + # find out what prizes are available to blog users that are not + # under probation + return render_template('prize_redeem.html') + +Any number of these can be chained, but it is also possible to use the +constructor classes directly to combine these things together:: + + from flask.ext.principal import AndPermission, OrPermission + + allperms = AndPermission(blog_poster, blog_reviewer, blog_admin) + anyperms = OrPermission(blog_poster, blog_reviewer, blog_admin) + +Custom Permissions +------------------ + +Sometimes your permissions may be determined by other circumstances specific to +whatever you need to implement. For that you can create custom classes to +address your needs like so:: + + from flask.ext.principal import BasePermission + + class CustomPermission(BasePermission): + def allows(self, identity): + # Implement other conditions that allow this to pass + return False + +These custom permissions can be combined together via the bitwise operators as +explained above. API === From 0e6ffd5d8c02f410095d3bba30926a9b89f2ef83 Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Tue, 8 Oct 2013 00:36:57 +1300 Subject: [PATCH 17/17] Should have fail return a proper response anyway. --- tests/test_principal.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_principal.py b/tests/test_principal.py index e31c52a..ffdb540 100644 --- a/tests/test_principal.py +++ b/tests/test_principal.py @@ -260,14 +260,13 @@ def not_base(): @app.route('/mixed_ops_fail') def mixed_ops_fail(): - result = [] mixed_perms = (admin_permission | manager_permission | (reviewer_role_permission & editor_role_permission)) i = Identity('editor') identity_changed.send(app, identity=i) with mixed_perms.require(): - result.append('fail') + return Response('fail') @app.route('/mixed_ops1') def mixed_ops1():