-
Notifications
You must be signed in to change notification settings - Fork 601
[ENG-445] refact: user change password py to pydantic #3667
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
72cfcd5
c35a9e3
52c1abe
c6ccecc
2826b9f
87c10ce
9fe812c
467f1a2
06ed06a
9412d6d
8c91598
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,68 +1,69 @@ | ||
| from django.contrib.auth import get_user_model | ||
| from django.contrib.auth.password_validation import validate_password | ||
| from django.conf import settings | ||
| from django.contrib.auth.password_validation import ( | ||
| get_password_validators, | ||
| validate_password, | ||
| ) | ||
| from django.core.exceptions import ValidationError as DjangoValidationError | ||
| from drf_spectacular.utils import extend_schema, extend_schema_view | ||
| from rest_framework import serializers, status | ||
| from pydantic import BaseModel, ValidationInfo, field_validator, model_validator | ||
| from rest_framework.generics import UpdateAPIView | ||
| from rest_framework.response import Response | ||
|
|
||
| User = get_user_model() | ||
| from care.emr.api.viewsets.base import emr_exception_handler | ||
|
|
||
|
|
||
| class ChangePasswordSerializer(serializers.Serializer): | ||
| """ | ||
| Serializer for the change password endpoint. | ||
| class ChangePasswordSpec(BaseModel): | ||
| old_password: str | ||
| new_password: str | ||
|
|
||
| Validates the new password using Django's built-in password validators. | ||
| """ | ||
| @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 | ||
|
|
||
| old_password = serializers.CharField(required=True) | ||
| new_password = serializers.CharField(required=True) | ||
| @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) | ||
|
|
||
| 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) | ||
| validate_password( | ||
| self.new_password, | ||
| user=user, | ||
| password_validators=get_password_validators( | ||
| settings.AUTH_PASSWORD_VALIDATORS | ||
| ), | ||
| ) | ||
| except DjangoValidationError as e: | ||
| raise serializers.ValidationError(e.messages) from e | ||
| return value | ||
| raise ValueError(e.messages) from e | ||
|
nandkishorr marked this conversation as resolved.
|
||
| return self | ||
|
Comment on lines
+27
to
+44
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When old password is correct but new-password validation fails, |
||
|
|
||
|
|
||
| @extend_schema_view( | ||
| put=extend_schema(tags=["users"]), | ||
| patch=extend_schema(tags=["users"]), | ||
| put=extend_schema(tags=["users"], request=ChangePasswordSpec), | ||
| patch=extend_schema(tags=["users"], request=ChangePasswordSpec), | ||
| ) | ||
|
nandkishorr marked this conversation as resolved.
coderabbitai[bot] marked this conversation as resolved.
|
||
| class ChangePasswordView(UpdateAPIView): | ||
| """ | ||
| API endpoint for allowing authenticated users to change their password. | ||
| """ | ||
|
|
||
| serializer_class = ChangePasswordSerializer | ||
| model = User | ||
| def get_exception_handler(self): | ||
| return emr_exception_handler | ||
|
|
||
| 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") | ||
| ): | ||
| return Response( | ||
| { | ||
| "old_password": [ | ||
| "Wrong password entered. Please check your password." | ||
| ] | ||
| }, | ||
| status=status.HTTP_400_BAD_REQUEST, | ||
| ) | ||
| data = ChangePasswordSpec.model_validate( | ||
| request.data, context={"user": request.user} | ||
|
nandkishorr marked this conversation as resolved.
|
||
| ) | ||
|
|
||
| self.object.set_password(serializer.validated_data.get("new_password")) | ||
| self.object.save() | ||
| request.user.set_password(data.new_password) | ||
| request.user.save() | ||
| return Response({"message": "Password updated successfully"}) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,51 +1,90 @@ | ||
| from django.contrib.auth import get_user_model | ||
| from django.urls import reverse | ||
| from rest_framework import status | ||
| from rest_framework.test import APITestCase | ||
|
|
||
| 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"} | ||
|
nandkishorr marked this conversation as resolved.
|
||
|
|
||
| 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) | ||
| 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_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) | ||
| # Check that 'new_password' is in the error response keys | ||
| self.assertIn("new_password", response.data) | ||
|
|
||
| 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) | ||
| self.assertEqual( | ||
| response.data["errors"][0]["msg"], | ||
| "Value error, Wrong password entered. Please check your password.", | ||
| ) | ||
|
|
||
| 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.assertIn("old_password", response.data) | ||
|
|
||
| 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) | ||
| 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")) | ||
|
nandkishorr marked this conversation as resolved.
nandkishorr marked this conversation as resolved.
|
||
|
|
||
| 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"}) | ||
Uh oh!
There was an error while loading. Please reload this page.