diff --git a/arches/app/permissions/arches_default_allow.py b/arches/app/permissions/arches_default_allow.py index b61996e6e9f..17a11528b24 100644 --- a/arches/app/permissions/arches_default_allow.py +++ b/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 diff --git a/arches/app/permissions/arches_default_deny.py b/arches/app/permissions/arches_default_deny.py index 741e305c93e..55aa6f77e96 100644 --- a/arches/app/permissions/arches_default_deny.py +++ b/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 diff --git a/arches/app/permissions/arches_permission_base.py b/arches/app/permissions/arches_permission_base.py index 32243baa8bf..0907afd4c01 100644 --- a/arches/app/permissions/arches_permission_base.py +++ b/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 ) diff --git a/tests/permissions/permissions_arches_default_allow_tests.py b/tests/permissions/permissions_arches_default_allow_tests.py index 6b0a3e669a0..a8dcf7b0a88 100644 --- a/tests/permissions/permissions_arches_default_allow_tests.py +++ b/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, diff --git a/tests/views/api/test_permissions.py b/tests/views/api/test_permissions.py index 4002b29b580..eb297abd4da 100644 --- a/tests/views/api/test_permissions.py +++ b/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}' )