From 16164f1f05d0db168701da60fb3765458eb6a9b6 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 18 Oct 2023 15:01:15 -0400 Subject: [PATCH 01/41] work in progress --- src/registrar/forms/domain.py | 3 ++- src/registrar/models/domain.py | 5 +++- .../templates/domain_nameservers.html | 27 ++++++++++++------- src/registrar/views/domain.py | 8 ++++-- 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 8abc7e14a..d14ed41ba 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -25,7 +25,8 @@ class DomainNameserverForm(forms.Form): """Form for changing nameservers.""" server = forms.CharField(label="Name server", strip=True) - # when adding IPs to this form ensure they are stripped as well + + ip = forms.CharField(label="IP address", strip=True, required=False) NameserverFormset = formset_factory( diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index bab993b04..80bb1ab30 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -322,6 +322,7 @@ class Domain(TimeStampedModel, DomainHelper): ) elif ip is not None and ip != []: for addr in ip: + logger.info(f"ip address {addr}") if not self._valid_ip_addr(addr): raise NameserverError( code=nsErrorCodes.INVALID_IP, nameserver=nameserver, ip=ip @@ -334,7 +335,9 @@ class Domain(TimeStampedModel, DomainHelper): returns: isValid (boolean)-True for valid ip address""" try: + logger.info(f"in valid_ip_addr: {ipToTest}") ip = ipaddress.ip_address(ipToTest) + logger.info(ip.version) return ip.version == 6 or ip.version == 4 except ValueError: @@ -602,7 +605,7 @@ class Domain(TimeStampedModel, DomainHelper): if len(hosts) > 13: raise NameserverError(code=nsErrorCodes.TOO_MANY_HOSTS) - if self.state not in [self.State.DNS_NEEDED, self.State.READY]: + if self.state not in [self.State.DNS_NEEDED, self.State.READY, self.State.UNKNOWN]: raise ActionNotAllowed("Nameservers can not be " "set in the current state") logger.info("Setting nameservers") diff --git a/src/registrar/templates/domain_nameservers.html b/src/registrar/templates/domain_nameservers.html index 596eec524..37fa1eb85 100644 --- a/src/registrar/templates/domain_nameservers.html +++ b/src/registrar/templates/domain_nameservers.html @@ -22,15 +22,24 @@ {% for form in formset %}
- {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" %} - {% if forloop.counter <= 2 %} - {% with attr_required=True %} - {% input_with_errors form.server %} - {% endwith %} - {% else %} - {% input_with_errors form.server %} - {% endif %} - {% endwith %} +
+
+ {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" %} + {% if forloop.counter <= 2 %} + {% with attr_required=True %} + {% input_with_errors form.server %} + {% endwith %} + {% else %} + {% input_with_errors form.server %} + {% endif %} + {% endwith %} +
+
+ {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" %} + {% input_with_errors form.ip %} + {% endwith %} +
+
{% endfor %} diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 36b7a9445..dbf65fe6b 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -173,7 +173,7 @@ class DomainNameserversView(DomainPermissionView, FormMixin): if nameservers is not None: # Add existing nameservers as initial data - initial_data.extend({"server": name} for name, *ip in nameservers) + initial_data.extend({"server": name, "ip": ip} for name, ip in nameservers) # Ensure at least 3 fields, filled or empty while len(initial_data) < 2: @@ -198,6 +198,7 @@ class DomainNameserversView(DomainPermissionView, FormMixin): for i, form in enumerate(formset): form.fields["server"].label += f" {i+1}" + form.fields["ip"].label += f" {i+1}" if i < 2: form.fields["server"].required = True else: @@ -221,7 +222,10 @@ class DomainNameserversView(DomainPermissionView, FormMixin): nameservers = [] for form in formset: try: - as_tuple = (form.cleaned_data["server"],) + as_tuple = ( + form.cleaned_data["server"], + [form.cleaned_data["ip"]] + ) nameservers.append(as_tuple) except KeyError: # no server information in this field, skip it From 51307d3b12bd1c8c27aeb5600e768d4573135c55 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 18 Oct 2023 15:36:41 -0400 Subject: [PATCH 02/41] some error validation, light refactor of ip checking in model --- src/registrar/forms/domain.py | 9 ++++- src/registrar/models/domain.py | 6 +-- .../templates/domain_nameservers.html | 6 +-- src/registrar/views/domain.py | 37 ++++++++++++++++--- 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index d14ed41ba..b93950df0 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -26,7 +26,14 @@ class DomainNameserverForm(forms.Form): server = forms.CharField(label="Name server", strip=True) - ip = forms.CharField(label="IP address", strip=True, required=False) + ip = forms.CharField( + label="IP address", + strip=True, + required=False, + validators=[ + # TODO in progress + ], + ) NameserverFormset = formset_factory( diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 80bb1ab30..0b9139277 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -313,14 +313,14 @@ class Domain(TimeStampedModel, DomainHelper): NameserverError (if exception hit) Returns: None""" - if self.isSubdomain(nameserver) and (ip is None or ip == []): + if self.isSubdomain(nameserver) and (ip is None or ip == [] or ip != []): raise NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver) - elif not self.isSubdomain(nameserver) and (ip is not None and ip != []): + elif not self.isSubdomain(nameserver) and (ip is not None and ip != [] and ip != ['']): raise NameserverError( code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, nameserver=nameserver, ip=ip ) - elif ip is not None and ip != []: + elif ip is not None and ip != [] and ip != ['']: for addr in ip: logger.info(f"ip address {addr}") if not self._valid_ip_addr(addr): diff --git a/src/registrar/templates/domain_nameservers.html b/src/registrar/templates/domain_nameservers.html index 37fa1eb85..fffd4b8c0 100644 --- a/src/registrar/templates/domain_nameservers.html +++ b/src/registrar/templates/domain_nameservers.html @@ -16,7 +16,7 @@ {% include "includes/required_fields.html" %} -
+ {% csrf_token %} {{ formset.management_form }} @@ -26,7 +26,7 @@
{% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" %} {% if forloop.counter <= 2 %} - {% with attr_required=True %} + {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} {% input_with_errors form.server %} {% endwith %} {% else %} @@ -35,7 +35,7 @@ {% endwith %}
- {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" %} + {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" add_group_class="usa-form-group--unstyled-error" %} {% input_with_errors form.ip %} {% endwith %}
diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index dbf65fe6b..f8e144f7f 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -23,6 +23,7 @@ from registrar.models import ( UserDomainRole, ) from registrar.models.public_contact import PublicContact +from registrar.utility.errors import NameserverError from ..forms import ( ContactForm, @@ -222,20 +223,44 @@ class DomainNameserversView(DomainPermissionView, FormMixin): nameservers = [] for form in formset: try: + ip_string = form.cleaned_data["ip"] + # Split the string into a list using a comma as the delimiter + ip_list = ip_string.split(',') + # Remove any leading or trailing whitespace from each IP in the list + # this will return [''] if no ips have been entered, which is taken + # into account in the model in checkHostIPCombo + ip_list = [ip.strip() for ip in ip_list] + as_tuple = ( form.cleaned_data["server"], - [form.cleaned_data["ip"]] + ip_list, ) nameservers.append(as_tuple) except KeyError: # no server information in this field, skip it pass domain = self.get_object() - domain.nameservers = nameservers - - messages.success( - self.request, "The name servers for this domain have been updated." - ) + + try: + domain.nameservers = nameservers + except NameserverError as Err: + # TODO: move into literal + messages.error(self.request, 'Whoops, Nameservers Error') + # messages.error(self.request, GENERIC_ERROR) + logger.error(f"Nameservers error: {Err}") + # TODO: registry is not throwing an error when no connection + # TODO: merge 1103 and use literals + except RegistryError as Err: + 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}") + else: + messages.success( + self.request, "The name servers for this domain have been updated." + ) # superclass has the redirect return super().form_valid(formset) From 5e316adfd494329a1b593ed04d7c596d1024f6b8 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 18 Oct 2023 15:57:20 -0400 Subject: [PATCH 03/41] fix typo in check for isSubdomain --- src/registrar/models/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 0b9139277..c4c38774d 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -313,7 +313,7 @@ class Domain(TimeStampedModel, DomainHelper): NameserverError (if exception hit) Returns: None""" - if self.isSubdomain(nameserver) and (ip is None or ip == [] or ip != []): + if self.isSubdomain(nameserver) and (ip is None or ip == [] or ip == ['']): raise NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver) elif not self.isSubdomain(nameserver) and (ip is not None and ip != [] and ip != ['']): From 1a01fdc98d162407cce4106338f254dd2f3d51d4 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 18 Oct 2023 16:30:54 -0400 Subject: [PATCH 04/41] merge --- src/registrar/views/domain.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 81ab32b80..d743d9d9e 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -23,11 +23,8 @@ from registrar.models import ( UserDomainRole, ) from registrar.models.public_contact import PublicContact -<<<<<<< HEAD from registrar.utility.errors import NameserverError -======= from registrar.models.utility.contact_error import ContactError ->>>>>>> main from ..forms import ( ContactForm, @@ -284,11 +281,9 @@ class DomainNameserversView(DomainFormBaseView): except KeyError: # no server information in this field, skip it pass -<<<<<<< HEAD - domain = self.get_object() try: - domain.nameservers = nameservers + self.object.nameservers = nameservers except NameserverError as Err: # TODO: move into literal messages.error(self.request, 'Whoops, Nameservers Error') @@ -307,13 +302,6 @@ class DomainNameserversView(DomainFormBaseView): messages.success( self.request, "The name servers for this domain have been updated." ) -======= - self.object.nameservers = nameservers - - messages.success( - self.request, "The name servers for this domain have been updated." - ) ->>>>>>> main # superclass has the redirect return super().form_valid(formset) From 8dba1234c1ba8d918eeea16fac5ad1543adab045 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 18 Oct 2023 18:54:01 -0400 Subject: [PATCH 05/41] wip on validation in form --- src/registrar/forms/domain.py | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index b93950df0..416d265ef 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -5,8 +5,9 @@ from django.core.validators import MinValueValidator, MaxValueValidator, RegexVa from django.forms import formset_factory from phonenumber_field.widgets import RegionalPhoneNumberWidget +from registrar.utility.errors import NameserverError -from ..models import Contact, DomainInformation +from ..models import Contact, DomainInformation, Domain from .common import ( ALGORITHM_CHOICES, DIGEST_TYPE_CHOICES, @@ -21,18 +22,35 @@ class DomainAddUserForm(forms.Form): email = forms.EmailField(label="Email") +class IPAddressField(forms.CharField): + # def __init__(self, server_value, *args, **kwargs): + # self.server_value = server_value + # super().__init__(*args, **kwargs) + + def validate(self, value): + super().validate(value) # Run the default CharField validation + + ip_list = [ip.strip() for ip in value.split(",")] # Split IPs and remove whitespace + + # TODO: pass hostname from view? + hostname = "" + + # Call the IP validation method from Domain + try: + Domain.checkHostIPCombo(hostname, ip_list) + except NameserverError as e: + raise forms.ValidationError(str(e)) + + class DomainNameserverForm(forms.Form): """Form for changing nameservers.""" server = forms.CharField(label="Name server", strip=True) - ip = forms.CharField( - label="IP address", - strip=True, + ip = IPAddressField( + label="IP address", + strip=True, required=False, - validators=[ - # TODO in progress - ], ) From b4596d3bcd31e966b0b2d6b7048b6a10344bd7e1 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 19 Oct 2023 02:55:02 -0400 Subject: [PATCH 06/41] wip making view communicate data with form --- src/registrar/forms/domain.py | 77 +++++++++++++++++++++++++++++------ src/registrar/views/domain.py | 16 ++++++-- 2 files changed, 77 insertions(+), 16 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 416d265ef..fe6a879e8 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -23,23 +23,25 @@ class DomainAddUserForm(forms.Form): class IPAddressField(forms.CharField): - # def __init__(self, server_value, *args, **kwargs): - # self.server_value = server_value - # super().__init__(*args, **kwargs) + + - def validate(self, value): + def validate(self, value): super().validate(value) # Run the default CharField validation - ip_list = [ip.strip() for ip in value.split(",")] # Split IPs and remove whitespace + # ip_list = [ip.strip() for ip in value.split(",")] # Split IPs and remove whitespace - # TODO: pass hostname from view? - hostname = "" + # # TODO: pass hostname from view? - # Call the IP validation method from Domain - try: - Domain.checkHostIPCombo(hostname, ip_list) - except NameserverError as e: - raise forms.ValidationError(str(e)) + # hostname = self.form.cleaned_data.get("server", "") + + # print(f"hostname {hostname}") + + # # Call the IP validation method from Domain + # try: + # Domain.checkHostIPCombo(hostname, ip_list) + # except NameserverError as e: + # raise forms.ValidationError(str(e)) class DomainNameserverForm(forms.Form): @@ -51,7 +53,58 @@ class DomainNameserverForm(forms.Form): label="IP address", strip=True, required=False, + # validators=[ + # django.core.validators.validate_ipv46_address + # ], ) + + def __init__(self, *args, **kwargs): + # Access the context object passed to the form + print(f"domain domain domain {kwargs}") + self.domain = kwargs.pop('rachid', None) + print(f"domain domain domain {kwargs}") + super().__init__(*args, **kwargs) + + # def __init__(self, request, *args, **kwargs): + # # Pass the request object to the form during initialization + # self.request = request + # super().__init__(*args, **kwargs) + + def clean(self): + cleaned_data = super().clean() + server = cleaned_data.get('server', '') + ip = cleaned_data.get('ip', '') + + # make sure there's a nameserver if an ip is passed + if ip: + ip_list = [ip.strip() for ip in ip.split(",")] + if not server: + # If 'server' is empty, disallow 'ip' input + raise forms.ValidationError("Name server must be provided to enter IP address.") + try: + Domain.checkHostIPCombo(server, ip_list) + except NameserverError as e: + raise forms.ValidationError(str(e)) + + # if there's a nameserver, validate nameserver/ip combo + domain, _ = Domain.objects.get_or_create(name="magazine-claim.gov") + + # Access session data from the request object + # session_data = self.request.session.get('nameservers_form_domain', None) + + print(f"domain domain domain {self.domain}") + + if server: + if ip: + ip_list = [ip.strip() for ip in ip.split(",")] + else: + ip_list = [""] + try: + Domain.checkHostIPCombo(domain, server, ip_list) + except NameserverError as e: + raise forms.ValidationError(str(e)) + + return cleaned_data NameserverFormset = formset_factory( diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index d743d9d9e..8c6fa5910 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.forms import BaseFormSet from registrar.models import ( Domain, @@ -213,12 +214,18 @@ class DomainDNSView(DomainBaseView): template_name = "domain_dns.html" -class DomainNameserversView(DomainFormBaseView): +class DomainNameserversView(DomainFormBaseView, BaseFormSet): """Domain nameserver editing view.""" template_name = "domain_nameservers.html" form_class = NameserverFormset - + + def get_formset_kwargs(self, index): + kwargs = super().get_formset_kwargs(index) + # kwargs['domain'] = self.object # Pass the context data + kwargs.update({"domain", self.object}) + return kwargs + def get_initial(self): """The initial value for the form (which is a formset here).""" nameservers = self.object.nameservers @@ -247,8 +254,7 @@ class DomainNameserversView(DomainFormBaseView): def get_form(self, **kwargs): """Override the labels and required fields every time we get a formset.""" - formset = super().get_form(**kwargs) - + formset = super().get_form(**kwargs) for i, form in enumerate(formset): form.fields["server"].label += f" {i+1}" form.fields["ip"].label += f" {i+1}" @@ -261,6 +267,8 @@ class DomainNameserversView(DomainFormBaseView): def form_valid(self, formset): """The formset is valid, perform something with it.""" + self.request.session['nameservers_form_domain'] = self.object + # Set the nameservers from the formset nameservers = [] for form in formset: From d48312f74b4f51e8e9e57170c7f0e9084e1550d5 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 19 Oct 2023 13:24:35 -0400 Subject: [PATCH 07/41] wip passing domain from view to form --- src/registrar/forms/domain.py | 26 ++++++++++---------------- src/registrar/views/domain.py | 8 +++++++- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index fe6a879e8..dbee3fa4d 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -60,9 +60,8 @@ class DomainNameserverForm(forms.Form): def __init__(self, *args, **kwargs): # Access the context object passed to the form - print(f"domain domain domain {kwargs}") - self.domain = kwargs.pop('rachid', None) - print(f"domain domain domain {kwargs}") + print(f"kwargs in __init__ {kwargs}") + self.domain = kwargs.pop('domain', None) super().__init__(*args, **kwargs) # def __init__(self, request, *args, **kwargs): @@ -78,27 +77,20 @@ class DomainNameserverForm(forms.Form): # make sure there's a nameserver if an ip is passed if ip: ip_list = [ip.strip() for ip in ip.split(",")] - if not server: + if len(server) < len(ip_list): # If 'server' is empty, disallow 'ip' input raise forms.ValidationError("Name server must be provided to enter IP address.") - try: - Domain.checkHostIPCombo(server, ip_list) - except NameserverError as e: - raise forms.ValidationError(str(e)) - # if there's a nameserver, validate nameserver/ip combo - domain, _ = Domain.objects.get_or_create(name="magazine-claim.gov") + # if there's a nameserver and an ip, validate nameserver/ip combo + domain, _ = Domain.objects.get_or_create(name="realize-shake-too.gov") # Access session data from the request object # session_data = self.request.session.get('nameservers_form_domain', None) - print(f"domain domain domain {self.domain}") + print(f"domain in clean: {self.domain}") - if server: - if ip: - ip_list = [ip.strip() for ip in ip.split(",")] - else: - ip_list = [""] + if server and ip: + ip_list = [ip.strip() for ip in ip.split(",")] try: Domain.checkHostIPCombo(domain, server, ip_list) except NameserverError as e: @@ -110,6 +102,8 @@ class DomainNameserverForm(forms.Form): NameserverFormset = formset_factory( DomainNameserverForm, extra=1, + max_num=13, + validate_max=True, ) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 8c6fa5910..f82326a67 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -219,11 +219,13 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): template_name = "domain_nameservers.html" form_class = NameserverFormset + model = Domain def get_formset_kwargs(self, index): kwargs = super().get_formset_kwargs(index) # kwargs['domain'] = self.object # Pass the context data kwargs.update({"domain", self.object}) + print(f"kwargs in get_formset_kwargs {kwargs}") return kwargs def get_initial(self): @@ -254,8 +256,12 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): def get_form(self, **kwargs): """Override the labels and required fields every time we get a formset.""" - formset = super().get_form(**kwargs) + # kwargs.update({"domain", self.object}) + + formset = super().get_form(**kwargs) + for i, form in enumerate(formset): + # form = self.get_form(self, **kwargs) form.fields["server"].label += f" {i+1}" form.fields["ip"].label += f" {i+1}" if i < 2: From 150e89d2ee47e8287b1d2b51076eda07c6a2b94a Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 19 Oct 2023 16:42:36 -0400 Subject: [PATCH 08/41] added form validation --- src/registrar/forms/domain.py | 28 ++++++------------- src/registrar/models/domain.py | 24 ++++++++++------ .../templates/domain_nameservers.html | 1 + src/registrar/views/domain.py | 24 ++++++++++------ 4 files changed, 40 insertions(+), 37 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index dbee3fa4d..1a85cb373 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -47,6 +47,8 @@ class IPAddressField(forms.CharField): class DomainNameserverForm(forms.Form): """Form for changing nameservers.""" + domain = forms.CharField(widget=forms.HiddenInput, required=False) + server = forms.CharField(label="Name server", strip=True) ip = IPAddressField( @@ -58,21 +60,12 @@ class DomainNameserverForm(forms.Form): # ], ) - def __init__(self, *args, **kwargs): - # Access the context object passed to the form - print(f"kwargs in __init__ {kwargs}") - self.domain = kwargs.pop('domain', None) - super().__init__(*args, **kwargs) - - # def __init__(self, request, *args, **kwargs): - # # Pass the request object to the form during initialization - # self.request = request - # super().__init__(*args, **kwargs) - def clean(self): cleaned_data = super().clean() server = cleaned_data.get('server', '') ip = cleaned_data.get('ip', '') + domain = cleaned_data.get('domain', '') + print(f"clean is called on {domain} {server}") # make sure there's a nameserver if an ip is passed if ip: @@ -82,15 +75,12 @@ class DomainNameserverForm(forms.Form): raise forms.ValidationError("Name server must be provided to enter IP address.") # if there's a nameserver and an ip, validate nameserver/ip combo - domain, _ = Domain.objects.get_or_create(name="realize-shake-too.gov") - # Access session data from the request object - # session_data = self.request.session.get('nameservers_form_domain', None) - - print(f"domain in clean: {self.domain}") - - if server and ip: - ip_list = [ip.strip() for ip in ip.split(",")] + if server: + if ip: + ip_list = [ip.strip() for ip in ip.split(",")] + else: + ip_list = [''] try: Domain.checkHostIPCombo(domain, server, ip_list) except NameserverError as e: diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index efbb42bfb..978e442e3 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -291,14 +291,16 @@ class Domain(TimeStampedModel, DomainHelper): newDict[tup[0]] = tup[1] return newDict - def isSubdomain(self, nameserver: str): + @classmethod + def isSubdomain(cls, name: str, nameserver: str): """Returns boolean if the domain name is found in the argument passed""" subdomain_pattern = r"([\w-]+\.)*" - full_pattern = subdomain_pattern + self.name + full_pattern = subdomain_pattern + name regex = re.compile(full_pattern) return bool(regex.match(nameserver)) - def checkHostIPCombo(self, nameserver: str, ip: list[str]): + @classmethod + def checkHostIPCombo(cls, name: str, nameserver: str, ip: list[str]): """Checks the parameters past for a valid combination raises error if: - nameserver is a subdomain but is missing ip @@ -312,23 +314,27 @@ class Domain(TimeStampedModel, DomainHelper): NameserverError (if exception hit) Returns: None""" - if self.isSubdomain(nameserver) and (ip is None or ip == [] or ip == ['']): + logger.info("checkHostIPCombo is called on %s, %s", name, nameserver) + if cls.isSubdomain(name, nameserver) and (ip is None or ip == [] or ip == ['']): + raise NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver) - elif not self.isSubdomain(nameserver) and (ip is not None and ip != [] and ip != ['']): + elif not cls.isSubdomain(name, nameserver) and (ip is not None and ip != [] and ip != ['']): raise NameserverError( code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, nameserver=nameserver, ip=ip ) elif ip is not None and ip != [] and ip != ['']: for addr in ip: logger.info(f"ip address {addr}") - if not self._valid_ip_addr(addr): + if not cls._valid_ip_addr(addr): raise NameserverError( code=nsErrorCodes.INVALID_IP, nameserver=nameserver, ip=ip ) + logger.info("got no errors") return None - def _valid_ip_addr(self, ipToTest: str): + @classmethod + def _valid_ip_addr(cls, ipToTest: str): """returns boolean if valid ip address string We currently only accept v4 or v6 ips returns: @@ -383,7 +389,7 @@ class Domain(TimeStampedModel, DomainHelper): if newHostDict[prevHost] is not None and set( newHostDict[prevHost] ) != set(addrs): - self.checkHostIPCombo(nameserver=prevHost, ip=newHostDict[prevHost]) + self.__class__.checkHostIPCombo(name=self.name, nameserver=prevHost, ip=newHostDict[prevHost]) updated_values.append((prevHost, newHostDict[prevHost])) new_values = { @@ -393,7 +399,7 @@ class Domain(TimeStampedModel, DomainHelper): } for nameserver, ip in new_values.items(): - self.checkHostIPCombo(nameserver=nameserver, ip=ip) + self.__class__.checkHostIPCombo(name=self.name, nameserver=nameserver, ip=ip) return (deleted_values, updated_values, new_values, previousHostDict) diff --git a/src/registrar/templates/domain_nameservers.html b/src/registrar/templates/domain_nameservers.html index fffd4b8c0..aa53719ea 100644 --- a/src/registrar/templates/domain_nameservers.html +++ b/src/registrar/templates/domain_nameservers.html @@ -24,6 +24,7 @@
+ {{ form.domain }} {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" %} {% if forloop.counter <= 2 %} {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index f82326a67..06361a3b3 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -221,13 +221,6 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): form_class = NameserverFormset model = Domain - def get_formset_kwargs(self, index): - kwargs = super().get_formset_kwargs(index) - # kwargs['domain'] = self.object # Pass the context data - kwargs.update({"domain", self.object}) - print(f"kwargs in get_formset_kwargs {kwargs}") - return kwargs - def get_initial(self): """The initial value for the form (which is a formset here).""" nameservers = self.object.nameservers @@ -257,7 +250,6 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): def get_form(self, **kwargs): """Override the labels and required fields every time we get a formset.""" # kwargs.update({"domain", self.object}) - formset = super().get_form(**kwargs) for i, form in enumerate(formset): @@ -268,8 +260,22 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): form.fields["server"].required = True else: form.fields["server"].required = False + form.fields["domain"].initial = self.object.name + print(f"domain in get_form {self.object.name}") return formset + + def post(self, request, *args, **kwargs): + """Form submission posts to this view. + This post method harmonizes using DomainBaseView and FormMixin + """ + self._get_domain(request) + form = self.get_form() + if form.is_valid(): + return self.form_valid(form) + else: + return self.form_invalid(form) + def form_valid(self, formset): """The formset is valid, perform something with it.""" @@ -286,7 +292,7 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): # this will return [''] if no ips have been entered, which is taken # into account in the model in checkHostIPCombo ip_list = [ip.strip() for ip in ip_list] - + as_tuple = ( form.cleaned_data["server"], ip_list, From 43b6d1e380dc1b93145b6e6aef7d8c809ed61cae Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 19 Oct 2023 17:26:26 -0400 Subject: [PATCH 09/41] form level error handling at the field level --- src/registrar/forms/domain.py | 16 ++++++++++++---- src/registrar/models/domain.py | 3 --- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 1a85cb373..303c43690 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -5,7 +5,10 @@ from django.core.validators import MinValueValidator, MaxValueValidator, RegexVa from django.forms import formset_factory from phonenumber_field.widgets import RegionalPhoneNumberWidget -from registrar.utility.errors import NameserverError +from registrar.utility.errors import ( + NameserverError, + NameserverErrorCodes as nsErrorCodes +) from ..models import Contact, DomainInformation, Domain from .common import ( @@ -70,9 +73,9 @@ class DomainNameserverForm(forms.Form): # make sure there's a nameserver if an ip is passed if ip: ip_list = [ip.strip() for ip in ip.split(",")] - if len(server) < len(ip_list): + if not server and len(ip_list) > 0: # If 'server' is empty, disallow 'ip' input - raise forms.ValidationError("Name server must be provided to enter IP address.") + self.add_error('server', "Nameserver must be provided to enter IP address.") # if there's a nameserver and an ip, validate nameserver/ip combo @@ -84,7 +87,12 @@ class DomainNameserverForm(forms.Form): try: Domain.checkHostIPCombo(domain, server, ip_list) except NameserverError as e: - raise forms.ValidationError(str(e)) + if e.code == nsErrorCodes.GLUE_RECORD_NOT_ALLOWED: + self.add_error('server', "Name server address does not match domain name") + elif e.code == nsErrorCodes.MISSING_IP: + self.add_error('ip', "Subdomains require an IP address") + else: + self.add_error('ip', str(e)) return cleaned_data diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 978e442e3..933ae63be 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -314,7 +314,6 @@ class Domain(TimeStampedModel, DomainHelper): NameserverError (if exception hit) Returns: None""" - logger.info("checkHostIPCombo is called on %s, %s", name, nameserver) if cls.isSubdomain(name, nameserver) and (ip is None or ip == [] or ip == ['']): raise NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver) @@ -330,7 +329,6 @@ class Domain(TimeStampedModel, DomainHelper): raise NameserverError( code=nsErrorCodes.INVALID_IP, nameserver=nameserver, ip=ip ) - logger.info("got no errors") return None @classmethod @@ -340,7 +338,6 @@ class Domain(TimeStampedModel, DomainHelper): returns: isValid (boolean)-True for valid ip address""" try: - logger.info(f"in valid_ip_addr: {ipToTest}") ip = ipaddress.ip_address(ipToTest) logger.info(ip.version) return ip.version == 6 or ip.version == 4 From 37ada699a0d30ff19c5ab75eb957f0470fb0dd68 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 19 Oct 2023 17:52:41 -0400 Subject: [PATCH 10/41] cancel button, template tweaks --- src/registrar/forms/domain.py | 4 +-- .../templates/domain_nameservers.html | 25 ++++++++++++++++--- .../templates/includes/form_messages.html | 2 +- src/registrar/views/domain.py | 15 ++++++----- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 303c43690..a8a285e99 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -55,7 +55,7 @@ class DomainNameserverForm(forms.Form): server = forms.CharField(label="Name server", strip=True) ip = IPAddressField( - label="IP address", + label="IP Address (IPv4 or IPv6)", strip=True, required=False, # validators=[ @@ -75,7 +75,7 @@ class DomainNameserverForm(forms.Form): ip_list = [ip.strip() for ip in ip.split(",")] if not server and len(ip_list) > 0: # If 'server' is empty, disallow 'ip' input - self.add_error('server', "Nameserver must be provided to enter IP address.") + self.add_error('server', "Name server must be provided to enter IP address.") # if there's a nameserver and an ip, validate nameserver/ip combo diff --git a/src/registrar/templates/domain_nameservers.html b/src/registrar/templates/domain_nameservers.html index aa53719ea..e4323615a 100644 --- a/src/registrar/templates/domain_nameservers.html +++ b/src/registrar/templates/domain_nameservers.html @@ -11,8 +11,16 @@

DNS name servers

-

Before your domain can be used we'll need information about your domain - name servers.

+

Before your domain can be used we’ll need information about your domain name servers. Name server records indicate which DNS server is authoritative for your domain.

+ +

Add a name server record by entering the address (e.g., ns1.nameserver.com) in the name server fields below. You may add up to 13 name servers.

+ +
+
+

Add an IP address only when your name server's address includes your domain name (e.g., if your domain name is "example.gov" and your name server is "ns1.example.gov,” then an IP address is required.) To add multiple IP addresses, separate them with commas.

+

This step is uncommon unless you self-host your DNS or use custom addresses for your nameserver.

+
+
{% include "includes/required_fields.html" %} @@ -36,7 +44,7 @@ {% endwith %}
- {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" add_group_class="usa-form-group--unstyled-error" %} + {% with sublabel_text="Ex: 86.124.49.54 or 2001:db8::1234:5678" %} {% input_with_errors form.ip %} {% endwith %}
@@ -44,7 +52,7 @@
{% endfor %} - +
+ +
{% endblock %} {# domain_content #} diff --git a/src/registrar/templates/includes/form_messages.html b/src/registrar/templates/includes/form_messages.html index c7b704f67..e2888e4ee 100644 --- a/src/registrar/templates/includes/form_messages.html +++ b/src/registrar/templates/includes/form_messages.html @@ -1,6 +1,6 @@ {% if messages %} {% for message in messages %} -
+
{{ message }}
diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 06361a3b3..23306bda9 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -255,7 +255,6 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): for i, form in enumerate(formset): # form = self.get_form(self, **kwargs) form.fields["server"].label += f" {i+1}" - form.fields["ip"].label += f" {i+1}" if i < 2: form.fields["server"].required = True else: @@ -270,11 +269,15 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): This post method harmonizes using DomainBaseView and FormMixin """ self._get_domain(request) - form = self.get_form() - if form.is_valid(): - return self.form_valid(form) + formset = self.get_form() + + 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(form) + return self.form_invalid(formset) def form_valid(self, formset): """The formset is valid, perform something with it.""" @@ -320,7 +323,7 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): logger.error(f"Registry error: {Err}") else: messages.success( - self.request, "The name servers for this domain have been updated." + self.request, "The name servers for this domain have been updated. Keep in mind that DNS changes may take some time to propagate across the internet. It can take anywhere from a few minutes to 48 hours for your changes to take place." ) # superclass has the redirect From 4d604b68b8cbd6d3c70c5d845001a5dc6ca7a4e9 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 20 Oct 2023 14:09:41 -0400 Subject: [PATCH 11/41] fixed formatting of error message in form --- src/registrar/templates/domain_nameservers.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_nameservers.html b/src/registrar/templates/domain_nameservers.html index e4323615a..4baf5d389 100644 --- a/src/registrar/templates/domain_nameservers.html +++ b/src/registrar/templates/domain_nameservers.html @@ -44,7 +44,7 @@ {% endwith %}
- {% with sublabel_text="Ex: 86.124.49.54 or 2001:db8::1234:5678" %} + {% with sublabel_text="Ex: 86.124.49.54 or 2001:db8::1234:5678" add_group_class="usa-form-group--unstyled-error" %} {% input_with_errors form.ip %} {% endwith %}
From 3f068bc209f5c7d1f80a17cd6e900ecd901fc1a3 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 20 Oct 2023 20:11:44 -0400 Subject: [PATCH 12/41] fixed some issues with nameserver getter and setter, as well as view get_initial --- src/registrar/models/domain.py | 4 ++-- src/registrar/views/domain.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 933ae63be..d4c634f21 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1384,7 +1384,7 @@ class Domain(TimeStampedModel, DomainHelper): @transition( field="state", - source=[State.DNS_NEEDED], + source=[State.DNS_NEEDED, State.READY], target=State.READY, # conditions=[dns_not_needed] ) @@ -1549,7 +1549,7 @@ class Domain(TimeStampedModel, DomainHelper): data = registry.send(req, cleaned=True).res_data[0] host = { "name": name, - "addrs": getattr(data, "addrs", ...), + "addrs": [item.addr for item in getattr(data, "addrs", [])], "cr_date": getattr(data, "cr_date", ...), "statuses": getattr(data, "statuses", ...), "tr_date": getattr(data, "tr_date", ...), diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 23306bda9..4d977891d 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -228,7 +228,7 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): if nameservers is not None: # Add existing nameservers as initial data - initial_data.extend({"server": name, "ip": ip} for name, ip in nameservers) + initial_data.extend({"server": name, "ip": ','.join(ip)} for name, ip in nameservers) # Ensure at least 3 fields, filled or empty while len(initial_data) < 2: From 6d3f5bf5ef9d4c529feba90313608020e5df834b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 23 Oct 2023 13:08:59 -0400 Subject: [PATCH 13/41] Add a delete button on forms on Nameservers, refactor JS and DS/Key data for DRY code --- src/registrar/assets/js/get-gov.js | 89 +++++++------------ src/registrar/assets/sass/_theme/_forms.scss | 4 + src/registrar/templates/domain_dsdata.html | 4 +- src/registrar/templates/domain_keydata.html | 4 +- .../templates/domain_nameservers.html | 19 ++-- 5 files changed, 55 insertions(+), 65 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index c21060382..aff3384eb 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -231,39 +231,11 @@ function handleValidationClick(e) { /** - * An IIFE that attaches a click handler for our dynamic nameservers form - * - * Only does something on a single page, but it should be fast enough to run - * it everywhere. + * Prepare the namerservers and DS data forms delete buttons + * We will call this on the forms init, and also every time we add a form + * */ -(function prepareNameserverForms() { - let serverForm = document.querySelectorAll(".server-form"); - let container = document.querySelector("#form-container"); - let addButton = document.querySelector("#add-nameserver-form"); - let totalForms = document.querySelector("#id_form-TOTAL_FORMS"); - - let formNum = serverForm.length-1; - if (addButton) - addButton.addEventListener('click', addForm); - - function addForm(e){ - let newForm = serverForm[2].cloneNode(true); - let formNumberRegex = RegExp(`form-(\\d){1}-`,'g'); - let formLabelRegex = RegExp(`Name server (\\d){1}`, 'g'); - let formExampleRegex = RegExp(`ns(\\d){1}`, 'g'); - - formNum++; - newForm.innerHTML = newForm.innerHTML.replace(formNumberRegex, `form-${formNum}-`); - newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `Name server ${formNum+1}`); - newForm.innerHTML = newForm.innerHTML.replace(formExampleRegex, `ns${formNum+1}`); - container.insertBefore(newForm, addButton); - newForm.querySelector("input").value = ""; - - totalForms.setAttribute('value', `${formNum+1}`); - } -})(); - -function prepareDeleteButtons() { +function prepareDeleteButtons(formLabel) { let deleteButtons = document.querySelectorAll(".delete-record"); let totalForms = document.querySelector("#id_form-TOTAL_FORMS"); @@ -273,13 +245,13 @@ function prepareDeleteButtons() { }); function removeForm(e){ - let formToRemove = e.target.closest(".ds-record"); + let formToRemove = e.target.closest(".repeatable-form"); formToRemove.remove(); - let forms = document.querySelectorAll(".ds-record"); + let forms = document.querySelectorAll(".repeatable-form"); totalForms.setAttribute('value', `${forms.length}`); let formNumberRegex = RegExp(`form-(\\d){1}-`, 'g'); - let formLabelRegex = RegExp(`DS Data record (\\d){1}`, 'g'); + let formLabelRegex = RegExp(`${formLabel} (\\d){1}`, 'g'); forms.forEach((form, index) => { // Iterate over child nodes of the current element @@ -294,8 +266,9 @@ function prepareDeleteButtons() { }); }); - Array.from(form.querySelectorAll('h2, legend')).forEach((node) => { - node.textContent = node.textContent.replace(formLabelRegex, `DS Data record ${index + 1}`); + // h2 and legend for DS form, label for nameservers + Array.from(form.querySelectorAll('h2, legend, label')).forEach((node) => { + node.textContent = node.textContent.replace(formLabelRegex, `${formLabel} ${index + 1}`); }); }); @@ -303,39 +276,44 @@ function prepareDeleteButtons() { } /** - * An IIFE that attaches a click handler for our dynamic DNSSEC forms + * An IIFE that attaches a click handler for our dynamic formsets * + * Only does something on a few pages, but it should be fast enough to run + * it everywhere. */ -(function prepareDNSSECForms() { - let serverForm = document.querySelectorAll(".ds-record"); +(function prepareFormsetsForms() { + let repeatableForm = document.querySelectorAll(".repeatable-form"); let container = document.querySelector("#form-container"); - let addButton = document.querySelector("#add-ds-form"); + let addButton = document.querySelector("#add-form"); let totalForms = document.querySelector("#id_form-TOTAL_FORMS"); + let cloneIndex = 0; + let formLabel = ''; + if (document.title.includes("DNS name servers |")) { + cloneIndex = 2; + formLabel = "Name server"; + } else if ((document.title.includes("DS Data |")) || (document.title.includes("Key Data |"))) { + formLabel = "DS Data record"; + } // Attach click event listener on the delete buttons of the existing forms - prepareDeleteButtons(); + prepareDeleteButtons(formLabel); - // Attack click event listener on the add button if (addButton) addButton.addEventListener('click', addForm); - /* - * Add a formset to the end of the form. - * For each element in the added formset, name the elements with the prefix, - * form-{#}-{element_name} where # is the index of the formset and element_name - * is the element's name. - * Additionally, update the form element's metadata, including totalForms' value. - */ function addForm(e){ - let forms = document.querySelectorAll(".ds-record"); + let forms = document.querySelectorAll(".repeatable-form"); let formNum = forms.length; - let newForm = serverForm[0].cloneNode(true); + let newForm = repeatableForm[cloneIndex].cloneNode(true); let formNumberRegex = RegExp(`form-(\\d){1}-`,'g'); - let formLabelRegex = RegExp(`DS Data record (\\d){1}`, 'g'); + let formLabelRegex = RegExp(`${formLabel} (\\d){1}`, 'g'); + // For the eample on Nameservers + let formExampleRegex = RegExp(`ns(\\d){1}`, 'g'); formNum++; newForm.innerHTML = newForm.innerHTML.replace(formNumberRegex, `form-${formNum-1}-`); - newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `DS Data record ${formNum}`); + newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `${formLabel} ${formNum}`); + newForm.innerHTML = newForm.innerHTML.replace(formExampleRegex, `ns${formNum}`); container.insertBefore(newForm, addButton); let inputs = newForm.querySelectorAll("input"); @@ -379,7 +357,6 @@ function prepareDeleteButtons() { totalForms.setAttribute('value', `${formNum}`); // Attach click event listener on the delete buttons of the new form - prepareDeleteButtons(); + prepareDeleteButtons(formLabel); } - })(); diff --git a/src/registrar/assets/sass/_theme/_forms.scss b/src/registrar/assets/sass/_theme/_forms.scss index ed118bb94..529fb8245 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-bottom-075 { + margin-bottom: units(1.5); +} + .usa-form--extra-large { max-width: none; } diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index ca4dce783..086d602e7 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -52,7 +52,7 @@ {{ formset.management_form }} {% for form in formset %} -
+
DS Data record {{forloop.counter}} @@ -97,7 +97,7 @@
{% endfor %} - + {% endif %} +
{% endfor %} - - + {% comment %} Work around USWDS' button margins to add some spacing between the submit and the 'add more' + This solution still works when we remove the 'add more' at 13 forms {% endcomment %} +
+ +
From 0dbcb749a6c754a9fe724dcd36a6e2496473b8d5 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 23 Oct 2023 21:47:23 -0400 Subject: [PATCH 15/41] fixed cancel button on nameservers --- .../templates/domain_nameservers.html | 18 ++++++++---------- src/registrar/views/domain.py | 4 +++- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/registrar/templates/domain_nameservers.html b/src/registrar/templates/domain_nameservers.html index f03570218..eebba921d 100644 --- a/src/registrar/templates/domain_nameservers.html +++ b/src/registrar/templates/domain_nameservers.html @@ -76,16 +76,14 @@ class="usa-button" >Save + + - -
- -
{% endblock %} {# domain_content #} diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 4d977891d..36ac74f94 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -10,6 +10,7 @@ import logging from django.contrib import messages 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 @@ -272,7 +273,8 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): formset = self.get_form() if "btn-cancel-click" in request.POST: - return redirect("/", {"formset": formset}, RequestContext(request)) + url = self.get_success_url() + return HttpResponseRedirect(url) if formset.is_valid(): return self.form_valid(formset) From 293eb40cffb31a1749d843919afe5adeb5b320fb Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 23 Oct 2023 22:10:03 -0400 Subject: [PATCH 16/41] removed some logging; fixed some existing unit tests --- src/registrar/models/domain.py | 2 -- src/registrar/tests/common.py | 4 ++-- src/registrar/tests/test_models_domain.py | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 9c0195345..eda24fbaa 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -324,7 +324,6 @@ class Domain(TimeStampedModel, DomainHelper): ) elif ip is not None and ip != [] and ip != ['']: for addr in ip: - logger.info(f"ip address {addr}") if not cls._valid_ip_addr(addr): raise NameserverError( code=nsErrorCodes.INVALID_IP, nameserver=nameserver, ip=ip @@ -339,7 +338,6 @@ class Domain(TimeStampedModel, DomainHelper): isValid (boolean)-True for valid ip address""" try: ip = ipaddress.ip_address(ipToTest) - logger.info(ip.version) return ip.version == 6 or ip.version == 4 except ValueError: diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 8cd5fd6ba..43554241c 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -755,7 +755,7 @@ class MockEppLib(TestCase): mockDataInfoHosts = fakedEppObject( "lastPw", cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35), - addrs=["1.2.3.4", "2.3.4.5"], + addrs=[common.Ip(addr="1.2.3.4"), common.Ip(addr="2.3.4.5")], ) mockDataHostChange = fakedEppObject( @@ -810,7 +810,7 @@ class MockEppLib(TestCase): "ns2.nameserverwithip.gov", "ns3.nameserverwithip.gov", ], - addrs=["1.2.3.4", "2.3.4.5"], + addrs=[common.Ip(addr="1.2.3.4"), common.Ip(addr="2.3.4.5")], ) infoDomainCheckHostIPCombo = fakedEppObject( diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 5759df1be..29bf58a2a 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -107,7 +107,7 @@ class TestDomainCache(MockEppLib): } expectedHostsDict = { "name": self.mockDataInfoDomain.hosts[0], - "addrs": self.mockDataInfoHosts.addrs, + "addrs": [item.addr for item in self.mockDataInfoHosts.addrs], "cr_date": self.mockDataInfoHosts.cr_date, } From 2bf7847654a02bba7ad313965eae6aba946b353e Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 24 Oct 2023 08:15:04 -0400 Subject: [PATCH 17/41] consolidation of nameserver view error messages --- src/registrar/forms/domain.py | 12 +++++++++--- src/registrar/utility/errors.py | 12 ++++++++---- src/registrar/views/domain.py | 4 ++-- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index a8a285e99..3c70ab41e 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -75,7 +75,7 @@ class DomainNameserverForm(forms.Form): ip_list = [ip.strip() for ip in ip.split(",")] if not server and len(ip_list) > 0: # If 'server' is empty, disallow 'ip' input - self.add_error('server', "Name server must be provided to enter IP address.") + self.add_error('server', NameserverError(code=nsErrorCodes.MISSING_HOST)) # if there's a nameserver and an ip, validate nameserver/ip combo @@ -88,9 +88,15 @@ class DomainNameserverForm(forms.Form): Domain.checkHostIPCombo(domain, server, ip_list) except NameserverError as e: if e.code == nsErrorCodes.GLUE_RECORD_NOT_ALLOWED: - self.add_error('server', "Name server address does not match domain name") + self.add_error( + 'server', + NameserverError(code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED) + ) elif e.code == nsErrorCodes.MISSING_IP: - self.add_error('ip', "Subdomains require an IP address") + self.add_error( + 'ip', + NameserverError(code=nsErrorCodes.MISSING_IP) + ) else: self.add_error('ip', str(e)) diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index f7bc743d6..c4bbc86c9 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -36,6 +36,7 @@ class NameserverErrorCodes(IntEnum): INVALID_IP = 3 TOO_MANY_HOSTS = 4 UNABLE_TO_UPDATE_DOMAIN = 5 + MISSING_HOST = 6 class NameserverError(Exception): @@ -45,10 +46,10 @@ class NameserverError(Exception): """ _error_mapping = { - NameserverErrorCodes.MISSING_IP: "Nameserver {} needs to have an " - "IP address because it is a subdomain", - NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED: "Nameserver {} cannot be linked " - "because it is not a subdomain", + NameserverErrorCodes.MISSING_IP: "Subdomains require an IP address", + NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED: ( + "Name server address does not match domain name" + ), NameserverErrorCodes.INVALID_IP: "Nameserver {} has an invalid IP address: {}", NameserverErrorCodes.TOO_MANY_HOSTS: ( "Too many hosts provided, you may not have more than 13 nameservers." @@ -57,6 +58,9 @@ class NameserverError(Exception): "Unable to update domain, changes were not applied." "Check logs as a Registry Error is the likely cause" ), + NameserverErrorCodes.MISSING_HOST: ( + "Name server must be provided to enter IP address." + ), } def __init__(self, *args, code=None, nameserver=None, ip=None, **kwargs): diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 639b83dfe..7583b062e 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -318,10 +318,10 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): # TODO: merge 1103 and use literals except RegistryError as Err: if Err.is_connection_error(): - messages.error(self.request, 'CANNOT_CONTACT_REGISTRY') + messages.error(self.request, CANNOT_CONTACT_REGISTRY) logger.error(f"Registry connection error: {Err}") else: - messages.error(self.request, 'GENERIC_ERROR') + messages.error(self.request, GENERIC_ERROR) logger.error(f"Registry error: {Err}") else: messages.success( From a7ba33b52364107ac983a244906b5b8b1dd103f1 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 24 Oct 2023 08:25:18 -0400 Subject: [PATCH 18/41] fixed broken tests --- src/registrar/tests/test_nameserver_error.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/tests/test_nameserver_error.py b/src/registrar/tests/test_nameserver_error.py index c64717eb5..1e87d48e1 100644 --- a/src/registrar/tests/test_nameserver_error.py +++ b/src/registrar/tests/test_nameserver_error.py @@ -11,8 +11,7 @@ class TestNameserverError(TestCase): """Test NameserverError when no ip address is passed""" nameserver = "nameserver val" expected = ( - f"Nameserver {nameserver} needs to have an " - "IP address because it is a subdomain" + "Subdomains require an IP address" ) nsException = NameserverError( From d532449b3d6c2adf6dc3fd36a60637108c9291b3 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 24 Oct 2023 09:37:43 -0400 Subject: [PATCH 19/41] cleaned up test cases and formatting --- src/registrar/forms/domain.py | 50 +++++++------------- src/registrar/models/domain.py | 23 ++++++--- src/registrar/tests/test_nameserver_error.py | 4 +- src/registrar/views/domain.py | 39 ++++++++------- 4 files changed, 55 insertions(+), 61 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 3c70ab41e..516e0abd2 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -7,7 +7,7 @@ from django.forms import formset_factory from phonenumber_field.widgets import RegionalPhoneNumberWidget from registrar.utility.errors import ( NameserverError, - NameserverErrorCodes as nsErrorCodes + NameserverErrorCodes as nsErrorCodes, ) from ..models import Contact, DomainInformation, Domain @@ -26,26 +26,9 @@ class DomainAddUserForm(forms.Form): class IPAddressField(forms.CharField): - - - - def validate(self, value): + def validate(self, value): super().validate(value) # Run the default CharField validation - # ip_list = [ip.strip() for ip in value.split(",")] # Split IPs and remove whitespace - - # # TODO: pass hostname from view? - - # hostname = self.form.cleaned_data.get("server", "") - - # print(f"hostname {hostname}") - - # # Call the IP validation method from Domain - # try: - # Domain.checkHostIPCombo(hostname, ip_list) - # except NameserverError as e: - # raise forms.ValidationError(str(e)) - class DomainNameserverForm(forms.Form): """Form for changing nameservers.""" @@ -62,12 +45,12 @@ class DomainNameserverForm(forms.Form): # django.core.validators.validate_ipv46_address # ], ) - + def clean(self): cleaned_data = super().clean() - server = cleaned_data.get('server', '') - ip = cleaned_data.get('ip', '') - domain = cleaned_data.get('domain', '') + server = cleaned_data.get("server", "") + ip = cleaned_data.get("ip", "") + domain = cleaned_data.get("domain", "") print(f"clean is called on {domain} {server}") # make sure there's a nameserver if an ip is passed @@ -75,30 +58,29 @@ class DomainNameserverForm(forms.Form): ip_list = [ip.strip() for ip in ip.split(",")] if not server and len(ip_list) > 0: # If 'server' is empty, disallow 'ip' input - self.add_error('server', NameserverError(code=nsErrorCodes.MISSING_HOST)) - + self.add_error( + "server", NameserverError(code=nsErrorCodes.MISSING_HOST) + ) + # if there's a nameserver and an ip, validate nameserver/ip combo - + if server: if ip: ip_list = [ip.strip() for ip in ip.split(",")] else: - ip_list = [''] + ip_list = [""] try: Domain.checkHostIPCombo(domain, server, ip_list) except NameserverError as e: if e.code == nsErrorCodes.GLUE_RECORD_NOT_ALLOWED: self.add_error( - 'server', - NameserverError(code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED) + "server", + NameserverError(code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED), ) elif e.code == nsErrorCodes.MISSING_IP: - self.add_error( - 'ip', - NameserverError(code=nsErrorCodes.MISSING_IP) - ) + self.add_error("ip", NameserverError(code=nsErrorCodes.MISSING_IP)) else: - self.add_error('ip', str(e)) + self.add_error("ip", str(e)) return cleaned_data diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index eda24fbaa..d82e0b20b 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -314,15 +314,16 @@ class Domain(TimeStampedModel, DomainHelper): NameserverError (if exception hit) Returns: None""" - if cls.isSubdomain(name, nameserver) and (ip is None or ip == [] or ip == ['']): - + if cls.isSubdomain(name, nameserver) and (ip is None or ip == [] or ip == [""]): raise NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver) - elif not cls.isSubdomain(name, nameserver) and (ip is not None and ip != [] and ip != ['']): + elif not cls.isSubdomain(name, nameserver) and ( + ip is not None and ip != [] and ip != [""] + ): raise NameserverError( code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, nameserver=nameserver, ip=ip ) - elif ip is not None and ip != [] and ip != ['']: + elif ip is not None and ip != [] and ip != [""]: for addr in ip: if not cls._valid_ip_addr(addr): raise NameserverError( @@ -384,7 +385,9 @@ class Domain(TimeStampedModel, DomainHelper): if newHostDict[prevHost] is not None and set( newHostDict[prevHost] ) != set(addrs): - self.__class__.checkHostIPCombo(name=self.name, nameserver=prevHost, ip=newHostDict[prevHost]) + self.__class__.checkHostIPCombo( + name=self.name, nameserver=prevHost, ip=newHostDict[prevHost] + ) updated_values.append((prevHost, newHostDict[prevHost])) new_values = { @@ -394,7 +397,9 @@ class Domain(TimeStampedModel, DomainHelper): } for nameserver, ip in new_values.items(): - self.__class__.checkHostIPCombo(name=self.name, nameserver=nameserver, ip=ip) + self.__class__.checkHostIPCombo( + name=self.name, nameserver=nameserver, ip=ip + ) return (deleted_values, updated_values, new_values, previousHostDict) @@ -605,7 +610,11 @@ class Domain(TimeStampedModel, DomainHelper): if len(hosts) > 13: raise NameserverError(code=nsErrorCodes.TOO_MANY_HOSTS) - if self.state not in [self.State.DNS_NEEDED, self.State.READY, self.State.UNKNOWN]: + if self.state not in [ + self.State.DNS_NEEDED, + self.State.READY, + self.State.UNKNOWN, + ]: raise ActionNotAllowed("Nameservers can not be " "set in the current state") logger.info("Setting nameservers") diff --git a/src/registrar/tests/test_nameserver_error.py b/src/registrar/tests/test_nameserver_error.py index 1e87d48e1..218a48231 100644 --- a/src/registrar/tests/test_nameserver_error.py +++ b/src/registrar/tests/test_nameserver_error.py @@ -10,9 +10,7 @@ class TestNameserverError(TestCase): def test_with_no_ip(self): """Test NameserverError when no ip address is passed""" nameserver = "nameserver val" - expected = ( - "Subdomains require an IP address" - ) + expected = "Subdomains require an IP address" nsException = NameserverError( code=nsErrorCodes.MISSING_IP, nameserver=nameserver diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 7583b062e..01c31e9bc 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -15,7 +15,6 @@ from django.shortcuts import redirect from django.template import RequestContext from django.urls import reverse from django.views.generic.edit import FormMixin -from django.forms import BaseFormSet from registrar.models import ( Domain, @@ -215,13 +214,13 @@ class DomainDNSView(DomainBaseView): template_name = "domain_dns.html" -class DomainNameserversView(DomainFormBaseView, BaseFormSet): +class DomainNameserversView(DomainFormBaseView): """Domain nameserver editing view.""" template_name = "domain_nameservers.html" form_class = NameserverFormset model = Domain - + def get_initial(self): """The initial value for the form (which is a formset here).""" nameservers = self.object.nameservers @@ -229,7 +228,9 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): if nameservers is not None: # Add existing nameservers as initial data - initial_data.extend({"server": name, "ip": ','.join(ip)} for name, ip in nameservers) + initial_data.extend( + {"server": name, "ip": ",".join(ip)} for name, ip in nameservers + ) # Ensure at least 3 fields, filled or empty while len(initial_data) < 2: @@ -251,8 +252,8 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): def get_form(self, **kwargs): """Override the labels and required fields every time we get a formset.""" # kwargs.update({"domain", self.object}) - formset = super().get_form(**kwargs) - + formset = super().get_form(**kwargs) + for i, form in enumerate(formset): # form = self.get_form(self, **kwargs) form.fields["server"].label += f" {i+1}" @@ -263,7 +264,7 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): form.fields["domain"].initial = self.object.name print(f"domain in get_form {self.object.name}") return formset - + def post(self, request, *args, **kwargs): """Form submission posts to this view. @@ -271,33 +272,33 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): """ self._get_domain(request) formset = self.get_form() - + if "btn-cancel-click" in request.POST: url = self.get_success_url() return HttpResponseRedirect(url) - + 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.""" - self.request.session['nameservers_form_domain'] = self.object - + self.request.session["nameservers_form_domain"] = self.object + # Set the nameservers from the formset nameservers = [] for form in formset: try: ip_string = form.cleaned_data["ip"] # Split the string into a list using a comma as the delimiter - ip_list = ip_string.split(',') + ip_list = ip_string.split(",") # Remove any leading or trailing whitespace from each IP in the list # this will return [''] if no ips have been entered, which is taken # into account in the model in checkHostIPCombo ip_list = [ip.strip() for ip in ip_list] - + as_tuple = ( form.cleaned_data["server"], ip_list, @@ -306,12 +307,12 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): except KeyError: # no server information in this field, skip it pass - + try: self.object.nameservers = nameservers except NameserverError as Err: # TODO: move into literal - messages.error(self.request, 'Whoops, Nameservers Error') + messages.error(self.request, "Whoops, Nameservers Error") # messages.error(self.request, GENERIC_ERROR) logger.error(f"Nameservers error: {Err}") # TODO: registry is not throwing an error when no connection @@ -325,7 +326,11 @@ class DomainNameserversView(DomainFormBaseView, BaseFormSet): logger.error(f"Registry error: {Err}") else: messages.success( - self.request, "The name servers for this domain have been updated. Keep in mind that DNS changes may take some time to propagate across the internet. It can take anywhere from a few minutes to 48 hours for your changes to take place." + self.request, + "The name servers for this domain have been updated. " + "Keep in mind that DNS changes may take some time to " + "propagate across the internet. It can take anywhere " + "from a few minutes to 48 hours for your changes to take place.", ) # superclass has the redirect From 182d1a61c0347caba0ffc31f48c24721876f7c7d Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 24 Oct 2023 13:02:02 -0400 Subject: [PATCH 20/41] more consolidation of error messages and their text --- src/epplibwrapper/__init__.py | 4 +-- src/epplibwrapper/errors.py | 3 -- src/registrar/forms/domain.py | 15 ++++++-- src/registrar/utility/errors.py | 37 +++++++++++++++++++ src/registrar/views/domain.py | 63 ++++++++++++++++++++++++++------- 5 files changed, 101 insertions(+), 21 deletions(-) diff --git a/src/epplibwrapper/__init__.py b/src/epplibwrapper/__init__.py index d0138d73c..dd6664a3a 100644 --- a/src/epplibwrapper/__init__.py +++ b/src/epplibwrapper/__init__.py @@ -45,7 +45,7 @@ except NameError: # Attn: these imports should NOT be at the top of the file try: from .client import CLIENT, commands - from .errors import RegistryError, ErrorCode, CANNOT_CONTACT_REGISTRY, GENERIC_ERROR + from .errors import RegistryError, ErrorCode from epplib.models import common, info from epplib.responses import extensions from epplib import responses @@ -61,6 +61,4 @@ __all__ = [ "info", "ErrorCode", "RegistryError", - "CANNOT_CONTACT_REGISTRY", - "GENERIC_ERROR", ] diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index dba5f328c..d34ed5e91 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -1,8 +1,5 @@ from enum import IntEnum -CANNOT_CONTACT_REGISTRY = "Update failed. Cannot contact the registry." -GENERIC_ERROR = "Value entered was wrong." - class ErrorCode(IntEnum): """ diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 516e0abd2..93d42e53d 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -75,10 +75,21 @@ class DomainNameserverForm(forms.Form): if e.code == nsErrorCodes.GLUE_RECORD_NOT_ALLOWED: self.add_error( "server", - NameserverError(code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED), + NameserverError( + code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, + nameserver=domain, + ip=ip_list + ), ) elif e.code == nsErrorCodes.MISSING_IP: - self.add_error("ip", NameserverError(code=nsErrorCodes.MISSING_IP)) + self.add_error( + "ip", + NameserverError( + code=nsErrorCodes.MISSING_IP, + nameserver=domain, + ip=ip_list, + ) + ) else: self.add_error("ip", str(e)) diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index c4bbc86c9..1f658b341 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -20,6 +20,41 @@ class ActionNotAllowed(Exception): pass +class GenericErrorCodes(IntEnum): + """Used across the registrar for + error mapping. + Overview of generic error codes: + - 1 GENERIC_ERROR a generic value error + - 2 CANNOT_CONTACT_REGISTRY a connection error w registry + """ + + GENERIC_ERROR = 1 + CANNOT_CONTACT_REGISTRY = 2 + + +class GenericError(Exception): + """ + GenericError class used to raise exceptions across + the registrar + """ + + _error_mapping = { + GenericErrorCodes.CANNOT_CONTACT_REGISTRY: "Update failed. Cannot contact the registry.", + GenericErrorCodes.GENERIC_ERROR: ( + "Value entered was wrong." + ), + } + + def __init__(self, *args, code=None, **kwargs): + super().__init__(*args, **kwargs) + self.code = code + if self.code in self._error_mapping: + self.message = self._error_mapping.get(self.code) + + def __str__(self): + return f"{self.message}" + + class NameserverErrorCodes(IntEnum): """Used in the NameserverError class for error mapping. @@ -29,6 +64,8 @@ class NameserverErrorCodes(IntEnum): value but is not a subdomain - 3 INVALID_IP invalid ip address format or invalid version - 4 TOO_MANY_HOSTS more than the max allowed host values + - 5 UNABLE_TO_UPDATE_DOMAIN unable to update the domain + - 6 MISSING_HOST host is missing for a nameserver """ MISSING_IP = 1 diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 01c31e9bc..b0f00f03a 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -24,7 +24,12 @@ from registrar.models import ( UserDomainRole, ) from registrar.models.public_contact import PublicContact -from registrar.utility.errors import NameserverError +from registrar.utility.errors import ( + GenericError, + GenericErrorCodes, + NameserverError, + NameserverErrorCodes as nsErrorCodes, +) from registrar.models.utility.contact_error import ContactError from ..forms import ( @@ -44,8 +49,6 @@ from epplibwrapper import ( common, extensions, RegistryError, - CANNOT_CONTACT_REGISTRY, - GENERIC_ERROR, ) from ..utility.email import send_templated_email, EmailSendingError @@ -311,18 +314,32 @@ class DomainNameserversView(DomainFormBaseView): try: self.object.nameservers = nameservers except NameserverError as Err: - # TODO: move into literal - messages.error(self.request, "Whoops, Nameservers Error") - # messages.error(self.request, GENERIC_ERROR) + # NamserverErrors *should* be caught in form; if reached here, + # there was an uncaught error in submission (through EPP) + messages.error( + self.request, + NameserverError( + code=nsErrorCodes.UNABLE_TO_UPDATE_DOMAIN + ) + ) logger.error(f"Nameservers error: {Err}") # TODO: registry is not throwing an error when no connection - # TODO: merge 1103 and use literals except RegistryError as Err: if Err.is_connection_error(): - messages.error(self.request, CANNOT_CONTACT_REGISTRY) + messages.error( + self.request, + GenericError( + code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY + ) + ) logger.error(f"Registry connection error: {Err}") else: - messages.error(self.request, GENERIC_ERROR) + messages.error( + self.request, + GenericError( + code=GenericErrorCodes.GENERIC_ERROR + ) + ) logger.error(f"Registry error: {Err}") else: messages.success( @@ -688,7 +705,12 @@ class DomainSecurityEmailView(DomainFormBaseView): # If no default is created for security_contact, # then we cannot connect to the registry. if contact is None: - messages.error(self.request, CANNOT_CONTACT_REGISTRY) + messages.error( + self.request, + GenericError( + code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY + ) + ) return redirect(self.get_success_url()) contact.email = new_email @@ -697,13 +719,28 @@ class DomainSecurityEmailView(DomainFormBaseView): contact.save() except RegistryError as Err: if Err.is_connection_error(): - messages.error(self.request, CANNOT_CONTACT_REGISTRY) + messages.error( + self.request, + GenericError( + code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY + ) + ) logger.error(f"Registry connection error: {Err}") else: - messages.error(self.request, GENERIC_ERROR) + messages.error( + self.request, + GenericError( + code=GenericErrorCodes.GENERIC_ERROR + ) + ) logger.error(f"Registry error: {Err}") except ContactError as Err: - messages.error(self.request, GENERIC_ERROR) + messages.error( + self.request, + GenericError( + code=GenericErrorCodes.GENERIC_ERROR + ) + ) logger.error(f"Generic registry error: {Err}") else: messages.success( From 497e81f6fa50e04155c64afa7623917e6e4011a9 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 24 Oct 2023 15:07:12 -0400 Subject: [PATCH 21/41] formatting for linting --- src/registrar/forms/domain.py | 4 ++-- src/registrar/utility/errors.py | 6 +++--- src/registrar/views/domain.py | 32 +++++++------------------------- 3 files changed, 12 insertions(+), 30 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 93d42e53d..b9aaee566 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -78,7 +78,7 @@ class DomainNameserverForm(forms.Form): NameserverError( code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, nameserver=domain, - ip=ip_list + ip=ip_list, ), ) elif e.code == nsErrorCodes.MISSING_IP: @@ -88,7 +88,7 @@ class DomainNameserverForm(forms.Form): code=nsErrorCodes.MISSING_IP, nameserver=domain, ip=ip_list, - ) + ), ) else: self.add_error("ip", str(e)) diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 1f658b341..41caed837 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -39,10 +39,10 @@ class GenericError(Exception): """ _error_mapping = { - GenericErrorCodes.CANNOT_CONTACT_REGISTRY: "Update failed. Cannot contact the registry.", - GenericErrorCodes.GENERIC_ERROR: ( - "Value entered was wrong." + GenericErrorCodes.CANNOT_CONTACT_REGISTRY: ( + "Update failed. Cannot contact the registry." ), + GenericErrorCodes.GENERIC_ERROR: ("Value entered was wrong."), } def __init__(self, *args, code=None, **kwargs): diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index b0f00f03a..a2ca6d8cb 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -317,10 +317,7 @@ class DomainNameserversView(DomainFormBaseView): # NamserverErrors *should* be caught in form; if reached here, # there was an uncaught error in submission (through EPP) messages.error( - self.request, - NameserverError( - code=nsErrorCodes.UNABLE_TO_UPDATE_DOMAIN - ) + self.request, NameserverError(code=nsErrorCodes.UNABLE_TO_UPDATE_DOMAIN) ) logger.error(f"Nameservers error: {Err}") # TODO: registry is not throwing an error when no connection @@ -328,17 +325,12 @@ class DomainNameserversView(DomainFormBaseView): if Err.is_connection_error(): messages.error( self.request, - GenericError( - code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY - ) + GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY), ) logger.error(f"Registry connection error: {Err}") else: messages.error( - self.request, - GenericError( - code=GenericErrorCodes.GENERIC_ERROR - ) + self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR) ) logger.error(f"Registry error: {Err}") else: @@ -707,9 +699,7 @@ class DomainSecurityEmailView(DomainFormBaseView): if contact is None: messages.error( self.request, - GenericError( - code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY - ) + GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY), ) return redirect(self.get_success_url()) @@ -721,25 +711,17 @@ class DomainSecurityEmailView(DomainFormBaseView): if Err.is_connection_error(): messages.error( self.request, - GenericError( - code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY - ) + GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY), ) logger.error(f"Registry connection error: {Err}") else: messages.error( - self.request, - GenericError( - code=GenericErrorCodes.GENERIC_ERROR - ) + self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR) ) logger.error(f"Registry error: {Err}") except ContactError as Err: messages.error( - self.request, - GenericError( - code=GenericErrorCodes.GENERIC_ERROR - ) + self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR) ) logger.error(f"Generic registry error: {Err}") else: From 46f03cd4e42afa070026ec877748be1c3e0fc0e3 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 24 Oct 2023 15:58:25 -0400 Subject: [PATCH 22/41] tweak error msg on invalid ip --- src/registrar/utility/errors.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 41caed837..5e28bc728 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -87,7 +87,7 @@ class NameserverError(Exception): NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED: ( "Name server address does not match domain name" ), - NameserverErrorCodes.INVALID_IP: "Nameserver {} has an invalid IP address: {}", + NameserverErrorCodes.INVALID_IP: "{}: Enter an IP address in the required format.", NameserverErrorCodes.TOO_MANY_HOSTS: ( "Too many hosts provided, you may not have more than 13 nameservers." ), @@ -106,7 +106,7 @@ class NameserverError(Exception): if self.code in self._error_mapping: self.message = self._error_mapping.get(self.code) if nameserver is not None and ip is not None: - self.message = self.message.format(str(nameserver), str(ip)) + self.message = self.message.format(str(nameserver)) elif nameserver is not None: self.message = self.message.format(str(nameserver)) elif ip is not None: From 517390cc55a391618d1560f7dc7ed5a0690fad25 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 25 Oct 2023 13:00:25 -0400 Subject: [PATCH 23/41] fixed bugs in ip list handling, reformatted clean function in forms, removed custom field type for ip address in nameserverform --- src/registrar/forms/domain.py | 90 +++++++++----------- src/registrar/models/domain.py | 10 +-- src/registrar/tests/test_nameserver_error.py | 2 +- src/registrar/utility/errors.py | 4 +- src/registrar/views/domain.py | 8 +- 5 files changed, 56 insertions(+), 58 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index b9aaee566..3b65c979b 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -37,64 +37,58 @@ class DomainNameserverForm(forms.Form): server = forms.CharField(label="Name server", strip=True) - ip = IPAddressField( - label="IP Address (IPv4 or IPv6)", - strip=True, - required=False, - # validators=[ - # django.core.validators.validate_ipv46_address - # ], - ) + ip = forms.CharField(label="IP Address (IPv4 or IPv6)", strip=True, required=False) def clean(self): + # clean is called from clean_forms, which is called from is_valid + # after clean_fields. it is used to determine form level errors. + # is_valid is typically called from view during a post cleaned_data = super().clean() + self.clean_empty_strings(cleaned_data) server = cleaned_data.get("server", "") - ip = cleaned_data.get("ip", "") + ip = cleaned_data.get("ip", None) domain = cleaned_data.get("domain", "") - print(f"clean is called on {domain} {server}") - # make sure there's a nameserver if an ip is passed - if ip: - ip_list = [ip.strip() for ip in ip.split(",")] - if not server and len(ip_list) > 0: - # If 'server' is empty, disallow 'ip' input - self.add_error( - "server", NameserverError(code=nsErrorCodes.MISSING_HOST) - ) + ip_list = self.extract_ip_list(ip) - # if there's a nameserver and an ip, validate nameserver/ip combo - - if server: - if ip: - ip_list = [ip.strip() for ip in ip.split(",")] - else: - ip_list = [""] - try: - Domain.checkHostIPCombo(domain, server, ip_list) - except NameserverError as e: - if e.code == nsErrorCodes.GLUE_RECORD_NOT_ALLOWED: - self.add_error( - "server", - NameserverError( - code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, - nameserver=domain, - ip=ip_list, - ), - ) - elif e.code == nsErrorCodes.MISSING_IP: - self.add_error( - "ip", - NameserverError( - code=nsErrorCodes.MISSING_IP, - nameserver=domain, - ip=ip_list, - ), - ) - else: - self.add_error("ip", str(e)) + if ip and not server and ip_list: + self.add_error("server", NameserverError(code=nsErrorCodes.MISSING_HOST)) + elif server: + self.validate_nameserver_ip_combo(domain, server, ip_list) return cleaned_data + def clean_empty_strings(self, cleaned_data): + ip = cleaned_data.get("ip", "") + if ip and len(ip.strip()) == 0: + cleaned_data["ip"] = None + + def extract_ip_list(self, ip): + return [ip.strip() for ip in ip.split(",")] if ip else [] + + def validate_nameserver_ip_combo(self, domain, server, ip_list): + try: + Domain.checkHostIPCombo(domain, server, ip_list) + except NameserverError as e: + if e.code == nsErrorCodes.GLUE_RECORD_NOT_ALLOWED: + self.add_error( + "server", + NameserverError( + code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, + nameserver=domain, + ip=ip_list, + ), + ) + elif e.code == nsErrorCodes.MISSING_IP: + self.add_error( + "ip", + NameserverError( + code=nsErrorCodes.MISSING_IP, nameserver=domain, ip=ip_list + ), + ) + else: + self.add_error("ip", str(e)) + NameserverFormset = formset_factory( DomainNameserverForm, diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index d82e0b20b..451838395 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -260,7 +260,7 @@ class Domain(TimeStampedModel, DomainHelper): """Creates the host object in the registry doesn't add the created host to the domain returns ErrorCode (int)""" - if addrs is not None: + if addrs is not None and addrs != []: addresses = [epp.Ip(addr=addr) for addr in addrs] request = commands.CreateHost(name=host, addrs=addresses) else: @@ -314,16 +314,14 @@ class Domain(TimeStampedModel, DomainHelper): NameserverError (if exception hit) Returns: None""" - if cls.isSubdomain(name, nameserver) and (ip is None or ip == [] or ip == [""]): + if cls.isSubdomain(name, nameserver) and (ip is None or ip == []): raise NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver) - elif not cls.isSubdomain(name, nameserver) and ( - ip is not None and ip != [] and ip != [""] - ): + elif not cls.isSubdomain(name, nameserver) and (ip is not None and ip != []): raise NameserverError( code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, nameserver=nameserver, ip=ip ) - elif ip is not None and ip != [] and ip != [""]: + elif ip is not None and ip != []: for addr in ip: if not cls._valid_ip_addr(addr): raise NameserverError( diff --git a/src/registrar/tests/test_nameserver_error.py b/src/registrar/tests/test_nameserver_error.py index 218a48231..0657b6599 100644 --- a/src/registrar/tests/test_nameserver_error.py +++ b/src/registrar/tests/test_nameserver_error.py @@ -35,7 +35,7 @@ class TestNameserverError(TestCase): ip = "ip val" nameserver = "nameserver val" - expected = f"Nameserver {nameserver} has an invalid IP address: {ip}" + expected = f"{nameserver}: Enter an IP address in the required format." nsException = NameserverError( code=nsErrorCodes.INVALID_IP, nameserver=nameserver, ip=ip ) diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 5e28bc728..64c86ee01 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -87,7 +87,9 @@ class NameserverError(Exception): NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED: ( "Name server address does not match domain name" ), - NameserverErrorCodes.INVALID_IP: "{}: Enter an IP address in the required format.", + NameserverErrorCodes.INVALID_IP: ( + "{}: Enter an IP address in the required format." + ), NameserverErrorCodes.TOO_MANY_HOSTS: ( "Too many hosts provided, you may not have more than 13 nameservers." ), diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index a2ca6d8cb..da501b273 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -295,8 +295,12 @@ class DomainNameserversView(DomainFormBaseView): for form in formset: try: ip_string = form.cleaned_data["ip"] - # Split the string into a list using a comma as the delimiter - ip_list = ip_string.split(",") + # ip_string will be None or a string of IP addresses + # comma-separated + ip_list = [] + if ip_string: + # Split the string into a list using a comma as the delimiter + ip_list = ip_string.split(",") # Remove any leading or trailing whitespace from each IP in the list # this will return [''] if no ips have been entered, which is taken # into account in the model in checkHostIPCombo From 04168419e7f4cfdf1531bd1ed48b7a766c624c3b Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 25 Oct 2023 20:23:35 -0400 Subject: [PATCH 24/41] unit tests for nameserver views --- src/registrar/tests/test_views.py | 185 ++++++++++++++++++++++++++++-- 1 file changed, 178 insertions(+), 7 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 1c215ec05..59a4ab700 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -10,6 +10,10 @@ from .common import MockEppLib, completed_application # type: ignore from django_webtest import WebTest # type: ignore import boto3_mocking # type: ignore +from registrar.utility.errors import ( + NameserverError, + NameserverErrorCodes, +) from registrar.models import ( DomainApplication, @@ -1393,20 +1397,165 @@ class TestDomainNameservers(TestDomainOverview): ) self.assertContains(page, "DNS name servers") - @skip("Broken by adding registry connection fix in ticket 848") - def test_domain_nameservers_form(self): + def test_domain_nameservers_form_submit_one_nameserver(self): """Can change domain's nameservers. Uses self.app WebTest because we need to interact with forms. """ + # initial nameservers page has one server with two ips nameservers_page = self.app.get( reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id}) ) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # attempt to submit the form with only one nameserver, should error + # regarding required fields with less_console_noise(): # swallow log warning message result = nameservers_page.form.submit() - # form submission was a post, response should be a redirect + # 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 required field. form requires a minimum of 2 name servers + self.assertContains(result, "This field is required.", count=2, status_code=200) + + def test_domain_nameservers_form_submit_subdomain_missing_ip(self): + """Can change domain's nameservers. + + Uses self.app WebTest because we need to interact with forms. + """ + # initial nameservers page has one server with two ips + nameservers_page = self.app.get( + reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id}) + ) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # attempt to submit the form without two hosts, both subdomains, + # only one has ips + nameservers_page.form["form-1-server"] = "ns2.igorville.gov" + with less_console_noise(): # swallow log warning message + result = nameservers_page.form.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 required field. subdomain missing an ip + self.assertContains( + result, + str(NameserverError(code=NameserverErrorCodes.MISSING_IP)), + count=2, + status_code=200 + ) + + def test_domain_nameservers_form_submit_missing_host(self): + """Can change domain's nameservers. + + Uses self.app WebTest because we need to interact with forms. + """ + # initial nameservers page has one server with two ips + nameservers_page = self.app.get( + reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id}) + ) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # attempt to submit the form without two hosts, both subdomains, + # only one has ips + nameservers_page.form["form-1-ip"] = "127.0.0.1" + with less_console_noise(): # swallow log warning message + result = nameservers_page.form.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 required field. nameserver has ip but missing host + self.assertContains( + result, + str(NameserverError(code=NameserverErrorCodes.MISSING_HOST)), + count=2, + status_code=200 + ) + + def test_domain_nameservers_form_submit_glue_record_not_allowed(self): + """Can change domain's nameservers. + + Uses self.app WebTest because we need to interact with forms. + """ + nameserver1 = "ns1.igorville.gov" + nameserver2 = "ns2.igorville.com" + valid_ip = "127.0.0.1" + # initial nameservers page has one server with two ips + nameservers_page = self.app.get( + reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id}) + ) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # attempt to submit the form without two hosts, both subdomains, + # only one has ips + nameservers_page.form["form-0-server"] = nameserver1 + nameservers_page.form["form-1-server"] = nameserver2 + nameservers_page.form["form-1-ip"] = valid_ip + with less_console_noise(): # swallow log warning message + result = nameservers_page.form.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 required field. nameserver has ip but missing host + self.assertContains( + result, + str(NameserverError( + code=NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED + )), + count=2, + status_code=200 + ) + + def test_domain_nameservers_form_submit_invalid_ip(self): + """Can change domain's nameservers. + + Uses self.app WebTest because we need to interact with forms. + """ + nameserver = "ns2.igorville.gov" + invalid_ip = "123" + # initial nameservers page has one server with two ips + nameservers_page = self.app.get( + reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id}) + ) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # attempt to submit the form without two hosts, both subdomains, + # only one has ips + nameservers_page.form["form-1-server"] = nameserver + nameservers_page.form["form-1-ip"] = invalid_ip + with less_console_noise(): # swallow log warning message + result = nameservers_page.form.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 required field. nameserver has ip but missing host + self.assertContains( + result, + str(NameserverError( + code=NameserverErrorCodes.INVALID_IP, + nameserver=nameserver + )), + count=2, + status_code=200 + ) + + def test_domain_nameservers_form_submits_successfully(self): + """Can change domain's nameservers. + + Uses self.app WebTest because we need to interact with forms. + """ + nameserver1 = "ns1.igorville.gov" + nameserver2 = "ns2.igorville.gov" + invalid_ip = "127.0.0.1" + # initial nameservers page has one server with two ips + nameservers_page = self.app.get( + reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id}) + ) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # attempt to submit the form without two hosts, both subdomains, + # only one has ips + nameservers_page.form["form-0-server"] = nameserver1 + nameservers_page.form["form-1-server"] = nameserver2 + nameservers_page.form["form-1-ip"] = invalid_ip + with less_console_noise(): # swallow log warning message + result = nameservers_page.form.submit() + # form submission was a successful post, response should be a 302 self.assertEqual(result.status_code, 302) self.assertEqual( result["Location"], @@ -1416,7 +1565,29 @@ class TestDomainNameservers(TestDomainOverview): page = result.follow() self.assertContains(page, "The name servers for this domain have been updated") - @skip("Broken by adding registry connection fix in ticket 848") + # self.assertContains(result, "This field is required.", count=2, status_code=200) + # add a second name server, which is a subdomain, but don't + # submit the required ip + # nameservers_page.form["form-1-server"] = "ns1.igorville.gov" + # with less_console_noise(): # swallow log warning message + # result = nameservers_page.form.submit() + # # form submission was a post with an error, response should be a 200 + # self.assertContains( + # result, + # str(NameserverError(code=NameserverErrorCodes.MISSING_IP)), + # count=2, + # status_code=200 + # ) + # form submission was a post, response should be a redirect + # self.assertEqual(result.status_code, 302) + # self.assertEqual( + # result["Location"], + # reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id}), + # ) + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # page = result.follow() + # self.assertContains(page, "The name servers for this domain have been updated") + def test_domain_nameservers_form_invalid(self): """Can change domain's nameservers. @@ -1433,9 +1604,9 @@ class TestDomainNameservers(TestDomainOverview): with less_console_noise(): # swallow logged warning message result = nameservers_page.form.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, "This field is required", count=2, status_code=200) + # error text appears four times, twice at the top of the page, + # once around each required field. + self.assertContains(result, "This field is required", count=4, status_code=200) class TestDomainAuthorizingOfficial(TestDomainOverview): From 62f3367c26ca7911c08b312626daea7f878997c7 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 25 Oct 2023 20:44:21 -0400 Subject: [PATCH 25/41] formatted code, removed comments and cluttering debugs, updated error messages --- .../templates/domain_nameservers.html | 2 +- src/registrar/tests/test_nameserver_error.py | 2 +- src/registrar/tests/test_views.py | 44 +++++-------------- src/registrar/utility/errors.py | 4 +- src/registrar/views/domain.py | 3 -- 5 files changed, 15 insertions(+), 40 deletions(-) diff --git a/src/registrar/templates/domain_nameservers.html b/src/registrar/templates/domain_nameservers.html index eebba921d..d00126698 100644 --- a/src/registrar/templates/domain_nameservers.html +++ b/src/registrar/templates/domain_nameservers.html @@ -13,7 +13,7 @@

Before your domain can be used we’ll need information about your domain name servers. Name server records indicate which DNS server is authoritative for your domain.

-

Add a name server record by entering the address (e.g., ns1.nameserver.com) in the name server fields below. You may add up to 13 name servers.

+

Add a name server record by entering the address (e.g., ns1.nameserver.com) in the name server fields below. You must add at least two name servers (13 max).

diff --git a/src/registrar/tests/test_nameserver_error.py b/src/registrar/tests/test_nameserver_error.py index 0657b6599..89a6dfcce 100644 --- a/src/registrar/tests/test_nameserver_error.py +++ b/src/registrar/tests/test_nameserver_error.py @@ -10,7 +10,7 @@ class TestNameserverError(TestCase): def test_with_no_ip(self): """Test NameserverError when no ip address is passed""" nameserver = "nameserver val" - expected = "Subdomains require an IP address" + expected = "Using your domain for a name server requires an IP address" nsException = NameserverError( code=nsErrorCodes.MISSING_IP, nameserver=nameserver diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 59a4ab700..28384ca91 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1440,7 +1440,7 @@ class TestDomainNameservers(TestDomainOverview): result, str(NameserverError(code=NameserverErrorCodes.MISSING_IP)), count=2, - status_code=200 + status_code=200, ) def test_domain_nameservers_form_submit_missing_host(self): @@ -1466,7 +1466,7 @@ class TestDomainNameservers(TestDomainOverview): result, str(NameserverError(code=NameserverErrorCodes.MISSING_HOST)), count=2, - status_code=200 + status_code=200, ) def test_domain_nameservers_form_submit_glue_record_not_allowed(self): @@ -1495,11 +1495,9 @@ class TestDomainNameservers(TestDomainOverview): # the required field. nameserver has ip but missing host self.assertContains( result, - str(NameserverError( - code=NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED - )), + str(NameserverError(code=NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED)), count=2, - status_code=200 + status_code=200, ) def test_domain_nameservers_form_submit_invalid_ip(self): @@ -1526,12 +1524,13 @@ class TestDomainNameservers(TestDomainOverview): # the required field. nameserver has ip but missing host self.assertContains( result, - str(NameserverError( - code=NameserverErrorCodes.INVALID_IP, - nameserver=nameserver - )), + str( + NameserverError( + code=NameserverErrorCodes.INVALID_IP, nameserver=nameserver + ) + ), count=2, - status_code=200 + status_code=200, ) def test_domain_nameservers_form_submits_successfully(self): @@ -1565,29 +1564,6 @@ class TestDomainNameservers(TestDomainOverview): page = result.follow() self.assertContains(page, "The name servers for this domain have been updated") - # self.assertContains(result, "This field is required.", count=2, status_code=200) - # add a second name server, which is a subdomain, but don't - # submit the required ip - # nameservers_page.form["form-1-server"] = "ns1.igorville.gov" - # with less_console_noise(): # swallow log warning message - # result = nameservers_page.form.submit() - # # form submission was a post with an error, response should be a 200 - # self.assertContains( - # result, - # str(NameserverError(code=NameserverErrorCodes.MISSING_IP)), - # count=2, - # status_code=200 - # ) - # form submission was a post, response should be a redirect - # self.assertEqual(result.status_code, 302) - # self.assertEqual( - # result["Location"], - # reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id}), - # ) - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # page = result.follow() - # self.assertContains(page, "The name servers for this domain have been updated") - def test_domain_nameservers_form_invalid(self): """Can change domain's nameservers. diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 64c86ee01..c1d3c5849 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -83,7 +83,9 @@ class NameserverError(Exception): """ _error_mapping = { - NameserverErrorCodes.MISSING_IP: "Subdomains require an IP address", + NameserverErrorCodes.MISSING_IP: ( + "Using your domain for a name server requires an IP address" + ), NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED: ( "Name server address does not match domain name" ), diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index da501b273..f795c4333 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -254,18 +254,15 @@ class DomainNameserversView(DomainFormBaseView): def get_form(self, **kwargs): """Override the labels and required fields every time we get a formset.""" - # kwargs.update({"domain", self.object}) formset = super().get_form(**kwargs) for i, form in enumerate(formset): - # form = self.get_form(self, **kwargs) form.fields["server"].label += f" {i+1}" if i < 2: form.fields["server"].required = True else: form.fields["server"].required = False form.fields["domain"].initial = self.object.name - print(f"domain in get_form {self.object.name}") return formset def post(self, request, *args, **kwargs): From 167cb9d995ab6934e24e06e4abf46d56cbe0d9b8 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 25 Oct 2023 21:56:52 -0400 Subject: [PATCH 26/41] formatting change --- src/registrar/views/domain.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 784d2af94..be6026bc3 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -499,10 +499,15 @@ class DomainDsDataView(DomainFormBaseView): self.object.dnssecdata = dnssecdata except RegistryError as err: if err.is_connection_error(): - messages.error(self.request, GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY)) + messages.error( + self.request, + GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY), + ) logger.error(f"Registry connection error: {err}") else: - messages.error(self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR)) + messages.error( + self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR) + ) logger.error(f"Registry error: {err}") return self.form_invalid(formset) else: From eedd5d7dbff003b0c5597f51f86e5eb8f97a0016 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 25 Oct 2023 22:43:41 -0400 Subject: [PATCH 27/41] fixed create_host bug not properly submitting ip version for v6; refactored update_host as well --- src/registrar/models/domain.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 522590e69..8215d0a7a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -261,7 +261,13 @@ class Domain(TimeStampedModel, DomainHelper): doesn't add the created host to the domain returns ErrorCode (int)""" if addrs is not None and addrs != []: - addresses = [epp.Ip(addr=addr) for addr in addrs] + addresses = [ + epp.Ip( + addr=addr, + ip="v6" if self.is_ipv6(addr) else None + ) + for addr in addrs + ] request = commands.CreateHost(name=host, addrs=addresses) else: request = commands.CreateHost(name=host) @@ -1541,10 +1547,12 @@ class Domain(TimeStampedModel, DomainHelper): return [] for ip_addr in ip_list: - if self.is_ipv6(ip_addr): - edited_ip_list.append(epp.Ip(addr=ip_addr, ip="v6")) - else: # default ip addr is v4 - edited_ip_list.append(epp.Ip(addr=ip_addr)) + edited_ip_list.append( + epp.Ip( + addr=ip_addr, + ip="v6" if self.is_ipv6(ip_addr) else None + ) + ) return edited_ip_list From 2f05772623cf9318f8305fe02c217d1907be1ce5 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 26 Oct 2023 06:16:55 -0400 Subject: [PATCH 28/41] formatting changes --- src/registrar/models/domain.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 8215d0a7a..5344a14c1 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -262,10 +262,7 @@ class Domain(TimeStampedModel, DomainHelper): returns ErrorCode (int)""" if addrs is not None and addrs != []: addresses = [ - epp.Ip( - addr=addr, - ip="v6" if self.is_ipv6(addr) else None - ) + epp.Ip(addr=addr, ip="v6" if self.is_ipv6(addr) else None) for addr in addrs ] request = commands.CreateHost(name=host, addrs=addresses) @@ -1548,10 +1545,7 @@ class Domain(TimeStampedModel, DomainHelper): for ip_addr in ip_list: edited_ip_list.append( - epp.Ip( - addr=ip_addr, - ip="v6" if self.is_ipv6(ip_addr) else None - ) + epp.Ip(addr=ip_addr, ip="v6" if self.is_ipv6(ip_addr) else None) ) return edited_ip_list From bd27bfc9de7cdd2fe2a5e16f95d3385d55d71bd2 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 26 Oct 2023 11:34:11 -0400 Subject: [PATCH 29/41] fix usa-alert class on form_messages --- src/registrar/templates/includes/form_messages.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/includes/form_messages.html b/src/registrar/templates/includes/form_messages.html index e2888e4ee..c7b704f67 100644 --- a/src/registrar/templates/includes/form_messages.html +++ b/src/registrar/templates/includes/form_messages.html @@ -1,6 +1,6 @@ {% if messages %} {% for message in messages %} -
+
{{ message }}
From 4fdbb231411d0a4b24419a3581fa24e147898342 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 26 Oct 2023 12:41:44 -0400 Subject: [PATCH 30/41] updated comment --- src/registrar/views/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index be6026bc3..b531fe8b7 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -296,7 +296,7 @@ class DomainNameserversView(DomainFormBaseView): # Split the string into a list using a comma as the delimiter ip_list = ip_string.split(",") # Remove any leading or trailing whitespace from each IP in the list - # this will return [''] if no ips have been entered, which is taken + # this will return [] if no ips have been entered, which is taken # into account in the model in checkHostIPCombo ip_list = [ip.strip() for ip in ip_list] From f928bc4da42e268162afbdbcd8ee95031d59fa88 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 30 Oct 2023 12:37:37 -0400 Subject: [PATCH 31/41] remove all whitespace from ip addresses --- src/registrar/views/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index b531fe8b7..2f3f2b9ed 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -298,7 +298,7 @@ class DomainNameserversView(DomainFormBaseView): # Remove any leading or trailing whitespace from each IP in the list # this will return [] if no ips have been entered, which is taken # into account in the model in checkHostIPCombo - ip_list = [ip.strip() for ip in ip_list] + ip_list = [ip.replace(" ", "") for ip in ip_list] as_tuple = ( form.cleaned_data["server"], From 34629439ee1934b36e318a7fc8df81ef84afd07c Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 30 Oct 2023 12:51:45 -0400 Subject: [PATCH 32/41] updated some js for readability and clean code --- src/registrar/assets/js/get-gov.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index e456575c7..1c678a4d6 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -238,10 +238,8 @@ function handleValidationClick(e) { function prepareDeleteButtons(formLabel) { let deleteButtons = document.querySelectorAll(".delete-record"); let totalForms = document.querySelector("#id_form-TOTAL_FORMS"); - let isNameserversForm = false; + let isNameserversForm = document.title.includes("DNS name servers |"); let addButton = document.querySelector("#add-form"); - if (document.title.includes("DNS name servers |")) - isNameserversForm = true; // Loop through each delete button and attach the click event listener deleteButtons.forEach((deleteButton) => { @@ -256,7 +254,7 @@ function prepareDeleteButtons(formLabel) { let formNumberRegex = RegExp(`form-(\\d){1}-`, 'g'); let formLabelRegex = RegExp(`${formLabel} (\\d+){1}`, 'g'); - // For the eample on Nameservers + // For the example on Nameservers let formExampleRegex = RegExp(`ns(\\d+){1}`, 'g'); forms.forEach((form, index) => { @@ -305,7 +303,7 @@ function prepareDeleteButtons(formLabel) { } }); - // Remove the add more button if we have less than 13 forms + // Display the add more button if we have less than 13 forms if (isNameserversForm && forms.length <= 13) { addButton.classList.remove("display-none") } @@ -327,9 +325,8 @@ function prepareDeleteButtons(formLabel) { let totalForms = document.querySelector("#id_form-TOTAL_FORMS"); let cloneIndex = 0; let formLabel = ''; - let isNameserversForm = false; - if (document.title.includes("DNS name servers |")) { - isNameserversForm = true; + let isNameserversForm = document.title.includes("DNS name servers |"); + if (isNameserversForm) { cloneIndex = 2; formLabel = "Name server"; } else if ((document.title.includes("DS Data |")) || (document.title.includes("Key Data |"))) { From 91e2a7906bbeeba57aa04670ba7f433d1ce84f62 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 30 Oct 2023 14:23:14 -0400 Subject: [PATCH 33/41] updated validation and removal of whitespace; raise RegistryErrors on create_host and update_host --- src/registrar/forms/domain.py | 2 ++ src/registrar/models/domain.py | 4 ++-- src/registrar/views/domain.py | 4 ---- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index c7ebd0807..3aca7af6d 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -45,6 +45,8 @@ class DomainNameserverForm(forms.Form): self.clean_empty_strings(cleaned_data) server = cleaned_data.get("server", "") ip = cleaned_data.get("ip", None) + # remove ANY spaces in the ip field + ip = ip.replace(" ", "") domain = cleaned_data.get("domain", "") ip_list = self.extract_ip_list(ip) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index acfdc1a7b..07e49dfdd 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -277,7 +277,7 @@ class Domain(TimeStampedModel, DomainHelper): return response.code except RegistryError as e: logger.error("Error _create_host, code was %s error was %s" % (e.code, e)) - return e.code + raise e def _convert_list_to_dict(self, listToConvert: list[tuple[str, list]]): """converts a list of hosts into a dictionary @@ -1593,7 +1593,7 @@ class Domain(TimeStampedModel, DomainHelper): return response.code except RegistryError as e: logger.error("Error _update_host, code was %s error was %s" % (e.code, e)) - return e.code + raise e def addAndRemoveHostsFromDomain( self, hostsToAdd: list[str], hostsToDelete: list[str] diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 4fd01bd0c..ede44b1d5 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -295,10 +295,6 @@ class DomainNameserversView(DomainFormBaseView): if ip_string: # Split the string into a list using a comma as the delimiter ip_list = ip_string.split(",") - # Remove any leading or trailing whitespace from each IP in the list - # this will return [] if no ips have been entered, which is taken - # into account in the model in checkHostIPCombo - ip_list = [ip.replace(" ", "").strip() for ip in ip_list] as_tuple = ( form.cleaned_data["server"], From c81a9753c320057cfad847674e7a4a830ee749ed Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 30 Oct 2023 14:35:09 -0400 Subject: [PATCH 34/41] comments on test cases --- src/registrar/tests/test_views.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 29a89c740..95af4c542 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1447,7 +1447,7 @@ class TestDomainNameservers(TestDomainOverview): self.assertContains(page, "DNS name servers") def test_domain_nameservers_form_submit_one_nameserver(self): - """Can change domain's nameservers. + """Nameserver form submitted with one nameserver throws error. Uses self.app WebTest because we need to interact with forms. """ @@ -1467,7 +1467,7 @@ class TestDomainNameservers(TestDomainOverview): self.assertContains(result, "This field is required.", count=2, status_code=200) def test_domain_nameservers_form_submit_subdomain_missing_ip(self): - """Can change domain's nameservers. + """Nameserver form catches missing ip error on subdomain. Uses self.app WebTest because we need to interact with forms. """ @@ -1493,7 +1493,7 @@ class TestDomainNameservers(TestDomainOverview): ) def test_domain_nameservers_form_submit_missing_host(self): - """Can change domain's nameservers. + """Nameserver form catches error when host is missing. Uses self.app WebTest because we need to interact with forms. """ @@ -1519,7 +1519,8 @@ class TestDomainNameservers(TestDomainOverview): ) def test_domain_nameservers_form_submit_glue_record_not_allowed(self): - """Can change domain's nameservers. + """Nameserver form catches error when IP is present + but host not subdomain. Uses self.app WebTest because we need to interact with forms. """ @@ -1550,7 +1551,7 @@ class TestDomainNameservers(TestDomainOverview): ) def test_domain_nameservers_form_submit_invalid_ip(self): - """Can change domain's nameservers. + """Nameserver form catches invalid IP on submission. Uses self.app WebTest because we need to interact with forms. """ @@ -1583,7 +1584,7 @@ class TestDomainNameservers(TestDomainOverview): ) def test_domain_nameservers_form_submits_successfully(self): - """Can change domain's nameservers. + """Nameserver form submits successfully with valid input. Uses self.app WebTest because we need to interact with forms. """ @@ -1614,7 +1615,7 @@ class TestDomainNameservers(TestDomainOverview): self.assertContains(page, "The name servers for this domain have been updated") def test_domain_nameservers_form_invalid(self): - """Can change domain's nameservers. + """Nameserver form does not submit with invalid data. Uses self.app WebTest because we need to interact with forms. """ From 65bb9ff2e1d28964e37d05db9b3cb0c86f43d0f3 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 30 Oct 2023 13:28:41 -0700 Subject: [PATCH 35/41] Fix typo --- src/registrar/templates/domain_dnssec.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_dnssec.html b/src/registrar/templates/domain_dnssec.html index 691ba79b2..c67a7243e 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 an 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 %} From 7eea1b5d83de4a2ad15d051359ef9cd828528aa0 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 30 Oct 2023 13:53:32 -0700 Subject: [PATCH 36/41] Fix wording from website to domain --- src/registrar/templates/domain_dnssec.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_dnssec.html b/src/registrar/templates/domain_dnssec.html index c67a7243e..c92ca2b78 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 an 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 an 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 %} From 7e04179c2011827866bfb7c742daefbda29ee7fd Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 30 Oct 2023 23:59:50 -0600 Subject: [PATCH 37/41] Linted --- .../commands/master_domain_migrations.py | 125 ++++++------------ .../commands/utility/terminal_helper.py | 1 - .../test_transition_domain_migrations.py | 31 +---- 3 files changed, 45 insertions(+), 112 deletions(-) diff --git a/src/registrar/management/commands/master_domain_migrations.py b/src/registrar/management/commands/master_domain_migrations.py index a5660b704..206b63276 100644 --- a/src/registrar/management/commands/master_domain_migrations.py +++ b/src/registrar/management/commands/master_domain_migrations.py @@ -8,11 +8,6 @@ import logging import argparse import sys -import os - -from django.test import Client - -from django_fsm import TransitionNotAllowed # type: ignore from django.core.management import BaseCommand from django.core.management import call_command @@ -22,7 +17,6 @@ from registrar.models import ( DomainInformation, DomainInvitation, TransitionDomain, - User, ) from registrar.management.commands.utility.terminal_helper import ( @@ -100,19 +94,26 @@ class Command(BaseCommand): parser.add_argument( "--migrationDirectory", default="migrationData", - help="The location of the files used for load_transition_domain migration script", + help=( + "The location of the files used for" + "load_transition_domain migration script" + ), ) parser.add_argument( "--migrationFilenames", - default="escrow_domain_contacts.daily.gov.GOV.txt,escrow_contacts.daily.gov.GOV.txt,escrow_domain_statuses.daily.gov.GOV.txt", - help="""The files used for load_transition_domain migration script. - Must appear IN ORDER and separated by commas: + default="""escrow_domain_contacts.daily.gov.GOV.txt, + escrow_contacts.daily.gov.GOV.txt, + escrow_domain_statuses.daily.gov.GOV.txt""", + help="""The files used for load_transition_domain migration script. + Must appear IN ORDER and separated by commas: domain_contacts_filename.txt,contacts_filename.txt,domain_statuses_filename.txt - + where... - - domain_contacts_filename is the Data file with domain contact information + - domain_contacts_filename is the Data file with domain contact + information - contacts_filename is the Data file with contact information - - domain_statuses_filename is the Data file with domain status information""", + - domain_statuses_filename is the Data file with domain status + information""", ) parser.add_argument( @@ -196,13 +197,15 @@ class Command(BaseCommand): if len(matching_domain_informations) == 0: TerminalHelper.print_conditional( debug_on, - f"""{TerminalColors.YELLOW}Missing Domain Information{TerminalColors.ENDC}""", + f"""{TerminalColors.YELLOW}Missing Domain Information + {TerminalColors.ENDC}""", ) missing_domain_informations.append(transition_domain_name) if len(matching_domain_invitations) == 0: TerminalHelper.print_conditional( debug_on, - f"""{TerminalColors.YELLOW}Missing Domain Invitation{TerminalColors.ENDC}""", + f"""{TerminalColors.YELLOW}Missing Domain Invitation + {TerminalColors.ENDC}""", ) missing_domain_invites.append(transition_domain_name) @@ -225,22 +228,26 @@ class Command(BaseCommand): logger.info( f"""{TerminalColors.OKGREEN} ============= FINISHED ANALYSIS =============== - + {total_missing_domains} Missing Domains: (These are transition domains that are missing from the Domain Table) - {TerminalColors.YELLOW}{missing_domains_as_string}{TerminalColors.OKGREEN} - + {TerminalColors.YELLOW}{missing_domains_as_string} + {TerminalColors.OKGREEN} {total_duplicate_domains} Duplicate Domains: - (These are transition domains which have duplicate entries in the Domain Table) - {TerminalColors.YELLOW}{duplicate_domains_as_string}{TerminalColors.OKGREEN} - + (These are transition domains which have duplicate + entries in the Domain Table) + {TerminalColors.YELLOW}{duplicate_domains_as_string} + {TerminalColors.OKGREEN} {total_missing_domain_informations} Domain Information Entries missing: - (These are transition domains which have no entries in the Domain Information Table) - {TerminalColors.YELLOW}{missing_domain_informations_as_string}{TerminalColors.OKGREEN} - + (These are transition domains which have no entries + in the Domain Information Table) + {TerminalColors.YELLOW}{missing_domain_informations_as_string} + {TerminalColors.OKGREEN} {total_missing_domain_invitations} Domain Invitations missing: - (These are transition domains which have no entires in the Domain Invitation Table) - {TerminalColors.YELLOW}{missing_domain_invites_as_string}{TerminalColors.OKGREEN} + (These are transition domains which have no entires in + the Domain Invitation Table) + {TerminalColors.YELLOW}{missing_domain_invites_as_string} + {TerminalColors.OKGREEN} {TerminalColors.ENDC} """ ) @@ -377,13 +384,10 @@ class Command(BaseCommand): logger.info( f""" {TerminalColors.YELLOW} - PLEASE Re-Run the script with the correct file location and filenames: - - EXAMPLE: - docker compose run -T app ./manage.py test_domain_migration --runMigrations --migrationDirectory /app/tmp --migrationFilenames escrow_domain_contacts.daily.gov.GOV.txt escrow_contacts.daily.gov.GOV.txt escrow_domain_statuses.daily.gov.GOV.txt - + PLEASE Re-Run the script with the correct + file location and filenames: """ - ) # noqa + ) return # Proceed executing the migration scripts @@ -400,33 +404,6 @@ class Command(BaseCommand): ) self.run_transfer_script(debug_on, prompts_enabled) - def simulate_user_logins(self, debug_on): - """Simulates logins for users (this will add - Domain Information objects to our tables)""" - - # logger.info(f"" - # f"{TerminalColors.OKCYAN}" - # f"================== SIMULATING LOGINS ==================" - # f"{TerminalColors.ENDC}") - - # for invite in DomainInvitation.objects.all(): #TODO: move to unit test - # #DEBUG: - # TerminalHelper.print_conditional(debug_on, - # f"{TerminalColors.OKCYAN}" - # f"Processing invite: {invite}" - # f"{TerminalColors.ENDC}") - # # get a user with this email address - # user, user_created = User.objects.get_or_create(email=invite.email, username=invite.email) - # #DEBUG: - # TerminalHelper.print_conditional(user_created, - # f"""{TerminalColors.OKCYAN}No user found (creating temporary user object){TerminalColors.ENDC}""") - # TerminalHelper.print_conditional(debug_on, - # f"""{TerminalColors.OKCYAN}Executing first-time login for user: {user}{TerminalColors.ENDC}""") - # user.first_login() - # if user_created: - # logger.info(f"""{TerminalColors.YELLOW}(Deleting temporary user object){TerminalColors.ENDC}""") - # user.delete() - def handle( self, **options, @@ -453,9 +430,6 @@ class Command(BaseCommand): debug_on = options.get("debug") prompts_enabled = options.get("prompt") run_migrations_enabled = options.get("runMigrations") - simulate_user_login_enabled = ( - False # TODO: delete? Moving to unit test... options.get("triggerLogins") - ) TerminalHelper.print_conditional( debug_on, @@ -538,30 +512,7 @@ class Command(BaseCommand): ) prompt_continuation_of_analysis = True - # STEP 2 -- SIMULATE LOGINS - # Simulate user login for each user in domain - # invitation if specified by user OR if running - # migration scripts. - # (NOTE: Although users can choose to run login - # simulations separately (for testing purposes), - # if we are running all migration scripts, we should - # automatically execute this as the final step - # to ensure Domain Information objects get added - # to the database.) - - if run_migrations_enabled and simulate_user_login_enabled: - if prompts_enabled: - simulate_user_login_enabled = TerminalHelper.query_yes_no( - f"""{TerminalColors.FAIL} - Proceed with simulating user logins? - {TerminalColors.ENDC}""" - ) - if not simulate_user_login_enabled: - return - self.simulate_user_logins(debug_on) - prompt_continuation_of_analysis = True - - # STEP 3 -- SEND INVITES + # STEP 2 -- SEND INVITES proceed_with_sending_invites = run_migrations_enabled if prompts_enabled and run_migrations_enabled: proceed_with_sending_invites = TerminalHelper.query_yes_no( @@ -574,7 +525,7 @@ class Command(BaseCommand): self.run_send_invites_script(debug_on, prompts_enabled) prompt_continuation_of_analysis = True - # STEP 4 -- ANALYZE TABLES & GENERATE REPORT + # STEP 3 -- ANALYZE TABLES & GENERATE REPORT # Analyze tables for corrupt data... if prompt_continuation_of_analysis and prompts_enabled: # ^ (only prompt if we ran steps 1 and/or 2) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 1bc2cbb33..625599b8b 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -1,5 +1,4 @@ import logging -import os import sys logger = logging.getLogger(__name__) diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index 4bd65ea9a..11324cb37 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -1,4 +1,3 @@ -from unittest.mock import patch from django.test import TestCase from registrar.models import ( @@ -10,9 +9,6 @@ from registrar.models import ( UserDomainRole, ) -from registrar.management.commands.master_domain_migrations import ( - Command as master_migration_command, -) from django.core.management import call_command @@ -52,7 +48,7 @@ class TestLogins(TestCase): call_command("transfer_transition_domains_to_domains") def run_master_script(self): - command = call_command( + call_command( "master_domain_migrations", runMigrations=True, migrationDirectory=f"{self.test_data_file_location}", @@ -154,23 +150,8 @@ class TestLogins(TestCase): self.run_master_script() - # # TODO: instead of patching....there has got to be a way of making sure subsequent commands use the django database - # # Patch subroutines for migrations - # def side_effect(): - # self.run_load_domains() - # self.run_transfer_domains() - # patcher = patch("registrar.management.commands.master_domain_migrations.Command.run_migration_scripts") - # mocked_get = patcher.start() - # mocked_get.side_effect = side_effect - # # Patch subroutines for sending invitations - # def side_effect(): - # # TODO: what should happen here? - # return - # patcher = patch("registrar.management.commands.master_domain_migrations.Command.run_send_invites_script") - # mocked_get = patcher.start() - # mocked_get.side_effect = side_effect - - # STEP 2: (analyze the tables just like the migration script does, but add assert statements) + # STEP 2: (analyze the tables just like the + # migration script does, but add assert statements) expected_total_transition_domains = 8 expected_total_domains = 4 expected_total_domain_informations = 0 @@ -178,7 +159,8 @@ class TestLogins(TestCase): expected_missing_domains = 0 expected_duplicate_domains = 0 - # we expect 8 missing domain invites since the migration does not auto-login new users + # we expect 8 missing domain invites since the + # migration does not auto-login new users expected_missing_domain_informations = 8 # we expect 1 missing invite from anomaly.gov (an injected error) expected_missing_domain_invitations = 1 @@ -196,7 +178,8 @@ class TestLogins(TestCase): def test_load_transition_domain(self): self.run_load_domains() - # STEP 2: (analyze the tables just like the migration script does, but add assert statements) + # STEP 2: (analyze the tables just like the migration + # script does, but add assert statements) expected_total_transition_domains = 8 expected_total_domains = 0 expected_total_domain_informations = 0 From 644034fe96328424a230282ac2dc230d7599579b Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 31 Oct 2023 10:22:45 -0600 Subject: [PATCH 38/41] linted (except for security flags) --- src/registrar/management/commands/utility/terminal_helper.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 625599b8b..7327ef3bd 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -22,6 +22,7 @@ class TerminalColors: class TerminalHelper: + @staticmethod def query_yes_no(question: str, default="yes") -> bool: """Ask a yes/no question via raw_input() and return their answer. @@ -52,6 +53,7 @@ class TerminalHelper: else: logger.info("Please respond with 'yes' or 'no' " "(or 'y' or 'n').\n") + @staticmethod def print_conditional(print_condition: bool, print_statement: str): """This function reduces complexity of debug statements in other functions. @@ -61,12 +63,13 @@ class TerminalHelper: if print_condition: logger.info(print_statement) + @staticmethod def prompt_for_execution( system_exit_on_terminate: bool, info_to_inspect: str, prompt_title: str ) -> bool: """Create to reduce code complexity. Prompts the user to inspect the given string - and asks if they wish to execute it. + and asks if they wish to proceed. Returns true if the user responds (y), Returns false if the user responds (n)""" From 29a422d114d28b671777d39db85bd6dffd9c051b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 31 Oct 2023 11:40:18 -0600 Subject: [PATCH 39/41] Fixed SEC issue --- .../commands/cat_files_into_getgov.py | 66 +++++++++++++++---- 1 file changed, 52 insertions(+), 14 deletions(-) diff --git a/src/registrar/management/commands/cat_files_into_getgov.py b/src/registrar/management/commands/cat_files_into_getgov.py index 5de44ba73..ceb7ba6b2 100644 --- a/src/registrar/management/commands/cat_files_into_getgov.py +++ b/src/registrar/management/commands/cat_files_into_getgov.py @@ -3,9 +3,12 @@ import glob import logging import os +import string from django.core.management import BaseCommand +from registrar.management.commands.utility.terminal_helper import TerminalHelper + logger = logging.getLogger(__name__) @@ -27,6 +30,7 @@ class Command(BaseCommand): def handle(self, **options): file_extension: str = options.get("file_extension").lstrip(".") directory = options.get("directory") + helper = TerminalHelper() # file_extension is always coerced as str, Truthy is OK to use here. if not file_extension or not isinstance(file_extension, str): @@ -38,29 +42,63 @@ class Command(BaseCommand): for src_file_path in matching_extensions: filename = os.path.basename(src_file_path) + exit_status = -1 do_command = True - exit_status: int desired_file_path = f"{directory}/{filename}" if os.path.exists(desired_file_path): # For linter - prompt = " Do you want to replace it? (y/n) " - replace = input(f"{desired_file_path} already exists. {prompt}") - if replace.lower() != "y": + prompt = "Do you want to replace it?" + replace = f"{desired_file_path} already exists. {prompt}" + if not helper.query_yes_no(replace): do_command = False - if do_command: - copy_from = f"../tmp/{filename}" - self.cat(copy_from, desired_file_path) - exit_status = os.system(f"cat ../tmp/{filename} > {desired_file_path}") - - if exit_status == 0: - logger.info(f"Successfully copied {filename}") - else: - logger.error(f"Failed to copy {filename}") + try: + if do_command: + copy_from = f"../tmp/{filename}" + exit_status = self.cat(copy_from, desired_file_path) + except ValueError as err: + raise err + finally: + if exit_status == 0: + logger.info(f"Successfully copied {filename}") + else: + logger.error(f"Failed to copy {filename}") def cat(self, copy_from, copy_to): """Runs the cat command to copy_from a location to copy_to a location""" - exit_status = os.system(f"cat {copy_from} > {copy_to}") + + # copy_from will be system defined + self.check_file_path(copy_from, check_directory = False) + self.check_file_path(copy_to) + + # This command can only be ran from inside cf ssh getgov-{sandbox} + # It has no utility when running locally, and to exploit this + # you would have to have ssh access anyway, which is a bigger problem. + exit_status = os.system(f"cat {copy_from} > {copy_to}") # nosec return exit_status + + def check_file_path(self, file_path: str, check_directory = True): + """Does a check on user input to ensure validity""" + if not isinstance(file_path, str): + raise ValueError("Invalid path provided") + + # Remove any initial/final whitespace + file_path = file_path.strip() + + # Check for any attempts to move up in the directory structure + if ".." in file_path and check_directory: + raise ValueError( + "Moving up in the directory structure is not allowed" + ) + + # Check for any invalid characters + valid_chars = f"/-_.() {string.ascii_letters}{string.digits}" + for char in file_path: + if char not in valid_chars: + raise ValueError( + f"Invalid character {char} in file path" + ) + + return file_path From e1257608db73b9508ec24169c64306a8385732dc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 31 Oct 2023 11:47:22 -0600 Subject: [PATCH 40/41] Fix linter issues --- .../management/commands/cat_files_into_getgov.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/registrar/management/commands/cat_files_into_getgov.py b/src/registrar/management/commands/cat_files_into_getgov.py index ceb7ba6b2..6c46994ea 100644 --- a/src/registrar/management/commands/cat_files_into_getgov.py +++ b/src/registrar/management/commands/cat_files_into_getgov.py @@ -70,35 +70,31 @@ class Command(BaseCommand): copy_from a location to copy_to a location""" # copy_from will be system defined - self.check_file_path(copy_from, check_directory = False) + self.check_file_path(copy_from, check_directory=False) self.check_file_path(copy_to) # This command can only be ran from inside cf ssh getgov-{sandbox} # It has no utility when running locally, and to exploit this # you would have to have ssh access anyway, which is a bigger problem. - exit_status = os.system(f"cat {copy_from} > {copy_to}") # nosec + exit_status = os.system(f"cat {copy_from} > {copy_to}") # nosec return exit_status - def check_file_path(self, file_path: str, check_directory = True): + def check_file_path(self, file_path: str, check_directory=True): """Does a check on user input to ensure validity""" if not isinstance(file_path, str): raise ValueError("Invalid path provided") - + # Remove any initial/final whitespace file_path = file_path.strip() # Check for any attempts to move up in the directory structure if ".." in file_path and check_directory: - raise ValueError( - "Moving up in the directory structure is not allowed" - ) + raise ValueError("Moving up in the directory structure is not allowed") # Check for any invalid characters valid_chars = f"/-_.() {string.ascii_letters}{string.digits}" for char in file_path: if char not in valid_chars: - raise ValueError( - f"Invalid character {char} in file path" - ) + raise ValueError(f"Invalid character {char} in file path") return file_path From 81cc16d1b3575bd7ad6ca532a144aee5babff423 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 31 Oct 2023 12:08:08 -0600 Subject: [PATCH 41/41] Updated documentation --- docs/operations/data_migration.md | 113 ++++++++++++++++++++++++------ 1 file changed, 90 insertions(+), 23 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 00f84a897..316098378 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -87,12 +87,12 @@ FILE 3: **escrow_domain_statuses.daily.gov.GOV.txt** -> has the map of domains a We need to run a few scripts to parse these files into our domain tables. We can do this both locally and in a sandbox. -## OPTION 1: SANDBOX -## Load migration data onto a production or sandbox environment +### SANDBOX MIGRATION SETUP +### Load migration data onto a production or sandbox environment **WARNING:** All files uploaded in this manner are temporary, i.e. they will be deleted when the app is restaged. Do not use this method to store data you want to keep around permanently. -### STEP 1: Use scp to transfer data +#### STEP 1: Use scp to transfer data CloudFoundry supports scp as means of transferring data locally to our environment. If you are dealing with a batch of files, try sending across a tar.gz and unpacking that. **Login to Cloud.gov** @@ -134,7 +134,7 @@ Copy this code into the password prompt from earlier. NOTE: You can use different utilities to copy this onto the clipboard for you. If you are on Windows, try the command `cf ssh-code | clip`. On Mac, this will be `cf ssh-code | pbcopy` -### STEP 2: Transfer uploaded files to the getgov directory +#### STEP 2: Transfer uploaded files to the getgov directory Due to the nature of how Cloud.gov operates, the getgov directory is dynamically generated whenever the app is built under the tmp/ folder. We can directly upload files to the tmp/ folder but cannot target the generated getgov folder directly, as we need to spin up a shell to access this. From here, we can move those uploaded files into the getgov directory using the `cat` command. Note that you will have to repeat this for each file you want to move, so it is better to use a tar.gz for multiple, and unpack it inside of the `datamigration` folder. **SSH into your sandbox** @@ -168,7 +168,7 @@ tar -xvf migrationdata/{FILE_NAME}.tar.gz -C migrationdata/ --strip-components=1 -#### Manual method +##### Manual method If the `cat_files_into_getgov.py` script isn't working, follow these steps instead. **Move the desired file into the correct directory** @@ -178,34 +178,37 @@ cat ../tmp/{filename} > migrationdata/{filename} ``` -### STEP 3: Load Transition Domain data into TransitionDomain table -Run the following script to transfer the existing data on our .txt files to our DB. -```shell -./manage.py load_transition_domain migrationdata/escrow_domain_contacts.daily.gov.GOV.txt migrationdata/escrow_contacts.daily.gov.GOV.txt migrationdata/escrow_domain_statuses.daily.gov.GOV.txt +#### You are now ready to run migration scripts (see "Running the Migration Scripts") ``` -## OPTION 2: LOCAL -## Load migration data onto our local environments +``` +### LOCAL MIGRATION SETUP (TESTING PURPOSES ONLY) +### Load migration data onto our local environments -Transferring this data from these files into our domain tables happens in two steps; +***IMPORTANT: only use test data, to avoid publicizing PII in our public repo.*** -***IMPORTANT: only run the following locally, to avoid publicizing PII in our public repo.*** - -### STEP 1: Load Transition Domain data into TransitionDomain table - -**SETUP** -In order to use the management command, we need to add the files to a folder under `src/`. +In order to run the scripts locally, we need to add the files to a folder under `src/`. This will allow Docker to mount the files to a container (under `/app`) for our use. - Create a folder called `tmp` underneath `src/` - Add the above files to this folder - Open a terminal and navigate to `src/` -Then run the following command (This will parse the three files in your `tmp` folder and load the information into the TransitionDomain table); +#### You are now ready to run migration scripts (see "Running the Migration Scripts") + +### RUNNING THE MIGRATION SCRIPTS + +**NOTE: Whil we recommend executing the following scripts individually (Steps 1-3), migrations can also be done 'all at once' using the "Run Migration Feature" in step 4. Use with discretion.** + +#### 1 - Load Transition Domains +Run the following command (This will parse the three files in your `tmp` folder and load the information into the TransitionDomain table); ```shell -docker compose run -T app ./manage.py load_transition_domain /app/tmp/escrow_domain_contacts.daily.gov.GOV.txt /app/tmp/escrow_contacts.daily.gov.GOV.txt /app/tmp/escrow_domain_statuses.daily.gov.GOV.txt +docker compose run -T app ./manage.py load_transition_domain /app/tmp/escrow_domain_contacts.daily.gov.GOV.txt /app/tmp/escrow_contacts.daily.gov.GOV.txt /app/tmp/escrow_domain_statuses.daily.gov.GOV.txt --debug ``` +**For Sandbox**: +Change "/app/tmp" to point to the sandbox directory + **OPTIONAL COMMAND LINE ARGUMENTS**: `--debug` This will print out additional, detailed logs. @@ -214,7 +217,7 @@ This will print out additional, detailed logs. Directs the script to load only the first 100 entries into the table. You can adjust this number as needed for testing purposes. `--resetTable` -This will delete all the data loaded into transtion_domain. It is helpful if you want to see the entries reload from scratch or for clearing test data. +This will delete all the data in transtion_domain. It is helpful if you want to see the entries reload from scratch or for clearing test data. ### STEP 2: Transfer Transition Domain data into main Domain tables @@ -224,7 +227,7 @@ Now that we've loaded all the data into TransitionDomain, we need to update the In the same terminal as used in STEP 1, run the command below; (This will parse the data in TransitionDomain and either create a corresponding Domain object, OR, if a corresponding Domain already exists, it will update that Domain with the incoming status. It will also create DomainInvitation objects for each user associated with the domain): ```shell -docker compose run -T app ./manage.py transfer_transition_domains_to_domains +docker compose run -T app ./manage.py transfer_transition_domains_to_domains --debug ``` **OPTIONAL COMMAND LINE ARGUMENTS**: @@ -232,4 +235,68 @@ docker compose run -T app ./manage.py transfer_transition_domains_to_domains This will print out additional, detailed logs. `--limitParse 100` -Directs the script to load only the first 100 entries into the table. You can adjust this number as needed for testing purposes. \ No newline at end of file +Directs the script to load only the first 100 entries into the table. You can adjust this number as needed for testing purposes. + +### STEP 3: Send Domain invitations +### Run the send invitations script + +To send invitations for every transition domain in the transition domain table, execute the following command: +`docker compose run -T app send_domain_invitations -s` + +### STEP 4: Test the results +### Run the migration analyzer + +This script's main function is to scan the transition domain and domain tables for any anomalies. It produces a simple report of missing or duplicate data. NOTE: some missing data might be expected depending on the nature of our migrations so use best judgement when evaluating the results. + +**ANALYZE ONLY** +To analyze our database without running migrations, execute the script without any optional arguments: +`docker compose run -T app ./manage.py master_domain_migrations --debug` + +**RUN MIGRATIONS FEATURE** +To run the migrations again (all above migration steps) before analyzing, execute the following command (read the documentation on the terminal arguments below. Everything used by the migration scripts can also be passed into this script and will have the same effects). NOTE: --debug and --prompt allow you to step through the migration process and exit it after each step if you need to. It is recommended that you use these arguments when using the --runMigrations feature: +`docker compose run -T app ./manage.py master_domain_migrations --runMigrations --debug --prompt` + +#### OPTIONAL ARGUMENTS +`--runMigrations` +A boolean (default to true), which triggers running +all scripts (in sequence) for transition domain migrations + +`--migrationDirectory` +**default="migrationData"** (<--This is the sandbox directory) +The location of the files used for load_transition_domain migration script +EXAMPLE USAGE: +--migrationDirectory /app/tmp + +`--migrationFilenames` +**default=escrow_domain_contacts.daily.gov.GOV.txt,escrow_contacts.daily.gov.GOV.txt,escrow_domain_statuses.daily.gov.GOV.txt** (<--These are the usual names for the files. The script will throw a warning if it cannot find these exact files, in which case you will need to supply the correct filenames) +The files used for load_transition_domain migration script. +Must appear IN ORDER and comma-delimited: +EXAMPLE USAGE: +--migrationFilenames domain_contacts_filename.txt,contacts_filename.txt,domain_statuses_filename.txt +where... +- domain_contacts_filename is the Data file with domain contact information +- contacts_filename is the Data file with contact information +- domain_statuses_filename is the Data file with domain status information + +`--sep` +Delimiter for the migration scripts to correctly parse the given text files. +(usually this can remain at default value of |) + +`--debug` +A boolean (default to true), which activates additional print statements + +`--prompt` +A boolean (default to true), which activates terminal prompts +that allows the user to step through each portion of this +script. + +`--limitParse` +Used by the migration scripts (load_transition_domain) to set the limit for the +number of data entries to insert. Set to 0 (or just don't use this +argument) to parse every entry. This was provided primarily for testing +purposes + +`--resetTable` +Used by the migration scripts to trigger a prompt for deleting all table entries. +Useful for testing purposes, but USE WITH CAUTION +``` \ No newline at end of file