Skip to content

Commit

Permalink
User: Make email required at all times, password required for new users
Browse files Browse the repository at this point in the history
  • Loading branch information
Maffooch committed Sep 19, 2024
1 parent a643544 commit 1c1b633
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 11 deletions.
9 changes: 5 additions & 4 deletions dojo/api_v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down Expand Up @@ -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)

Expand Down
4 changes: 3 additions & 1 deletion dojo/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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="")

Expand All @@ -2186,6 +2187,7 @@ def __init__(self, *args, **kwargs):


class EditDojoUserForm(forms.ModelForm):
email = forms.EmailField(required=True)

class Meta:
model = Dojo_User
Expand Down
3 changes: 3 additions & 0 deletions tests/user_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
14 changes: 8 additions & 6 deletions unittests/test_apiv2_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": "[email protected]",
"password": password,
}, format="json")
self.assertEqual(r.status_code, 201, r.content[:1000])
Expand All @@ -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": "[email protected]",
"password": "weakPassword",
}, format="json")
self.assertEqual(r.status_code, 400, r.content[:1000])
Expand All @@ -59,30 +55,36 @@ def test_user_change_password(self):
# some user
r = self.client.post(reverse("user-list"), {
"username": "api-user-4",
"email": "[email protected]",
"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": "[email protected]",
}, 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": "[email protected]",
}, 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": "[email protected]",
"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": "[email protected]",
}, 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"))
9 changes: 9 additions & 0 deletions unittests/test_rest_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 1c1b633

Please sign in to comment.