Skip to content

Commit

Permalink
Merge pull request #2025 from cisagov/rh/1979-fix-unknown-state
Browse files Browse the repository at this point in the history
ISSUE 1979 PT 2: Unknown State Remdiation Pt 2
  • Loading branch information
therealslimhsiehdy authored Apr 19, 2024
2 parents c169ab3 + 01788ce commit 61e6206
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 23 deletions.
56 changes: 56 additions & 0 deletions src/registrar/models/domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -1689,13 +1689,69 @@ def _delete_hosts_if_not_used(self, hostsToDelete: list[str]):
else:
logger.error("Error _delete_hosts_if_not_used, code was %s error was %s" % (e.code, e))

def _fix_unknown_state(self, cleaned):
"""
_fix_unknown_state: Calls _add_missing_contacts_if_unknown
to add contacts in as needed (or return an error). Otherwise
if we are able to add contacts and the state is out of UNKNOWN
and (and should be into DNS_NEEDED), we double check the
current state and # of nameservers and update the state from there
"""
try:
self._add_missing_contacts_if_unknown(cleaned)

except Exception as e:
logger.error(
"%s couldn't _add_missing_contacts_if_unknown, error was %s."
"Domain will still be in UNKNOWN state." % (self.name, e)
)
if len(self.nameservers) >= 2 and (self.state != self.State.READY):
self.ready()
self.save()

@transition(field="state", source=State.UNKNOWN, target=State.DNS_NEEDED)
def _add_missing_contacts_if_unknown(self, cleaned):
"""
_add_missing_contacts_if_unknown: Add contacts (SECURITY, TECHNICAL, and/or ADMINISTRATIVE)
if they are missing, AND switch the state to DNS_NEEDED from UNKNOWN (if it
is in an UNKNOWN state, that is an error state)
Note: The transition state change happens at the end of the function
"""

missingAdmin = True
missingSecurity = True
missingTech = True

if len(cleaned.get("_contacts")) < 3:
for contact in cleaned.get("_contacts"):
if contact.type == PublicContact.ContactTypeChoices.ADMINISTRATIVE:
missingAdmin = False
if contact.type == PublicContact.ContactTypeChoices.SECURITY:
missingSecurity = False
if contact.type == PublicContact.ContactTypeChoices.TECHNICAL:
missingTech = False

# We are only creating if it doesn't exist so we don't overwrite
if missingAdmin:
administrative_contact = self.get_default_administrative_contact()
administrative_contact.save()
if missingSecurity:
security_contact = self.get_default_security_contact()
security_contact.save()
if missingTech:
technical_contact = self.get_default_technical_contact()
technical_contact.save()

def _fetch_cache(self, fetch_hosts=False, fetch_contacts=False):
"""Contact registry for info about a domain."""
try:
data_response = self._get_or_create_domain()
cache = self._extract_data_from_response(data_response)
cleaned = self._clean_cache(cache, data_response)
self._update_hosts_and_contacts(cleaned, fetch_hosts, fetch_contacts)

