This is an automated email from the ASF dual-hosted git repository.
dill0wn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git
The following commit(s) were added to refs/heads/master by this push:
new d49f88334 [#8556] remove TruthyCallable, has_access() now returns a
normal bool
d49f88334 is described below
commit d49f883347dc76b35dab9dc78326f9aec6a59fd5
Author: Dave Brondsema <dbronds...@slashdotmedia.com>
AuthorDate: Wed Jun 12 11:30:09 2024 -0400
[#8556] remove TruthyCallable, has_access() now returns a normal bool
---
Allura/allura/controllers/auth.py | 6 +-
Allura/allura/controllers/basetest_project_root.py | 2 +-
Allura/allura/controllers/rest.py | 2 +-
Allura/allura/lib/security.py | 134 ++++++++++-----------
Allura/allura/lib/utils.py | 25 ----
Allura/allura/tests/test_plugin.py | 1 -
Allura/allura/tests/test_utils.py | 21 ----
.../033-change-comment-anon-permissions.py | 2 +-
8 files changed, 73 insertions(+), 120 deletions(-)
diff --git a/Allura/allura/controllers/auth.py
b/Allura/allura/controllers/auth.py
index 1699f6447..23565d98c 100644
--- a/Allura/allura/controllers/auth.py
+++ b/Allura/allura/controllers/auth.py
@@ -523,9 +523,9 @@ class AuthController(BaseController):
log.info("Can't find repo at %s on repo_path %s",
rest[0], repo_path)
return disallow
- return dict(allow_read=bool(has_access(c.app, 'read', user)),
- allow_write=bool(has_access(c.app, 'write', user)),
- allow_create=bool(has_access(c.app, 'create', user)))
+ return dict(allow_read=has_access(c.app, 'read', user),
+ allow_write=has_access(c.app, 'write', user),
+ allow_create=has_access(c.app, 'create', user))
@expose('jinja:allura:templates/pwd_expired.html')
@without_trailing_slash
diff --git a/Allura/allura/controllers/basetest_project_root.py
b/Allura/allura/controllers/basetest_project_root.py
index 90dc7e88c..a110a962f 100644
--- a/Allura/allura/controllers/basetest_project_root.py
+++ b/Allura/allura/controllers/basetest_project_root.py
@@ -181,7 +181,7 @@ class SecurityTest:
@expose()
def needs_project_access_ok(self):
pred = has_access(c.project, 'read')
- if not pred():
+ if not pred:
log.info('Inside needs_project_access, c.user = %s' % c.user)
require(pred)
return ''
diff --git a/Allura/allura/controllers/rest.py
b/Allura/allura/controllers/rest.py
index b42605eca..f5d1400d6 100644
--- a/Allura/allura/controllers/rest.py
+++ b/Allura/allura/controllers/rest.py
@@ -555,7 +555,7 @@ def rest_has_access(obj, user, perm):
resp = {'result': False}
user = M.User.by_username(user)
if user:
- resp['result'] = bool(security.has_access(obj, perm, user=user))
+ resp['result'] = security.has_access(obj, perm, user=user)
return resp
diff --git a/Allura/allura/lib/security.py b/Allura/allura/lib/security.py
index 0fbacf6d6..1db9f19c2 100644
--- a/Allura/allura/lib/security.py
+++ b/Allura/allura/lib/security.py
@@ -33,8 +33,6 @@ from itertools import chain
from ming.utils import LazyProperty
import tg
-from allura.lib.utils import TruthyCallable
-
if typing.TYPE_CHECKING:
from allura.model import M
@@ -306,7 +304,7 @@ def debug_obj(obj) -> str:
return str(obj)
-def has_access(obj, permission: str, user: M.User | None = None, project:
M.Project | None = None) -> TruthyCallable:
+def has_access(obj, permission: str, user: M.User | None = None, project:
M.Project | None = None, roles = None) -> bool:
'''Return whether the given user has the permission name on the given
object.
- First, all the roles for a user in the given project context are
computed.
@@ -349,69 +347,65 @@ def has_access(obj, permission: str, user: M.User | None
= None, project: M.Proj
DEBUG = False
- def predicate(obj=obj, user=user, project=project, roles=None) -> bool:
- if obj is None:
- if DEBUG:
- log.debug(f'{user} denied {permission} on {debug_obj(obj)}
({debug_obj(project)})')
- return False
- if roles is None:
- if user is None:
- user = c.user
- assert user, 'c.user should always be at least M.User.anonymous()'
- cred = Credentials.get()
- if project is None:
- if isinstance(obj, M.Neighborhood):
- project = obj.neighborhood_project
- if project is None:
- log.error('Neighborhood project missing for %s', obj)
- return False
- elif isinstance(obj, M.Project):
- project = obj.root_project
- else:
- project = getattr(obj, 'project', None) or c.project
- project = project.root_project
- roles: RoleCache = cred.user_roles(user_id=user._id,
project_id=project._id).reaching_roles
-
- # TODO: move deny logic into loop below; see ticket [#6715]
- if is_denied(obj, permission, user, project):
- if DEBUG:
- log.debug(f"{user.username} '{permission}' denied on
{debug_obj(obj)} ({debug_obj(project)})")
- return False
-
- chainable_roles = []
- for role in roles:
- for ace in obj.acl:
- if M.ACE.match(ace, role['_id'], permission):
- if ace.access == M.ACE.ALLOW:
- # access is allowed
- if DEBUG:
- log.debug(
- f"{user.username} '{permission}' granted on
{debug_obj(obj)} ({debug_obj(project)})")
- return True
- else:
- # access is denied for this particular role
- if DEBUG:
- log.debug(f"{user.username} '{permission}' denied
for role={role['name'] or role['_id']} (BUT continuing to see if other roles
permit) on {debug_obj(obj)} ({debug_obj(project)})")
- break
+ if obj is None:
+ if DEBUG:
+ log.debug(f'{user} denied {permission} on {debug_obj(obj)}
({debug_obj(project)})')
+ return False
+ if roles is None:
+ if user is None:
+ user = c.user
+ assert user, 'c.user should always be at least M.User.anonymous()'
+ cred = Credentials.get()
+ if project is None:
+ if isinstance(obj, M.Neighborhood):
+ project = obj.neighborhood_project
+ if project is None:
+ log.error('Neighborhood project missing for %s', obj)
+ return False
+ elif isinstance(obj, M.Project):
+ project = obj.root_project
else:
- # access neither allowed or denied, may chain to parent context
- chainable_roles.append(role)
- parent = obj.parent_security_context()
- if parent and chainable_roles:
- result = has_access(parent, permission, user=user,
project=project)(
- roles=tuple(chainable_roles))
- elif not isinstance(obj, M.Neighborhood):
- result = has_access(project.neighborhood, 'admin', user=user)
- if not (result or isinstance(obj, M.Project)):
- result = has_access(project, 'admin', user=user)
- else:
- result = False
- result = bool(result)
+ project = getattr(obj, 'project', None) or c.project
+ project = project.root_project
+ roles: RoleCache = cred.user_roles(user_id=user._id,
project_id=project._id).reaching_roles
+
+ # TODO: move deny logic into loop below; see ticket [#6715]
+ if is_denied(obj, permission, user, project):
if DEBUG:
- log.debug(
- f"{user.username} '{permission}' {result} from parent(s) on
{debug_obj(obj)} ({debug_obj(project)})")
- return result
- return TruthyCallable(predicate)
+ log.debug(f"{user.username} '{permission}' denied on
{debug_obj(obj)} ({debug_obj(project)})")
+ return False
+
+ chainable_roles = []
+ for role in roles:
+ for ace in obj.acl:
+ if M.ACE.match(ace, role['_id'], permission):
+ if ace.access == M.ACE.ALLOW:
+ # access is allowed
+ if DEBUG:
+ log.debug(
+ f"{user.username} '{permission}' granted on
{debug_obj(obj)} ({debug_obj(project)})")
+ return True
+ else:
+ # access is denied for this particular role
+ if DEBUG:
+ log.debug(f"{user.username} '{permission}' denied for
role={role['name'] or role['_id']} (BUT continuing to see if other roles
permit) on {debug_obj(obj)} ({debug_obj(project)})")
+ break
+ else:
+ # access neither allowed or denied, may chain to parent context
+ chainable_roles.append(role)
+ parent = obj.parent_security_context()
+ if parent and chainable_roles:
+ result = has_access(parent, permission, user=user, project=project,
roles=tuple(chainable_roles))
+ elif not isinstance(obj, M.Neighborhood):
+ result = has_access(project.neighborhood, 'admin', user=user)
+ if not (result or isinstance(obj, M.Project)):
+ result = has_access(project, 'admin', user=user)
+ else:
+ result = False
+ if DEBUG:
+ log.debug(
+ f"{user.username} '{permission}' {result} from parent(s) on
{debug_obj(obj)} ({debug_obj(project)})")
+ return result
def all_allowed(obj, user_or_role=None, project=None):
@@ -495,14 +489,20 @@ def require(predicate, message=None):
'''
Example: ``require(has_access(c.app, 'read'))``
- :param callable predicate: truth function to call
+ :param callable|bool predicate: truth function to call, or truth value
:param str message: message to show upon failure
:raises: HTTPForbidden or HTTPUnauthorized
'''
from allura import model as M
- if predicate():
- return
+
+ if callable(predicate):
+ if predicate():
+ return
+ else:
+ if predicate:
+ return
+
if not message:
message = """You don't have permission to do that.
You must ask a project administrator for rights to
perform this task.
diff --git a/Allura/allura/lib/utils.py b/Allura/allura/lib/utils.py
index bba85e618..d025f4f67 100644
--- a/Allura/allura/lib/utils.py
+++ b/Allura/allura/lib/utils.py
@@ -396,31 +396,6 @@ class AntiSpam:
return before_validate(antispam_hook)
-class TruthyCallable:
- '''
- Wraps a callable to make it truthy in a boolean context.
-
- Assumes the callable returns a truthy value and can be called with no args.
- '''
-
- def __init__(self, callable):
- self.callable = callable
-
- def __call__(self, *args, **kw):
- return self.callable(*args, **kw)
-
- def __bool__(self):
- return self.callable()
-
- def __eq__(self, other):
- if other is True and bool(self):
- return True
- elif other is False and not bool(self):
- return True
- else:
- return NotImplemented
-
-
class TransformedDict(MutableMapping):
"""
diff --git a/Allura/allura/tests/test_plugin.py
b/Allura/allura/tests/test_plugin.py
index c57e9e4a4..085e3a143 100644
--- a/Allura/allura/tests/test_plugin.py
+++ b/Allura/allura/tests/test_plugin.py
@@ -30,7 +30,6 @@ from allura import model as M
from allura.lib import plugin
from allura.lib import phone
from allura.lib import helpers as h
-from allura.lib.utils import TruthyCallable
from allura.lib.plugin import ProjectRegistrationProvider
from allura.lib.plugin import ThemeProvider
from allura.lib.exceptions import ProjectConflict, ProjectShortnameInvalid
diff --git a/Allura/allura/tests/test_utils.py
b/Allura/allura/tests/test_utils.py
index 131e9bf95..57b1b830b 100644
--- a/Allura/allura/tests/test_utils.py
+++ b/Allura/allura/tests/test_utils.py
@@ -169,27 +169,6 @@ class TestAntispam(unittest.TestCase):
return encrypted_form
-class TestTruthyCallable(unittest.TestCase):
-
- def test_everything(self):
- def wrapper_func(bool_flag):
- def predicate(bool_flag=bool_flag):
- return bool_flag
- return utils.TruthyCallable(predicate)
- true_predicate = wrapper_func(True)
- false_predicate = wrapper_func(False)
- assert true_predicate(True) is True
- assert false_predicate(False) is False
- assert true_predicate() is True
- assert false_predicate() is False
- assert bool(true_predicate) is True
- assert bool(false_predicate) is False
-
- t, f = True, False # use variables because '== True' would generate
warnings, and we do want '==' not 'is'
- assert true_predicate == t
- assert false_predicate == f
-
-
class TestCaseInsensitiveDict(unittest.TestCase):
def test_everything(self):
diff --git a/scripts/migrations/033-change-comment-anon-permissions.py
b/scripts/migrations/033-change-comment-anon-permissions.py
index 7285c479f..2cfb824d9 100644
--- a/scripts/migrations/033-change-comment-anon-permissions.py
+++ b/scripts/migrations/033-change-comment-anon-permissions.py
@@ -47,7 +47,7 @@ def main():
for chunk in utils.chunked_find(ForumPost, {'app_config_id': tool._id}):
for p in chunk:
- has_access = bool(security.has_access(p, 'moderate',
M.User.anonymous()))
+ has_access = security.has_access(p, 'moderate', M.User.anonymous())
if has_access:
anon_role_id = None
