Merge pull request #980 from onaio/fix-unbound-local-variable-ret_query

fix xform meta permissions - ensure the submitted by user is part of the query
onaio · Mar 30, 2017 · 9a4fd93 · 9a4fd93
2 parents c4df244 + 2867292
commit 9a4fd93
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 35 deletions.
diff --git a/onadata/libs/permissions.py b/onadata/libs/permissions.py
@@ -415,11 +415,11 @@ def filter_queryset_xform_meta_perms_sql(xform, user, query):
                 query = json.loads(query)
                 if isinstance(query, list):
                     query = query[0]
-
             else:
                 query = dict()
-                query.update({"_submitted_by": user.username})
-                ret_query = json.dumps(query)
+
+            query.update({"_submitted_by": user.username})
+            ret_query = json.dumps(query)
 
         except (ValueError, AttributeError):
             query_list = list()

diff --git a/onadata/libs/tests/test_permissions.py b/onadata/libs/tests/test_permissions.py
@@ -1,24 +1,20 @@
 from guardian.shortcuts import get_users_with_perms
+from mock import patch
 
 from onadata.apps.api import tools
 from onadata.apps.main.models.user_profile import UserProfile
 from onadata.apps.main.tests.test_base import TestBase
 from onadata.libs.permissions import (
-    get_object_users_with_permissions,
-    ManagerRole,
-    CAN_ADD_XFORM_TO_PROFILE,
-    ReadOnlyRole,
-    OwnerRole,
-    EditorRole,
-    ReadOnlyRoleNoDownload)
+    CAN_ADD_XFORM_TO_PROFILE, DataEntryMinorRole, EditorRole, ManagerRole,
+    NoRecordsPermission, OwnerRole, ReadOnlyRole, ReadOnlyRoleNoDownload,
+    filter_queryset_xform_meta_perms_sql, get_object_users_with_permissions)
 
 
 def perms_for(user, obj):
     return get_users_with_perms(obj, attach_perms=True).get(user) or []
 
 
 class TestPermissions(TestBase):
-
     def test_manager_role_add(self):
         bob, created = UserProfile.objects.get_or_create(user=self.user)
         alice = self._create_user('alice', 'alice')
@@ -33,14 +29,12 @@ def test_manager_has_role(self):
         alice = self._create_user('alice', 'alice')
 
         self.assertFalse(ManagerRole.user_has_role(alice, bob))
-        self.assertFalse(ManagerRole.has_role(
-            perms_for(alice, bob), bob))
+        self.assertFalse(ManagerRole.has_role(perms_for(alice, bob), bob))
 
         ManagerRole.add(alice, bob)
 
         self.assertTrue(ManagerRole.user_has_role(alice, bob))
-        self.assertTrue(ManagerRole.has_role(
-            perms_for(alice, bob), bob))
+        self.assertTrue(ManagerRole.has_role(perms_for(alice, bob), bob))
 
     def test_reassign_role(self):
         self._publish_transportation_form()
@@ -51,17 +45,17 @@ def test_reassign_role(self):
         ManagerRole.add(alice, self.xform)
 
         self.assertTrue(ManagerRole.user_has_role(alice, self.xform))
-        self.assertTrue(ManagerRole.has_role(
-            perms_for(alice, self.xform), self.xform))
+        self.assertTrue(
+            ManagerRole.has_role(perms_for(alice, self.xform), self.xform))
 
         ReadOnlyRole.add(alice, self.xform)
 
         self.assertFalse(ManagerRole.user_has_role(alice, self.xform))
         self.assertTrue(ReadOnlyRole.user_has_role(alice, self.xform))
-        self.assertFalse(ManagerRole.has_role(
-            perms_for(alice, self.xform), self.xform))
-        self.assertTrue(ReadOnlyRole.has_role(
-            perms_for(alice, self.xform), self.xform))
+        self.assertFalse(
+            ManagerRole.has_role(perms_for(alice, self.xform), self.xform))
+        self.assertTrue(
+            ReadOnlyRole.has_role(perms_for(alice, self.xform), self.xform))
 
     def test_reassign_role_owner_to_editor(self):
         self._publish_transportation_form()
