From bbd3feaeec6ecc611d84e3d9f45eb8494e0baf15 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 17 Oct 2023 17:17:02 -0400 Subject: [PATCH 01/26] Axe all Key data components except in model --- src/registrar/config/urls.py | 5 - src/registrar/forms/__init__.py | 2 - src/registrar/forms/common.py | 15 +- src/registrar/forms/domain.py | 43 ----- src/registrar/models/domain.py | 9 +- src/registrar/templates/domain_dnssec.html | 28 +--- src/registrar/templates/domain_dsdata.html | 32 +--- src/registrar/templates/domain_keydata.html | 110 ------------- src/registrar/templates/domain_sidebar.html | 11 +- src/registrar/tests/common.py | 15 -- src/registrar/tests/test_models_domain.py | 73 -------- src/registrar/tests/test_views.py | 118 +------------ src/registrar/views/__init__.py | 1 - src/registrar/views/domain.py | 174 ++------------------ 14 files changed, 28 insertions(+), 608 deletions(-) delete mode 100644 src/registrar/templates/domain_keydata.html diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index bd2215620..c00d1c589 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -100,11 +100,6 @@ urlpatterns = [ views.DomainDsDataView.as_view(), name="domain-dns-dnssec-dsdata", ), - path( - "domain//dns/dnssec/keydata", - views.DomainKeyDataView.as_view(), - name="domain-dns-dnssec-keydata", - ), path( "domain//your-contact-information", views.DomainYourContactInformationView.as_view(), diff --git a/src/registrar/forms/__init__.py b/src/registrar/forms/__init__.py index 7d2baf646..c3aa89fed 100644 --- a/src/registrar/forms/__init__.py +++ b/src/registrar/forms/__init__.py @@ -8,6 +8,4 @@ from .domain import ( DomainDnssecForm, DomainDsdataFormset, DomainDsdataForm, - DomainKeydataFormset, - DomainKeydataForm, ) diff --git a/src/registrar/forms/common.py b/src/registrar/forms/common.py index 159113488..585d5ed3e 100644 --- a/src/registrar/forms/common.py +++ b/src/registrar/forms/common.py @@ -1,6 +1,6 @@ # common.py # -# ALGORITHM_CHOICES are options for alg attribute in DS Data and Key Data +# ALGORITHM_CHOICES are options for alg attribute in DS Data # reference: # https://www.iana.org/assignments/dns-sec-alg-numbers/dns-sec-alg-numbers.xhtml ALGORITHM_CHOICES = [ @@ -24,15 +24,4 @@ DIGEST_TYPE_CHOICES = [ (0, "(0) Reserved"), (1, "(1) SHA-256"), ] -# PROTOCOL_CHOICES are options for protocol attribute in Key Data -# reference: https://datatracker.ietf.org/doc/html/rfc4034#section-2.1.2 -PROTOCOL_CHOICES = [ - (3, "(3) DNSSEC"), -] -# FLAG_CHOICES are options for flags attribute in Key Data -# reference: https://datatracker.ietf.org/doc/html/rfc4034#section-2.1.1 -FLAG_CHOICES = [ - (0, "(0)"), - (256, "(256) ZSK"), - (257, "(257) KSK"), -] + diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 8abc7e14a..6bbade5ef 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -10,8 +10,6 @@ from ..models import Contact, DomainInformation from .common import ( ALGORITHM_CHOICES, DIGEST_TYPE_CHOICES, - FLAG_CHOICES, - PROTOCOL_CHOICES, ) @@ -188,44 +186,3 @@ DomainDsdataFormset = formset_factory( extra=0, can_delete=True, ) - - -class DomainKeydataForm(forms.Form): - """Form for adding or editing DNSSEC Key Data to a domain.""" - - flag = forms.TypedChoiceField( - required=True, - label="Flag", - coerce=int, - choices=FLAG_CHOICES, - error_messages={"required": ("Flag is required.")}, - ) - - protocol = forms.TypedChoiceField( - required=True, - label="Protocol", - coerce=int, - choices=PROTOCOL_CHOICES, - error_messages={"required": ("Protocol is required.")}, - ) - - algorithm = forms.TypedChoiceField( - required=True, - label="Algorithm", - coerce=int, - choices=[(None, "--Select--")] + ALGORITHM_CHOICES, # type: ignore - error_messages={"required": ("Algorithm is required.")}, - ) - - pub_key = forms.CharField( - required=True, - label="Pub key", - error_messages={"required": ("Pub key is required.")}, - ) - - -DomainKeydataFormset = formset_factory( - DomainKeydataForm, - extra=0, - can_delete=True, -) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index bab993b04..1aad4fef4 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -486,12 +486,11 @@ class Domain(TimeStampedModel, DomainHelper): addExtension: dict remExtension: dict - addExtension includes all dsData or keyData to be added - remExtension includes all dsData or keyData to be removed + addExtension includes all dsData to be added + remExtension includes all dsData to be removed - method operates on dsData OR keyData, never a mix of the two; - operates based on which is present in _dnssecdata; - if neither is present, addExtension will be empty dict, and + method operates on dsData; + if dsData is not present, addExtension will be empty dict, and remExtension will be all existing dnssecdata to be deleted """ diff --git a/src/registrar/templates/domain_dnssec.html b/src/registrar/templates/domain_dnssec.html index 5eedb2184..c4a19470e 100644 --- a/src/registrar/templates/domain_dnssec.html +++ b/src/registrar/templates/domain_dnssec.html @@ -7,14 +7,14 @@

DNSSEC

-

DNSSEC, or DNS Security Extensions, is additional security layer to protect your domain. Enabling DNSSEC ensures that when someone visits your domain, they can be certain that it's connecting to the correct server, preventing potential hijacking or tampering with your domain's records.

+

DNSSEC, or DNS Security Extensions, is additional security layer to protect your website. Enabling DNSSEC ensures that when someone visits your domain, they can be certain that it's connecting to the correct server, preventing potential hijacking or tampering with your domain's records.

{% csrf_token %} {% if has_dnssec_records %}
- In order to fully disable DNSSEC on your domain, you will need to work with your DNS provider to remove your DNSSEC-related records from your zone. + In order to fully disable DNSSEC on your domain, you will need to work with your DNS provider to remove your DNSSEC-related records from your zone.
Disable DNSSEC - {% elif dnssec_enabled %} -
-

Add DS Records

-

In order to enable DNSSEC and add Delegation Signer (DS) records, you must first configure it with your DNS hosting service. Your configuration will determine whether you need to add DS Data or Key Data. Contact your DNS hosting provider if you are unsure which record type to add.

-

- Add DS Data - Add Key Data - -

-
{% else %}
- It is strongly recommended that you only enable DNSSEC if you know how to set it up properly at your hosting service. If you make a mistake, it could cause your domain name to stop working. + It is strongly recommended that you only enable DNSSEC if you know how to set it up properly at your hosting service. If you make a mistake, it could cause your domain name to stop working.
- + Enable DNSSEC
{% endif %}
diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index ca4dce783..ac38bd87f 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -8,41 +8,17 @@ {% include "includes/form_errors.html" with form=form %} {% endfor %} - {% if domain.dnssecdata is None and not dnssec_ds_confirmed %} + {% if domain.dnssecdata is None %}
- You have no DS Data added. Enable DNSSEC by adding DS Data or return to the DNSSEC page and click 'enable.' + You have no DS Data added. Enable DNSSEC by adding DS Data.
{% endif %}

DS Data

- {% if domain.dnssecdata is not None and domain.dnssecdata.keyData is not None %} -
-
-

Warning, you cannot add DS Data

-

- You cannot add DS Data because you have already added Key Data. Delete your Key Data records in order to add DS Data. -

-
-
- {% elif not dnssec_ds_confirmed %} -

In order to enable DNSSEC, you must first configure it with your DNS hosting service.

-

Enter the values given by your DNS provider for DS Data.

-

Required fields are marked with an asterisk (*).

-
- {% csrf_token %} - -
- {% else %} +

In order to enable DNSSEC, you must first configure it with your DNS hosting service.

Enter the values given by your DNS provider for DS Data.

{% include "includes/required_fields.html" %} @@ -119,5 +95,5 @@ >Cancel - {% endif %} + {% endblock %} {# domain_content #} diff --git a/src/registrar/templates/domain_keydata.html b/src/registrar/templates/domain_keydata.html deleted file mode 100644 index 167d86370..000000000 --- a/src/registrar/templates/domain_keydata.html +++ /dev/null @@ -1,110 +0,0 @@ -{% extends "domain_base.html" %} -{% load static field_helpers url_helpers %} - -{% block title %}Key Data | {{ domain.name }} | {% endblock %} - -{% block domain_content %} - {% for form in formset %} - {% include "includes/form_errors.html" with form=form %} - {% endfor %} - -

Key Data

- - {% if domain.dnssecdata is not None and domain.dnssecdata.dsData is not None %} -
-
-

Warning, you cannot add Key Data

-

- You cannot add Key Data because you have already added DS Data. Delete your DS Data records in order to add Key Data. -

-
-
- {% elif not dnssec_key_confirmed %} -

In order to enable DNSSEC and add DS records, you must first configure it with your DNS hosting service. Your configuration will determine whether you need to add DS Data or Key Data. Contact your DNS hosting provider if you are unsure which record type to add.

-
- {% csrf_token %} - -
- {% else %} - -

Enter the values given by your DNS provider for DS Key Data.

- {% include "includes/required_fields.html" %} - -
- {% csrf_token %} - {{ formset.management_form }} - - {% for form in formset %} -
- - DS Data record {{forloop.counter}} - -

DS Data record {{forloop.counter}}

- -
-
- {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} - {% input_with_errors form.flag %} - {% endwith %} -
-
- {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} - {% input_with_errors form.protocol %} - {% endwith %} -
-
- {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} - {% input_with_errors form.algorithm %} - {% endwith %} -
-
- -
-
- {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} - {% input_with_errors form.pub_key %} - {% endwith %} -
-
- -
-
- -
-
- -
- {% endfor %} - - - - -
- -
- -
- {% endif %} -{% endblock %} {# domain_content #} diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 1acd87eeb..fda3b322d 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -34,7 +34,7 @@ > DNSSEC - {% if domain.dnssecdata is not None or request.path|startswith:url and request.path|endswith:'data' %} + {% if domain.dnssecdata is not None or request.path|startswith:url and request.path|endswith:'dsdata' %}
  • {% url 'domain-dns-dnssec-dsdata' pk=domain.id as url %} @@ -44,15 +44,6 @@ DS Data
    • - -
  • - {% url 'domain-dns-dnssec-keydata' pk=domain.id as url %} - - DS Key Data - -
{% endif %} diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 7ae107006..239e50ae8 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -732,12 +732,6 @@ class MockEppLib(TestCase): "digestType": 1, "digest": "ec0bdd990b39feead889f0ba613db4adecb4adec", } - keyDataDict = { - "flags": 257, - "protocol": 3, - "alg": 1, - "pubKey": "AQPJ////4Q==", - } dnssecExtensionWithDsData = extensions.DNSSECExtension( **{ "dsData": [ @@ -753,11 +747,6 @@ class MockEppLib(TestCase): ], # type: ignore } ) - dnssecExtensionWithKeyData = extensions.DNSSECExtension( - **{ - "keyData": [common.DNSSECKeyData(**keyDataDict)], # type: ignore - } - ) dnssecExtensionRemovingDsData = extensions.DNSSECExtension() infoDomainHasIP = fakedEppObject( @@ -851,10 +840,6 @@ class MockEppLib(TestCase): self.mockDataInfoDomain, self.dnssecExtensionWithMultDsData, ), - "dnssec-keydata.gov": ( - self.mockDataInfoDomain, - self.dnssecExtensionWithKeyData, - ), "dnssec-none.gov": (self.mockDataInfoDomain, None), "my-nameserver.gov": ( self.infoDomainTwoHosts diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index ef3084f9c..e612d7b22 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -1798,79 +1798,6 @@ class TestRegistrantDNSSEC(MockEppLib): patcher.stop() - def test_user_adds_dnssec_keydata(self): - """ - Scenario: Registrant adds DNSSEC key data. - Verify that both the setter and getter are functioning properly - - This test verifies: - 1 - setter calls UpdateDomain command - 2 - setter adds the UpdateDNSSECExtension extension to the command - 3 - setter causes the getter to call info domain on next get from cache - 4 - getter properly parses dnssecdata from InfoDomain response and sets to cache - - """ - - # need to use a separate patcher and side_effect for this test, as - # response from InfoDomain must be different for different iterations - # of the same command - def side_effect(_request, cleaned): - if isinstance(_request, commands.InfoDomain): - if mocked_send.call_count == 1: - return MagicMock(res_data=[self.mockDataInfoDomain]) - else: - return MagicMock( - res_data=[self.mockDataInfoDomain], - extensions=[self.dnssecExtensionWithKeyData], - ) - else: - return MagicMock(res_data=[self.mockDataInfoHosts]) - - patcher = patch("registrar.models.domain.registry.send") - mocked_send = patcher.start() - mocked_send.side_effect = side_effect - - domain, _ = Domain.objects.get_or_create(name="dnssec-keydata.gov") - - domain.dnssecdata = self.dnssecExtensionWithKeyData - # get the DNS SEC extension added to the UpdateDomain command - # and verify that it is properly sent - # args[0] is the _request sent to registry - args, _ = mocked_send.call_args - # assert that the extension matches - self.assertEquals( - args[0].extensions[0], - self.createUpdateExtension(self.dnssecExtensionWithKeyData), - ) - # test that the dnssecdata getter is functioning properly - dnssecdata_get = domain.dnssecdata - mocked_send.assert_has_calls( - [ - call( - commands.UpdateDomain( - name="dnssec-keydata.gov", - nsset=None, - keyset=None, - registrant=None, - auth_info=None, - ), - cleaned=True, - ), - call( - commands.InfoDomain( - name="dnssec-keydata.gov", - ), - cleaned=True, - ), - ] - ) - - self.assertEquals( - dnssecdata_get.keyData, self.dnssecExtensionWithKeyData.keyData - ) - - patcher.stop() - def test_update_is_unsuccessful(self): """ Scenario: An update to the dns data is unsuccessful diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 06fddfde7..e8946c52c 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1075,7 +1075,6 @@ class TestWithDomainPermissions(TestWithUser): self.domain_multdsdata, _ = Domain.objects.get_or_create( name="dnssec-multdsdata.gov" ) - self.domain_keydata, _ = Domain.objects.get_or_create(name="dnssec-keydata.gov") # We could simply use domain (igorville) but this will be more readable in tests # that inherit this setUp self.domain_dnssec_none, _ = Domain.objects.get_or_create( @@ -1090,9 +1089,6 @@ class TestWithDomainPermissions(TestWithUser): DomainInformation.objects.get_or_create( creator=self.user, domain=self.domain_multdsdata ) - DomainInformation.objects.get_or_create( - creator=self.user, domain=self.domain_keydata - ) DomainInformation.objects.get_or_create( creator=self.user, domain=self.domain_dnssec_none ) @@ -1107,9 +1103,6 @@ class TestWithDomainPermissions(TestWithUser): domain=self.domain_multdsdata, role=UserDomainRole.Roles.ADMIN, ) - UserDomainRole.objects.get_or_create( - user=self.user, domain=self.domain_keydata, role=UserDomainRole.Roles.ADMIN - ) UserDomainRole.objects.get_or_create( user=self.user, domain=self.domain_dnssec_none, @@ -1561,38 +1554,13 @@ class TestDomainDNSSEC(TestDomainOverview): def test_dnssec_page_refreshes_enable_button(self): """DNSSEC overview page loads when domain has no DNSSEC data - and shows a 'Enable DNSSEC' button. When button is clicked the template - updates. When user navigates away then comes back to the page, the - 'Enable DNSSEC' button is shown again.""" - # home_page = self.app.get("/") - + and shows a 'Enable DNSSEC' button.""" + page = self.client.get( reverse("domain-dns-dnssec", kwargs={"pk": self.domain.id}) ) self.assertContains(page, "Enable DNSSEC") - # Prepare the data for the POST request - post_data = { - "enable_dnssec": "Enable DNSSEC", - } - updated_page = self.client.post( - reverse("domain-dns-dnssec", kwargs={"pk": self.domain.id}), - post_data, - follow=True, - ) - - self.assertEqual(updated_page.status_code, 200) - - self.assertContains(updated_page, "Add DS Data") - self.assertContains(updated_page, "Add Key Data") - - self.app.get("/") - - back_to_page = self.client.get( - reverse("domain-dns-dnssec", kwargs={"pk": self.domain.id}) - ) - self.assertContains(back_to_page, "Enable DNSSEC") - def test_dnssec_page_loads_with_data_in_domain(self): """DNSSEC overview page loads when domain has DNSSEC data and the template contains a button to disable DNSSEC.""" @@ -1637,44 +1605,6 @@ class TestDomainDNSSEC(TestDomainOverview): ) self.assertContains(page, "DS Data record 1") - def test_ds_form_loads_with_key_data(self): - """DNSSEC Add DS Data page loads when there is - domain DNSSEC KEY data and shows an alert""" - - page = self.client.get( - reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_keydata.id}) - ) - self.assertContains(page, "Warning, you cannot add DS Data") - - def test_key_form_loads_with_no_domain_data(self): - """DNSSEC Add Key Data page loads when there is no - domain DNSSEC data and shows a button to Add DS Key record""" - - page = self.client.get( - reverse( - "domain-dns-dnssec-keydata", kwargs={"pk": self.domain_dnssec_none.id} - ) - ) - self.assertContains(page, "Add DS Key record") - - def test_key_form_loads_with_key_data(self): - """DNSSEC Add Key Data page loads when there is - domain DNSSEC Key data and shows the data""" - - page = self.client.get( - reverse("domain-dns-dnssec-keydata", kwargs={"pk": self.domain_keydata.id}) - ) - self.assertContains(page, "DS Data record 1") - - def test_key_form_loads_with_ds_data(self): - """DNSSEC Add Key Data page loads when there is - domain DNSSEC DS data and shows an alert""" - - page = self.client.get( - reverse("domain-dns-dnssec-keydata", kwargs={"pk": self.domain_dsdata.id}) - ) - self.assertContains(page, "Warning, you cannot add Key Data") - def test_ds_data_form_submits(self): """DS Data form submits successfully @@ -1719,50 +1649,6 @@ class TestDomainDNSSEC(TestDomainOverview): # the field. self.assertContains(result, "Key tag is required", count=2, status_code=200) - def test_key_data_form_submits(self): - """Key Data form submits successfully - - Uses self.app WebTest because we need to interact with forms. - """ - add_data_page = self.app.get( - reverse("domain-dns-dnssec-keydata", kwargs={"pk": self.domain_keydata.id}) - ) - session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - with less_console_noise(): # swallow log warning message - result = add_data_page.forms[0].submit() - # form submission was a post, response should be a redirect - self.assertEqual(result.status_code, 302) - self.assertEqual( - result["Location"], - reverse("domain-dns-dnssec-keydata", kwargs={"pk": self.domain_keydata.id}), - ) - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - page = result.follow() - self.assertContains( - page, "The Key Data records for this domain have been updated." - ) - - def test_key_data_form_invalid(self): - """Key Data form errors with invalid data - - Uses self.app WebTest because we need to interact with forms. - """ - add_data_page = self.app.get( - reverse("domain-dns-dnssec-keydata", kwargs={"pk": self.domain_keydata.id}) - ) - session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # first two nameservers are required, so if we empty one out we should - # get a form error - add_data_page.forms[0]["form-0-pub_key"] = "" - with less_console_noise(): # swallow logged warning message - result = add_data_page.forms[0].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, "Pub key is required", count=2, status_code=200) - class TestApplicationStatus(TestWithUser, WebTest): def setUp(self): diff --git a/src/registrar/views/__init__.py b/src/registrar/views/__init__.py index 5fd81df8c..c1400d7c0 100644 --- a/src/registrar/views/__init__.py +++ b/src/registrar/views/__init__.py @@ -7,7 +7,6 @@ from .domain import ( DomainNameserversView, DomainDNSSECView, DomainDsDataView, - DomainKeyDataView, DomainYourContactInformationView, DomainSecurityEmailView, DomainUsersView, diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 36b7a9445..1e0505353 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -33,8 +33,6 @@ from ..forms import ( DomainDnssecForm, DomainDsdataFormset, DomainDsdataForm, - DomainKeydataFormset, - DomainKeydataForm, ) from epplibwrapper import ( @@ -280,12 +278,6 @@ class DomainDNSSECView(DomainPermissionView, FormMixin): errmsg = "Error removing existing DNSSEC record(s)." logger.error(errmsg + ": " + err) messages.error(self.request, errmsg) - request.session["dnssec_ds_confirmed"] = False - request.session["dnssec_key_confirmed"] = False - elif "enable_dnssec" in request.POST: - request.session["dnssec_enabled"] = True - request.session["dnssec_ds_confirmed"] = False - request.session["dnssec_key_confirmed"] = False return self.form_valid(form) @@ -303,24 +295,17 @@ class DomainDsDataView(DomainPermissionView, FormMixin): dnssecdata: extensions.DNSSECExtension = domain.dnssecdata initial_data = [] - if dnssecdata is not None: - if dnssecdata.keyData is not None: - # TODO: Throw an error - # Note: This is moot if we're - # removing key data - pass - - if dnssecdata.dsData is not None: - # Add existing nameservers as initial data - initial_data.extend( - { - "key_tag": record.keyTag, - "algorithm": record.alg, - "digest_type": record.digestType, - "digest": record.digest, - } - for record in dnssecdata.dsData - ) + if dnssecdata is not None and dnssecdata.dsData is not None: + # Add existing nameservers as initial data + initial_data.extend( + { + "key_tag": record.keyTag, + "algorithm": record.alg, + "digest_type": record.digestType, + "digest": record.digest, + } + for record in dnssecdata.dsData + ) # Ensure at least 1 record, filled or empty while len(initial_data) == 0: @@ -338,18 +323,6 @@ class DomainDsDataView(DomainPermissionView, FormMixin): # use "formset" instead of "form" for the key context["formset"] = context.pop("form") - # set the dnssec_ds_confirmed flag in the context for this view - # based either on the existence of DS Data in the domain, - # or on the flag stored in the session - domain = self.get_object() - dnssecdata: extensions.DNSSECExtension = domain.dnssecdata - - if dnssecdata is not None and dnssecdata.dsData is not None: - self.request.session["dnssec_ds_confirmed"] = True - - context["dnssec_ds_confirmed"] = self.request.session.get( - "dnssec_ds_confirmed", False - ) return context def post(self, request, *args, **kwargs): @@ -357,11 +330,6 @@ class DomainDsDataView(DomainPermissionView, FormMixin): self.object = self.get_object() formset = self.get_form() - if "confirm-ds" in request.POST: - request.session["dnssec_ds_confirmed"] = True - request.session["dnssec_key_confirmed"] = False - return super().form_valid(formset) - if "btn-cancel-click" in request.POST: return redirect("/", {"formset": formset}, RequestContext(request)) @@ -411,126 +379,6 @@ class DomainDsDataView(DomainPermissionView, FormMixin): return super().form_valid(formset) -class DomainKeyDataView(DomainPermissionView, FormMixin): - """Domain DNSSEC key data editing view.""" - - template_name = "domain_keydata.html" - form_class = DomainKeydataFormset - form = DomainKeydataForm - - def get_initial(self): - """The initial value for the form (which is a formset here).""" - domain = self.get_object() - dnssecdata: extensions.DNSSECExtension = domain.dnssecdata - initial_data = [] - - if dnssecdata is not None: - if dnssecdata.dsData is not None: - # TODO: Throw an error? - # Note: this is moot if we're - # removing Key data - pass - - if dnssecdata.keyData is not None: - # Add existing keydata as initial data - initial_data.extend( - { - "flag": record.flags, - "protocol": record.protocol, - "algorithm": record.alg, - "pub_key": record.pubKey, - } - for record in dnssecdata.keyData - ) - - # Ensure at least 1 record, filled or empty - while len(initial_data) == 0: - initial_data.append({}) - - return initial_data - - def get_success_url(self): - """Redirect to the Key Data page for the domain.""" - return reverse("domain-dns-dnssec-keydata", kwargs={"pk": self.object.pk}) - - def get_context_data(self, **kwargs): - """Adjust context from FormMixin for formsets.""" - context = super().get_context_data(**kwargs) - # use "formset" instead of "form" for the key - context["formset"] = context.pop("form") - - # set the dnssec_key_confirmed flag in the context for this view - # based either on the existence of Key Data in the domain, - # or on the flag stored in the session - domain = self.get_object() - dnssecdata: extensions.DNSSECExtension = domain.dnssecdata - - if dnssecdata is not None and dnssecdata.keyData is not None: - self.request.session["dnssec_key_confirmed"] = True - - context["dnssec_key_confirmed"] = self.request.session.get( - "dnssec_key_confirmed", False - ) - return context - - def post(self, request, *args, **kwargs): - """Formset submission posts to this view.""" - self.object = self.get_object() - formset = self.get_form() - - if "confirm-key" in request.POST: - request.session["dnssec_key_confirmed"] = True - request.session["dnssec_ds_confirmed"] = False - self.object.save() - return super().form_valid(formset) - - if "btn-cancel-click" in request.POST: - return redirect("/", {"formset": formset}, RequestContext(request)) - - 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.""" - - # Set the nameservers from the formset - dnssecdata = extensions.DNSSECExtension() - - for form in formset: - try: - # if 'delete' not in form.cleaned_data - # or form.cleaned_data['delete'] == False: - keyrecord = { - "flags": int(form.cleaned_data["flag"]), - "protocol": int(form.cleaned_data["protocol"]), - "alg": int(form.cleaned_data["algorithm"]), - "pubKey": form.cleaned_data["pub_key"], - } - if dnssecdata.keyData is None: - dnssecdata.keyData = [] - dnssecdata.keyData.append(common.DNSSECKeyData(**keyrecord)) - except KeyError: - # no server information in this field, skip it - pass - domain = self.get_object() - try: - domain.dnssecdata = dnssecdata - except RegistryError as err: - errmsg = "Error updating DNSSEC data in the registry." - logger.error(errmsg) - logger.error(err) - messages.error(self.request, errmsg) - return self.form_invalid(formset) - else: - messages.success( - self.request, "The Key Data records for this domain have been updated." - ) - # superclass has the redirect - return super().form_valid(formset) - - class DomainYourContactInformationView(DomainPermissionView, FormMixin): """Domain your contact information editing view.""" From b0262f6d34fcce4428d642a2819bcf0c51c7bb62 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 17 Oct 2023 17:23:47 -0400 Subject: [PATCH 02/26] removed keyData from domain model for dnssec --- src/registrar/models/domain.py | 31 +------------------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 1aad4fef4..aea3deecd 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -523,32 +523,9 @@ class Domain(TimeStampedModel, DomainHelper): else: addDnssecdata["dsData"] = None - elif _dnssecdata and _dnssecdata.keyData is not None: - # initialize addDnssecdata and remDnssecdata for keyData - addDnssecdata["keyData"] = _dnssecdata.keyData - - if oldDnssecdata and len(oldDnssecdata.keyData) > 0: - # if existing keyData not in new keyData, mark for removal - keyDataForRemoval = [ - keyData - for keyData in oldDnssecdata.keyData - if keyData not in _dnssecdata.keyData - ] - if len(keyDataForRemoval) > 0: - remDnssecdata["keyData"] = keyDataForRemoval - - # if new keyData not in existing keyData, mark for add - keyDataForAdd = [ - keyData - for keyData in _dnssecdata.keyData - if keyData not in oldDnssecdata.keyData - ] - if len(keyDataForAdd) > 0: - addDnssecdata["keyData"] = keyDataForAdd else: - # there are no new dsData or keyData, remove all + # there are no new dsData, remove all dsData from existing remDnssecdata["dsData"] = getattr(oldDnssecdata, "dsData", None) - remDnssecdata["keyData"] = getattr(oldDnssecdata, "keyData", None) return addDnssecdata, remDnssecdata @@ -558,12 +535,10 @@ class Domain(TimeStampedModel, DomainHelper): addParams = { "maxSigLife": _addDnssecdata.get("maxSigLife", None), "dsData": _addDnssecdata.get("dsData", None), - "keyData": _addDnssecdata.get("keyData", None), } remParams = { "maxSigLife": _remDnssecdata.get("maxSigLife", None), "remDsData": _remDnssecdata.get("dsData", None), - "remKeyData": _remDnssecdata.get("keyData", None), } addRequest = commands.UpdateDomain(name=self.name) addExtension = commands.UpdateDomainDNSSECExtension(**addParams) @@ -575,15 +550,11 @@ class Domain(TimeStampedModel, DomainHelper): if ( "dsData" in _addDnssecdata and _addDnssecdata["dsData"] is not None - or "keyData" in _addDnssecdata - and _addDnssecdata["keyData"] is not None ): registry.send(addRequest, cleaned=True) if ( "dsData" in _remDnssecdata and _remDnssecdata["dsData"] is not None - or "keyData" in _remDnssecdata - and _remDnssecdata["keyData"] is not None ): registry.send(remRequest, cleaned=True) except RegistryError as e: From 6ef6044459c7369ea40dfbdf43b8bc78db6ce01c Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 17 Oct 2023 17:31:05 -0400 Subject: [PATCH 03/26] lint --- src/registrar/forms/common.py | 1 - src/registrar/models/domain.py | 10 ++-------- src/registrar/tests/test_views.py | 2 +- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/registrar/forms/common.py b/src/registrar/forms/common.py index 585d5ed3e..a27545a84 100644 --- a/src/registrar/forms/common.py +++ b/src/registrar/forms/common.py @@ -24,4 +24,3 @@ DIGEST_TYPE_CHOICES = [ (0, "(0) Reserved"), (1, "(1) SHA-256"), ] - diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index aea3deecd..6d911c975 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -547,15 +547,9 @@ class Domain(TimeStampedModel, DomainHelper): remExtension = commands.UpdateDomainDNSSECExtension(**remParams) remRequest.add_extension(remExtension) try: - if ( - "dsData" in _addDnssecdata - and _addDnssecdata["dsData"] is not None - ): + if "dsData" in _addDnssecdata and _addDnssecdata["dsData"] is not None: registry.send(addRequest, cleaned=True) - if ( - "dsData" in _remDnssecdata - and _remDnssecdata["dsData"] is not None - ): + if "dsData" in _remDnssecdata and _remDnssecdata["dsData"] is not None: registry.send(remRequest, cleaned=True) except RegistryError as e: logger.error( diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index e8946c52c..2a1cd8fed 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1555,7 +1555,7 @@ class TestDomainDNSSEC(TestDomainOverview): def test_dnssec_page_refreshes_enable_button(self): """DNSSEC overview page loads when domain has no DNSSEC data and shows a 'Enable DNSSEC' button.""" - + page = self.client.get( reverse("domain-dns-dnssec", kwargs={"pk": self.domain.id}) ) From 1ea318bdbcc01b2a41b18666474bb53475f40c8b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 17 Oct 2023 17:47:34 -0400 Subject: [PATCH 04/26] swap out the order of the alerts so that the form error alerts are below the informational one --- src/registrar/templates/domain_dsdata.html | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index ac38bd87f..5aa1a9969 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -4,10 +4,6 @@ {% block title %}DS Data | {{ domain.name }} | {% endblock %} {% block domain_content %} - {% for form in formset %} - {% include "includes/form_errors.html" with form=form %} - {% endfor %} - {% if domain.dnssecdata is None %}
@@ -16,6 +12,10 @@
{% endif %} + {% for form in formset %} + {% include "includes/form_errors.html" with form=form %} + {% endfor %} +

DS Data

In order to enable DNSSEC, you must first configure it with your DNS hosting service.

From 5dd76bfeefb8a8945b5d9f2703f80039ff6496bb Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 18 Oct 2023 11:06:06 -0600 Subject: [PATCH 05/26] Add logic for transition domains in user-login handler Signed-off-by: CocoByte --- src/registrar/models/user.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index acf59cb68..5d73866a4 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -4,6 +4,8 @@ from django.contrib.auth.models import AbstractUser from django.db import models from .domain_invitation import DomainInvitation +from registrar.models import TransitionDomain +from registrar.models import DomainInformation from phonenumber_field.modelfields import PhoneNumberField # type: ignore @@ -81,6 +83,22 @@ class User(AbstractUser): logger.warn( "Failed to retrieve invitation %s", invitation, exc_info=True ) + + transition_domain_exists = TransitionDomain.objects.filter( + username=self.email + ).exists() + if transition_domain_exists: + # Looks like the user logged in with the same e-mail as + # a corresponding transition domain. Create a Domain + # Information object. + # TODO: Do we need to check for existing Domain Info objects? + # TODO: Should we add a Domain to the DomainInfo object? + # NOTE that adding a user role for this user + # as admin for this domain is already done + # in the incitation.retrieve() method. So + # we don't need to do that here. + new_domain_info = DomainInformation(creator=self) + new_domain_info.save() class Meta: permissions = [ From 44b3e78aed1ee0743a4630ad44ca1a94bebb5b21 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 18 Oct 2023 13:28:49 -0600 Subject: [PATCH 06/26] updated user login logic for one-to-many transition domains relationship, and added some error handling --- src/registrar/models/user.py | 89 ++++++++++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 20 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 5d73866a4..afc16bb68 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -6,6 +6,7 @@ from django.db import models from .domain_invitation import DomainInvitation from registrar.models import TransitionDomain from registrar.models import DomainInformation +from registrar.models import Domain from phonenumber_field.modelfields import PhoneNumberField # type: ignore @@ -64,12 +65,9 @@ class User(AbstractUser): def is_restricted(self): return self.status == self.RESTRICTED - def first_login(self): - """Callback when the user is authenticated for the very first time. - - When a user first arrives on the site, we need to retrieve any domain - invitations that match their email address. - """ + def check_domain_invitations_on_login(self): + """When a user first arrives on the site, we need to retrieve any domain + invitations that match their email address.""" for invitation in DomainInvitation.objects.filter( email=self.email, status=DomainInvitation.INVITED ): @@ -83,22 +81,73 @@ class User(AbstractUser): logger.warn( "Failed to retrieve invitation %s", invitation, exc_info=True ) - - transition_domain_exists = TransitionDomain.objects.filter( + + def check_transition_domains_on_login(self): + """When a user first arrives on the site, we need to check + if they are logging in with the same e-mail as a + transition domain and update our database accordingly.""" + + for transition_domain in TransitionDomain.objects.filter( username=self.email - ).exists() - if transition_domain_exists: + ): # Looks like the user logged in with the same e-mail as - # a corresponding transition domain. Create a Domain - # Information object. - # TODO: Do we need to check for existing Domain Info objects? - # TODO: Should we add a Domain to the DomainInfo object? - # NOTE that adding a user role for this user - # as admin for this domain is already done - # in the incitation.retrieve() method. So - # we don't need to do that here. - new_domain_info = DomainInformation(creator=self) - new_domain_info.save() + # one or more corresponding transition domains. + # Create corresponding DomainInformation objects. + + # NOTE: adding an ADMIN user role for this user + # for each domain should already be done + # in the invitation.retrieve() method. + # However, if the migration scripts for transition + # domain objects were not executed correctly, + # there could be transition domains without + # any corresponding Domain & DomainInvitation objects, + # which means the invitation.retrieve() method might + # not execute. + # Check that there is a corresponding domain object + # for this transition domain. If not, we have an error + # with our data and migrations need to be run again. + # TODO: how should we handle this? + + # Get the domain that corresponds with this transition domain + domain_exists = Domain.objects.filter(name=transition_domain.name).exists() + if not domain_exists: + logger.warn("""There are transition domains without + corresponding domain objects! + Please run migration scripts for transition domains + (See data_migration.md)""") + # TODO: throw exception?? + break + domain = Domain.objects.get(name=transition_domain.name) + + # Create a domain information object, if one doesn't + # already exist + domain_info_exists = DomainInformation.objects.filter( + creator=self, + domain=domain + ).exists() + if not domain_info_exists: + # TODO: Should we add a Domain to the DomainInfo object? + new_domain_info = DomainInformation( + creator=self, + domain=domain) + new_domain_info.save() + + def first_login(self): + """Callback when the user is authenticated for the very first time. + + When a user first arrives on the site, we need to retrieve any domain + invitations that match their email address. + + We also need to check if they are logging in with the same e-mail + as a transition domain and update our database accordingly. + """ + + # PART 1: DOMAIN INVITATIONS + self.check_domain_invitations_on_login() + + # PART 2: TRANSITION DOMAINS + self.check_transition_domains_on_login() + class Meta: permissions = [ From 7998508881509982e3c00d2562b814c34ca7b6d9 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 19 Oct 2023 17:16:09 -0600 Subject: [PATCH 07/26] Added unit tests Signed-off-by: CocoByte --- src/registrar/models/user.py | 56 ++++++++++++++++++++++-------- src/registrar/tests/test_models.py | 46 +++++++++++++++++++++++- 2 files changed, 86 insertions(+), 16 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index afc16bb68..378b6b24e 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -4,9 +4,9 @@ from django.contrib.auth.models import AbstractUser from django.db import models from .domain_invitation import DomainInvitation -from registrar.models import TransitionDomain -from registrar.models import DomainInformation -from registrar.models import Domain +from .transition_domain import TransitionDomain +from .domain_information import DomainInformation +from .domain import Domain from phonenumber_field.modelfields import PhoneNumberField # type: ignore @@ -82,6 +82,26 @@ class User(AbstractUser): "Failed to retrieve invitation %s", invitation, exc_info=True ) + def create_domain_and_invite(self, transition_domain: TransitionDomain): + print("creating DOMAIN") + new_domain = Domain( + name=transition_domain.domain_name, state=transition_domain.status + ) + new_domain.save() + # check that a domain invitation doesn't already + # exist for this e-mail / Domain pair + domain_email_already_in_domain_invites = DomainInvitation.objects.filter( + email=transition_domain.username.lower(), domain=new_domain + ).exists() + if not domain_email_already_in_domain_invites: + + print("creating INVITATION") + # Create new domain invitation + new_domain_invitation = DomainInvitation( + email=transition_domain.username.lower(), domain=new_domain + ) + new_domain_invitation.save() + def check_transition_domains_on_login(self): """When a user first arrives on the site, we need to check if they are logging in with the same e-mail as a @@ -106,27 +126,26 @@ class User(AbstractUser): # Check that there is a corresponding domain object # for this transition domain. If not, we have an error # with our data and migrations need to be run again. - # TODO: how should we handle this? # Get the domain that corresponds with this transition domain - domain_exists = Domain.objects.filter(name=transition_domain.name).exists() + domain_exists = Domain.objects.filter(name=transition_domain.domain_name).exists() if not domain_exists: logger.warn("""There are transition domains without corresponding domain objects! Please run migration scripts for transition domains (See data_migration.md)""") - # TODO: throw exception?? - break - domain = Domain.objects.get(name=transition_domain.name) + # No need to throw an exception...just create a domain + # and domain invite, then proceed as normal + self.create_domain_and_invite(transition_domain) + + domain = Domain.objects.get(name=transition_domain.domain_name) # Create a domain information object, if one doesn't # already exist domain_info_exists = DomainInformation.objects.filter( - creator=self, domain=domain ).exists() if not domain_info_exists: - # TODO: Should we add a Domain to the DomainInfo object? new_domain_info = DomainInformation( creator=self, domain=domain) @@ -139,14 +158,21 @@ class User(AbstractUser): invitations that match their email address. We also need to check if they are logging in with the same e-mail - as a transition domain and update our database accordingly. + as a transition domain and update our domainInfo objects accordingly. """ - # PART 1: DOMAIN INVITATIONS - self.check_domain_invitations_on_login() - - # PART 2: TRANSITION DOMAINS + # PART 1: TRANSITION DOMAINS + # + # NOTE: THIS MUST RUN FIRST + # (If we have an issue where transition domains were + # not fully converted into Domain and DomainInvitation + # objects, this method will fill in the gaps. + # This will ensure the Domain Invitations method + # runs correctly (no missing invites)) self.check_transition_domains_on_login() + + # PART 2: DOMAIN INVITATIONS + self.check_domain_invitations_on_login() class Meta: diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 2c6f78ef5..ffbd0fc55 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -14,7 +14,8 @@ from registrar.models import ( UserDomainRole, ) -import boto3_mocking # type: ignore +import boto3_mocking +from registrar.models.transition_domain import TransitionDomain # type: ignore from .common import MockSESClient, less_console_noise, completed_application from django_fsm import TransitionNotAllowed @@ -612,3 +613,46 @@ class TestInvitations(TestCase): """A new user's first_login callback retrieves their invitations.""" self.user.first_login() self.assertTrue(UserDomainRole.objects.get(user=self.user, domain=self.domain)) + +class TestUser(TestCase): + """For now, just test actions that + occur on user login.""" + + def setUp(self): + self.email = "mayor@igorville.gov" + self.domain_name = "igorvilleInTransition.gov" + self.user, _ = User.objects.get_or_create(email=self.email) + + # clean out the roles each time + UserDomainRole.objects.all().delete() + + TransitionDomain.objects.get_or_create(username="mayor@igorville.gov", + domain_name=self.domain_name) + + def tearDown(self): + super().tearDown() + Domain.objects.all().delete() + DomainInvitation.objects.all().delete() + DomainInformation.objects.all().delete() + TransitionDomain.objects.all().delete() + User.objects.all().delete() + + def test_check_transition_domains_on_login(self): + """A new user's first_login callback checks transition domains. + Makes DomainInformation object.""" + self.domain, _ = Domain.objects.get_or_create(name=self.domain_name) + + self.user.first_login() + self.assertTrue(DomainInformation.objects.get(domain=self.domain)) + + def test_check_transition_domains_without_domains_on_login(self): + """A new user's first_login callback checks transition domains. + This test makes sure that in the event a domain does not exist + for a given transition domain, both a domain and domain invitation + are created.""" + self.user.first_login() + self.assertTrue(Domain.objects.get(name=self.domain_name)) + + domain = Domain.objects.get(name=self.domain_name) + self.assertTrue(DomainInvitation.objects.get(email=self.email, domain=domain)) + self.assertTrue(DomainInformation.objects.get(domain=domain)) \ No newline at end of file From bc14129f953720ee90c0ada655a60c2c6ca1e10a Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 19 Oct 2023 22:45:16 -0600 Subject: [PATCH 08/26] mostly linted Signed-off-by: CocoByte --- src/registrar/models/user.py | 44 ++++++++++++++---------------- src/registrar/tests/test_models.py | 14 ++++++---- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 378b6b24e..e2fff98b8 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -83,22 +83,21 @@ class User(AbstractUser): ) def create_domain_and_invite(self, transition_domain: TransitionDomain): - print("creating DOMAIN") - new_domain = Domain( - name=transition_domain.domain_name, state=transition_domain.status - ) + transition_domain_name = transition_domain.domain_name + transition_domain_status = transition_domain.status + transition_domain_email = transition_domain.username + + new_domain = Domain(name=transition_domain_name, state=transition_domain_status) new_domain.save() # check that a domain invitation doesn't already # exist for this e-mail / Domain pair domain_email_already_in_domain_invites = DomainInvitation.objects.filter( - email=transition_domain.username.lower(), domain=new_domain + email=transition_domain_email.lower(), domain=new_domain ).exists() if not domain_email_already_in_domain_invites: - - print("creating INVITATION") # Create new domain invitation new_domain_invitation = DomainInvitation( - email=transition_domain.username.lower(), domain=new_domain + email=transition_domain_email.lower(), domain=new_domain ) new_domain_invitation.save() @@ -107,18 +106,16 @@ class User(AbstractUser): if they are logging in with the same e-mail as a transition domain and update our database accordingly.""" - for transition_domain in TransitionDomain.objects.filter( - username=self.email - ): + for transition_domain in TransitionDomain.objects.filter(username=self.email): # Looks like the user logged in with the same e-mail as - # one or more corresponding transition domains. + # one or more corresponding transition domains. # Create corresponding DomainInformation objects. # NOTE: adding an ADMIN user role for this user # for each domain should already be done - # in the invitation.retrieve() method. + # in the invitation.retrieve() method. # However, if the migration scripts for transition - # domain objects were not executed correctly, + # domain objects were not executed correctly, # there could be transition domains without # any corresponding Domain & DomainInvitation objects, # which means the invitation.retrieve() method might @@ -128,12 +125,16 @@ class User(AbstractUser): # with our data and migrations need to be run again. # Get the domain that corresponds with this transition domain - domain_exists = Domain.objects.filter(name=transition_domain.domain_name).exists() + domain_exists = Domain.objects.filter( + name=transition_domain.domain_name + ).exists() if not domain_exists: - logger.warn("""There are transition domains without - corresponding domain objects! + logger.warn( + """There are transition domains without + corresponding domain objects! Please run migration scripts for transition domains - (See data_migration.md)""") + (See data_migration.md)""" + ) # No need to throw an exception...just create a domain # and domain invite, then proceed as normal self.create_domain_and_invite(transition_domain) @@ -146,9 +147,7 @@ class User(AbstractUser): domain=domain ).exists() if not domain_info_exists: - new_domain_info = DomainInformation( - creator=self, - domain=domain) + new_domain_info = DomainInformation(creator=self, domain=domain) new_domain_info.save() def first_login(self): @@ -162,7 +161,7 @@ class User(AbstractUser): """ # PART 1: TRANSITION DOMAINS - # + # # NOTE: THIS MUST RUN FIRST # (If we have an issue where transition domains were # not fully converted into Domain and DomainInvitation @@ -173,7 +172,6 @@ class User(AbstractUser): # PART 2: DOMAIN INVITATIONS self.check_domain_invitations_on_login() - class Meta: permissions = [ diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index ffbd0fc55..f1a1724c5 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -614,8 +614,9 @@ class TestInvitations(TestCase): self.user.first_login() self.assertTrue(UserDomainRole.objects.get(user=self.user, domain=self.domain)) + class TestUser(TestCase): - """For now, just test actions that + """For now, just test actions that occur on user login.""" def setUp(self): @@ -626,8 +627,9 @@ class TestUser(TestCase): # clean out the roles each time UserDomainRole.objects.all().delete() - TransitionDomain.objects.get_or_create(username="mayor@igorville.gov", - domain_name=self.domain_name) + TransitionDomain.objects.get_or_create( + username="mayor@igorville.gov", domain_name=self.domain_name + ) def tearDown(self): super().tearDown() @@ -644,10 +646,10 @@ class TestUser(TestCase): self.user.first_login() self.assertTrue(DomainInformation.objects.get(domain=self.domain)) - + def test_check_transition_domains_without_domains_on_login(self): """A new user's first_login callback checks transition domains. - This test makes sure that in the event a domain does not exist + This test makes sure that in the event a domain does not exist for a given transition domain, both a domain and domain invitation are created.""" self.user.first_login() @@ -655,4 +657,4 @@ class TestUser(TestCase): domain = Domain.objects.get(name=self.domain_name) self.assertTrue(DomainInvitation.objects.get(email=self.email, domain=domain)) - self.assertTrue(DomainInformation.objects.get(domain=domain)) \ No newline at end of file + self.assertTrue(DomainInformation.objects.get(domain=domain)) From 1a3c095e0bcfbcc6f4ec588ea08113bed535d994 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 19 Oct 2023 23:41:19 -0600 Subject: [PATCH 09/26] linted Signed-off-by: CocoByte --- src/registrar/models/user.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index e2fff98b8..7207a5a61 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -87,19 +87,23 @@ class User(AbstractUser): transition_domain_status = transition_domain.status transition_domain_email = transition_domain.username - new_domain = Domain(name=transition_domain_name, state=transition_domain_status) - new_domain.save() - # check that a domain invitation doesn't already - # exist for this e-mail / Domain pair - domain_email_already_in_domain_invites = DomainInvitation.objects.filter( - email=transition_domain_email.lower(), domain=new_domain - ).exists() - if not domain_email_already_in_domain_invites: - # Create new domain invitation - new_domain_invitation = DomainInvitation( - email=transition_domain_email.lower(), domain=new_domain + # type safety check. name should never be none + if transition_domain_name is not None: + new_domain = Domain( + name=transition_domain_name, state=transition_domain_status ) - new_domain_invitation.save() + new_domain.save() + # check that a domain invitation doesn't already + # exist for this e-mail / Domain pair + domain_email_already_in_domain_invites = DomainInvitation.objects.filter( + email=transition_domain_email.lower(), domain=new_domain + ).exists() + if not domain_email_already_in_domain_invites: + # Create new domain invitation + new_domain_invitation = DomainInvitation( + email=transition_domain_email.lower(), domain=new_domain + ) + new_domain_invitation.save() def check_transition_domains_on_login(self): """When a user first arrives on the site, we need to check From 95735598dc0dd60305c5a10074c3882b008780e9 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 20 Oct 2023 16:32:41 -0400 Subject: [PATCH 10/26] template revision, modal revision on dnssec, added modal on delete all on ds form --- src/registrar/assets/js/get-gov.js | 28 ++++ src/registrar/assets/sass/_theme/_forms.scss | 4 + src/registrar/templates/domain_dnssec.html | 25 ++- src/registrar/templates/domain_dsdata.html | 160 ++++++++++++------- src/registrar/templates/includes/modal.html | 71 ++++++-- src/registrar/views/domain.py | 39 ++++- 6 files changed, 240 insertions(+), 87 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index c21060382..1813deea0 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -383,3 +383,31 @@ function prepareDeleteButtons() { } })(); + +/** + * An IIFE that triggers a modal on the DS Data Form under certain conditions + * + */ +(function triggerModalOnDsDataForm() { + let saveButon = document.querySelector("#save-ds-data"); + + // if (saveButon) + // saveButon.addEventListener('click', triggerModalIfModalTriggerExists); + + // function triggerModalIfModalTriggerExists(e){ + if (saveButon) { + let i = 0; + var tryToTriggerModal = setInterval(function() { + i++; + if (i > 100) { + clearInterval(tryToTriggerModal); + } + let modalTrigger = document.querySelector("#ds-toggle-dnssec-alert"); + if (modalTrigger) { + modalTrigger.click() + clearInterval(tryToTriggerModal); + } + }, 50); + + } +})(); diff --git a/src/registrar/assets/sass/_theme/_forms.scss b/src/registrar/assets/sass/_theme/_forms.scss index ed118bb94..38b42c3d0 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-top-1 { + margin-top: units(1); +} + .usa-form--extra-large { max-width: none; } diff --git a/src/registrar/templates/domain_dnssec.html b/src/registrar/templates/domain_dnssec.html index c4a19470e..a851400ae 100644 --- a/src/registrar/templates/domain_dnssec.html +++ b/src/registrar/templates/domain_dnssec.html @@ -7,19 +7,32 @@

DNSSEC

-

DNSSEC, or DNS Security Extensions, is additional security layer to protect your website. Enabling DNSSEC ensures that when someone visits your domain, they can be certain that it's connecting to the correct server, preventing potential hijacking or tampering with your domain's records.

+

DNSSEC, or DNS Security Extensions, is additional security layer to protect your website. Enabling DNSSEC ensures that when someone visits your website, they can be certain that it’s connecting to the correct server, preventing potential hijacking or tampering with your domain's records.

{% csrf_token %} {% if has_dnssec_records %} -
-
- In order to fully disable DNSSEC on your domain, you will need to work with your DNS provider to remove your DNSSEC-related records from your zone. +
+
+

+

A note on disabling DNSSEC

+
    +
  • After clicking disable DNSSEC here, you will need to wait to disable DNSSEC at your DNS hosting provider after the expiration of the TTL (“time to live”) on your host’s records, which is usually between 1 hour and a day
    • +
  • If you disable DNSSEC at your DNS hosting provider before the expiration of the TTL, you may cause your domain to appear offline
    • +
  • Ask your DNS host for more information on their TTL expiry time
    • +
+

DNSSEC is enabled on your domain

Disable DNSSEC - {% include 'includes/modal.html' with modal_heading="Are you sure you want to continue?" modal_description="Your DNSSEC records will be deleted from the registry." modal_button=modal_button|safe %} + {% include 'includes/modal.html' with modal_heading="Are you sure you want to continue?" modal_button=modal_button|safe %}
{% endblock %} {# domain_content #} diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index 5aa1a9969..b387d34b6 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -21,79 +21,117 @@

In order to enable DNSSEC, you must first configure it with your DNS hosting service.

Enter the values given by your DNS provider for DS Data.

- {% include "includes/required_fields.html" %} - - {% csrf_token %} - {{ formset.management_form }} + {% if not dnssec_ds_confirmed %} +

Required fields are marked with an asterisk (*).

+ + {% csrf_token %} + + + {% else %} + {% include "includes/required_fields.html" %} - {% for form in formset %} -
+
+ {% csrf_token %} + {{ formset.management_form }} - DS Data record {{forloop.counter}} + {% for form in formset %} +
-

DS Data record {{forloop.counter}}

+ DS Data record {{forloop.counter}} -
-
- {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} - {% input_with_errors form.key_tag %} - {% endwith %} +

DS Data record {{forloop.counter}}

+ +
+
+ {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} + {% input_with_errors form.key_tag %} + {% endwith %} +
+
+ {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} + {% input_with_errors form.algorithm %} + {% endwith %} +
+
+ {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} + {% input_with_errors form.digest_type %} + {% endwith %} +
-
- {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} - {% input_with_errors form.algorithm %} - {% endwith %} + +
+
+ {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} + {% input_with_errors form.digest %} + {% endwith %} +
-
- {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} - {% input_with_errors form.digest_type %} - {% endwith %} + +
+
+ +
-
-
-
- {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} - {% input_with_errors form.digest %} - {% endwith %} -
-
+
+ {% endfor %} -
-
- -
-
+ -
- {% endfor %} + + - +
+ +
+ {% endif %} - - - -
- -
+ {% if trigger_modal %} + + {% endif %} + {# Use data-force-action to take esc out of the equation and pass cancel_button_resets_ds_form to effectuate a reset in the view #} +
+ {% include 'includes/modal.html' with cancel_button_resets_ds_form=True modal_heading="Are you sure you want to continue?" modal_description="some good text" modal_button=modal_button|safe %} +
{% endblock %} {# domain_content #} diff --git a/src/registrar/templates/includes/modal.html b/src/registrar/templates/includes/modal.html index 996759576..1e4bc78dc 100644 --- a/src/registrar/templates/includes/modal.html +++ b/src/registrar/templates/includes/modal.html @@ -1,3 +1,5 @@ +{% load static form_helpers url_helpers %} +

@@ -18,25 +20,60 @@
  • - + {% comment %} The cancel button the DS form actually triggers a context change in the view, + in addition to being a close modal hook {% endcomment %} + {% if cancel_button_resets_ds_form %} +
    + {% csrf_token %} + +
    + {% else %} + + {% endif %}

    • - + {% comment %} The cancel button the DS form actually triggers a context change in the view, + in addition to being a close modal hook {% endcomment %} + {% if cancel_button_resets_ds_form %} +
    + {% csrf_token %} + +
    + {% else %} + + {% endif %}
    diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1e0505353..275079ffb 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -14,6 +14,7 @@ from django.shortcuts import redirect from django.template import RequestContext from django.urls import reverse from django.views.generic.edit import FormMixin +from django.shortcuts import render from registrar.models import ( Domain, @@ -252,7 +253,7 @@ class DomainDNSSECView(DomainPermissionView, FormMixin): # Create HTML for the modal button modal_button = ( '' ) @@ -322,6 +323,19 @@ class DomainDsDataView(DomainPermissionView, FormMixin): context = super().get_context_data(**kwargs) # use "formset" instead of "form" for the key context["formset"] = context.pop("form") + + # Create HTML for the modal button + modal_button = ( + '' + ) + + # context to back out of a broken form on all fields delete + context["modal_button"] = modal_button + context["dnssec_ds_confirmed"] = self.request.session.get( + "dnssec_ds_confirmed", False + ) return context @@ -329,16 +343,35 @@ class DomainDsDataView(DomainPermissionView, FormMixin): """Formset submission posts to this view.""" self.object = self.get_object() formset = self.get_form() + override = False + # switch to form in template + if "confirm-ds" in request.POST: + request.session["dnssec_ds_confirmed"] = True + return super().form_valid(formset) + + # This is called but the form cancel button, but also by the modal's X and cacel button if "btn-cancel-click" in request.POST: - return redirect("/", {"formset": formset}, RequestContext(request)) + return render(request, 'domain_dsdata.html', {'formset': formset}) + + if "disable-override-click" in request.POST: + override = True + # we are deleteing all data, + # switch out of form + request.session["dnssec_ds_confirmed"] = False + + if len(formset) == 0 and formset.initial != [{}] and override == False: + # trigger the modal + context = self.get_context_data(**kwargs) + context["trigger_modal"] = True + return self.render_to_response(context) if formset.is_valid(): return self.form_valid(formset) else: return self.form_invalid(formset) - def form_valid(self, formset): + def form_valid(self, formset, **kwargs): """The formset is valid, perform something with it.""" # Set the dnssecdata from the formset From ffbae457bf28e4a6cd676f4f7d3ddb2e571d96e6 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 20 Oct 2023 16:41:47 -0400 Subject: [PATCH 11/26] revise disable dnssec copy on dnssec page --- src/registrar/templates/domain_dnssec.html | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/registrar/templates/domain_dnssec.html b/src/registrar/templates/domain_dnssec.html index a851400ae..8e19e8360 100644 --- a/src/registrar/templates/domain_dnssec.html +++ b/src/registrar/templates/domain_dnssec.html @@ -7,7 +7,7 @@

    DNSSEC

    -

    DNSSEC, or DNS Security Extensions, is additional security layer to protect your website. Enabling DNSSEC ensures that when someone visits your website, they can be certain that it’s connecting to the correct server, preventing potential hijacking or tampering with your domain's records.

    +

    DNSSEC, or DNS Security Extensions, is additional security layer to protect your website. Enabling DNSSEC ensures that when someone visits your website, they can be certain that it’s connecting to the correct server, preventing potential hijacking or tampering with your domain's records.

    {% csrf_token %} @@ -21,12 +21,13 @@

    -

    A note on disabling DNSSEC

    +

    To fully disable DNSSEC

      -
    • After clicking disable DNSSEC here, you will need to wait to disable DNSSEC at your DNS hosting provider after the expiration of the TTL (“time to live”) on your host’s records, which is usually between 1 hour and a day
      • -
    • If you disable DNSSEC at your DNS hosting provider before the expiration of the TTL, you may cause your domain to appear offline
      • -
    • Ask your DNS host for more information on their TTL expiry time
      • +
    • Click “Disable DNSSEC” below.
      • +
    • Wait until the Time to Live (TTL) expires on your DNSSEC records managed by your DNS hosting provider. This is often less than 24 hours, but confirm with your provider.
      • +
    • After the TTL expiration, disable DNSSEC at your DNS hosting provider.
    +

    Warning: If you disable DNSSEC at your DNS hosting provider before TTL expiration, this may cause your domain to appear offline.

    DNSSEC is enabled on your domain

    From 0d12c17aff932f80ad04c7a4541d7e18074c4a45 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 20 Oct 2023 17:02:21 -0400 Subject: [PATCH 12/26] Add Vicky to fixtures --- src/registrar/fixtures_users.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/registrar/fixtures_users.py b/src/registrar/fixtures_users.py index dfe51785b..f03fb025d 100644 --- a/src/registrar/fixtures_users.py +++ b/src/registrar/fixtures_users.py @@ -86,6 +86,12 @@ class UserFixture: "first_name": "Kristina", "last_name": "Yin", }, + { + "username": "ac49d7c1-368a-4e6b-8f1d-60250e20a16f", + "first_name": "Vicky", + "last_name": "Chin", + "email": "szu.chin@associates.cisa.dhs.gov", + }, ] STAFF = [ @@ -150,6 +156,12 @@ class UserFixture: "last_name": "Yin-Analyst", "email": "kristina.yin+1@gsa.gov", }, + { + "username": "8f42302e-b83a-4c9e-8764-fc19e2cea576", + "first_name": "Vickster-Analyst", + "last_name": "Chin-Analyst", + "email": "szu.chin@ecstech.com", + }, ] def load_users(cls, users, group_name): From 8bf2432b15c3e7a960077ed1869ddcecb21e7fce Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 23 Oct 2023 16:43:45 -0400 Subject: [PATCH 13/26] remove the dnssec confirmed session --- src/registrar/templates/domain_dsdata.html | 6 +++--- src/registrar/views/domain.py | 17 +++-------------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index b387d34b6..45b72a9a2 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -22,7 +22,7 @@

    Enter the values given by your DNS provider for DS Data.

    - {% if not dnssec_ds_confirmed %} + {% comment %} {% if not dnssec_ds_confirmed %}

    Required fields are marked with an asterisk (Add new record - {% else %} + {% else %} {% endcomment %} {% include "includes/required_fields.html" %}

    @@ -111,7 +111,7 @@ >Cancel
    - {% endif %} + {% comment %} {% endif %} {% endcomment %} {% if trigger_modal %} Date: Mon, 23 Oct 2023 18:15:48 -0400 Subject: [PATCH 14/26] working wip --- src/registrar/templates/domain_dsdata.html | 6 ++-- src/registrar/templates/includes/modal.html | 2 +- src/registrar/views/domain.py | 35 ++++++++++++++++++--- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index 45b72a9a2..afe230376 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -100,12 +100,12 @@ class="usa-button" >Save - + ' + ) + + # context to back out of a broken form on all fields delete + context["modal_button"] = modal_button return self.render_to_response(context) - if formset.is_valid(): + if formset.is_valid() or override: return self.form_valid(formset) else: return self.form_invalid(formset) @@ -363,6 +387,7 @@ class DomainDsDataView(DomainPermissionView, FormMixin): def form_valid(self, formset, **kwargs): """The formset is valid, perform something with it.""" + logger.info("form_valid is called") # Set the dnssecdata from the formset dnssecdata = extensions.DNSSECExtension() @@ -386,7 +411,9 @@ class DomainDsDataView(DomainPermissionView, FormMixin): pass domain = self.get_object() try: + logger.debug("attempting to set dnssecdata") domain.dnssecdata = dnssecdata + logger.debug("successfully set the dnssecdata") except RegistryError as err: errmsg = "Error updating DNSSEC data in the registry." logger.error(errmsg) From d50628f14d0fde24fd90bfc865078dec59d3d513 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 23 Oct 2023 18:32:22 -0400 Subject: [PATCH 15/26] cleaned up working code for review --- src/registrar/templates/domain_dsdata.html | 139 +++++++++------------ src/registrar/views/domain.py | 33 +---- 2 files changed, 67 insertions(+), 105 deletions(-) diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index afe230376..832361f6e 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -22,96 +22,79 @@

    Enter the values given by your DNS provider for DS Data.

    - {% comment %} {% if not dnssec_ds_confirmed %} -

    Required fields are marked with an asterisk (*).

    - - {% csrf_token %} - - - {% else %} {% endcomment %} - {% include "includes/required_fields.html" %} + {% include "includes/required_fields.html" %} -
    - {% csrf_token %} - {{ formset.management_form }} + + {% csrf_token %} + {{ formset.management_form }} - {% for form in formset %} -
    + {% for form in formset %} +
    - DS Data record {{forloop.counter}} + DS Data record {{forloop.counter}} -

    DS Data record {{forloop.counter}}

    +

    DS Data record {{forloop.counter}}

    -
    -
    - {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} - {% input_with_errors form.key_tag %} - {% endwith %} -
    -
    - {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} - {% input_with_errors form.algorithm %} - {% endwith %} -
    -
    - {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} - {% input_with_errors form.digest_type %} - {% endwith %} -
    +
    +
    + {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} + {% input_with_errors form.key_tag %} + {% endwith %}
    - -
    -
    - {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} - {% input_with_errors form.digest %} - {% endwith %} -
    +
    + {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} + {% input_with_errors form.algorithm %} + {% endwith %}
    - -
    -
    - -
    +
    + {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} + {% input_with_errors form.digest_type %} + {% endwith %}
    +
    -
    - {% endfor %} +
    +
    + {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} + {% input_with_errors form.digest %} + {% endwith %} +
    +
    - +
    +
    + +
    +
    - - - - - {% comment %} {% endif %} {% endcomment %} + + + + + + {% if trigger_modal %}     Disable DNSSEC' - ) - - # context to back out of a broken form on all fields delete - context["modal_button"] = modal_button return context @@ -342,30 +332,22 @@ class DomainDsDataView(DomainPermissionView, FormMixin): self.object = self.get_object() formset = self.get_form() override = False - - # log the context and the formset - logger.info("==================FORMSET================") - logger.info(formset) - logger.info("=======>>>>>>CONTEXT<<<<<<===========") - logger.info(self.get_context_data(**kwargs)) - logger.info("========^^^^^^^CONTEXT^^^^^^^^========") - # This is called by the form cancel button, and also by the modal's X and cancel buttons if "btn-cancel-click" in request.POST: - logger.info(">>>>>>>clicked cancel") url = self.get_success_url() return HttpResponseRedirect(url) - #return reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.object.pk}) - # return redirect("domain-dns-dnssec-dsdata", kwargs={"pk": self.object.pk}) + # This is called by the Disable DNSSEC modal to override if "disable-override-click" in request.POST: - logger.info(">>>>>>>>clicked disable") override = True - if len(formset) == 0 and formset.initial != [{}] and override == False: + # This is called when all DNSSEC data has been deleted and the + # Save button is pressed + if len(formset) == 0 and formset.initial == [{}] and override == False: # trigger the modal - logger.info(">>>>>>>>clicked save") + # get context data from super() rather than self + # to preserve the context["form"] context = super().get_context_data(form=formset) context["trigger_modal"] = True # Create HTML for the modal button @@ -387,7 +369,6 @@ class DomainDsDataView(DomainPermissionView, FormMixin): def form_valid(self, formset, **kwargs): """The formset is valid, perform something with it.""" - logger.info("form_valid is called") # Set the dnssecdata from the formset dnssecdata = extensions.DNSSECExtension() @@ -411,9 +392,7 @@ class DomainDsDataView(DomainPermissionView, FormMixin): pass domain = self.get_object() try: - logger.debug("attempting to set dnssecdata") domain.dnssecdata = dnssecdata - logger.debug("successfully set the dnssecdata") except RegistryError as err: errmsg = "Error updating DNSSEC data in the registry." logger.error(errmsg) From 6400fa5f4b4e68bda803db44bc8c6c58c6a2334f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 23 Oct 2023 20:58:09 -0400 Subject: [PATCH 16/26] fixed logic on save with no dnssec data records; fixed link on dns page --- src/registrar/templates/domain_dns.html | 2 +- src/registrar/views/domain.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/domain_dns.html b/src/registrar/templates/domain_dns.html index b16c1cb8b..45529a19e 100644 --- a/src/registrar/templates/domain_dns.html +++ b/src/registrar/templates/domain_dns.html @@ -14,7 +14,7 @@ {% url 'domain-dns-nameservers' pk=domain.id as url %}

    DNS name servers

    - {% url 'domain-dnssec' pk=domain.id as url %} + {% url 'domain-dns-dnssec' pk=domain.id as url %}

    DNSSEC

    {% endblock %} {# domain_content #} diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1058c537a..90e915a60 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -382,7 +382,7 @@ class DomainDsDataView(DomainFormBaseView): # This is called when all DNSSEC data has been deleted and the # Save button is pressed - if len(formset) == 0 and formset.initial == [{}] and override == False: + if len(formset) == 0 and formset.initial != [{}] and override == False: # trigger the modal # get context data from super() rather than self # to preserve the context["form"] From d35475148115e8ae653a3952f540b856f60ed1f7 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 23 Oct 2023 21:16:12 -0400 Subject: [PATCH 17/26] fixed tests and updated code formatting --- src/registrar/tests/test_views.py | 2 +- src/registrar/views/domain.py | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 0fba81fc1..6614cf8e8 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1125,7 +1125,7 @@ class TestWithDomainPermissions(TestWithUser): UserDomainRole.objects.get_or_create( user=self.user, domain=self.domain_multdsdata, - role=UserDomainRole.Roles.ADMIN, + role=UserDomainRole.Roles.MANAGER, ) UserDomainRole.objects.get_or_create( user=self.user, diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 90e915a60..2dcefae39 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -12,10 +12,8 @@ from django.contrib.messages.views import SuccessMessageMixin from django.db import IntegrityError from django.http import HttpResponseRedirect from django.shortcuts import redirect -from django.template import RequestContext from django.urls import reverse from django.views.generic.edit import FormMixin -from django.shortcuts import render from registrar.models import ( Domain, @@ -371,18 +369,19 @@ class DomainDsDataView(DomainFormBaseView): formset = self.get_form() override = False - # This is called by the form cancel button, and also by the modal's X and cancel buttons + # This is called by the form cancel button, + # and also by the modal's X and cancel buttons if "btn-cancel-click" in request.POST: url = self.get_success_url() return HttpResponseRedirect(url) - + # This is called by the Disable DNSSEC modal to override if "disable-override-click" in request.POST: override = True - + # This is called when all DNSSEC data has been deleted and the # Save button is pressed - if len(formset) == 0 and formset.initial != [{}] and override == False: + if len(formset) == 0 and formset.initial != [{}] and override is False: # trigger the modal # get context data from super() rather than self # to preserve the context["form"] From 65b2053a4fc2f1917d0301d9c5aa707411414034 Mon Sep 17 00:00:00 2001 From: dave-kennedy-ecs <111779554+dave-kennedy-ecs@users.noreply.github.com> Date: Mon, 23 Oct 2023 21:18:37 -0400 Subject: [PATCH 18/26] Update src/registrar/templates/domain_sidebar.html updated logic in domain sidebar for code readability Co-authored-by: zandercymatics <141044360+zandercymatics@users.noreply.github.com> --- src/registrar/templates/domain_sidebar.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 65c5254e9..c365638c1 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -34,7 +34,7 @@ > DNSSEC - {% if domain.dnssecdata is not None or request.path|startswith:url and request.path|endswith:'dsdata' %} + {% if domain.dnssecdata is not None or (request.path|startswith:url and request.path|endswith:'dsdata') %}
    • {% url 'domain-dns-dnssec-dsdata' pk=domain.id as url %} From ce6fc5a9ac2d98e735d85b5d15f95de959b11f4e Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 23 Oct 2023 21:29:26 -0400 Subject: [PATCH 19/26] fixed problem with sidebar --- src/registrar/templates/domain_sidebar.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index c365638c1..65c5254e9 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -34,7 +34,7 @@ > DNSSEC - {% if domain.dnssecdata is not None or (request.path|startswith:url and request.path|endswith:'dsdata') %} + {% if domain.dnssecdata is not None or request.path|startswith:url and request.path|endswith:'dsdata' %}
      • {% url 'domain-dns-dnssec-dsdata' pk=domain.id as url %} From 2f40d52254a53e4ea1cc70fe23f19ae73e222c1a Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 24 Oct 2023 12:03:40 -0400 Subject: [PATCH 20/26] edit modal text --- src/registrar/templates/domain_dsdata.html | 2 +- src/registrar/views/domain.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index 832361f6e..f961250a1 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -114,7 +114,7 @@ aria-describedby="Your DNSSEC records will be deleted from the registry." data-force-action > - {% include 'includes/modal.html' with cancel_button_resets_ds_form=True modal_heading="Are you sure you want to continue?" modal_description="some good text" modal_button=modal_button|safe %} + {% include 'includes/modal.html' with cancel_button_resets_ds_form=True modal_heading="Caution: Deleting all DS records will disable DNSSEC on your domain" modal_description="To avoid causing your domain to appear offline you should wait to delete your DS records at your DNS host until the Time to Live (TTL) expires. This is often less than 24 hours, but confirm with your provider." modal_button=modal_button|safe %} {% endblock %} {# domain_content #} diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 2dcefae39..3f6e6df72 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -391,7 +391,7 @@ class DomainDsDataView(DomainFormBaseView): modal_button = ( '' + 'name="disable-override-click">Delete all records' ) # context to back out of a broken form on all fields delete From 548735cb078cd7af144ba1c9a73b20eb6b2bd758 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 24 Oct 2023 12:06:30 -0400 Subject: [PATCH 21/26] edit modal text again --- src/registrar/templates/domain_dsdata.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index f961250a1..bdf4deb46 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -114,7 +114,7 @@ aria-describedby="Your DNSSEC records will be deleted from the registry." data-force-action > - {% include 'includes/modal.html' with cancel_button_resets_ds_form=True modal_heading="Caution: Deleting all DS records will disable DNSSEC on your domain" modal_description="To avoid causing your domain to appear offline you should wait to delete your DS records at your DNS host until the Time to Live (TTL) expires. This is often less than 24 hours, but confirm with your provider." modal_button=modal_button|safe %} + {% include 'includes/modal.html' with cancel_button_resets_ds_form=True modal_heading="Warning: You are about to delete all DS records on your domain" modal_description="To fully disable DNSSEC: In addition to deleting your DS records here you’ll also need to delete the DS records at your DNS host. To avoid causing your domain to appear offline you should wait to delete your DS records at your DNS host until the Time to Live (TTL) expires. This is often less than 24 hours, but confirm with your provider." modal_button=modal_button|safe %} {% endblock %} {# domain_content #} From 15f656b77a751ea942a9d17044f3c417a9a12a99 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 24 Oct 2023 12:19:48 -0400 Subject: [PATCH 22/26] edit modal text on dnssec page --- src/registrar/templates/domain_dnssec.html | 2 +- src/registrar/views/domain.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/domain_dnssec.html b/src/registrar/templates/domain_dnssec.html index 8e19e8360..691ba79b2 100644 --- a/src/registrar/templates/domain_dnssec.html +++ b/src/registrar/templates/domain_dnssec.html @@ -56,7 +56,7 @@ aria-labelledby="Are you sure you want to continue?" aria-describedby="Your DNSSEC records will be deleted from the registry." > - {% include 'includes/modal.html' with modal_heading="Are you sure you want to continue?" modal_button=modal_button|safe %} + {% include 'includes/modal.html' with modal_heading="Are you sure you want to disable DNSSEC?" modal_button=modal_button|safe %} {% endblock %} {# domain_content #} diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 3f6e6df72..b51b931dc 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -292,7 +292,7 @@ class DomainDNSSECView(DomainFormBaseView): modal_button = ( '' + 'name="disable_dnssec">Confirm' ) context["modal_button"] = modal_button From e78e95df0dc80de1f6d241f1fc53ad5ad3a095af Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 24 Oct 2023 16:29:54 -0400 Subject: [PATCH 23/26] fix outline btn style (remove underline) when anchor emlement) --- src/registrar/assets/sass/_theme/_buttons.scss | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_buttons.scss b/src/registrar/assets/sass/_theme/_buttons.scss index 0857ec603..cb2117fb9 100644 --- a/src/registrar/assets/sass/_theme/_buttons.scss +++ b/src/registrar/assets/sass/_theme/_buttons.scss @@ -22,8 +22,11 @@ a.breadcrumb__back { } } -a.usa-button:not(.usa-button--unstyled, .usa-button--outline) { +a.usa-button { text-decoration: none; +} + +a.usa-button:not(.usa-button--unstyled, .usa-button--outline) { color: color('white'); } @@ -111,15 +114,3 @@ a.usa-button--unstyled:visited { margin-left: units(2); } } - - -// WARNING: crazy hack ahead: -// Cancel button(s) on the DNSSEC form pages -// We want to position the cancel button on the -// dnssec forms next to the submit button -// This button's markup is in its own form -.btn-cancel { - position: relative; - top: -39.2px; - left: 88px; -} From 76be3c6b7296ecb53024d267b52949e4903f449c Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 24 Oct 2023 17:36:27 -0400 Subject: [PATCH 24/26] test_ds_data_form_modal --- src/registrar/assets/js/get-gov.js | 33 +++++++++++++++--------------- src/registrar/tests/test_views.py | 16 +++++++++++++++ 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 1813deea0..851de8fcf 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -390,24 +390,23 @@ function prepareDeleteButtons() { */ (function triggerModalOnDsDataForm() { let saveButon = document.querySelector("#save-ds-data"); - - // if (saveButon) - // saveButon.addEventListener('click', triggerModalIfModalTriggerExists); - // function triggerModalIfModalTriggerExists(e){ + // The view context will cause a hitherto hidden modal trigger to + // show up. On save, we'll test for that modal trigger appearing. We'll + // run that test once every 100 ms for 5 secs, which should balance performance + // while accounting for network or lag issues. if (saveButon) { - let i = 0; - var tryToTriggerModal = setInterval(function() { - i++; - if (i > 100) { - clearInterval(tryToTriggerModal); - } - let modalTrigger = document.querySelector("#ds-toggle-dnssec-alert"); - if (modalTrigger) { - modalTrigger.click() - clearInterval(tryToTriggerModal); - } - }, 50); - + let i = 0; + var tryToTriggerModal = setInterval(function() { + i++; + if (i > 100) { + clearInterval(tryToTriggerModal); + } + let modalTrigger = document.querySelector("#ds-toggle-dnssec-alert"); + if (modalTrigger) { + modalTrigger.click() + clearInterval(tryToTriggerModal); + } + }, 50); } })(); diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 6614cf8e8..6bd59facb 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1701,6 +1701,22 @@ class TestDomainDNSSEC(TestDomainOverview): ) self.assertContains(page, "DS Data record 1") + def test_ds_data_form_modal(self): + """When user clicks on save, a modal pops up.""" + add_data_page = self.app.get( + reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_dsdata.id}) + ) + self.assertNotContains(add_data_page, "Trigger Disable DNSSEC Modal") + # Simulate a delete all data + form_data = {} + response = self.client.post( + reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_dsdata.id}), + data=form_data, + ) + self.assertEqual(response.status_code, 200) # Adjust status code as needed + # Now check to see whether the JS trigger for the modal is present on the page + self.assertContains(response, "Trigger Disable DNSSEC Modal") + def test_ds_data_form_submits(self): """DS Data form submits successfully From 7436a8a4632c605f7c972a8161fbc208a1bc1dcf Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 25 Oct 2023 11:22:04 -0400 Subject: [PATCH 25/26] add coment on test_ds_data_form_modal --- src/registrar/tests/test_views.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 6bd59facb..bff64eb33 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1706,6 +1706,10 @@ class TestDomainDNSSEC(TestDomainOverview): add_data_page = self.app.get( reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_dsdata.id}) ) + # Assert that a hidden trigger for the modal does not exist. + # This hidden trigger will pop on the page when certain condition are met: + # 1) Initial form contained DS data, 2) All data is deleted and form is + # submitted. self.assertNotContains(add_data_page, "Trigger Disable DNSSEC Modal") # Simulate a delete all data form_data = {} From 90b12a6c48a339d95fda5f399f9ab6085ce544d9 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 25 Oct 2023 21:35:17 -0400 Subject: [PATCH 26/26] generic error handling for RegistryErrors as in other views --- src/registrar/views/domain.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index b51b931dc..a2f2c1198 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -430,10 +430,12 @@ class DomainDsDataView(DomainFormBaseView): try: self.object.dnssecdata = dnssecdata except RegistryError as err: - errmsg = "Error updating DNSSEC data in the registry." - logger.error(errmsg) - logger.error(err) - messages.error(self.request, errmsg) + if err.is_connection_error(): + messages.error(self.request, CANNOT_CONTACT_REGISTRY) + logger.error(f"Registry connection error: {err}") + else: + messages.error(self.request, GENERIC_ERROR) + logger.error(f"Registry error: {err}") return self.form_invalid(formset) else: messages.success(