Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 4.0.x] [Fixes #10214] metadata_only filter not working properly #10269

Merged
merged 1 commit into from
Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 16 additions & 19 deletions geonode/base/api/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,17 @@
#
#########################################################################
import logging
from django.conf import settings
from django.contrib.auth import get_user_model
from django.shortcuts import get_object_or_404

from rest_framework import permissions
from rest_framework.filters import BaseFilterBackend
from geonode.security.permissions import BASIC_MANAGE_PERMISSIONS, DOWNLOAD_PERMISSIONS, EDIT_PERMISSIONS, VIEW_PERMISSIONS

from distutils.util import strtobool
from geonode.security.utils import (
get_users_with_perms,
get_resources_with_perms)
get_visible_resources)
from geonode.groups.models import GroupProfile
from rest_framework.permissions import DjangoModelPermissions
from guardian.shortcuts import get_objects_for_user
Expand Down Expand Up @@ -194,26 +195,22 @@ class ResourceBasePermissionsFilter(BaseFilterBackend):
A filter backend that limits results to those where the requesting user
has read object level permissions.
"""
shortcut_kwargs = {
'accept_global_perms': True,
}

def filter_queryset(self, request, queryset, view):
# We want to defer this import until runtime, rather than import-time.
# See https://github.com/encode/django-rest-framework/issues/4608
# (Also see #1624 for why we need to make this import explicitly)

user = request.user
# perm_format = '%(app_label)s.view_%(model_name)s'
# permission = self.perm_format % {
# 'app_label': queryset.model._meta.app_label,
# 'model_name': queryset.model._meta.model_name,
# }

obj_with_perms = get_resources_with_perms(user, shortcut_kwargs=self.shortcut_kwargs)
logger.debug(f" user: {user} -- obj_with_perms: {obj_with_perms}")

return queryset.filter(id__in=obj_with_perms.values('id'))
try:
metadata_only = strtobool(request.query_params.get("filter{metadata_only}", "None"))
except Exception:
metadata_only = None

return get_visible_resources(
queryset,
request.user,
metadata_only=metadata_only,
admin_approval_required=settings.ADMIN_MODERATE_UPLOADS,
unpublished_not_visible=settings.RESOURCE_PUBLISHING,
private_groups_not_visibile=settings.GROUP_PRIVATE_RESOURCES
)


class UserHasPerms(DjangoModelPermissions):
Expand Down
51 changes: 30 additions & 21 deletions geonode/base/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,13 @@ def test_base_resources(self):
response = self.client.get(url, format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 28)

url = "{base_url}?{params}".format(base_url=reverse('base-resources-list'), params="filter{metadata_only}=false")
# Anonymous
response = self.client.get(url, format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 26)
response.data['resources'][0].get('executions')
# Pagination
Expand Down Expand Up @@ -494,7 +501,7 @@ def test_base_resources(self):
# Admin
self.assertTrue(self.client.login(username='admin', password='admin'))

response = self.client.get(f"{url}?page_size=17", format='json')
response = self.client.get(f"{url}&page_size=17", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 26)
Expand All @@ -505,25 +512,27 @@ def test_base_resources(self):
resource = ResourceBase.objects.filter(owner__username='bobby').first()
self.assertEqual(resource.owner.username, 'bobby')
# Admin
response = self.client.get(f"{url}/{resource.id}/", format='json')
url_with_id = "{base_url}/{res_id}?{params}".format(base_url=reverse('base-resources-list'), res_id=resource.id, params="filter{metadata_only}=false")

response = self.client.get(f"{url_with_id}", format='json')
self.assertEqual(response.data['resource']['state'], enumerations.STATE_PROCESSED)
self.assertEqual(response.data['resource']['sourcetype'], enumerations.SOURCE_TYPE_LOCAL)
self.assertTrue('change_resourcebase' in list(response.data['resource']['perms']))
# Annonymous
self.assertIsNone(self.client.logout())
response = self.client.get(f"{url}/{resource.id}/", format='json')
response = self.client.get(f"{url_with_id}", format='json')
self.assertFalse('change_resourcebase' in list(response.data['resource']['perms']))
# user owner
self.assertTrue(self.client.login(username='bobby', password='bob'))
response = self.client.get(f"{url}/{resource.id}/", format='json')
response = self.client.get(f"{url_with_id}", format='json')
self.assertTrue('change_resourcebase' in list(response.data['resource']['perms']))
# user not owner and not assigned
self.assertTrue(self.client.login(username='norman', password='norman'))
response = self.client.get(f"{url}/{resource.id}/", format='json')
response = self.client.get(f"{url_with_id}", format='json')
self.assertFalse('change_resourcebase' in list(response.data['resource']['perms']))
# Check executions are returned when deffered
# all resources
response = self.client.get(f'{url}?include[]=executions', format='json')
response = self.client.get(f'{url}&include[]=executions', format='json')
self.assertEqual(response.status_code, 200)
self.assertIsNotNone(response.data['resources'][0].get('executions'))
# specific resource
Expand Down Expand Up @@ -555,7 +564,7 @@ def test_base_resources(self):
)
}]
self.assertTrue(self.client.login(username='bobby', password='bob'))
response = self.client.get(f'{url}/{resource.id}?include[]=executions', format='json')
response = self.client.get(f'{url_with_id}&include[]=executions', format='json')
self.assertEqual(response.status_code, 200)
self.assertIsNotNone(response.data['resource'].get('executions'))
self.assertEqual(response.data['resource'].get('executions'), expected_executions_results)
Expand All @@ -567,7 +576,7 @@ def test_base_resources(self):
self.assertEqual(6, resource.tkeywords.count())
# Admin
self.assertTrue(self.client.login(username='admin', password='admin'))
response = self.client.get(f"{url}/{resource.id}/", format='json')
response = self.client.get(f"{url_with_id}", format='json')
self.assertIsNotNone(response.data['resource']['tkeywords'])
self.assertEqual(6, len(response.data['resource']['tkeywords']))
self.assertListEqual(
Expand Down Expand Up @@ -721,15 +730,15 @@ def test_filter_resources(self):
self.assertTrue(self.client.login(username='admin', password='admin'))

# Filter by owner == bobby
response = self.client.get(f"{url}?filter{{owner.username}}=bobby", format='json')
response = self.client.get(f"{url}?filter{{owner.username}}=bobby&filter{{metadata_only}}=false", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 3)
# Pagination
self.assertEqual(len(response.data['resources']), 3)

# Filter by resource_type == document
response = self.client.get(f"{url}?filter{{resource_type}}=document", format='json')
response = self.client.get(f"{url}?filter{{resource_type}}=document&filter{{metadata_only}}=false", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 9)
Expand All @@ -738,7 +747,7 @@ def test_filter_resources(self):

# Filter by resource_type == layer and title like 'common morx'
response = self.client.get(
f"{url}?filter{{resource_type}}=dataset&filter{{title.icontains}}=common morx", format='json')
f"{url}?filter{{resource_type}}=dataset&filter{{title.icontains}}=common morx&filter{{metadata_only}}=false", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 1)
Expand All @@ -747,7 +756,7 @@ def test_filter_resources(self):

# Filter by Keywords
response = self.client.get(
f"{url}?filter{{keywords.name}}=here", format='json')
f"{url}?filter{{keywords.name}}=here&filter{{metadata_only}}=false", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 1)
Expand All @@ -756,7 +765,7 @@ def test_filter_resources(self):

# Filter by Metadata Regions
response = self.client.get(
f"{url}?filter{{regions.name.icontains}}=Italy", format='json')
f"{url}?filter{{regions.name.icontains}}=Italy&filter{{metadata_only}}=false", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 0)
Expand All @@ -765,29 +774,29 @@ def test_filter_resources(self):

# Filter by Metadata Categories
response = self.client.get(
f"{url}?filter{{category.identifier}}=elevation", format='json')
f"{url}?filter{{category.identifier}}=elevation&filter{{metadata_only}}=false", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 6)
# Pagination
self.assertEqual(len(response.data['resources']), 6)

# Extent Filter
response = self.client.get(f"{url}?page_size=26&extent=-180,-90,180,90", format='json')
response = self.client.get(f"{url}?page_size=26&extent=-180,-90,180,90&filter{{metadata_only}}=false", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 26)
# Pagination
self.assertEqual(len(response.data['resources']), 26)

response = self.client.get(f"{url}?page_size=26&extent=0,0,100,100", format='json')
response = self.client.get(f"{url}?page_size=26&extent=0,0,100,100&filter{{metadata_only}}=false", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 26)
# Pagination
self.assertEqual(len(response.data['resources']), 26)

response = self.client.get(f"{url}?page_size=26&extent=-10,-10,-1,-1", format='json')
response = self.client.get(f"{url}?page_size=26&extent=-10,-10,-1,-1&filter{{metadata_only}}=false", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 12)
Expand All @@ -797,7 +806,7 @@ def test_filter_resources(self):
# Extent Filter: Crossing Dateline
extent = "-180.0000,56.9689,-162.5977,70.7435,155.9180,56.9689,180.0000,70.7435"
response = self.client.get(
f"{url}?page_size=26&extent={extent}", format='json')
f"{url}?page_size=26&extent={extent}&filter{{metadata_only}}=false", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 12)
Expand All @@ -816,7 +825,7 @@ def test_sort_resources(self):
f"{url}?sort[]=title", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 26)
self.assertEqual(response.data['total'], 28)
# Pagination
self.assertEqual(len(response.data['resources']), 10)

Expand All @@ -830,7 +839,7 @@ def test_sort_resources(self):
f"{url}?sort[]=-title", format='json')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 5)
self.assertEqual(response.data['total'], 26)
self.assertEqual(response.data['total'], 28)
# Pagination
self.assertEqual(len(response.data['resources']), 10)

Expand Down Expand Up @@ -1663,7 +1672,7 @@ def test_embed_urls(self):

resources = ResourceBase.objects.all()
for resource in resources:
url = reverse('base-resources-detail', kwargs={'pk': resource.pk})
url = "{base_url}?{params}".format(base_url=reverse('base-resources-detail', kwargs={'pk': resource.pk}), params="filter{metadata_only}=false")
response = self.client.get(url, format='json')
if resource.title.endswith('metadata true'):
self.assertEqual(response.status_code, 404)
Expand Down
32 changes: 32 additions & 0 deletions geonode/security/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1505,6 +1505,38 @@ def test_get_visible_resources_should_return_updated_resource_with_metadata_only
actual = get_visible_resources(queryset=layers, user=get_user_model().objects.get(username=self.user))
self.assertEqual(layers.filter(dirty_state=False).count(), len(actual))

def test_get_visible_resources_should_return_resource_with_metadata_only_true(self):
'''
If metadata only is provided, it should return only the metadata resources
'''
try:
dataset = create_single_dataset("dataset_with_metadata_only_True")
dataset.metadata_only = True
dataset.save()

layers = Dataset.objects.all()
actual = get_visible_resources(queryset=layers, metadata_only=True, user=get_user_model().objects.get(username=self.user))
self.assertEqual(1, actual.count())
finally:
if dataset:
dataset.delete()

def test_get_visible_resources_should_return_resource_with_metadata_only_none(self):
'''
If metadata only is provided, it should return only the metadata resources
'''
try:
dataset = create_single_dataset("dataset_with_metadata_only_True")
dataset.metadata_only = True
dataset.save()

layers = Dataset.objects.all()
actual = get_visible_resources(queryset=layers, metadata_only=None, user=get_user_model().objects.get(username=self.user))
self.assertEqual(layers.count(), actual.count())
finally:
if dataset:
dataset.delete()

@override_settings(
ADMIN_MODERATE_UPLOADS=True,
RESOURCE_PUBLISHING=True,
Expand Down
8 changes: 5 additions & 3 deletions geonode/security/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,11 @@ def get_visible_resources(queryset,
except Exception:
pass

# Hide Dirty State Resources
filter_set = queryset.filter(
Q(dirty_state=False) & Q(metadata_only=metadata_only))
filter_set = queryset.filter(dirty_state=False)

if metadata_only is not None:
# Hide Dirty State Resources
filter_set = filter_set.filter(metadata_only=metadata_only)

if not is_admin:
if user:
Expand Down