From 171109b7007acd37c23dec72baa2b2ec3ed2a2d9 Mon Sep 17 00:00:00 2001 From: Oliver Roick Date: Thu, 19 Jan 2017 18:41:48 +0100 Subject: [PATCH 1/3] Fix #961 -- Move view detail permissions into project user policy --- cadasta/config/permissions/org-member.json | 30 ------------------- cadasta/config/permissions/project-user.json | 30 +++++++++++++++++++ .../test_views_api_party_relationships.py | 4 +-- .../test_views_api_tenure_relationships.py | 4 +-- cadasta/party/tests/test_views_default.py | 21 ++++++++++++- cadasta/party/views/default.py | 1 - .../test_views_api_spatial_relationships.py | 4 +-- .../tests/test_views_api_spatial_units.py | 5 ++-- 8 files changed, 58 insertions(+), 41 deletions(-) diff --git a/cadasta/config/permissions/org-member.json b/cadasta/config/permissions/org-member.json index 9240acced..7e256ddc8 100644 --- a/cadasta/config/permissions/org-member.json +++ b/cadasta/config/permissions/org-member.json @@ -10,36 +10,6 @@ "party.list", "party_rel.list", "tenure_rel.list", "resource.list"], "object": ["project/$organization/*"] - }, - { - "effect": "allow", - "action": ["spatial.view"], - "object": ["spatial/$organization/*/*"] - }, - { - "effect": "allow", - "action": ["spatial_rel.view"], - "object": ["spatial_rel/$organization/*/*"] - }, - { - "effect": "allow", - "action": ["party.view"], - "object": ["party/$organization/*/*"] - }, - { - "effect": "allow", - "action": ["party_rel.view"], - "object": ["party_rel/$organization/*/*"] - }, - { - "effect": "allow", - "action": ["tenure_rel.view"], - "object": ["tenure_rel/$organization/*/*"] - }, - { - "effect": "allow", - "action": ["resource.view"], - "object": ["resource/$organization/*/*"] } ] } diff --git a/cadasta/config/permissions/project-user.json b/cadasta/config/permissions/project-user.json index 6c00659ad..46679ea32 100644 --- a/cadasta/config/permissions/project-user.json +++ b/cadasta/config/permissions/project-user.json @@ -4,5 +4,35 @@ // additional permissions over those given to all users. This may // change in the future. In particular, project users may be // permitted access to projects that are normally private. + { + "effect": "allow", + "action": ["spatial.view"], + "object": ["spatial/$organization/*/*"] + }, + { + "effect": "allow", + "action": ["spatial_rel.view"], + "object": ["spatial_rel/$organization/*/*"] + }, + { + "effect": "allow", + "action": ["party.view"], + "object": ["party/$organization/*/*"] + }, + { + "effect": "allow", + "action": ["party_rel.view"], + "object": ["party_rel/$organization/*/*"] + }, + { + "effect": "allow", + "action": ["tenure_rel.view"], + "object": ["tenure_rel/$organization/*/*"] + }, + { + "effect": "allow", + "action": ["resource.view"], + "object": ["resource/$organization/*/*"] + } ] } diff --git a/cadasta/party/tests/test_views_api_party_relationships.py b/cadasta/party/tests/test_views_api_party_relationships.py index 7d9733a82..1db5bb738 100644 --- a/cadasta/party/tests/test_views_api_party_relationships.py +++ b/cadasta/party/tests/test_views_api_party_relationships.py @@ -283,8 +283,8 @@ def test_get_private_record_based_on_org_membership(self): OrganizationRole.objects.create(organization=self.org, user=user) response = self.request(user=user) - assert response.status_code == 200 - assert response.content['id'] == self.rel.id + assert response.status_code == 403 + assert response.content['detail'] == PermissionDenied.default_detail class PartyRelationshipUpdateAPITest(APITestCase, UserTestCase, TestCase): diff --git a/cadasta/party/tests/test_views_api_tenure_relationships.py b/cadasta/party/tests/test_views_api_tenure_relationships.py index 172d7dec3..b30ff2651 100644 --- a/cadasta/party/tests/test_views_api_tenure_relationships.py +++ b/cadasta/party/tests/test_views_api_tenure_relationships.py @@ -267,8 +267,8 @@ def test_get_private_record_based_on_org_membership(self): user=user) response = self.request(user=user) - assert response.status_code == 200 - assert response.content['id'] == self.rel.id + assert response.status_code == 403 + assert response.content['detail'] == PermissionDenied.default_detail class TenureRelationshipUpdateAPITest(APITestCase, UserTestCase, TestCase): diff --git a/cadasta/party/tests/test_views_default.py b/cadasta/party/tests/test_views_default.py index 3acb208f6..0543d5677 100644 --- a/cadasta/party/tests/test_views_default.py +++ b/cadasta/party/tests/test_views_default.py @@ -83,9 +83,28 @@ def test_get_from_non_existent_project(self): def test_get_with_unauthorized_user(self): user = UserFactory.create() + response = self.request(user=user) + assert response.status_code == 302 + + def test_get_with_user_without_view_permissions(self): + user = UserFactory.create() + clauses = { + 'clause': [ + { + 'effect': 'allow', + 'object': ['project/*/*'], + 'action': ['project.*.*', 'party.list'] + } + ] + } + policy = Policy.objects.create( + name='allow', + body=json.dumps(clauses)) + assign_user_policies(user, policy) + response = self.request(user=user) assert response.status_code == 200 - assert response.content == self.render_content(object_list=[]) + assert response.content == self.expected_content def test_get_with_unauthenticated_user(self): response = self.request() diff --git a/cadasta/party/views/default.py b/cadasta/party/views/default.py index ec51a5576..ec47625e2 100644 --- a/cadasta/party/views/default.py +++ b/cadasta/party/views/default.py @@ -19,7 +19,6 @@ class PartiesList(LoginPermissionRequiredMixin, template_name = 'party/party_list.html' permission_required = 'party.list' permission_denied_message = error_messages.PARTY_LIST - permission_filter_queryset = ('party.view',) no_jsonattrs_in_queryset = True diff --git a/cadasta/spatial/tests/test_views_api_spatial_relationships.py b/cadasta/spatial/tests/test_views_api_spatial_relationships.py index 2fcad2cb5..55c7880a3 100644 --- a/cadasta/spatial/tests/test_views_api_spatial_relationships.py +++ b/cadasta/spatial/tests/test_views_api_spatial_relationships.py @@ -272,8 +272,8 @@ def test_get_private_record_based_on_org_membership(self): user=user) response = self.request(user=user) - assert response.status_code == 200 - assert response.content['id'] == self.rel.id + assert response.status_code == 403 + assert response.content['detail'] == PermissionDenied.default_detail class SpatialRelationshipUpdateAPITest(APITestCase, UserTestCase, TestCase): diff --git a/cadasta/spatial/tests/test_views_api_spatial_units.py b/cadasta/spatial/tests/test_views_api_spatial_units.py index 7cbcfbac5..d68519624 100644 --- a/cadasta/spatial/tests/test_views_api_spatial_units.py +++ b/cadasta/spatial/tests/test_views_api_spatial_units.py @@ -435,9 +435,8 @@ def test_get_private_record_based_on_org_membership(self): user=user) response = self.request(user=user) - assert response.status_code == 200 - print(response.content) - assert response.content['properties']['id'] == self.su.id + assert response.status_code == 403 + assert response.content['detail'] == PermissionDenied.default_detail class SpatialUnitUpdateAPITest(APITestCase, UserTestCase, TestCase): From c6224ca28447bd37f54af9425906f14511ce0b1b Mon Sep 17 00:00:00 2001 From: Oliver Roick Date: Tue, 24 Jan 2017 16:39:39 +0100 Subject: [PATCH 2/3] Change comment on project user policy --- cadasta/config/permissions/project-user.json | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cadasta/config/permissions/project-user.json b/cadasta/config/permissions/project-user.json index 46679ea32..3069c60fe 100644 --- a/cadasta/config/permissions/project-user.json +++ b/cadasta/config/permissions/project-user.json @@ -1,9 +1,8 @@ { "clause": [ - // Currently, "ordinary" users associated with a project have no - // additional permissions over those given to all users. This may - // change in the future. In particular, project users may be - // permitted access to projects that are normally private. + // In addition to the permissions provided by the organization member + // policy, project users can view details of locations, parties, + // relationships and resources. { "effect": "allow", "action": ["spatial.view"], From 878b32ef9db5fc92b4fcc83ccb429313e12f49b7 Mon Sep 17 00:00:00 2001 From: Oliver Roick Date: Thu, 9 Feb 2017 11:02:41 +0100 Subject: [PATCH 3/3] Restrict permissions to projects --- cadasta/config/permissions/project-user.json | 12 ++++++------ cadasta/core/static/css/main.css | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cadasta/config/permissions/project-user.json b/cadasta/config/permissions/project-user.json index 3069c60fe..a58ce0dab 100644 --- a/cadasta/config/permissions/project-user.json +++ b/cadasta/config/permissions/project-user.json @@ -6,32 +6,32 @@ { "effect": "allow", "action": ["spatial.view"], - "object": ["spatial/$organization/*/*"] + "object": ["spatial/$organization/$project/*"] }, { "effect": "allow", "action": ["spatial_rel.view"], - "object": ["spatial_rel/$organization/*/*"] + "object": ["spatial_rel/$organization/$project/*"] }, { "effect": "allow", "action": ["party.view"], - "object": ["party/$organization/*/*"] + "object": ["party/$organization/$project/*"] }, { "effect": "allow", "action": ["party_rel.view"], - "object": ["party_rel/$organization/*/*"] + "object": ["party_rel/$organization/$project/*"] }, { "effect": "allow", "action": ["tenure_rel.view"], - "object": ["tenure_rel/$organization/*/*"] + "object": ["tenure_rel/$organization/$project/*"] }, { "effect": "allow", "action": ["resource.view"], - "object": ["resource/$organization/*/*"] + "object": ["resource/$organization/$project/*"] } ] } diff --git a/cadasta/core/static/css/main.css b/cadasta/core/static/css/main.css index 0c9a53fde..45fea9566 100644 --- a/cadasta/core/static/css/main.css +++ b/cadasta/core/static/css/main.css @@ -6448,4 +6448,4 @@ h1 .label { text-transform: uppercase; padding-bottom: 2px; } -/*# sourceMappingURL=../../../../../static/css/main.css.map */ +/*# sourceMappingURL=../../../../../static/css/main.css.map */ \ No newline at end of file