Merge pull request Kinto#573 from mozilla-services/plug-permission-ba…

…ckend-principals

Fix principals of permission backend not being plugged by default

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**
 

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'),

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

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

cliquet/tests/test_authentication.py

@@ -8,30 +8,59 @@
 
 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)
+
+    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):