if self.state == self.State.UNKNOWN:
self._fix_unknown_state(cleaned)
if fetch_hosts:
self._update_hosts_and_ips_in_db(cleaned)
if fetch_contacts:
Expand Down
26 changes: 22 additions & 4 deletions src/registrar/tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,6 @@ def setUp(self):
self.domain_2, _ = Domain.objects.get_or_create(name="adomain2.gov", state=Domain.State.DNS_NEEDED)
self.domain_3, _ = Domain.objects.get_or_create(name="ddomain3.gov", state=Domain.State.ON_HOLD)
self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN)
self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN)
self.domain_5, _ = Domain.objects.get_or_create(
name="bdomain5.gov", state=Domain.State.DELETED, deleted=timezone.make_aware(datetime(2023, 11, 1))
)
Expand Down Expand Up @@ -977,7 +976,20 @@ def dummyInfoContactResultData(
mockDataInfoDomain = fakedEppObject(
"fakePw",
cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)),
contacts=[common.DomainContact(contact="123", type=PublicContact.ContactTypeChoices.SECURITY)],
contacts=[
common.DomainContact(
contact="securityContact",
type=PublicContact.ContactTypeChoices.SECURITY,
),
common.DomainContact(
contact="technicalContact",
type=PublicContact.ContactTypeChoices.TECHNICAL,
),
common.DomainContact(
contact="adminContact",
type=PublicContact.ContactTypeChoices.ADMINISTRATIVE,
),
],
hosts=["fake.host.com"],
statuses=[
common.Status(state="serverTransferProhibited", description="", lang="en"),
Expand Down Expand Up @@ -1047,10 +1059,13 @@ def dummyInfoContactResultData(
ex_date=date(2023, 11, 15),
)
mockDataInfoContact = mockDataInfoDomain.dummyInfoContactResultData(
"123", "[email protected]", datetime(2023, 5, 25, 19, 45, 35), "lastPw"
id="123", email="[email protected]", cr_date=datetime(2023, 5, 25, 19, 45, 35), pw="lastPw"
)
mockDataSecurityContact = mockDataInfoDomain.dummyInfoContactResultData(
id="securityContact", email="[email protected]", cr_date=datetime(2023, 5, 25, 19, 45, 35), pw="lastPw"
)
InfoDomainWithContacts = fakedEppObject(
"fakepw",
"fakePw",
cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)),
contacts=[
common.DomainContact(
Expand All @@ -1072,6 +1087,7 @@ def dummyInfoContactResultData(
common.Status(state="inactive", description="", lang="en"),
],
registrant="regContact",
ex_date=date(2023, 11, 15),
)

InfoDomainWithDefaultSecurityContact = fakedEppObject(
Expand Down Expand Up @@ -1498,6 +1514,8 @@ def mockInfoDomainCommands(self, _request, cleaned):
"meow.gov": (self.mockDataInfoDomainSubdomainAndIPAddress, None),
"fakemeow.gov": (self.mockDataInfoDomainNotSubdomainNoIP, None),
"subdomainwoip.gov": (self.mockDataInfoDomainSubdomainNoIP, None),
"ddomain3.gov": (self.InfoDomainWithContacts, None),
"igorville.gov": (self.InfoDomainWithContacts, None),
}

# Retrieve the corresponding values from the dictionary
Expand Down
6 changes: 5 additions & 1 deletion src/registrar/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@
Domain,
DomainRequest,
DomainInformation,
DraftDomain,
User,
DomainInvitation,
Contact,
PublicContact,
Host,
Website,
DraftDomain,
)
from registrar.models.user_domain_role import UserDomainRole
from registrar.models.verified_by_staff import VerifiedByStaff
Expand Down Expand Up @@ -690,6 +692,8 @@ def test_place_and_remove_hold_epp(self):

def tearDown(self):
super().tearDown()
PublicContact.objects.all().delete()
Host.objects.all().delete()
Domain.objects.all().delete()
DomainInformation.objects.all().delete()
DomainRequest.objects.all().delete()
Expand Down
77 changes: 63 additions & 14 deletions src/registrar/tests/test_models_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ def test_cache_nested_elements_not_subdomain(self):
common.DomainContact(contact="123", type="security"),
]
expectedContactsDict = {
PublicContact.ContactTypeChoices.ADMINISTRATIVE: None,
PublicContact.ContactTypeChoices.SECURITY: "123",
PublicContact.ContactTypeChoices.TECHNICAL: None,
PublicContact.ContactTypeChoices.ADMINISTRATIVE: "adminContact",
PublicContact.ContactTypeChoices.SECURITY: "securityContact",
PublicContact.ContactTypeChoices.TECHNICAL: "technicalContact",
}
expectedHostsDict = {
"name": self.mockDataInfoDomain.hosts[0],
Expand All @@ -129,6 +129,7 @@ def test_cache_nested_elements_not_subdomain(self):
# The contact list should not contain what is sent by the registry by default,
# as _fetch_cache will transform the type to PublicContact
self.assertNotEqual(domain._cache["contacts"], expectedUnfurledContactsList)

self.assertEqual(domain._cache["contacts"], expectedContactsDict)

# get and check hosts is set correctly
Expand Down Expand Up @@ -203,19 +204,20 @@ def test_cache_nested_elements_is_subdomain(self):
def test_map_epp_contact_to_public_contact(self):
# Tests that the mapper is working how we expect
with less_console_noise():
domain, _ = Domain.objects.get_or_create(name="registry.gov")
domain, _ = Domain.objects.get_or_create(name="registry.gov", state=Domain.State.DNS_NEEDED)
security = PublicContact.ContactTypeChoices.SECURITY
mapped = domain.map_epp_contact_to_public_contact(
self.mockDataInfoContact,
self.mockDataInfoContact.id,
self.mockDataSecurityContact,
self.mockDataSecurityContact.id,
security,
)

# id, registry_id, and contact are the same thing
expected_contact = PublicContact(
domain=domain,
contact_type=security,
registry_id="123",
email="123@mail.gov",
registry_id="securityContact",
email="security@mail.gov",
voice="+1.8882820870",
fax="+1-212-9876543",
pw="lastPw",
Expand All @@ -232,7 +234,6 @@ def test_map_epp_contact_to_public_contact(self):
# two duplicate objects. We would expect
# these not to have the same state.
expected_contact._state = mapped._state

# Mapped object is what we expect
self.assertEqual(mapped.__dict__, expected_contact.__dict__)

Expand All @@ -243,9 +244,9 @@ def test_map_epp_contact_to_public_contact(self):
registry_id=domain.security_contact.registry_id,
contact_type=security,
).get()

# DB Object is the same as the mapped object
self.assertEqual(db_object, in_db)

domain.security_contact = in_db
# Trigger the getter
_ = domain.security_contact
Expand Down Expand Up @@ -309,6 +310,40 @@ def test_errors_map_epp_contact_to_public_contact(self):
)
self.assertEqual(context.exception.code, desired_error)

