Skip to content

Commit

Permalink
[Fixes #12512] Expose publish, approval and feature permissions insid…
Browse files Browse the repository at this point in the history
…e PermissionLevelMixin (#12516)

* [Fixes #8848] Implment can_feature, can_publish and can_approve methods
* [Fixes #12512] comment removed
  • Loading branch information
mattiagiupponi authored Aug 21, 2024
1 parent a282678 commit f8dabd5
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 85 deletions.
4 changes: 1 addition & 3 deletions geonode/base/api/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,7 @@ def has_permission(self, request, view):
)

# getting the user permission for that resource
resource_perms = list(res.get_user_perms(request.user))
if getattr(res, "get_real_instance", None):
resource_perms.extend(list(res.get_real_instance().get_user_perms(request.user)))
resource_perms = res.get_user_perms(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
10 changes: 1 addition & 9 deletions geonode/base/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -523,15 +523,7 @@ class Meta:
def to_representation(self, instance):
request = self.context.get("request", None)
resource = ResourceBase.objects.get(pk=instance)
return (
(
resource.get_user_perms(request.user)
.union(resource.get_self_resource().get_user_perms(request.user))
.union(resource.get_real_instance().get_user_perms(request.user))
)
if request and request.user and resource
else []
)
return resource.get_user_perms(request.user) if request and request.user and resource else []


class LinksSerializer(DynamicModelSerializer):
Expand Down
6 changes: 1 addition & 5 deletions geonode/base/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -536,11 +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 = list(
resourcebase_obj.get_self_resource()
.get_user_perms(request.user)
.union(resourcebase_obj.get_user_perms(request.user))
)
perms_list = resourcebase_obj.get_user_perms(request.user)

group = None
if resourcebase_obj.group:
Expand Down
4 changes: 1 addition & 3 deletions geonode/geoapps/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,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 = list(
geoapp_obj.get_self_resource().get_user_perms(request.user).union(geoapp_obj.get_user_perms(request.user))
)
perms_list = geoapp_obj.get_user_perms(request.user)

group = None
if geoapp_obj.group:
Expand Down
2 changes: 1 addition & 1 deletion geonode/layers/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,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 = list(layer.get_self_resource().get_user_perms(request.user).union(layer.get_user_perms(request.user)))
perms_list = layer.get_user_perms(request.user)

return render(
request,
Expand Down
130 changes: 78 additions & 52 deletions geonode/security/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,64 +367,90 @@ def get_user_perms(self, user):
"""
Returns a list of permissions a user has on a given resource.
"""
# To avoid circular import
from geonode.base.models import Configuration

config = Configuration.load()
ctype = ContentType.objects.get_for_model(self)
ctype_resource_base = ContentType.objects.get_for_model(self.get_self_resource())

PERMISSIONS_TO_FETCH = VIEW_PERMISSIONS + DOWNLOAD_PERMISSIONS + ADMIN_PERMISSIONS + SERVICE_PERMISSIONS
# include explicit permissions appliable to "subtype == 'vector'"
if self.subtype in ["vector", "vector_time"]:
PERMISSIONS_TO_FETCH += DATASET_ADMIN_PERMISSIONS
elif self.subtype == "raster":
PERMISSIONS_TO_FETCH += DATASET_EDIT_STYLE_PERMISSIONS

resource_perms = Permission.objects.filter(
codename__in=PERMISSIONS_TO_FETCH, content_type_id__in=[ctype.id, ctype_resource_base.id]
).values_list("codename", flat=True)

# Don't filter for admin users
if not user.is_superuser:
user_model = get_user_obj_perms_model(self)
user_resource_perms = user_model.objects.filter(
object_pk=self.pk,
content_type_id__in=[ctype.id, ctype_resource_base.id],
user__username=str(user),
permission__codename__in=resource_perms,
)
# get user's implicit perms for anyone flag
implicit_perms = get_perms(user, self)
# filter out implicit permissions unappliable to "subtype != 'vector'"
if self.subtype == "raster":
implicit_perms = list(set(implicit_perms) - set(DATASET_EDIT_DATA_PERMISSIONS))
elif self.subtype != "vector":
implicit_perms = list(set(implicit_perms) - set(DATASET_ADMIN_PERMISSIONS))

resource_perms = user_resource_perms.union(
user_model.objects.filter(permission__codename__in=implicit_perms)
).values_list("permission__codename", flat=True)

# filter out permissions for edit, change or publish if readonly mode is active
perm_prefixes = ["change", "delete", "publish"]
if config.read_only:
clauses = (Q(codename__contains=prefix) for prefix in perm_prefixes)
query = reduce(operator.or_, clauses)
if user.is_superuser:
resource_perms = resource_perms.exclude(query)
else:
perm_objects = Permission.objects.filter(codename__in=resource_perms)
resource_perms = perm_objects.exclude(query).values_list("codename", flat=True)

return resource_perms
def calculate_perms(instance, user):
# To avoid circular import
from geonode.base.models import Configuration

config = Configuration.load()
ctype = ContentType.objects.get_for_model(instance)
ctype_resource_base = ContentType.objects.get_for_model(instance.get_self_resource())

PERMISSIONS_TO_FETCH = VIEW_PERMISSIONS + DOWNLOAD_PERMISSIONS + ADMIN_PERMISSIONS + SERVICE_PERMISSIONS
# include explicit permissions appliable to "subtype == 'vector'"
if instance.subtype in ["vector", "vector_time"]:
PERMISSIONS_TO_FETCH += DATASET_ADMIN_PERMISSIONS
elif instance.subtype == "raster":
PERMISSIONS_TO_FETCH += DATASET_EDIT_STYLE_PERMISSIONS

resource_perms = Permission.objects.filter(
codename__in=PERMISSIONS_TO_FETCH, content_type_id__in=[ctype.id, ctype_resource_base.id]
).values_list("codename", flat=True)

# Don't filter for admin users
if not user.is_superuser:
user_model = get_user_obj_perms_model(instance)
user_resource_perms = user_model.objects.filter(
object_pk=instance.pk,
content_type_id__in=[ctype.id, ctype_resource_base.id],
user__username=str(user),
permission__codename__in=resource_perms,
)
# get user's implicit perms for anyone flag
implicit_perms = get_perms(user, instance)
# filter out implicit permissions unappliable to "subtype != 'vector'"
if instance.subtype == "raster":
implicit_perms = list(set(implicit_perms) - set(DATASET_EDIT_DATA_PERMISSIONS))
elif instance.subtype != "vector":
implicit_perms = list(set(implicit_perms) - set(DATASET_ADMIN_PERMISSIONS))

resource_perms = user_resource_perms.union(
user_model.objects.filter(permission__codename__in=implicit_perms)
).values_list("permission__codename", flat=True)

# filter out permissions for edit, change or publish if readonly mode is active
perm_prefixes = ["change", "delete", "publish"]
if config.read_only:
clauses = (Q(codename__contains=prefix) for prefix in perm_prefixes)
query = reduce(operator.or_, clauses)
if user.is_superuser:
resource_perms = resource_perms.exclude(query)
else:
perm_objects = Permission.objects.filter(codename__in=resource_perms)
resource_perms = perm_objects.exclude(query).values_list("codename", flat=True)
return resource_perms

perms = calculate_perms(self, user)

if getattr(self, "get_real_instance", None):
perms.union(calculate_perms(self.get_real_instance(), user))

if getattr(self, "get_self_resource", None):
perms.union(calculate_perms(self.get_self_resource(), user))

perms_as_list = list(set(perms))

if user.is_anonymous:
# anonymous cannot feature/approve or pusblish, we can return here the perms
return perms_as_list

if not perms_as_list:
return perms_as_list

if user.can_feature(self):
perms_as_list.append("feature_resourcebase")
if user.can_approve(self):
perms_as_list.append("approve_resourcebase")
if user.can_publish(self):
perms_as_list.append("publish_resourcebase")

return perms_as_list

def user_can(self, user, permission):
"""
Checks if a has a given permission to the resource.
"""
resource = self.get_self_resource()
user_perms = self.get_user_perms(user).union(resource.get_user_perms(user))
user_perms = self.get_user_perms(user)

if permission not in user_perms:
# TODO cater for permissions with syntax base.permission_codename
Expand Down
31 changes: 25 additions & 6 deletions geonode/security/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1854,8 +1854,9 @@ def test_set_compact_permissions(self):
"change_resourcebase_permissions",
"delete_resourcebase",
"download_resourcebase",
"publish_resourcebase",
"view_resourcebase",
"approve_resourcebase",
"publish_resourcebase",
],
self.group_manager: [],
self.group_member: [],
Expand All @@ -1878,16 +1879,18 @@ def test_set_compact_permissions(self):
"change_resourcebase_permissions",
"delete_resourcebase",
"download_resourcebase",
"publish_resourcebase",
"view_resourcebase",
"approve_resourcebase",
"publish_resourcebase",
],
self.group_manager: ["view_resourcebase"],
self.group_manager: ["view_resourcebase", "publish_resourcebase", "approve_resourcebase"],
self.group_member: ["view_resourcebase"],
self.not_group_member: [
"change_resourcebase",
"view_resourcebase",
"download_resourcebase",
"change_resourcebase_metadata",
"approve_resourcebase",
],
self.anonymous_user: ["view_resourcebase"],
},
Expand Down Expand Up @@ -1922,6 +1925,7 @@ def test_permissions_are_set_as_expected_resource_publishing_True(self):
"change_resourcebase",
"change_resourcebase_metadata",
"change_resourcebase_permissions",
"approve_resourcebase",
],
self.group_manager: [
"change_resourcebase",
Expand All @@ -1930,6 +1934,7 @@ def test_permissions_are_set_as_expected_resource_publishing_True(self):
"download_resourcebase",
"change_resourcebase_permissions",
"view_resourcebase",
"approve_resourcebase",
"publish_resourcebase",
],
self.group_member: ["download_resourcebase", "view_resourcebase"],
Expand All @@ -1947,6 +1952,7 @@ def test_permissions_are_set_as_expected_resource_publishing_True(self):
"change_resourcebase",
"change_resourcebase_metadata",
"change_resourcebase_permissions",
"approve_resourcebase",
],
self.group_manager: [
"change_resourcebase",
Expand All @@ -1955,6 +1961,7 @@ def test_permissions_are_set_as_expected_resource_publishing_True(self):
"download_resourcebase",
"view_resourcebase",
"change_resourcebase_permissions",
"approve_resourcebase",
"publish_resourcebase",
],
self.group_member: ["download_resourcebase", "view_resourcebase"],
Expand Down Expand Up @@ -1994,10 +2001,11 @@ def test_permissions_are_set_as_expected_admin_upload_resource_publishing_True(s
"change_resourcebase",
"change_resourcebase_metadata",
"change_resourcebase_permissions",
"publish_resourcebase",
"delete_resourcebase",
"download_resourcebase",
"view_resourcebase",
"approve_resourcebase",
"publish_resourcebase",
],
self.group_member: ["download_resourcebase", "view_resourcebase"],
self.not_group_member: [],
Expand All @@ -2015,10 +2023,11 @@ def test_permissions_are_set_as_expected_admin_upload_resource_publishing_True(s
"change_resourcebase",
"change_resourcebase_metadata",
"change_resourcebase_permissions",
"publish_resourcebase",
"delete_resourcebase",
"download_resourcebase",
"view_resourcebase",
"approve_resourcebase",
"publish_resourcebase",
],
self.group_member: ["download_resourcebase", "view_resourcebase"],
self.not_group_member: ["view_resourcebase"],
Expand Down Expand Up @@ -2065,6 +2074,8 @@ def test_permissions_are_set_as_expected_admin_upload_resource_publishing_False(
"download_resourcebase",
"publish_resourcebase",
"view_resourcebase",
"approve_resourcebase",
"publish_resourcebase",
],
self.group_manager: [],
self.group_member: [],
Expand All @@ -2086,8 +2097,9 @@ def test_permissions_are_set_as_expected_admin_upload_resource_publishing_False(
"download_resourcebase",
"publish_resourcebase",
"view_resourcebase",
"approve_resourcebase",
],
self.group_manager: ["view_resourcebase"],
self.group_manager: ["view_resourcebase", "approve_resourcebase", "publish_resourcebase"],
self.group_member: ["view_resourcebase"],
self.not_group_member: ["view_resourcebase", "change_resourcebase"],
self.anonymous_user: ["view_resourcebase"],
Expand Down Expand Up @@ -2127,6 +2139,7 @@ def test_permissions_on_user_role_promotion_to_manager(self):
"view_resourcebase",
"publish_resourcebase",
"change_resourcebase_permissions",
"approve_resourcebase",
],
self.group_member: [
"change_resourcebase",
Expand All @@ -2136,6 +2149,7 @@ def test_permissions_on_user_role_promotion_to_manager(self):
"view_resourcebase",
"publish_resourcebase",
"change_resourcebase_permissions",
"approve_resourcebase",
],
}
try:
Expand Down Expand Up @@ -2204,6 +2218,7 @@ def test_permissions_on_user_role_demote_to_member_only_RESOURCE_PUBLISHING_acti
"change_resourcebase",
"change_resourcebase_metadata",
"change_resourcebase_permissions",
"approve_resourcebase",
],
self.group_manager: ["download_resourcebase", "view_resourcebase"],
self.group_member: ["download_resourcebase", "view_resourcebase"],
Expand Down Expand Up @@ -2234,6 +2249,7 @@ def test_permissions_on_user_role_promote_to_manager_only_RESOURCE_PUBLISHING_ac
"change_resourcebase",
"change_resourcebase_metadata",
"change_resourcebase_permissions",
"approve_resourcebase",
],
self.group_manager: [
"change_resourcebase",
Expand All @@ -2243,6 +2259,8 @@ def test_permissions_on_user_role_promote_to_manager_only_RESOURCE_PUBLISHING_ac
"view_resourcebase",
"publish_resourcebase",
"change_resourcebase_permissions",
"approve_resourcebase",
"publish_resourcebase",
],
self.group_member: [
"change_resourcebase",
Expand All @@ -2252,6 +2270,7 @@ def test_permissions_on_user_role_promote_to_manager_only_RESOURCE_PUBLISHING_ac
"view_resourcebase",
"publish_resourcebase",
"change_resourcebase_permissions",
"approve_resourcebase",
],
}
for authorized_subject, expected_perms in expected.items():
Expand Down
8 changes: 2 additions & 6 deletions geonode/services/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,7 @@ def harvest_resources_handle_get(request, service, handler):
{"id": "type-filter", "data_key": "type"},
]

perms_list = list(
service.get_self_resource().get_user_perms(request.user).union(service.get_user_perms(request.user))
)
perms_list = service.get_user_perms(request.user)

result = render(
request,
Expand Down Expand Up @@ -251,9 +249,7 @@ def service_detail(request, service_id):

permissions_json = _perms_info_json(service)

perms_list = list(
service.get_self_resource().get_user_perms(request.user).union(service.get_user_perms(request.user))
)
perms_list = service.get_user_perms(request.user)

harvested_resources_ids = []
if service.harvester:
Expand Down

0 comments on commit f8dabd5

Please sign in to comment.