Skip to content

Commit 2fc157f

Browse files
[Fixes #12713] Permissions Registry and permissions handler (#12714)
* Add handler for permissions workflow * [Fixes #12713] Add tests * [Fixes #12713] Add tests * [Fixes #12713] Refactor permissions handler * [Fixes #12713] Usage of permissions registry * [Fixes #12713] fix black and flake8 * [Fixes #12713] fix black and flake8 * [Fixes #12713] Usage of permissions registry, fix tests * [Fixes #12713] Usage of permissions registry, fix tests * [Fixes #12713] Usage of permissions registry also for user perms * [Fixes #12713] make UserHasPerm user the registry * [Fixes #12713] fix tests * [Fixes #12713] fix tests * [Fixes #12713] fix tests * [Fixes #12713] fix tests * [Fixes #12713] fix tests * [Fixes #12713] Add remove function in permissions registry * [Fixes #12713] fix PR comments * [Fixes #12713] fix PR comments * [Fixes #12713] fix PR comments * [Fixes #12713] fix PR comments * [Fixes #12713] fix formatting --------- Co-authored-by: Giovanni Allegri <[email protected]>
1 parent 750a6c1 commit 2fc157f

File tree

26 files changed

+325
-85
lines changed

26 files changed

+325
-85
lines changed

geonode/base/api/permissions.py

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@
3535
from geonode.groups.models import GroupProfile
3636
from rest_framework.permissions import DjangoModelPermissions
3737
from guardian.shortcuts import get_objects_for_user
38-
from itertools import chain
39-
from guardian.shortcuts import get_groups_with_perms
38+
from geonode.security.registry import permissions_registry
4039

4140
logger = logging.getLogger(__name__)
4241

@@ -251,19 +250,10 @@ def has_permission(self, request, view):
251250
)
252251

253252
# getting the user permission for that resource
254-
resource_perms = res.get_user_perms(request.user)
255-
256-
groups = get_groups_with_perms(res, attach_perms=True)
257-
# we are making this because the request.user.groups sometimes returns empty si is not fully reliable
258-
for group, perm in groups.items():
259-
# checking if the user is in that group
260-
if group.user_set.filter(username=request.user).exists():
261-
resource_perms = list(chain(resource_perms, perm))
262-
263-
if request.user.has_perm("base.add_resourcebase"):
264-
resource_perms.append("add_resourcebase")
265-
# merging all available permissions into a single list
266-
available_perms = list(set(resource_perms))
253+
available_perms = permissions_registry.get_perms(
254+
instance=res, user=request.user, include_user_add_resource=True
255+
)
256+
267257
# fixup the permissions name
268258
perms_without_base = [x.replace("base.", "") for x in perms]
269259
# if at least one of the permissions is available the request is True

geonode/base/api/serializers.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
from geonode.security.utils import get_resources_with_perms, get_geoapp_subtypes
7070
from geonode.resource.models import ExecutionRequest
7171
from django.contrib.gis.geos import Polygon
72+
from geonode.security.registry import permissions_registry
7273

7374
logger = logging.getLogger(__name__)
7475

@@ -523,7 +524,11 @@ class Meta:
523524
def to_representation(self, instance):
524525
request = self.context.get("request", None)
525526
resource = ResourceBase.objects.get(pk=instance)
526-
return resource.get_user_perms(request.user) if request and request.user and resource else []
527+
return (
528+
permissions_registry.get_perms(instance=resource, user=request.user)
529+
if request and request.user and resource
530+
else []
531+
)
527532

528533

529534
class LinksSerializer(DynamicModelSerializer):

geonode/base/api/tests.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
)
9595
from geonode.resource.api.tasks import resouce_service_dispatcher
9696
from guardian.shortcuts import assign_perm
97+
from geonode.security.registry import permissions_registry
9798

9899
logger = logging.getLogger(__name__)
99100

