From a42264715b9dafea45f718a82aad7b804e49d3fc Mon Sep 17 00:00:00 2001 From: Clinton Blackburn Date: Fri, 23 Jun 2017 18:28:19 -0400 Subject: [PATCH] Corrected create permissions for ExperimentData create endpoint --- lms/djangoapps/experiments/factories.py | 2 +- lms/djangoapps/experiments/permissions.py | 7 +++++ lms/djangoapps/experiments/serializers.py | 16 +++++++++-- .../experiments/tests/test_views.py | 28 +++++++++++++++++-- lms/djangoapps/experiments/views.py | 8 +++++- 5 files changed, 54 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/experiments/factories.py b/lms/djangoapps/experiments/factories.py index b718a246e0..a0f028a670 100644 --- a/lms/djangoapps/experiments/factories.py +++ b/lms/djangoapps/experiments/factories.py @@ -10,5 +10,5 @@ class ExperimentDataFactory(factory.DjangoModelFactory): user = factory.SubFactory(UserFactory) experiment_id = factory.fuzzy.FuzzyInteger(0) - key = factory.Faker('word') + key = factory.Sequence(lambda n: n) value = factory.Faker('word') diff --git a/lms/djangoapps/experiments/permissions.py b/lms/djangoapps/experiments/permissions.py index 3e61be1eb5..2e1df6861f 100644 --- a/lms/djangoapps/experiments/permissions.py +++ b/lms/djangoapps/experiments/permissions.py @@ -8,5 +8,12 @@ class IsStaffOrOwner(permissions.IsStaffOrOwner): """ def has_permission(self, request, view): + # Staff users can create data for anyone. + # Non-staff users can only create data for themselves. + if view.action == 'create': + username = request.user.username + return super(IsStaffOrOwner, self).has_permission(request, view) or ( + username == request.data.get('user', username)) + # The view will handle filtering for the current user return True diff --git a/lms/djangoapps/experiments/serializers.py b/lms/djangoapps/experiments/serializers.py index c50433a692..895d72d8ce 100644 --- a/lms/djangoapps/experiments/serializers.py +++ b/lms/djangoapps/experiments/serializers.py @@ -1,12 +1,22 @@ +from django.contrib.auth import get_user_model from rest_framework import serializers from .models import ExperimentData +User = get_user_model() # pylint:disable=invalid-name + + +class ExperimentDataCreateSerializer(serializers.ModelSerializer): + user = serializers.SlugRelatedField(slug_field='username', default=serializers.CurrentUserDefault(), required=False, + queryset=User.objects.all()) + + class Meta(object): + model = ExperimentData + fields = ('id', 'experiment_id', 'user', 'key', 'value', 'created', 'modified',) + class ExperimentDataSerializer(serializers.ModelSerializer): user = serializers.SlugRelatedField(read_only=True, slug_field='username', default=serializers.CurrentUserDefault()) - class Meta(object): - model = ExperimentData - fields = ('id', 'experiment_id', 'user', 'key', 'value', 'created', 'modified',) + class Meta(ExperimentDataCreateSerializer.Meta): read_only_fields = ('user',) diff --git a/lms/djangoapps/experiments/tests/test_views.py b/lms/djangoapps/experiments/tests/test_views.py index 817d4a8319..c4312414b7 100644 --- a/lms/djangoapps/experiments/tests/test_views.py +++ b/lms/djangoapps/experiments/tests/test_views.py @@ -96,8 +96,32 @@ class ExperimentDataViewSetTests(APITestCase): response = self.client.post(url, {}) self.assertEqual(response.status_code, 401) - self.assert_data_created_for_user(UserFactory()) - self.assert_data_created_for_user(UserFactory()) + user = UserFactory() + data = { + 'experiment_id': 1, + 'key': 'foo', + 'value': 'bar', + } + self.client.login(username=user.username, password=UserFactory._DEFAULT_PASSWORD) + + # Users can create data for themselves + response = self.client.post(url, data) + self.assertEqual(response.status_code, 201) + ExperimentData.objects.get(user=user) + + # A non-staff user cannot create data for another user + other_user = UserFactory() + data['user'] = other_user.username + response = self.client.post(url, data) + self.assertEqual(response.status_code, 403) + self.assertFalse(ExperimentData.objects.filter(user=other_user).exists()) + + # A staff user can create data for other users + user.is_staff = True + user.save() + response = self.client.post(url, data) + self.assertEqual(response.status_code, 201) + ExperimentData.objects.get(user=other_user) def test_put_as_create(self): """ Users should be able to use PUT to create new data. """ diff --git a/lms/djangoapps/experiments/views.py b/lms/djangoapps/experiments/views.py index 4f6bf93d3e..c0e5b22185 100644 --- a/lms/djangoapps/experiments/views.py +++ b/lms/djangoapps/experiments/views.py @@ -5,7 +5,7 @@ from rest_framework.filters import DjangoFilterBackend from experiments import filters from experiments.models import ExperimentData from experiments.permissions import IsStaffOrOwner -from experiments.serializers import ExperimentDataSerializer +from experiments.serializers import ExperimentDataCreateSerializer, ExperimentDataSerializer from openedx.core.lib.api.authentication import SessionAuthenticationAllowInactiveUser @@ -21,6 +21,11 @@ class ExperimentDataViewSet(viewsets.ModelViewSet): queryset = queryset.filter(user=self.request.user) return super(ExperimentDataViewSet, self).filter_queryset(queryset) + def get_serializer_class(self): + if self.action == 'create': + return ExperimentDataCreateSerializer + return ExperimentDataSerializer + def create_or_update(self, request, *args, **kwargs): # If we have a primary key, treat this as a regular update request if self.kwargs.get('pk'): @@ -40,4 +45,5 @@ class ExperimentDataViewSet(viewsets.ModelViewSet): except ExperimentData.DoesNotExist: pass + self.action = 'create' return self.create(request, *args, **kwargs)