Skip to content

Commit

Permalink
[Fixes #12713] Usage of permissions registry also for user perms
Browse files Browse the repository at this point in the history
  • Loading branch information
mattiagiupponi committed Jan 13, 2025
1 parent 7e0f900 commit 64bc820
Show file tree
Hide file tree
Showing 22 changed files with 71 additions and 87 deletions.
3 changes: 2 additions & 1 deletion geonode/base/api/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from guardian.shortcuts import get_objects_for_user
from itertools import chain
from guardian.shortcuts import get_groups_with_perms
from geonode.security.registry import permissions_registry

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -251,7 +252,7 @@ def has_permission(self, request, view):
)

# getting the user permission for that resource
resource_perms = res.get_user_perms(request.user)
resource_perms = permissions_registry.get_perms(instance=res, user=request.user)

groups = get_groups_with_perms(res, attach_perms=True)
# we are making this because the request.user.groups sometimes returns empty si is not fully reliable
Expand Down
2 changes: 1 addition & 1 deletion geonode/base/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ def to_representation(self, instance):
request = self.context.get("request", None)
resource = ResourceBase.objects.get(pk=instance)
return (
permissions_registry.get_perms(instance=resource, user=request.user, include_virtual=True)
permissions_registry.get_perms(instance=resource, user=request.user)
if request and request.user and resource
else []
)
Expand Down
17 changes: 4 additions & 13 deletions geonode/base/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2377,10 +2377,7 @@ def test_resource_service_copy_with_perms_dataset_set_default_perms(self):
self.assertTrue(
"bobby"
in "bobby"
in [
x.username
for x in permissions_registry.get_perms(instance=resource, include_virtual=True).get("users", [])
]
in [x.username for x in permissions_registry.get_perms(instance=resource).get("users", [])]
)
# copying the resource, should remove the perms for bobby
# only the default perms should be available
Expand All @@ -2398,17 +2395,11 @@ def test_resource_service_copy_with_perms_dataset_set_default_perms(self):
self.assertIsNotNone(_resource)
self.assertNotIn(
"bobby",
[
x.username
for x in permissions_registry.get_perms(instance=_resource, include_virtual=True).get("users", [])
],
[x.username for x in permissions_registry.get_perms(instance=_resource).get("users", [])],
)
self.assertIn(
"admin",
[
x.username
for x in permissions_registry.get_perms(instance=_resource, include_virtual=True).get("users", [])
],
[x.username for x in permissions_registry.get_perms(instance=_resource).get("users", [])],
)

def test_resource_service_copy_with_perms_doc(self):
Expand Down Expand Up @@ -3447,7 +3438,7 @@ def test_simple_resourcebase_can_be_created_by_resourcemanager(self):
"groups": {anonymous_group: set(["view_resourcebase"])},
}

actual_perms = permissions_registry.get_perms(instance=resource, include_virtual=True).copy()
actual_perms = permissions_registry.get_perms(instance=resource).copy()
self.assertIsNotNone(actual_perms)
self.assertTrue(self.user in actual_perms["users"].keys())
self.assertTrue(anonymous_group in actual_perms["groups"].keys())
Expand Down
4 changes: 2 additions & 2 deletions geonode/base/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@

from geonode.base.forms import CategoryForm, TKeywordForm, ThesaurusAvailableForm
from geonode.base.models import Thesaurus, TopicCategory
from geonode.security.registry import permissions_registry

from .forms import ResourceBaseForm

Expand Down Expand Up @@ -535,8 +536,7 @@ def resourcebase_embed(request, resourcebaseid, template="base/base_edit.html"):

# Call this first in order to be sure "perms_list" is correct
permissions_json = _perms_info_json(resourcebase_obj)

perms_list = resourcebase_obj.get_user_perms(request.user)
perms_list = permissions_registry.get_perms(instance=resourcebase_obj, user=request.user)

group = None
if resourcebase_obj.group:
Expand Down
2 changes: 1 addition & 1 deletion geonode/documents/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ def test_set_document_permissions(self):

# Test that previous permissions for users other than ones specified in
# the perm_spec (and the document owner) were
current_perms = permissions_registry.get_perms(instance=document, include_virtual=True)
current_perms = permissions_registry.get_perms(instance=document)
self.assertEqual(len(current_perms["users"]), 1)

# Test that the User permissions specified in the perm_spec were
Expand Down
3 changes: 2 additions & 1 deletion geonode/geoapps/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
from geonode.base.forms import CategoryForm, TKeywordForm, ThesaurusAvailableForm
from geonode.base.models import Thesaurus, TopicCategory
from geonode.utils import resolve_object
from geonode.security.registry import permissions_registry

from .forms import GeoAppForm

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

perms_list = geoapp_obj.get_user_perms(request.user)
perms_list = permissions_registry.get_perms(instance=geoapp_obj, user=request.user)