@@ -2374,7 +2375,9 @@ def test_resource_service_copy_with_perms_dataset_set_default_perms(self):
23742375
}
23752376
resource.set_permissions(_perms)
23762377
# checking that bobby is in the original dataset perms list
2377-
self.assertTrue("bobby" in "bobby" in [x.username for x in resource.get_all_level_info().get("users", [])])
2378+
self.assertIn(
2379+
"bobby", [x.username for x in permissions_registry.get_perms(instance=resource).get("users", [])]
2380+
)
23782381
# copying the resource, should remove the perms for bobby
23792382
# only the default perms should be available
23802383
copy_url = reverse("importer_resource_copy", kwargs={"pk": resource.pk})
@@ -2389,8 +2392,14 @@ def test_resource_service_copy_with_perms_dataset_set_default_perms(self):
23892392
self.assertEqual("finished", self.client.get(response.json().get("status_url")).json().get("status"))
23902393
_resource = Dataset.objects.filter(title__icontains="test_copy_with_perms").last()
23912394
self.assertIsNotNone(_resource)
2392-
self.assertNotIn("bobby", [x.username for x in _resource.get_all_level_info().get("users", [])])
2393-
self.assertIn("admin", [x.username for x in _resource.get_all_level_info().get("users", [])])
2395+
self.assertNotIn(
2396+
"bobby",
2397+
[x.username for x in permissions_registry.get_perms(instance=_resource).get("users", [])],
2398+
)
2399+
self.assertIn(
2400+
"admin",
2401+
[x.username for x in permissions_registry.get_perms(instance=_resource).get("users", [])],
2402+
)
23942403

23952404
def test_resource_service_copy_with_perms_doc(self):
23962405
files = os.path.join(gisdata.GOOD_DATA, "vector/single_point.shp")
@@ -3428,7 +3437,7 @@ def test_simple_resourcebase_can_be_created_by_resourcemanager(self):
34283437
"groups": {anonymous_group: set(["view_resourcebase"])},
34293438
}
34303439

3431-
actual_perms = resource.get_all_level_info().copy()
3440+
actual_perms = permissions_registry.get_perms(instance=resource).copy()
34323441
self.assertIsNotNone(actual_perms)
34333442
self.assertTrue(self.user in actual_perms["users"].keys())
34343443
self.assertTrue(anonymous_group in actual_perms["groups"].keys())

geonode/base/views.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272

7373
from geonode.base.forms import CategoryForm, TKeywordForm, ThesaurusAvailableForm
7474
from geonode.base.models import Thesaurus, TopicCategory
75+
from geonode.security.registry import permissions_registry
7576

7677
from .forms import ResourceBaseForm
7778

@@ -509,8 +510,7 @@ def resourcebase_embed(request, resourcebaseid, template="base/base_edit.html"):
509510

510511
# Call this first in order to be sure "perms_list" is correct
511512
permissions_json = _perms_info_json(resourcebase_obj)
512-
513-
perms_list = resourcebase_obj.get_user_perms(request.user)
513+
perms_list = permissions_registry.get_perms(instance=resourcebase_obj, user=request.user)
514514

515515
group = None
516516
if resourcebase_obj.group:

geonode/documents/tests.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
from geonode.upload.api.exceptions import FileUploadLimitException
6161

6262
from .forms import DocumentCreateForm
63+
from geonode.security.registry import permissions_registry
6364

6465

6566
TEST_GIF = os.path.join(os.path.dirname(__file__), "tests/data/img.gif")
@@ -401,8 +402,8 @@ def test_set_document_permissions(self):
401402
self.assertFalse(self.anonymous_user.has_perm("view_resourcebase", document.get_self_resource()))
402403

403404
# Test that previous permissions for users other than ones specified in
404-
# the perm_spec (and the document owner) were removed
405-
current_perms = document.get_all_level_info()
405+
# the perm_spec (and the document owner) were
406+
current_perms = permissions_registry.get_perms(instance=document)
406407
self.assertEqual(len(current_perms["users"]), 1)
407408

408409
# Test that the User permissions specified in the perm_spec were

