Merge pull request #468 from CSCfi/CSCMETAX-625-end-user-api-bug

CSCMETAX-625: [FIX] End User API: Ensure user access perms in bulk up…

src/metax_api/services/common_service.py

@@ -228,7 +228,7 @@ def update_bulk(cls, request, model_obj, serializer_class, **kwargs):
 
         for row in request.data:
 
-            instance = cls._get_object_for_update(model_obj, row, results,
+            instance = cls._get_object_for_update(request, model_obj, row, results,
                                                   cls._request_has_header(request, 'HTTP_IF_UNMODIFIED_SINCE'))
 
             if not instance:
@@ -337,7 +337,7 @@ def _append_error(results, serializer, error):
             results['failed'].append({ 'object': serializer.initial_data, 'errors': str(error) })
 
     @staticmethod
-    def _get_object_for_update(model_obj, row, results, check_unmodified_since):
+    def _get_object_for_update(request, model_obj, row, results, check_unmodified_since):
         """
         Find the target object being updated using a row from the request payload.
 
@@ -357,6 +357,25 @@ def _get_object_for_update(model_obj, row, results, check_unmodified_since):
         except ValidationError as e:
             results['failed'].append({ 'object': row, 'errors': { 'detail': e.detail } })
 
+        if instance and not instance.user_has_access(request):
+
+            # dont reveal anything from the actual instance
+            ret = {}
+
+            if 'id' in row:
+                ret['id'] = row['id']
+            if 'identifier' in row:
+                ret['identifier'] = row['identifier']
+
+            results['failed'].append({
+                'object': ret,
+                'errors': {
+                    'detail': ['You are not permitted to access this resource.']
+                }
+            })
+
+            instance = None
+
         if instance and check_unmodified_since:
             if 'date_modified' not in row:
                 results['failed'].append({

src/metax_api/tests/api/rest/base/views/datasets/write.py

@@ -2948,9 +2948,15 @@ def test_other_users_cant_edit_dataset(self):
         response = self.client.get('/rest/datasets/1', format="json")
         modified_data = response.data
         modified_data['research_dataset']['value'] = 112233
+
         response = self.client.put('/rest/datasets/1', modified_data, format="json")
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
 
+        response = self.client.put('/rest/datasets', [modified_data], format="json")
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        # ^ individual errors do not have error codes, only the general request
+        # has an error code for a failed request.
+
     @responses.activate
     def test_user_can_delete_dataset(self):
         self._set_cr_owner_to_token_user(1)

src/metax_api/tests/api/rest/base/views/files/write.py

@@ -1240,6 +1240,48 @@ def test_user_can_only_update_permitted_file_fields(self):
                     continue
                 raise
 
+    @responses.activate
+    def test_user_can_update_files_in_their_projects(self):
+        '''
+        Ensure users can edit files in projects they are a member of.
+        '''
+        proj = File.objects.only('project_identifier').get(pk=1).project_identifier
+
+        response = self.client.get('/rest/files?project_identifier=%s' % proj,
+            format="json")
+
+        file = response.data['results'][0]
+
+        self.token['group_names'].append('fairdata:IDA01:%s' % proj)
+        self._use_http_authorization(method='bearer', token=self.token)
+
+        response = self.client.put('/rest/files/%s' % file['id'], file, format="json")
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        response = self.client.put('/rest/files', [file], format="json")
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+    @responses.activate
+    def test_user_cant_update_files_in_others_projects(self):
+        '''
+        Ensure users can not edit files in projects they are not a member of.
+        '''
+        proj = File.objects.only('project_identifier').get(pk=1).project_identifier
+
+        response = self.client.get('/rest/files?project_identifier=%s' % proj,
+            format="json")
+
+        file = response.data['results'][0]
+
+        self.token['group_names'] = ['no_files_for_this_project']
+        self._use_http_authorization(method='bearer', token=self.token)
+
+        response = self.client.put('/rest/files/%s' % file['id'], file, format="json")
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+        response = self.client.put('/rest/files', [file], format="json")
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
 
 class FileApiWriteDryrunTest(FileApiWriteCommon):