Merge pull request #5364 from hypothesis/update-profile-permission

Protect `PATCH /api/profile` with a permission

h/routes.py

@@ -112,7 +112,9 @@ def includeme(config):
     config.add_route('api.groups',
                      '/api/groups',
                      factory='h.traversal.GroupRoot')
-    config.add_route('api.profile', '/api/profile')
+    config.add_route('api.profile',
+                     '/api/profile',
+                     factory='h.traversal.ProfileRoot')
     config.add_route('api.debug_token', '/api/debug-token')
     config.add_route('api.group_member',
                      '/api/groups/{pubid}/members/{userid}',

h/traversal/__init__.py

@@ -6,6 +6,7 @@
 from h.traversal.roots import AuthClientRoot
 from h.traversal.roots import OrganizationRoot
 from h.traversal.roots import OrganizationLogoRoot
+from h.traversal.roots import ProfileRoot
 from h.traversal.roots import GroupRoot
 from h.traversal.roots import UserRoot
 from h.traversal.contexts import AnnotationContext
@@ -20,6 +21,7 @@
     "OrganizationRoot",
     "OrganizationLogoRoot",
     "GroupRoot",
+    "ProfileRoot",
     "UserRoot",
     "AnnotationContext",
     "OrganizationContext",

h/traversal/roots.py

@@ -216,6 +216,18 @@ def __getitem__(self, pubid):
             raise KeyError()
 
 
+class ProfileRoot(object):
+    """
+    Simple Root for API profile endpoints
+    """
+    __acl__ = [
+        (Allow, role.User, 'update'),
+    ]
+
+    def __init__(self, request):
+        self.request = request
+
+
 class UserRoot(object):
     """
     Root factory for routes whose context is an :py:class:`h.traversal.UserContext`.

h/views/api/profile.py

@@ -2,8 +2,6 @@
 
 from __future__ import unicode_literals
 
-from pyramid import security
-
 from h import session as h_session
 from h.exceptions import APIError
 from h.views.api.config import api_config
@@ -20,13 +18,15 @@ def profile(request):
 
 @api_config(route_name='api.profile',
             request_method='PATCH',
-            effective_principals=security.Authenticated,
+            permission='update',
             link_name='profile.update',
             description="Update a user's preferences")
 def update_preferences(request):
     preferences = request.json_body.get('preferences', {})
 
     svc = request.find_service(name='user')
+    # TODO: The following exception doesn't match convention for validation
+    # used in other endpoints
     try:
         svc.update_preferences(request.user, **preferences)
     except TypeError as e:

tests/functional/api/test_profile.py

@@ -61,6 +61,72 @@ def test_it_returns_profile_for_third_party_authd_user(self,
         assert group_ids == []
 
 
+@pytest.mark.functional
+class TestPatchProfile(object):
+
+    def test_it_allows_authenticated_user(self, app, user_with_token):
+        """PATCH profile will always act on the auth'd user's profile."""
+
+        user, token = user_with_token
+
+        headers = {'Authorization': native_str('Bearer {}'.format(token.value))}
+        profile = {
+            'preferences': {
+                'show_sidebar_tutorial': True
+            }
+        }
+
+        res = app.patch_json('/api/profile', profile, headers=headers)
+
+        # The ``show_sidebar_tutorial`` property is only present if
+        # its value is True
+        assert 'show_sidebar_tutorial' in res.json['preferences']
+        assert res.status_code == 200
+
+    def test_it_updates_user_profile(self, app, user_with_token):
+        """PATCH profile will always act on the auth'd user's profile."""
+
+        user, token = user_with_token
+
+        headers = {'Authorization': native_str('Bearer {}'.format(token.value))}
+        profile = {
+            'preferences': {
+                'show_sidebar_tutorial': False
+            }
+        }
+
+        res = app.patch_json('/api/profile', profile, headers=headers)
+
+        # The ``show_sidebar_tutorial`` property is only present if
+        # its value is True
+        assert 'show_sidebar_tutorial' not in res.json['preferences']
+        assert res.status_code == 200
+
+    def test_it_raises_http_404_if_unauthenticated(self, app):
+        # FIXME: This should return a 403
+        profile = {
+            'preferences': {
+                'show_sidebar_tutorial': False
+            }
+        }
+
+        res = app.patch_json('/api/profile', profile, expect_errors=True)
+
+        assert res.status_code == 404
+
+    @pytest.mark.xfail
+    def test_it_raises_http_403_if_unauthenticated(self, app):
+        profile = {
+            'preferences': {
+                'show_sidebar_tutorial': False
+            }
+        }
+
+        res = app.patch_json('/api/profile', profile, expect_errors=True)
+
+        assert res.status_code == 403
+
+
 @pytest.fixture
 def user(db_session, factories):
     user = factories.User()

tests/h/routes_test.py

@@ -93,7 +93,7 @@ def test_includeme():
         call('api.groups',
              '/api/groups',
              factory='h.traversal.GroupRoot'),
-        call('api.profile', '/api/profile'),
+        call('api.profile', '/api/profile', factory='h.traversal.ProfileRoot'),
         call('api.debug_token', '/api/debug-token'),
         call('api.group_member', '/api/groups/{pubid}/members/{userid}', factory='h.traversal.GroupRoot', traverse='/{pubid}'),
         call('api.search', '/api/search'),

tests/h/traversal/roots_test.py

@@ -17,6 +17,7 @@
 from h.traversal.roots import OrganizationRoot
 from h.traversal.roots import OrganizationLogoRoot
 from h.traversal.roots import GroupRoot
+from h.traversal.roots import ProfileRoot
 from h.traversal.roots import UserRoot
 from h.traversal.contexts import AnnotationContext
 
@@ -269,6 +270,27 @@ def organization_logo_factory(self, pyramid_request):
         return OrganizationLogoRoot(pyramid_request)
 
 
+class TestProfileRoot(object):
+
+    def test_it_assigns_update_permission_with_user_role(self, pyramid_config, pyramid_request):
+        policy = pyramid.authorization.ACLAuthorizationPolicy()
+        pyramid_config.testing_securitypolicy('acct:adminuser@foo', [role.User])
+        pyramid_config.set_authorization_policy(policy)
+
+        context = ProfileRoot(pyramid_request)
+
+        assert pyramid_request.has_permission('update', context)
+
+    def test_it_does_not_assign_update_permission_without_user_role(self, pyramid_config, pyramid_request):
+        policy = pyramid.authorization.ACLAuthorizationPolicy()
+        pyramid_config.testing_securitypolicy('acct:adminuser@foo', ['whatever'])
+        pyramid_config.set_authorization_policy(policy)
+
+        context = ProfileRoot(pyramid_request)
+
+        assert not pyramid_request.has_permission('update', context)
+
+
 @pytest.mark.usefixtures("groups")
 class TestGroupRoot(object):