Merge remote-tracking branch 'origin/main' into rjm/1192-delete-ns-1-2

docs/operations/runbooks/rotate_application_secrets.md

@@ -46,7 +46,7 @@ This is a standard Django secret key. See Django documentation for tips on gener
 
 This is the base64 encoded private key used in the OpenID Connect authentication flow with Login.gov. It is used to sign a token during user login; the signature is examined by Login.gov before their API grants access to user data.
 
-Generate a new key using this command (or whatever is most recently recommended by Login.gov):
+Generate a new key using this command (or whatever is most recently [recommended by Login.gov](https://developers.login.gov/testing/#creating-a-public-certificate)):
 
 ```bash
 openssl req -nodes -x509 -days 365 -newkey rsa:2048 -keyout private.pem -out public.crt

src/registrar/assets/js/get-gov.js

@@ -238,10 +238,8 @@ function handleValidationClick(e) {
 function prepareDeleteButtons(formLabel) {
   let deleteButtons = document.querySelectorAll(".delete-record");
   let totalForms = document.querySelector("#id_form-TOTAL_FORMS");
-  let isNameserversForm = false;
+  let isNameserversForm = document.title.includes("DNS name servers |");
   let addButton = document.querySelector("#add-form");
-  if (document.title.includes("DNS name servers |"))
-    isNameserversForm = true;
 
   // Loop through each delete button and attach the click event listener
   deleteButtons.forEach((deleteButton) => {
@@ -309,7 +307,7 @@ function prepareDeleteButtons(formLabel) {
         }
       });
 
-      // Remove the add more button if we have less than 13 forms
+      // Display the add more button if we have less than 13 forms
       if (isNameserversForm && forms.length <= 13) {
         addButton.classList.remove("display-none");
       }
@@ -339,9 +337,8 @@ function prepareDeleteButtons(formLabel) {
   let totalForms = document.querySelector("#id_form-TOTAL_FORMS");
   let cloneIndex = 0;
   let formLabel = '';
-  let isNameserversForm = false;
+  let isNameserversForm = document.title.includes("DNS name servers |");
-  if (document.title.includes("DNS name servers |")) {
+  if (isNameserversForm) {
-    isNameserversForm = true;
     cloneIndex = 2;
     formLabel = "Name server";
   } else if ((document.title.includes("DS Data |")) || (document.title.includes("Key Data |"))) {

src/registrar/forms/domain.py

@@ -54,6 +54,8 @@ class DomainNameserverForm(forms.Form):
         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)

src/registrar/models/domain.py

@@ -256,7 +256,6 @@ class Domain(TimeStampedModel, DomainHelper):
         hostList = []
         for host in hosts:
             hostList.append((host["name"], host["addrs"]))
-
         return hostList
 
     def _create_host(self, host, addrs):
@@ -278,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
@@ -1594,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]

src/registrar/templates/domain_detail.html

@@ -29,7 +29,7 @@
 
     {% url 'domain-dns-nameservers' pk=domain.id as url %}
     {% if domain.nameservers|length > 0 %}
-      {% include "includes/summary_item.html" with title='DNS name servers' value=domain.nameservers list='true' edit_link=url %}
+      {% include "includes/summary_item.html" with title='DNS name servers' domains='true' value=domain.nameservers list='true' edit_link=url %}
     {% else %}
       <h2 class="margin-top-neg-1"> DNS name servers </h2>
       <p> No DNS name servers have been added yet. Before your domain can be used we’ll need information about your domain name servers.</p>

src/registrar/templates/includes/summary_item.html

@@ -43,7 +43,6 @@
         {% else %}
           {% include "includes/contact.html" with contact=value %}
         {% endif %}
-      <!-- TODO #687 add here another elif for nameservers to show item Ips -->
       {% elif list %}
         {% if value|length == 1 %}
             {% if users %}
@@ -56,6 +55,17 @@
             {% for item in value %}
             {% if users %}
               <li>{{ item.user.email }}</li>
+            {% elif domains %}
+              <li>
+                {{ item.0 }}
+                {% if item.1 %}
+                  ({% spaceless %}
+                  {% for addr in item.1 %}
+                    {{addr}}{% if not forloop.last %}, {% endif %}
+                  {% endfor %}
+                  {% endspaceless %})
+                {% endif %}
+              </li>
             {% else %}
               <li>{{ item }}</li>
             {% endif %}

src/registrar/tests/common.py

@@ -738,6 +738,7 @@ class MockEppLib(TestCase):
             "ns1.cats-are-superior3.com",
         ],
     )
+
     infoDomainNoHost = fakedEppObject(
         "my-nameserver.gov",
         cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35),
@@ -793,7 +794,20 @@ class MockEppLib(TestCase):
     infoDomainHasIP = fakedEppObject(
         "nameserverwithip.gov",
         cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35),
-        contacts=[],
+        contacts=[
+            common.DomainContact(
+                contact="securityContact",
+                type=PublicContact.ContactTypeChoices.SECURITY,
+            ),
+            common.DomainContact(
+                contact="technicalContact",
+                type=PublicContact.ContactTypeChoices.TECHNICAL,
+            ),
+            common.DomainContact(
+                contact="adminContact",
+                type=PublicContact.ContactTypeChoices.ADMINISTRATIVE,
+            ),
+        ],
         hosts=[
             "ns1.nameserverwithip.gov",
             "ns2.nameserverwithip.gov",
@@ -802,6 +816,29 @@ class MockEppLib(TestCase):
         addrs=[common.Ip(addr="1.2.3.4"), common.Ip(addr="2.3.4.5")],
     )
 
+    justNameserver = fakedEppObject(
+        "justnameserver.com",
+        cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35),
+        contacts=[
+            common.DomainContact(
+                contact="securityContact",
+                type=PublicContact.ContactTypeChoices.SECURITY,
+            ),
+            common.DomainContact(
+                contact="technicalContact",
+                type=PublicContact.ContactTypeChoices.TECHNICAL,
+            ),
+            common.DomainContact(
+                contact="adminContact",
+                type=PublicContact.ContactTypeChoices.ADMINISTRATIVE,
+            ),
+        ],
+        hosts=[
+            "ns1.justnameserver.com",
+            "ns2.justnameserver.com",
+        ],
+    )
+
     infoDomainCheckHostIPCombo = fakedEppObject(
         "nameserversubdomain.gov",
         cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35),
@@ -922,6 +959,7 @@ class MockEppLib(TestCase):
             "threenameserversDomain.gov": (self.infoDomainThreeHosts, None),
             "defaultsecurity.gov": (self.InfoDomainWithDefaultSecurityContact, None),
             "defaulttechnical.gov": (self.InfoDomainWithDefaultTechnicalContact, None),
+            "justnameserver.com": (self.justNameserver, None),
         }
 
         # Retrieve the corresponding values from the dictionary

src/registrar/tests/test_views.py

@@ -1099,6 +1099,13 @@ class TestWithDomainPermissions(TestWithUser):
     def setUp(self):
         super().setUp()
         self.domain, _ = Domain.objects.get_or_create(name="igorville.gov")
+        self.domain_with_ip, _ = Domain.objects.get_or_create(
+            name="nameserverwithip.gov"
+        )
+        self.domain_just_nameserver, _ = Domain.objects.get_or_create(
+            name="justnameserver.com"
+        )
+
         self.domain_dsdata, _ = Domain.objects.get_or_create(name="dnssec-dsdata.gov")
         self.domain_multdsdata, _ = Domain.objects.get_or_create(
             name="dnssec-multdsdata.gov"
@@ -1108,9 +1115,11 @@ class TestWithDomainPermissions(TestWithUser):
         self.domain_dnssec_none, _ = Domain.objects.get_or_create(
             name="dnssec-none.gov"
         )
+
         self.domain_information, _ = DomainInformation.objects.get_or_create(
             creator=self.user, domain=self.domain
         )
+
         DomainInformation.objects.get_or_create(
             creator=self.user, domain=self.domain_dsdata
         )
@@ -1120,9 +1129,17 @@ class TestWithDomainPermissions(TestWithUser):
         DomainInformation.objects.get_or_create(
             creator=self.user, domain=self.domain_dnssec_none
         )
+        DomainInformation.objects.get_or_create(
+            creator=self.user, domain=self.domain_with_ip
+        )
+        DomainInformation.objects.get_or_create(
+            creator=self.user, domain=self.domain_just_nameserver
+        )
+
         self.role, _ = UserDomainRole.objects.get_or_create(
             user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER
         )
+
         UserDomainRole.objects.get_or_create(
             user=self.user, domain=self.domain_dsdata, role=UserDomainRole.Roles.MANAGER
         )
@@ -1136,6 +1153,16 @@ class TestWithDomainPermissions(TestWithUser):
             domain=self.domain_dnssec_none,
             role=UserDomainRole.Roles.MANAGER,
         )
+        UserDomainRole.objects.get_or_create(
+            user=self.user,
+            domain=self.domain_with_ip,
+            role=UserDomainRole.Roles.MANAGER,
+        )
+        UserDomainRole.objects.get_or_create(
+            user=self.user,
+            domain=self.domain_just_nameserver,
+            role=UserDomainRole.Roles.MANAGER,
+        )
 
     def tearDown(self):
         try:
@@ -1219,6 +1246,37 @@ class TestDomainOverview(TestWithDomainPermissions, WebTest):
             response = self.client.get(reverse("domain", kwargs={"pk": self.domain.id}))
             self.assertEqual(response.status_code, 403)
 
+    def test_domain_see_just_nameserver(self):
+        home_page = self.app.get("/")
+        self.assertContains(home_page, "justnameserver.com")
+
+        # View nameserver on Domain Overview page
+        detail_page = self.app.get(
+            reverse("domain", kwargs={"pk": self.domain_just_nameserver.id})
+        )
+
+        self.assertContains(detail_page, "justnameserver.com")
+        self.assertContains(detail_page, "ns1.justnameserver.com")
+        self.assertContains(detail_page, "ns2.justnameserver.com")
+
+    def test_domain_see_nameserver_and_ip(self):
+        home_page = self.app.get("/")
+        self.assertContains(home_page, "nameserverwithip.gov")
+
+        # View nameserver on Domain Overview page
+        detail_page = self.app.get(
+            reverse("domain", kwargs={"pk": self.domain_with_ip.id})
+        )
+
+        self.assertContains(detail_page, "nameserverwithip.gov")
+
+        self.assertContains(detail_page, "ns1.nameserverwithip.gov")
+        self.assertContains(detail_page, "ns2.nameserverwithip.gov")
+        self.assertContains(detail_page, "ns3.nameserverwithip.gov")
+        # Splitting IP addresses bc there is odd whitespace and can't strip text
+        self.assertContains(detail_page, "(1.2.3.4,")
+        self.assertContains(detail_page, "2.3.4.5)")
+
 
 class TestDomainManagers(TestDomainOverview):
     def test_domain_managers(self):
@@ -1389,7 +1447,7 @@ class TestDomainNameservers(TestDomainOverview):
         self.assertContains(page, "DNS name servers")
 
     def test_domain_nameservers_form_submit_one_nameserver(self):
-        """Can change domain's nameservers.
+        """Nameserver form submitted with one nameserver throws error.
 
         Uses self.app WebTest because we need to interact with forms.
         """
@@ -1414,7 +1472,7 @@ class TestDomainNameservers(TestDomainOverview):
         )
 
     def test_domain_nameservers_form_submit_subdomain_missing_ip(self):
-        """Can change domain's nameservers.
+        """Nameserver form catches missing ip error on subdomain.
 
         Uses self.app WebTest because we need to interact with forms.
         """
@@ -1440,7 +1498,7 @@ class TestDomainNameservers(TestDomainOverview):
         )
 
     def test_domain_nameservers_form_submit_missing_host(self):
-        """Can change domain's nameservers.
+        """Nameserver form catches error when host is missing.
 
         Uses self.app WebTest because we need to interact with forms.
         """
@@ -1466,7 +1524,8 @@ class TestDomainNameservers(TestDomainOverview):
         )
 
     def test_domain_nameservers_form_submit_glue_record_not_allowed(self):
-        """Can change domain's nameservers.
+        """Nameserver form catches error when IP is present
+        but host not subdomain.
 
         Uses self.app WebTest because we need to interact with forms.
         """
@@ -1497,7 +1556,7 @@ class TestDomainNameservers(TestDomainOverview):
         )
 
     def test_domain_nameservers_form_submit_invalid_ip(self):
-        """Can change domain's nameservers.
+        """Nameserver form catches invalid IP on submission.
 
         Uses self.app WebTest because we need to interact with forms.
         """
@@ -1530,7 +1589,7 @@ class TestDomainNameservers(TestDomainOverview):
         )
 
     def test_domain_nameservers_form_submits_successfully(self):
-        """Can change domain's nameservers.
+        """Nameserver form submits successfully with valid input.
 
         Uses self.app WebTest because we need to interact with forms.
         """
@@ -1561,7 +1620,7 @@ class TestDomainNameservers(TestDomainOverview):
         self.assertContains(page, "The name servers for this domain have been updated")
 
     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.
         """

src/registrar/views/domain.py

@@ -295,10 +295,6 @@ class DomainNameserversView(DomainFormBaseView):
                 if ip_string:
                     # Split the string into a list using a comma as the delimiter
                     ip_list = ip_string.split(",")
-                # Remove any leading or trailing whitespace from each IP in the list
-                # this will return [] if no ips have been entered, which is taken
-                # into account in the model in checkHostIPCombo
-                ip_list = [ip.strip() for ip in ip_list]
 
                 as_tuple = (
                     form.cleaned_data["server"],