From 01351f140ef4092fb4de668bbcb9ac15f8387dbd Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Fri, 13 Nov 2015 12:55:23 +0100 Subject: [PATCH 1/5] Fix Pyramid Transaction Manager setup --- cliquet/__init__.py | 3 ++- cliquet/storage/postgresql/client.py | 8 ++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/cliquet/__init__.py b/cliquet/__init__.py index 9e021b7f7..42c54c5fa 100644 --- a/cliquet/__init__.py +++ b/cliquet/__init__.py @@ -112,7 +112,8 @@ def includeme(config): config.include("cornice") # Per-request transaction. - config.include("pyramid_tm") + if settings['transaction_per_request']: + config.include("pyramid_tm") # Include cliquet plugins after init, unlike pyramid includes. includes = aslist(settings['includes']) diff --git a/cliquet/storage/postgresql/client.py b/cliquet/storage/postgresql/client.py index a21c09685..3d89fed21 100644 --- a/cliquet/storage/postgresql/client.py +++ b/cliquet/storage/postgresql/client.py @@ -69,7 +69,7 @@ def create_from_config(config, prefix=''): "Refer to installation section in documentation.") raise ImportWarning(message) - from zope.sqlalchemy import ZopeTransactionExtension, invalidate + from zope.sqlalchemy import invalidate from sqlalchemy.orm import sessionmaker, scoped_session settings = config.get_settings().copy() @@ -94,11 +94,7 @@ def create_from_config(config, prefix=''): engine = sqlalchemy.engine_from_config(settings, prefix=prefix, url=url) # Initialize thread-safe session factory. - options = {} - if transaction_per_request: - # Plug with Pyramid transaction manager - options['extension'] = ZopeTransactionExtension() - session_factory = scoped_session(sessionmaker(bind=engine, **options)) + session_factory = scoped_session(sessionmaker(bind=engine)) # Store one client per URI. commit_manually = (not transaction_per_request) From 128c559ae709722ad0a91b79362b12793a08ce06 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 12 Nov 2015 13:32:35 +0100 Subject: [PATCH 2/5] Fix principals of permission backend not being plugged by default --- CHANGELOG.rst | 1 + cliquet/__init__.py | 1 + cliquet/authorization.py | 24 ++++++++++++++++++++++++ cliquet/tests/test_authentication.py | 26 +++++++++++++++++--------- 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c46991c0f..9ff57d091 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -39,6 +39,7 @@ This document describes changes between each past release. **Bug fixes** - Fix crash with Redis backend if record parent/id is unicode (fixes #556) +- Fix principals of permission backend not being plugged by default (#573) **Internal changes** diff --git a/cliquet/__init__.py b/cliquet/__init__.py index 42c54c5fa..852094467 100644 --- a/cliquet/__init__.py +++ b/cliquet/__init__.py @@ -83,6 +83,7 @@ 'userid_hmac_secret': '', 'version_prefix_redirect_enabled': True, 'trailing_slash_redirect_enabled': True, + 'multiauth.groupfinder': 'cliquet.authorization.groupfinder', 'multiauth.policies': 'basicauth', 'multiauth.policy.basicauth.use': ('cliquet.authentication.' 'BasicAuthAuthenticationPolicy'), diff --git a/cliquet/authorization.py b/cliquet/authorization.py index 67f33d93e..57999c8d9 100644 --- a/cliquet/authorization.py +++ b/cliquet/authorization.py @@ -13,6 +13,30 @@ PRIVATE = 'private' +def groupfinder(userid, request): + """Fetch principals from permission backend for the specified `userid`. + + This is plugged by default using the ``multiauth.groupfinder`` setting. + """ + backend = getattr(request.registry, 'permission', None) + # Permission backend not configured. Ignore. + if not backend: + return [] + + # Anonymous safety check. + if not hasattr(request, 'prefixed_userid'): + return [] + + # Query the permission backend only once per request (e.g. batch). + + reify_key = request.prefixed_userid + '_principals' + if reify_key not in request.bound_data: + principals = backend.user_principals(request.prefixed_userid) + request.bound_data[reify_key] = principals + + return request.bound_data[reify_key] + + @implementer(IAuthorizationPolicy) class AuthorizationPolicy(object): # Callable that takes an object id and a permission and returns diff --git a/cliquet/tests/test_authentication.py b/cliquet/tests/test_authentication.py index 42c9576e7..87dc6ba31 100644 --- a/cliquet/tests/test_authentication.py +++ b/cliquet/tests/test_authentication.py @@ -8,30 +8,38 @@ class AuthenticationPoliciesTest(BaseWebTest, unittest.TestCase): - sample_url = '/mushrooms' - sample_basicauth = {'Authorization': 'Basic bWF0OjE='} - def test_basic_auth_is_accepted_by_default(self): - app = self.make_app() - app.get(self.sample_url, headers=self.sample_basicauth, status=200) + self.app.get(self.collection_url, headers=self.headers, status=200) def test_basic_auth_is_accepted_if_enabled_in_settings(self): app = self.make_app({'multiauth.policies': 'basicauth'}) - app.get(self.sample_url, headers=self.sample_basicauth, status=200) + app.get(self.collection_url, headers=self.headers, status=200) def test_basic_auth_is_declined_if_disabled_in_settings(self): app = self.make_app({ 'multiauth.policies': 'dummy', 'multiauth.policy.dummy.use': ('pyramid.authentication.' 'RepozeWho1AuthenticationPolicy')}) - app.get(self.sample_url, headers=self.sample_basicauth, status=401) + app.get(self.collection_url, headers=self.headers, status=401) def test_views_are_forbidden_if_unknown_auth_method(self): app = self.make_app({'multiauth.policies': 'basicauth'}) self.headers['Authorization'] = 'Carrier' - app.get(self.sample_url, headers=self.headers, status=401) + app.get(self.collection_url, headers=self.headers, status=401) self.headers['Authorization'] = 'Carrier pigeon' - app.get(self.sample_url, headers=self.headers, status=401) + app.get(self.collection_url, headers=self.headers, status=401) + + def test_principals_are_fetched_from_permission_backend(self): + patch = mock.patch(('cliquet.tests.support.' + 'AllowAuthorizationPolicy.permits')) + self.addCleanup(patch.stop) + mocked = patch.start() + + self.permission.add_user_principal(self.principal, 'group:admin') + self.app.get(self.collection_url, headers=self.headers) + + _, principals, _ = mocked.call_args[0] + self.assertIn('group:admin', principals) class BasicAuthenticationPolicyTest(unittest.TestCase): From 54c3811c9a51899c397e8d151e938d05a2914a45 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 16 Nov 2015 15:59:58 +0100 Subject: [PATCH 3/5] Revert "Fix Pyramid Transaction Manager setup" This reverts commit 13d1ff085c34b3b4fc3cae5dc5d69b4b735a835f. --- cliquet/__init__.py | 3 +-- cliquet/storage/postgresql/client.py | 8 ++++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/cliquet/__init__.py b/cliquet/__init__.py index 852094467..6fe894aea 100644 --- a/cliquet/__init__.py +++ b/cliquet/__init__.py @@ -113,8 +113,7 @@ def includeme(config): config.include("cornice") # Per-request transaction. - if settings['transaction_per_request']: - config.include("pyramid_tm") + config.include("pyramid_tm") # Include cliquet plugins after init, unlike pyramid includes. includes = aslist(settings['includes']) diff --git a/cliquet/storage/postgresql/client.py b/cliquet/storage/postgresql/client.py index 3d89fed21..a21c09685 100644 --- a/cliquet/storage/postgresql/client.py +++ b/cliquet/storage/postgresql/client.py @@ -69,7 +69,7 @@ def create_from_config(config, prefix=''): "Refer to installation section in documentation.") raise ImportWarning(message) - from zope.sqlalchemy import invalidate + from zope.sqlalchemy import ZopeTransactionExtension, invalidate from sqlalchemy.orm import sessionmaker, scoped_session settings = config.get_settings().copy() @@ -94,7 +94,11 @@ def create_from_config(config, prefix=''): engine = sqlalchemy.engine_from_config(settings, prefix=prefix, url=url) # Initialize thread-safe session factory. - session_factory = scoped_session(sessionmaker(bind=engine)) + options = {} + if transaction_per_request: + # Plug with Pyramid transaction manager + options['extension'] = ZopeTransactionExtension() + session_factory = scoped_session(sessionmaker(bind=engine, **options)) # Store one client per URI. commit_manually = (not transaction_per_request) From 20239dbd4d6f837328cfcb1b72d1a48a34e4d0ae Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 16 Nov 2015 16:09:36 +0100 Subject: [PATCH 4/5] Do not invalidate if transaction not enabled --- cliquet/storage/postgresql/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cliquet/storage/postgresql/client.py b/cliquet/storage/postgresql/client.py index a21c09685..dd41b14b1 100644 --- a/cliquet/storage/postgresql/client.py +++ b/cliquet/storage/postgresql/client.py @@ -12,7 +12,7 @@ class PostgreSQLClient(object): def __init__(self, session_factory, commit_manually=True, invalidate=None): self.session_factory = session_factory self.commit_manually = commit_manually - self.invalidate = invalidate or (lambda: None) + self.invalidate = invalidate or (lambda session: None) # # Register ujson, globally for all futur cursors # with self.connect() as cursor: @@ -36,7 +36,7 @@ def connect(self, readonly=False, force_commit=False): session = self.session_factory() # Start context yield session - if not readonly: + if not readonly and not self.commit_manually: # Mark session as dirty. self.invalidate(session) # Success From 02e3cecda04e5af541ca15cf87e1913d3b3d2a73 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 16 Nov 2015 22:25:07 +0100 Subject: [PATCH 5/5] Add test for principals caching --- cliquet/tests/test_authentication.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/cliquet/tests/test_authentication.py b/cliquet/tests/test_authentication.py index 87dc6ba31..72c21a928 100644 --- a/cliquet/tests/test_authentication.py +++ b/cliquet/tests/test_authentication.py @@ -41,6 +41,27 @@ def test_principals_are_fetched_from_permission_backend(self): _, principals, _ = mocked.call_args[0] self.assertIn('group:admin', principals) + def test_user_principals_are_cached_per_user(self): + patch = mock.patch.object(self.permission, 'user_principals', + wraps=self.permission.user_principals) + self.addCleanup(patch.stop) + mocked = patch.start() + batch = { + "defaults": { + "headers": self.headers, + "path": "/mushrooms" + }, + "requests": [ + {}, + {}, + {}, + {"headers": {"Authorization": "Basic Ym9iOg=="}}, + {"headers": {"Authorization": "Basic bWF0Og=="}}, + ] + } + self.app.post_json('/batch', batch) + self.assertEqual(mocked.call_count, 3) + class BasicAuthenticationPolicyTest(unittest.TestCase): def setUp(self):