fix: add extra validation to VerifiedName serializer

edx · Sep 7, 2021 · f23f189 · f23f189
1 parent 510f3c5
commit f23f189
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 22 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -14,6 +14,11 @@ Change Log
 Unreleased
 ~~~~~~~~~~
 
+[0.9.1] - 2021-09-07
+~~~~~~~~~~~~~~~~~~~~
+* Add extra validation for the VerifiedName serializer, throwing a 400 error if
+  `verified_name` contains HTML or a URL.
+
 [0.9.0] - 2021-09-01
 ~~~~~~~~~~~~~~~~~~~~
 * Add is verified name enabled to the API

diff --git a/edx_name_affirmation/__init__.py b/edx_name_affirmation/__init__.py
@@ -2,6 +2,6 @@
 Django app housing name affirmation logic.
 """
 
-__version__ = '0.9.0'
+__version__ = '0.9.1'
 
 default_app_config = 'edx_name_affirmation.apps.EdxNameAffirmationConfig'  # pylint: disable=invalid-name
diff --git a/edx_name_affirmation/serializers.py b/edx_name_affirmation/serializers.py
@@ -1,4 +1,7 @@
 """Defines serializers used by the Name Affirmation API"""
+
+import re
+
 from rest_framework import serializers
 
 from django.contrib.auth import get_user_model
@@ -30,6 +33,26 @@ class Meta:
             "proctored_exam_attempt_id", "status"
         )
 
+    def validate_verified_name(self, verified_name):
+        if self._contains_html(verified_name):
+            raise serializers.ValidationError('Name cannot contain the following characters: < >')
+        if self._contains_url(verified_name):
+            raise serializers.ValidationError('Name cannot contain a URL')
+
+    def _contains_html(self, string):
+        """
+        Validator method to check whether a string contains HTML tags
+        """
+        regex = re.compile('(<|>)', re.UNICODE)
+        return bool(regex.search(string))
+
+    def _contains_url(self, string):
+        """
+        Validator method to check whether a string contains a url
+        """
+        regex = re.findall(r'https|http?://(?:[-\w.]|(?:%[\da-fA-F]{2}))+', string)
+        return bool(regex)
+
 
 class VerifiedNameConfigSerializer(serializers.ModelSerializer):
     """

diff --git a/edx_name_affirmation/tests/test_views.py b/edx_name_affirmation/tests/test_views.py
@@ -26,19 +26,11 @@
 User = get_user_model()
 
 
-class VerifiedNameViewTests(LoggedInTestCase):
+class NameAffirmationViewsTestCase(LoggedInTestCase):
     """
-    Tests for the VerifiedNameView
+    Base test class for Name Affirmation views
     """
 
-    VERIFIED_NAME = 'Jonathan Doe'
-    PROFILE_NAME = 'Jon Doe'
-
-    OTHER_VERIFIED_NAME = 'Robert Smith'
-    OTHER_PROFILE_NAME = 'Bob Smith'
-
-    ATTEMPT_ID = 11111
-
     def setUp(self):
         super().setUp()
         # Create a fresh config with default values
@@ -48,6 +40,21 @@ def tearDown(self):
         super().tearDown()
         cache.clear()
 
+
+@ddt.ddt
+class VerifiedNameViewTests(NameAffirmationViewsTestCase):
+    """
+    Tests for the VerifiedNameView
+    """
+
+    VERIFIED_NAME = 'Jonathan Doe'
+    PROFILE_NAME = 'Jon Doe'
+
+    OTHER_VERIFIED_NAME = 'Robert Smith'
+    OTHER_PROFILE_NAME = 'Bob Smith'
+
+    ATTEMPT_ID = 11111
+
     def test_verified_name(self):
         verified_name = self._create_verified_name(status=VerifiedNameStatus.APPROVED)
 
@@ -168,6 +175,21 @@ def test_post_403_non_staff(self):
         )
         self.assertEqual(response.status_code, 403)
 
+    @ddt.data('<html>Verified Name</html>', 'https://verifiedname.com')
+    def test_post_400_invalid_name(self, verified_name):
+        verified_name_data = {
+            'username': self.user.username,
+            'profile_name': self.PROFILE_NAME,
+            'verified_name': verified_name,
+            'verification_attempt_id': self.ATTEMPT_ID,
+            'status': VerifiedNameStatus.SUBMITTED.value,
+        }
+        response = self.client.post(
+            reverse('edx_name_affirmation:verified_name'),
+            verified_name_data
+        )
+        self.assertEqual(response.status_code, 400)
+
     def test_post_400_invalid_serializer(self):
         verified_name_data = {
             'username': self.user.username,
@@ -233,10 +255,11 @@ def _get_expected_data(
 
 
 @ddt.ddt
-class VerifiedNameHistoryViewTests(LoggedInTestCase):
+class VerifiedNameHistoryViewTests(NameAffirmationViewsTestCase):
     """
     Tests for the VerifiedNameHistoryView
     """
+
     def test_get(self):
         verified_name_history = self._create_verified_name_history(self.user)
         expected_response = self._get_expected_response(self.user, verified_name_history)
@@ -338,20 +361,11 @@ def _get_expected_response(
         return expected_response
 
 
-class VerifiedNameConfigViewTests(LoggedInTestCase):
+class VerifiedNameConfigViewTests(NameAffirmationViewsTestCase):
     """
     Tests for the VerifiedNameConfigView
     """
 
-    def setUp(self):
-        super().setUp()
-        # Create a fresh config with default values
-        VerifiedNameConfig.objects.create(user=self.user)
-
-    def tearDown(self):
-        super().tearDown()
-        cache.clear()
-
     def test_post_201(self):
         config_data = {
             'username': self.user.username,