Permissions filter for targets on actions

TOMToolkit · Feb 6, 2025 · bfeff3b · bfeff3b
1 parent 076898f
commit bfeff3b
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 8 deletions.
diff --git a/tom_targets/tests/tests.py b/tom_targets/tests/tests.py
@@ -1959,14 +1959,14 @@ def setUp(self):
         self.private_user_target = SiderealTargetFactory.create(permissions=Target.Permissions.PRIVATE)
 
     def test_open_targets_visible(self):
-        result = target_permission_filter(AnonymousUser(), Target.objects.all())
+        result = target_permission_filter(AnonymousUser(), Target.objects.all(), 'view_target')
         self.assertIn(self.open_target, result)
         self.assertNotIn(self.public_target, result)
         self.assertNotIn(self.private_group_target, result)
         self.assertNotIn(self.private_user_target, result)
 
     def test_public_targets_visible(self):
-        result = target_permission_filter(self.user, Target.objects.all())
+        result = target_permission_filter(self.user, Target.objects.all(), 'view_target')
         self.assertIn(self.open_target, result)
         self.assertIn(self.public_target, result)
         self.assertNotIn(self.private_group_target, result)
@@ -1975,25 +1975,38 @@ def test_public_targets_visible(self):
     def test_private_group_permission(self):
         self.group.user_set.add(self.user)
         assign_perm('tom_targets.view_target', self.group, self.private_group_target)
-        result = target_permission_filter(self.user, Target.objects.all())
+        result = target_permission_filter(self.user, Target.objects.all(), 'view_target')
         self.assertIn(self.open_target, result)
         self.assertIn(self.public_target, result)
         self.assertIn(self.private_group_target, result)
         self.assertNotIn(self.private_user_target, result)
 
     def test_private_user_permission(self):
         assign_perm('tom_targets.view_target', self.user, self.private_user_target)
-        result = target_permission_filter(self.user, Target.objects.all())
+        result = target_permission_filter(self.user, Target.objects.all(), 'view_target')
         self.assertIn(self.open_target, result)
         self.assertIn(self.public_target, result)
         self.assertNotIn(self.private_group_target, result)
         self.assertIn(self.private_user_target, result)
 
+    def test_private_user_permission_wrong_action(self):
+        assign_perm('tom_targets.view_target', self.user, self.private_user_target)
+        result = target_permission_filter(self.user, Target.objects.all(), 'delete_target')
+        self.assertIn(self.open_target, result)
+        self.assertIn(self.public_target, result)
+        self.assertNotIn(self.private_group_target, result)
+        self.assertNotIn(self.private_user_target, result)
+
     def test_superuser_permission(self):
         self.user.is_superuser = True
         self.user.save()
-        result = target_permission_filter(self.user, Target.objects.all())
+        result = target_permission_filter(self.user, Target.objects.all(), 'view_target')
         self.assertIn(self.open_target, result)
         self.assertIn(self.public_target, result)
         self.assertIn(self.private_group_target, result)
         self.assertIn(self.private_user_target, result)
+
+    def test_typo_in_action(self):
+        """Make sure a typo doesn't expose data"""
+        with self.assertRaises(AssertionError):
+            target_permission_filter(self.user, Target.objects.all(), 'view_targett')
diff --git a/tom_targets/views.py b/tom_targets/views.py
@@ -61,7 +61,9 @@
 logger = logging.getLogger(__name__)
 logger.setLevel(logging.DEBUG)
 
-def target_permission_filter(user, qs):
+def target_permission_filter(user, qs, action):
+    assert action in ['view_target', 'change_target', 'delete_target']
+
     if user.is_authenticated:
         if user.is_superuser:
             # Do not filter the queryset by permissions at all
@@ -70,7 +72,7 @@ def target_permission_filter(user, qs):
             # Exclude targets that are private except for those that the user has explicit permissions to view
             private_targets = qs.filter(permissions=Target.Permissions.PRIVATE)
             public_targets = qs.exclude(permissions=Target.Permissions.PRIVATE)
-            return public_targets | get_objects_for_user(user, f'{Target._meta.app_label}.view_target', private_targets)
+            return public_targets | get_objects_for_user(user, f'{Target._meta.app_label}.{action}', private_targets)
     else:
         # Only allow open targets
         return qs.exclude(permissions__in=[Target.Permissions.PUBLIC, Target.Permissions.PRIVATE])
@@ -107,7 +109,7 @@ def get_context_data(self, *args, **kwargs):
 
     def get_queryset(self, *args, **kwargs):
         qs = super().get_queryset(*args, **kwargs)
-        return target_permission_filter(self.request.user, qs)
+        return target_permission_filter(self.request.user, qs, 'view_target')
 
 
 class TargetNameSearchView(RedirectView):