diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 2188a1764..c539838e0 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -2,6 +2,7 @@ from itertools import zip_longest import logging from datetime import date from string import digits +from typing import Optional from django_fsm import FSMField, transition, TransitionNotAllowed # type: ignore from django.db import models @@ -283,7 +284,7 @@ class Domain(TimeStampedModel, DomainHelper): return e.code @Cache - def dnssecdata(self) -> extensions.DNSSECExtension: + def dnssecdata(self) -> Optional[extensions.DNSSECExtension]: try: return self._get_property("dnssecdata") except Exception as err: @@ -291,17 +292,18 @@ class Domain(TimeStampedModel, DomainHelper): # TODO - 433 error handling ticket should address this logger.info("Domain does not have dnssec data defined %s" % err) return None - + def getDnssecdataChanges( - self, _dnssecdata: dict - ) -> tuple[dict, dict]: + self, + _dnssecdata: Optional[extensions.DNSSECExtension] + ) -> tuple[dict, dict]: """ calls self.dnssecdata, it should pull from cache but may result in an epp call returns tuple of 2 values as follows: addExtension: dict remExtension: dict - + addExtension includes all dsData or keyData to be added remExtension includes all dsData or keyData to be removed @@ -311,34 +313,76 @@ class Domain(TimeStampedModel, DomainHelper): remExtension will be all existing dnssecdata to be deleted """ + if isinstance(_dnssecdata, extensions.DNSSECExtension): + logger.info("extension is properly typed") + else: + logger.info("extension is NOT properly typed") + oldDnssecdata = self.dnssecdata - addDnssecdata = {"dsData": [], "keyData": [],} - remDnssecdata = {"dsData": [], "keyData": [],} - - if _dnssecdata and len(_dnssecdata["dsData"]) > 0: + addDnssecdata: dict = {} + # "dsData": [], + # "keyData": [], + # } + remDnssecdata: dict = {} + # "dsData": [], + # "keyData": [], + # } + if _dnssecdata and _dnssecdata.dsData is not None: + logger.info("there is submitted dsdata for comparison") + logger.info("there is %s submitted records", len(_dnssecdata.dsData)) # initialize addDnssecdata and remDnssecdata for dsData - addDnssecdata["dsData"] = _dnssecdata["dsData"] - remDnssecdata["dsData"] = [] + addDnssecdata["dsData"] = _dnssecdata.dsData + # remDnssecdata["dsData"] = [] if oldDnssecdata and len(oldDnssecdata.dsData) > 0: + logger.info("there is existing ds data for comparison") + logger.info("there is %s existing records for compare", len(oldDnssecdata.dsData)) # if existing dsData not in new dsData, mark for removal - remDnssecdata["dsData"] = [dsData for dsData in oldDnssecdata.dsData if dsData not in _dnssecdata["dsData"]] + dsDataForRemoval = [ + dsData + for dsData in oldDnssecdata.dsData + if dsData not in _dnssecdata.dsData + ] + if len(dsDataForRemoval) > 0: + logger.info("ds data marked for removal") + remDnssecdata["dsData"] = dsDataForRemoval # if new dsData not in existing dsData, mark for add - addDnssecdata["dsData"] = [dsData for dsData in _dnssecdata["dsData"] if dsData not in oldDnssecdata.dsData] - - elif _dnssecdata and len(_dnssecdata["keyData"]) > 0: + dsDataForAdd = [ + dsData + for dsData in _dnssecdata.dsData + if dsData not in oldDnssecdata.dsData + ] + if len(dsDataForAdd) > 0: + logger.info("ds data marked for add") + addDnssecdata["dsData"] = dsDataForAdd + else: + addDnssecdata["dsData"] = None + + elif _dnssecdata and _dnssecdata.keyData is not None: # initialize addDnssecdata and remDnssecdata for keyData - addDnssecdata["keyData"] = _dnssecdata["keyData"] - remDnssecdata["keyData"] = [] + addDnssecdata["keyData"] = _dnssecdata.keyData + # remDnssecdata["keyData"] = [] if oldDnssecdata and len(oldDnssecdata.keyData) > 0: # if existing keyData not in new keyData, mark for removal - remDnssecdata["keyData"] = [keyData for keyData in oldDnssecdata.keyData if keyData not in _dnssecdata["keyData"]] + 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 - addDnssecdata["keyData"] = [keyData for keyData in _dnssecdata["keyData"] if keyData not in oldDnssecdata.keyData] + 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 remDnssecdata["dsData"] = getattr(oldDnssecdata, "dsData", None) @@ -347,7 +391,7 @@ class Domain(TimeStampedModel, DomainHelper): return addDnssecdata, remDnssecdata @dnssecdata.setter # type: ignore - def dnssecdata(self, _dnssecdata: dict): + def dnssecdata(self, _dnssecdata: Optional[extensions.DNSSECExtension]): _addDnssecdata, _remDnssecdata = self.getDnssecdataChanges(_dnssecdata) addParams = { "maxSigLife": _addDnssecdata.get("maxSigLife", None), @@ -366,12 +410,26 @@ class Domain(TimeStampedModel, DomainHelper): remExtension = commands.UpdateDomainDNSSECExtension(**remParams) remRequest.add_extension(remExtension) try: - if len(_addDnssecdata.get("dsData", [])) > 0 or len(_addDnssecdata.get("keyData",[])) > 0: + if ( + "dsData" in _addDnssecdata and + _addDnssecdata["dsData"] is not None + or "keyData" in _addDnssecdata and + _addDnssecdata["keyData"] is not None + ): + logger.info("sending addition") registry.send(addRequest, cleaned=True) - if len(_remDnssecdata.get("dsData", [])) > 0 or len(_remDnssecdata.get("keyData", [])) > 0: + if ( + "dsData" in _remDnssecdata and + _remDnssecdata["dsData"] is not None + or "keyData" in _remDnssecdata and + _remDnssecdata["keyData"] is not None + ): + logger.info("sending removal") registry.send(remRequest, cleaned=True) except RegistryError as e: - logger.error("Error updating DNSSEC, code was %s error was %s" % (e.code, e)) + logger.error( + "Error updating DNSSEC, code was %s error was %s" % (e.code, e) + ) raise e @nameservers.setter # type: ignore diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 6ede8965b..4d399fd29 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -706,22 +706,19 @@ class MockEppLib(TestCase): } dnssecExtensionWithDsData: Mapping[Any, Any] = { "dsData": [common.DSData(**addDsData1)], # type: ignore - "keyData": [], } dnssecExtensionWithMultDsData: Mapping[str, Any] = { "dsData": [ common.DSData(**addDsData1), # type: ignore common.DSData(**addDsData2), # type: ignore ], - "keyData": [], } dnssecExtensionWithKeyData: Mapping[str, Any] = { "keyData": [common.DNSSECKeyData(**keyDataDict)], # type: ignore - "dsData": [], } dnssecExtensionRemovingDsData: Mapping[Any, Any] = { - "dsData": [], - "keyData": [], + "dsData": None, + "keyData": None, } def mockSend(self, _request, cleaned): diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index b8d648e45..2a0a820ec 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -984,7 +984,9 @@ class TestRegistrantDNSSEC(MockEppLib): """Rule: Registrants may modify their secure DNS data""" # helper function to create UpdateDomainDNSSECExtention object for verification - def createUpdateExtension(self, dnssecdata: extensions.DNSSECExtension, remove=False): + def createUpdateExtension( + self, dnssecdata: extensions.DNSSECExtension, remove=False + ): if not remove: return commands.UpdateDomainDNSSECExtension( maxSigLife=dnssecdata.maxSigLife, @@ -1033,7 +1035,9 @@ class TestRegistrantDNSSEC(MockEppLib): """ domain, _ = Domain.objects.get_or_create(name="dnssec-dsdata.gov") - domain.dnssecdata = self.dnssecExtensionWithDsData + domain.dnssecdata = extensions.DNSSECExtension( + **self.dnssecExtensionWithDsData + ) # 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 @@ -1098,9 +1102,9 @@ class TestRegistrantDNSSEC(MockEppLib): domain, _ = Domain.objects.get_or_create(name="dnssec-dsdata.gov") # set the dnssecdata once - domain.dnssecdata = self.dnssecExtensionWithDsData + domain.dnssecdata = extensions.DNSSECExtension(**self.dnssecExtensionWithDsData) # set the dnssecdata again - domain.dnssecdata = self.dnssecExtensionWithDsData + domain.dnssecdata = extensions.DNSSECExtension(**self.dnssecExtensionWithDsData) # test that the dnssecdata getter is functioning properly dnssecdata_get = domain.dnssecdata self.mockedSendFunction.assert_has_calls( @@ -1155,7 +1159,7 @@ class TestRegistrantDNSSEC(MockEppLib): domain, _ = Domain.objects.get_or_create(name="dnssec-multdsdata.gov") - domain.dnssecdata = self.dnssecExtensionWithMultDsData + domain.dnssecdata = extensions.DNSSECExtension(**self.dnssecExtensionWithMultDsData) # 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 @@ -1201,16 +1205,18 @@ class TestRegistrantDNSSEC(MockEppLib): This test verifies: 1 - setter initially calls InfoDomain command - 2 - invalidate cache forces second InfoDomain command (to match mocks) + 2 - first setter calls UpdateDomain command + 3 - second setter calls InfoDomain command again 3 - setter then calls UpdateDomain command 4 - setter adds the UpdateDNSSECExtension extension to the command with rem """ domain, _ = Domain.objects.get_or_create(name="dnssec-dsdata.gov") - dnssecdata_get_initial = domain.dnssecdata # call to force initial mock - domain._invalidate_cache() - domain.dnssecdata = self.dnssecExtensionRemovingDsData + # dnssecdata_get_initial = domain.dnssecdata # call to force initial mock + # domain._invalidate_cache() + domain.dnssecdata = extensions.DNSSECExtension(**self.dnssecExtensionWithDsData) + domain.dnssecdata = extensions.DNSSECExtension(**self.dnssecExtensionRemovingDsData) # 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 @@ -1220,7 +1226,7 @@ class TestRegistrantDNSSEC(MockEppLib): args[0].extensions[0], self.createUpdateExtension( extensions.DNSSECExtension(**self.dnssecExtensionWithDsData), - remove=True + remove=True, ), ) self.mockedSendFunction.assert_has_calls( @@ -1231,6 +1237,16 @@ class TestRegistrantDNSSEC(MockEppLib): ), cleaned=True, ), + call( + commands.UpdateDomain( + name="dnssec-dsdata.gov", + nsset=None, + keyset=None, + registrant=None, + auth_info=None, + ), + cleaned=True, + ), call( commands.InfoDomain( name="dnssec-dsdata.gov", @@ -1246,7 +1262,7 @@ class TestRegistrantDNSSEC(MockEppLib): auth_info=None, ), cleaned=True, - ), + ), ] ) @@ -1265,7 +1281,7 @@ class TestRegistrantDNSSEC(MockEppLib): domain, _ = Domain.objects.get_or_create(name="dnssec-keydata.gov") - domain.dnssecdata = self.dnssecExtensionWithKeyData + domain.dnssecdata = extensions.DNSSECExtension(**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 @@ -1314,7 +1330,7 @@ class TestRegistrantDNSSEC(MockEppLib): domain, _ = Domain.objects.get_or_create(name="dnssec-invalid.gov") with self.assertRaises(RegistryError) as err: - domain.dnssecdata = self.dnssecExtensionWithDsData + domain.dnssecdata = extensions.DNSSECExtension(**self.dnssecExtensionWithDsData) self.assertTrue( err.is_client_error() or err.is_session_error() or err.is_server_error() ) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index b4509b162..6cc89682e 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -362,8 +362,8 @@ class DomainDsdataView(DomainPermissionView, FormMixin): def form_valid(self, formset): """The formset is valid, perform something with it.""" - # Set the nameservers from the formset - dnssecdata = {"dsData": []} + # Set the dnssecdata from the formset + dnssecdata = extensions.DNSSECExtension() for form in formset: try: @@ -375,13 +375,13 @@ class DomainDsdataView(DomainPermissionView, FormMixin): "digestType": int(form.cleaned_data["digest_type"]), "digest": form.cleaned_data["digest"], } - dnssecdata["dsData"].append(common.DSData(**dsrecord)) + if dnssecdata.dsData is None: + dnssecdata.dsData = [] + dnssecdata.dsData.append(common.DSData(**dsrecord)) except KeyError: # no server information in this field, skip it pass domain = self.get_object() - if len(dnssecdata["dsData"]) == 0: - dnssecdata = {} try: domain.dnssecdata = dnssecdata except RegistryError as err: @@ -483,7 +483,7 @@ class DomainKeydataView(DomainPermissionView, FormMixin): """The formset is valid, perform something with it.""" # Set the nameservers from the formset - dnssecdata = {"keyData": []} + dnssecdata = extensions.DNSSECExtension() for form in formset: try: @@ -495,13 +495,13 @@ class DomainKeydataView(DomainPermissionView, FormMixin): "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() - if len(dnssecdata["keyData"]) == 0: - dnssecdata = {} try: domain.dnssecdata = dnssecdata except RegistryError as err: