From 72cfcd5f9b181bcaa6185527ec3734a4984028a3 Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Fri, 29 May 2026 16:52:16 +0530 Subject: [PATCH 1/5] refact:added pydantic --- care/users/api/viewsets/change_password.py | 68 +++++++++------------- 1 file changed, 28 insertions(+), 40 deletions(-) diff --git a/care/users/api/viewsets/change_password.py b/care/users/api/viewsets/change_password.py index 7d6779f874..04d7fb7697 100644 --- a/care/users/api/viewsets/change_password.py +++ b/care/users/api/viewsets/change_password.py @@ -1,64 +1,52 @@ -from django.contrib.auth import get_user_model -from django.contrib.auth.password_validation import validate_password -from django.core.exceptions import ValidationError as DjangoValidationError +from django.conf import settings +from django.contrib.auth.password_validation import ( + get_password_validators, + validate_password, +) from drf_spectacular.utils import extend_schema, extend_schema_view -from rest_framework import serializers, status +from pydantic import BaseModel +from rest_framework import status from rest_framework.generics import UpdateAPIView from rest_framework.response import Response -User = get_user_model() - - -class ChangePasswordSerializer(serializers.Serializer): - """ - Serializer for the change password endpoint. - - Validates the new password using Django's built-in password validators. - """ - - old_password = serializers.CharField(required=True) - new_password = serializers.CharField(required=True) - def validate_new_password(self, value): - """ - Validate the new password against Django's password policies. - """ - user = self.context["request"].user - try: - validate_password(value, user=user) - except DjangoValidationError as e: - raise serializers.ValidationError(e.messages) from e - return value +class ChangePasswordSpec(BaseModel): + old_password: str + new_password: str @extend_schema_view( put=extend_schema(tags=["users"]), patch=extend_schema(tags=["users"]), + request=ChangePasswordSpec, ) class ChangePasswordView(UpdateAPIView): """ API endpoint for allowing authenticated users to change their password. """ - serializer_class = ChangePasswordSerializer - model = User - def update(self, request, *args, **kwargs): """ Handle password update request for the authenticated user. """ - self.object = self.request.user - serializer = self.get_serializer(data=request.data) - serializer.is_valid(raise_exception=True) - - if not self.object.check_password( - serializer.validated_data.get("old_password") - ): + data = ChangePasswordSpec(**request.data) + if not request.user.check_password(data.old_password): return Response( - {"old_password": ["Wrong password entered. Please check your password."]}, + { + "old_password": [ + "Wrong password entered. Please check your password." + ] + }, status=status.HTTP_400_BAD_REQUEST, ) - - self.object.set_password(serializer.validated_data.get("new_password")) - self.object.save() + validate_password( + data.new_password, + user=request.user, + password_validators=get_password_validators( + settings.AUTH_PASSWORD_VALIDATORS + ), + ) + + request.user.set_password(data.new_password) + request.user.save() return Response({"message": "Password updated successfully"}) From c35a9e32e76a0f711b4034745bce96e836dae18d Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Fri, 29 May 2026 17:03:23 +0530 Subject: [PATCH 2/5] refact:updted the tests --- care/users/tests/__init__.py | 0 care/users/tests/test_change_password.py | 68 ++++++++++++------------ 2 files changed, 33 insertions(+), 35 deletions(-) create mode 100644 care/users/tests/__init__.py diff --git a/care/users/tests/__init__.py b/care/users/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/care/users/tests/test_change_password.py b/care/users/tests/test_change_password.py index 811de87060..1594a1b682 100644 --- a/care/users/tests/test_change_password.py +++ b/care/users/tests/test_change_password.py @@ -1,51 +1,49 @@ from django.urls import reverse from rest_framework import status -from rest_framework.test import APITestCase -from django.contrib.auth import get_user_model -User = get_user_model() +from care.utils.tests.base import CareAPITestBase -class TestChangePassword(APITestCase): +class UserChangePasswordTestCase(CareAPITestBase): def setUp(self): - self.user = User.objects.create_user( - username="vipul", - password="StrongPass@123", + super().setUp() + self.user = self.create_user_with_password( + username="testuser", password="password123" ) self.client.force_authenticate(user=self.user) self.url = reverse("change_password_view") + self.payload = {"old_password": "password123", "new_password": "newpassword456"} - def test_weak_password_is_rejected(self): - """Test that passwords failing Django's built-in validation are rejected.""" - payload = { - "old_password": "StrongPass@123", - "new_password": "123", # Too short for Django's default (8 chars) - } - response = self.client.put(self.url, payload) - - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - # Check that 'new_password' is in the error response keys - self.assertIn("new_password", response.data) + def test_change_password_success(self): + response = self.client.put(self.url, self.payload, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["message"], "Password updated successfully") + self.user.refresh_from_db() + self.assertTrue(self.user.check_password("newpassword456")) - def test_wrong_old_password_fails(self): - """Ensure the user must provide the correct current password.""" - payload = { - "old_password": "WrongCurrentPassword", - "new_password": "NewStrongPass@456", - } - response = self.client.put(self.url, payload) + def test_change_password_wrong_old_password(self): + self.payload["old_password"] = "wrongpassword" + response = self.client.put(self.url, self.payload, format="json") self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertIn("old_password", response.data) + self.assertEqual( + response.data["old_password"][0], + "Wrong password entered. Please check your password.", + ) - def test_password_change_success(self): - """Test that a valid password change works correctly.""" - payload = { - "old_password": "StrongPass@123", - "new_password": "NewStrongPass@456", - } - response = self.client.put(self.url, payload) - self.assertEqual(response.status_code, status.HTTP_200_OK) + def test_change_password_weak_new_password(self): + self.payload["new_password"] = "123" + response = self.client.put(self.url, self.payload, format="json") + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertContains( + response, + "This password is too short", + status_code=status.HTTP_400_BAD_REQUEST, + ) - # Verify the password actually changed in the DB + def test_change_password_invalid_password(self): + self.payload["new_password"] = "password123" + response = self.client.put(self.url, self.payload, format="json") + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.user.refresh_from_db() - self.assertTrue(self.user.check_password("NewStrongPass@456")) + self.assertTrue(self.user.check_password("password123")) From 52c1abeb564f7b16c57f2afe3446a080b3d3f685 Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Fri, 29 May 2026 18:51:09 +0530 Subject: [PATCH 3/5] cleanup --- care/emr/tests/test_reset_password_api.py | 45 ---------------------- care/users/api/viewsets/change_password.py | 21 ++++++++-- care/users/tests/test_change_password.py | 42 ++++++++++++++++++++ 3 files changed, 59 insertions(+), 49 deletions(-) diff --git a/care/emr/tests/test_reset_password_api.py b/care/emr/tests/test_reset_password_api.py index 85fa53c556..5dc0cbe243 100644 --- a/care/emr/tests/test_reset_password_api.py +++ b/care/emr/tests/test_reset_password_api.py @@ -457,48 +457,3 @@ def test_reset_password_request_email_failure(self): format="json", ) self.assertEqual(response.status_code, 400) - - def test_change_password_with_leading_whitespace(self): - """ - Test that password with leading whitespace is handled consistently. - The password should be stripped before validation, matching login behavior. - """ - self.client.force_authenticate(user=self.user) - new_password = "newpassword@123" - response = self.client.put( - self.change_password_url, - {"old_password": f" {self.password}", "new_password": new_password}, - format="json", - ) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.data, {"message": "Password updated successfully"}) - - def test_change_password_with_trailing_whitespace(self): - """ - Test that password with trailing whitespace is handled consistently. - The password should be stripped before validation, matching login behavior. - """ - self.client.force_authenticate(user=self.user) - new_password = "newpassword@123" - response = self.client.put( - self.change_password_url, - {"old_password": f"{self.password} ", "new_password": new_password}, - format="json", - ) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.data, {"message": "Password updated successfully"}) - - def test_change_password_with_leading_and_trailing_whitespace(self): - """ - Test that password with both leading and trailing whitespace is handled consistently. - The password should be stripped before validation, matching login behavior. - """ - self.client.force_authenticate(user=self.user) - new_password = "newpassword@123" - response = self.client.put( - self.change_password_url, - {"old_password": f" {self.password} ", "new_password": new_password}, - format="json", - ) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.data, {"message": "Password updated successfully"}) diff --git a/care/users/api/viewsets/change_password.py b/care/users/api/viewsets/change_password.py index 04d7fb7697..15db6acdb2 100644 --- a/care/users/api/viewsets/change_password.py +++ b/care/users/api/viewsets/change_password.py @@ -4,32 +4,45 @@ validate_password, ) from drf_spectacular.utils import extend_schema, extend_schema_view -from pydantic import BaseModel +from pydantic import BaseModel, field_validator from rest_framework import status from rest_framework.generics import UpdateAPIView from rest_framework.response import Response +from care.emr.api.viewsets.base import emr_exception_handler + class ChangePasswordSpec(BaseModel): old_password: str new_password: str + @field_validator("old_password", "new_password", mode="before") + @classmethod + def strip_passwords(cls, v): + """Strip leading and trailing whitespace from passwords.""" + if isinstance(v, str): + return v.strip() + return v + @extend_schema_view( - put=extend_schema(tags=["users"]), - patch=extend_schema(tags=["users"]), - request=ChangePasswordSpec, + put=extend_schema(tags=["users"], request=ChangePasswordSpec), + patch=extend_schema(tags=["users"], request=ChangePasswordSpec), ) class ChangePasswordView(UpdateAPIView): """ API endpoint for allowing authenticated users to change their password. """ + def get_exception_handler(self): + return emr_exception_handler + def update(self, request, *args, **kwargs): """ Handle password update request for the authenticated user. """ data = ChangePasswordSpec(**request.data) + if not request.user.check_password(data.old_password): return Response( { diff --git a/care/users/tests/test_change_password.py b/care/users/tests/test_change_password.py index 1594a1b682..10b2abea00 100644 --- a/care/users/tests/test_change_password.py +++ b/care/users/tests/test_change_password.py @@ -47,3 +47,45 @@ def test_change_password_invalid_password(self): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.user.refresh_from_db() self.assertTrue(self.user.check_password("password123")) + + def test_change_password_with_leading_whitespace(self): + """ + Test that password with leading whitespace is handled consistently. + The password should be stripped before validation, matching login behavior. + """ + self.payload["old_password"] = f" {self.payload['old_password']}" + response = self.client.put( + self.url, + self.payload, + format="json", + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data, {"message": "Password updated successfully"}) + + def test_change_password_with_trailing_whitespace(self): + """ + Test that password with trailing whitespace is handled consistently. + The password should be stripped before validation, matching login behavior. + """ + self.payload["old_password"] = f"{self.payload['old_password']} " + response = self.client.put( + self.url, + self.payload, + format="json", + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data, {"message": "Password updated successfully"}) + + def test_change_password_with_leading_and_trailing_whitespace(self): + """ + Test that password with both leading and trailing whitespace is handled consistently. + The password should be stripped before validation, matching login behavior. + """ + self.payload["old_password"] = f" {self.payload['old_password']} " + response = self.client.put( + self.url, + self.payload, + format="json", + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data, {"message": "Password updated successfully"}) From c6cceccd64fc57c716314b2d3d3398b9f098b054 Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Sun, 31 May 2026 15:47:03 +0530 Subject: [PATCH 4/5] fix:added suggested changes --- care/users/api/viewsets/change_password.py | 44 ++++++++++++---------- care/users/tests/test_change_password.py | 5 +-- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/care/users/api/viewsets/change_password.py b/care/users/api/viewsets/change_password.py index 15db6acdb2..b6ef3fc458 100644 --- a/care/users/api/viewsets/change_password.py +++ b/care/users/api/viewsets/change_password.py @@ -3,9 +3,9 @@ get_password_validators, validate_password, ) +from django.core.exceptions import ValidationError as DjangoValidationError from drf_spectacular.utils import extend_schema, extend_schema_view -from pydantic import BaseModel, field_validator -from rest_framework import status +from pydantic import BaseModel, ValidationInfo, field_validator, model_validator from rest_framework.generics import UpdateAPIView from rest_framework.response import Response @@ -24,6 +24,27 @@ def strip_passwords(cls, v): return v.strip() return v + @model_validator(mode="after") + def validate_passwords(self, info: ValidationInfo): + user = info.context.get("user") + + if not user.check_password(self.old_password): + msg = "Wrong password entered. Please check your password." + raise ValueError(msg) + + try: + validate_password( + self.new_password, + user=user, + password_validators=get_password_validators( + settings.AUTH_PASSWORD_VALIDATORS + ), + ) + except DjangoValidationError as e: + raise ValueError(e.messages) from e + + return self + @extend_schema_view( put=extend_schema(tags=["users"], request=ChangePasswordSpec), @@ -41,23 +62,8 @@ def update(self, request, *args, **kwargs): """ Handle password update request for the authenticated user. """ - data = ChangePasswordSpec(**request.data) - - if not request.user.check_password(data.old_password): - return Response( - { - "old_password": [ - "Wrong password entered. Please check your password." - ] - }, - status=status.HTTP_400_BAD_REQUEST, - ) - validate_password( - data.new_password, - user=request.user, - password_validators=get_password_validators( - settings.AUTH_PASSWORD_VALIDATORS - ), + data = ChangePasswordSpec.model_validate( + request.data, context={"user": request.user} ) request.user.set_password(data.new_password) diff --git a/care/users/tests/test_change_password.py b/care/users/tests/test_change_password.py index 10b2abea00..63f4276727 100644 --- a/care/users/tests/test_change_password.py +++ b/care/users/tests/test_change_password.py @@ -25,10 +25,9 @@ def test_change_password_wrong_old_password(self): self.payload["old_password"] = "wrongpassword" response = self.client.put(self.url, self.payload, format="json") self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertIn("old_password", response.data) self.assertEqual( - response.data["old_password"][0], - "Wrong password entered. Please check your password.", + response.data["errors"][0]["msg"], + "Value error, Wrong password entered. Please check your password.", ) def test_change_password_weak_new_password(self): From 9fe812c3c95904945561fff242916a7fad8544d8 Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Mon, 8 Jun 2026 14:38:19 +0530 Subject: [PATCH 5/5] cleanup --- care/users/api/viewsets/change_password.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/care/users/api/viewsets/change_password.py b/care/users/api/viewsets/change_password.py index b6ef3fc458..1554efb70c 100644 --- a/care/users/api/viewsets/change_password.py +++ b/care/users/api/viewsets/change_password.py @@ -27,7 +27,6 @@ def strip_passwords(cls, v): @model_validator(mode="after") def validate_passwords(self, info: ValidationInfo): user = info.context.get("user") - if not user.check_password(self.old_password): msg = "Wrong password entered. Please check your password." raise ValueError(msg) @@ -42,7 +41,6 @@ def validate_passwords(self, info: ValidationInfo): ) except DjangoValidationError as e: raise ValueError(e.messages) from e - return self