geonode/geoapps/views.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
from geonode.base.forms import CategoryForm, TKeywordForm, ThesaurusAvailableForm
4747
from geonode.base.models import Thesaurus, TopicCategory
4848
from geonode.utils import resolve_object
49+
from geonode.security.registry import permissions_registry
4950

5051
from .forms import GeoAppForm
5152

@@ -106,7 +107,7 @@ def geoapp_edit(request, geoappid, template="apps/app_edit.html"):
106107
# Call this first in order to be sure "perms_list" is correct
107108
permissions_json = _perms_info_json(geoapp_obj)
108109

109-
perms_list = geoapp_obj.get_user_perms(request.user)
110+
perms_list = permissions_registry.get_perms(instance=geoapp_obj, user=request.user)
110111

111112
group = None
112113
if geoapp_obj.group:

geonode/geoserver/security.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
from geonode.geoserver.helpers import geofence, gf_utils, gs_catalog
3838
from geonode.groups.models import GroupProfile
3939
from geonode.utils import get_dataset_workspace
40+
from geonode.security.registry import permissions_registry
4041

4142

4243
logger = logging.getLogger(__name__)
@@ -349,8 +350,7 @@ def sync_resources_with_guardian(resource=None, force=False):
349350
batch = AutoPriorityBatch(gf_utils.get_first_available_priority(), f"Sync resources {dataset}")
350351

351352
gf_utils.collect_delete_layer_rules(get_dataset_workspace(dataset), dataset.name, batch)
352-
353-
perm_spec = dataset.get_all_level_info()
353+
perm_spec = permissions_registry.get_perms(instance=dataset)
354354
# All the other users
355355
if "users" in perm_spec:
356356
for user, perms in perm_spec["users"].items():

geonode/geoserver/tests/integration.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
from geonode.decorators import on_ogc_backend
4343
from geonode.base.models import TopicCategory, Link
4444
from geonode.geoserver.helpers import set_attributes_from_geoserver
45+
from geonode.security.registry import permissions_registry
4546

4647
LOCAL_TIMEOUT = 300
4748

@@ -351,4 +352,4 @@ def test_default_anonymous_permissions(self):
351352
saved_dataset.delete()
352353

353354
def get_user_resource_perms(self, instance, user):
354-
return list(instance.get_user_perms(user).union(instance.get_self_resource().get_user_perms(user)))
355+
return permissions_registry.get_perms(instance=instance, user=user)

geonode/groups/tests.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
from geonode.groups.models import GroupProfile, GroupMember, GroupCategory
3838

3939
from geonode.base.populate_test_data import all_public, create_models, remove_models, create_single_dataset
40+
from geonode.security.registry import permissions_registry
4041

4142
logger = logging.getLogger(__name__)
4243

@@ -334,7 +335,7 @@ def test_perms_info(self):
334335
# Add test to test perms being sent to the front end.
335336
layer = Dataset.objects.first()
336337
layer.set_default_permissions()
337-
perms_info = layer.get_all_level_info()
338+
perms_info = permissions_registry.get_perms(instance=layer)
338339

339340
# Ensure there is only one group 'anonymous' by default
340341
self.assertEqual(len(perms_info["groups"].keys()), 1)
@@ -695,7 +696,7 @@ def test_group_activity_pages_render(self):
695696
try:
696697
# Add test to test perms being sent to the front end.
697698
dataset.set_default_permissions()
698-
perms_info = dataset.get_all_level_info()
699+
perms_info = permissions_registry.get_perms(instance=dataset)
699700

700701
# Ensure there is only one group 'anonymous' by default
701702
self.assertEqual(len(perms_info["groups"].keys()), 1)

geonode/layers/tests.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676

7777
from geonode.base.populate_test_data import all_public, create_models, remove_models, create_single_dataset
7878
from geonode.layers.download_handler import DatasetDownloadHandler
79+
from geonode.security.registry import permissions_registry
7980

8081
logger = logging.getLogger(__name__)
8182

