From ee071e28e274cc76e7233a58d08f810ccd274a45 Mon Sep 17 00:00:00 2001 From: RegisSinjari Date: Thu, 7 Mar 2024 08:09:24 +0100 Subject: [PATCH] [Fixes #11995] Implement POST and PATCH methods for the User API, refactored validation in serializer --- geonode/base/api/serializers.py | 35 +++++++++++++++++++++++++++++++++ geonode/people/tests.py | 35 +++++++++++++++++++++++++++++++++ geonode/people/views.py | 21 +++++--------------- 3 files changed, 75 insertions(+), 16 deletions(-) diff --git a/geonode/base/api/serializers.py b/geonode/base/api/serializers.py index 261ce68c7c9..202c1f50c07 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"): + data.pop("username", None) + 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/people/tests.py b/geonode/people/tests.py index 2a7193d263f..694bfcbb0d1 100644 --- a/geonode/people/tests.py +++ b/geonode/people/tests.py @@ -718,3 +718,38 @@ def test_users_register_email_verification(self): # 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"]) diff --git a/geonode/people/views.py b/geonode/people/views.py index 746b093bd23..5d919129b6a 100644 --- a/geonode/people/views.py +++ b/geonode/people/views.py @@ -204,26 +204,15 @@ def perform_create(self, serializer): user = self.request.user if not (user.is_superuser or user.is_staff): raise PermissionDenied() - - email_payload = self.request.data.get("email", "") - password_payload = self.request.data.get("password", "") - - if settings.ACCOUNT_EMAIL_REQUIRED and email_payload == "": - raise ValidationError(detail="email missing from payload") - self.request.data["password"] = password_validation(password_payload) instance = serializer.save() return instance def update(self, request, *args, **kwargs): - kwargs["partial"] = True - user = self.request.user - if not (user.is_superuser or user.is_staff): - request.data.pop("is_superuser", None) - request.data.pop("is_staff", None) - password_payload = self.request.data.get("password", "") - if password_payload: - request.data["password"] = password_validation(password_payload) - return super().update(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"],