Merge pull request #1236 from cisagov/dk/1101-nameservers-regex

Issue 1101 - Regex form validation on nameservers

src/registrar/forms/domain.py

@@ -53,6 +53,11 @@ class DomainNameserverForm(forms.Form):
         cleaned_data = super().clean()
         self.clean_empty_strings(cleaned_data)
         server = cleaned_data.get("server", "")
+        # remove ANY spaces in the server field
+        server = server.replace(" ", "")
+        # lowercase the server
+        server = server.lower()
+        cleaned_data["server"] = server
         ip = cleaned_data.get("ip", None)
         # remove ANY spaces in the ip field
         ip = ip.replace(" ", "")
@@ -60,9 +65,8 @@ class DomainNameserverForm(forms.Form):
 
         ip_list = self.extract_ip_list(ip)
 
-        if ip and not server and ip_list:
-            self.add_error("server", NameserverError(code=nsErrorCodes.MISSING_HOST))
-        elif server:
+        # validate if the form has a server or an ip
+        if (ip and ip_list) or server:
             self.validate_nameserver_ip_combo(domain, server, ip_list)
 
         return cleaned_data
@@ -95,6 +99,20 @@ class DomainNameserverForm(forms.Form):
                         code=nsErrorCodes.MISSING_IP, nameserver=domain, ip=ip_list
                     ),
                 )
+            elif e.code == nsErrorCodes.MISSING_HOST:
+                self.add_error(
+                    "server",
+                    NameserverError(
+                        code=nsErrorCodes.MISSING_HOST, nameserver=domain, ip=ip_list
+                    ),
+                )
+            elif e.code == nsErrorCodes.INVALID_HOST:
+                self.add_error(
+                    "server",
+                    NameserverError(
+                        code=nsErrorCodes.INVALID_HOST, nameserver=server, ip=ip_list
+                    ),
+                )
             else:
                 self.add_error("ip", str(e))
 

src/registrar/models/domain.py

@@ -3,7 +3,6 @@ import logging
 import ipaddress
 import re
 from datetime import date
-from string import digits
 from typing import Optional
 
 from django_fsm import FSMField, transition, TransitionNotAllowed  # type: ignore
@@ -277,7 +276,12 @@ class Domain(TimeStampedModel, DomainHelper):
             return response.code
         except RegistryError as e:
             logger.error("Error _create_host, code was %s error was %s" % (e.code, e))
-            raise e
+            # OBJECT_EXISTS is an expected error code that should not raise
+            # an exception, rather return the code to be handled separately
+            if e.code == ErrorCode.OBJECT_EXISTS:
+                return e.code
+            else:
+                raise e
 
     def _convert_list_to_dict(self, listToConvert: list[tuple[str, list]]):
         """converts a list of hosts into a dictionary
@@ -304,6 +308,31 @@ class Domain(TimeStampedModel, DomainHelper):
         regex = re.compile(full_pattern)
         return bool(regex.match(nameserver))
 
+    @classmethod
+    def isValidHost(cls, nameserver: str):
+        """Checks for validity of nameserver string based on these rules:
+        - first character is alpha or digit
+        - first and last character in each label is alpha or digit
+        - all characters alpha (lowercase), digit, -, or .
+        - each label has a min length of 1 and a max length of 63
+        - total host name has a max length of 253
+        """
+        # pattern to test for valid domain
+        # label pattern for each section of the host name, separated by .
+        labelpattern = r"[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?"
+        # lookahead pattern ensures first character not - and total length < 254
+        lookaheadpatterns = r"^((?!-))(?=.{1,253}\.?$)"
+        # pattern assembles lookaheadpatterns and ensures there are at least
+        # 3 labels in the host name
+        pattern = lookaheadpatterns + labelpattern + r"(\." + labelpattern + r"){2,}$"
+
+        # attempt to match the pattern
+        match = re.match(pattern, nameserver)
+
+        # return true if nameserver matches
+        # otherwise false
+        return bool(match)
+
     @classmethod
     def checkHostIPCombo(cls, name: str, nameserver: str, ip: list[str]):
         """Checks the parameters past for a valid combination
@@ -311,6 +340,8 @@ class Domain(TimeStampedModel, DomainHelper):
             - nameserver is a subdomain but is missing ip
             - nameserver is not a subdomain but has ip
             - nameserver is a subdomain but an ip passed is invalid
+            - nameserver is not a valid domain
+            - ip is provided but is missing domain
 
         Args:
             hostname (str)- nameserver or subdomain
