diff --git a/.circleci/config.yml b/.circleci/config.yml index 2f8b3842003..2945c6ce190 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -21,7 +21,7 @@ jobs: - checkout - setup_remote_docker: docker_layer_caching: false - version: 20.10.12 + version: docker24 - run: name: Build the stack diff --git a/geonode/base/api/permissions.py b/geonode/base/api/permissions.py index 75d19ea36d3..8dc3cffdcfa 100644 --- a/geonode/base/api/permissions.py +++ b/geonode/base/api/permissions.py @@ -23,6 +23,7 @@ from rest_framework import permissions from rest_framework.filters import BaseFilterBackend +from geonode.people.utils import get_available_users from geonode.security.permissions import ( BASIC_MANAGE_PERMISSIONS, DOWNLOAD_PERMISSIONS, @@ -139,6 +140,11 @@ def has_object_permission(self, request, view, obj): elif hasattr(obj, "user"): _request_matches = obj.user == request.user + if isinstance(obj, get_user_model()) and not request.user.is_anonymous: + if request.method in permissions.SAFE_METHODS and obj in get_available_users(request.user): + return True + return _request_matches + if not _request_matches: _request_matches = request.user in get_users_with_perms(obj) return _request_matches diff --git a/geonode/base/api/serializers.py b/geonode/base/api/serializers.py index 261ce68c7c9..8024d8673b4 100644 --- a/geonode/base/api/serializers.py +++ b/geonode/base/api/serializers.py @@ -17,6 +17,7 @@ # ######################################################################### import logging + from slugify import slugify from urllib.parse import urljoin import json @@ -30,6 +31,9 @@ from django.db.models.query import QuerySet from geonode.people import Roles from django.http import QueryDict +from django.contrib.auth.hashers import make_password +from django.contrib.auth.password_validation import validate_password +from django.forms import ValidationError as ValidationErrorForm from deprecated import deprecated from rest_framework import serializers @@ -358,6 +362,37 @@ class Meta: view_name = "users-list" fields = ("pk", "username", "first_name", "last_name", "avatar", "perms", "is_superuser", "is_staff", "email") + @staticmethod + def password_validation(password_payload): + try: + validate_password(password_payload) + except ValidationErrorForm as err: + raise serializers.ValidationError(detail=",".join(err.messages)) + return make_password(password_payload) + + def validate(self, data): + request = self.context["request"] + user = request.user + # only admins/staff can edit these permissions + if not (user.is_superuser or user.is_staff): + data.pop("is_superuser", None) + data.pop("is_staff", None) + # username cant be changed + if request.method in ("PUT", "PATCH") and data.get("username"): + raise serializers.ValidationError(detail="username cannot be updated") + email = data.get("email") + # Email is required on post + if request.method in ("POST") and settings.ACCOUNT_EMAIL_REQUIRED and not email: + raise serializers.ValidationError(detail="email missing from payload") + # email should be unique + if get_user_model().objects.filter(email=email).exists(): + raise serializers.ValidationError("A user is already registered with that email") + # password validation + password = request.data.get("password") + if password: + data["password"] = self.password_validation(password) + return data + @classmethod def setup_eager_loading(cls, queryset): """Perform necessary eager loading of data.""" diff --git a/geonode/base/api/tests.py b/geonode/base/api/tests.py index af05bc67bb6..9f32e5e6c56 100644 --- a/geonode/base/api/tests.py +++ b/geonode/base/api/tests.py @@ -388,9 +388,7 @@ def test_register_users(self): Ensure users are created with default groups. """ url = reverse("users-list") - user_data = { - "username": "new_user", - } + user_data = {"username": "new_user", "password": "@!2XJSL_S&V^0nt", "email": "user@exampl2e.com"} self.assertTrue(self.client.login(username="admin", password="admin")) response = self.client.post(url, data=user_data, format="json") self.assertEqual(response.status_code, 201) @@ -419,7 +417,7 @@ def test_update_user_profile(self): username="user_test_delete", email="user_test_delete@geonode.org", password="user" ) url = reverse("users-detail", kwargs={"pk": user.pk}) - data = {"first_name": "user"} + data = {"first_name": "user", "password": "@!2XJSL_S&V^0nt", "email": "user@exampl2e.com"} # Anonymous response = self.client.patch(url, data=data, format="json") self.assertEqual(response.status_code, 403) @@ -430,14 +428,15 @@ def test_update_user_profile(self): # User self profile self.assertTrue(self.client.login(username="user_test_delete", password="user")) response = self.client.patch(url, data=data, format="json") - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 200) # Group manager group = GroupProfile.objects.create(slug="test_group_manager", title="test_group_manager") group.join(user) group.join(get_user_model().objects.get(username="norman"), role="manager") self.assertTrue(self.client.login(username="norman", password="norman")) response = self.client.post(url, data=data, format="json") - self.assertEqual(response.status_code, 403) + # malformed url on post + self.assertEqual(response.status_code, 405) # Admin can edit user self.assertTrue(self.client.login(username="admin", password="admin")) response = self.client.patch(url, data={"first_name": "user_admin"}, format="json") @@ -465,15 +464,15 @@ def test_delete_user_profile(self): # Bob can't delete user self.assertTrue(self.client.login(username="bobby", password="bob")) response = self.client.delete(url, format="json") - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 405) # User can not delete self profile self.assertTrue(self.client.login(username="user_test_delete", password="user")) response = self.client.delete(url, format="json") - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 405) # Admin can delete user self.assertTrue(self.client.login(username="admin", password="admin")) response = self.client.delete(url, format="json") - self.assertEqual(response.status_code, 204) + self.assertEqual(response.status_code, 405) finally: user.delete() diff --git a/geonode/people/tests.py b/geonode/people/tests.py index f3636c975c1..b306bbea220 100644 --- a/geonode/people/tests.py +++ b/geonode/people/tests.py @@ -16,6 +16,7 @@ # along with this program. If not, see . # ######################################################################### +import django from django.test.utils import override_settings from mock import MagicMock, PropertyMock, patch from geonode.tests.base import GeoNodeBaseTestSupport @@ -471,3 +472,317 @@ def test_new_user_is_no_assigned_automatically_to_contributors_if_disabled(self) """ new_user = get_user_model().objects.create(username="random_username") self.assertFalse("contributors" in [x.name for x in new_user.groups.iterator()]) + + def test_users_api_valid_post(self): + data = { + "username": "usernam3e", + "first_name": "Registered", + "password": "@!2XJSL_S&V^0nt", + "last_name": "Test", + "avatar": "https://www.gravatar.com/avatar/7a68c67c8d409ff07e42aa5d5ab7b765/?s=240", + "perms": ["add_resource"], + "is_superuser": False, + "is_staff": False, + "email": "fpglf@poc.com", + } + + self.client.login(username="admin", password="admin") + response = self.client.post(reverse("users-list"), data=data, content_type="application/json") + self.assertEqual(response.status_code, 201) + + def test_users_api_post_not_admin(self): + data = { + "username": "usernam3e", + "first_name": "Registered", + "password": "@!2XJSL_S&V^0nt", + "last_name": "Test", + "avatar": "https://www.gravatar.com/avatar/7a68c67c8d409ff07e42aa5d5ab7b765/?s=240", + "perms": ["add_resource"], + "is_superuser": True, + "is_staff": True, + "email": "fpglf@poc.com", + } + bobby = get_user_model().objects.get(username="bobby") + self.client.login(username="bobby", password="bob") + # assert that bobby is not a super user or staff + self.assertFalse(bobby.is_superuser) + self.assertFalse(bobby.is_staff) + response = self.client.post(reverse("users-list"), data=data, content_type="application/json") + self.assertEqual(response.status_code, 403) + + def test_users_api_patch_self(self): + + bobby = get_user_model().objects.get(username="bobby") + self.assertTrue(self.client.login(username="bobby", password="bob")) + self.assertTrue(bobby.is_authenticated) + # bobby wants to edit his own data + data = {"first_name": "Robert"} + # before change + self.assertNotEqual(bobby.first_name, "Robert") + + # and can acess even if he's not admin or staff + self.assertFalse(bobby.is_superuser) + self.assertFalse(bobby.is_staff) + + url = f"{reverse('users-list')}/{bobby.pk}" + response = self.client.patch(url, data=data, content_type="application/json") + response_json = response.json() + self.assertEqual(response.status_code, 200) + self.assertEqual(response_json["user"]["first_name"], "Robert") + + def test_users_api_patch_self_as_superuser(self): + + bobby = get_user_model().objects.get(username="bobby") + self.assertTrue(self.client.login(username="bobby", password="bob")) + self.assertTrue(bobby.is_authenticated) + # bobby wants to edit his own data + data = { + "first_name": "Robert", + "is_superuser": True, + "is_staff": True, + } + # before change + self.assertNotEqual(bobby.first_name, "Robert") + + # and can acess even if he's not admin or staff + self.assertFalse(bobby.is_superuser) + self.assertFalse(bobby.is_staff) + + url = f"{reverse('users-list')}/{bobby.pk}" + response = self.client.patch(url, data=data, content_type="application/json") + response_json = response.json() + + self.assertEqual(response.status_code, 200) + self.assertEqual(response_json["user"]["first_name"], "Robert") + + self.assertFalse(response_json["user"]["is_superuser"]) + self.assertFalse(response_json["user"]["is_staff"]) + # check db side too + bobby = get_user_model().objects.get(username="bobby") + self.assertFalse(bobby.is_superuser) + self.assertFalse(bobby.is_staff) + + def test_users_api_patch_others_from_non_admin(self): + + bobby = get_user_model().objects.get(username="bobby") + profile = get_user_model().objects.get(username="user1") + + self.assertTrue(self.client.login(username="bobby", password="bob")) + self.assertTrue(bobby.is_authenticated) + # bobby wants to edit his user's data + data = {"first_name": "Norman Sky", "password": "@!2XJSL_S&V^0nt", "email": "bob@bob.com"} + + # Bobby is not superuser or staff + self.assertFalse(bobby.is_superuser) + self.assertFalse(bobby.is_staff) + + url = f"{reverse('users-list')}/{profile.pk}" + response = self.client.patch(url, data=data, content_type="application/json") + + # bobby is not permitted to update user data + self.assertEqual(response.status_code, 403) + + def test_users_api_patch_others_from_admin(self): + + bobby = get_user_model().objects.get(username="bobby") + admin = get_user_model().objects.get(username="admin") + + self.assertTrue(self.client.login(username="admin", password="admin")) + self.assertTrue(admin.is_authenticated) + # admin wants to edit his bobby's data + data = {"first_name": "Robert Baratheon", "password": "@!2XJSL_S&V^0nt000", "email": "bob@bob.com"} + + # Admin is superuser or staff + self.assertTrue(admin.is_superuser or admin.is_staff) + + url = f"{reverse('users-list')}/{bobby.pk}" + response = self.client.patch(url, data=data, content_type="application/json") + + # admin is permitted to update bobby's data + self.assertEqual(response.status_code, 200) + + self.assertEqual(response.json()["user"]["first_name"], "Robert Baratheon") + + @override_settings(ACCOUNT_EMAIL_REQUIRED=True) + def test_users_api_empty_email(self): + """ + If the environment variable ACCOUNT_EMAIL_REQUIRED is set to True, + the email will be mandatory in the payload. + """ + data = { + "username": "usernam3e", + "first_name": "Registered", + "password": "@!2XJSL_S&V^0nt", + "last_name": "Test", + "avatar": "https://www.gravatar.com/avatar/7a68c67c8d409ff07e42aa5d5ab7b765/?s=240", + "perms": ["add_resource"], + "is_superuser": False, + "is_staff": False, + } + # ensure there is no email in payload + data.pop("email", None) + + self.client.login(username="admin", password="admin") + response = self.client.post(reverse("users-list"), data=data, content_type="application/json") + + # endpoint throws Exception on missing email + self.assertEqual(response.status_code, 400) + self.assertTrue("email missing from payload" in response.json()["errors"]) + + @override_settings( + AUTH_PASSWORD_VALIDATORS=[ + { + "NAME": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator", + }, + { + "NAME": "django.contrib.auth.password_validation.MinimumLengthValidator", + "OPTIONS": { + "min_length": 14, + }, + }, + { + "NAME": "django.contrib.auth.password_validation.CommonPasswordValidator", + }, + { + "NAME": "geonode.people.password_validators.UppercaseValidator", + }, + { + "NAME": "geonode.people.password_validators.NumberValidator", + "OPTIONS": { + "min_digits": 1, + }, + }, + { + "NAME": "geonode.people.password_validators.LowercaseValidator", + }, + { + "NAME": "geonode.people.password_validators.SpecialCharsValidator", + }, + ] + ) + def test_users_api_invalid_password(self): + """ + If a password validator is set via AUTH_PASSWORD_VALIDATORS, + the API will return an error if the validation fails + """ + error_codes = [ + "This password is too short. It must contain at least 14 characters.", + "The password must contain at least1 digit(s), 0-9.", + ] + data = { + "username": "usernam3e", + "first_name": "Registered", + "password": "whitetext", + "last_name": "Test", + "avatar": "https://www.gravatar.com/avatar/7a68c67c8d409ff07e42aa5d5ab7b765/?s=240", + "perms": ["add_resource"], + "is_superuser": False, + "is_staff": False, + "email": "fpglf@poc.com", + } + + self.client.login(username="admin", password="admin") + response = self.client.post(reverse("users-list"), data=data, content_type="application/json") + self.assertEqual(response.status_code, 400) + + for error in error_codes: + self.assertTrue(error in response.json()["errors"][0]) + + @override_settings( + ACCOUNT_EMAIL_VERIFICATION="mandatory", + EMAIL_HOST="localhost", + EMAIL_HOST_USER="", + EMAIL_HOST_PASSWORD="", + EMAIL_PORT="25", + ) + def test_users_register_email_verification(self): + """ + If the email confirmation requirement is configured, + a verification email will be sent to the user before allowing them to log in. + """ + data = { + "username": "usernam3e", + "email": "user@exampl2e.com", + "password1": "@!2XJSL_S&V^0nt", + "password2": "@!2XJSL_S&V^0nt", + } + + response = self.client.post(reverse("account_signup"), data=data, format="json") + # response should be a redirect to the confirmation email + self.assertEqual(response.status_code, 302) + + # check that user was created + get_user_model().objects.get(email=data["email"]) + + email_box = django.core.mail.outbox + # assert that an email was sent to the email provided in the payload + self.assertEqual(len(email_box), 1) + self.assertTrue(data["email"] in email_box[0].to) + + def test_users_api_patch_password_from_admin(self): + bobby = get_user_model().objects.get(username="bobby") + admin = get_user_model().objects.get(username="admin") + + self.assertTrue(self.client.login(username="admin", password="admin")) + self.assertTrue(admin.is_authenticated) + + # admin wants to edit his bobby's data + data = {"password": "@!2XJSL_S&V^0nt000"} + # Admin is superuser or staff + self.assertTrue(admin.is_superuser or admin.is_staff) + old_pass = bobby.password + + url = f"{reverse('users-list')}/{bobby.pk}" + response = self.client.patch(url, data=data, content_type="application/json") + + # admin is permitted to update bobby's data + self.assertEqual(response.status_code, 200) + # bobbys password has changed + bobby.refresh_from_db() + # asserting not equal from the password salt + self.assertNotEqual(bobby.password, old_pass) + + def test_users_api_add_existing_email(self): + data = {"username": "teddy", "password": "@!2XJSL_S&V^0nt", "email": "teddy@teddy.com"} + self.client.login(username="admin", password="admin") + response = self.client.post(reverse("users-list"), data=data, content_type="application/json") + self.assertEqual(response.status_code, 201) + + # try to readd the same email + data = {"username": "teddy1", "password": "@!2XJSL_S&V^0nt", "email": "teddy@teddy.com"} + response = self.client.post(reverse("users-list"), data=data, content_type="application/json") + self.assertEqual(response.status_code, 400) + self.assertTrue("A user is already registered with that email" in response.json()["errors"]) + + def test_users_api_add_existing_username(self): + bobby = get_user_model().objects.get(username="bobby") + + data = {"username": bobby.get_username(), "password": "@!2XJSL_S&V^0nt", "email": "bobby@bobby.com"} + self.client.login(username="admin", password="admin") + response = self.client.post(reverse("users-list"), data=data, content_type="application/json") + self.assertEqual(response.status_code, 400) + self.assertTrue("A user with that username already exists." in response.json()["errors"]) + + def test_users_api_patch_username(self): + + bobby = get_user_model().objects.get(username="bobby") + admin = get_user_model().objects.get(username="admin") + + self.assertTrue(self.client.login(username="admin", password="admin")) + # admin wants to edit his bobby's data + data = { + "first_name": "Robert Baratheon", + "username": "bob", + "password": "@!2XJSL_S&V^0nt000", + "email": "bob@bob.com", + } + + # Admin is superuser or staff + self.assertTrue(admin.is_superuser or admin.is_staff) + + url = f"{reverse('users-list')}/{bobby.pk}" + response = self.client.patch(url, data=data, content_type="application/json") + + # username cannot be updated + self.assertEqual(response.status_code, 400) + self.assertTrue("username cannot be updated" in response.json()["errors"]) diff --git a/geonode/people/views.py b/geonode/people/views.py index 270f550144f..e62dbdd1327 100644 --- a/geonode/people/views.py +++ b/geonode/people/views.py @@ -28,14 +28,12 @@ from django.http import HttpResponseForbidden from django.db.models import Q from django.views import View - from geonode.tasks.tasks import send_email from geonode.people.forms import ProfileForm from geonode.people.utils import get_available_users from geonode.base.auth import get_or_create_token from geonode.people.forms import ForgotUsernameForm from geonode.base.views import user_and_group_permission - from dal import autocomplete from dynamic_rest.filters import DynamicFilterBackend, DynamicSortingFilter @@ -48,13 +46,14 @@ from geonode.base.models import ResourceBase from geonode.base.api.filters import DynamicSearchFilter from geonode.groups.models import GroupProfile, GroupMember -from geonode.base.api.permissions import IsSelfOrAdminOrReadOnly +from geonode.base.api.permissions import IsOwnerOrAdmin from geonode.base.api.serializers import UserSerializer, GroupProfileSerializer, ResourceBaseSerializer from geonode.base.api.pagination import GeoNodeApiPagination from rest_framework.authentication import SessionAuthentication, BasicAuthentication from geonode.security.utils import get_visible_resources from guardian.shortcuts import get_objects_for_user +from rest_framework.exceptions import PermissionDenied class SetUserLayerPermission(View): @@ -167,10 +166,11 @@ class UserViewSet(DynamicModelViewSet): API endpoint that allows users to be viewed or edited. """ + http_method_names = ["get", "post", "patch"] authentication_classes = [SessionAuthentication, BasicAuthentication, OAuth2Authentication] permission_classes = [ IsAuthenticated, - IsSelfOrAdminOrReadOnly, + IsOwnerOrAdmin, ] filter_backends = [DynamicFilterBackend, DynamicSortingFilter, DynamicSearchFilter] serializer_class = UserSerializer @@ -189,6 +189,21 @@ def get_queryset(self): queryset = self.get_serializer_class().setup_eager_loading(queryset) return queryset.order_by("username") + def perform_create(self, serializer): + user = self.request.user + if not (user.is_superuser or user.is_staff): + raise PermissionDenied() + serializer.is_valid(raise_exception=True) + instance = serializer.save() + return instance + + def update(self, request, *args, **kwargs): + instance = self.get_object() + serializer = self.get_serializer(instance, data=request.data, partial=True) + serializer.is_valid(raise_exception=True) + serializer.save() + return Response(serializer.data) + @extend_schema( methods=["get"], responses={200: ResourceBaseSerializer(many=True)},