[RFC,05/11] REST: Add Persons to the API

This exports person objects via the REST API.

Security Constraints:
 * The API is read-only to authenticated users

Signed-off-by: Andy Doan <andy.doan@linaro.org>
---
 patchwork/tests/test_rest_api.py | 36 ++++++++++++++++++++++++++++++++++++
 patchwork/views/rest_api.py      | 14 +++++++++++++-
 2 files changed, 49 insertions(+), 1 deletion(-)

On 15 Apr 13:24, Andy Doan wrote:
> This exports person objects via the REST API.
> 
> Security Constraints:
>  * The API is read-only to authenticated users
> 
> Signed-off-by: Andy Doan <andy.doan@linaro.org>

Shaping up nicely, though I do worry about how much we're exposing.
See below.

Stephen

> ---
>  patchwork/tests/test_rest_api.py | 36 ++++++++++++++++++++++++++++++++++++
>  patchwork/views/rest_api.py      | 14 +++++++++++++-
>  2 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py
> index 6f2f988..76f0c12 100644
> --- a/patchwork/tests/test_rest_api.py
> +++ b/patchwork/tests/test_rest_api.py
> @@ -106,3 +106,39 @@ class TestProjectAPI(APITestCase):
>          resp = self.client.delete('/api/1.0/projects/1/')
>          self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
>          self.assertEqual(1, Project.objects.all().count())
> +
> +
> +@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
> +class TestPersonAPI(APITestCase):
> +    fixtures = ['default_states']
> +
> +    def test_anonymous_list(self):
> +        """The API should reject anonymous users."""
> +        resp = self.client.get('/api/1.0/people/')
> +        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> +    def test_authenticated_list(self):
> +        """This API requires authenticated users."""
> +        user = create_user()
> +        self.client.force_authenticate(user=user)
> +        resp = self.client.get('/api/1.0/people/')
> +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> +        self.assertEqual(1, resp.data['count'])
> +        self.assertEqual(user.username, resp.data['results'][0]['name'])
> +        self.assertEqual(user.email, resp.data['results'][0]['email'])
> +
> +    def test_readonly(self):
> +        defaults.project.save()
> +        user = create_maintainer(defaults.project)
> +        user.is_superuser = True
> +        user.save()
> +        self.client.force_authenticate(user=user)
> +
> +        resp = self.client.delete('/api/1.0/people/1/')
> +        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> +        resp = self.client.patch('/api/1.0/people/1/', {'email': 'foo@f.com'})
> +        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> +        resp = self.client.post('/api/1.0/people/', {'email': 'foo@f.com'})
> +        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> diff --git a/patchwork/views/rest_api.py b/patchwork/views/rest_api.py
> index f6385a3..ab576cc 100644
> --- a/patchwork/views/rest_api.py
> +++ b/patchwork/views/rest_api.py
> @@ -19,7 +19,7 @@
>  
>  from django.conf.urls import url, include
>  
> -from patchwork.models import Project
> +from patchwork.models import Person, Project
>  
>  from rest_framework import permissions
>  from rest_framework.pagination import PageNumberPagination
> @@ -48,6 +48,12 @@ class PatchworkPermission(permissions.BasePermission):
>          return obj.is_editable(request.user)
>  
>  
> +class AuthenticatedReadOnly(permissions.BasePermission):
> +    def has_permission(self, request, view):
> +        authenticated = request.user.is_authenticated()
> +        return authenticated and request.method in permissions.SAFE_METHODS
> +
> +
>  class PatchworkViewSet(ModelViewSet):
>      pagination_class = PageSizePagination
>  
> @@ -62,12 +68,18 @@ def create_model_serializer(model_class):
>      return PatchworkSerializer
>  
>  
> +class PeopleViewSet(PatchworkViewSet):
> +    permission_classes = (AuthenticatedReadOnly,)
> +    serializer_class = create_model_serializer(Person)
> +
> +

As pointed out in the previous patch, this exposes every attribute of
the object including some potentially undesirable ones. The linked
user profile is included here: do we really want it to be?

>  class ProjectViewSet(PatchworkViewSet):
>      permission_classes = (PatchworkPermission,)
>      serializer_class = create_model_serializer(Project)
>  
>  
>  router = DefaultRouter()
> +router.register('people', PeopleViewSet, 'person')
>  router.register('projects', ProjectViewSet, 'project')
>  
>  urlpatterns = [
> -- 
> 2.7.4
> 
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

On 05/09/2016 08:39 AM, Finucane, Stephen wrote:
>> +class PeopleViewSet(PatchworkViewSet):
>> >+    permission_classes = (AuthenticatedReadOnly,)
>> >+    serializer_class = create_model_serializer(Person)
>> >+
>> >+
> As pointed out in the previous patch, this exposes every attribute of
> the object including some potentially undesirable ones. The linked
> user profile is included here: do we really want it to be?

I can expose less as you see in the later patches. However, this only 
exposes something like:

	{
             "id": 51,
             "email": "andy.doan@linaro.org",
             "name": "Andy Doan",
             "user": 101
         },

Let me know what you'd like.