From 1c1b633ce9ac9723aed656ddb8371dad5b6b2f4a Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Thu, 19 Sep 2024 13:32:54 -0500 Subject: [PATCH] User: Make email required at all times, password required for new users --- dojo/api_v2/serializers.py | 9 +++++---- dojo/forms.py | 4 +++- tests/user_test.py | 3 +++ unittests/test_apiv2_user.py | 14 ++++++++------ unittests/test_rest_framework.py | 9 +++++++++ 5 files changed, 28 insertions(+), 11 deletions(-) diff --git a/dojo/api_v2/serializers.py b/dojo/api_v2/serializers.py index a0d4298b74..10c07b3f3d 100644 --- a/dojo/api_v2/serializers.py +++ b/dojo/api_v2/serializers.py @@ -429,6 +429,7 @@ class Meta: class UserSerializer(serializers.ModelSerializer): date_joined = serializers.DateTimeField(read_only=True) last_login = serializers.DateTimeField(read_only=True, allow_null=True) + email = serializers.EmailField(required=True) password = serializers.CharField( write_only=True, style={"input_type": "password"}, @@ -549,12 +550,12 @@ def validate(self, data): msg = "Only superusers are allowed to add or edit superusers." raise ValidationError(msg) - if ( - self.context["request"].method in ["PATCH", "PUT"] - and "password" in data - ): + if self.context["request"].method in ["PATCH", "PUT"] and "password" in data: msg = "Update of password though API is not allowed" raise ValidationError(msg) + if self.context["request"].method == "POST" and "password" not in data: + msg = "Passwords must be supplied for new users" + raise ValidationError(msg) else: return super().validate(data) diff --git a/dojo/forms.py b/dojo/forms.py index dde58a38b6..fd5c55a7b6 100644 --- a/dojo/forms.py +++ b/dojo/forms.py @@ -2168,8 +2168,9 @@ def clean(self): class AddDojoUserForm(forms.ModelForm): + email = forms.EmailField(required=True) password = forms.CharField(widget=forms.PasswordInput, - required=False, + required=True, validators=[validate_password], help_text="") @@ -2186,6 +2187,7 @@ def __init__(self, *args, **kwargs): class EditDojoUserForm(forms.ModelForm): + email = forms.EmailField(required=True) class Meta: model = Dojo_User diff --git a/tests/user_test.py b/tests/user_test.py index dcaa9c845f..607b8a7b4e 100644 --- a/tests/user_test.py +++ b/tests/user_test.py @@ -59,6 +59,9 @@ def test_create_user_with_writer_global_role(self): # username driver.find_element(By.ID, "id_username").clear() driver.find_element(By.ID, "id_username").send_keys("userWriter") + # password + driver.find_element(By.ID, "id_password").clear() + driver.find_element(By.ID, "id_password").send_keys("Def3ctD0jo&") # First Name driver.find_element(By.ID, "id_first_name").clear() driver.find_element(By.ID, "id_first_name").send_keys("Writer") diff --git a/unittests/test_apiv2_user.py b/unittests/test_apiv2_user.py index 88f91bfb5f..9b9fe02618 100644 --- a/unittests/test_apiv2_user.py +++ b/unittests/test_apiv2_user.py @@ -26,16 +26,11 @@ def test_user_list(self): self.assertNotIn(item, user, r.content[:1000]) def test_user_add(self): - # simple user without password - r = self.client.post(reverse("user-list"), { - "username": "api-user-1", - }, format="json") - self.assertEqual(r.status_code, 201, r.content[:1000]) - # user with good password password = "testTEST1234!@#$" r = self.client.post(reverse("user-list"), { "username": "api-user-2", + "email": "admin@dojo.com", "password": password, }, format="json") self.assertEqual(r.status_code, 201, r.content[:1000]) @@ -50,6 +45,7 @@ def test_user_add(self): # user with weak password r = self.client.post(reverse("user-list"), { "username": "api-user-3", + "email": "admin@dojo.com", "password": "weakPassword", }, format="json") self.assertEqual(r.status_code, 400, r.content[:1000]) @@ -59,6 +55,8 @@ def test_user_change_password(self): # some user r = self.client.post(reverse("user-list"), { "username": "api-user-4", + "email": "admin@dojo.com", + "password": "testTEST1234!@#$", }, format="json") self.assertEqual(r.status_code, 201, r.content[:1000]) user_id = r.json()["id"] @@ -66,16 +64,19 @@ def test_user_change_password(self): r = self.client.put("{}{}/".format(reverse("user-list"), user_id), { "username": "api-user-4", "first_name": "first", + "email": "admin@dojo.com", }, format="json") self.assertEqual(r.status_code, 200, r.content[:1000]) r = self.client.patch("{}{}/".format(reverse("user-list"), user_id), { "last_name": "last", + "email": "admin@dojo.com", }, format="json") self.assertEqual(r.status_code, 200, r.content[:1000]) r = self.client.put("{}{}/".format(reverse("user-list"), user_id), { "username": "api-user-4", + "email": "admin@dojo.com", "password": "testTEST1234!@#$", }, format="json") self.assertEqual(r.status_code, 400, r.content[:1000]) @@ -83,6 +84,7 @@ def test_user_change_password(self): r = self.client.patch("{}{}/".format(reverse("user-list"), user_id), { "password": "testTEST1234!@#$", + "email": "admin@dojo.com", }, format="json") self.assertEqual(r.status_code, 400, r.content[:1000]) self.assertIn("Update of password though API is not allowed", r.content.decode("utf-8")) diff --git a/unittests/test_rest_framework.py b/unittests/test_rest_framework.py index aa9318ba8f..d065c54361 100644 --- a/unittests/test_rest_framework.py +++ b/unittests/test_rest_framework.py @@ -1699,6 +1699,15 @@ def __init__(self, *args, **kwargs): self.deleted_objects = 25 BaseClass.RESTEndpointTest.__init__(self, *args, **kwargs) + def test_create(self): + payload = self.payload.copy() | { + "password": "testTEST1234!@#$", + } + length = self.endpoint_model.objects.count() + response = self.client.post(self.url, payload) + self.assertEqual(201, response.status_code, response.content[:1000]) + self.assertEqual(self.endpoint_model.objects.count(), length + 1) + def test_create_user_with_non_configuration_permissions(self): payload = self.payload.copy() payload["configuration_permissions"] = [25, 26] # these permissions exist but user can not assign them becaause they are not "configuration_permissions"