@@ -72,17 +66,17 @@ def test_reassign_role_owner_to_editor(self):
         OwnerRole.add(alice, self.xform)
 
         self.assertTrue(OwnerRole.user_has_role(alice, self.xform))
-        self.assertTrue(OwnerRole.has_role(
-            perms_for(alice, self.xform), self.xform))
+        self.assertTrue(
+            OwnerRole.has_role(perms_for(alice, self.xform), self.xform))
 
         EditorRole.add(alice, self.xform)
 
         self.assertFalse(OwnerRole.user_has_role(alice, self.xform))
         self.assertTrue(EditorRole.user_has_role(alice, self.xform))
-        self.assertFalse(OwnerRole.has_role(
-            perms_for(alice, self.xform), self.xform))
-        self.assertTrue(EditorRole.has_role(
-            perms_for(alice, self.xform), self.xform))
+        self.assertFalse(
+            OwnerRole.has_role(perms_for(alice, self.xform), self.xform))
+        self.assertTrue(
+            EditorRole.has_role(perms_for(alice, self.xform), self.xform))
 
     def test_get_object_users_with_permission(self):
         alice = self._create_user('alice', 'alice')
@@ -103,14 +97,45 @@ def test_readonly_no_downloads_has_role(self):
         self._publish_transportation_form()
         alice = self._create_user('alice', 'alice')
 
-        self.assertFalse(ReadOnlyRoleNoDownload.user_has_role(alice,
-                                                              self.xform))
-        self.assertFalse(ReadOnlyRoleNoDownload.has_role(
-            perms_for(alice, self.xform), self.xform))
+        self.assertFalse(
+            ReadOnlyRoleNoDownload.user_has_role(alice, self.xform))
+        self.assertFalse(
+            ReadOnlyRoleNoDownload.has_role(
+                perms_for(alice, self.xform), self.xform))
 
         ReadOnlyRoleNoDownload.add(alice, self.xform)
 
-        self.assertTrue(ReadOnlyRoleNoDownload.user_has_role(alice,
-                                                             self.xform))
-        self.assertTrue(ReadOnlyRoleNoDownload.has_role(
-            perms_for(alice, self.xform), self.xform))
+        self.assertTrue(
+            ReadOnlyRoleNoDownload.user_has_role(alice, self.xform))
+        self.assertTrue(
+            ReadOnlyRoleNoDownload.has_role(
+                perms_for(alice, self.xform), self.xform))
+
+    @patch('onadata.libs.permissions._check_meta_perms_enabled')
+    def test_filter_queryset_xform_meta_perms_sql(self, check_meta_mock):
+        self._publish_transportation_form()
+
+        query = '{"_id": 1}'
+        result = filter_queryset_xform_meta_perms_sql(self.xform, self.user,
+                                                      query)
+        self.assertEqual(result, query)
+
+        check_meta_mock.return_value = True
+        alice = self._create_user('alice', 'alice')
+
+        # no records
+        with self.assertRaises(NoRecordsPermission):
+            filter_queryset_xform_meta_perms_sql(self.xform, alice, query)
+
+        DataEntryMinorRole.add(alice, self.xform)
+
+        # meta perms test
+        result = filter_queryset_xform_meta_perms_sql(self.xform, alice, query)
+        self.assertEqual(result, '{"_submitted_by": "alice", "_id": 1}')
+
+        query = '[{"_id": 1}]'
+        result = filter_queryset_xform_meta_perms_sql(self.xform, alice, query)
+        self.assertEqual(result, '{"_submitted_by": "alice", "_id": 1}')
+
+        result = filter_queryset_xform_meta_perms_sql(self.xform, alice, None)
+        self.assertEqual(result, '{"_submitted_by": "alice"}')