group = None
if geoapp_obj.group:
Expand Down
2 changes: 1 addition & 1 deletion geonode/geoserver/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ def sync_resources_with_guardian(resource=None, force=False):
batch = AutoPriorityBatch(gf_utils.get_first_available_priority(), f"Sync resources {dataset}")

gf_utils.collect_delete_layer_rules(get_dataset_workspace(dataset), dataset.name, batch)
perm_spec = permissions_registry.get_perms(instance=dataset, include_virtual=True)
perm_spec = permissions_registry.get_perms(instance=dataset)
# All the other users
if "users" in perm_spec:
for user, perms in perm_spec["users"].items():
Expand Down
4 changes: 3 additions & 1 deletion geonode/geoserver/tests/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
from geonode.decorators import on_ogc_backend
from geonode.base.models import TopicCategory, Link
from geonode.geoserver.helpers import set_attributes_from_geoserver
from geonode.security.registry import permissions_registry

LOCAL_TIMEOUT = 300

Expand Down Expand Up @@ -351,4 +352,5 @@ def test_default_anonymous_permissions(self):
saved_dataset.delete()

def get_user_resource_perms(self, instance, user):
return list(instance.get_user_perms(user).union(instance.get_self_resource().get_user_perms(user)))
return permissions_registry.get_perms(instance=instance, user=user)
# return list(instance.get_user_perms(user).union(instance.get_self_resource().get_user_perms(user)))
4 changes: 2 additions & 2 deletions geonode/groups/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ def test_perms_info(self):
# Add test to test perms being sent to the front end.
layer = Dataset.objects.first()
layer.set_default_permissions()
perms_info = permissions_registry.get_perms(instance=layer, include_virtual=True)
perms_info = permissions_registry.get_perms(instance=layer)

# Ensure there is only one group 'anonymous' by default
self.assertEqual(len(perms_info["groups"].keys()), 1)
Expand Down Expand Up @@ -696,7 +696,7 @@ def test_group_activity_pages_render(self):
try:
# Add test to test perms being sent to the front end.
dataset.set_default_permissions()
perms_info = permissions_registry.get_perms(instance=dataset, include_virtual=True)
perms_info = permissions_registry.get_perms(instance=dataset)

# Ensure there is only one group 'anonymous' by default
self.assertEqual(len(perms_info["groups"].keys()), 1)
Expand Down
18 changes: 9 additions & 9 deletions geonode/layers/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ def test_assign_change_dataset_data_perm(self):
layer = Dataset.objects.first()
user = get_anonymous_user()
layer.set_permissions({"users": {user.username: ["change_dataset_data"]}})
perms = permissions_registry.get_perms(instance=layer, include_virtual=True)
perms = permissions_registry.get_perms(instance=layer)
self.assertNotIn(user, perms["users"])
self.assertNotIn(user.username, perms["users"])

Expand Down Expand Up @@ -737,13 +737,13 @@ def test_surrogate_escape_string(self):
def test_assign_remove_permissions(self):
# Assing
layer = Dataset.objects.all().first()
perm_spec = permissions_registry.get_perms(instance=layer, include_virtual=True)
perm_spec = permissions_registry.get_perms(instance=layer)
self.assertNotIn(get_user_model().objects.get(username="norman"), perm_spec["users"])

utils.set_datasets_permissions(
"edit", resources_names=[layer.name], users_usernames=["norman"], delete_flag=False, verbose=True
)
perm_spec = permissions_registry.get_perms(instance=layer, include_virtual=True)
perm_spec = permissions_registry.get_perms(instance=layer)
_c = 0
if "users" in perm_spec:
for _u in perm_spec["users"]:
Expand All @@ -756,7 +756,7 @@ def test_assign_remove_permissions(self):
utils.set_datasets_permissions(
"read", resources_names=[layer.name], users_usernames=["norman"], delete_flag=True, verbose=True
)
perm_spec = permissions_registry.get_perms(instance=layer, include_virtual=True)
perm_spec = permissions_registry.get_perms(instance=layer)
_c = 0
if "users" in perm_spec:
for _u in perm_spec["users"]:
Expand All @@ -769,15 +769,15 @@ def test_assign_remove_permissions(self):
def test_assign_remove_permissions_for_groups(self):
# Assing
layer = Dataset.objects.all().first()
perm_spec = permissions_registry.get_perms(instance=layer, include_virtual=True)
perm_spec = permissions_registry.get_perms(instance=layer)
group_profile = GroupProfile.objects.create(slug="group1", title="group1", access="public")
self.assertNotIn(group_profile, perm_spec["groups"])