def test_fix_unknown_to_ready_state(self):
"""
Scenario: A error occurred and the domain's state is in UNKONWN
which shouldn't happen. The biz logic and test is to make sure
we resolve that UNKNOWN state to READY because it has 2 nameservers.
Note:
* Default state when you do get_or_create is UNKNOWN
* justnameserver.com has 2 nameservers which is why we are using it
* justnameserver.com also has all 3 contacts hence 0 count
"""
with less_console_noise():
domain, _ = Domain.objects.get_or_create(name="justnameserver.com")
# trigger the getter
_ = domain.nameservers
self.assertEqual(domain.state, Domain.State.READY)
self.assertEqual(PublicContact.objects.filter(domain=domain.id).count(), 0)

def test_fix_unknown_to_dns_needed_state(self):
"""
Scenario: A error occurred and the domain's state is in UNKONWN
which shouldn't happen. The biz logic and test is to make sure
we resolve that UNKNOWN state to DNS_NEEDED because it has 1 nameserver.
Note:
* Default state when you do get_or_create is UNKNOWN
* defaulttechnical.gov has 1 nameservers which is why we are using it
* defaulttechnical.gov already has a security contact (1) hence 2 count
"""
with less_console_noise():
domain, _ = Domain.objects.get_or_create(name="defaulttechnical.gov")
# trigger the getter
_ = domain.nameservers
self.assertEqual(domain.state, Domain.State.DNS_NEEDED)
self.assertEqual(PublicContact.objects.filter(domain=domain.id).count(), 2)


class TestDomainCreation(MockEppLib):
"""Rule: An approved domain request must result in a domain"""
Expand Down Expand Up @@ -346,7 +381,7 @@ def test_accessing_domain_properties_creates_domain_in_registry(self):
Given that no domain object exists in the registry
When a property is accessed
Then Domain sends `commands.CreateDomain` to the registry
And `domain.state` is set to `UNKNOWN`
And `domain.state` is set to `DNS_NEEDED`
And `domain.is_active()` returns False
"""
with less_console_noise():
Expand Down Expand Up @@ -375,7 +410,7 @@ def test_accessing_domain_properties_creates_domain_in_registry(self):
any_order=False, # Ensure calls are in the specified order
)

self.assertEqual(domain.state, Domain.State.UNKNOWN)
self.assertEqual(domain.state, Domain.State.DNS_NEEDED)
self.assertEqual(domain.is_active(), False)

@skip("assertion broken with mock addition")
Expand All @@ -400,6 +435,7 @@ def tearDown(self) -> None:
DomainInformation.objects.all().delete()
DomainRequest.objects.all().delete()
PublicContact.objects.all().delete()
Host.objects.all().delete()
Domain.objects.all().delete()
User.objects.all().delete()
DraftDomain.objects.all().delete()
Expand Down Expand Up @@ -485,6 +521,7 @@ def test_first_ready(self):

def tearDown(self) -> None:
PublicContact.objects.all().delete()
Host.objects.all().delete()
Domain.objects.all().delete()
super().tearDown()

Expand Down Expand Up @@ -624,6 +661,7 @@ def tearDown(self):
self.domain._invalidate_cache()
self.domain_contact._invalidate_cache()
PublicContact.objects.all().delete()
Host.objects.all().delete()
Domain.objects.all().delete()

def test_no_security_email(self):
Expand Down Expand Up @@ -998,10 +1036,10 @@ def test_is_disclosed_on_security_contact(self):
And the field `disclose` is set to true for DF.EMAIL
"""
with less_console_noise():
domain, _ = Domain.objects.get_or_create(name="igorville.gov")
domain, _ = Domain.objects.get_or_create(name="igorville.gov", state=Domain.State.DNS_NEEDED)
expectedSecContact = PublicContact.get_default_security()
expectedSecContact.domain = domain
expectedSecContact.email = "123@mail.gov"
expectedSecContact.email = "security@mail.gov"
domain.security_contact = expectedSecContact
expectedCreateCommand = self._convertPublicContactToEpp(expectedSecContact, disclose_email=True)
self.mockedSendFunction.assert_any_call(expectedCreateCommand, cleaned=True)
Expand Down Expand Up @@ -1847,6 +1885,8 @@ def setUp(self):
self.domain, _ = Domain.objects.get_or_create(name="fake.gov")

