From 23a40ce1c90c08528471d35b79967a8c05a013af Mon Sep 17 00:00:00 2001 From: marthamareal Date: Wed, 21 Apr 2021 00:01:56 +0300 Subject: [PATCH 1/4] - Create contributors group - Assign users to contributors group by default --- geonode/base/api/tests.py | 20 ++++ geonode/base/populate_test_data.py | 14 ++- geonode/groups/admin.py | 2 - geonode/groups/models.py | 4 +- .../commands/setcontributorsgroup.py | 28 +++++ geonode/people/models.py | 22 +++- geonode/people/signals.py | 8 +- geonode/security/models.py | 101 ++++++++---------- geonode/security/permissions.py | 28 +++++ geonode/security/utils.py | 4 +- geonode/tests/test_rest_api.py | 6 +- 11 files changed, 168 insertions(+), 69 deletions(-) create mode 100644 geonode/people/management/commands/setcontributorsgroup.py create mode 100644 geonode/security/permissions.py diff --git a/geonode/base/api/tests.py b/geonode/base/api/tests.py index c1a1d37ed84..4b4009bbf69 100644 --- a/geonode/base/api/tests.py +++ b/geonode/base/api/tests.py @@ -169,6 +169,11 @@ def test_users_list(self): self.assertEqual(response.data['user']['username'], 'admin') self.assertIsNotNone(response.data['user']['avatar']) + # anonymous users are not in contributors group + url = reverse('users-detail', kwargs={'pk': -1}) + response = self.client.get(url, format='json') + self.assertNotIn('ADD_RESOURCE', response.data['user']['perms']) + # Bobby self.assertTrue(self.client.login(username='bobby', password='bob')) # Bobby cannot access other users' details @@ -192,6 +197,21 @@ def test_users_list(self): logger.debug(response.data) self.assertEqual(response.data['user']['username'], 'bobby') self.assertIsNotNone(response.data['user']['avatar']) + # default contributor group_perm is returned in perms + self.assertIn('ADD_RESOURCE', response.data['user']['perms']) + + def test_register_users(self): + """ + Ensure users are created with default groups. + """ + url = reverse('users-list') + user_data = { + 'username': 'new_user', + } + response = self.client.post(url, data=user_data, format='json') + self.assertEqual(response.status_code, 201) + # default contributor group_perm is returned in perms + self.assertIn('ADD_RESOURCE', response.data['user']['perms']) def test_base_resources(self): """ diff --git a/geonode/base/populate_test_data.py b/geonode/base/populate_test_data.py index 208a08104d0..ba922ad0dfc 100644 --- a/geonode/base/populate_test_data.py +++ b/geonode/base/populate_test_data.py @@ -30,9 +30,10 @@ from django.db import transaction from django.utils import timezone from django.contrib.gis.geos import Polygon -from django.contrib.auth.models import Group +from django.contrib.auth.models import Permission, Group from django.core.serializers import serialize from django.contrib.auth import get_user_model +from django.contrib.contenttypes.models import ContentType from django.core.files.uploadedfile import SimpleUploadedFile from geonode import geoserver # noqa @@ -154,6 +155,14 @@ def create_models(type=None, integration=False): with transaction.atomic(): map_data, user_data, people_data, layer_data, document_data = create_fixtures() anonymous_group, created = Group.objects.get_or_create(name='anonymous') + cont_group, created = Group.objects.get_or_create(name='contributors') + ctype = ContentType.objects.get_for_model(cont_group) + perm, created = Permission.objects.get_or_create( + codename='base_addresourcebase', + name='Can add resources', + content_type=ctype + ) + cont_group.permissions.add(perm) logger.debug("[SetUp] Get or create user admin") u, created = get_user_model().objects.get_or_create(username='admin') u.set_password('admin') @@ -172,6 +181,9 @@ def create_models(type=None, integration=False): u.last_name = last_name u.save() u.groups.add(anonymous_group) + + if not (u.is_superuser or u.is_staff or u.is_anonymous): + u.groups.add(cont_group) users.append(u) logger.debug(f"[SetUp] Add group {anonymous_group}") diff --git a/geonode/groups/admin.py b/geonode/groups/admin.py index f5d76e0c486..e106c6d688d 100644 --- a/geonode/groups/admin.py +++ b/geonode/groups/admin.py @@ -19,7 +19,6 @@ ######################################################################### from django.contrib import admin -from django.contrib.auth.models import Group from modeltranslation.admin import TranslationAdmin from geonode.base.admin import set_user_and_group_layer_permission @@ -44,5 +43,4 @@ class GroupProfileAdmin(admin.ModelAdmin): actions = [set_user_and_group_layer_permission] -admin.site.unregister(Group) admin.site.register(models.GroupProfile, GroupProfileAdmin) diff --git a/geonode/groups/models.py b/geonode/groups/models.py index 7a082314e92..64606bfcc33 100644 --- a/geonode/groups/models.py +++ b/geonode/groups/models.py @@ -268,7 +268,7 @@ def delete(self, *args, **kwargs): def promote(self, *args, **kwargs): self.role = "manager" if settings.ADMIN_MODERATE_UPLOADS or settings.RESOURCE_PUBLISHING: - from geonode.security.models import ADMIN_PERMISSIONS + from geonode.security.permissions import ADMIN_PERMISSIONS queryset = get_objects_for_user( self.user, 'base.view_resourcebase').filter(group=self.group.group) for _r in queryset.exclude(owner=self.user): @@ -279,7 +279,7 @@ def promote(self, *args, **kwargs): def demote(self, *args, **kwargs): self.role = "member" if settings.ADMIN_MODERATE_UPLOADS or settings.RESOURCE_PUBLISHING: - from geonode.security.models import ADMIN_PERMISSIONS + from geonode.security.permissions import ADMIN_PERMISSIONS queryset = get_objects_for_user( self.user, 'base.view_resourcebase').filter(group=self.group.group) for _r in queryset.exclude(owner=self.user): diff --git a/geonode/people/management/commands/setcontributorsgroup.py b/geonode/people/management/commands/setcontributorsgroup.py new file mode 100644 index 00000000000..61bfefa0bdc --- /dev/null +++ b/geonode/people/management/commands/setcontributorsgroup.py @@ -0,0 +1,28 @@ +from django.contrib.auth import get_user_model +from django.contrib.auth.models import Group, Permission +from django.contrib.contenttypes.models import ContentType +from django.core.management.base import BaseCommand + + +class Command(BaseCommand): + help = ( + "Assign existing users to contributors group" + ) + + def handle(self, *args, **options): + cont_group, created = Group.objects.get_or_create(name='contributors') + ctype = ContentType.objects.get_for_model(cont_group) + perm, created = Permission.objects.get_or_create( + codename='base_addresourcebase', + name='Can add resources', + content_type=ctype, + ) + if perm: + cont_group.permissions.add(perm) + # Exclude admins and anonymous user + users_to_update = get_user_model().objects.filter( + pk__gt=0, is_staff=False, is_superuser=False + ) + for user in users_to_update: + cont_group.user_set.add(user) + self.stdout.write("All users are assigned to contributors group") diff --git a/geonode/people/models.py b/geonode/people/models.py index fe370947ccd..54bb19170b8 100644 --- a/geonode/people/models.py +++ b/geonode/people/models.py @@ -30,13 +30,15 @@ from django.urls import reverse from django.contrib.sites.models import Site from django.utils.translation import ugettext_lazy as _ -from django.contrib.auth.models import AbstractUser, UserManager +from django.contrib.auth.models import AbstractUser, Permission, UserManager from django.contrib.auth.signals import user_logged_in, user_logged_out from taggit.managers import TaggableManager from geonode.base.enumerations import COUNTRIES +from geonode.base.models import Configuration from geonode.groups.models import GroupProfile +from geonode.security.permissions import PERMISSIONS, READ_ONLY_AFFECTED_PERMISSIONS from allauth.account.signals import user_signed_up from allauth.socialaccount.signals import social_account_added @@ -196,7 +198,23 @@ def location(self): @property def perms(self): - return [] + if self.is_superuser or self.is_staff: + # return all permissions for admins + perms = PERMISSIONS.values() + else: + user_groups = self.groups.values_list('name', flat=True) + group_perms = Permission.objects.filter( + group__name__in=user_groups + ).distinct().values_list('codename', flat=True) + # return constant names defined by GeoNode + perms = [PERMISSIONS[db_perm] for db_perm in group_perms] + + # check READ_ONLY mode + config = Configuration.load() + if config.read_only: + # exclude permissions affected by readonly + perms = [perm for perm in perms if perm not in READ_ONLY_AFFECTED_PERMISSIONS] + return perms def save(self, *args, **kwargs): super(Profile, self).save(*args, **kwargs) diff --git a/geonode/people/signals.py b/geonode/people/signals.py index 4c90401bb91..e1583149d97 100644 --- a/geonode/people/signals.py +++ b/geonode/people/signals.py @@ -125,12 +125,16 @@ def notify_admins_new_signup(sender, **kwargs): def profile_post_save(instance, sender, **kwargs): """ - Make sure the user belongs by default to the anonymous group. - This will make sure that anonymous permissions will be granted to the new users. + Make sure the user belongs by default to the anonymous and contributors groups. + This will make sure that anonymous and contributors permissions will be granted to the new users. """ from django.contrib.auth.models import Group anon_group, created = Group.objects.get_or_create(name='anonymous') instance.groups.add(anon_group) + is_anonymous = instance.username == 'AnonymousUser' + if not (instance.is_staff or instance.is_superuser or is_anonymous): + cont_group, created = Group.objects.get_or_create(name='contributors') + instance.groups.add(cont_group) if groups_settings.AUTO_ASSIGN_REGISTERED_MEMBERS_TO_REGISTERED_MEMBERS_GROUP_AT == 'activation': created = kwargs.get('created', False) diff --git a/geonode/security/models.py b/geonode/security/models.py index 01791e4a8a7..fe243da6cb8 100644 --- a/geonode/security/models.py +++ b/geonode/security/models.py @@ -19,12 +19,15 @@ ######################################################################### import logging import traceback +import operator +from functools import reduce from django.conf import settings from django.contrib.auth.models import Group, Permission from django.contrib.auth import get_user_model from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ObjectDoesNotExist +from django.db.models import Q from geonode.groups.conf import settings as groups_settings @@ -37,6 +40,12 @@ from geonode.groups.models import GroupProfile +from .permissions import ( + ADMIN_PERMISSIONS, + LAYER_ADMIN_PERMISSIONS, + VIEW_PERMISSIONS, +) + from .utils import ( get_users_with_perms, set_owner_permissions, @@ -48,24 +57,6 @@ logger = logging.getLogger("geonode.security.models") -VIEW_PERMISSIONS = [ - 'view_resourcebase', - 'download_resourcebase', -] - -ADMIN_PERMISSIONS = [ - 'change_resourcebase_metadata', - 'change_resourcebase', - 'delete_resourcebase', - 'change_resourcebase_permissions', - 'publish_resourcebase', -] - -LAYER_ADMIN_PERMISSIONS = [ - 'change_layer_data', - 'change_layer_style' -] - class PermissionLevelError(Exception): pass @@ -396,6 +387,10 @@ 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) PERMISSIONS_TO_FETCH = VIEW_PERMISSIONS + ADMIN_PERMISSIONS + LAYER_ADMIN_PERMISSIONS @@ -405,48 +400,44 @@ def get_user_perms(self, user): ).values_list('codename', flat=True) # Don't filter for admin users - if user.is_superuser or user.is_staff: - return resource_perms - - user_model = get_user_obj_perms_model(self) - user_resource_perms = user_model.objects.filter( - object_pk=self.pk, - content_type_id=ctype.id, - user__username=str(user), - permission__codename__in=resource_perms - ) - - # get user's implicit perms for anyone flag - implicit_perms = get_perms(user, self) + if not (user.is_superuser or user.is_staff): + user_model = get_user_obj_perms_model(self) + user_resource_perms = user_model.objects.filter( + object_pk=self.pk, + content_type_id=ctype.id, + user__username=str(user), + permission__codename__in=resource_perms + ) + # get user's implicit perms for anyone flag + implicit_perms = get_perms(user, self) + + 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 or user.is_staff): + 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 user_resource_perms.union( - user_model.objects.filter(permission__codename__in=implicit_perms) - ).values_list('permission__codename', flat=True) + return resource_perms def user_can(self, user, permission): """ Checks if a has a given permission to the resource """ - # To avoid circular import - from geonode.base.models import Configuration - - config = Configuration.load() - # Check read-only status if given permission is for edit, change or publish - perm_prefixes = ['change', 'delete', 'publish'] - if any(prefix in permission for prefix in perm_prefixes): - if config.read_only: - return False resource = self.get_self_resource() user_perms = self.get_user_perms(user).union(resource.get_user_perms(user)) - is_admin = user.is_superuser - is_staff = user.is_staff - is_owner = user == self.owner - try: - is_manager = user.groupmember_set.all().filter( - role='manager').exists() - except Exception: - is_manager = False - has_access = is_admin or is_staff or is_owner or is_manager or user.has_perm(permission, obj=self) - if permission in user_perms or has_access: - return True - return False + + if permission not in user_perms: + # TODO cater for permissions with syntax base.permission_codename + # eg 'base.change_resourcebase' + return False + + return True diff --git a/geonode/security/permissions.py b/geonode/security/permissions.py new file mode 100644 index 00000000000..5fc3f083d27 --- /dev/null +++ b/geonode/security/permissions.py @@ -0,0 +1,28 @@ +# Permissions mapping +PERMISSIONS = { + 'base_addresourcebase': 'ADD_RESOURCE', + } + +# The following permissions will be filtered out when READ_ONLY mode is active +READ_ONLY_AFFECTED_PERMISSIONS = [ + 'ADD_RESOURCE', +] + +# Permissions on resources +VIEW_PERMISSIONS = [ + 'view_resourcebase', + 'download_resourcebase', +] + +ADMIN_PERMISSIONS = [ + 'change_resourcebase_metadata', + 'change_resourcebase', + 'delete_resourcebase', + 'change_resourcebase_permissions', + 'publish_resourcebase', +] + +LAYER_ADMIN_PERMISSIONS = [ + 'change_layer_data', + 'change_layer_style' +] diff --git a/geonode/security/utils.py b/geonode/security/utils.py index 84ad17d4b41..ed53246fceb 100644 --- a/geonode/security/utils.py +++ b/geonode/security/utils.py @@ -112,7 +112,7 @@ def get_users_with_perms(obj): """ Override of the Guardian get_users_with_perms """ - from .models import (VIEW_PERMISSIONS, ADMIN_PERMISSIONS, LAYER_ADMIN_PERMISSIONS) + from .permissions import (VIEW_PERMISSIONS, ADMIN_PERMISSIONS, LAYER_ADMIN_PERMISSIONS) ctype = ContentType.objects.get_for_model(obj) permissions = {} PERMISSIONS_TO_FETCH = VIEW_PERMISSIONS + ADMIN_PERMISSIONS + LAYER_ADMIN_PERMISSIONS @@ -667,7 +667,7 @@ def sync_geofence_with_guardian(layer, perms, user=None, group=None, group_perms def set_owner_permissions(resource, members=None): """assign all admin permissions to the owner""" - from .models import (VIEW_PERMISSIONS, ADMIN_PERMISSIONS, LAYER_ADMIN_PERMISSIONS) + from .permissions import (VIEW_PERMISSIONS, ADMIN_PERMISSIONS, LAYER_ADMIN_PERMISSIONS) if resource.polymorphic_ctype: # Owner & Manager Admin Perms admin_perms = VIEW_PERMISSIONS + ADMIN_PERMISSIONS diff --git a/geonode/tests/test_rest_api.py b/geonode/tests/test_rest_api.py index 74a5b27ee0f..f917ee2c5dd 100644 --- a/geonode/tests/test_rest_api.py +++ b/geonode/tests/test_rest_api.py @@ -62,7 +62,7 @@ def test_regular_user_hide_private(self, super_mock, mocked_profile): mock_bundle.configure_mock(**attrs) groups = GroupAuthorization().read_list(['not_empty_but_fake'], mock_bundle) - self.assertEqual(1, groups.count()) + self.assertEqual(2, groups.count()) @patch('geonode.api.authorization.ApiLockdownAuthorization.read_list', return_value=Group.objects.exclude(name='anonymous')) @@ -80,7 +80,7 @@ def test_regular_user(self, super_mock, mocked_profile): mock_bundle.configure_mock(**attrs) groups = GroupAuthorization().read_list(['not_empty_but_fake'], mock_bundle) - self.assertEqual(2, groups.count()) + self.assertEqual(3, groups.count()) @patch('geonode.api.authorization.ApiLockdownAuthorization.read_list', return_value=Group.objects.exclude(name='anonymous')) @@ -98,7 +98,7 @@ def test_anonymous_user(self, super_mock, mocked_profile): mock_bundle.configure_mock(**attrs) groups = GroupAuthorization().read_list(['not_empty_but_fake'], mock_bundle) - self.assertEqual(1, groups.count()) + self.assertEqual(2, groups.count()) class TestGroupProfileResAuthorization(GeoNodeBaseTestSupport): From d19622640b3ba0e403dc3cdf51802faaab393ffc Mon Sep 17 00:00:00 2001 From: marthamareal Date: Mon, 3 May 2021 16:24:17 +0300 Subject: [PATCH 2/4] use migration to set contributors --- geonode/base/api/tests.py | 6 +-- geonode/base/fixtures/group_test_data.json | 4 +- .../commands/setcontributorsgroup.py | 28 ----------- .../migrations/0032_set_contributors_group.py | 49 +++++++++++++++++++ geonode/security/permissions.py | 4 +- 5 files changed, 56 insertions(+), 35 deletions(-) delete mode 100644 geonode/people/management/commands/setcontributorsgroup.py create mode 100644 geonode/people/migrations/0032_set_contributors_group.py diff --git a/geonode/base/api/tests.py b/geonode/base/api/tests.py index 4b4009bbf69..b485d1f7479 100644 --- a/geonode/base/api/tests.py +++ b/geonode/base/api/tests.py @@ -172,7 +172,7 @@ def test_users_list(self): # anonymous users are not in contributors group url = reverse('users-detail', kwargs={'pk': -1}) response = self.client.get(url, format='json') - self.assertNotIn('ADD_RESOURCE', response.data['user']['perms']) + self.assertNotIn('add_resource', response.data['user']['perms']) # Bobby self.assertTrue(self.client.login(username='bobby', password='bob')) @@ -198,7 +198,7 @@ def test_users_list(self): self.assertEqual(response.data['user']['username'], 'bobby') self.assertIsNotNone(response.data['user']['avatar']) # default contributor group_perm is returned in perms - self.assertIn('ADD_RESOURCE', response.data['user']['perms']) + self.assertIn('add_resource', response.data['user']['perms']) def test_register_users(self): """ @@ -211,7 +211,7 @@ def test_register_users(self): response = self.client.post(url, data=user_data, format='json') self.assertEqual(response.status_code, 201) # default contributor group_perm is returned in perms - self.assertIn('ADD_RESOURCE', response.data['user']['perms']) + self.assertIn('add_resource', response.data['user']['perms']) def test_base_resources(self): """ diff --git a/geonode/base/fixtures/group_test_data.json b/geonode/base/fixtures/group_test_data.json index 914d8e9e57f..99ccd41160e 100644 --- a/geonode/base/fixtures/group_test_data.json +++ b/geonode/base/fixtures/group_test_data.json @@ -8,7 +8,7 @@ } }, { - "pk": 2, + "pk": 3, "model": "auth.group", "fields": { "name": "registered-members", @@ -22,7 +22,7 @@ "slug": "registered-members", "last_modified": "2019-09-09 15:45:34", "created": "2019-09-09 15:45:34", - "group": 2, + "group": 3, "title": "Registered Members" }, "model": "groups.groupprofile", diff --git a/geonode/people/management/commands/setcontributorsgroup.py b/geonode/people/management/commands/setcontributorsgroup.py deleted file mode 100644 index 61bfefa0bdc..00000000000 --- a/geonode/people/management/commands/setcontributorsgroup.py +++ /dev/null @@ -1,28 +0,0 @@ -from django.contrib.auth import get_user_model -from django.contrib.auth.models import Group, Permission -from django.contrib.contenttypes.models import ContentType -from django.core.management.base import BaseCommand - - -class Command(BaseCommand): - help = ( - "Assign existing users to contributors group" - ) - - def handle(self, *args, **options): - cont_group, created = Group.objects.get_or_create(name='contributors') - ctype = ContentType.objects.get_for_model(cont_group) - perm, created = Permission.objects.get_or_create( - codename='base_addresourcebase', - name='Can add resources', - content_type=ctype, - ) - if perm: - cont_group.permissions.add(perm) - # Exclude admins and anonymous user - users_to_update = get_user_model().objects.filter( - pk__gt=0, is_staff=False, is_superuser=False - ) - for user in users_to_update: - cont_group.user_set.add(user) - self.stdout.write("All users are assigned to contributors group") diff --git a/geonode/people/migrations/0032_set_contributors_group.py b/geonode/people/migrations/0032_set_contributors_group.py new file mode 100644 index 00000000000..78762eb2bd1 --- /dev/null +++ b/geonode/people/migrations/0032_set_contributors_group.py @@ -0,0 +1,49 @@ +# Assign the contributors group to users according to #7364 +from django.contrib.auth import get_user_model +from django.contrib.auth.models import Group, Permission +from django.contrib.contenttypes.models import ContentType + +from django.db import migrations +from django.db.migrations.operations import RunPython + + +def forwards_func(apps, schema_editor): + # assign contributors group to users + cont_group, created = Group.objects.get_or_create(name='contributors') + ctype = ContentType.objects.get_for_model(cont_group) + perm, created = Permission.objects.get_or_create( + codename='base_addresourcebase', + name='Can add resources', + content_type=ctype, + ) + if perm: + cont_group.permissions.add(perm) + # Exclude admins and anonymous user + users_to_update = get_user_model().objects.filter( + pk__gt=0, is_staff=False, is_superuser=False + ) + for user in users_to_update: + cont_group.user_set.add(user) + +def reverse_func(apps, schema_editor): + # remove contributors group from users + try: + cont_group = Group.objects.get(name='contributors') + users_to_update = get_user_model().objects.filter( + pk__gt=0, is_staff=False, is_superuser=False + ) + for user in users_to_update: + user.groups.remove(cont_group) + except Exception: + pass + + +class Migration(migrations.Migration): + + dependencies = [ + ('people', '0031_merge_20210205_0824'), + ] + + operations = [ + RunPython(forwards_func, reverse_func) + ] diff --git a/geonode/security/permissions.py b/geonode/security/permissions.py index 5fc3f083d27..2790e305269 100644 --- a/geonode/security/permissions.py +++ b/geonode/security/permissions.py @@ -1,11 +1,11 @@ # Permissions mapping PERMISSIONS = { - 'base_addresourcebase': 'ADD_RESOURCE', + 'base_addresourcebase': 'add_resource', } # The following permissions will be filtered out when READ_ONLY mode is active READ_ONLY_AFFECTED_PERMISSIONS = [ - 'ADD_RESOURCE', + 'add_resource', ] # Permissions on resources From ecadee32fb1c12cbe06248d2cd523d75a7b92575 Mon Sep 17 00:00:00 2001 From: afabiani Date: Tue, 4 May 2021 16:50:09 +0200 Subject: [PATCH 3/4] - Making "Groups" editable from admin form and make sure the new ones are assigned only when the user is actually created (cherry picked from commit e7adddaf81ef3eba5d1e5b82ba75cb4f7857b8bd) --- geonode/people/admin.py | 2 +- geonode/people/signals.py | 27 ++++++++++++++++----------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/geonode/people/admin.py b/geonode/people/admin.py index c71247b62c1..7c80938e391 100644 --- a/geonode/people/admin.py +++ b/geonode/people/admin.py @@ -79,7 +79,7 @@ class ProfileAdmin(admin.ModelAdmin): search_fields = ( 'username', 'organization', 'profile', 'first_name', 'last_name', 'email') - readonly_fields = ("groups", ) + # readonly_fields = ("groups", ) ordering = ('username',) filter_horizontal = ('groups', 'user_permissions',) actions = [set_user_and_group_layer_permission] diff --git a/geonode/people/signals.py b/geonode/people/signals.py index e1583149d97..cdad1db4620 100644 --- a/geonode/people/signals.py +++ b/geonode/people/signals.py @@ -32,10 +32,11 @@ from django.db import IntegrityError from django.db.models import Q -from geonode.base.auth import (get_or_create_token, - delete_old_tokens, - set_session_token, - remove_session_token) +from geonode.base.auth import ( + get_or_create_token, + delete_old_tokens, + set_session_token, + remove_session_token) from geonode.groups.models import GroupProfile from geonode.groups.conf import settings as groups_settings @@ -129,15 +130,19 @@ def profile_post_save(instance, sender, **kwargs): This will make sure that anonymous and contributors permissions will be granted to the new users. """ from django.contrib.auth.models import Group - anon_group, created = Group.objects.get_or_create(name='anonymous') - instance.groups.add(anon_group) - is_anonymous = instance.username == 'AnonymousUser' - if not (instance.is_staff or instance.is_superuser or is_anonymous): - cont_group, created = Group.objects.get_or_create(name='contributors') - instance.groups.add(cont_group) + + created = kwargs.get('created', False) + + if created: + anon_group, _ = Group.objects.get_or_create(name='anonymous') + instance.groups.add(anon_group) + is_anonymous = instance.username == 'AnonymousUser' + + if Group.objects.filter(name='contributors').count() and not (instance.is_staff or instance.is_superuser or is_anonymous): + cont_group = Group.objects.get(name='contributors') + instance.groups.add(cont_group) if groups_settings.AUTO_ASSIGN_REGISTERED_MEMBERS_TO_REGISTERED_MEMBERS_GROUP_AT == 'activation': - created = kwargs.get('created', False) became_active = instance.is_active and (not instance._previous_active_state or created) if became_active: _add_user_to_registered_members(instance) From 5fef64c0492e2b8944f11fb3b973d437d2127760 Mon Sep 17 00:00:00 2001 From: afabiani Date: Tue, 4 May 2021 16:50:44 +0200 Subject: [PATCH 4/4] [Code Quality] Autopep8 code formatting (cherry picked from commit d037ba38fddca45689b2cc8bd3ec5ad6147cc4d0) --- geonode/base/populate_test_data.py | 2 +- geonode/documents/views.py | 2 +- geonode/geoapps/views.py | 4 ++-- geonode/layers/views.py | 4 ++-- geonode/maps/views.py | 10 +++++----- .../people/migrations/0032_set_contributors_group.py | 4 ++-- geonode/security/models.py | 2 +- geonode/security/permissions.py | 2 +- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/geonode/base/populate_test_data.py b/geonode/base/populate_test_data.py index ba922ad0dfc..90db0998626 100644 --- a/geonode/base/populate_test_data.py +++ b/geonode/base/populate_test_data.py @@ -161,7 +161,7 @@ def create_models(type=None, integration=False): codename='base_addresourcebase', name='Can add resources', content_type=ctype - ) + ) cont_group.permissions.add(perm) logger.debug("[SetUp] Get or create user admin") u, created = get_user_model().objects.get_or_create(username='admin') diff --git a/geonode/documents/views.py b/geonode/documents/views.py index 37a151773b5..0baa8741138 100644 --- a/geonode/documents/views.py +++ b/geonode/documents/views.py @@ -122,7 +122,7 @@ def document_detail(request, docid): perms_list = list( document.get_self_resource().get_user_perms(request.user) .union(document.get_user_perms(request.user)) - ) + ) group = None if document.group: diff --git a/geonode/geoapps/views.py b/geonode/geoapps/views.py index 09c76c016eb..fd3da50b709 100644 --- a/geonode/geoapps/views.py +++ b/geonode/geoapps/views.py @@ -141,7 +141,7 @@ def geoapp_detail(request, geoappid, template='apps/app_detail.html'): perms_list = list( geoapp_obj.get_self_resource().get_user_perms(request.user) .union(geoapp_obj.get_user_perms(request.user)) - ) + ) group = None if geoapp_obj.group: try: @@ -210,7 +210,7 @@ def geoapp_edit(request, geoappid, template='apps/app_edit.html'): perms_list = list( geoapp_obj.get_self_resource().get_user_perms(request.user) .union(geoapp_obj.get_user_perms(request.user)) - ) + ) group = None if geoapp_obj.group: diff --git a/geonode/layers/views.py b/geonode/layers/views.py index 58f54dbb740..1c0c3b54206 100644 --- a/geonode/layers/views.py +++ b/geonode/layers/views.py @@ -623,7 +623,7 @@ def sld_definition(style): perms_list = list( layer.get_self_resource().get_user_perms(request.user) .union(layer.get_user_perms(request.user)) - ) + ) group = None if layer.group: @@ -1669,7 +1669,7 @@ def layer_metadata_detail( perms_list = list( layer.get_self_resource().get_user_perms(request.user) .union(layer.get_user_perms(request.user)) - ) + ) return render(request, template, context={ "resource": layer, diff --git a/geonode/maps/views.py b/geonode/maps/views.py index fcd19fcc95d..61a34cb25e8 100644 --- a/geonode/maps/views.py +++ b/geonode/maps/views.py @@ -157,7 +157,7 @@ def map_detail(request, mapid, template='maps/map_detail.html'): perms_list = list( map_obj.get_self_resource().get_user_perms(request.user) .union(map_obj.get_user_perms(request.user)) - ) + ) group = None if map_obj.group: @@ -554,7 +554,7 @@ def map_view(request, mapid, layer_name=None, perms_list = list( map_obj.get_self_resource().get_user_perms(request.user) .union(map_obj.get_user_perms(request.user)) - ) + ) if layer_name: config = add_layers_to_map_config( request, map_obj, (layer_name, ), False) @@ -671,7 +671,7 @@ def map_edit(request, mapid, template='maps/map_edit.html'): perms_list = list( map_obj.get_self_resource().get_user_perms(request.user) .union(map_obj.get_user_perms(request.user)) - ) + ) return render(request, template, context={ 'mapId': mapid, 'config': json.dumps(config), @@ -722,14 +722,14 @@ def new_map(request, template='maps/map_new.html'): perms_list = list( layer_obj.get_self_resource().get_user_perms(request.user) .union(layer_obj.get_user_perms(request.user)) - ) + ) except Exception: pass elif map_obj: perms_list = list( map_obj.get_self_resource().get_user_perms(request.user) .union(map_obj.get_user_perms(request.user)) - ) + ) context_dict = { 'config': config, 'map': map_obj, diff --git a/geonode/people/migrations/0032_set_contributors_group.py b/geonode/people/migrations/0032_set_contributors_group.py index 78762eb2bd1..b4448ae24c8 100644 --- a/geonode/people/migrations/0032_set_contributors_group.py +++ b/geonode/people/migrations/0032_set_contributors_group.py @@ -9,9 +9,9 @@ def forwards_func(apps, schema_editor): # assign contributors group to users - cont_group, created = Group.objects.get_or_create(name='contributors') + cont_group, _ = Group.objects.get_or_create(name='contributors') ctype = ContentType.objects.get_for_model(cont_group) - perm, created = Permission.objects.get_or_create( + perm, _ = Permission.objects.get_or_create( codename='base_addresourcebase', name='Can add resources', content_type=ctype, diff --git a/geonode/security/models.py b/geonode/security/models.py index fe243da6cb8..5d04b8aed1b 100644 --- a/geonode/security/models.py +++ b/geonode/security/models.py @@ -413,7 +413,7 @@ def get_user_perms(self, user): resource_perms = user_resource_perms.union( user_model.objects.filter(permission__codename__in=implicit_perms) - ).values_list('permission__codename', flat=True) + ).values_list('permission__codename', flat=True) # filter out permissions for edit, change or publish if readonly mode is active perm_prefixes = ['change', 'delete', 'publish'] diff --git a/geonode/security/permissions.py b/geonode/security/permissions.py index 2790e305269..2191bd0ef63 100644 --- a/geonode/security/permissions.py +++ b/geonode/security/permissions.py @@ -1,7 +1,7 @@ # Permissions mapping PERMISSIONS = { 'base_addresourcebase': 'add_resource', - } +} # The following permissions will be filtered out when READ_ONLY mode is active READ_ONLY_AFFECTED_PERMISSIONS = [