# giving manage permissions to the group
utils.set_datasets_permissions(
"manage", resources_names=[layer.name], groups_names=["group1"], delete_flag=False, verbose=True
)
perm_spec = permissions_registry.get_perms(instance=layer, include_virtual=True)
perm_spec = permissions_registry.get_perms(instance=layer)
expected = {
"change_dataset_data",
"change_dataset_style",
Expand All @@ -796,7 +796,7 @@ def test_assign_remove_permissions_for_groups(self):
utils.set_datasets_permissions(
"view", resources_names=[layer.name], groups_names=["group1"], delete_flag=False, verbose=True
)
perm_spec = permissions_registry.get_perms(instance=layer, include_virtual=True)
perm_spec = permissions_registry.get_perms(instance=layer)
expected = {"view_resourcebase"}
# checking the perms list
self.assertSetEqual(expected, set(perm_spec["groups"][group_profile.group]))
Expand All @@ -805,7 +805,7 @@ def test_assign_remove_permissions_for_groups(self):
utils.set_datasets_permissions(
"view", resources_names=[layer.name], groups_names=["group1"], delete_flag=True, verbose=True
)
perm_spec = permissions_registry.get_perms(instance=layer, include_virtual=True)
perm_spec = permissions_registry.get_perms(instance=layer)
# checking the perms list
self.assertTrue(group_profile.group not in perm_spec["groups"])

Expand Down Expand Up @@ -1816,7 +1816,7 @@ def _create_arguments(self, perms_type, mode="set"):
def _assert_perms(self, expected_perms, dataset, username, assertion=True):
dataset.refresh_from_db()

perms = permissions_registry.get_perms(instance=dataset, include_virtual=True)
perms = permissions_registry.get_perms(instance=dataset)
if assertion:
self.assertTrue(username in [user.username for user in perms["users"]])
actual = set(
Expand Down
3 changes: 2 additions & 1 deletion geonode/layers/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
from geonode.people.forms import ProfileForm
from geonode.utils import check_ogc_backend, llbbox_to_mercator, resolve_object
from geonode.geoserver.helpers import ogc_server_settings
from geonode.security.registry import permissions_registry

if check_ogc_backend(geoserver.BACKEND_PACKAGE):
from geonode.geoserver.helpers import gs_catalog
Expand Down Expand Up @@ -646,7 +647,7 @@ def dataset_metadata_detail(request, layername, template="datasets/dataset_metad
site_url = settings.SITEURL.rstrip("/") if settings.SITEURL.startswith("http") else settings.SITEURL

register_event(request, "view_metadata", layer)
perms_list = layer.get_user_perms(request.user)
perms_list = permissions_registry.get_perms(instance=layer, user=request.user)

return render(
request,
Expand Down
6 changes: 3 additions & 3 deletions geonode/people/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def test_set_unset_user_dataset_permissions(self):
)
for layer in self.layers:
user = get_user_model().objects.first()
perm_spec = permissions_registry.get_perms(instance=layer, include_virtual=True)
perm_spec = permissions_registry.get_perms(instance=layer)
self.assertFalse(user in perm_spec["users"], f"{layer} - {user}")

@override_settings(ASYNC_SIGNALS=False)
Expand Down Expand Up @@ -167,7 +167,7 @@ def test_set_unset_group_dataset_permissions(self):
verbose=True,
)
for layer in self.layers:
perm_spec = permissions_registry.get_perms(instance=layer, include_virtual=True)
perm_spec = permissions_registry.get_perms(instance=layer)
self.assertTrue(self.groups[0] in perm_spec["groups"])

@override_settings(ASYNC_SIGNALS=False)
Expand Down Expand Up @@ -215,7 +215,7 @@ def test_unset_group_dataset_perms(self):
verbose=True,
)
for layer in self.layers:
perm_spec = permissions_registry.get_perms(instance=layer, include_virtual=True)
perm_spec = permissions_registry.get_perms(instance=layer)
self.assertTrue(user not in perm_spec["users"])

def test_forgot_username(self):
Expand Down
2 changes: 1 addition & 1 deletion geonode/resource/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ def _safe_assign_perm(perm, user_or_group, obj=None):
uuid,
instance=_resource,
owner=owner,
permissions=permissions_registry.get_perms(instance=_resource, include_virtual=True),
permissions=permissions_registry.get_perms(instance=_resource),
created=created,
):
# This might not be a severe error. E.g. for datasets outside of local GeoServer
Expand Down
2 changes: 1 addition & 1 deletion geonode/security/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ def user_can(self, user, permission):
"""
Checks if a has a given permission to the resource.
"""
user_perms = self.get_user_perms(user)
user_perms = permissions_registry.get_perms(instance=self, user=user)

if permission not in user_perms:
# TODO cater for permissions with syntax base.permission_codename
Expand Down
2 changes: 1 addition & 1 deletion geonode/security/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def __check_item(self, item):

def fixup_perms(self, instance, payload, include_virtual=True, *args, **kwargs):
for handler in self.REGISTRY:
payload = handler.fixup_perms(instance, payload, include_virtual, *args, **kwargs)
payload = handler.fixup_perms(instance, payload, include_virtual=include_virtual, *args, **kwargs)
return payload

def get_perms(self, instance, user=None, include_virtual=True, *args, **kwargs):
Expand Down
Loading

0 comments on commit 64bc820

Please sign in to comment.