def tearDown(self):
PublicContact.objects.all().delete()
Host.objects.all().delete()
Domain.objects.all().delete()
super().tearDown()

Expand Down Expand Up @@ -1904,6 +1944,7 @@ def side_effect(_request, cleaned):
),
cleaned=True,
),
call(commands.InfoHost(name="fake.host.com"), cleaned=True),
call(
commands.UpdateDomain(
name="dnssec-dsdata.gov",
Expand Down Expand Up @@ -1976,6 +2017,13 @@ def side_effect(_request, cleaned):
),
cleaned=True,
),
call(
commands.InfoDomain(
name="dnssec-dsdata.gov",
),
cleaned=True,
),
call(commands.InfoHost(name="fake.host.com"), cleaned=True),
call(
commands.UpdateDomain(
name="dnssec-dsdata.gov",
Expand Down Expand Up @@ -2129,6 +2177,7 @@ def side_effect(_request, cleaned):
),
cleaned=True,
),
call(commands.InfoHost(name="fake.host.com"), cleaned=True),
call(
commands.UpdateDomain(
name="dnssec-dsdata.gov",
Expand Down
2 changes: 1 addition & 1 deletion src/registrar/tests/test_reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ def test_export_domains_to_writer_security_emails(self):
"adomain10.gov,Federal,Armed Forces Retirement Home,Ready\n"
"adomain2.gov,Interstate,(blank),Dns needed\n"
"cdomain11.govFederal-ExecutiveWorldWarICentennialCommissionReady\n"
"ddomain3.gov,Federal,Armed Forces Retirement Home,123@mail.gov,On hold,2023-05-25\n"
"ddomain3.gov,Federal,Armed Forces Retirement Home,security@mail.gov,On hold,2023-11-15\n"
"defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,(blank),Ready\n"
"zdomain12.govInterstateReady\n"
)
Expand Down
6 changes: 3 additions & 3 deletions src/registrar/tests/test_views_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ def test_unknown_domain_does_not_show_as_expired_on_homepage(self):
self.assertContains(home_page, "DNS needed")

def test_unknown_domain_does_not_show_as_expired_on_detail_page(self):
"""An UNKNOWN domain does not show as expired on the detail page.
"""An UNKNOWN domain should not exist on the detail_page anymore.
It shows as 'DNS needed'"""
# At the time of this test's writing, there are 6 UNKNOWN domains inherited
# from constructors. Let's reset.
Expand All @@ -262,9 +262,9 @@ def test_unknown_domain_does_not_show_as_expired_on_detail_page(self):
igorville = Domain.objects.get(name="igorville.gov")
self.assertEquals(igorville.state, Domain.State.UNKNOWN)
detail_page = home_page.click("Manage", index=0)
self.assertNotContains(detail_page, "Expired")
self.assertContains(detail_page, "Expired")

self.assertContains(detail_page, "DNS needed")
self.assertNotContains(detail_page, "DNS needed")

def test_domain_detail_blocked_for_ineligible_user(self):
"""We could easily duplicate this test for all domain management
Expand Down

0 comments on commit 61e6206

Please sign in to comment.