Merge pull request #287 from circlingthesun/fix-acl

Fix #264

cornice/pyramidhook.py

@@ -170,6 +170,23 @@ def register_service_views(config, service):
     cornice_parameters = ('filters', 'validators', 'schema', 'klass',
                           'error_handler', 'deserializer') + CORS_PARAMETERS
 
+    # 1. register route
+
+    route_args = {}
+
+    if hasattr(service, 'acl'):
+        route_args['factory'] = make_route_factory(service.acl)
+
+    elif hasattr(service, 'factory'):
+        route_args['factory'] = service.factory
+
+    if hasattr(service, 'traverse'):
+        route_args['traverse'] = service.traverse
+
+    config.add_route(route_name, service.path, **route_args)
+
+    # 2. register view(s)
+
     for method, view, args in service.definitions:
 
         args = copy.copy(args)  # make a copy of the dict to not modify it
@@ -191,21 +208,12 @@ def register_service_views(config, service):
             if item in args:
                 del args[item]
 
-        # if acl is present, then convert it to a "factory"
-        if 'acl' in args:
-            args["factory"] = make_route_factory(args.pop('acl'))
-
-        # 1. register route
-        route_args = {}
-        if 'factory' in args:
-            route_args['factory'] = args.pop('factory')
-
-        if 'traverse' in args:
-            route_args['traverse'] = args.pop('traverse')
-
-        config.add_route(route_name, service.path, **route_args)
+        # These attributes are used in routes not views
+        deprecated_attrs = ['acl', 'factory', 'traverse']
+        for attr in deprecated_attrs:
+            if attr in args:
+                args.pop(attr)
 
-        # 2. register view(s)
         # pop and compute predicates which get passed through to Pyramid 1:1
 
         predicate_definitions = _pop_complex_predicates(args)

cornice/resource.py

@@ -26,7 +26,7 @@ def wrapper(klass):
         services = {}
 
         if 'collection_path' in kw:
-            prefixes = ('collection_', '')
+            prefixes = ('', 'collection_')
         else:
             prefixes = ('',)
 
@@ -41,6 +41,9 @@ def wrapper(klass):
                 elif k not in service_args:
                     service_args[k] = kw[k]
 
+            if prefix == 'collection_' and service_args.get('collection_acl'):
+                service_args['acl'] = service_args['collection_acl']
+
             # create service
             service_name = (service_args.pop('name', None) or
                             klass.__name__.lower())

cornice/service.py

@@ -287,6 +287,25 @@ def add_view(self, method, view, **kwargs):
         :param **kwargs: additional configuration for this view,
                         including `permission`.
         """
+
+        if 'acl' in kwargs:
+            kwargs.pop('acl')
+            msg = ("Warning: Using acl in method definitions is deprecated."
+                   "Use service or resource instead.")
+            warnings.warn(msg, DeprecationWarning)
+
+        if 'factory' in kwargs:
+            kwargs.pop('factory')
+            msg = ("Warning: Using acl in method definitions is deprecated."
+                   "Use service or resource instead.")
+            warnings.warn(msg, DeprecationWarning)
+
+        if 'traverse' in kwargs:
+            kwargs.pop('traverse')
+            msg = ("Warning: Using traverse in method definitions is "
+                   "deprecated. Use service or resource instead.")
+            warnings.warn(msg, DeprecationWarning)
+
         method = method.upper()
         if 'schema' in kwargs:
             # this is deprecated and unusable because multiple schema

cornice/tests/test_pyramidhook.py

@@ -29,28 +29,26 @@
 
 from mock import MagicMock
 
-service = Service(name="service", path="/service")
-
-
-@service.get()
-def return_404(request):
-    raise HTTPNotFound()
-
-
 def my_acl(request):
     return [(Allow, 'alice', 'read'),
             (Allow, 'bob', 'write'),
             (Deny, 'carol', 'write'),
             (Allow, 'dan', ('write', 'update')),
         ]
 
+service = Service(name="service", path="/service", acl=my_acl)
+
+
+@service.get()
+def return_404(request):
+    raise HTTPNotFound()
 
-@service.put(acl=my_acl, permission='update')
+@service.put(permission='update')
 def update_view(request):
     return "updated_view"
 
 
-@service.delete(acl=my_acl, permission='write')
+@service.delete(permission='write')
 def return_yay(request):
     return "yay"
 

cornice/tests/test_resource.py

@@ -4,7 +4,14 @@
 import json
 
 from pyramid import testing
+from pyramid.authentication import AuthTktAuthenticationPolicy
+from pyramid.authorization import ACLAuthorizationPolicy
+from pyramid.security import Allow
+from pyramid.httpexceptions import (
+    HTTPOk, HTTPForbidden
+)
 from webtest import TestApp
+import mock
 
 from cornice.resource import resource
 from cornice.resource import view
@@ -17,6 +24,23 @@
 USERS = {1: {'name': 'gawel'}, 2: {'name': 'tarek'}}
 
 
+def my_collection_acl(request):
+    return [(Allow, 'alice', 'read')]
+
+
+@resource(collection_path='/thing', path='/thing/{id}',
+          name='thing_service', collection_acl=my_collection_acl)
+class Thing(object):
+
+    def __init__(self, request, context=None):
+        self.request = request
+        self.context = context
+
+    @view(permission='read')
+    def collection_get(self):
+        return 'yay'
+
+
 @resource(collection_path='/users', path='/users/{id}',
           name='user_service', factory=dummy_factory)
 class User(object):
@@ -54,6 +78,11 @@ def setUp(self):
         self.config = testing.setUp()
         self.config.add_renderer('jsonp', JSONP(param_name='callback'))
         self.config.include("cornice")
+        self.authz_policy = ACLAuthorizationPolicy()
+        self.config.set_authorization_policy(self.authz_policy)
+
+        self.authn_policy = AuthTktAuthenticationPolicy('$3kr1t')
+        self.config.set_authentication_policy(self.authn_policy)
         self.config.scan("cornice.tests.test_resource")
         self.app = TestApp(CatchErrors(self.config.make_wsgi_app()))
 
@@ -110,6 +139,16 @@ def test_explicit_service_name(self):
         route_url = testing.DummyRequest().route_url
         self.assert_(route_url('user_service', id=42))  # service must exist
 
+    def test_acl_support_unauthenticated_thing_get(self):
+        # calling a view with permissions without an auth'd user => 403
+        self.app.get('/thing', status=HTTPForbidden.code)
+
+    def test_acl_support_authenticated_allowed_thing_get(self):
+        with mock.patch.object(self.authn_policy, 'unauthenticated_userid',
+                               return_value='alice'):
+            result = self.app.get('/thing', status=HTTPOk.code)
+            self.assertEqual("yay", result.json)
+
     if validationapp.COLANDER:
         def test_schema_on_resource(self):
             User.schema = CorniceSchema.from_colander(