diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 4226e6ac9..f5e229b0a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -294,9 +294,8 @@ class Domain(TimeStampedModel, DomainHelper): return None def getDnssecdataChanges( - self, - _dnssecdata: Optional[extensions.DNSSECExtension] - ) -> 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 @@ -392,17 +391,17 @@ class Domain(TimeStampedModel, DomainHelper): remRequest.add_extension(remExtension) try: if ( - "dsData" in _addDnssecdata and - _addDnssecdata["dsData"] is not None - or "keyData" in _addDnssecdata and - _addDnssecdata["keyData"] is not None + "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 + "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: diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index d28171155..f27713454 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -7,7 +7,7 @@ import random from string import ascii_uppercase from django.test import TestCase from unittest.mock import MagicMock, Mock, patch -from typing import List, Dict, Mapping, Any +from typing import List, Dict from django.conf import settings from django.contrib.auth import get_user_model, login @@ -704,18 +704,26 @@ class MockEppLib(TestCase): "alg": 1, "pubKey": "AQPJ////4Q==", } - dnssecExtensionWithDsData = extensions.DNSSECExtension(**{ - "dsData": [common.DSData(**addDsData1)], - }) - dnssecExtensionWithMultDsData = extensions.DNSSECExtension(**{ - "dsData": [ - common.DSData(**addDsData1), # type: ignore - common.DSData(**addDsData2), # type: ignore - ], - }) - dnssecExtensionWithKeyData = extensions.DNSSECExtension(**{ - "keyData": [common.DNSSECKeyData(**keyDataDict)], # type: ignore - }) + dnssecExtensionWithDsData = extensions.DNSSECExtension( + **{ + "dsData": [ + common.DSData(**addDsData1) # type: ignore + ], # type: ignore + } + ) + dnssecExtensionWithMultDsData = extensions.DNSSECExtension( + **{ + "dsData": [ + common.DSData(**addDsData1), # type: ignore + common.DSData(**addDsData2), # type: ignore + ], # type: ignore + } + ) + dnssecExtensionWithKeyData = extensions.DNSSECExtension( + **{ + "keyData": [common.DNSSECKeyData(**keyDataDict)], # type: ignore + } + ) dnssecExtensionRemovingDsData = extensions.DNSSECExtension() def mockSend(self, _request, cleaned): @@ -756,35 +764,20 @@ class MockEppLib(TestCase): if getattr(_request, "name", None) == "security.gov": return MagicMock(res_data=[self.infoDomainNoContact]) elif getattr(_request, "name", None) == "dnssec-dsdata.gov": - if self.mockedSendFunction.call_count == 1: - return MagicMock(res_data=[self.mockDataInfoDomain]) - else: - return MagicMock( - res_data=[self.mockDataInfoDomain], - extensions=[ - self.dnssecExtensionWithDsData - ], - ) + return MagicMock( + res_data=[self.mockDataInfoDomain], + extensions=[self.dnssecExtensionWithDsData], + ) elif getattr(_request, "name", None) == "dnssec-multdsdata.gov": - if self.mockedSendFunction.call_count == 1: - return MagicMock(res_data=[self.mockDataInfoDomain]) - else: - return MagicMock( - res_data=[self.mockDataInfoDomain], - extensions=[ - self.dnssecExtensionWithMultDsData - ], - ) + return MagicMock( + res_data=[self.mockDataInfoDomain], + extensions=[self.dnssecExtensionWithMultDsData], + ) elif getattr(_request, "name", None) == "dnssec-keydata.gov": - if self.mockedSendFunction.call_count == 1: - return MagicMock(res_data=[self.mockDataInfoDomain]) - else: - return MagicMock( - res_data=[self.mockDataInfoDomain], - extensions=[ - self.dnssecExtensionWithKeyData - ], - ) + return MagicMock( + res_data=[self.mockDataInfoDomain], + extensions=[self.dnssecExtensionWithKeyData], + ) elif getattr(_request, "name", None) == "dnssec-none.gov": # this case is not necessary, but helps improve readability return MagicMock(res_data=[self.mockDataInfoDomain]) diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 05f56e0f4..cf2d002f5 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -1034,23 +1034,40 @@ class TestRegistrantDNSSEC(MockEppLib): """ + # 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.dnssecExtensionWithDsData], + ) + 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-dsdata.gov") domain.dnssecdata = 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 - args, _ = self.mockedSendFunction.call_args + args, _ = mocked_send.call_args # assert that the extension on the update matches self.assertEquals( args[0].extensions[0], - self.createUpdateExtension( - self.dnssecExtensionWithDsData - ), + self.createUpdateExtension(self.dnssecExtensionWithDsData), ) # test that the dnssecdata getter is functioning properly dnssecdata_get = domain.dnssecdata - self.mockedSendFunction.assert_has_calls( + mocked_send.assert_has_calls( [ call( commands.InfoDomain( @@ -1077,9 +1094,9 @@ class TestRegistrantDNSSEC(MockEppLib): ] ) - self.assertEquals( - dnssecdata_get.dsData, self.dnssecExtensionWithDsData.dsData - ) + self.assertEquals(dnssecdata_get.dsData, self.dnssecExtensionWithDsData.dsData) + + patcher.stop() def test_dnssec_is_idempotent(self): """ @@ -1095,9 +1112,28 @@ class TestRegistrantDNSSEC(MockEppLib): 3 - setter causes the getter to call info domain on next get from cache 4 - UpdateDomain command is not called on second setter (no change) 5 - 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.dnssecExtensionWithDsData], + ) + 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-dsdata.gov") # set the dnssecdata once @@ -1106,7 +1142,7 @@ class TestRegistrantDNSSEC(MockEppLib): domain.dnssecdata = self.dnssecExtensionWithDsData # test that the dnssecdata getter is functioning properly dnssecdata_get = domain.dnssecdata - self.mockedSendFunction.assert_has_calls( + mocked_send.assert_has_calls( [ call( commands.InfoDomain( @@ -1139,9 +1175,9 @@ class TestRegistrantDNSSEC(MockEppLib): ] ) - self.assertEquals( - dnssecdata_get.dsData, self.dnssecExtensionWithDsData.dsData - ) + self.assertEquals(dnssecdata_get.dsData, self.dnssecExtensionWithDsData.dsData) + + patcher.stop() def test_user_adds_dnssec_data_multiple_dsdata(self): """ @@ -1156,23 +1192,40 @@ class TestRegistrantDNSSEC(MockEppLib): """ + # 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.dnssecExtensionWithMultDsData], + ) + 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-multdsdata.gov") domain.dnssecdata = 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 - args, _ = self.mockedSendFunction.call_args + args, _ = mocked_send.call_args # assert that the extension matches self.assertEquals( args[0].extensions[0], - self.createUpdateExtension( - self.dnssecExtensionWithMultDsData - ), + self.createUpdateExtension(self.dnssecExtensionWithMultDsData), ) # test that the dnssecdata getter is functioning properly dnssecdata_get = domain.dnssecdata - self.mockedSendFunction.assert_has_calls( + mocked_send.assert_has_calls( [ call( commands.UpdateDomain( @@ -1197,6 +1250,8 @@ class TestRegistrantDNSSEC(MockEppLib): dnssecdata_get.dsData, self.dnssecExtensionWithMultDsData.dsData ) + patcher.stop() + def test_user_removes_dnssec_data(self): """ Scenario: Registrant removes DNSSEC ds data. @@ -1211,6 +1266,25 @@ class TestRegistrantDNSSEC(MockEppLib): """ + # 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.dnssecExtensionWithDsData], + ) + 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-dsdata.gov") # dnssecdata_get_initial = domain.dnssecdata # call to force initial mock # domain._invalidate_cache() @@ -1219,7 +1293,7 @@ class TestRegistrantDNSSEC(MockEppLib): # 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, _ = self.mockedSendFunction.call_args + args, _ = mocked_send.call_args # assert that the extension on the update matches self.assertEquals( args[0].extensions[0], @@ -1228,7 +1302,7 @@ class TestRegistrantDNSSEC(MockEppLib): remove=True, ), ) - self.mockedSendFunction.assert_has_calls( + mocked_send.assert_has_calls( [ call( commands.InfoDomain( @@ -1265,6 +1339,8 @@ class TestRegistrantDNSSEC(MockEppLib): ] ) + patcher.stop() + def test_user_adds_dnssec_keydata(self): """ Scenario: Registrant adds DNSSEC key data. @@ -1278,23 +1354,40 @@ class TestRegistrantDNSSEC(MockEppLib): """ + # 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, _ = self.mockedSendFunction.call_args + args, _ = mocked_send.call_args # assert that the extension matches self.assertEquals( args[0].extensions[0], - self.createUpdateExtension( - self.dnssecExtensionWithKeyData - ), + self.createUpdateExtension(self.dnssecExtensionWithKeyData), ) # test that the dnssecdata getter is functioning properly dnssecdata_get = domain.dnssecdata - self.mockedSendFunction.assert_has_calls( + mocked_send.assert_has_calls( [ call( commands.UpdateDomain( @@ -1319,6 +1412,8 @@ class TestRegistrantDNSSEC(MockEppLib): 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/views/domain.py b/src/registrar/views/domain.py index 6cc89682e..2781be038 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -497,7 +497,7 @@ class DomainKeydataView(DomainPermissionView, FormMixin): } if dnssecdata.keyData is None: dnssecdata.keyData = [] - dnssecdata["keyData"].append(common.DNSSECKeyData(**keyrecord)) + dnssecdata.keyData.append(common.DNSSECKeyData(**keyrecord)) except KeyError: # no server information in this field, skip it pass