diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index d48a14c6b..2c95c3e74 100644 --- a/src/registrar/forms/domain.py +++ b/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)) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 07e49dfdd..c5c0b63f7 100644 --- a/src/registrar/models/domain.py +++ b/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] diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 1a77e44b1..49badd5d7 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/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}(?
-

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.

+

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.

This step is uncommon unless you self-host your DNS or use custom addresses for your nameserver.

diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index bce442c9c..746625dbe 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/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""" diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 92fd5af44..c30e84248 100644 --- a/src/registrar/tests/test_views.py +++ b/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. diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 5d62953ac..d135aa1c2 100644 --- a/src/registrar/utility/errors.py +++ b/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):