Merge pull request #5444 from hypothesis/get-group-api

Add `GET /groups/{id}` endpoint

docs/_extra/api-reference/hypothesis.yaml

@@ -373,6 +373,41 @@ paths:
         - developerAPIKey: []
 
   /groups/{id}:
+    get:
+      tags:
+        - groups
+      summary: Fetch a group
+      description: >
+        Any request (including unauthenticated requests) may retrieve any publicly-readable group.
+        When using AuthClient credentials, it is additionally possible to retrieve any group within
+        the associated authority.  Requests authenticated with an API key are able to retrieve any
+        publicly-readable group as well as any private group that the authenticated user created or
+        is a member of.
+      operationID: getGroup
+      parameters:
+        - name: id
+          in: path
+          description: the group's `id` or `groupid`
+          required: true
+          type: string
+        - name: expand
+          description: >
+            Array of fields to expand in the response
+          in: query
+          type: array
+          items:
+            type: string
+            enum:
+              - organization
+      responses:
+        '200':
+          description: Success
+          schema:
+            $ref: '#/definitions/GroupResult'
+      security:
+        - authClientCredentials: []
+        - developerAPIKey: []
+
     patch:
       tags:
         - groups

h/auth/policy.py

@@ -16,6 +16,7 @@
 AUTH_CLIENT_API_WHITELIST = [
     ('api.groups', 'POST'),
     ('api.group', 'PATCH'),
+    ('api.group', 'GET'),
     ('api.group_upsert', 'PUT'),
     ('api.group_member', 'POST'),
     ('api.users', 'POST'),

h/models/group.py

@@ -187,13 +187,6 @@ def is_public(self):
 
     def __acl__(self):
         terms = []
-        # This authority principal may be used to grant auth clients
-        # permissions for groups within their authority
-        authority_principal = "client_authority:{}".format(self.authority)
-
-        # auth_clients that have the same authority as the target group
-        # may add members to it
-        terms.append((security.Allow, authority_principal, 'member_add'))
 
         join_principal = _join_principal(self)
         if join_principal is not None:
@@ -214,13 +207,24 @@ def __acl__(self):
         if self.creator:
             # The creator of the group should be able to update it
             terms.append((security.Allow, self.creator.userid, 'admin'))
-            # auth_clients that have the same authority as this group
-            # should be allowed to update it
-            terms.append((security.Allow, authority_principal, 'admin'))
             terms.append((security.Allow, self.creator.userid, 'moderate'))
             # The creator may update this group in an upsert context
             terms.append((security.Allow, self.creator.userid, 'upsert'))
 
+        # This authority principal may be used to grant auth clients
+        # permissions for groups within their authority
+        authority_principal = "client_authority:{}".format(self.authority)
+
+        # auth_clients that have the same authority as the target group
+        # may add members to it
+        terms.append((security.Allow, authority_principal, 'member_add'))
+        # auth_clients that have the same authority as this group
+        # should be allowed to update it
+        terms.append((security.Allow, authority_principal, 'admin'))
+        # auth_clients with matching authority should be able to read
+        # the group
+        terms.append((security.Allow, authority_principal, 'read'))
+
         terms.append(security.DENY_ALL)
 
         return terms

h/routes.py

@@ -119,7 +119,7 @@ def includeme(config):
                      traverse='/{id}')
     config.add_route('api.group',
                      '/api/groups/{id}',
-                     request_method='PATCH',
+                     request_method=('GET', 'PATCH',),
                      factory='h.traversal.GroupRoot',
                      traverse='/{id}')
     config.add_route('api.profile',

h/views/api/groups.py

@@ -69,6 +69,18 @@ def create(request):
     return GroupJSONPresenter(GroupContext(group, request)).asdict(expand=['organization'])
 
 
+@api_config(route_name='api.group',
+            request_method='GET',
+            permission='read',
+            description='Fetch a group')
+def read(group, request):
+    """Fetch a group."""
+
+    expand = request.GET.getall('expand') or []
+
+    return GroupJSONPresenter(GroupContext(group, request)).asdict(expand=expand)
+
+
 @api_config(route_name='api.group',
             request_method='PATCH',
             permission='admin',

tests/functional/api/groups/test_read.py

@@ -0,0 +1,114 @@
+# -*- coding: utf-8 -*-
+
+from __future__ import unicode_literals
+
+import pytest
+import base64
+
+
+from h.models.auth_client import GrantType
+
+native_str = str
+
+
+@pytest.mark.functional
+class TestReadGroup(object):
+
+    def test_it_returns_http_200_for_world_readable_group_pubid(self, app, factories, db_session):
+        group = factories.OpenGroup()
+        db_session.commit()
+
+        res = app.get('/api/groups/{pubid}'.format(pubid=group.pubid))
+
+        assert res.status_code == 200
+
+        data = res.json
+        assert 'id' in data
+
+    def test_it_returns_http_200_for_world_readable_groupid(self, app, factories, db_session):
+        factories.OpenGroup(authority_provided_id='foo', authority='bar.com')
+        db_session.commit()
+        res = app.get('/api/groups/group:foo@bar.com')
+
+        assert res.status_code == 200
+
+    def test_it_returns_http_404_for_private_group_no_authentication(self, app, factories, db_session):
+        group = factories.Group()
+        db_session.commit()
+
+        res = app.get('/api/groups/{pubid}'.format(pubid=group.pubid), expect_errors=True)
+
+        assert res.status_code == 404
+
+    def test_it_returns_http_200_for_private_group_with_creator_authentication(self, app, user_with_token, token_auth_header, factories, db_session):
+        user, _ = user_with_token
+        group = factories.Group(creator=user)
+        db_session.commit()
+
+        res = app.get('/api/groups/{pubid}'.format(pubid=group.pubid), headers=token_auth_header)
+
+        assert res.status_code == 200
+
+    def test_it_returns_http_200_for_private_group_with_member_authentication(self, app, user_with_token, token_auth_header, factories, db_session):
+        user, _ = user_with_token
+        group = factories.Group()
+        group.members.append(user)
+        db_session.commit()
+
+        res = app.get('/api/groups/{pubid}'.format(pubid=group.pubid), headers=token_auth_header)
+
+        assert res.status_code == 200
+
+    def test_it_returns_http_404_for_private_group_if_token_user_not_creator(self, app, token_auth_header, factories, db_session):
+        group = factories.Group()
+        db_session.commit()
+
+        res = app.get('/api/groups/{pubid}'.format(pubid=group.pubid), headers=token_auth_header, expect_errors=True)
+
+        assert res.status_code == 404
+
+    def test_it_returns_http_200_for_private_group_with_auth_client_matching_authority(self, app, auth_client_header, factories, db_session):
+        group = factories.Group(authority='thirdparty.com')
+        db_session.commit()
+
+        res = app.get('/api/groups/{pubid}'.format(pubid=group.pubid), headers=auth_client_header)
+
+        assert res.status_code == 200
+
+    def test_it_returns_http_404_for_private_group_with_auth_client_mismatched_authority(self, app, auth_client_header, factories, db_session):
+        group = factories.Group(authority='somewhere-else.com')
+        db_session.commit()
+
+        res = app.get('/api/groups/{pubid}'.format(pubid=group.pubid), headers=auth_client_header, expect_errors=True)
+
+        assert res.status_code == 404
+
+
+@pytest.fixture
+def auth_client(db_session, factories):
+    auth_client = factories.ConfidentialAuthClient(authority='thirdparty.com',
+                                                   grant_type=GrantType.client_credentials)
+    db_session.commit()
+    return auth_client
+
+
+@pytest.fixture
+def auth_client_header(auth_client):
+    user_pass = "{client_id}:{secret}".format(client_id=auth_client.id, secret=auth_client.secret)
+    encoded = base64.standard_b64encode(user_pass.encode('utf-8'))
+    return {native_str('Authorization'): native_str("Basic {creds}".format(creds=encoded.decode('ascii')))}
+
+
+@pytest.fixture
+def user_with_token(db_session, factories):
+    user = factories.User()
+    token = factories.DeveloperToken(userid=user.userid)
+    db_session.add(token)
+    db_session.commit()
+    return (user, token)
+
+
+@pytest.fixture
+def token_auth_header(user_with_token):
+    user, token = user_with_token
+    return {native_str('Authorization'): native_str('Bearer {}'.format(token.value))}

tests/h/models/group_test.py

@@ -293,11 +293,13 @@ def test_auth_client_without_matching_authority_does_not_have_admin_permission(s
     def test_creator_has_upsert_permissions(self, group, authz_policy):
         assert authz_policy.permits(group, 'acct:luke@example.com', 'upsert')
 
-    def test_no_admin_permission_when_no_creator(self, group, authz_policy):
+    def test_admin_allowed_only_for_authority_when_no_creator(self, group, authz_policy):
         group.creator = None
 
         principals = authz_policy.principals_allowed_by_permission(group, 'admin')
-        assert len(principals) == 0
+
+        assert 'client_authority:example.com' in principals
+        assert authz_policy.permits(group, ['flip', 'client_authority:example.com'], 'admin')
 
     def test_no_moderate_permission_when_no_creator(self, group, authz_policy):
         group.creator = None

tests/h/routes_test.py

@@ -100,7 +100,7 @@ def test_includeme():
              traverse='/{id}'),
         call('api.group',
              '/api/groups/{id}',
-             request_method='PATCH',
+             request_method=('GET', 'PATCH',),
              factory='h.traversal.GroupRoot',
              traverse='/{id}'),
         call('api.profile', '/api/profile', factory='h.traversal.ProfileRoot'),

tests/h/views/api/groups_test.py

@@ -229,6 +229,34 @@ def pyramid_request(self, pyramid_request, factories):
         return pyramid_request
 
 
+@pytest.mark.usefixtures('GroupJSONPresenter',
+                         'GroupContext',)
+class TestReadGroup(object):
+
+    def test_it_creates_group_context_from_group_model(self, GroupContext, factories, pyramid_request):
+        group = factories.Group()
+
+        views.read(group, pyramid_request)
+
+        GroupContext.assert_called_once_with(group, pyramid_request)
+
+    def test_it_forwards_expand_param_to_presenter(self, GroupJSONPresenter, factories, pyramid_request):
+        pyramid_request.params['expand'] = 'organization'
+        group = factories.Group()
+
+        views.read(group, pyramid_request)
+
+        GroupJSONPresenter.return_value.asdict.assert_called_once_with(['organization'])
+
+    def test_it_returns_presented_group(self, GroupJSONPresenter, factories, pyramid_request):
+        pyramid_request.params['expand'] = 'organization'
+        group = factories.Group()
+
+        presented = views.read(group, pyramid_request)
+
+        assert presented == GroupJSONPresenter.return_value.asdict.return_value
+
+
 @pytest.mark.usefixtures('UpdateGroupAPISchema',
                          'group_service',
                          'group_update_service',