diff --git a/src/epplibwrapper/__init__.py b/src/epplibwrapper/__init__.py index d0138d73c..dd6664a3a 100644 --- a/src/epplibwrapper/__init__.py +++ b/src/epplibwrapper/__init__.py @@ -45,7 +45,7 @@ except NameError: # Attn: these imports should NOT be at the top of the file try: from .client import CLIENT, commands - from .errors import RegistryError, ErrorCode, CANNOT_CONTACT_REGISTRY, GENERIC_ERROR + from .errors import RegistryError, ErrorCode from epplib.models import common, info from epplib.responses import extensions from epplib import responses @@ -61,6 +61,4 @@ __all__ = [ "info", "ErrorCode", "RegistryError", - "CANNOT_CONTACT_REGISTRY", - "GENERIC_ERROR", ] diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index dba5f328c..d34ed5e91 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -1,8 +1,5 @@ from enum import IntEnum -CANNOT_CONTACT_REGISTRY = "Update failed. Cannot contact the registry." -GENERIC_ERROR = "Value entered was wrong." - class ErrorCode(IntEnum): """ diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 851de8fcf..1c678a4d6 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -231,41 +231,15 @@ function handleValidationClick(e) { /** - * An IIFE that attaches a click handler for our dynamic nameservers form - * - * Only does something on a single page, but it should be fast enough to run - * it everywhere. + * Prepare the namerservers and DS data forms delete buttons + * We will call this on the forms init, and also every time we add a form + * */ -(function prepareNameserverForms() { - let serverForm = document.querySelectorAll(".server-form"); - let container = document.querySelector("#form-container"); - let addButton = document.querySelector("#add-nameserver-form"); - let totalForms = document.querySelector("#id_form-TOTAL_FORMS"); - - let formNum = serverForm.length-1; - if (addButton) - addButton.addEventListener('click', addForm); - - function addForm(e){ - let newForm = serverForm[2].cloneNode(true); - let formNumberRegex = RegExp(`form-(\\d){1}-`,'g'); - let formLabelRegex = RegExp(`Name server (\\d){1}`, 'g'); - let formExampleRegex = RegExp(`ns(\\d){1}`, 'g'); - - formNum++; - newForm.innerHTML = newForm.innerHTML.replace(formNumberRegex, `form-${formNum}-`); - newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `Name server ${formNum+1}`); - newForm.innerHTML = newForm.innerHTML.replace(formExampleRegex, `ns${formNum+1}`); - container.insertBefore(newForm, addButton); - newForm.querySelector("input").value = ""; - - totalForms.setAttribute('value', `${formNum+1}`); - } -})(); - -function prepareDeleteButtons() { +function prepareDeleteButtons(formLabel) { let deleteButtons = document.querySelectorAll(".delete-record"); let totalForms = document.querySelector("#id_form-TOTAL_FORMS"); + let isNameserversForm = document.title.includes("DNS name servers |"); + let addButton = document.querySelector("#add-form"); // Loop through each delete button and attach the click event listener deleteButtons.forEach((deleteButton) => { @@ -273,13 +247,15 @@ function prepareDeleteButtons() { }); function removeForm(e){ - let formToRemove = e.target.closest(".ds-record"); + let formToRemove = e.target.closest(".repeatable-form"); formToRemove.remove(); - let forms = document.querySelectorAll(".ds-record"); + let forms = document.querySelectorAll(".repeatable-form"); totalForms.setAttribute('value', `${forms.length}`); let formNumberRegex = RegExp(`form-(\\d){1}-`, 'g'); - let formLabelRegex = RegExp(`DS Data record (\\d){1}`, 'g'); + let formLabelRegex = RegExp(`${formLabel} (\\d+){1}`, 'g'); + // For the example on Nameservers + let formExampleRegex = RegExp(`ns(\\d+){1}`, 'g'); forms.forEach((form, index) => { // Iterate over child nodes of the current element @@ -294,48 +270,88 @@ function prepareDeleteButtons() { }); }); - Array.from(form.querySelectorAll('h2, legend')).forEach((node) => { - node.textContent = node.textContent.replace(formLabelRegex, `DS Data record ${index + 1}`); + // h2 and legend for DS form, label for nameservers + Array.from(form.querySelectorAll('h2, legend, label, p')).forEach((node) => { + + // Ticket: 1192 + // if (isNameserversForm && index <= 1 && !node.innerHTML.includes('*')) { + // // Create a new element + // const newElement = document.createElement('abbr'); + // newElement.textContent = '*'; + // // TODO: finish building abbr + + // // Append the new element to the parent + // node.appendChild(newElement); + // // Find the next sibling that is an input element + // let nextInputElement = node.nextElementSibling; + + // while (nextInputElement) { + // if (nextInputElement.tagName === 'INPUT') { + // // Found the next input element + // console.log(nextInputElement); + // break; + // } + // nextInputElement = nextInputElement.nextElementSibling; + // } + // nextInputElement.required = true; + // } + + // Ticket: 1192 - remove if + if (!(isNameserversForm && index <= 1)) { + node.textContent = node.textContent.replace(formLabelRegex, `${formLabel} ${index + 1}`); + node.textContent = node.textContent.replace(formExampleRegex, `ns${index + 1}`); + } }); + + // Display the add more button if we have less than 13 forms + if (isNameserversForm && forms.length <= 13) { + addButton.classList.remove("display-none") + } }); } } /** - * An IIFE that attaches a click handler for our dynamic DNSSEC forms + * An IIFE that attaches a click handler for our dynamic formsets * + * Only does something on a few pages, but it should be fast enough to run + * it everywhere. */ -(function prepareDNSSECForms() { - let serverForm = document.querySelectorAll(".ds-record"); +(function prepareFormsetsForms() { + let repeatableForm = document.querySelectorAll(".repeatable-form"); let container = document.querySelector("#form-container"); - let addButton = document.querySelector("#add-ds-form"); + let addButton = document.querySelector("#add-form"); let totalForms = document.querySelector("#id_form-TOTAL_FORMS"); + let cloneIndex = 0; + let formLabel = ''; + let isNameserversForm = document.title.includes("DNS name servers |"); + if (isNameserversForm) { + cloneIndex = 2; + formLabel = "Name server"; + } else if ((document.title.includes("DS Data |")) || (document.title.includes("Key Data |"))) { + formLabel = "DS Data record"; + } // Attach click event listener on the delete buttons of the existing forms - prepareDeleteButtons(); + prepareDeleteButtons(formLabel); - // Attack click event listener on the add button if (addButton) addButton.addEventListener('click', addForm); - /* - * Add a formset to the end of the form. - * For each element in the added formset, name the elements with the prefix, - * form-{#}-{element_name} where # is the index of the formset and element_name - * is the element's name. - * Additionally, update the form element's metadata, including totalForms' value. - */ function addForm(e){ - let forms = document.querySelectorAll(".ds-record"); + let forms = document.querySelectorAll(".repeatable-form"); let formNum = forms.length; - let newForm = serverForm[0].cloneNode(true); + let newForm = repeatableForm[cloneIndex].cloneNode(true); let formNumberRegex = RegExp(`form-(\\d){1}-`,'g'); - let formLabelRegex = RegExp(`DS Data record (\\d){1}`, 'g'); + let formLabelRegex = RegExp(`${formLabel} (\\d){1}`, 'g'); + // For the eample on Nameservers + let formExampleRegex = RegExp(`ns(\\d){1}`, 'g'); formNum++; newForm.innerHTML = newForm.innerHTML.replace(formNumberRegex, `form-${formNum-1}-`); - newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `DS Data record ${formNum}`); + newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `${formLabel} ${formNum}`); + newForm.innerHTML = newForm.innerHTML.replace(formExampleRegex, `ns${formNum}`); container.insertBefore(newForm, addButton); let inputs = newForm.querySelectorAll("input"); @@ -379,9 +395,13 @@ function prepareDeleteButtons() { totalForms.setAttribute('value', `${formNum}`); // Attach click event listener on the delete buttons of the new form - prepareDeleteButtons(); - } + prepareDeleteButtons(formLabel); + // Hide the add more button if we have 13 forms + if (isNameserversForm && formNum == 13) { + addButton.classList.add("display-none") + } + } })(); /** diff --git a/src/registrar/assets/sass/_theme/_forms.scss b/src/registrar/assets/sass/_theme/_forms.scss index 38b42c3d0..d0bfbee67 100644 --- a/src/registrar/assets/sass/_theme/_forms.scss +++ b/src/registrar/assets/sass/_theme/_forms.scss @@ -4,6 +4,10 @@ margin-top: units(3); } +.usa-form .usa-button.margin-bottom-075 { + margin-bottom: units(1.5); +} + .usa-form .usa-button.margin-top-1 { margin-top: units(1); } diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 6bbade5ef..3aca7af6d 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -5,8 +5,12 @@ from django.core.validators import MinValueValidator, MaxValueValidator, RegexVa from django.forms import formset_factory from phonenumber_field.widgets import RegionalPhoneNumberWidget +from registrar.utility.errors import ( + NameserverError, + NameserverErrorCodes as nsErrorCodes, +) -from ..models import Contact, DomainInformation +from ..models import Contact, DomainInformation, Domain from .common import ( ALGORITHM_CHOICES, DIGEST_TYPE_CHOICES, @@ -19,16 +23,78 @@ class DomainAddUserForm(forms.Form): email = forms.EmailField(label="Email") +class IPAddressField(forms.CharField): + def validate(self, value): + super().validate(value) # Run the default CharField validation + + class DomainNameserverForm(forms.Form): """Form for changing nameservers.""" + domain = forms.CharField(widget=forms.HiddenInput, required=False) + server = forms.CharField(label="Name server", strip=True) - # when adding IPs to this form ensure they are stripped as well + + ip = forms.CharField(label="IP Address (IPv4 or IPv6)", strip=True, required=False) + + def clean(self): + # clean is called from clean_forms, which is called from is_valid + # after clean_fields. it is used to determine form level errors. + # is_valid is typically called from view during a post + cleaned_data = super().clean() + self.clean_empty_strings(cleaned_data) + server = cleaned_data.get("server", "") + ip = cleaned_data.get("ip", None) + # remove ANY spaces in the ip field + ip = ip.replace(" ", "") + domain = cleaned_data.get("domain", "") + + 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: + self.validate_nameserver_ip_combo(domain, server, ip_list) + + return cleaned_data + + def clean_empty_strings(self, cleaned_data): + ip = cleaned_data.get("ip", "") + if ip and len(ip.strip()) == 0: + cleaned_data["ip"] = None + + def extract_ip_list(self, ip): + return [ip.strip() for ip in ip.split(",")] if ip else [] + + def validate_nameserver_ip_combo(self, domain, server, ip_list): + try: + Domain.checkHostIPCombo(domain, server, ip_list) + except NameserverError as e: + if e.code == nsErrorCodes.GLUE_RECORD_NOT_ALLOWED: + self.add_error( + "server", + NameserverError( + code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, + nameserver=domain, + ip=ip_list, + ), + ) + elif e.code == nsErrorCodes.MISSING_IP: + self.add_error( + "ip", + NameserverError( + code=nsErrorCodes.MISSING_IP, nameserver=domain, ip=ip_list + ), + ) + else: + self.add_error("ip", str(e)) NameserverFormset = formset_factory( DomainNameserverForm, extra=1, + max_num=13, + validate_max=True, ) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 12cb8b5db..07e49dfdd 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -262,8 +262,11 @@ class Domain(TimeStampedModel, DomainHelper): """Creates the host object in the registry doesn't add the created host to the domain returns ErrorCode (int)""" - if addrs is not None: - addresses = [epp.Ip(addr=addr) for addr in addrs] + if addrs is not None and addrs != []: + addresses = [ + epp.Ip(addr=addr, ip="v6" if self.is_ipv6(addr) else None) + for addr in addrs + ] request = commands.CreateHost(name=host, addrs=addresses) else: request = commands.CreateHost(name=host) @@ -274,7 +277,7 @@ 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)) - return e.code + raise e def _convert_list_to_dict(self, listToConvert: list[tuple[str, list]]): """converts a list of hosts into a dictionary @@ -293,14 +296,16 @@ class Domain(TimeStampedModel, DomainHelper): newDict[tup[0]] = tup[1] return newDict - def isSubdomain(self, nameserver: str): + @classmethod + def isSubdomain(cls, name: str, nameserver: str): """Returns boolean if the domain name is found in the argument passed""" subdomain_pattern = r"([\w-]+\.)*" - full_pattern = subdomain_pattern + self.name + full_pattern = subdomain_pattern + name regex = re.compile(full_pattern) return bool(regex.match(nameserver)) - def checkHostIPCombo(self, nameserver: str, ip: list[str]): + @classmethod + def checkHostIPCombo(cls, name: str, nameserver: str, ip: list[str]): """Checks the parameters past for a valid combination raises error if: - nameserver is a subdomain but is missing ip @@ -314,22 +319,23 @@ class Domain(TimeStampedModel, DomainHelper): NameserverError (if exception hit) Returns: None""" - if self.isSubdomain(nameserver) and (ip is None or ip == []): + if cls.isSubdomain(name, nameserver) and (ip is None or ip == []): raise NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver) - elif not self.isSubdomain(nameserver) and (ip is not None and ip != []): + elif not cls.isSubdomain(name, nameserver) and (ip is not None and ip != []): raise NameserverError( code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, nameserver=nameserver, ip=ip ) elif ip is not None and ip != []: for addr in ip: - if not self._valid_ip_addr(addr): + if not cls._valid_ip_addr(addr): raise NameserverError( code=nsErrorCodes.INVALID_IP, nameserver=nameserver, ip=ip ) return None - def _valid_ip_addr(self, ipToTest: str): + @classmethod + def _valid_ip_addr(cls, ipToTest: str): """returns boolean if valid ip address string We currently only accept v4 or v6 ips returns: @@ -382,7 +388,9 @@ class Domain(TimeStampedModel, DomainHelper): if newHostDict[prevHost] is not None and set( newHostDict[prevHost] ) != set(addrs): - self.checkHostIPCombo(nameserver=prevHost, ip=newHostDict[prevHost]) + self.__class__.checkHostIPCombo( + name=self.name, nameserver=prevHost, ip=newHostDict[prevHost] + ) updated_values.append((prevHost, newHostDict[prevHost])) new_values = { @@ -392,7 +400,9 @@ class Domain(TimeStampedModel, DomainHelper): } for nameserver, ip in new_values.items(): - self.checkHostIPCombo(nameserver=nameserver, ip=ip) + self.__class__.checkHostIPCombo( + name=self.name, nameserver=nameserver, ip=ip + ) return (deleted_values, updated_values, new_values, previousHostDict) @@ -567,7 +577,11 @@ class Domain(TimeStampedModel, DomainHelper): if len(hosts) > 13: raise NameserverError(code=nsErrorCodes.TOO_MANY_HOSTS) - if self.state not in [self.State.DNS_NEEDED, self.State.READY]: + if self.state not in [ + self.State.DNS_NEEDED, + self.State.READY, + self.State.UNKNOWN, + ]: raise ActionNotAllowed("Nameservers can not be " "set in the current state") logger.info("Setting nameservers") @@ -1351,7 +1365,7 @@ class Domain(TimeStampedModel, DomainHelper): @transition( field="state", - source=[State.DNS_NEEDED], + source=[State.DNS_NEEDED, State.READY], target=State.READY, # conditions=[dns_not_needed] ) @@ -1514,7 +1528,7 @@ class Domain(TimeStampedModel, DomainHelper): data = registry.send(req, cleaned=True).res_data[0] host = { "name": name, - "addrs": getattr(data, "addrs", ...), + "addrs": [item.addr for item in getattr(data, "addrs", [])], "cr_date": getattr(data, "cr_date", ...), "statuses": getattr(data, "statuses", ...), "tr_date": getattr(data, "tr_date", ...), @@ -1539,10 +1553,9 @@ class Domain(TimeStampedModel, DomainHelper): return [] for ip_addr in ip_list: - if self.is_ipv6(ip_addr): - edited_ip_list.append(epp.Ip(addr=ip_addr, ip="v6")) - else: # default ip addr is v4 - edited_ip_list.append(epp.Ip(addr=ip_addr)) + edited_ip_list.append( + epp.Ip(addr=ip_addr, ip="v6" if self.is_ipv6(ip_addr) else None) + ) return edited_ip_list @@ -1580,7 +1593,7 @@ 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)) - return e.code + raise e def addAndRemoveHostsFromDomain( self, hostsToAdd: list[str], hostsToDelete: list[str] diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index bdf4deb46..927628b11 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -29,7 +29,7 @@ {{ formset.management_form }} {% for form in formset %} -
+
DS Data record {{forloop.counter}} @@ -74,7 +74,7 @@
{% endfor %} - + {% endif %} + + {% endfor %} - - - + {% comment %} Work around USWDS' button margins to add some spacing between the submit and the 'add more' + This solution still works when we remove the 'add more' at 13 forms {% endcomment %} +
+ + +
+ {% endblock %} {# domain_content #} diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index e66bc7723..ad464bb3e 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -756,7 +756,7 @@ class MockEppLib(TestCase): mockDataInfoHosts = fakedEppObject( "lastPw", cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35), - addrs=["1.2.3.4", "2.3.4.5"], + addrs=[common.Ip(addr="1.2.3.4"), common.Ip(addr="2.3.4.5")], ) mockDataHostChange = fakedEppObject( @@ -813,7 +813,7 @@ class MockEppLib(TestCase): "ns2.nameserverwithip.gov", "ns3.nameserverwithip.gov", ], - addrs=["1.2.3.4", "2.3.4.5"], + addrs=[common.Ip(addr="1.2.3.4"), common.Ip(addr="2.3.4.5")], ) justNameserver = fakedEppObject( diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index c6b2b7ce9..bce442c9c 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -107,7 +107,7 @@ class TestDomainCache(MockEppLib): } expectedHostsDict = { "name": self.mockDataInfoDomain.hosts[0], - "addrs": self.mockDataInfoHosts.addrs, + "addrs": [item.addr for item in self.mockDataInfoHosts.addrs], "cr_date": self.mockDataInfoHosts.cr_date, } diff --git a/src/registrar/tests/test_nameserver_error.py b/src/registrar/tests/test_nameserver_error.py index c64717eb5..89a6dfcce 100644 --- a/src/registrar/tests/test_nameserver_error.py +++ b/src/registrar/tests/test_nameserver_error.py @@ -10,10 +10,7 @@ class TestNameserverError(TestCase): def test_with_no_ip(self): """Test NameserverError when no ip address is passed""" nameserver = "nameserver val" - expected = ( - f"Nameserver {nameserver} needs to have an " - "IP address because it is a subdomain" - ) + expected = "Using your domain for a name server requires an IP address" nsException = NameserverError( code=nsErrorCodes.MISSING_IP, nameserver=nameserver @@ -38,7 +35,7 @@ class TestNameserverError(TestCase): ip = "ip val" nameserver = "nameserver val" - expected = f"Nameserver {nameserver} has an invalid IP address: {ip}" + expected = f"{nameserver}: Enter an IP address in the required format." nsException = NameserverError( code=nsErrorCodes.INVALID_IP, nameserver=nameserver, ip=ip ) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 0a472b885..95af4c542 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -10,6 +10,10 @@ from .common import MockEppLib, completed_application # type: ignore from django_webtest import WebTest # type: ignore import boto3_mocking # type: ignore +from registrar.utility.errors import ( + NameserverError, + NameserverErrorCodes, +) from registrar.models import ( DomainApplication, @@ -1442,20 +1446,165 @@ class TestDomainNameservers(TestDomainOverview): ) self.assertContains(page, "DNS name servers") - @skip("Broken by adding registry connection fix in ticket 848") - def test_domain_nameservers_form(self): - """Can change domain's nameservers. + def test_domain_nameservers_form_submit_one_nameserver(self): + """Nameserver form submitted with one nameserver throws error. Uses self.app WebTest because we need to interact with forms. """ + # 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 with only one nameserver, should error + # regarding required fields with less_console_noise(): # swallow log warning message result = nameservers_page.form.submit() - # form submission was a post, response should be a redirect + # 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. form requires a minimum of 2 name servers + self.assertContains(result, "This field is required.", count=2, status_code=200) + + def test_domain_nameservers_form_submit_subdomain_missing_ip(self): + """Nameserver form catches missing ip error on subdomain. + + Uses self.app WebTest because we need to interact with forms. + """ + # 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"] = "ns2.igorville.gov" + 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. subdomain missing an ip + self.assertContains( + result, + str(NameserverError(code=NameserverErrorCodes.MISSING_IP)), + count=2, + status_code=200, + ) + + def test_domain_nameservers_form_submit_missing_host(self): + """Nameserver form catches error when host is missing. + + Uses self.app WebTest because we need to interact with forms. + """ + # 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-ip"] = "127.0.0.1" + 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 ip but missing host + self.assertContains( + result, + str(NameserverError(code=NameserverErrorCodes.MISSING_HOST)), + count=2, + status_code=200, + ) + + def test_domain_nameservers_form_submit_glue_record_not_allowed(self): + """Nameserver form catches error when IP is present + but host not subdomain. + + Uses self.app WebTest because we need to interact with forms. + """ + nameserver1 = "ns1.igorville.gov" + nameserver2 = "ns2.igorville.com" + valid_ip = "127.0.0.1" + # 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-0-server"] = nameserver1 + nameservers_page.form["form-1-server"] = nameserver2 + 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 ip but missing host + self.assertContains( + result, + str(NameserverError(code=NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED)), + count=2, + status_code=200, + ) + + def test_domain_nameservers_form_submit_invalid_ip(self): + """Nameserver form catches invalid IP on submission. + + Uses self.app WebTest because we need to interact with forms. + """ + nameserver = "ns2.igorville.gov" + invalid_ip = "123" + # 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"] = invalid_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 ip but missing host + self.assertContains( + result, + str( + NameserverError( + code=NameserverErrorCodes.INVALID_IP, nameserver=nameserver + ) + ), + count=2, + status_code=200, + ) + + def test_domain_nameservers_form_submits_successfully(self): + """Nameserver form submits successfully with valid input. + + Uses self.app WebTest because we need to interact with forms. + """ + nameserver1 = "ns1.igorville.gov" + nameserver2 = "ns2.igorville.gov" + invalid_ip = "127.0.0.1" + # 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-0-server"] = nameserver1 + nameservers_page.form["form-1-server"] = nameserver2 + nameservers_page.form["form-1-ip"] = invalid_ip + with less_console_noise(): # swallow log warning message + result = nameservers_page.form.submit() + # form submission was a successful post, response should be a 302 self.assertEqual(result.status_code, 302) self.assertEqual( result["Location"], @@ -1465,9 +1614,8 @@ class TestDomainNameservers(TestDomainOverview): page = result.follow() self.assertContains(page, "The name servers for this domain have been updated") - @skip("Broken by adding registry connection fix in ticket 848") def test_domain_nameservers_form_invalid(self): - """Can change domain's nameservers. + """Nameserver form does not submit with invalid data. Uses self.app WebTest because we need to interact with forms. """ @@ -1482,9 +1630,9 @@ class TestDomainNameservers(TestDomainOverview): with less_console_noise(): # swallow logged 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 field. - self.assertContains(result, "This field is required", count=2, status_code=200) + # error text appears four times, twice at the top of the page, + # once around each required field. + self.assertContains(result, "This field is required", count=4, status_code=200) class TestDomainAuthorizingOfficial(TestDomainOverview): diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index f7bc743d6..c1d3c5849 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -20,6 +20,41 @@ class ActionNotAllowed(Exception): pass +class GenericErrorCodes(IntEnum): + """Used across the registrar for + error mapping. + Overview of generic error codes: + - 1 GENERIC_ERROR a generic value error + - 2 CANNOT_CONTACT_REGISTRY a connection error w registry + """ + + GENERIC_ERROR = 1 + CANNOT_CONTACT_REGISTRY = 2 + + +class GenericError(Exception): + """ + GenericError class used to raise exceptions across + the registrar + """ + + _error_mapping = { + GenericErrorCodes.CANNOT_CONTACT_REGISTRY: ( + "Update failed. Cannot contact the registry." + ), + GenericErrorCodes.GENERIC_ERROR: ("Value entered was wrong."), + } + + def __init__(self, *args, code=None, **kwargs): + super().__init__(*args, **kwargs) + self.code = code + if self.code in self._error_mapping: + self.message = self._error_mapping.get(self.code) + + def __str__(self): + return f"{self.message}" + + class NameserverErrorCodes(IntEnum): """Used in the NameserverError class for error mapping. @@ -29,6 +64,8 @@ class NameserverErrorCodes(IntEnum): value but is not a subdomain - 3 INVALID_IP invalid ip address format or invalid version - 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 """ MISSING_IP = 1 @@ -36,6 +73,7 @@ class NameserverErrorCodes(IntEnum): INVALID_IP = 3 TOO_MANY_HOSTS = 4 UNABLE_TO_UPDATE_DOMAIN = 5 + MISSING_HOST = 6 class NameserverError(Exception): @@ -45,11 +83,15 @@ class NameserverError(Exception): """ _error_mapping = { - NameserverErrorCodes.MISSING_IP: "Nameserver {} needs to have an " - "IP address because it is a subdomain", - NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED: "Nameserver {} cannot be linked " - "because it is not a subdomain", - NameserverErrorCodes.INVALID_IP: "Nameserver {} has an invalid IP address: {}", + NameserverErrorCodes.MISSING_IP: ( + "Using your domain for a name server requires an IP address" + ), + NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED: ( + "Name server address does not match domain name" + ), + NameserverErrorCodes.INVALID_IP: ( + "{}: Enter an IP address in the required format." + ), NameserverErrorCodes.TOO_MANY_HOSTS: ( "Too many hosts provided, you may not have more than 13 nameservers." ), @@ -57,6 +99,9 @@ class NameserverError(Exception): "Unable to update domain, changes were not applied." "Check logs as a Registry Error is the likely cause" ), + NameserverErrorCodes.MISSING_HOST: ( + "Name server must be provided to enter IP address." + ), } def __init__(self, *args, code=None, nameserver=None, ip=None, **kwargs): @@ -65,7 +110,7 @@ class NameserverError(Exception): if self.code in self._error_mapping: self.message = self._error_mapping.get(self.code) if nameserver is not None and ip is not None: - self.message = self.message.format(str(nameserver), str(ip)) + self.message = self.message.format(str(nameserver)) elif nameserver is not None: self.message = self.message.format(str(nameserver)) elif ip is not None: diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index a2f2c1198..ede44b1d5 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -23,6 +23,12 @@ from registrar.models import ( UserDomainRole, ) from registrar.models.public_contact import PublicContact +from registrar.utility.errors import ( + GenericError, + GenericErrorCodes, + NameserverError, + NameserverErrorCodes as nsErrorCodes, +) from registrar.models.utility.contact_error import ContactError from ..forms import ( @@ -40,8 +46,6 @@ from epplibwrapper import ( common, extensions, RegistryError, - CANNOT_CONTACT_REGISTRY, - GENERIC_ERROR, ) from ..utility.email import send_templated_email, EmailSendingError @@ -215,6 +219,7 @@ class DomainNameserversView(DomainFormBaseView): template_name = "domain_nameservers.html" form_class = NameserverFormset + model = Domain def get_initial(self): """The initial value for the form (which is a formset here).""" @@ -223,7 +228,9 @@ class DomainNameserversView(DomainFormBaseView): if nameservers is not None: # Add existing nameservers as initial data - initial_data.extend({"server": name} for name, *ip in nameservers) + initial_data.extend( + {"server": name, "ip": ",".join(ip)} for name, ip in nameservers + ) # Ensure at least 3 fields, filled or empty while len(initial_data) < 2: @@ -252,25 +259,82 @@ class DomainNameserversView(DomainFormBaseView): form.fields["server"].required = True else: form.fields["server"].required = False + form.fields["domain"].initial = self.object.name return formset + def post(self, request, *args, **kwargs): + """Form submission posts to this view. + + This post method harmonizes using DomainBaseView and FormMixin + """ + self._get_domain(request) + formset = self.get_form() + + if "btn-cancel-click" in request.POST: + url = self.get_success_url() + return HttpResponseRedirect(url) + + if formset.is_valid(): + return self.form_valid(formset) + else: + return self.form_invalid(formset) + def form_valid(self, formset): """The formset is valid, perform something with it.""" + self.request.session["nameservers_form_domain"] = self.object + # Set the nameservers from the formset nameservers = [] for form in formset: try: - as_tuple = (form.cleaned_data["server"],) + ip_string = form.cleaned_data["ip"] + # ip_string will be None or a string of IP addresses + # comma-separated + ip_list = [] + if ip_string: + # Split the string into a list using a comma as the delimiter + ip_list = ip_string.split(",") + + as_tuple = ( + form.cleaned_data["server"], + ip_list, + ) nameservers.append(as_tuple) except KeyError: # no server information in this field, skip it pass - self.object.nameservers = nameservers - messages.success( - self.request, "The name servers for this domain have been updated." - ) + try: + self.object.nameservers = nameservers + except NameserverError as Err: + # NamserverErrors *should* be caught in form; if reached here, + # there was an uncaught error in submission (through EPP) + messages.error( + self.request, NameserverError(code=nsErrorCodes.UNABLE_TO_UPDATE_DOMAIN) + ) + logger.error(f"Nameservers error: {Err}") + # TODO: registry is not throwing an error when no connection + except RegistryError as Err: + if Err.is_connection_error(): + messages.error( + self.request, + GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY), + ) + logger.error(f"Registry connection error: {Err}") + else: + messages.error( + self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR) + ) + logger.error(f"Registry error: {Err}") + else: + messages.success( + self.request, + "The name servers for this domain have been updated. " + "Keep in mind that DNS changes may take some time to " + "propagate across the internet. It can take anywhere " + "from a few minutes to 48 hours for your changes to take place.", + ) # superclass has the redirect return super().form_valid(formset) @@ -431,10 +495,15 @@ class DomainDsDataView(DomainFormBaseView): self.object.dnssecdata = dnssecdata except RegistryError as err: if err.is_connection_error(): - messages.error(self.request, CANNOT_CONTACT_REGISTRY) + messages.error( + self.request, + GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY), + ) logger.error(f"Registry connection error: {err}") else: - messages.error(self.request, GENERIC_ERROR) + messages.error( + self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR) + ) logger.error(f"Registry error: {err}") return self.form_invalid(formset) else: @@ -510,7 +579,10 @@ class DomainSecurityEmailView(DomainFormBaseView): # If no default is created for security_contact, # then we cannot connect to the registry. if contact is None: - messages.error(self.request, CANNOT_CONTACT_REGISTRY) + messages.error( + self.request, + GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY), + ) return redirect(self.get_success_url()) contact.email = new_email @@ -519,13 +591,20 @@ class DomainSecurityEmailView(DomainFormBaseView): contact.save() except RegistryError as Err: if Err.is_connection_error(): - messages.error(self.request, CANNOT_CONTACT_REGISTRY) + messages.error( + self.request, + GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY), + ) logger.error(f"Registry connection error: {Err}") else: - messages.error(self.request, GENERIC_ERROR) + messages.error( + self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR) + ) logger.error(f"Registry error: {Err}") except ContactError as Err: - messages.error(self.request, GENERIC_ERROR) + messages.error( + self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR) + ) logger.error(f"Generic registry error: {Err}") else: messages.success(