From 4cdf605bf2c14344f4e3edbf6d87fd41c4c486bd Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 15 Nov 2023 06:09:35 -0500 Subject: [PATCH 1/9] wip --- src/registrar/forms/domain.py | 86 +++++++++++++++++++++++++++++++++-- src/registrar/views/domain.py | 2 +- 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 6cb5b338f..54df80b14 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -16,6 +16,8 @@ from .common import ( DIGEST_TYPE_CHOICES, ) +import re + class DomainAddUserForm(forms.Form): """Form for adding a user to a domain.""" @@ -228,12 +230,16 @@ class DomainDnssecForm(forms.Form): class DomainDsdataForm(forms.Form): """Form for adding or editing DNSSEC DS Data to a domain.""" + def validate_hexadecimal(value): + if not re.match(r'^[0-9a-fA-F]+$', value): + raise forms.ValidationError('Digest must contain only alphanumeric characters [0-9,a-f].') + key_tag = forms.IntegerField( required=True, label="Key tag", validators=[ - MinValueValidator(0, message="Value must be between 0 and 65535"), - MaxValueValidator(65535, message="Value must be between 0 and 65535"), + MinValueValidator(0, message="Key tag must be less than 65535"), + MaxValueValidator(65535, message="Key tag must be less than 65535"), ], error_messages={"required": ("Key tag is required.")}, ) @@ -257,9 +263,83 @@ class DomainDsdataForm(forms.Form): digest = forms.CharField( required=True, label="Digest", - error_messages={"required": ("Digest is required.")}, + validators=[validate_hexadecimal], + max_length=64, + error_messages={ + "required": "Digest is required.", + "max_length": "Digest must be at most 64 characters long.", + }, ) + 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() + digest_type = cleaned_data.get("digest_type", 0) + digest = cleaned_data.get("digest", "") + # validate length of digest depending on digest_type + if digest_type == 1 and len(digest) != 40: + self.add_error( + "digest", + DsDa) + # remove ANY spaces in the server field + server = server.replace(" ", "") + # lowercase the server + server = server.lower() + cleaned_data["server"] = 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) + + # validate if the form has a server or an ip + if (ip and ip_list) or 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), + ) + elif e.code == nsErrorCodes.MISSING_HOST: + self.add_error( + "server", + NameserverError(code=nsErrorCodes.MISSING_HOST, nameserver=domain, ip=ip_list), + ) + elif e.code == nsErrorCodes.INVALID_HOST: + self.add_error( + "server", + NameserverError(code=nsErrorCodes.INVALID_HOST, nameserver=server, ip=ip_list), + ) + else: + self.add_error("ip", str(e)) + DomainDsdataFormset = formset_factory( DomainDsdataForm, diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 88fad1567..7b8997de0 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -445,7 +445,7 @@ class DomainDsDataView(DomainFormBaseView): modal_button = ( '' + 'name="disable-override-click">Remove all DS Data' ) # context to back out of a broken form on all fields delete From 48caa775dddaeb82c67945e545aa954ab04ee6e6 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 15 Nov 2023 06:54:35 -0500 Subject: [PATCH 2/9] digest validation; error messages; test cases --- src/registrar/forms/domain.py | 75 ++++--------------- src/registrar/tests/test_views.py | 118 +++++++++++++++++++++++++++++- src/registrar/utility/errors.py | 27 ++++++- 3 files changed, 155 insertions(+), 65 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 54df80b14..49a1f0f3c 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -8,6 +8,8 @@ from phonenumber_field.widgets import RegionalPhoneNumberWidget from registrar.utility.errors import ( NameserverError, NameserverErrorCodes as nsErrorCodes, + DsDataError, + DsDataErrorCodes, ) from ..models import Contact, DomainInformation, Domain @@ -232,14 +234,14 @@ class DomainDsdataForm(forms.Form): def validate_hexadecimal(value): if not re.match(r'^[0-9a-fA-F]+$', value): - raise forms.ValidationError('Digest must contain only alphanumeric characters [0-9,a-f].') + raise forms.ValidationError(str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_CHARS))) key_tag = forms.IntegerField( required=True, label="Key tag", validators=[ - MinValueValidator(0, message="Key tag must be less than 65535"), - MaxValueValidator(65535, message="Key tag must be less than 65535"), + MinValueValidator(0, message=str(DsDataError(code=DsDataErrorCodes.INVALID_KEYTAG_SIZE))), + MaxValueValidator(65535, message=str(DsDataError(code=DsDataErrorCodes.INVALID_KEYTAG_SIZE))), ], error_messages={"required": ("Key tag is required.")}, ) @@ -257,7 +259,7 @@ class DomainDsdataForm(forms.Form): label="Digest type", coerce=int, # need to coerce into int so dsData objects can be compared choices=[(None, "--Select--")] + DIGEST_TYPE_CHOICES, # type: ignore - error_messages={"required": ("Digest Type is required.")}, + error_messages={"required": ("Digest type is required.")}, ) digest = forms.CharField( @@ -267,7 +269,7 @@ class DomainDsdataForm(forms.Form): max_length=64, error_messages={ "required": "Digest is required.", - "max_length": "Digest must be at most 64 characters long.", + "max_length": str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_LENGTH)), }, ) @@ -282,64 +284,15 @@ class DomainDsdataForm(forms.Form): if digest_type == 1 and len(digest) != 40: self.add_error( "digest", - DsDa) - # remove ANY spaces in the server field - server = server.replace(" ", "") - # lowercase the server - server = server.lower() - cleaned_data["server"] = 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) - - # validate if the form has a server or an ip - if (ip and ip_list) or server: - self.validate_nameserver_ip_combo(domain, server, ip_list) - + DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_SHA1), + ) + elif digest_type == 2 and len(digest) != 64: + self.add_error( + "digest", + DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_SHA256), + ) 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), - ) - elif e.code == nsErrorCodes.MISSING_HOST: - self.add_error( - "server", - NameserverError(code=nsErrorCodes.MISSING_HOST, nameserver=domain, ip=ip_list), - ) - elif e.code == nsErrorCodes.INVALID_HOST: - self.add_error( - "server", - NameserverError(code=nsErrorCodes.INVALID_HOST, nameserver=server, ip=ip_list), - ) - else: - self.add_error("ip", str(e)) - DomainDsdataFormset = formset_factory( DomainDsdataForm, diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 8f8e161a2..763f9a2ac 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -17,6 +17,8 @@ from registrar.utility.errors import ( SecurityEmailErrorCodes, GenericError, GenericErrorCodes, + DsDataError, + DsDataErrorCodes, ) from registrar.models import ( @@ -1878,7 +1880,30 @@ class TestDomainDNSSEC(TestDomainOverview): self.assertContains(page, "The DS Data records for this domain have been updated.") def test_ds_data_form_invalid(self): - """DS Data form errors with invalid data + """DS Data form errors with invalid data (missing required fields) + + Uses self.app WebTest because we need to interact with forms. + """ + add_data_page = self.app.get(reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_dsdata.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # all four form fields are required, so will test with each blank + add_data_page.forms[0]["form-0-key_tag"] = "" + add_data_page.forms[0]["form-0-algorithm"] = "" + add_data_page.forms[0]["form-0-digest_type"] = "" + add_data_page.forms[0]["form-0-digest"] = "" + with less_console_noise(): # swallow logged warning message + result = add_data_page.forms[0].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, "Key tag is required", count=2, status_code=200) + self.assertContains(result, "Algorithm is required", count=2, status_code=200) + self.assertContains(result, "Digest type is required", count=2, status_code=200) + self.assertContains(result, "Digest is required", count=2, status_code=200) + + def test_ds_data_form_invalid_keytag(self): + """DS Data form errors with invalid data (key tag too large) Uses self.app WebTest because we need to interact with forms. """ @@ -1887,13 +1912,100 @@ class TestDomainDNSSEC(TestDomainOverview): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) # first two nameservers are required, so if we empty one out we should # get a form error - add_data_page.forms[0]["form-0-key_tag"] = "" + add_data_page.forms[0]["form-0-key_tag"] = "65536" # > 65535 + add_data_page.forms[0]["form-0-algorithm"] = "" + add_data_page.forms[0]["form-0-digest_type"] = "" + add_data_page.forms[0]["form-0-digest"] = "" with less_console_noise(): # swallow logged warning message result = add_data_page.forms[0].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, "Key tag is required", count=2, status_code=200) + self.assertContains(result, str(DsDataError(code=DsDataErrorCodes.INVALID_KEYTAG_SIZE)), count=2, status_code=200) + + def test_ds_data_form_invalid_digest_length(self): + """DS Data form errors with invalid data (digest too long) + + Uses self.app WebTest because we need to interact with forms. + """ + add_data_page = self.app.get(reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_dsdata.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # first two nameservers are required, so if we empty one out we should + # get a form error + add_data_page.forms[0]["form-0-key_tag"] = "1234" + add_data_page.forms[0]["form-0-algorithm"] = "3" + add_data_page.forms[0]["form-0-digest_type"] = "1" + add_data_page.forms[0]["form-0-digest"] = "1234567890123456789012345678901234567890123456789012345678901234567890" + with less_console_noise(): # swallow logged warning message + result = add_data_page.forms[0].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, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_LENGTH)), count=2, status_code=200) + + def test_ds_data_form_invalid_digest_chars(self): + """DS Data form errors with invalid data (digest contains non hexadecimal chars) + + Uses self.app WebTest because we need to interact with forms. + """ + add_data_page = self.app.get(reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_dsdata.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # first two nameservers are required, so if we empty one out we should + # get a form error + add_data_page.forms[0]["form-0-key_tag"] = "1234" + add_data_page.forms[0]["form-0-algorithm"] = "3" + add_data_page.forms[0]["form-0-digest_type"] = "1" + add_data_page.forms[0]["form-0-digest"] = "GG1234" + with less_console_noise(): # swallow logged warning message + result = add_data_page.forms[0].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, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_CHARS)), count=2, status_code=200) + + def test_ds_data_form_invalid_digest_sha1(self): + """DS Data form errors with invalid data (digest is invalid sha-1) + + Uses self.app WebTest because we need to interact with forms. + """ + add_data_page = self.app.get(reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_dsdata.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # first two nameservers are required, so if we empty one out we should + # get a form error + add_data_page.forms[0]["form-0-key_tag"] = "1234" + add_data_page.forms[0]["form-0-algorithm"] = "3" + add_data_page.forms[0]["form-0-digest_type"] = "1" # SHA-1 + add_data_page.forms[0]["form-0-digest"] = "A123" + with less_console_noise(): # swallow logged warning message + result = add_data_page.forms[0].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, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_SHA1)), count=2, status_code=200) + + def test_ds_data_form_invalid_digest_sha256(self): + """DS Data form errors with invalid data (digest is invalid sha-256) + + Uses self.app WebTest because we need to interact with forms. + """ + add_data_page = self.app.get(reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_dsdata.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # first two nameservers are required, so if we empty one out we should + # get a form error + add_data_page.forms[0]["form-0-key_tag"] = "1234" + add_data_page.forms[0]["form-0-algorithm"] = "3" + add_data_page.forms[0]["form-0-digest_type"] = "2" # SHA-256 + add_data_page.forms[0]["form-0-digest"] = "GG1234" + with less_console_noise(): # swallow logged warning message + result = add_data_page.forms[0].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, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_SHA256)), count=2, status_code=200) class TestApplicationStatus(TestWithUser, WebTest): diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 13506e8a0..50d62933c 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -125,9 +125,19 @@ class DsDataErrorCodes(IntEnum): error mapping. Overview of ds data error codes: - 1 BAD_DATA bad data input in ds data + - 2 INVALID_DIGEST_SHA1 invalid digest for digest type SHA-1 + - 3 INVALID_DIGEST_SHA256 invalid digest for digest type SHA-256 + - 4 INVALID_DIGEST_LENGTH invalid digest length exceeds 64 + - 5 INVALID_DIGEST_CHARS invalid chars in digest + - 6 INVALID_KEYTAG_SIZE invalid key tag size > 65535 """ BAD_DATA = 1 + INVALID_DIGEST_SHA1 = 2 + INVALID_DIGEST_SHA256 = 3 + INVALID_DIGEST_LENGTH = 4 + INVALID_DIGEST_CHARS = 5 + INVALID_KEYTAG_SIZE = 6 class DsDataError(Exception): @@ -139,7 +149,22 @@ class DsDataError(Exception): _error_mapping = { DsDataErrorCodes.BAD_DATA: ( "There’s something wrong with the DS data you provided. If you need help email us at help@get.gov." - ) + ), + DsDataErrorCodes.INVALID_DIGEST_SHA1: ( + "SHA-1 digest must be exactly 40 characters." + ), + DsDataErrorCodes.INVALID_DIGEST_SHA256: ( + "SHA-256 digest must be exactly 64 characters." + ), + DsDataErrorCodes.INVALID_DIGEST_LENGTH: ( + "Digest must be at most 64 characters." + ), + DsDataErrorCodes.INVALID_DIGEST_CHARS: ( + "Digest must contain only alphanumeric characters [0-9,a-f]." + ), + DsDataErrorCodes.INVALID_KEYTAG_SIZE: ( + "Key tag must be less than 65535" + ), } def __init__(self, *args, code=None, **kwargs): From 9785bed0655b5c6faa6818d4190f3ac650873866 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 15 Nov 2023 07:50:17 -0500 Subject: [PATCH 3/9] formatted for linter --- src/registrar/forms/domain.py | 2 +- src/registrar/tests/test_views.py | 24 ++++++++++++++++++------ src/registrar/utility/errors.py | 20 +++++--------------- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 49a1f0f3c..1ef6ce354 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -233,7 +233,7 @@ class DomainDsdataForm(forms.Form): """Form for adding or editing DNSSEC DS Data to a domain.""" def validate_hexadecimal(value): - if not re.match(r'^[0-9a-fA-F]+$', value): + if not re.match(r"^[0-9a-fA-F]+$", value): raise forms.ValidationError(str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_CHARS))) key_tag = forms.IntegerField( diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 763f9a2ac..334fd2824 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1921,7 +1921,9 @@ class TestDomainDNSSEC(TestDomainOverview): # 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, str(DsDataError(code=DsDataErrorCodes.INVALID_KEYTAG_SIZE)), count=2, status_code=200) + self.assertContains( + result, str(DsDataError(code=DsDataErrorCodes.INVALID_KEYTAG_SIZE)), count=2, status_code=200 + ) def test_ds_data_form_invalid_digest_length(self): """DS Data form errors with invalid data (digest too long) @@ -1936,13 +1938,17 @@ class TestDomainDNSSEC(TestDomainOverview): add_data_page.forms[0]["form-0-key_tag"] = "1234" add_data_page.forms[0]["form-0-algorithm"] = "3" add_data_page.forms[0]["form-0-digest_type"] = "1" - add_data_page.forms[0]["form-0-digest"] = "1234567890123456789012345678901234567890123456789012345678901234567890" + add_data_page.forms[0][ + "form-0-digest" + ] = "1234567890123456789012345678901234567890123456789012345678901234567890" with less_console_noise(): # swallow logged warning message result = add_data_page.forms[0].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, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_LENGTH)), count=2, status_code=200) + self.assertContains( + result, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_LENGTH)), count=2, status_code=200 + ) def test_ds_data_form_invalid_digest_chars(self): """DS Data form errors with invalid data (digest contains non hexadecimal chars) @@ -1963,7 +1969,9 @@ class TestDomainDNSSEC(TestDomainOverview): # 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, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_CHARS)), count=2, status_code=200) + self.assertContains( + result, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_CHARS)), count=2, status_code=200 + ) def test_ds_data_form_invalid_digest_sha1(self): """DS Data form errors with invalid data (digest is invalid sha-1) @@ -1984,7 +1992,9 @@ class TestDomainDNSSEC(TestDomainOverview): # 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, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_SHA1)), count=2, status_code=200) + self.assertContains( + result, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_SHA1)), count=2, status_code=200 + ) def test_ds_data_form_invalid_digest_sha256(self): """DS Data form errors with invalid data (digest is invalid sha-256) @@ -2005,7 +2015,9 @@ class TestDomainDNSSEC(TestDomainOverview): # 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, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_SHA256)), count=2, status_code=200) + self.assertContains( + result, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_SHA256)), count=2, status_code=200 + ) class TestApplicationStatus(TestWithUser, WebTest): diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 50d62933c..2f9daa37d 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -150,21 +150,11 @@ class DsDataError(Exception): DsDataErrorCodes.BAD_DATA: ( "There’s something wrong with the DS data you provided. If you need help email us at help@get.gov." ), - DsDataErrorCodes.INVALID_DIGEST_SHA1: ( - "SHA-1 digest must be exactly 40 characters." - ), - DsDataErrorCodes.INVALID_DIGEST_SHA256: ( - "SHA-256 digest must be exactly 64 characters." - ), - DsDataErrorCodes.INVALID_DIGEST_LENGTH: ( - "Digest must be at most 64 characters." - ), - DsDataErrorCodes.INVALID_DIGEST_CHARS: ( - "Digest must contain only alphanumeric characters [0-9,a-f]." - ), - DsDataErrorCodes.INVALID_KEYTAG_SIZE: ( - "Key tag must be less than 65535" - ), + DsDataErrorCodes.INVALID_DIGEST_SHA1: ("SHA-1 digest must be exactly 40 characters."), + DsDataErrorCodes.INVALID_DIGEST_SHA256: ("SHA-256 digest must be exactly 64 characters."), + DsDataErrorCodes.INVALID_DIGEST_LENGTH: ("Digest must be at most 64 characters."), + DsDataErrorCodes.INVALID_DIGEST_CHARS: ("Digest must contain only alphanumeric characters [0-9,a-f]."), + DsDataErrorCodes.INVALID_KEYTAG_SIZE: ("Key tag must be less than 65535"), } def __init__(self, *args, code=None, **kwargs): From d109f5a265b0939a8bbcc9a02a881720ca2980ed Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 17 Nov 2023 06:31:46 -0500 Subject: [PATCH 4/9] added comment --- src/registrar/forms/domain.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 1ef6ce354..fb1de8d8d 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -233,6 +233,12 @@ class DomainDsdataForm(forms.Form): """Form for adding or editing DNSSEC DS Data to a domain.""" def validate_hexadecimal(value): + """ + Tests that string matches all hexadecimal values. + + Raise validation error to display error in form + if invalid caracters entered + """ if not re.match(r"^[0-9a-fA-F]+$", value): raise forms.ValidationError(str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_CHARS))) From 464279bf3374580898e4efd358f1ab108bce458b Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 17 Nov 2023 06:35:44 -0500 Subject: [PATCH 5/9] formatted for linter --- src/registrar/forms/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index fb1de8d8d..d15248c10 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -235,7 +235,7 @@ class DomainDsdataForm(forms.Form): def validate_hexadecimal(value): """ Tests that string matches all hexadecimal values. - + Raise validation error to display error in form if invalid caracters entered """ From a737f26c5000ae3d764d5857672023669d54b240 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 21 Nov 2023 10:31:22 -0500 Subject: [PATCH 6/9] updated error messages in security email and nameserver; updated modal in dnssec ds data --- src/registrar/forms/domain.py | 12 ++++++++++-- src/registrar/templates/domain_dsdata.html | 2 +- src/registrar/utility/errors.py | 17 ++++++----------- src/registrar/views/domain.py | 2 +- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index d15248c10..f7b99555c 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -10,6 +10,8 @@ from registrar.utility.errors import ( NameserverErrorCodes as nsErrorCodes, DsDataError, DsDataErrorCodes, + SecurityEmailError, + SecurityEmailErrorCodes, ) from ..models import Contact, DomainInformation, Domain @@ -156,7 +158,13 @@ class ContactForm(forms.ModelForm): class DomainSecurityEmailForm(forms.Form): """Form for adding or editing a security email to a domain.""" - security_email = forms.EmailField(label="Security email", required=False) + security_email = forms.EmailField( + label="Security email", + required=False, + error_messages={ + 'invalid': str(SecurityEmailError(code=SecurityEmailErrorCodes.BAD_DATA)), + } + ) class DomainOrgNameAddressForm(forms.ModelForm): @@ -237,7 +245,7 @@ class DomainDsdataForm(forms.Form): Tests that string matches all hexadecimal values. Raise validation error to display error in form - if invalid caracters entered + if invalid characters entered """ if not re.match(r"^[0-9a-fA-F]+$", value): raise forms.ValidationError(str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_CHARS))) diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index 927628b11..d049675fc 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -114,7 +114,7 @@ aria-describedby="Your DNSSEC records will be deleted from the registry." data-force-action > - {% include 'includes/modal.html' with cancel_button_resets_ds_form=True modal_heading="Warning: You are about to delete all DS records on your domain" modal_description="To fully disable DNSSEC: In addition to deleting your DS records here you’ll also need to delete the DS records at your DNS host. To avoid causing your domain to appear offline you should wait to delete your DS records at your DNS host until the Time to Live (TTL) expires. This is often less than 24 hours, but confirm with your provider." modal_button=modal_button|safe %} + {% include 'includes/modal.html' with cancel_button_resets_ds_form=True modal_heading="Warning: You are about to remove all DS records on your domain" modal_description="To fully disable DNSSEC: In addition to removing your DS records here you’ll also need to delete the DS records at your DNS host. To avoid causing your domain to appear offline you should wait to delete your DS records at your DNS host until the Time to Live (TTL) expires. This is often less than 24 hours, but confirm with your provider." modal_button=modal_button|safe %} {% endblock %} {# domain_content #} diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 2f9daa37d..25148e346 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -66,20 +66,18 @@ 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 - - 7 INVALID_HOST host is invalid for a nameserver - - 8 BAD_DATA bad data input for nameserver + - 5 MISSING_HOST host is missing for a nameserver + - 6 INVALID_HOST host is invalid for a nameserver + - 7 BAD_DATA bad data input for nameserver """ MISSING_IP = 1 GLUE_RECORD_NOT_ALLOWED = 2 INVALID_IP = 3 TOO_MANY_HOSTS = 4 - UNABLE_TO_UPDATE_DOMAIN = 5 - MISSING_HOST = 6 - INVALID_HOST = 7 - BAD_DATA = 8 + MISSING_HOST = 5 + INVALID_HOST = 6 + BAD_DATA = 7 class NameserverError(Exception): @@ -93,9 +91,6 @@ 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.TOO_MANY_HOSTS: ("Too many hosts provided, you may not have more than 13 nameservers."), - NameserverErrorCodes.UNABLE_TO_UPDATE_DOMAIN: ( - "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."), NameserverErrorCodes.INVALID_HOST: ("Enter a name server in the required format, like ns1.example.com"), NameserverErrorCodes.BAD_DATA: ( diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 616cd9e08..d265f82a9 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -308,7 +308,7 @@ class DomainNameserversView(DomainFormBaseView): except NameserverError as Err: # 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)) + messages.error(self.request, NameserverError(code=nsErrorCodes.BAD_DATA)) logger.error(f"Nameservers error: {Err}") # TODO: registry is not throwing an error when no connection except RegistryError as Err: From c9c9d7078267d7b3a61db74dc90841a4b2e1683d Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 21 Nov 2023 10:39:30 -0500 Subject: [PATCH 7/9] format for linter --- src/registrar/forms/domain.py | 4 ++-- src/registrar/models/domain.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index f7b99555c..282bbc84f 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -162,8 +162,8 @@ class DomainSecurityEmailForm(forms.Form): label="Security email", required=False, error_messages={ - 'invalid': str(SecurityEmailError(code=SecurityEmailErrorCodes.BAD_DATA)), - } + "invalid": str(SecurityEmailError(code=SecurityEmailErrorCodes.BAD_DATA)), + }, ) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index cb3b784e9..3bd2c0349 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -598,7 +598,7 @@ class Domain(TimeStampedModel, DomainHelper): # if unable to update domain raise error and stop if responseCode != ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY: - raise NameserverError(code=nsErrorCodes.UNABLE_TO_UPDATE_DOMAIN) + raise NameserverError(code=nsErrorCodes.BAD_DATA) successTotalNameservers = len(oldNameservers) - deleteCount + addToDomainCount From fa72f31d242ab169018b998215b72fd574392bc9 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 21 Nov 2023 13:24:06 -0500 Subject: [PATCH 8/9] removed maxlength message on ds data digest --- src/registrar/forms/domain.py | 2 -- src/registrar/tests/test_views.py | 24 ------------------------ src/registrar/utility/errors.py | 11 ++++------- 3 files changed, 4 insertions(+), 33 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 282bbc84f..6a9ea2128 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -280,10 +280,8 @@ class DomainDsdataForm(forms.Form): required=True, label="Digest", validators=[validate_hexadecimal], - max_length=64, error_messages={ "required": "Digest is required.", - "max_length": str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_LENGTH)), }, ) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 334fd2824..86397e96b 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1925,30 +1925,6 @@ class TestDomainDNSSEC(TestDomainOverview): result, str(DsDataError(code=DsDataErrorCodes.INVALID_KEYTAG_SIZE)), count=2, status_code=200 ) - def test_ds_data_form_invalid_digest_length(self): - """DS Data form errors with invalid data (digest too long) - - Uses self.app WebTest because we need to interact with forms. - """ - add_data_page = self.app.get(reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_dsdata.id})) - session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # first two nameservers are required, so if we empty one out we should - # get a form error - add_data_page.forms[0]["form-0-key_tag"] = "1234" - add_data_page.forms[0]["form-0-algorithm"] = "3" - add_data_page.forms[0]["form-0-digest_type"] = "1" - add_data_page.forms[0][ - "form-0-digest" - ] = "1234567890123456789012345678901234567890123456789012345678901234567890" - with less_console_noise(): # swallow logged warning message - result = add_data_page.forms[0].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, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_LENGTH)), count=2, status_code=200 - ) def test_ds_data_form_invalid_digest_chars(self): """DS Data form errors with invalid data (digest contains non hexadecimal chars) diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 25148e346..420c616cb 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -122,17 +122,15 @@ class DsDataErrorCodes(IntEnum): - 1 BAD_DATA bad data input in ds data - 2 INVALID_DIGEST_SHA1 invalid digest for digest type SHA-1 - 3 INVALID_DIGEST_SHA256 invalid digest for digest type SHA-256 - - 4 INVALID_DIGEST_LENGTH invalid digest length exceeds 64 - - 5 INVALID_DIGEST_CHARS invalid chars in digest - - 6 INVALID_KEYTAG_SIZE invalid key tag size > 65535 + - 4 INVALID_DIGEST_CHARS invalid chars in digest + - 5 INVALID_KEYTAG_SIZE invalid key tag size > 65535 """ BAD_DATA = 1 INVALID_DIGEST_SHA1 = 2 INVALID_DIGEST_SHA256 = 3 - INVALID_DIGEST_LENGTH = 4 - INVALID_DIGEST_CHARS = 5 - INVALID_KEYTAG_SIZE = 6 + INVALID_DIGEST_CHARS = 4 + INVALID_KEYTAG_SIZE = 5 class DsDataError(Exception): @@ -147,7 +145,6 @@ class DsDataError(Exception): ), DsDataErrorCodes.INVALID_DIGEST_SHA1: ("SHA-1 digest must be exactly 40 characters."), DsDataErrorCodes.INVALID_DIGEST_SHA256: ("SHA-256 digest must be exactly 64 characters."), - DsDataErrorCodes.INVALID_DIGEST_LENGTH: ("Digest must be at most 64 characters."), DsDataErrorCodes.INVALID_DIGEST_CHARS: ("Digest must contain only alphanumeric characters [0-9,a-f]."), DsDataErrorCodes.INVALID_KEYTAG_SIZE: ("Key tag must be less than 65535"), } From 0279796bb48069d9df203f64af9e2f157cded6f6 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 21 Nov 2023 13:27:52 -0500 Subject: [PATCH 9/9] removed an extraneous blank line --- src/registrar/tests/test_views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 86397e96b..936c344f7 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1925,7 +1925,6 @@ class TestDomainDNSSEC(TestDomainOverview): result, str(DsDataError(code=DsDataErrorCodes.INVALID_KEYTAG_SIZE)), count=2, status_code=200 ) - def test_ds_data_form_invalid_digest_chars(self): """DS Data form errors with invalid data (digest contains non hexadecimal chars)