feat: disallow '/' from Person.name (ietf-tools#4594)

* feat: disallow '/' from Person.name

* chore: migrate Person/HistoricalPerson to include new name validator

* test: test name validation

Author: jennifer-richards

ietf/person/migrations/0026_name_character_validator.py

@@ -0,0 +1,24 @@
+# Generated by Django 2.2.28 on 2022-10-17 10:18
+
+from django.db import migrations, models
+import ietf.person.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('person', '0025_chat_and_polls_apikey'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='historicalperson',
+            name='name',
+            field=models.CharField(db_index=True, help_text='Preferred long form of name.', max_length=255, validators=[ietf.person.models.name_character_validator], verbose_name='Full Name (Unicode)'),
+        ),
+        migrations.AlterField(
+            model_name='person',
+            name='name',
+            field=models.CharField(db_index=True, help_text='Preferred long form of name.', max_length=255, validators=[ietf.person.models.name_character_validator], verbose_name='Full Name (Unicode)'),
+        ),
+    ]

ietf/person/models.py

@@ -13,7 +13,7 @@
 
 from django.conf import settings
 from django.contrib.auth.models import User
-from django.core.exceptions import ObjectDoesNotExist
+from django.core.exceptions import ObjectDoesNotExist, ValidationError
 from django.core.validators import validate_email
 from django.db import models
 from django.template.loader import render_to_string
@@ -35,13 +35,18 @@
 from ietf.utils.models import ForeignKey, OneToOneField
 
 
+def name_character_validator(value):
+    if '/' in value:
+        raise ValidationError('Name cannot contain "/" character.')
+
+
 class Person(models.Model):
     history = HistoricalRecords()
     user = OneToOneField(User, blank=True, null=True, on_delete=models.SET_NULL)
     time = models.DateTimeField(default=datetime.datetime.now)      # When this Person record entered the system
     # The normal unicode form of the name.  This must be
     # set to the same value as the ascii-form if equal.
-    name = models.CharField("Full Name (Unicode)", max_length=255, db_index=True, help_text="Preferred long form of name.")
+    name = models.CharField("Full Name (Unicode)", max_length=255, db_index=True, help_text="Preferred long form of name.", validators=[name_character_validator])
     plain = models.CharField("Plain Name correction (Unicode)", max_length=64, default='', blank=True, help_text="Use this if you have a Spanish double surname.  Don't use this for nicknames, and don't use it unless you've actually observed that the datatracker shows your name incorrectly.")
     # The normal ascii-form of the name.
     ascii = models.CharField("Full Name (ASCII)", max_length=255, help_text="Name as rendered in ASCII (Latin, unaccented) characters.")
@@ -262,7 +267,7 @@ def cdn_photo_url(self, size=80):
 
 
 class PersonExtResource(models.Model):
-    person = ForeignKey(Person) 
+    person = ForeignKey(Person)
     name = models.ForeignKey(ExtResourceName, on_delete=models.CASCADE)
     display_name = models.CharField(max_length=255, default='', blank=True)
     value = models.CharField(max_length=2083) # 2083 is the maximum legal URL length
@@ -366,7 +371,7 @@ def salt():
     ("/api/iesg/position", "/api/iesg/position", "Area Director"),
     ("/api/v2/person/person", "/api/v2/person/person", "Robot"),
     ("/api/meeting/session/video/url", "/api/meeting/session/video/url", "Recording Manager"),
-    ("/api/notify/meeting/registration", "/api/notify/meeting/registration", "Robot"), 
+    ("/api/notify/meeting/registration", "/api/notify/meeting/registration", "Robot"),
     ("/api/notify/meeting/bluesheet", "/api/notify/meeting/bluesheet", "Recording Manager"),
     ("/api/notify/session/attendees", "/api/notify/session/attendees", "Recording Manager"),
     ("/api/notify/session/chatlog", "/api/notify/session/chatlog", "Recording Manager"),
@@ -444,4 +449,4 @@ class Meta:
 
 class PersonApiKeyEvent(PersonEvent):
     key = ForeignKey(PersonalApiKey)
-    
+

ietf/person/tests.py

@@ -10,6 +10,7 @@
 from pyquery import PyQuery
 
 
+from django.core.exceptions import ValidationError
 from django.http import HttpRequest
 from django.test import override_settings
 from django.urls import reverse as urlreverse
@@ -191,6 +192,12 @@ def test_cdn_photo_url_cdn_off(self):
         p = PersonFactory(with_bio=True)
         self.assertNotIn('cdn-cgi/photo',p.cdn_photo_url())
 
+    def test_invalid_name_characters_rejected(self):
+        slash_person = PersonFactory.build(name='I have a /', user=None)  # build() does not save the new object
+        with self.assertRaises(ValidationError):
+            slash_person.full_clean()  # calls validators (save() does *not*)
+
+
 class PersonUtilsTests(TestCase):
     def test_determine_merge_order(self):
         p1 = get_person_no_user()