From 523c699865602ff2fb47396f12886f5e15f9e222 Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Thu, 17 Nov 2022 07:43:53 -0600 Subject: [PATCH 1/5] Make tests a little less noisy --- src/api/tests/common.py | 49 +++++++++++++++++++++++++++++++ src/api/tests/test_available.py | 7 +++-- src/registrar/tests/common.py | 52 +++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 src/api/tests/common.py diff --git a/src/api/tests/common.py b/src/api/tests/common.py new file mode 100644 index 000000000..cdf2f8f30 --- /dev/null +++ b/src/api/tests/common.py @@ -0,0 +1,49 @@ +import os +import logging + +from contextlib import contextmanager + + +def get_handlers(): + """Obtain pointers to all StreamHandlers.""" + handlers = {} + + rootlogger = logging.getLogger() + for h in rootlogger.handlers: + if isinstance(h, logging.StreamHandler): + handlers[h.name] = h + + for logger in logging.Logger.manager.loggerDict.values(): + if not isinstance(logger, logging.PlaceHolder): + for h in logger.handlers: + if isinstance(h, logging.StreamHandler): + handlers[h.name] = h + + return handlers + + +@contextmanager +def less_console_noise(): + """ + Context manager to use in tests to silence console logging. + + This is helpful on tests which trigger console messages + (such as errors) which are normal and expected. + + It can easily be removed to debug a failing test. + """ + restore = {} + handlers = get_handlers() + devnull = open(os.devnull, "w") + + # redirect all the streams + for handler in handlers.values(): + prior = handler.setStream(devnull) + restore[handler.name] = prior + try: + # run the test + yield + finally: + # restore the streams + for handler in handlers.values(): + handler.setStream(restore[handler.name]) diff --git a/src/api/tests/test_available.py b/src/api/tests/test_available.py index 6f6e3775c..0529882f2 100644 --- a/src/api/tests/test_available.py +++ b/src/api/tests/test_available.py @@ -7,6 +7,7 @@ from django.contrib.auth import get_user_model from django.test import TestCase, RequestFactory from ..views import available, _domains, in_domains +from .common import less_console_noise API_BASE_PATH = "/api/v1/available/" @@ -104,10 +105,12 @@ class AvailableAPITest(TestCase): def test_available_post(self): """Cannot post to the /available/ API endpoint.""" - response = self.client.post(API_BASE_PATH + "nonsense") + with less_console_noise(): + response = self.client.post(API_BASE_PATH + "nonsense") self.assertEqual(response.status_code, 405) def test_available_bad_input(self): self.client.force_login(self.user) - response = self.client.get(API_BASE_PATH + "blah!;") + with less_console_noise(): + response = self.client.get(API_BASE_PATH + "blah!;") self.assertEqual(response.status_code, 400) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 3c885b948..8bda74b4e 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1,7 +1,57 @@ +import os +import logging + +from contextlib import contextmanager + from django.conf import settings from django.contrib.auth import get_user_model, login +def get_handlers(): + """Obtain pointers to all StreamHandlers.""" + handlers = {} + + rootlogger = logging.getLogger() + for h in rootlogger.handlers: + if isinstance(h, logging.StreamHandler): + handlers[h.name] = h + + for logger in logging.Logger.manager.loggerDict.values(): + if not isinstance(logger, logging.PlaceHolder): + for h in logger.handlers: + if isinstance(h, logging.StreamHandler): + handlers[h.name] = h + + return handlers + + +@contextmanager +def less_console_noise(): + """ + Context manager to use in tests to silence console logging. + + This is helpful on tests which trigger console messages + (such as errors) which are normal and expected. + + It can easily be removed to debug a failing test. + """ + restore = {} + handlers = get_handlers() + devnull = open(os.devnull, "w") + + # redirect all the streams + for handler in handlers.values(): + prior = handler.setStream(devnull) + restore[handler.name] = prior + try: + # run the test + yield + finally: + # restore the streams + for handler in handlers.values(): + handler.setStream(restore[handler.name]) + + class MockUserLogin: def __init__(self, get_response): self.get_response = get_response @@ -23,3 +73,5 @@ class MockUserLogin: response = self.get_response(request) return response + + From 2b91a3c1d1505d799d06e6223de8f27eae8f7ad1 Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Thu, 17 Nov 2022 07:50:28 -0600 Subject: [PATCH 2/5] Second revision of Domain model draft --- src/api/views.py | 8 +- src/epp/__init__.py | 0 src/epp/mock_epp.py | 40 ++++ ..._domain_host_nameserver_hostip_and_more.py | 164 +++++++++++++ src/registrar/models/__init__.py | 12 + src/registrar/models/domain.py | 221 ++++++++++++++++++ src/registrar/models/domain_application.py | 6 +- src/registrar/models/host.py | 30 +++ src/registrar/models/host_ip.py | 30 +++ src/registrar/models/nameserver.py | 12 + src/registrar/models/website.py | 23 -- src/registrar/tests/test_models.py | 48 +++- 12 files changed, 561 insertions(+), 33 deletions(-) create mode 100644 src/epp/__init__.py create mode 100644 src/epp/mock_epp.py create mode 100644 src/registrar/migrations/0002_domain_host_nameserver_hostip_and_more.py create mode 100644 src/registrar/models/domain.py create mode 100644 src/registrar/models/host.py create mode 100644 src/registrar/models/host_ip.py create mode 100644 src/registrar/models/nameserver.py diff --git a/src/api/views.py b/src/api/views.py index 042a447e3..ab9a151d6 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -11,7 +11,7 @@ import requests from cachetools.func import ttl_cache -from registrar.models import Website +from registrar.models import Domain DOMAIN_FILE_URL = ( "https://raw.githubusercontent.com/cisagov/dotgov-data/main/current-full.csv" @@ -35,7 +35,7 @@ def _domains(): # get the domain before the first comma domain = line.split(",", 1)[0] # sanity-check the string we got from the file here - if Website.string_could_be_domain(domain): + if Domain.string_could_be_domain(domain): # lowercase everything when we put it in domains domains.add(domain.lower()) return domains @@ -68,8 +68,8 @@ def available(request, domain=""): # validate that the given domain could be a domain name and fail early if # not. if not ( - Website.string_could_be_domain(domain) - or Website.string_could_be_domain(domain + ".gov") + Domain.string_could_be_domain(domain) + or Domain.string_could_be_domain(domain + ".gov") ): raise BadRequest("Invalid request.") # a domain is available if it is NOT in the list of current domains diff --git a/src/epp/__init__.py b/src/epp/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/epp/mock_epp.py b/src/epp/mock_epp.py new file mode 100644 index 000000000..8d3121d83 --- /dev/null +++ b/src/epp/mock_epp.py @@ -0,0 +1,40 @@ +""" +This file defines a number of mock functions which can be used to simulate +communication with the registry until that integration is implemented. +""" +from datetime import datetime + +def domain_check(_): + """ Is domain available for registration? """ + return True + +def domain_info(domain): + """ What does the registry know about this domain? """ + return { + "name": domain, + "roid": "EXAMPLE1-REP", + "status": ["ok"], + "registrant": "jd1234", + "contact": { + "admin": "sh8013", + "tech": None, + }, + "ns": { + f"ns1.{domain}", + f"ns2.{domain}", + }, + "host": [ + f"ns1.{domain}", + f"ns2.{domain}", + ], + "sponsor": "ClientX", + "creator": "ClientY", + # TODO: think about timezones + "creation_date": datetime.today(), + "updator": "ClientX", + "last_update_date": datetime.today(), + "expiration_date": datetime.today(), + "last_transfer_date": datetime.today(), + } + + diff --git a/src/registrar/migrations/0002_domain_host_nameserver_hostip_and_more.py b/src/registrar/migrations/0002_domain_host_nameserver_hostip_and_more.py new file mode 100644 index 000000000..b41c5908d --- /dev/null +++ b/src/registrar/migrations/0002_domain_host_nameserver_hostip_and_more.py @@ -0,0 +1,164 @@ +# Generated by Django 4.1.3 on 2022-11-17 13:46 + +from django.conf import settings +import django.core.validators +from django.db import migrations, models +import django.db.models.deletion +import django_fsm + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0001_initial"), + ] + + operations = [ + migrations.CreateModel( + name="Domain", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ( + "name", + models.CharField( + default=None, + help_text="Fully qualified domain name", + max_length=253, + ), + ), + ( + "is_active", + django_fsm.FSMField( + choices=[(True, "Yes"), (False, "No")], + default=False, + help_text="Domain is live in the registry", + max_length=50, + protected=True, + ), + ), + ("owners", models.ManyToManyField(to=settings.AUTH_USER_MODEL)), + ], + ), + migrations.CreateModel( + name="Host", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ( + "name", + models.CharField( + default=None, + help_text="Fully qualified domain name", + max_length=253, + unique=True, + ), + ), + ( + "domain", + models.ForeignKey( + help_text="Domain to which this host belongs", + on_delete=django.db.models.deletion.PROTECT, + to="registrar.domain", + ), + ), + ], + options={ + "abstract": False, + }, + ), + migrations.CreateModel( + name="Nameserver", + fields=[ + ( + "host_ptr", + models.OneToOneField( + auto_created=True, + on_delete=django.db.models.deletion.CASCADE, + parent_link=True, + primary_key=True, + serialize=False, + to="registrar.host", + ), + ), + ], + options={ + "abstract": False, + }, + bases=("registrar.host",), + ), + migrations.CreateModel( + name="HostIP", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ( + "address", + models.CharField( + default=None, + help_text="IP address", + max_length=46, + validators=[django.core.validators.validate_ipv46_address], + ), + ), + ( + "host", + models.ForeignKey( + help_text="Host to which this IP address belongs", + on_delete=django.db.models.deletion.PROTECT, + to="registrar.host", + ), + ), + ], + options={ + "abstract": False, + }, + ), + migrations.AlterField( + model_name="domainapplication", + name="requested_domain", + field=models.OneToOneField( + blank=True, + help_text="The requested domain", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="domain_application", + to="registrar.domain", + ), + ), + migrations.AddConstraint( + model_name="domain", + constraint=models.UniqueConstraint( + condition=models.Q(("is_active", True)), + fields=("name",), + name="unique_domain_name_in_registry", + ), + ), + ] diff --git a/src/registrar/models/__init__.py b/src/registrar/models/__init__.py index ef2f2e587..1bb9dde84 100644 --- a/src/registrar/models/__init__.py +++ b/src/registrar/models/__init__.py @@ -2,6 +2,10 @@ from auditlog.registry import auditlog # type: ignore from .contact import Contact from .domain_application import DomainApplication +from .domain import Domain +from .host_ip import HostIP +from .host import Host +from .nameserver import Nameserver from .user_profile import UserProfile from .user import User from .website import Website @@ -9,6 +13,10 @@ from .website import Website __all__ = [ "Contact", "DomainApplication", + "Domain", + "HostIP", + "Host", + "Nameserver", "UserProfile", "User", "Website", @@ -16,6 +24,10 @@ __all__ = [ auditlog.register(Contact) auditlog.register(DomainApplication) +auditlog.register(Domain) +auditlog.register(HostIP) +auditlog.register(Host) +auditlog.register(Nameserver) auditlog.register(UserProfile) auditlog.register(User) auditlog.register(Website) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py new file mode 100644 index 000000000..ab0ecaa28 --- /dev/null +++ b/src/registrar/models/domain.py @@ -0,0 +1,221 @@ +import logging +import re + +from django.db import models +from django_fsm import FSMField, transition # type: ignore + +from epp.mock_epp import domain_info, domain_check + +from .utility.time_stamped_model import TimeStampedModel +from .domain_application import DomainApplication +from .user import User + +logger = logging.getLogger(__name__) + + +class Domain(TimeStampedModel): + """ + Manage the lifecycle of domain names. + + The registry is the source of truth for this data and this model exists: + 1. To tie ownership information in the registrar to + DNS entries in the registry; and + 2. To allow a new registrant to draft DNS entries before their + application is approved + """ + class Meta: + constraints = [ + # draft domains may share the same name, but + # once approved, they must be globally unique + models.UniqueConstraint( + fields=['name'], + condition=models.Q(is_active=True), + name='unique_domain_name_in_registry' + ), + ] + + class Status(models.TextChoices): + """The status codes we can receive from the registry.""" + # Requests to delete the object MUST be rejected. + CLIENT_DELETE_PROHIBITED = "clientDeleteProhibited" + SERVER_DELETE_PROHIBITED = "serverDeleteProhibited" + + # DNS delegation information MUST NOT be published for the object. + CLIENT_HOLD = "clientHold" + SERVER_HOLD = "serverHold" + + # Requests to renew the object MUST be rejected. + CLIENT_RENEW_PROHIBITED = "clientRenewProhibited" + SERVER_RENEW_PROHIBITED = "serverRenewProhibited" + + # Requests to transfer the object MUST be rejected. + CLIENT_TRANSFER_PROHIBITED = "clientTransferProhibited" + SERVER_TRANSFER_PROHIBITED = "serverTransferProhibited" + + # Requests to update the object (other than to remove this status) + # MUST be rejected. + CLIENT_UPDATE_PROHIBITED = "clientUpdateProhibited" + SERVER_UPDATE_PROHIBITED = "serverUpdateProhibited" + + # Delegation information has not been associated with the object. + # This is the default status when a domain object is first created + # and there are no associated host objects for the DNS delegation. + # This status can also be set by the server when all host-object + # associations are removed. + INACTIVE = "inactive" + + # This is the normal status value for an object that has no pending + # operations or prohibitions. This value is set and removed by the + # server as other status values are added or removed. + OK = "ok" + + # A transform command has been processed for the object, but the + # action has not been completed by the server. Server operators can + # delay action completion for a variety of reasons, such as to allow + # for human review or third-party action. A transform command that + # is processed, but whose requested action is pending, is noted with + # response code 1001. + PENDING_CREATE = "pendingCreate" + PENDING_DELETE = "pendingDelete" + PENDING_RENEW = "pendingRenew" + PENDING_TRANSFER = "pendingTransfer" + PENDING_UPDATE = "pendingUpdate" + + + # a domain name is alphanumeric or hyphen, up to 63 characters, doesn't + # begin or end with a hyphen, followed by a TLD of 2-6 alphabetic characters + DOMAIN_REGEX = re.compile(r"^(?!-)[A-Za-z0-9-]{1,63}(? bool: + """Return True if the string could be a domain name, otherwise False.""" + if cls.DOMAIN_REGEX.match(domain): + return True + return False + + @classmethod + def available(cls, domain: str) -> bool: + """Check if a domain is available. Not implemented. """ + return domain_check(domain) + + def transfer(self): + """ Going somewhere. Not implemented. """ + pass + + def renew(self): + """ Time to renew. Not implemented. """ + pass + + def _get_property(self, property): + """Get some info about a domain.""" + if not self.is_active: return None + if not hasattr(self, "info"): + try: + # get info from registry + self.info = domain_info(self.name) + except Exception as e: + logger.error(e) + # TODO: back off error handling + return None + if hasattr(self, "info") and (property in self.info): + return self.info[property] + else: + return None + + def could_be_domain(self) -> bool: + """Could this instance be a domain?""" + # short-circuit if self.website is null/None + if not self.name: + return False + return self.string_could_be_domain(str(self.name)) + + @transition(field="is_active", source="*", target=True) + def activate(self): + """This domain should be made live.""" + if hasattr(self, "domain_application"): + if self.domain_application.status != DomainApplication.APPROVED: + raise ValueError("Cannot activate. Application must be approved.") + if Domain.objects.filter(name=self.name, is_active=True).exists(): + raise ValueError("Cannot activate. Domain name is already in use.") + # TODO: depending on the details of our registry integration + # we will either contact the registry and deploy the domain + # in this function OR we will verify that it has already been + # activated and reject this state transition if it has not + pass + + @transition(field="is_active", source="*", target=False) + def deactivate(self): + """This domain should not be live.""" + # there are security concerns to having this function exist + # within the codebase; discuss these with the project lead + # if there is a feature request to implement this + raise Exception("Cannot revoke, contact registry.") + + def __str__(self) -> str: + return self.name + + @property + def roid(self): + return self._get_property("roid") + + @property + def status(self): + return self._get_property("status") + + @property + def registrant(self): + return self._get_property("registrant") + + @property + def sponsor(self): + return self._get_property("sponsor") + + @property + def creator(self): + return self._get_property("creator") + + @property + def creation_date(self): + return self._get_property("creation_date") + + @property + def updator(self): + return self._get_property("updator") + + @property + def last_update_date(self): + return self._get_property("last_update_date") + + @property + def expiration_date(self): + return self._get_property("expiration_date") + + @property + def last_transfer_date(self): + return self._get_property("last_transfer_date") + + name = models.CharField( + max_length=253, + blank=False, + default=None, # prevent saving without a value + help_text="Fully qualified domain name", + ) + + # we use `is_active` rather than `domain_application.status` + # because domains may exist without associated applications + is_active = FSMField( + choices=[ + (True, "Yes"), + (False, "No"), + ], + default=False, + protected=True, + help_text="Domain is live in the registry", + ) + + # TODO: determine the relationship between this field + # and the domain application's `creator` and `submitter` + owners = models.ManyToManyField( + User, + help_text="", + ) diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 736735ace..f1d611489 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -147,12 +147,12 @@ class DomainApplication(TimeStampedModel): related_name="current+", ) - requested_domain = models.ForeignKey( - Website, + requested_domain = models.OneToOneField( + "Domain", null=True, blank=True, help_text="The requested domain", - related_name="requested+", + related_name="domain_application", on_delete=models.PROTECT, ) alternative_domains = models.ManyToManyField( diff --git a/src/registrar/models/host.py b/src/registrar/models/host.py new file mode 100644 index 000000000..0e4133a71 --- /dev/null +++ b/src/registrar/models/host.py @@ -0,0 +1,30 @@ +from django.db import models + +from .utility.time_stamped_model import TimeStampedModel +from .domain import Domain + +class Host(TimeStampedModel): + """ + Hosts are internet-connected computers. + + They may handle email, serve websites, or perform other tasks. + + The registry is the source of truth for this data. + + This model exists ONLY to allow a new registrant to draft DNS entries + before their application is approved. + """ + name = models.CharField( + max_length=253, + null=False, + blank=False, + default=None, # prevent saving without a value + unique=True, + help_text="Fully qualified domain name", + ) + + domain = models.ForeignKey( + Domain, + on_delete=models.PROTECT, + help_text="Domain to which this host belongs", + ) diff --git a/src/registrar/models/host_ip.py b/src/registrar/models/host_ip.py new file mode 100644 index 000000000..87feca14f --- /dev/null +++ b/src/registrar/models/host_ip.py @@ -0,0 +1,30 @@ +from django.db import models +from django.core.validators import validate_ipv46_address + +from .utility.time_stamped_model import TimeStampedModel +from .host import Host + +class HostIP(TimeStampedModel): + """ + Hosts may have one or more IP addresses. + + The registry is the source of truth for this data. + + This model exists ONLY to allow a new registrant to draft DNS entries + before their application is approved. + """ + address = models.CharField( + max_length=46, + null=False, + blank=False, + default=None, # prevent saving without a value + validators=[validate_ipv46_address], + help_text="IP address", + ) + + host = models.ForeignKey( + Host, + on_delete=models.PROTECT, + help_text="Host to which this IP address belongs", + ) + diff --git a/src/registrar/models/nameserver.py b/src/registrar/models/nameserver.py new file mode 100644 index 000000000..8716c0383 --- /dev/null +++ b/src/registrar/models/nameserver.py @@ -0,0 +1,12 @@ +from .host import Host + +class Nameserver(Host): + """ + A nameserver is a host which has been delegated to respond to DNS queries. + + The registry is the source of truth for this data. + + This model exists ONLY to allow a new registrant to draft DNS entries + before their application is approved. + """ + pass \ No newline at end of file diff --git a/src/registrar/models/website.py b/src/registrar/models/website.py index 83c4b8222..a0db7a2a2 100644 --- a/src/registrar/models/website.py +++ b/src/registrar/models/website.py @@ -1,5 +1,3 @@ -import re - from django.db import models @@ -16,26 +14,5 @@ class Website(models.Model): help_text="", ) - # a domain name is alphanumeric or hyphen, up to 63 characters, doesn't - # begin or end with a hyphen, followed by a TLD of 2-6 alphabetic characters - DOMAIN_REGEX = re.compile(r"^(?!-)[A-Za-z0-9-]{1,63}(? bool: - """Return True if the string could be a domain name, otherwise False. - - TODO: when we have a Domain class, this could be a classmethod there. - """ - if cls.DOMAIN_REGEX.match(domain): - return True - return False - - def could_be_domain(self) -> bool: - """Could this instance be a domain?""" - # short-circuit if self.website is null/None - if not self.website: - return False - return self.string_could_be_domain(str(self.website)) - def __str__(self) -> str: return str(self.website) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 761f25a22..a77a4a935 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1,7 +1,7 @@ from django.test import TestCase from django.db.utils import IntegrityError -from registrar.models import Contact, DomainApplication, User, Website +from registrar.models import Contact, DomainApplication, User, Website, Domain class TestDomainApplication(TestCase): @@ -22,6 +22,7 @@ class TestDomainApplication(TestCase): contact = Contact.objects.create() com_website, _ = Website.objects.get_or_create(website="igorville.com") gov_website, _ = Website.objects.get_or_create(website="igorville.gov") + domain, _ = Domain.objects.get_or_create(name="igorville.gov") application = DomainApplication.objects.create( creator=user, investigator=user, @@ -35,7 +36,7 @@ class TestDomainApplication(TestCase): state_territory="CA", zip_code="12345-6789", authorizing_official=contact, - requested_domain=gov_website, + requested_domain=domain, submitter=contact, purpose="Igorville rules!", security_email="security@igorville.gov", @@ -56,9 +57,50 @@ class TestDomainApplication(TestCase): def test_status_fsm_submit_succeed(self): user, _ = User.objects.get_or_create() - site = Website.objects.create(website="igorville.gov") + site = Domain.objects.create(name="igorville.gov") application = DomainApplication.objects.create( creator=user, requested_domain=site ) application.submit() self.assertEqual(application.status, application.SUBMITTED) + + +class TestDomain(TestCase): + def test_empty_create_fails(self): + """Can't create a completely empty domain.""" + with self.assertRaisesRegex(IntegrityError, "name"): + Domain.objects.create() + + def test_minimal_create(self): + """Can create with just a name.""" + domain = Domain.objects.create(name="igorville.gov") + self.assertEquals(domain.is_active, False) + + def test_get_status(self): + """Returns proper status based on `is_active`.""" + domain = Domain.objects.create(name="igorville.gov") + domain.save() + self.assertEquals(None, domain.status) + domain.activate() + domain.save() + self.assertIn("ok", domain.status) + + + def test_fsm_activate_fail_unique(self): + # can't activate domain if name is not unique + d1, _ = Domain.objects.get_or_create(name="igorville.gov") + d2, _ = Domain.objects.get_or_create(name="igorville.gov") + d1.activate() + d1.save() + with self.assertRaises(ValueError): + d2.activate() + + def test_fsm_activate_fail_unapproved(self): + # can't activate domain if application isn't approved + d1, _ = Domain.objects.get_or_create(name="igorville.gov") + user, _ = User.objects.get_or_create() + application = DomainApplication.objects.create(creator=user) + d1.domain_application = application + d1.save() + with self.assertRaises(ValueError): + d1.activate() From eda5e9751b4e636c34c9ff2a550ed1b7df8c7bec Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Thu, 17 Nov 2022 08:26:41 -0600 Subject: [PATCH 3/5] Fix lint errors --- src/epp/mock_epp.py | 8 ++++---- src/registrar/forms/application_wizard.py | 6 +++--- ...2_domain_host_nameserver_hostip_and_more.py | 2 +- src/registrar/models/domain.py | 18 ++++++++++-------- src/registrar/models/host.py | 6 ++++-- src/registrar/models/host_ip.py | 3 ++- src/registrar/models/nameserver.py | 4 +++- src/registrar/tests/common.py | 2 -- src/registrar/tests/test_models.py | 1 - 9 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/epp/mock_epp.py b/src/epp/mock_epp.py index 8d3121d83..7d1d1d84e 100644 --- a/src/epp/mock_epp.py +++ b/src/epp/mock_epp.py @@ -4,12 +4,14 @@ communication with the registry until that integration is implemented. """ from datetime import datetime + def domain_check(_): - """ Is domain available for registration? """ + """Is domain available for registration?""" return True + def domain_info(domain): - """ What does the registry know about this domain? """ + """What does the registry know about this domain?""" return { "name": domain, "roid": "EXAMPLE1-REP", @@ -36,5 +38,3 @@ def domain_info(domain): "expiration_date": datetime.today(), "last_transfer_date": datetime.today(), } - - diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 90ef2c958..0cf0eb6e0 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -9,7 +9,7 @@ from django.contrib.auth.mixins import LoginRequiredMixin from formtools.wizard.views import NamedUrlSessionWizardView # type: ignore -from registrar.models import DomainApplication, Website +from registrar.models import DomainApplication, Domain logger = logging.getLogger(__name__) @@ -134,8 +134,8 @@ class ApplicationWizard(LoginRequiredMixin, NamedUrlSessionWizardView): # This isn't really the requested_domain field # but we need something in this field to make the form submittable - requested_site, _ = Website.objects.get_or_create( - website=contact_data["organization_name"] + ".gov" + requested_site, _ = Domain.objects.get_or_create( + name=contact_data["organization_name"] + ".gov" ) application.requested_domain = requested_site return application diff --git a/src/registrar/migrations/0002_domain_host_nameserver_hostip_and_more.py b/src/registrar/migrations/0002_domain_host_nameserver_hostip_and_more.py index b41c5908d..fb00d0a22 100644 --- a/src/registrar/migrations/0002_domain_host_nameserver_hostip_and_more.py +++ b/src/registrar/migrations/0002_domain_host_nameserver_hostip_and_more.py @@ -4,7 +4,7 @@ from django.conf import settings import django.core.validators from django.db import migrations, models import django.db.models.deletion -import django_fsm +import django_fsm # type: ignore class Migration(migrations.Migration): diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index ab0ecaa28..b5a2255c7 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -23,19 +23,21 @@ class Domain(TimeStampedModel): 2. To allow a new registrant to draft DNS entries before their application is approved """ + class Meta: constraints = [ # draft domains may share the same name, but # once approved, they must be globally unique models.UniqueConstraint( - fields=['name'], + fields=["name"], condition=models.Q(is_active=True), - name='unique_domain_name_in_registry' + name="unique_domain_name_in_registry", ), ] class Status(models.TextChoices): """The status codes we can receive from the registry.""" + # Requests to delete the object MUST be rejected. CLIENT_DELETE_PROHIBITED = "clientDeleteProhibited" SERVER_DELETE_PROHIBITED = "serverDeleteProhibited" @@ -81,7 +83,6 @@ class Domain(TimeStampedModel): PENDING_TRANSFER = "pendingTransfer" PENDING_UPDATE = "pendingUpdate" - # a domain name is alphanumeric or hyphen, up to 63 characters, doesn't # begin or end with a hyphen, followed by a TLD of 2-6 alphabetic characters DOMAIN_REGEX = re.compile(r"^(?!-)[A-Za-z0-9-]{1,63}(? bool: - """Check if a domain is available. Not implemented. """ + """Check if a domain is available. Not implemented.""" return domain_check(domain) def transfer(self): - """ Going somewhere. Not implemented. """ + """Going somewhere. Not implemented.""" pass def renew(self): - """ Time to renew. Not implemented. """ + """Time to renew. Not implemented.""" pass def _get_property(self, property): """Get some info about a domain.""" - if not self.is_active: return None + if not self.is_active: + return None if not hasattr(self, "info"): try: # get info from registry @@ -194,7 +196,7 @@ class Domain(TimeStampedModel): def last_transfer_date(self): return self._get_property("last_transfer_date") - name = models.CharField( + name = models.CharField( max_length=253, blank=False, default=None, # prevent saving without a value diff --git a/src/registrar/models/host.py b/src/registrar/models/host.py index 0e4133a71..132e72792 100644 --- a/src/registrar/models/host.py +++ b/src/registrar/models/host.py @@ -3,10 +3,11 @@ from django.db import models from .utility.time_stamped_model import TimeStampedModel from .domain import Domain + class Host(TimeStampedModel): """ Hosts are internet-connected computers. - + They may handle email, serve websites, or perform other tasks. The registry is the source of truth for this data. @@ -14,7 +15,8 @@ class Host(TimeStampedModel): This model exists ONLY to allow a new registrant to draft DNS entries before their application is approved. """ - name = models.CharField( + + name = models.CharField( max_length=253, null=False, blank=False, diff --git a/src/registrar/models/host_ip.py b/src/registrar/models/host_ip.py index 87feca14f..c6aa1a3c8 100644 --- a/src/registrar/models/host_ip.py +++ b/src/registrar/models/host_ip.py @@ -4,6 +4,7 @@ from django.core.validators import validate_ipv46_address from .utility.time_stamped_model import TimeStampedModel from .host import Host + class HostIP(TimeStampedModel): """ Hosts may have one or more IP addresses. @@ -13,6 +14,7 @@ class HostIP(TimeStampedModel): This model exists ONLY to allow a new registrant to draft DNS entries before their application is approved. """ + address = models.CharField( max_length=46, null=False, @@ -27,4 +29,3 @@ class HostIP(TimeStampedModel): on_delete=models.PROTECT, help_text="Host to which this IP address belongs", ) - diff --git a/src/registrar/models/nameserver.py b/src/registrar/models/nameserver.py index 8716c0383..913b3ab03 100644 --- a/src/registrar/models/nameserver.py +++ b/src/registrar/models/nameserver.py @@ -1,5 +1,6 @@ from .host import Host + class Nameserver(Host): """ A nameserver is a host which has been delegated to respond to DNS queries. @@ -9,4 +10,5 @@ class Nameserver(Host): This model exists ONLY to allow a new registrant to draft DNS entries before their application is approved. """ - pass \ No newline at end of file + + pass diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 8bda74b4e..b6c7c01d8 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -73,5 +73,3 @@ class MockUserLogin: response = self.get_response(request) return response - - diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index a77a4a935..78d4cb196 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -85,7 +85,6 @@ class TestDomain(TestCase): domain.save() self.assertIn("ok", domain.status) - def test_fsm_activate_fail_unique(self): # can't activate domain if name is not unique d1, _ = Domain.objects.get_or_create(name="igorville.gov") From 6fd29aea75a93e30a031fbbdccd2687134763313 Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Mon, 28 Nov 2022 11:49:01 -0600 Subject: [PATCH 4/5] Respond to PR feedback --- docs/developer/README.md | 15 ++++++++ src/registrar/models/domain.py | 21 ++++++++--- src/registrar/models/host.py | 1 + src/registrar/models/host_ip.py | 1 + src/registrar/models/nameserver.py | 2 ++ src/registrar/tests/test_models.py | 57 ++++++++++++++++++++++++++++-- 6 files changed, 91 insertions(+), 6 deletions(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index d58dd31ad..cd33aeea6 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -78,6 +78,21 @@ To test behind logged in pages with external tools, like `pa11y-ci` or `OWASP Za to MIDDLEWARE in settings.py. **Remove it when you are finished testing.** +### Reducing console noise in tests + +Some tests, particularly when using Django's test client, will print errors. + +These errors do not indicate test failure, but can make the output hard to read. + +To silence them, we have a helper function `less_console_noise`: + +```python +from .common import less_console_noise +... + with less_console_noise(): + # +``` + ### Accessibility Scanning The tool `pa11y-ci` is used to scan pages for compliance with a set of diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index b5a2255c7..12a8507f7 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -36,7 +36,12 @@ class Domain(TimeStampedModel): ] class Status(models.TextChoices): - """The status codes we can receive from the registry.""" + """ + The status codes we can receive from the registry. + + These are detailed in RFC 5731 in section 2.3. + https://www.rfc-editor.org/std/std69.txt + """ # Requests to delete the object MUST be rejected. CLIENT_DELETE_PROHIBITED = "clientDeleteProhibited" @@ -96,7 +101,9 @@ class Domain(TimeStampedModel): @classmethod def available(cls, domain: str) -> bool: - """Check if a domain is available. Not implemented.""" + """Check if a domain is available. + + Not implemented. Returns a dummy value for testing.""" return domain_check(domain) def transfer(self): @@ -119,9 +126,15 @@ class Domain(TimeStampedModel): logger.error(e) # TODO: back off error handling return None - if hasattr(self, "info") and (property in self.info): - return self.info[property] + if hasattr(self, "info"): + if property in self.info: + return self.info[property] + else: + raise KeyError( + "Requested key %s was not found in registry data." % str(property) + ) else: + # TODO: return an error if registry cannot be contacted return None def could_be_domain(self) -> bool: diff --git a/src/registrar/models/host.py b/src/registrar/models/host.py index 132e72792..23f6c7659 100644 --- a/src/registrar/models/host.py +++ b/src/registrar/models/host.py @@ -28,5 +28,6 @@ class Host(TimeStampedModel): domain = models.ForeignKey( Domain, on_delete=models.PROTECT, + related_name="host", # access this Host via the Domain as `domain.host` help_text="Domain to which this host belongs", ) diff --git a/src/registrar/models/host_ip.py b/src/registrar/models/host_ip.py index c6aa1a3c8..22847b77c 100644 --- a/src/registrar/models/host_ip.py +++ b/src/registrar/models/host_ip.py @@ -27,5 +27,6 @@ class HostIP(TimeStampedModel): host = models.ForeignKey( Host, on_delete=models.PROTECT, + related_name="ip", # access this HostIP via the Host as `host.ip` help_text="Host to which this IP address belongs", ) diff --git a/src/registrar/models/nameserver.py b/src/registrar/models/nameserver.py index 913b3ab03..13295f5b5 100644 --- a/src/registrar/models/nameserver.py +++ b/src/registrar/models/nameserver.py @@ -11,4 +11,6 @@ class Nameserver(Host): before their application is approved. """ + # there is nothing here because all of the fields are + # defined over there on the Host class pass diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 78d4cb196..535ab9360 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -2,6 +2,7 @@ from django.test import TestCase from django.db.utils import IntegrityError from registrar.models import Contact, DomainApplication, User, Website, Domain +from unittest import skip class TestDomainApplication(TestCase): @@ -86,7 +87,7 @@ class TestDomain(TestCase): self.assertIn("ok", domain.status) def test_fsm_activate_fail_unique(self): - # can't activate domain if name is not unique + """Can't activate domain if name is not unique.""" d1, _ = Domain.objects.get_or_create(name="igorville.gov") d2, _ = Domain.objects.get_or_create(name="igorville.gov") d1.activate() @@ -95,7 +96,7 @@ class TestDomain(TestCase): d2.activate() def test_fsm_activate_fail_unapproved(self): - # can't activate domain if application isn't approved + """Can't activate domain if application isn't approved.""" d1, _ = Domain.objects.get_or_create(name="igorville.gov") user, _ = User.objects.get_or_create() application = DomainApplication.objects.create(creator=user) @@ -103,3 +104,55 @@ class TestDomain(TestCase): d1.save() with self.assertRaises(ValueError): d1.activate() + + +@skip("Not implemented yet.") +class TestDomainApplicationLifeCycle(TestCase): + def test_application_approval(self): + # DomainApplication is created + # test: Domain is created and is inactive + # analyst approves DomainApplication + # test: Domain is activated + pass + + def test_application_rejection(self): + # DomainApplication is created + # test: Domain is created and is inactive + # analyst rejects DomainApplication + # test: Domain remains inactive + pass + + def test_application_deleted_before_approval(self): + # DomainApplication is created + # test: Domain is created and is inactive + # admin deletes DomainApplication + # test: Domain is deleted; Hosts, HostIps and Nameservers are deleted + pass + + def test_application_deleted_following_approval(self): + # DomainApplication is created + # test: Domain is created and is inactive + # analyst approves DomainApplication + # admin deletes DomainApplication + # test: DomainApplication foreign key field on Domain is set to null + pass + + def test_application_approval_with_conflicting_name(self): + # DomainApplication #1 is created + # test: Domain #1 is created and is inactive + # analyst approves DomainApplication #1 + # test: Domain #1 is activated + # DomainApplication #2 is created, with the same domain name string + # test: Domain #2 is created and is inactive + # analyst approves DomainApplication #2 + # test: error is raised + # test: DomainApplication #1 remains approved + # test: Domain #1 remains active + # test: DomainApplication #2 remains in investigating + # test: Domain #2 remains inactive + pass + + def test_application_approval_with_network_errors(self): + # TODO: scenario wherein application is approved, + # but attempts to contact the registry to activate the domain fail + pass From 54b6b525b5a71b3b895d32075d341218ee6d627c Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Mon, 28 Nov 2022 13:02:19 -0600 Subject: [PATCH 5/5] Fix failing tests --- src/registrar/admin.py | 28 +++++++++++++++---- ..._domain_host_nameserver_hostip_and_more.py | 5 ++-- src/registrar/models/domain.py | 3 +- src/registrar/models/domain_application.py | 4 +-- src/registrar/templates/home.html | 2 +- src/registrar/tests/test_views.py | 9 ++++-- 6 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 0ffcaaedc..19cf60729 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -4,7 +4,7 @@ from django.contrib.contenttypes.models import ContentType from django.http.response import HttpResponseRedirect from django.urls import reverse -from .models import User, UserProfile, DomainApplication, Website +from . import models class AuditedAdmin(admin.ModelAdmin): @@ -26,7 +26,7 @@ class UserProfileInline(admin.StackedInline): """Edit a user's profile on the user page.""" - model = UserProfile + model = models.UserProfile class MyUserAdmin(UserAdmin): @@ -36,6 +36,24 @@ class MyUserAdmin(UserAdmin): inlines = [UserProfileInline] -admin.site.register(User, MyUserAdmin) -admin.site.register(DomainApplication, AuditedAdmin) -admin.site.register(Website, AuditedAdmin) +class HostIPInline(admin.StackedInline): + + """Edit an ip address on the host page.""" + + model = models.HostIP + + +class MyHostAdmin(AuditedAdmin): + + """Custom host admin class to use our inlines.""" + + inlines = [HostIPInline] + + +admin.site.register(models.User, MyUserAdmin) +admin.site.register(models.Contact, AuditedAdmin) +admin.site.register(models.DomainApplication, AuditedAdmin) +admin.site.register(models.Domain, AuditedAdmin) +admin.site.register(models.Host, MyHostAdmin) +admin.site.register(models.Nameserver, MyHostAdmin) +admin.site.register(models.Website, AuditedAdmin) diff --git a/src/registrar/migrations/0002_domain_host_nameserver_hostip_and_more.py b/src/registrar/migrations/0002_domain_host_nameserver_hostip_and_more.py index fb00d0a22..738deabe5 100644 --- a/src/registrar/migrations/0002_domain_host_nameserver_hostip_and_more.py +++ b/src/registrar/migrations/0002_domain_host_nameserver_hostip_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.1.3 on 2022-11-17 13:46 +# Generated by Django 4.1.3 on 2022-11-28 19:07 from django.conf import settings import django.core.validators @@ -43,7 +43,6 @@ class Migration(migrations.Migration): default=False, help_text="Domain is live in the registry", max_length=50, - protected=True, ), ), ("owners", models.ManyToManyField(to=settings.AUTH_USER_MODEL)), @@ -77,6 +76,7 @@ class Migration(migrations.Migration): models.ForeignKey( help_text="Domain to which this host belongs", on_delete=django.db.models.deletion.PROTECT, + related_name="host", to="registrar.domain", ), ), @@ -133,6 +133,7 @@ class Migration(migrations.Migration): models.ForeignKey( help_text="Host to which this IP address belongs", on_delete=django.db.models.deletion.PROTECT, + related_name="ip", to="registrar.host", ), ), diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 12a8507f7..e834df5d7 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -224,7 +224,8 @@ class Domain(TimeStampedModel): (False, "No"), ], default=False, - protected=True, + # TODO: how to edit models in Django admin if protected = True + protected=False, help_text="Domain is live in the registry", ) diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 05fa051cf..c159c0474 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -211,8 +211,8 @@ class DomainApplication(TimeStampedModel): def __str__(self): try: - if self.requested_domain and self.requested_domain.website: - return self.requested_domain.website + if self.requested_domain and self.requested_domain.name: + return self.requested_domain.name else: return f"{self.status} application created by {self.creator}" except Exception: diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index a4780c1c5..2010d87a2 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -32,7 +32,7 @@ {% for application in domain_applications %} - {{ application.requested_domain.website }} + {{ application.requested_domain.name }} {{ application.status }} {% endfor %} diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 96128b389..384b30014 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -5,9 +5,11 @@ from django.contrib.auth import get_user_model from django_webtest import WebTest # type: ignore -from registrar.models import DomainApplication, Website +from registrar.models import DomainApplication, Domain from registrar.forms.application_wizard import TITLES +from .common import less_console_noise + class TestViews(TestCase): def setUp(self): @@ -58,7 +60,7 @@ class LoggedInTests(TestWithUser): def test_home_lists_domain_applications(self): response = self.client.get("/") self.assertNotContains(response, "igorville.gov") - site = Website.objects.create(website="igorville.gov") + site = Domain.objects.create(name="igorville.gov") application = DomainApplication.objects.create( creator=self.user, requested_domain=site ) @@ -307,7 +309,8 @@ class FormTests(TestWithUser, WebTest): # following this redirect is a GET request, so include the cookie # here too. self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - final_result = review_result.follow() + with less_console_noise(): + final_result = review_result.follow() self.assertContains(final_result, "Thank you for your domain request") def test_application_form_conditional_federal(self):