Merge pull request #5325 from hypothesis/annotations-post-permission

Convert `POST /api/annotations` to use a permission for authz

h/routes.py

@@ -90,7 +90,9 @@ def includeme(config):
     # template generator in `h/views/api.py`
     config.add_route('api.index', '/api/')
     config.add_route('api.links', '/api/links')
-    config.add_route('api.annotations', '/api/annotations')
+    config.add_route('api.annotations',
+                     '/api/annotations',
+                     factory='h.traversal:AnnotationRoot')
     config.add_route('api.annotation',
                      '/api/annotations/{id:[A-Za-z0-9_-]{20,22}}',
                      factory='h.traversal:AnnotationRoot',

h/traversal/roots.py

@@ -67,6 +67,7 @@
     ALL_PERMISSIONS,
     DENY_ALL,
     Allow,
+    Authenticated
 )
 import sqlalchemy.exc
 import sqlalchemy.orm.exc
@@ -99,6 +100,11 @@ def __init__(self, request):
 
 class AnnotationRoot(object):
     """Root factory for routes whose context is an :py:class:`h.traversal.AnnotationContext`."""
+
+    __acl__ = [
+        (Allow, Authenticated, 'create'),
+    ]
+
     def __init__(self, request):
         self.request = request
 

h/views/api/annotations.py

@@ -18,7 +18,6 @@
 """
 from __future__ import unicode_literals
 from pyramid import i18n
-from pyramid import security
 import newrelic.agent
 
 from h import search as search_lib
@@ -74,7 +73,7 @@ def search(request):
 
 @api_config(route_name='api.annotations',
             request_method='POST',
-            effective_principals=security.Authenticated,
+            permission='create',
             link_name='annotation.create',
             description='Create an annotation')
 def create(request):

tests/functional/api/test_annotations.py

@@ -41,6 +41,31 @@ def test_annotation_read_jsonld(self, app, annotation):
 
 
 class TestWriteAnnotation(object):
+    def test_it_returns_http_404_if_unauthorized(self, app):
+        # FIXME: This should return a 403
+
+        # This isn't a valid payload, but it won't get validated because the
+        # authorization will fail first
+        annotation = {
+            'text': 'My annotation',
+            'uri': 'http://example.com',
+        }
+
+        res = app.post_json('/api/annotations', annotation, expect_errors=True)
+
+        assert res.status_code == 404
+
+    @pytest.mark.xfail
+    def test_it_returns_http_403_if_unauthorized(self, app):
+        annotation = {
+            'text': 'My annotation',
+            'uri': 'http://example.com',
+        }
+
+        res = app.post_json('/api/annotations', annotation, expect_errors=True)
+
+        assert res.status_code == 403
+
     def test_annotation_write_unauthorized_group(self, app, user_with_token, non_writeable_group):
         """
         Write an annotation to a group that doesn't allow writes.

tests/h/routes_test.py

@@ -73,7 +73,7 @@ def test_includeme():
         call('assets', '/assets/*subpath'),
         call('api.index', '/api/'),
         call('api.links', '/api/links'),
-        call('api.annotations', '/api/annotations'),
+        call('api.annotations', '/api/annotations', factory='h.traversal:AnnotationRoot'),
         call('api.annotation',
              '/api/annotations/{id:[A-Za-z0-9_-]{20,22}}',
              factory='h.traversal:AnnotationRoot',

tests/h/traversal/roots_test.py

@@ -72,6 +72,25 @@ def test_it_assigns_all_permissions_to_requests_with_admin_role(self,
 
 @pytest.mark.usefixtures('group_service', 'links_service')
 class TestAnnotationRoot(object):
+    def test_it_does_not_assign_create_permission_without_authenticated_user(self, pyramid_config, pyramid_request):
+        policy = pyramid.authorization.ACLAuthorizationPolicy()
+        pyramid_config.testing_securitypolicy(None, groupids=[pyramid.security.Everyone])
+        pyramid_config.set_authorization_policy(policy)
+
+        context = AnnotationRoot(pyramid_request)
+
+        assert not pyramid_request.has_permission('create', context)
+
+    def test_it_assigns_create_permission_to_authenticated_request(self, pyramid_config, pyramid_request):
+        policy = pyramid.authorization.ACLAuthorizationPolicy()
+        pyramid_config.testing_securitypolicy('acct:adminuser@foo',
+                                              groupids=[pyramid.security.Authenticated])
+        pyramid_config.set_authorization_policy(policy)
+
+        context = AnnotationRoot(pyramid_request)
+
+        assert pyramid_request.has_permission('create', context)
+
     def test_get_item_fetches_annotation(self, pyramid_request, storage):
         factory = AnnotationRoot(pyramid_request)