diff --git a/docker-compose-test.yml b/docker-compose-test.yml index 8edc8b2194b..28738fa28b6 100644 --- a/docker-compose-test.yml +++ b/docker-compose-test.yml @@ -3,7 +3,7 @@ version: '3.9' # Common Django template for GeoNode and Celery services below x-common-django: &default-common-django - image: geonode/geonode:local + image: geonode/geonode:4.2.5 build: context: ./ dockerfile: Dockerfile diff --git a/docker-compose.yml b/docker-compose.yml index e1e9341f69b..25de60e23cc 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -3,10 +3,10 @@ version: '3.9' # Common Django template for GeoNode and Celery services below x-common-django: &default-common-django - image: geonode/geonode:4.2.3 - build: - context: ./ - dockerfile: Dockerfile + image: geonode/geonode:4.2.5 + #build: + # context: ./ + # dockerfile: Dockerfile restart: unless-stopped env_file: - .env @@ -118,7 +118,7 @@ services: condition: service_healthy data-dir-conf: - image: geonode/geoserver_data:2.24.2-v1 + image: geonode/geoserver_data:2.24.2-v2 container_name: gsconf4${COMPOSE_PROJECT_NAME} entrypoint: sleep infinity volumes: diff --git a/geonode/__init__.py b/geonode/__init__.py index 08d6c164079..9f9489dd19f 100644 --- a/geonode/__init__.py +++ b/geonode/__init__.py @@ -19,7 +19,7 @@ import os -__version__ = (4, 2, 3, "final", 0) +__version__ = (4, 2, 6, "dev", 0) default_app_config = "geonode.apps.AppConfig" diff --git a/geonode/base/api/tests.py b/geonode/base/api/tests.py index bbff21b3267..94dae413cd5 100644 --- a/geonode/base/api/tests.py +++ b/geonode/base/api/tests.py @@ -1951,10 +1951,10 @@ def test_set_resource_thumbnail(self): self.assertEqual(response.json(), "The url must be of an image with format (png, jpeg or jpg)") # using Base64 data as an ASCII byte string - data["file"] = ( - "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAABHNCSVQICAgI\ + data[ + "file" + ] = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAABHNCSVQICAgI\ fAhkiAAAABl0RVh0U29mdHdhcmUAZ25vbWUtc2NyZWVuc2hvdO8Dvz4AAAANSURBVAiZYzAxMfkPAALYAZzx61+bAAAAAElFTkSuQmCC" - ) with patch("geonode.base.models.is_monochromatic_image") as _mck: _mck.return_value = False response = self.client.put(url, data=data, format="json") diff --git a/geonode/base/api/views.py b/geonode/base/api/views.py index d8b10e27763..bb417f3b3b5 100644 --- a/geonode/base/api/views.py +++ b/geonode/base/api/views.py @@ -1505,7 +1505,6 @@ def linked_resources(self, request, pk, *args, **kwargs): def base_linked_resources(instance, user, params): - try: resource_type = params.get("resource_type") link_type = params.get("link_type") diff --git a/geonode/base/models.py b/geonode/base/models.py index 5768df93281..c75918c755e 100644 --- a/geonode/base/models.py +++ b/geonode/base/models.py @@ -1707,8 +1707,7 @@ def set_contact_roles_from_metadata_edit(self, resource_base_form) -> bool: failed = False for role in self.get_multivalue_role_property_names(): try: - if resource_base_form.cleaned_data[role].exists(): - self.__setattr__(role, resource_base_form.cleaned_data[role]) + self.__setattr__(role, resource_base_form.cleaned_data[role]) except AttributeError: logger.warning(f"unable to set contact role {role} for {self} ...") failed = True @@ -1753,9 +1752,13 @@ def __create_role__( ContactRole.objects.filter(role=role, resource=self).delete() return __create_role__(self, role, user_profile) - elif isinstance(user_profile, list) and all(isinstance(x, get_user_model()) for x in user_profile): + elif isinstance(user_profile, list) and all( + get_user_model().objects.filter(username=x).exists() for x in user_profile + ): ContactRole.objects.filter(role=role, resource=self).delete() - return [__create_role__(self, role, profile) for profile in user_profile] + return [ + __create_role__(self, role, user) for user in get_user_model().objects.filter(username__in=user_profile) + ] elif user_profile is None: ContactRole.objects.filter(role=role, resource=self).delete() diff --git a/geonode/base/widgets.py b/geonode/base/widgets.py index b1281de7dd7..a4e32bb94d4 100644 --- a/geonode/base/widgets.py +++ b/geonode/base/widgets.py @@ -1,6 +1,7 @@ from typing import List from dal_select2_taggit.widgets import TaggitSelect2 +from django.http.request import QueryDict class TaggitSelect2Custom(TaggitSelect2): @@ -32,9 +33,8 @@ def value_from_datadict(self, data, files, name) -> List[str]: returns list of selected elements """ - try: - ret_list = data[name] - except KeyError: - ret_list = [] - finally: - return ret_list + if type(data) is dict and name in data: + return data[name] + elif type(data) is QueryDict: + return data.getlist(name) + return [] diff --git a/geonode/documents/templates/layouts/doc_panels.html b/geonode/documents/templates/layouts/doc_panels.html index d354531d314..c6eebcb6723 100644 --- a/geonode/documents/templates/layouts/doc_panels.html +++ b/geonode/documents/templates/layouts/doc_panels.html @@ -596,22 +596,6 @@ {% endblock document_more_contact_roles %} - -
-
{% trans "Responsible and Permissions" %}
-
-
- - - {{ document_form.owner }} -
-
- - - {{ document_form.metadata_author }} -
-
-
diff --git a/geonode/geoapps/api/serializers.py b/geonode/geoapps/api/serializers.py index 1a1e42e7c8a..c9fde60158d 100644 --- a/geonode/geoapps/api/serializers.py +++ b/geonode/geoapps/api/serializers.py @@ -61,12 +61,6 @@ def extra_create_checks(self, validated_data): self.extra_update_checks(validated_data) - def validate(self, data): - request = self.context.get("request") - if request: - data["owner"] = request.user - return data - def _sanitize_validated_data(self, validated_data, instance=None): # Extract users' profiles _user_profiles = {} diff --git a/geonode/geoapps/api/tests.py b/geonode/geoapps/api/tests.py index 4b3a545f182..4d38680e743 100644 --- a/geonode/geoapps/api/tests.py +++ b/geonode/geoapps/api/tests.py @@ -205,3 +205,48 @@ def test_create_checks_duplicated(self): self.assertEqual(exp.exception.category, "geoapp_api") self.assertEqual(exp.exception.default_code, "geoapp_exception") self.assertEqual(str(exp.exception.detail), "A GeoApp with the same 'name' already exists!") + + def test_geoapp_creation_owner_is_mantained(self): + """ + https://github.com/GeoNode/geonode/issues/12261 + The geoapp owner should be mantained even if another + user save the instance. + """ + + url = f"{reverse('geoapps-list')}?include[]=data" + data = { + "name": "Test Create", + "title": "Test Create", + "resource_type": "geostory", + "extent": {"coords": [1123692.0, 5338214.0, 1339852.0, 5482615.0], "srid": "EPSG:3857"}, + } + + self.assertTrue(self.client.login(username="norman", password="norman")) + + response = self.client.post(url, data=data, format="json") + + self.assertEqual(201, response.status_code) + # let's check that the owner is the request one + self.assertEqual("norman", response.json()["geoapp"]["owner"]["username"]) + + # let's change the user of the request + self.assertTrue(self.client.login(username="admin", password="admin")) + + sut = GeoApp.objects.get(pk=response.json()["geoapp"]["pk"]) + # Update: PATCH + # we ensure that norman is the resource owner + self.assertEqual("norman", sut.owner.username) + + url = reverse("geoapps-detail", kwargs={"pk": sut.pk}) + data = {"blob": {"test_data": {"test": ["test_4", "test_5", "test_6"]}}} + # sending the update of the geoapp + response = self.client.patch(url, data=json.dumps(data), content_type="application/json") + + self.assertEqual(response.status_code, 200) + + # ensure that the value of the owner is not changed + sut.refresh_from_db() + self.assertEqual("norman", sut.owner.username) + + # cleanup + sut.delete() diff --git a/geonode/geoapps/api/views.py b/geonode/geoapps/api/views.py index fbb6a5bf481..4d3099ae502 100644 --- a/geonode/geoapps/api/views.py +++ b/geonode/geoapps/api/views.py @@ -59,3 +59,11 @@ class GeoAppViewSet(DynamicModelViewSet): queryset = GeoApp.objects.all().order_by("-created") serializer_class = GeoAppSerializer pagination_class = GeoNodeApiPagination + + def perform_create(self, serializer): + """ + The owner is not passed from the FE + so we force the request.user to be the owner + in creation + """ + return serializer.save(owner=self.request.user) diff --git a/geonode/resource/manager.py b/geonode/resource/manager.py index ad64452d6dc..5673409488e 100644 --- a/geonode/resource/manager.py +++ b/geonode/resource/manager.py @@ -859,7 +859,12 @@ def _safe_assign_perm(perm, user_or_group, obj=None): ) else: for user_group in get_user_groups(_owner): - if not skip_registered_members_common_group(user_group): + # if AdvancedSecurityWorkflowManager.is_auto_publishing_workflow() is False, + # means that at least one config of the advanced workflow is set, which means that users group get view_permissions + if ( + not skip_registered_members_common_group(user_group) + and not AdvancedSecurityWorkflowManager.is_auto_publishing_workflow() + ): _safe_assign_perm("view_resourcebase", user_group, _resource.get_self_resource()) _prev_perm = ( _perm_spec["groups"].get(user_group, []) if "groups" in _perm_spec else [] @@ -883,7 +888,12 @@ def _safe_assign_perm(perm, user_or_group, obj=None): ) else: for user_group in get_user_groups(_owner): - if not skip_registered_members_common_group(user_group): + # if AdvancedSecurityWorkflowManager.is_auto_publishing_workflow() is False, + # means that at least one config of the advanced workflow is set, which means that users group get view_permissions + if ( + not skip_registered_members_common_group(user_group) + and not AdvancedSecurityWorkflowManager.is_auto_publishing_workflow() + ): _safe_assign_perm( "download_resourcebase", user_group, _resource.get_self_resource() ) diff --git a/geonode/security/models.py b/geonode/security/models.py index 2a878368827..de9ece62f6d 100644 --- a/geonode/security/models.py +++ b/geonode/security/models.py @@ -201,7 +201,12 @@ def set_default_permissions(self, owner=None, created=False): perm_spec["groups"][anonymous_group] = ["view_resourcebase"] else: for user_group in user_groups: - if not skip_registered_members_common_group(user_group): + # if aswm.is_auto_publishing_workflow() is False, means that at least one config of the advanced workflow + # is set, which means that users group get view_permissions + if ( + not skip_registered_members_common_group(user_group) + and not AdvancedSecurityWorkflowManager.is_auto_publishing_workflow() + ): perm_spec["groups"][user_group] = ["view_resourcebase"] anonymous_can_download = settings.DEFAULT_ANONYMOUS_DOWNLOAD_PERMISSION @@ -209,7 +214,12 @@ def set_default_permissions(self, owner=None, created=False): perm_spec["groups"][anonymous_group] = ["view_resourcebase", "download_resourcebase"] else: for user_group in user_groups: - if not skip_registered_members_common_group(user_group): + # if aswm.is_auto_publishing_workflow() is False, means that at least one config of the advanced workflow + # is set, which means that users group get view_permissions + if ( + not skip_registered_members_common_group(user_group) + and not AdvancedSecurityWorkflowManager.is_auto_publishing_workflow() + ): perm_spec["groups"][user_group] = ["view_resourcebase", "download_resourcebase"] AdvancedSecurityWorkflowManager.handle_moderated_uploads(self.uuid, instance=self) diff --git a/geonode/security/tests.py b/geonode/security/tests.py index 433f4aa6de7..d621a77a13d 100644 --- a/geonode/security/tests.py +++ b/geonode/security/tests.py @@ -20,6 +20,7 @@ import json import base64 import logging +import uuid import requests import importlib import mock @@ -2267,6 +2268,53 @@ def test_permissions_on_user_role_promote_to_manager_only_RESOURCE_PUBLISHING_ac set(expected_perms), set(perms_got), msg=f"use case #0 - user: {authorized_subject.username}" ) + @override_settings(DEFAULT_ANONYMOUS_VIEW_PERMISSION=False) + def test_if_anonymoys_default_perms_is_false_should_not_assign_perms_to_user_group(self): + """ + if DEFAULT_ANONYMOUS_VIEW_PERMISSION is False, the user's group should not get any permission + """ + + resource = resource_manager.create(str(uuid.uuid4), Dataset, defaults={"owner": self.group_member}) + self.assertFalse(self.group_profile.group in resource.get_all_level_info()["groups"].keys()) + + @override_settings(DEFAULT_ANONYMOUS_DOWNLOAD_PERMISSION=False) + def test_if_anonymoys_default_download_perms_is_false_should_not_assign_perms_to_user_group(self): + """ + if DEFAULT_ANONYMOUS_DOWNLOAD_PERMISSION is False, the user's group should not get any permission + """ + + resource = resource_manager.create(str(uuid.uuid4), Dataset, defaults={"owner": self.group_member}) + self.assertFalse(self.group_profile.group in resource.get_all_level_info()["groups"].keys()) + + @override_settings(DEFAULT_ANONYMOUS_DOWNLOAD_PERMISSION=False) + @override_settings(RESOURCE_PUBLISHING=True) + def test_if_anonymoys_default_perms_is_false_should_assign_perms_to_user_group_if_advanced_workflow_is_on(self): + """ + if DEFAULT_ANONYMOUS_DOWNLOAD_PERMISSION is False and the advanced workflow is activate + the user's group should get the view and download permission + """ + + resource = resource_manager.create(str(uuid.uuid4), Dataset, defaults={"owner": self.group_member}) + self.assertTrue(self.group_profile.group in resource.get_all_level_info()["groups"].keys()) + group_val = resource.get_all_level_info()["groups"][self.group_profile.group] + self.assertSetEqual({"view_resourcebase", "download_resourcebase"}, set(group_val)) + + @override_settings(DEFAULT_ANONYMOUS_VIEW_PERMISSION=False) + @override_settings(ADMIN_MODERATE_UPLOADS=True) + def test_if_anonymoys_default_perms_is_false_should_assign_perms_to_user_group_if_advanced_workflow_is_on_moderate( + self, + ): + """ + if DEFAULT_ANONYMOUS_VIEW_PERMISSION is False and the advanced workflow is activate + the user's group should get the view and download permission + """ + + resource = resource_manager.create(str(uuid.uuid4), Dataset, defaults={"owner": self.group_member}) + + self.assertTrue(self.group_profile.group in resource.get_all_level_info()["groups"].keys()) + group_val = resource.get_all_level_info()["groups"][self.group_profile.group] + self.assertSetEqual({"view_resourcebase", "download_resourcebase"}, set(group_val)) + @override_settings(RESOURCE_PUBLISHING=True) @override_settings(ADMIN_MODERATE_UPLOADS=True) diff --git a/geonode/security/utils.py b/geonode/security/utils.py index 91b567562de..312c2a417fb 100644 --- a/geonode/security/utils.py +++ b/geonode/security/utils.py @@ -221,8 +221,7 @@ def get_geoapp_subtypes(): def skip_registered_members_common_group(user_group): - _members_group_name = groups_settings.REGISTERED_MEMBERS_GROUP_NAME - if (settings.RESOURCE_PUBLISHING or settings.ADMIN_MODERATE_UPLOADS) and _members_group_name == user_group.name: + if groups_settings.REGISTERED_MEMBERS_GROUP_NAME == user_group.name: return True return False diff --git a/requirements.txt b/requirements.txt index 25559512627..70f7db16c82 100644 --- a/requirements.txt +++ b/requirements.txt @@ -88,8 +88,8 @@ pinax-notifications==6.0.0 pinax-ratings==4.0.0 # GeoNode org maintained apps. -django-geonode-mapstore-client==4.2.0 -geonode-importer==1.0.8 +-e git+https://github.com/GeoNode/geonode-mapstore-client.git@4.2.x#egg=django_geonode_mapstore_client +-e git+https://github.com/GeoNode/geonode-importer.git@master#egg=geonode-importer django-avatar==7.1.1 geonode-oauth-toolkit==2.2.2 geonode-user-messages==2.0.2 diff --git a/setup.cfg b/setup.cfg index 99d74f19ed4..1e9bdb2e87c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -114,8 +114,8 @@ install_requires = pinax-ratings==4.0.0 # GeoNode org maintained apps. - django-geonode-mapstore-client==4.2.0 - geonode-importer==1.0.8 + django-geonode-mapstore-client>=4.2.0,<5.0.0 + geonode-importer>=1.0.8 django-avatar==7.1.1 geonode-oauth-toolkit==2.2.2 geonode-user-messages==2.0.2