From 6e93f60f9dc7eaaf6f2daba5c02eafa2af9d5bc7 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 3 May 2024 10:13:14 -0400 Subject: [PATCH 01/13] chore: require Pyramid 2.0 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index a50dca0a..ee5afe9d 100644 --- a/setup.py +++ b/setup.py @@ -24,7 +24,7 @@ README = CHANGES = '' install_requires = [ - 'pyramid>=1.5dev,<2.0dev', # route_name argument to resource_url + 'pyramid>=2.0dev', 'ZODB', 'hypatia>=0.2', # query objects have intersection/union methods 'venusian>=1.0a3', # pyramid wants this too (prefer_finals...) From 2c268c4dde436b69e0792b3b9fddcc40235791e2 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 3 May 2024 09:49:56 -0400 Subject: [PATCH 02/13] tests: skip testing scaffold Until we figure out the replacement. --- pytest.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index 8e2c1f9a..c2861ed9 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,5 +1,5 @@ [pytest] -addopts = -l --strict +addopts = -l --strict --ignore=substanced/scaffolds norecursedirs = lib include .tox .git python_files = test_*.py tests.py filterwarnings = From e7589715e223968f7d4fddd71c7e9fde2ddf8b29 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 3 May 2024 10:03:02 -0400 Subject: [PATCH 03/13] fix: avoid deprecated 'datetime.datetime.utcnow' --- demos/blog/blog/views/retail.py | 4 +--- substanced/content/test_it.py | 2 -- substanced/locking/__init__.py | 3 +-- substanced/locking/tests/test_init.py | 16 ++++++++-------- substanced/util/test_it.py | 2 +- 5 files changed, 11 insertions(+), 16 deletions(-) diff --git a/demos/blog/blog/views/retail.py b/demos/blog/blog/views/retail.py index e8d52522..c3f62de6 100644 --- a/demos/blog/blog/views/retail.py +++ b/demos/blog/blog/views/retail.py @@ -123,9 +123,7 @@ def __init__(self, context, request): self.request = request def _nowtz(self): - now = datetime.datetime.utcnow() # naive - y, mo, d, h, mi, s = now.timetuple()[:6] - return datetime.datetime(y, mo, d, h, mi, s, tzinfo=pytz.utc) + return datetime.datetime.now(datetime.timezone.utc) def _get_feed_info(self): context = self.context diff --git a/substanced/content/test_it.py b/substanced/content/test_it.py index 2368712b..f11923ce 100644 --- a/substanced/content/test_it.py +++ b/substanced/content/test_it.py @@ -24,7 +24,6 @@ def test_add_with_meta(self): def test_create(self): registry = DummyRegistry() inst = self._makeOne(registry) - inst._utcnow = lambda *a: 1 content = testing.DummyResource() inst.content_types['dummy'] = lambda a: content inst.meta['dummy'] = {} @@ -34,7 +33,6 @@ def test_create(self): def test_create_with_oid(self): registry = DummyRegistry() inst = self._makeOne(registry) - inst._utcnow = lambda *a: 1 content = testing.DummyResource() inst.content_types['dummy'] = lambda a: content inst.meta['dummy'] = {} diff --git a/substanced/locking/__init__.py b/substanced/locking/__init__.py index ae57edba..0868dc60 100644 --- a/substanced/locking/__init__.py +++ b/substanced/locking/__init__.py @@ -7,7 +7,6 @@ import datetime import uuid -import pytz from zope.interface import implementer @@ -70,7 +69,7 @@ class UnlockError(LockingError): """ def now(): - return datetime.datetime.utcnow().replace(tzinfo=pytz.UTC) + return datetime.datetime.now(datetime.timezone.utc) class LockOwnerSchema(colander.SchemaNode): title = 'Owner' diff --git a/substanced/locking/tests/test_init.py b/substanced/locking/tests/test_init.py index 0834ba00..69e14aea 100644 --- a/substanced/locking/tests/test_init.py +++ b/substanced/locking/tests/test_init.py @@ -26,9 +26,9 @@ def _callFUT(self): return now() def test_it(self): - from pytz import UTC + import datetime result = self._callFUT() - self.assertEqual(result.tzinfo, UTC) + self.assertIs(result.tzinfo, datetime.timezone.utc) class TestLockOwnerSchema(unittest.TestCase): def _makeOne(self): @@ -212,14 +212,14 @@ def test_ctor(self): def test_refresh(self): import datetime inst = self._makeOne() - now = datetime.datetime.utcnow() + now = datetime.datetime.now(datetime.timezone.utc) inst.refresh(when=now) self.assertEqual(inst.last_refresh, now) def test_refresh_with_timeout(self): import datetime inst = self._makeOne() - now = datetime.datetime.utcnow() + now = datetime.datetime.now(datetime.timezone.utc) inst.refresh(timeout=30, when=now) self.assertEqual(inst.last_refresh, now) self.assertEqual(inst.timeout, 30) @@ -233,7 +233,7 @@ def test_expires_timeout_is_int(self): import datetime inst = self._makeOne() inst.timeout = 30 - now = datetime.datetime.utcnow() + now = datetime.datetime.now(datetime.timezone.utc) inst.last_refresh = now self.assertEqual(inst.expires(), now + datetime.timedelta(seconds=30)) @@ -246,7 +246,7 @@ def test_is_valid_expires_timeout_is_int(self): import datetime inst = self._makeOne() inst.timeout = 30 - now = datetime.datetime.utcnow() + now = datetime.datetime.now(datetime.timezone.utc) future = now + datetime.timedelta(seconds=60) inst.last_refresh = now self.assertTrue(inst.is_valid(now)) @@ -256,7 +256,7 @@ def test_is_valid_expires_resource_id_exists(self): import datetime inst = self._makeOne() inst.timeout = 30 - now = datetime.datetime.utcnow() + now = datetime.datetime.now(datetime.timezone.utc) inst.last_refresh = now inst.__objectmap__ = DummyObjectMap([1]) self.assertTrue(inst.is_valid(now)) @@ -265,7 +265,7 @@ def test_is_valid_expires_resource_id_notexist(self): import datetime inst = self._makeOne() inst.timeout = 30 - now = datetime.datetime.utcnow() + now = datetime.datetime.now(datetime.timezone.utc) inst.last_refresh = now inst.__objectmap__ = DummyObjectMap([]) self.assertFalse(inst.is_valid(now)) diff --git a/substanced/util/test_it.py b/substanced/util/test_it.py index a67cadf3..56a1cc69 100644 --- a/substanced/util/test_it.py +++ b/substanced/util/test_it.py @@ -392,7 +392,7 @@ def _callFUT(self, d): def test_it(self): import calendar import datetime - d = datetime.datetime.utcnow() + d = datetime.datetime.now(datetime.timezone.utc) result = self._callFUT(d) timetime = calendar.timegm(d.timetuple()) val = int(timetime) // 100 From 7d88ab457394ae3024fabbb5f830fc15e795e563 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 3 May 2024 10:15:58 -0400 Subject: [PATCH 04/13] chore: fix deprecated 'pyramid.security' imports --- demos/blog/blog/resources.py | 6 ++---- substanced/dump/__init__.py | 2 +- substanced/dump/test_it.py | 2 +- substanced/objectmap/__init__.py | 2 +- substanced/objectmap/views.py | 2 +- substanced/principal/__init__.py | 4 ++-- substanced/principal/subscribers.py | 2 +- substanced/principal/tests/test_subscribers.py | 2 +- substanced/root/__init__.py | 4 ++-- substanced/sdi/views/acl.py | 4 ++-- substanced/sdi/views/login.py | 4 ++-- substanced/sdi/views/tests/test_acl.py | 12 ++++++------ substanced/workflow/__init__.py | 10 +++++----- substanced/workflow/tests/test_workflow.py | 6 +++--- 14 files changed, 30 insertions(+), 32 deletions(-) diff --git a/demos/blog/blog/resources.py b/demos/blog/blog/resources.py index 575cba9d..a9296fbe 100644 --- a/demos/blog/blog/resources.py +++ b/demos/blog/blog/resources.py @@ -4,10 +4,8 @@ import deform.widget from persistent import Persistent -from pyramid.security import ( - Allow, - Everyone, - ) +from pyramid.authorization import Allow +from pyramid.authorization import Everyone from substanced.content import content from substanced.folder import Folder diff --git a/substanced/dump/__init__.py b/substanced/dump/__init__.py index 52ba81f9..757ce7e2 100644 --- a/substanced/dump/__init__.py +++ b/substanced/dump/__init__.py @@ -9,8 +9,8 @@ alsoProvides, ) from zope.interface.interface import InterfaceClass +from pyramid.authorization import AllPermissionsList, ALL_PERMISSIONS from pyramid.request import Request -from pyramid.security import AllPermissionsList, ALL_PERMISSIONS from pyramid.threadlocal import get_current_registry from pyramid.traversal import resource_path from pyramid.util import ( diff --git a/substanced/dump/test_it.py b/substanced/dump/test_it.py index fe817450..74679507 100644 --- a/substanced/dump/test_it.py +++ b/substanced/dump/test_it.py @@ -476,7 +476,7 @@ def _makeOne(self, name, registry): return ACLDumper(name, registry) def test_init_adds_yaml_stuff(self): - from pyramid.security import ALL_PERMISSIONS + from pyramid.authorization import ALL_PERMISSIONS from . import DUMP_ALL_PERMISSIONS yamlthing = DummyYAMLDumperLoader() registry = {'yaml_loader':yamlthing, 'yaml_dumper':yamlthing} diff --git a/substanced/objectmap/__init__.py b/substanced/objectmap/__init__.py index c857f055..14a05013 100644 --- a/substanced/objectmap/__init__.py +++ b/substanced/objectmap/__init__.py @@ -4,7 +4,7 @@ import BTrees import colander from persistent import Persistent -from pyramid.security import Allow +from pyramid.authorization import Allow from pyramid.traversal import ( resource_path_tuple, find_resource, diff --git a/substanced/objectmap/views.py b/substanced/objectmap/views.py index bb09c3b6..54f096c4 100644 --- a/substanced/objectmap/views.py +++ b/substanced/objectmap/views.py @@ -1,5 +1,5 @@ -from pyramid.view import view_defaults from pyramid.security import NO_PERMISSION_REQUIRED +from pyramid.view import view_defaults from . import ( find_objectmap, diff --git a/substanced/principal/__init__.py b/substanced/principal/__init__.py index 20cd819c..f1769866 100644 --- a/substanced/principal/__init__.py +++ b/substanced/principal/__init__.py @@ -14,11 +14,11 @@ import colander import pytz -from pyramid.renderers import render -from pyramid.security import ( +from pyramid.authorization import ( Allow, Everyone, ) +from pyramid.renderers import render from pyramid.threadlocal import get_current_registry from pyramid_mailer import get_mailer diff --git a/substanced/principal/subscribers.py b/substanced/principal/subscribers.py index 139c5959..184b8eb3 100644 --- a/substanced/principal/subscribers.py +++ b/substanced/principal/subscribers.py @@ -1,4 +1,4 @@ -from pyramid.security import Allow +from pyramid.authorization import Allow from ..event import ( subscribe_added, diff --git a/substanced/principal/tests/test_subscribers.py b/substanced/principal/tests/test_subscribers.py index b8ac6c26..93096a06 100644 --- a/substanced/principal/tests/test_subscribers.py +++ b/substanced/principal/tests/test_subscribers.py @@ -115,7 +115,7 @@ def test_it_user_has_no_oid(self): self.assertRaises(AttributeError, self._callFUT, event) def test_it(self): - from pyramid.security import Allow + from pyramid.authorization import Allow user = testing.DummyResource() user.__oid__ = 1 event = testing.DummyResource(object=user, loading=False) diff --git a/substanced/root/__init__.py b/substanced/root/__init__.py index b2604d61..fbaecef2 100644 --- a/substanced/root/__init__.py +++ b/substanced/root/__init__.py @@ -1,11 +1,11 @@ import colander from zope.interface import implementer -from pyramid.exceptions import ConfigurationError -from pyramid.security import ( +from pyramid.authorization import ( Allow, ALL_PERMISSIONS, ) +from pyramid.exceptions import ConfigurationError from ..interfaces import IRoot diff --git a/substanced/sdi/views/acl.py b/substanced/sdi/views/acl.py index daee6340..28c544e4 100644 --- a/substanced/sdi/views/acl.py +++ b/substanced/sdi/views/acl.py @@ -1,7 +1,6 @@ import logging -from pyramid.security import ( - NO_PERMISSION_REQUIRED, +from pyramid.authorization import ( ALL_PERMISSIONS, DENY_ALL, Deny, @@ -9,6 +8,7 @@ Authenticated, ) from pyramid.csrf import check_csrf_token +from pyramid.security import NO_PERMISSION_REQUIRED from pyramid.view import view_defaults from pyramid.location import lineage diff --git a/substanced/sdi/views/login.py b/substanced/sdi/views/login.py index edfd010c..2d24d176 100644 --- a/substanced/sdi/views/login.py +++ b/substanced/sdi/views/login.py @@ -1,13 +1,13 @@ +from pyramid.authorization import Authenticated +from pyramid.csrf import check_csrf_token from pyramid.httpexceptions import ( HTTPForbidden, HTTPFound ) from pyramid.renderers import get_renderer -from pyramid.csrf import check_csrf_token from pyramid.security import ( remember, forget, - Authenticated, NO_PERMISSION_REQUIRED, ) diff --git a/substanced/sdi/views/tests/test_acl.py b/substanced/sdi/views/tests/test_acl.py index f8837300..c10ec0f8 100644 --- a/substanced/sdi/views/tests/test_acl.py +++ b/substanced/sdi/views/tests/test_acl.py @@ -383,7 +383,7 @@ def test_add_no_permission(self): self.assertEqual(request.sdiapi.flashed, 'New ACE added') def test_add_all_permissions(self): - from pyramid.security import ALL_PERMISSIONS + from pyramid.authorization import ALL_PERMISSIONS from ....testing import make_site request = testing.DummyRequest() request.sdiapi = DummySDIAPI() @@ -409,7 +409,7 @@ def test_add_all_permissions(self): self.assertEqual(request.sdiapi.flashed, 'New ACE added') def test_add_Everyone(self): - from pyramid.security import Everyone + from pyramid.authorization import Everyone from ....testing import make_site request = testing.DummyRequest() request.sdiapi = DummySDIAPI() @@ -547,7 +547,7 @@ def test_get_local_acl_with_no_inherit(self): self.assertEqual(result, ('disabled', [])) def test_get_local_acl_with_all_permissions(self): - from pyramid.security import ALL_PERMISSIONS, Allow + from pyramid.authorization import ALL_PERMISSIONS, Allow request = testing.DummyRequest() site = self._makeSite() user = testing.DummyResource() @@ -577,7 +577,7 @@ def test_get_parent_acl_with_no_inherit(self): self.assertEqual(result, []) def test_get_parent_acl_with_noniter_permission(self): - from pyramid.security import Allow + from pyramid.authorization import Allow request = testing.DummyRequest() context = testing.DummyResource() context.__acl__ = [(Allow, 1, 'edit')] @@ -590,7 +590,7 @@ def test_get_parent_acl_with_noniter_permission(self): self.assertEqual(result, [(Allow, 'fred', ('edit',))]) def test_get_parent_acl_with_all_permissions(self): - from pyramid.security import Allow, ALL_PERMISSIONS + from pyramid.authorization import Allow, ALL_PERMISSIONS request = testing.DummyRequest() context = testing.DummyResource() context.__acl__ = [ (Allow, 1, ALL_PERMISSIONS) ] @@ -612,7 +612,7 @@ def test_get_principal_name_deleted_principal(self): self.assertEqual(result, '') def test_get_principal_name_Everyone(self): - from pyramid.security import Everyone + from pyramid.authorization import Everyone request = testing.DummyRequest() context = testing.DummyResource() context.__parent__ = None diff --git a/substanced/workflow/__init__.py b/substanced/workflow/__init__.py index 7bf4c82b..0a796c69 100644 --- a/substanced/workflow/__init__.py +++ b/substanced/workflow/__init__.py @@ -2,12 +2,12 @@ from persistent.mapping import PersistentMapping +from pyramid.authorization import ALL_PERMISSIONS +from pyramid.authorization import Allow +from pyramid.authorization import Authenticated +from pyramid.authorization import DENY_ALL +from pyramid.authorization import Everyone from pyramid.config import ConfigurationError -from pyramid.security import ALL_PERMISSIONS -from pyramid.security import Allow -from pyramid.security import Authenticated -from pyramid.security import DENY_ALL -from pyramid.security import Everyone from zope.interface import implementer from ..event import AfterTransition diff --git a/substanced/workflow/tests/test_workflow.py b/substanced/workflow/tests/test_workflow.py index 867c0fd1..b6d3a4b9 100644 --- a/substanced/workflow/tests/test_workflow.py +++ b/substanced/workflow/tests/test_workflow.py @@ -1100,9 +1100,9 @@ def test___call___wo_acl(self): state(object(), request={}, transition='dummy', workflow=object()) def test___call___w_acl(self): - from pyramid.security import Allow - from pyramid.security import Everyone - from pyramid.security import ALL_PERMISSIONS + from pyramid.authorization import Allow + from pyramid.authorization import Everyone + from pyramid.authorization import ALL_PERMISSIONS class _Content(object): pass content = _Content() From 6c5afdc2dba870632498280ad07f46d1cfa6a0c8 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 3 May 2024 10:16:58 -0400 Subject: [PATCH 05/13] chore: silence deprecation spew from chameleon/webob --- pytest.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pytest.ini b/pytest.ini index c2861ed9..fdaefa07 100644 --- a/pytest.ini +++ b/pytest.ini @@ -3,5 +3,7 @@ addopts = -l --strict --ignore=substanced/scaffolds norecursedirs = lib include .tox .git python_files = test_*.py tests.py filterwarnings = + ignore::DeprecationWarning:chameleon ignore::DeprecationWarning:pkg_resources + ignore::DeprecationWarning:webob ignore::DeprecationWarning:zodburi From 772c42760854352ffa2aa0df10155c190fcaa75c Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 3 May 2024 10:50:38 -0400 Subject: [PATCH 06/13] fix: use pyramid2-style security policy --- substanced/sdi/__init__.py | 89 ++++++++++++++++++++----- substanced/sdi/views/tests/test_undo.py | 20 ++++-- 2 files changed, 85 insertions(+), 24 deletions(-) diff --git a/substanced/sdi/__init__.py b/substanced/sdi/__init__.py index cc307ca5..0197f201 100644 --- a/substanced/sdi/__init__.py +++ b/substanced/sdi/__init__.py @@ -13,8 +13,10 @@ import venusian -from pyramid.authentication import AuthTktAuthenticationPolicy -from pyramid.authorization import ACLAuthorizationPolicy +from pyramid.authentication import AuthTktCookieHelper +from pyramid.authorization import ACLHelper +from pyramid.authorization import Authenticated +from pyramid.authorization import Everyone from pyramid.decorator import reify from pyramid.exceptions import ConfigurationError from pyramid.location import lineage @@ -41,6 +43,7 @@ from ..interfaces import IUserLocator from ..interfaces import ISDIAPI from ..principal import DefaultUserLocator +from ..principal import groupfinder try: # pyramid 1.9 and below @@ -541,6 +544,70 @@ def _bwcompat_kw(kw): kw[name] = val return kw + +class SubstancedSecurityPolicy: + """Pyramid 2.0 replacement for authn / authz policies + + See: + https://docs.pylonsproject.org/projects/pyramid/en/2.0-branch + /whatsnew-2.0.html#upgrading-from-built-in-policies + + TODO: Is this verbiage still correct? Useful? + + NB: we use the AuthTktCookieHelper rather than the session + version because using the session versionic + resources to be uncacheable. In particular, if you use the + UnencryptedBlahBlahBlah session factory, and anything asks for the + authenticated or unauthenticated user from the policy, the session needs + to be reserialized because that factory works by resetting the cookie on + every access to set the last-accessed value. In practice, pyramid_tm + asks for the unauthenticated user, so every static resource will have a + set-cookie header in it, making them uncacheable. This could also be + solved by using a different session factory (e.g. pyramid_redis_sessions) + which does not reserialize the cookie on every access. + """ + def __init__(self, secret): + self._helper = AuthTktCookieHelper(secret) + + def identity(self, request): + identity = self._helper.identify(request) + + if identity is None: + return None + + userid = identity['userid'] + principals = groupfinder(userid, request) + + if principals is not None: + return { + 'userid': userid, + 'principals': principals, + } + + def authenticated_userid(self, request): + identity = request.identity + + if identity is not None: + return identity['userid'] + + def permits(self, request, context, permission): + principals = set([Everyone]) + identity = request.identity + + if identity is not None: + principals.add(Authenticated) + principals.add(identity['userid']) + principals.update(identity['principals']) + + return ACLHelper().permits(context, principals, permission) + + def remember(self, request, userid, **kw): + return self._helper.remember(request, userid, **kw) + + def forget(self, request, **kw): + return self._helper.forget(request, **kw) + + def includeme(config): # pragma: no cover settings = config.registry.settings YEAR = 86400 * 365 @@ -566,20 +633,6 @@ def includeme(config): # pragma: no cover 'You must set a substanced.secret key in your .ini file') session_factory = SignedCookieSessionFactory(secret) config.set_session_factory(session_factory) - from ..principal import groupfinder - # NB: we use the AuthTktAuthenticationPolicy rather than the session - # authentication policy because using the session policy can cause static - # resources to be uncacheable. In particular, if you use the - # UnencryptedBlahBlahBlah session factory, and anything asks for the - # authenticated or unauthenticated user from the policy, the session needs - # to be reserialized because that factory works by resetting the cookie on - # every access to set the last-accessed value. In practice, pyramid_tm - # asks for the unauthenticated user, so every static resource will have a - # set-cookie header in it, making them uncacheable. This could also be - # solved by using a different session factory (e.g. pyramid_redis_sessions) - # which does not reserialize the cookie on every access. - authn_policy = AuthTktAuthenticationPolicy(secret, callback=groupfinder) - authz_policy = ACLAuthorizationPolicy() - config.set_authentication_policy(authn_policy) - config.set_authorization_policy(authz_policy) + sec_policy = SubstancedSecurityPolicy(secret) + config.set_security_policy(sec_policy) config.add_permission('sdi.edit-properties') # used by property machinery diff --git a/substanced/sdi/views/tests/test_undo.py b/substanced/sdi/views/tests/test_undo.py index f390f9c5..a3b8a67a 100644 --- a/substanced/sdi/views/tests/test_undo.py +++ b/substanced/sdi/views/tests/test_undo.py @@ -1,4 +1,6 @@ import unittest +from unittest import mock + from pyramid import testing class TestUndoViews(unittest.TestCase): @@ -175,9 +177,7 @@ def find_objectmap(ctx): def test_undo_multiple(self): import binascii - sec_pol = testing.DummySecurityPolicy(userid="phred") request = testing.DummyRequest() - request._get_authentication_policy = lambda: sec_pol context = testing.DummyResource() inst = self._makeOne(context, request) conn = DummyConnection() @@ -197,7 +197,12 @@ def getall(n): request.sdiapi = DummySDIAPI() txn = DummyTransaction() inst.transaction = txn - result = inst.undo_multiple() + + sec_pol = testing.DummySecurityPolicy(userid="phred") + with mock.patch("pyramid.security._get_security_policy") as gsp: + gsp.return_value = sec_pol + result = inst.undo_multiple() + self.assertEqual(result.location, '/mgmt_path') self.assertEqual(conn._db.tids, [b'a', b'b']) self.assertTrue(txn.committed) @@ -205,9 +210,7 @@ def getall(n): def test_undo_multiple_with_text_in_POST(self): import binascii - sec_pol = testing.DummySecurityPolicy(userid="phred") request = testing.DummyRequest() - request._get_authentication_policy = lambda: sec_pol context = testing.DummyResource() inst = self._makeOne(context, request) conn = DummyConnection() @@ -227,7 +230,12 @@ def getall(n): request.sdiapi = DummySDIAPI() txn = DummyTransaction() inst.transaction = txn - result = inst.undo_multiple() + + sec_pol = testing.DummySecurityPolicy(userid="phred") + with mock.patch("pyramid.security._get_security_policy") as gsp: + gsp.return_value = sec_pol + result = inst.undo_multiple() + self.assertEqual(result.location, '/mgmt_path') self.assertEqual(conn._db.tids, [b'a', b'b']) self.assertTrue(txn.committed) From 7c1a608071a7a0cb025a4ec4adc66471b57b7403 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 3 May 2024 11:41:53 -0400 Subject: [PATCH 07/13] chore: compute 'effective_principals' directly Pyramid 2.0 no longer provides that API. --- docs/cataloging.rst | 10 +++++++++- substanced/catalog/indexes.py | 16 +++++++++++++++- substanced/catalog/tests/test_indexes.py | 9 ++++++++- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/docs/cataloging.rst b/docs/cataloging.rst index ca6a509e..ec8f80ce 100644 --- a/docs/cataloging.rst +++ b/docs/cataloging.rst @@ -349,6 +349,10 @@ It is possible to postfilter catalog results using the .. code-block:: python + from pyramid.authorization import Everyone + + ... + def get_allowed_to_view(context, request): catalog = find_catalog(context, 'system') @@ -356,8 +360,12 @@ It is possible to postfilter catalog results using the resultset = q.execute() objectmap = find_objectmap(context) + identity = request.identity + effective_principals = [ + Everyone, identity["userid"] + ] + identity["principals"] return objectmap.allowed( - resultset.oids, request.effective_principals, 'view') + resultset.oids, effective_principals, 'view') The result of :meth:`~substanced.objectmap.ObjectMap.allowed` is a generator which returns oids, so the result must be listified if you intend to index into diff --git a/substanced/catalog/indexes.py b/substanced/catalog/indexes.py index 30916ce4..e93439fd 100644 --- a/substanced/catalog/indexes.py +++ b/substanced/catalog/indexes.py @@ -13,6 +13,7 @@ import hypatia.text import hypatia.util from persistent import Persistent +from pyramid.authorization import Everyone from pyramid.settings import asbool from pyramid.traversal import resource_path_tuple from pyramid.interfaces import IRequest @@ -408,6 +409,18 @@ def __init__(self, discriminator, family=None): def document_repr(self, docid, default=None): return 'N/A' + def _effective_principals(self, request): + identity = request.identity + if identity is None: + return [Everyone] + if isinstance(identity, dict): # assume our security policy + return [ + Everyone, + identity["userid"] + ] + identity["principals"] + else: + return [Everyone, identity] + def allows(self, principals, permission): """ ``principals`` may either be 1) a sequence of principal indentifiers, 2) a single principal identifier, or 3) a Pyramid @@ -417,9 +430,10 @@ def allows(self, principals, permission): ``permission`` must be a permission name. """ if IRequest.providedBy(principals): - principals = principals.effective_principals + principals = self._effective_principals(principals) elif not is_nonstr_iter(principals): principals = (principals,) + return AllowsComparator(self, (principals, permission)) class AllowsComparator(hypatia.query.Comparator): diff --git a/substanced/catalog/tests/test_indexes.py b/substanced/catalog/tests/test_indexes.py index b4b106dc..e31f6827 100644 --- a/substanced/catalog/tests/test_indexes.py +++ b/substanced/catalog/tests/test_indexes.py @@ -1,4 +1,6 @@ import unittest +from unittest import mock + from pyramid import testing import BTrees @@ -619,7 +621,12 @@ def test_document_repr(self): def test_allows_request(self): index = self._makeOne(None) request = testing.DummyRequest() - q = index.allows(request, 'edit') + + sec_pol = testing.DummySecurityPolicy() + with mock.patch("pyramid.security._get_security_policy") as gsp: + gsp.return_value = sec_pol + q = index.allows(request, 'edit') + self.assertEqual(q._value, (['system.Everyone'], 'edit')) def test_allows_iterable(self): From 84e0394b87c45356bfefbc68e6a29f2ae906c4ec Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 3 May 2024 11:43:16 -0400 Subject: [PATCH 08/13] chore: suppress moar deprectation spew --- pytest.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pytest.ini b/pytest.ini index fdaefa07..7c18ab73 100644 --- a/pytest.ini +++ b/pytest.ini @@ -3,7 +3,9 @@ addopts = -l --strict --ignore=substanced/scaffolds norecursedirs = lib include .tox .git python_files = test_*.py tests.py filterwarnings = + ignore::DeprecationWarning:ast ignore::DeprecationWarning:chameleon + ignore::DeprecationWarning:hypatia ignore::DeprecationWarning:pkg_resources ignore::DeprecationWarning:webob ignore::DeprecationWarning:zodburi From 9a74d423d9396ae1e01f5f99d7550660d13c5046 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 3 May 2024 11:59:34 -0400 Subject: [PATCH 09/13] chore: remove trailing ws --- substanced/sdi/tests/test_sdi.py | 54 ++++++++++++++++---------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/substanced/sdi/tests/test_sdi.py b/substanced/sdi/tests/test_sdi.py index 2064c320..253c99d5 100644 --- a/substanced/sdi/tests/test_sdi.py +++ b/substanced/sdi/tests/test_sdi.py @@ -104,7 +104,7 @@ def test_with_tab_near_middle(self): self.assertEqual(config._intr['tab_before'], CENTER2) self.assertEqual(config._intr['tab_after'], CENTER1) self.assertEqual(config._intr['tab_near'], MIDDLE) - + def test_with_tab_near_right(self): from .. import RIGHT, CENTER2, LAST config = self._makeConfig() @@ -159,7 +159,7 @@ def test_create_nondefaults(self): self.assertEqual(decorator.mapper, 'mapper') self.assertEqual(decorator.decorator, 'decorator') self.assertEqual(decorator.match_param, 'match_param') - + def test_call_function(self): decorator = self._makeOne() venusian = DummyVenusian() @@ -204,7 +204,7 @@ def setUp(self): def tearDown(self): testing.tearDown() - + def _callFUT(self, context, request, names=None): from .. import sdi_mgmt_views return sdi_mgmt_views(context, request, names) @@ -607,7 +607,7 @@ def test_gardenpath_with_tab_before_and_after(self): intr4['tab_condition'] = None intr4['tab_before'] = CENTER2 intr4['tab_after'] = CENTER1 - + intr = DummyIntrospectable(related=(view_intr1,), introspectable=intr) intr2 = DummyIntrospectable(related=(view_intr2,), introspectable=intr2) intr3 = DummyIntrospectable(related=(view_intr3,), introspectable=intr3) @@ -790,7 +790,7 @@ class MoreDirectContext(object): context = testing.DummyResource() result = self._callFUT(context, request) self.assertEqual(len(result), 0) - + class Test_default_sdi_addable(unittest.TestCase): def _callFUT(self, context, intr): from .. import default_sdi_addable @@ -800,12 +800,12 @@ def test_is_service_with_service_name_in_context(self): context = {'catalog':True} intr = {'meta':{'is_service':True, 'service_name':'catalog'}} self.assertFalse(self._callFUT(context, intr)) - + def test_is_service_with_service_name_not_in_context(self): context = {} intr = {'meta':{'is_service':True, 'service_name':'catalog'}} self.assertTrue(self._callFUT(context, intr)) - + def test_is_service_without_service_name(self): context = {'catalog':True} intr = {'meta':{'is_service':True}} @@ -850,7 +850,7 @@ def setUp(self): def tearDown(self): testing.tearDown() - + def _makeOne(self, request): from .. import sdiapi return sdiapi(request) @@ -922,7 +922,7 @@ def test_flash_error_converted_to_danger(self): inst = self._makeOne(request) inst.flash('message', 'error') self.assertEqual(request.session['_f_danger'], ['message']) - + def test_mgmt_path(self): from .. import MANAGE_ROUTE_NAME request = testing.DummyRequest() @@ -948,7 +948,7 @@ def resource_path(resource, *arg, **kw): inst = self._makeOne(request) result = inst.mgmt_path(context, 'a', b=1, route_name=route_name) self.assertEqual(result, '/path') - + def test_mgmt_url(self): from .. import MANAGE_ROUTE_NAME request = testing.DummyRequest() @@ -974,7 +974,7 @@ def resource_url(resource, *arg, **kw): inst = self._makeOne(request) result = inst.mgmt_url(context, 'a', b=1, route_name=route_name) self.assertEqual(result, 'http://example.com/path') - + def test_breadcrumbs_no_permissions(self): self.config.testing_securitypolicy(permissive=False) resource = testing.DummyResource() @@ -983,7 +983,7 @@ def test_breadcrumbs_no_permissions(self): inst = self._makeOne(request) result = inst.breadcrumbs() self.assertEqual(result, []) - + def test_breadcrumbs_with_permissions(self): self.config.testing_securitypolicy(permissive=True) resource = testing.DummyResource() @@ -1041,7 +1041,7 @@ def test_breadcrumbs_with_vroot(self): 'content_type':'Type', 'icon': None}] ) - + def test_sdi_title_exists(self): resource = testing.DummyResource() resource.sdi_title = 'My Title' @@ -1074,7 +1074,7 @@ def test_get_macro_without_name(self): config.include('pyramid_chameleon') macro = inst.get_macro('substanced.sdi.views:templates/master.pt') self.assertTrue(macro.macros) - + def test_get_macro_with_name(self): request = testing.DummyRequest() inst = self._makeOne(request) @@ -1090,7 +1090,7 @@ def test_is_mgmt_true(self): request.matched_route.name = 'substanced_manage' inst = self._makeOne(request) self.assertTrue(inst.is_mgmt()) - + def test_is_mgmt_false_wrong_name(self): request = testing.DummyRequest() request.matched_route = Dummy() @@ -1102,7 +1102,7 @@ def test_is_mgmt_false_missing_matched_route(self): request = testing.DummyRequest() inst = self._makeOne(request) self.assertFalse(inst.is_mgmt()) - + class Test_mgmt_path(unittest.TestCase): def _callFUT(self, *arg, **kw): from .. import mgmt_path @@ -1162,12 +1162,12 @@ def test_call_with_all(self): 'port':'port' } ) - + class DummyContent(object): def __init__(self, **kw): self.__dict__.update(kw) - + def metadata(self, context, name, default=None): return getattr(self, name, default) @@ -1177,7 +1177,7 @@ def typeof(self, resource): class DummyIntrospector(object): def __init__(self, results=()): self.results = list(results) - + def get_category(self, *arg): if self.results: return self.results.pop(0) @@ -1189,13 +1189,13 @@ class DummyVenusianInfo(object): module = None def __init__(self, **kw): self.__dict__.update(kw) - + class DummyVenusian(object): def __init__(self, info=None): if info is None: info = DummyVenusianInfo() self.info = info - + def attach(self, wrapped, callback, category): self.wrapped = wrapped self.callback = callback @@ -1216,7 +1216,7 @@ def __init__(self): def object_description(self, ob): return ob - + def maybe_dotted(self, thing): return thing @@ -1236,12 +1236,12 @@ def introspectable(self, category, discrim, desc, name): def action(self, discriminator, introspectables): self._actions.append((discriminator, introspectables)) - + class DummyIntrospectable(dict): def __init__(self, **kw): dict.__init__(self, **kw) self.related = {} - + def relate(self, category, discrim): self.related[category] = discrim @@ -1268,7 +1268,7 @@ def db(self): class DummyTransaction(object): def __init__(self): self.notes = [] - + def get(self): return self @@ -1281,7 +1281,7 @@ def setExtendedInfo(self, name, value): class DummySDIAPI(object): def __init__(self, result=None): self.result = result - + def mgmt_path(self, obj, *arg, **kw): return self.result or '/mgmt_path' @@ -1290,4 +1290,4 @@ def mgmt_url(self, obj, *arg, **kw): def flash_with_undo(self, val): self.flashed = val - + From 7fba9f9780b54c160fde3f0c40dd5931632b968b Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 3 May 2024 12:53:19 -0400 Subject: [PATCH 10/13] fix: further ignore 'substanced.scaffold' --- .coveragerc | 1 + 1 file changed, 1 insertion(+) diff --git a/.coveragerc b/.coveragerc index 16dd819f..e3987953 100644 --- a/.coveragerc +++ b/.coveragerc @@ -3,6 +3,7 @@ omit = substanced/*/evolve.py substanced/evolution/evolve*.py substanced/scripts/*.py + substanced/scaffolds/*.py [coverage:report] show_missing = true From d094428444609929e48233077d6eda2640a90354 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 3 May 2024 12:53:50 -0400 Subject: [PATCH 11/13] tests: restore coverage to 100% --- substanced/catalog/tests/test_indexes.py | 38 ++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/substanced/catalog/tests/test_indexes.py b/substanced/catalog/tests/test_indexes.py index e31f6827..965cfb83 100644 --- a/substanced/catalog/tests/test_indexes.py +++ b/substanced/catalog/tests/test_indexes.py @@ -618,6 +618,44 @@ def test_document_repr(self): index = self._makeOne(None) self.assertEqual(index.document_repr(None), 'N/A') + def test__effective_principals_wo_identity(self): + index = self._makeOne(None) + request = testing.DummyRequest() + + sec_pol = testing.DummySecurityPolicy() + with mock.patch("pyramid.security._get_security_policy") as gsp: + gsp.return_value = sec_pol + principals = index._effective_principals(request) + + self.assertEqual(principals, ['system.Everyone']) + + def test__effective_principals_w_identity_as_dict(self): + index = self._makeOne(None) + request = testing.DummyRequest() + + identity = { + "userid": "phred", + "principals": ["buffaloes"], + } + sec_pol = testing.DummySecurityPolicy(identity=identity) + with mock.patch("pyramid.security._get_security_policy") as gsp: + gsp.return_value = sec_pol + principals = index._effective_principals(request) + + self.assertEqual(principals, ['system.Everyone', 'phred', 'buffaloes']) + + def test__effective_principals_w_identity_as_other(self): + index = self._makeOne(None) + request = testing.DummyRequest() + + identity = 'phred' + sec_pol = testing.DummySecurityPolicy(identity=identity) + with mock.patch("pyramid.security._get_security_policy") as gsp: + gsp.return_value = sec_pol + principals = index._effective_principals(request) + + self.assertEqual(principals, ['system.Everyone', 'phred']) + def test_allows_request(self): index = self._makeOne(None) request = testing.DummyRequest() From 471318ba6fc31e4c2c2e1e502a47f2f8e3980793 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 3 May 2024 12:56:45 -0400 Subject: [PATCH 12/13] refactor: make new security policy more uniform/useful Add test coverage. --- substanced/sdi/__init__.py | 26 +++-- substanced/sdi/tests/test_sdi.py | 179 +++++++++++++++++++++++++++++++ 2 files changed, 191 insertions(+), 14 deletions(-) diff --git a/substanced/sdi/__init__.py b/substanced/sdi/__init__.py index 0197f201..2fda05ee 100644 --- a/substanced/sdi/__init__.py +++ b/substanced/sdi/__init__.py @@ -572,27 +572,23 @@ def __init__(self, secret): def identity(self, request): identity = self._helper.identify(request) - if identity is None: - return None - - userid = identity['userid'] - principals = groupfinder(userid, request) - - if principals is not None: + if identity is not None: + userid = identity['userid'] + principals = groupfinder(userid, request) return { 'userid': userid, - 'principals': principals, + 'principals': principals or (), } def authenticated_userid(self, request): - identity = request.identity + identity = self.identity(request) if identity is not None: return identity['userid'] def permits(self, request, context, permission): principals = set([Everyone]) - identity = request.identity + identity = self.identity(request) if identity is not None: principals.add(Authenticated) @@ -601,11 +597,13 @@ def permits(self, request, context, permission): return ACLHelper().permits(context, principals, permission) - def remember(self, request, userid, **kw): - return self._helper.remember(request, userid, **kw) + def remember(self, request, userid, max_age=None, tokens=()): + return self._helper.remember( + request, userid, max_age=max_age, tokens=tokens, + ) - def forget(self, request, **kw): - return self._helper.forget(request, **kw) + def forget(self, request): + return self._helper.forget(request) def includeme(config): # pragma: no cover diff --git a/substanced/sdi/tests/test_sdi.py b/substanced/sdi/tests/test_sdi.py index 253c99d5..3e26af71 100644 --- a/substanced/sdi/tests/test_sdi.py +++ b/substanced/sdi/tests/test_sdi.py @@ -1,6 +1,15 @@ import unittest +from unittest import mock + +from pyramid.authentication import AuthTktCookieHelper +from pyramid.authorization import ACLHelper +from pyramid.authorization import Authenticated +from pyramid.authorization import Everyone from pyramid import testing + +SECRET = "seekr1t" + class Test_add_mgmt_view(unittest.TestCase): def _callFUT(self, config, **kw): from .. import add_mgmt_view @@ -1163,6 +1172,176 @@ def test_call_with_all(self): } ) +class TestSubstancedSecurityPolicy(unittest.TestCase): + + helper_klass = mock.create_autospec(AuthTktCookieHelper, instance=False) + + def _getTargetClass(self): + from .. import SubstancedSecurityPolicy + return SubstancedSecurityPolicy + + def _makeOne(self, *arg, **kw): + self.helper_klass.reset_mock() + self.helper.reset_mock() + + with mock.patch( + "substanced.sdi.AuthTktCookieHelper", self.helper_klass, + ): + return self._getTargetClass()(*arg, secret=SECRET, **kw) + + @property + def helper(self): + return self.helper_klass.return_value + + def test___init__(self): + policy = self._makeOne() + + assert policy._helper is self.helper_klass.return_value + self.helper_klass.assert_called_once_with(SECRET) + + def test_identify_w_none(self): + self.helper.identify.return_value = None + policy = self._makeOne() + request = testing.DummyRequest() + + identity = policy.identity(request) + + self.assertIsNone(identity) + self.helper.identify.assert_called_once_with(request) + + def test_identify_w_userid_wo_groups(self): + self.helper.identify.return_value = {"userid": "phred"} + policy = self._makeOne() + request = testing.DummyRequest() + + with mock.patch("substanced.sdi.groupfinder") as gf: + gf.return_value = None + identity = policy.identity(request) + + self.assertEqual(identity, { + "userid": "phred", + "principals": (), + }) + self.helper.identify.assert_called_once_with(request) + + def test_identify_w_userid_and_groups(self): + self.helper.identify.return_value = {"userid": "phred"} + policy = self._makeOne() + request = testing.DummyRequest() + + with mock.patch("substanced.sdi.groupfinder") as gf: + gf.return_value = ["buffaloes"] + identity = policy.identity(request) + + self.assertEqual(identity, { + "userid": "phred", + "principals": ["buffaloes"], + }) + self.helper.identify.assert_called_once_with(request) + + def test_authenticated_userid_wo_identity(self): + self.helper.identify.return_value = None + policy = self._makeOne() + request = testing.DummyRequest() + + auid = policy.authenticated_userid(request) + + self.assertIsNone(auid) + self.helper.identify.assert_called_once_with(request) + + def test_authenticated_w_identity(self): + self.helper.identify.return_value = {"userid": "phred"} + policy = self._makeOne() + request = testing.DummyRequest() + + auid = policy.authenticated_userid(request) + + self.assertEqual(auid, "phred") + self.helper.identify.assert_called_once_with(request) + + def test_permits_wo_identity(self): + self.helper.identify.return_value = None + policy = self._makeOne() + request = testing.DummyRequest() + context = testing.DummyResource() + + aclh_klass = mock.create_autospec(ACLHelper) + aclh = aclh_klass.return_value + + with mock.patch("substanced.sdi.ACLHelper", aclh_klass): + auid = policy.permits(request, context, "testing") + + self.assertIs(auid, aclh.permits.return_value) + expected_principals = {Everyone} + aclh.permits.assert_called_once_with( + context, expected_principals, "testing", + ) + self.helper.identify.assert_called_once_with(request) + + def test_permits_w_identity(self): + self.helper.identify.return_value = {"userid": "phred"} + policy = self._makeOne() + request = testing.DummyRequest() + context = testing.DummyResource() + + gf = mock.Mock(spec_set=(), return_value = ["buffaloes"]) + aclh_klass = mock.create_autospec(ACLHelper) + aclh = aclh_klass.return_value + + with mock.patch.multiple( + "substanced.sdi", groupfinder=gf, ACLHelper=aclh_klass, + ): + auid = policy.permits(request, context, "testing") + + self.assertIs(auid, aclh.permits.return_value) + expected_principals = {Everyone, Authenticated, "phred", "buffaloes"} + aclh.permits.assert_called_once_with( + context, expected_principals, "testing", + ) + self.helper.identify.assert_called_once_with(request) + + def test_remember_wo_kw(self): + policy = self._makeOne() + request = testing.DummyRequest() + + result = policy.remember(request, "phred") + + self.assertIs(result, self.helper.remember.return_value) + self.helper.remember.assert_called_once_with( + request, "phred", max_age=None, tokens=(), + ) + + def test_remember_w_max_age(self): + policy = self._makeOne() + request = testing.DummyRequest() + + result = policy.remember(request, "phred", max_age=3600) + + self.assertIs(result, self.helper.remember.return_value) + self.helper.remember.assert_called_once_with( + request, "phred", max_age=3600, tokens=(), + ) + + def test_remember_w_tokens(self): + policy = self._makeOne() + request = testing.DummyRequest() + + result = policy.remember(request, "phred", tokens=("foo", "bar")) + + self.assertIs(result, self.helper.remember.return_value) + self.helper.remember.assert_called_once_with( + request, "phred", max_age=None, tokens=("foo", "bar"), + ) + + def test_forget(self): # AuthTktCookieHelper.forget takes no kwargs + policy = self._makeOne() + request = testing.DummyRequest() + + result = policy.forget(request) + + self.assertIs(result, self.helper.forget.return_value) + self.helper.forget.assert_called_once_with(request) + class DummyContent(object): def __init__(self, **kw): From 8f8131c1f3355eaaea053be410659417413b4241 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Thu, 29 Aug 2024 18:55:35 -0400 Subject: [PATCH 13/13] chore: changelog entry for this branch --- CHANGES.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index b75afa82..afab2ae7 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,9 @@ 1.0b1 (unreleased) ================== +- Update to use Pyramid >= 2.0. + See https://github.com/Pylons/substanced/pull/308 + - Drop old Python versions (< 3.8), add newer ones (up to 3.12). - Remove all the "straddle" compatibility stuff FBO Python 2.7 - Pin pyramid < 2.0dev, until it can be reviewed in depth.