@@ -319,7 +350,11 @@ class Domain(TimeStampedModel, DomainHelper):
             NameserverError (if exception hit)
         Returns:
             None"""
-        if cls.isSubdomain(name, nameserver) and (ip is None or ip == []):
+        if ip and not nameserver:
+            raise NameserverError(code=nsErrorCodes.MISSING_HOST)
+        elif nameserver and not cls.isValidHost(nameserver):
+            raise NameserverError(code=nsErrorCodes.INVALID_HOST, nameserver=nameserver)
+        elif cls.isSubdomain(name, nameserver) and (ip is None or ip == []):
             raise NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver)
 
         elif not cls.isSubdomain(name, nameserver) and (ip is not None and ip != []):
@@ -330,7 +365,7 @@ class Domain(TimeStampedModel, DomainHelper):
             for addr in ip:
                 if not cls._valid_ip_addr(addr):
                     raise NameserverError(
-                        code=nsErrorCodes.INVALID_IP, nameserver=nameserver, ip=ip
+                        code=nsErrorCodes.INVALID_IP, nameserver=nameserver[:40], ip=ip
                     )
         return None
 
@@ -1196,34 +1231,6 @@ class Domain(TimeStampedModel, DomainHelper):
     # ForeignKey on DomainInvitation creates an "invitations" member for
     # all of the invitations that have been sent for this domain
 
-    def _validate_host_tuples(self, hosts: list[tuple[str]]):
-        """
-        Helper function. Validate hostnames and IP addresses.
-
-        Raises:
-            ValueError if hostname or IP address appears invalid or mismatched.
-        """
-        for host in hosts:
-            hostname = host[0].lower()
-            addresses: tuple[str] = host[1:]  # type: ignore
-            if not bool(Domain.HOST_REGEX.match(hostname)):
-                raise ValueError("Invalid hostname: %s." % hostname)
-            if len(hostname) > Domain.MAX_LENGTH:
-                raise ValueError("Too long hostname: %s" % hostname)
-
-            is_subordinate = hostname.split(".", 1)[-1] == self.name
-            if is_subordinate and len(addresses) == 0:
-                raise ValueError(
-                    "Must supply IP addresses for subordinate host %s" % hostname
-                )
-            if not is_subordinate and len(addresses) > 0:
-                raise ValueError("Must not supply IP addresses for %s" % hostname)
-
-            for address in addresses:
-                allow = set(":." + digits)
-                if any(c not in allow for c in address):
-                    raise ValueError("Invalid IP address: %s." % address)
-
     def _get_or_create_domain(self):
         """Try to fetch info about this domain. Create it if it does not exist."""
         already_tried_to_create = False
@@ -1593,7 +1600,12 @@ class Domain(TimeStampedModel, DomainHelper):
             return response.code
         except RegistryError as e:
             logger.error("Error _update_host, code was %s error was %s" % (e.code, e))
-            raise e
+            # OBJECT_EXISTS is an expected error code that should not raise
+            # an exception, rather return the code to be handled separately
+            if e.code == ErrorCode.OBJECT_EXISTS:
+                return e.code
+            else:
+                raise e
 
     def addAndRemoveHostsFromDomain(
         self, hostsToAdd: list[str], hostsToDelete: list[str]

src/registrar/models/utility/domain_helper.py

@@ -11,10 +11,6 @@ class DomainHelper:
     # begin or end with a hyphen, followed by a TLD of 2-6 alphabetic characters
     DOMAIN_REGEX = re.compile(r"^(?!-)[A-Za-z0-9-]{1,63}(?<!-)\.[A-Za-z]{2,6}$")
 
-    # a domain name is alphanumeric or hyphen, has at least 2 dots, doesn't
-    # begin or end with a hyphen, followed by a TLD of 2-6 alphabetic characters
-    HOST_REGEX = re.compile(r"^((?!-)[A-Za-z0-9-]{1,63}(?<!-)\.){2,}([A-Za-z]){2,6}$")
-
     # a domain can be no longer than 253 characters in total
     MAX_LENGTH = 253
 

src/registrar/templates/domain_nameservers.html

@@ -17,7 +17,7 @@
 
   <div class="usa-alert usa-alert--slim usa-alert--info">
     <div class="usa-alert__body">
-      <p class="margin-top-0">Add an IP address only when your name server's address includes your domain name (e.g., if your domain name is "example.gov" and your name server is "ns1.example.gov,” then an IP address is required.) To add multiple IP addresses, separate them with commas.</p>
+      <p class="margin-top-0">Add an IP address only when your name server's address includes your domain name (e.g., if your domain name is “example.gov” and your name server is “ns1.example.gov,” then an IP address is required.) To add multiple IP addresses, separate them with commas.</p>
       <p class="margin-bottom-0">This step is uncommon unless you self-host your DNS or use custom addresses for your nameserver.</p>
     </div>
   </div>

src/registrar/tests/test_models_domain.py

@@ -1630,6 +1630,56 @@ class TestRegistrantNameservers(MockEppLib):
         return super().tearDown()
 
 
+class TestNameserverValidation(TestCase):
+    """Test the isValidDomain method which validates nameservers"""
+
+    def test_255_chars_is_too_long(self):
+        """Test that domain of 255 chars or longer is invalid"""
+        domain_too_long = (
+            "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+            ".bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+            ".bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+            ".bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+            ".bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+            ".bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.gov"
+        )
+        self.assertFalse(Domain.isValidHost(domain_too_long))
+
+    def test_64_char_label_too_long(self):
+        """Test that label of 64 characters or longer is invalid"""
+        label_too_long = (
+            "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+        )
+        domain_label_too_long = "www." + label_too_long + ".gov"
+        self.assertFalse(Domain.isValidHost(domain_label_too_long))
+
+    def test_only_tld_and_sld(self):
+        """Test that host with only a tld and sld is invalid"""
+        tld = "gov"
+        sld = "example"
+        domain_with_sld_and_tld = sld + "." + tld
+        self.assertFalse(Domain.isValidHost(domain_with_sld_and_tld))
+
+    def test_improper_chars_in_nameserver(self):
+        """Test that host with improper chars is invalid"""
+        invalid_chars = "*&^"
+        domain_with_invalid_chars = "www.bad--" + invalid_chars + ".gov"
+        self.assertFalse(Domain.isValidHost(domain_with_invalid_chars))
+
+    def test_misplaced_dashes(self):
+        """Test that misplaced dashes are invalid"""
+        self.assertFalse(Domain.isValidHost("-www.example.gov"))
+        self.assertFalse(Domain.isValidHost("www.example-.gov"))
+        self.assertTrue(Domain.isValidHost("www.ex-ample.gov"))
+
+    def test_valid_hostname(self):
+        """Test that valid hostnames are valid"""
+        self.assertTrue(Domain.isValidHost("www.tld.sld.gov"))
+        self.assertTrue(Domain.isValidHost("www.valid.c"))
+        self.assertTrue(Domain.isValidHost("2ww.valid.gov"))
+        self.assertTrue(Domain.isValidHost("w.t.g"))
+
+
 class TestRegistrantDNSSEC(MockEppLib):
     """Rule: Registrants may modify their secure DNS data"""
 

src/registrar/tests/test_views.py

@@ -1589,6 +1589,39 @@ class TestDomainNameservers(TestDomainOverview):
             status_code=200,
         )
 
+    def test_domain_nameservers_form_submit_invalid_host(self):
+        """Nameserver form catches invalid host on submission.
+
+        Uses self.app WebTest because we need to interact with forms.
+        """
+        nameserver = "invalid-nameserver.gov"
+        valid_ip = "123.2.45.111"
+        # initial nameservers page has one server with two ips
+        nameservers_page = self.app.get(
+            reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id})
+        )
+        session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
+        self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+        # attempt to submit the form without two hosts, both subdomains,
+        # only one has ips
+        nameservers_page.form["form-1-server"] = nameserver
+        nameservers_page.form["form-1-ip"] = valid_ip
+        with less_console_noise():  # swallow log warning message
+            result = nameservers_page.form.submit()
+        # form submission was a post with an error, response should be a 200
+        # error text appears twice, once at the top of the page, once around
+        # the required field.  nameserver has invalid host
+        self.assertContains(
+            result,
+            str(
+                NameserverError(
+                    code=NameserverErrorCodes.INVALID_HOST, nameserver=nameserver
+                )
+            ),
+            count=2,
+            status_code=200,
+        )
+
     def test_domain_nameservers_form_submits_successfully(self):
         """Nameserver form submits successfully with valid input.
 

src/registrar/utility/errors.py

@@ -68,6 +68,7 @@ class NameserverErrorCodes(IntEnum):
         - 4 TOO_MANY_HOSTS  more than the max allowed host values
         - 5 UNABLE_TO_UPDATE_DOMAIN unable to update the domain
         - 6 MISSING_HOST host is missing for a nameserver
+        - 7 INVALID_HOST host is invalid for a nameserver
     """
 
     MISSING_IP = 1
@@ -76,6 +77,7 @@ class NameserverErrorCodes(IntEnum):
     TOO_MANY_HOSTS = 4
     UNABLE_TO_UPDATE_DOMAIN = 5
     MISSING_HOST = 6
+    INVALID_HOST = 7
 
 
 class NameserverError(Exception):
@@ -104,6 +106,9 @@ class NameserverError(Exception):
         NameserverErrorCodes.MISSING_HOST: (
             "Name server must be provided to enter IP address."
         ),
+        NameserverErrorCodes.INVALID_HOST: (
+            "Enter a name server in the required format, like ns1.example.com"
+        ),
     }
 
     def __init__(self, *args, code=None, nameserver=None, ip=None, **kwargs):