diff --git a/src-ui/src/app/components/common/profile-edit-dialog/profile-edit-dialog.component.ts b/src-ui/src/app/components/common/profile-edit-dialog/profile-edit-dialog.component.ts index dfa5c56a8..c4a103397 100644 --- a/src-ui/src/app/components/common/profile-edit-dialog/profile-edit-dialog.component.ts +++ b/src-ui/src/app/components/common/profile-edit-dialog/profile-edit-dialog.component.ts @@ -183,6 +183,7 @@ export class ProfileEditDialogComponent this.newPassword && this.currentPassword !== this.newPassword const profile = Object.assign({}, this.form.value) delete profile.totp_code + this.error = null this.networkActive = true this.profileService .update(profile) @@ -204,6 +205,7 @@ export class ProfileEditDialogComponent }, error: (error) => { this.toastService.showError($localize`Error saving profile`, error) + this.error = error?.error this.networkActive = false }, }) diff --git a/src/documents/tests/test_api_permissions.py b/src/documents/tests/test_api_permissions.py index 8ffce1f95..bc81dabe9 100644 --- a/src/documents/tests/test_api_permissions.py +++ b/src/documents/tests/test_api_permissions.py @@ -648,7 +648,7 @@ class TestApiUser(DirectoriesMixin, APITestCase): user1 = { "username": "testuser", - "password": "test", + "password": "areallysupersecretpassword235", "first_name": "Test", "last_name": "User", } @@ -730,7 +730,7 @@ class TestApiUser(DirectoriesMixin, APITestCase): f"{self.ENDPOINT}{user1.pk}/", data={ "first_name": "Updated Name 2", - "password": "123xyz", + "password": "newreallystrongpassword456", }, ) diff --git a/src/documents/tests/test_api_profile.py b/src/documents/tests/test_api_profile.py index 8475459a2..2eedf3297 100644 --- a/src/documents/tests/test_api_profile.py +++ b/src/documents/tests/test_api_profile.py @@ -192,6 +192,65 @@ class TestApiProfile(DirectoriesMixin, APITestCase): self.assertEqual(user.first_name, user_data["first_name"]) self.assertEqual(user.last_name, user_data["last_name"]) + def test_update_profile_invalid_password_returns_field_error(self): + """ + GIVEN: + - Configured user + WHEN: + - API call is made to update profile with weak password + THEN: + - Profile update fails with password field error + """ + + user_data = { + "email": "new@email.com", + "password": "short", # shorter than default validator threshold + "first_name": "new first name", + "last_name": "new last name", + } + + response = self.client.patch(self.ENDPOINT, user_data) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("password", response.data) + self.assertIsInstance(response.data["password"], list) + self.assertTrue( + any( + "too short" in message.lower() for message in response.data["password"] + ), + ) + + def test_update_profile_placeholder_password_skips_validation(self): + """ + GIVEN: + - Configured user with existing password + WHEN: + - API call is made with the obfuscated placeholder password value + THEN: + - Profile is updated without changing the password or running validators + """ + + original_password = "orig-pass-12345" + self.user.set_password(original_password) + self.user.save() + + user_data = { + "email": "new@email.com", + "password": "*" * 12, # matches obfuscated value from serializer + "first_name": "new first name", + "last_name": "new last name", + } + + response = self.client.patch(self.ENDPOINT, user_data) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + user = User.objects.get(username=self.user.username) + self.assertTrue(user.check_password(original_password)) + self.assertEqual(user.email, user_data["email"]) + self.assertEqual(user.first_name, user_data["first_name"]) + self.assertEqual(user.last_name, user_data["last_name"]) + def test_update_auth_token(self): """ GIVEN: diff --git a/src/paperless/serialisers.py b/src/paperless/serialisers.py index 754a3c594..97b84fd14 100644 --- a/src/paperless/serialisers.py +++ b/src/paperless/serialisers.py @@ -9,6 +9,7 @@ from allauth.socialaccount.models import SocialApp from django.contrib.auth.models import Group from django.contrib.auth.models import Permission from django.contrib.auth.models import User +from django.contrib.auth.password_validation import validate_password from rest_framework import serializers from rest_framework.authtoken.serializers import AuthTokenSerializer @@ -19,6 +20,23 @@ from paperless_mail.serialisers import ObfuscatedPasswordField logger = logging.getLogger("paperless.settings") +class PasswordValidationMixin: + def _has_real_password(self, value: str | None) -> bool: + return bool(value) and value.replace("*", "") != "" + + def validate_password(self, value: str) -> str: + if not self._has_real_password(value): + return value + + request = self.context.get("request") if hasattr(self, "context") else None + user = self.instance or ( + request.user if request and hasattr(request, "user") else None + ) + validate_password(value, user) # raise ValidationError if invalid + + return value + + class PaperlessAuthTokenSerializer(AuthTokenSerializer): code = serializers.CharField( label="MFA Code", @@ -49,7 +67,7 @@ class PaperlessAuthTokenSerializer(AuthTokenSerializer): return attrs -class UserSerializer(serializers.ModelSerializer): +class UserSerializer(PasswordValidationMixin, serializers.ModelSerializer): password = ObfuscatedPasswordField(required=False) user_permissions = serializers.SlugRelatedField( many=True, @@ -87,11 +105,11 @@ class UserSerializer(serializers.ModelSerializer): return obj.get_group_permissions() def update(self, instance, validated_data): - if "password" in validated_data: - if len(validated_data.get("password").replace("*", "")) > 0: - instance.set_password(validated_data.get("password")) - instance.save() - validated_data.pop("password") + password = validated_data.pop("password", None) + if self._has_real_password(password): + instance.set_password(password) + instance.save() + super().update(instance, validated_data) return instance @@ -102,12 +120,7 @@ class UserSerializer(serializers.ModelSerializer): user_permissions = None if "user_permissions" in validated_data: user_permissions = validated_data.pop("user_permissions") - password = None - if ( - "password" in validated_data - and len(validated_data.get("password").replace("*", "")) > 0 - ): - password = validated_data.pop("password") + password = validated_data.pop("password", None) user = User.objects.create(**validated_data) # set groups if groups: @@ -116,7 +129,7 @@ class UserSerializer(serializers.ModelSerializer): if user_permissions: user.user_permissions.set(user_permissions) # set password - if password: + if self._has_real_password(password): user.set_password(password) user.save() return user @@ -156,7 +169,7 @@ class SocialAccountSerializer(serializers.ModelSerializer): return "Unknown App" -class ProfileSerializer(serializers.ModelSerializer): +class ProfileSerializer(PasswordValidationMixin, serializers.ModelSerializer): email = serializers.EmailField(allow_blank=True, required=False) password = ObfuscatedPasswordField(required=False, allow_null=False) auth_token = serializers.SlugRelatedField(read_only=True, slug_field="key") diff --git a/src/paperless/views.py b/src/paperless/views.py index 69375e1bc..e79c0e668 100644 --- a/src/paperless/views.py +++ b/src/paperless/views.py @@ -197,10 +197,10 @@ class ProfileView(GenericAPIView): serializer.is_valid(raise_exception=True) user = self.request.user if hasattr(self.request, "user") else None - if len(serializer.validated_data.get("password").replace("*", "")) > 0: - user.set_password(serializer.validated_data.get("password")) + password = serializer.validated_data.pop("password", None) + if password and password.replace("*", ""): + user.set_password(password) user.save() - serializer.validated_data.pop("password") for key, value in serializer.validated_data.items(): setattr(user, key, value)