@@ -641,7 +642,7 @@ def test_assign_change_dataset_data_perm(self):
641642
layer = Dataset.objects.first()
642643
user = get_anonymous_user()
643644
layer.set_permissions({"users": {user.username: ["change_dataset_data"]}})
644-
perms = layer.get_all_level_info()
645+
perms = permissions_registry.get_perms(instance=layer)
645646
self.assertNotIn(user, perms["users"])
646647
self.assertNotIn(user.username, perms["users"])
647648

@@ -736,13 +737,13 @@ def test_surrogate_escape_string(self):
736737
def test_assign_remove_permissions(self):
737738
# Assing
738739
layer = Dataset.objects.all().first()
739-
perm_spec = layer.get_all_level_info()
740+
perm_spec = permissions_registry.get_perms(instance=layer)
740741
self.assertNotIn(get_user_model().objects.get(username="norman"), perm_spec["users"])
741742

742743
utils.set_datasets_permissions(
743744
"edit", resources_names=[layer.name], users_usernames=["norman"], delete_flag=False, verbose=True
744745
)
745-
perm_spec = layer.get_all_level_info()
746+
perm_spec = permissions_registry.get_perms(instance=layer)
746747
_c = 0
747748
if "users" in perm_spec:
748749
for _u in perm_spec["users"]:
@@ -755,7 +756,7 @@ def test_assign_remove_permissions(self):
755756
utils.set_datasets_permissions(
756757
"read", resources_names=[layer.name], users_usernames=["norman"], delete_flag=True, verbose=True
757758
)
758-
perm_spec = layer.get_all_level_info()
759+
perm_spec = permissions_registry.get_perms(instance=layer)
759760
_c = 0
760761
if "users" in perm_spec:
761762
for _u in perm_spec["users"]:
@@ -768,15 +769,15 @@ def test_assign_remove_permissions(self):
768769
def test_assign_remove_permissions_for_groups(self):
769770
# Assing
770771
layer = Dataset.objects.all().first()
771-
perm_spec = layer.get_all_level_info()
772+
perm_spec = permissions_registry.get_perms(instance=layer)
772773
group_profile = GroupProfile.objects.create(slug="group1", title="group1", access="public")
773774
self.assertNotIn(group_profile, perm_spec["groups"])
774775

775776
# giving manage permissions to the group
776777
utils.set_datasets_permissions(
777778
"manage", resources_names=[layer.name], groups_names=["group1"], delete_flag=False, verbose=True
778779
)
779-
perm_spec = layer.get_all_level_info()
780+
perm_spec = permissions_registry.get_perms(instance=layer)
780781
expected = {
781782
"change_dataset_data",
782783
"change_dataset_style",
@@ -795,7 +796,7 @@ def test_assign_remove_permissions_for_groups(self):
795796
utils.set_datasets_permissions(
796797
"view", resources_names=[layer.name], groups_names=["group1"], delete_flag=False, verbose=True
797798
)
798-
perm_spec = layer.get_all_level_info()
799+
perm_spec = permissions_registry.get_perms(instance=layer)
799800
expected = {"view_resourcebase"}
800801
# checking the perms list
801802
self.assertSetEqual(expected, set(perm_spec["groups"][group_profile.group]))
@@ -804,7 +805,7 @@ def test_assign_remove_permissions_for_groups(self):
804805
utils.set_datasets_permissions(
805806
"view", resources_names=[layer.name], groups_names=["group1"], delete_flag=True, verbose=True
806807
)
807-
perm_spec = layer.get_all_level_info()
808+
perm_spec = permissions_registry.get_perms(instance=layer)
808809
# checking the perms list
809810
self.assertTrue(group_profile.group not in perm_spec["groups"])
810811

@@ -1851,7 +1852,7 @@ def _create_arguments(self, perms_type, mode="set"):
18511852
def _assert_perms(self, expected_perms, dataset, username, assertion=True):
18521853
dataset.refresh_from_db()
18531854

1854-
perms = dataset.get_all_level_info()
1855+
perms = permissions_registry.get_perms(instance=dataset)
18551856
if assertion:
18561857
self.assertTrue(username in [user.username for user in perms["users"]])
18571858
actual = set(

0 commit comments

Comments
 (0)