User: Make email required at all times, password required for new users (#10938)

* User: Make email required at all times, password required for new users

* fix tests

* update tests

Author: Maffooch

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)
 

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

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")

unittests/test_apiv2_notifications.py

@@ -33,6 +33,7 @@ def create_test_user(self):
         password = "testTEST1234!@#$"
         r = self.client.post(reverse("user-list"), {
             "username": "api-user-notification",
+            "email": "admin@dojo.com",
             "password": password,
         }, format="json")
         return r.json()["id"]

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,30 +55,36 @@ 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"]
 
         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])
         self.assertIn("Update of password though API is not allowed", r.content.decode("utf-8"))
 
         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"))

unittests/test_rest_framework.py

@@ -1699,8 +1699,19 @@ 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 = self.payload.copy() | {
+            "password": "testTEST1234!@#$",
+        }
         payload["configuration_permissions"] = [25, 26]  # these permissions exist but user can not assign them becaause they are not "configuration_permissions"
         response = self.client.post(self.url, payload)
         self.assertEqual(response.status_code, 400)