Fix user_can_* checks handling of resource kwarg

arches/app/permissions/arches_default_allow.py

@@ -261,11 +261,6 @@ def check_resource_instance_permissions(
         """
         result = ResourceInstancePermissions()
         try:
-            if resourceid == settings.SYSTEM_SETTINGS_RESOURCE_ID:
-                if not user.groups.filter(name="System Administrator").exists():
-                    result["permitted"] = False
-                    return result
-
             if not resource:
                 resource = ResourceInstance.objects.select_related(
                     "resource_instance_lifecycle_state"
@@ -274,6 +269,11 @@ def check_resource_instance_permissions(
                 raise ValueError("resourceid and resource are mutually incompatible")
             result["resource"] = resource
 
+            if str(resource.pk) == settings.SYSTEM_SETTINGS_RESOURCE_ID:
+                if not user.groups.filter(name="System Administrator").exists():
+                    result["permitted"] = False
+                    return result
+
             all_perms = self.get_perms(user, resource)
 
             if len(all_perms) == 0:  # no permissions assigned. permission implied

arches/app/permissions/arches_default_deny.py

@@ -119,7 +119,7 @@ def check_resource_instance_permissions(
             resource = ResourceInstance.objects.get(resourceinstanceid=resourceid)
         elif resourceid:
             raise ValueError("resourceid and resource are mutually incompatible")
-        if resourceid == settings.SYSTEM_SETTINGS_RESOURCE_ID:
+        if str(resource.pk) == settings.SYSTEM_SETTINGS_RESOURCE_ID:
             result["resource"] = resource
             if not user.groups.filter(name="System Administrator").exists():
                 result["permitted"] = False

arches/app/permissions/arches_permission_base.py

@@ -310,7 +310,7 @@ def user_can_read_resource(
         if user.is_authenticated:
             if user.is_superuser:
                 return True
-            if resourceid is not None and resourceid != "":
+            if resourceid or resource:
                 result = self.check_resource_instance_permissions(
                     user, resourceid, "view_resourceinstance", resource=resource
                 )
@@ -363,7 +363,7 @@ def user_can_edit_resource(
         if user.is_authenticated:
             if user.is_superuser:
                 return True
-            if resourceid:
+            if resourceid or resource:
                 result = self.check_resource_instance_permissions(
                     user, resourceid, "change_resourceinstance", resource=resource
                 )
@@ -399,7 +399,7 @@ def user_can_delete_resource(
         if user.is_authenticated:
             if user.is_superuser:
                 return True
-            if resourceid:
+            if resourceid or resource:
                 result = self.check_resource_instance_permissions(
                     user, resourceid, "delete_resourceinstance", resource=resource
                 )

tests/permissions/permissions_arches_default_allow_tests.py

@@ -15,7 +15,6 @@
 
 from unittest.mock import MagicMock, Mock, patch
 
-from django.test import override_settings
 from arches.app.models.resource import Resource
 from tests.permissions.base_permissions_framework_test import (
     ArchesPermissionFrameworkTestCase,

tests/views/api/test_permissions.py

@@ -1,3 +1,5 @@
+from django.conf import settings
+from django.contrib.auth.models import Group, User
 from django.db import connection
 from django.test import TestCase
 from django.test.utils import CaptureQueriesContext
@@ -15,11 +17,16 @@ class InstancePermissionsAPITest(TestCase):
     def setUpTestData(cls):
         cls.graph = Graph.new(
             name="INSTANCE_PERMISSIONS_TEST_GRAPH",
-            is_resource=True,
+            is_resource=False,  # creates a nodegroup, will undo this below.
             author="ARCHES TEST",
         )
+        cls.graph.isresource = True
+        cls.graph.resource_instance_lifecycle_id = (
+            settings.DEFAULT_RESOURCE_INSTANCE_LIFECYCLE_ID
+        )
+        cls.graph.save(validate=False)
 
-    def test_get(self):
+    def test_get_with_anonymous_user(self):
         resource = ResourceInstance.objects.create(graph=self.graph)
         with CaptureQueriesContext(connection) as queries:
             response = self.client.get(
@@ -31,5 +38,21 @@ def test_get(self):
         ]
         self.assertEqual(len(resource_selects), 1, list(queries))
         self.assertEqual(
-            response.content.decode(), '{"delete": false, "edit": false, "read": false}'
+            response.content.decode(), '{"delete": true, "edit": true, "read": true}'
+        )
+
+    def test_get_with_resource_editor_role(self):
+        resource = ResourceInstance.objects.create(graph=self.graph)
+        editor_group = Group.objects.get(name="Resource Editor")
+        test_user = User.objects.create()
+        test_user.groups.add(editor_group)
+        self.client.force_login(test_user)
+
+        response = self.client.get(
+            reverse("api_instance_permissions"),
+            QUERY_STRING=f"resourceinstanceid={resource.pk}",
+        )
+
+        self.assertEqual(
+            response.content.decode(), '{"delete": true, "edit": true, "read": true}'
         )