Skip to content

Commit

Permalink
Merge pull request #4701 from freelawproject/4687-fix-registration-fo…
Browse files Browse the repository at this point in the history
…rm-vuln

fix(users): Enforce validation on first_name for increase security
  • Loading branch information
mlissner authored Nov 19, 2024
2 parents 99c184a + bc9f169 commit 61f4bf4
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 0 deletions.
10 changes: 10 additions & 0 deletions cl/users/forms.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import re

from disposable_email_domains import blocklist
from django import forms
from django.contrib.auth import authenticate
Expand Down Expand Up @@ -188,6 +190,14 @@ def clean_email(self):
)
return email

def clean_first_name(self):
first_name = self.cleaned_data.get("first_name")
if re.search(r"""[!"#$%&()*+,./:;<=>?@[\]_{|}~]+""", first_name):
raise forms.ValidationError(
"First name must not contain any special characters."
)
return first_name


class EmailConfirmationForm(forms.Form):
email = forms.EmailField(
Expand Down
44 changes: 44 additions & 0 deletions cl/users/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,50 @@ async def test_signing_in(self) -> None:
)
self.assertRedirects(r, "/")

async def test_registration_rejects_malicious_first_name_input(
self,
) -> None:
tests = (
# Invalid
("evil.com", False),
("http://test", False),
("email@test.test", False),
("/test/", False),
# Valid
("My fullname", True),
("Test Test", True),
("Éric Terrien-Pascal", True),
("Tel'c", True),
)
for first_name, is_valid in tests:
with self.subTest(
f"Trying to register using {first_name} as first name.",
first_name=first_name,
is_valid=is_valid,
):
r = await self.async_client.post(
reverse("register"),
{
"username": "aamon",
"email": "user@free.law",
"password1": "a",
"password2": "a",
"first_name": first_name,
"last_name": "Marquis of Hell",
"skip_me_if_alive": "",
},
)
if not is_valid:
self.assertIn(
"First name must not contain any special characters.",
r.content.decode(),
)
else:
self.assertNotIn(
"First name must not contain any special characters.",
r.content.decode(),
)

async def test_confirming_an_email_address(self) -> None:
"""Tests whether we can confirm the case where an email is associated
with a single account.
Expand Down

0 comments on commit 61f4bf4

Please sign in to comment.