From 3c3f2a0c00c75ff45357d56af6132129ea4c70eb Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Fri, 9 Sep 2022 12:57:34 +0530 Subject: [PATCH 01/11] Added Cname validation without IPaddress --- .../scala/vinyldns/api/domain/DomainValidations.scala | 8 ++++++++ .../api/domain/batch/BatchChangeValidations.scala | 2 +- .../vinyldns/core/domain/DomainValidationErrors.scala | 5 +++++ .../scala/vinyldns/core/domain/SingleChangeError.scala | 3 ++- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala b/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala index 459285267..9fc771d37 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala @@ -27,6 +27,7 @@ import scala.util.matching.Regex Object to house common domain validations */ object DomainValidations { + val validFQDNRegex: Regex = """^(?:([0-9a-zA-Z_]{1,63}|[0-9a-zA-Z_]{1}[0-9a-zA-Z\-\/_]{0,61}[0-9a-zA-Z_]{1}|[*.]{2}[0-9a-zA-Z\-\/_]{0,60}[0-9a-zA-Z_]{1})\.)*$""".r val validIpv4Regex: Regex = @@ -57,6 +58,13 @@ object DomainValidations { val MX_PREFERENCE_MIN_VALUE: Int = 0 val MX_PREFERENCE_MAX_VALUE: Int = 65535 + // Cname check - Cname should not be IP address + def validateCName(name: Fqdn): ValidatedNel[DomainValidationError, Fqdn] = + validateIpv4Address(name.fqdn.dropRight(1)).isValid match { + case true => InvalidCName(name.toString).invalidNel + case false => validateHostName(name.fqdn).map(_ => name) + } + def validateHostName(name: Fqdn): ValidatedNel[DomainValidationError, Fqdn] = validateHostName(name.fqdn).map(_ => name) diff --git a/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeValidations.scala b/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeValidations.scala index efa72e443..a9d1e9b54 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeValidations.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeValidations.scala @@ -234,7 +234,7 @@ class BatchChangeValidations( record match { case a: AData => validateIpv4Address(a.address).asUnit case aaaa: AAAAData => validateIpv6Address(aaaa.address).asUnit - case cname: CNAMEData => validateHostName(cname.cname).asUnit + case cname: CNAMEData => validateCName(cname.cname).asUnit case ptr: PTRData => validateHostName(ptr.ptrdname).asUnit case txt: TXTData => validateTxtTextLength(txt.text).asUnit case mx: MXData => diff --git a/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala b/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala index 91486ce79..0590c4225 100644 --- a/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala +++ b/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala @@ -52,6 +52,11 @@ final case class InvalidDomainName(param: String) extends DomainValidationError "joined by dots, and terminated with a dot." } +final case class InvalidCName(param: String) extends DomainValidationError { + def message: String = + s"""Invalid Cname: "$param", valid cname should not be IP address""" +} + final case class InvalidLength(param: String, minLengthInclusive: Int, maxLengthInclusive: Int) extends DomainValidationError { def message: String = diff --git a/modules/core/src/main/scala/vinyldns/core/domain/SingleChangeError.scala b/modules/core/src/main/scala/vinyldns/core/domain/SingleChangeError.scala index a7ed47b4e..99731d7db 100644 --- a/modules/core/src/main/scala/vinyldns/core/domain/SingleChangeError.scala +++ b/modules/core/src/main/scala/vinyldns/core/domain/SingleChangeError.scala @@ -36,7 +36,7 @@ object DomainValidationErrorType extends Enumeration { CnameIsNotUniqueError, UserIsNotAuthorized, UserIsNotAuthorizedError, RecordNameNotUniqueInBatch, RecordInReverseZoneError, HighValueDomainError, MissingOwnerGroupId, ExistingMultiRecordError, NewMultiRecordError, CnameAtZoneApexError, RecordRequiresManualReview, UnsupportedOperation, - DeleteRecordDataDoesNotExist = Value + DeleteRecordDataDoesNotExist, InvalidCName = Value // $COVERAGE-OFF$ def from(error: DomainValidationError): DomainValidationErrorType = @@ -72,6 +72,7 @@ object DomainValidationErrorType extends Enumeration { case _: RecordRequiresManualReview => RecordRequiresManualReview case _: UnsupportedOperation => UnsupportedOperation case _: DeleteRecordDataDoesNotExist => DeleteRecordDataDoesNotExist + case _: InvalidCName => InvalidCName } // $COVERAGE-ON$ } From 2180bae712d362d756a4e74255b25ccf180f9e19 Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Fri, 9 Sep 2022 15:07:29 +0530 Subject: [PATCH 02/11] Added tests --- .../api/domain/DomainValidationsSpec.scala | 51 ++++++++++++++++++- .../batch/BatchChangeValidationsSpec.scala | 15 ++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala index b547dec43..64ce6b3a8 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala @@ -23,7 +23,7 @@ import org.scalatest._ import org.scalatest.propspec.AnyPropSpec import org.scalatest.matchers.should.Matchers import vinyldns.api.ValidationTestImprovements._ -import vinyldns.core.domain.{InvalidDomainName, InvalidLength} +import vinyldns.core.domain.{Fqdn, InvalidCName, InvalidDomainName, InvalidLength} class DomainValidationsSpec extends AnyPropSpec @@ -78,6 +78,55 @@ class DomainValidationsSpec validateHostName("asterisk.domain*.name.") shouldBe invalid } + property("Shortests fqdn name should be valid") { + val fqdn = Fqdn("a.") + validateCName(fqdn) shouldBe valid + } + + property("Ip address in cname should be invalid") { + val fqdn = Fqdn("1.2.3.4") + println(validateCName(fqdn)) + validateCName(fqdn) shouldBe invalid + } + + property("Longest fqdn name should be valid") { + val fqdn = Fqdn(("a" * 50 + ".") * 5) + validateCName(fqdn) shouldBe valid + } + + property("fqdn name should pass property-based testing") { + forAll(domainGenerator) { domain: String => + val domains= Fqdn(domain) + whenever(validateCName(domains).isValid) { + domains.fqdn.length should be > 0 + domains.fqdn.length should be < 256 + (domains.fqdn should fullyMatch).regex(validFQDNRegex) + domains.fqdn should endWith(".") + } + } + } + + property("fqdn names beginning with invalid characters should fail with InvalidDomainName") { + validateCName(Fqdn("/slash.domain.name.")).failWith[InvalidCName] + validateCName(Fqdn("-hyphen.domain.name.")).failWith[InvalidCName] + } + + property("fqdn names with underscores should pass property-based testing") { + validateCName(Fqdn("_underscore.domain.name.")).isValid + validateCName(Fqdn("under_score.domain.name.")).isValid + validateCName(Fqdn("underscore._domain.name.")).isValid + } + + // For wildcard records. '*' can only be in the beginning followed by '.' and domain name + property("fqdn names beginning with asterisk should pass property-based testing") { + validateCName(Fqdn("*.domain.name.")) shouldBe valid + validateCName(Fqdn("aste*risk.domain.name.")) shouldBe invalid + validateCName(Fqdn("*asterisk.domain.name.")) shouldBe invalid + validateCName(Fqdn("asterisk*.domain.name.")) shouldBe invalid + validateCName(Fqdn("asterisk.*domain.name."))shouldBe invalid + validateCName(Fqdn("asterisk.domain*.name.")) shouldBe invalid + } + property("Valid Ipv4 addresses should pass property-based testing") { forAll(validIpv4Gen) { validIp: String => val res = validateIpv4Address(validIp) diff --git a/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala index f322b2523..8ce5bf987 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala @@ -715,6 +715,21 @@ class BatchChangeValidationsSpec result should haveInvalid[DomainValidationError](InvalidDomainName(s"$invalidCNAMERecordData.")) } + property("""validateAddChangeInput: should fail with Invalid CNAME + |if validateRecordData fails for IPv4 Address in CNAME record data""".stripMargin) { + val invalidCNAMERecordData = "1.2.3.4" + val change = + AddChangeInput( + "test.comcast.com.", + RecordType.CNAME, + ttl, + CNAMEData(Fqdn(invalidCNAMERecordData)) + ) + val result = validateAddChangeInput(change, false) + + result should haveInvalid[DomainValidationError](InvalidCName(s"Fqdn($invalidCNAMERecordData.)")) + } + property("""validateAddChangeInput: should fail with InvalidLength |if validateRecordData fails for invalid CNAME record data""".stripMargin) { val invalidCNAMERecordData = "s" * 256 From 813138b5971723d6610c0c0c40adc9dbe4621b45 Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Fri, 9 Sep 2022 15:48:43 +0530 Subject: [PATCH 03/11] update --- .../scala/vinyldns/api/domain/DomainValidationsSpec.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala index 64ce6b3a8..a7ad20263 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala @@ -23,7 +23,7 @@ import org.scalatest._ import org.scalatest.propspec.AnyPropSpec import org.scalatest.matchers.should.Matchers import vinyldns.api.ValidationTestImprovements._ -import vinyldns.core.domain.{Fqdn, InvalidCName, InvalidDomainName, InvalidLength} +import vinyldns.core.domain.{Fqdn, InvalidDomainName, InvalidLength} class DomainValidationsSpec extends AnyPropSpec @@ -107,8 +107,8 @@ class DomainValidationsSpec } property("fqdn names beginning with invalid characters should fail with InvalidDomainName") { - validateCName(Fqdn("/slash.domain.name.")).failWith[InvalidCName] - validateCName(Fqdn("-hyphen.domain.name.")).failWith[InvalidCName] + validateCName(Fqdn("/slash.domain.name.")).failWith[InvalidDomainName] + validateCName(Fqdn("-hyphen.domain.name.")).failWith[InvalidDomainName] } property("fqdn names with underscores should pass property-based testing") { From f29ae4af8b761dba5e878d34377560a915b6b9ec Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Fri, 9 Sep 2022 17:13:04 +0530 Subject: [PATCH 04/11] Added func tests --- .../test/functional/tests/batch/create_batch_change_test.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/api/src/test/functional/tests/batch/create_batch_change_test.py b/modules/api/src/test/functional/tests/batch/create_batch_change_test.py index 3a56caf63..c8d8b5b3a 100644 --- a/modules/api/src/test/functional/tests/batch/create_batch_change_test.py +++ b/modules/api/src/test/functional/tests/batch/create_batch_change_test.py @@ -1945,7 +1945,8 @@ def test_cname_recordtype_add_checks(shared_zone_test_context): get_change_CNAME_json(existing_forward_fqdn), get_change_CNAME_json(existing_cname_fqdn), get_change_CNAME_json(f"0.{ip4_zone_name}", cname="duplicate.in.db."), - get_change_CNAME_json(f"user-add-unauthorized.{dummy_zone_name}") + get_change_CNAME_json(f"user-add-unauthorized.{dummy_zone_name}"), + get_change_CNAME_json(f"invalid-ipv4-{parent_zone_name}", cname="1.2.3.4") ] } @@ -2021,6 +2022,9 @@ def test_cname_recordtype_add_checks(shared_zone_test_context): assert_failed_change_in_error_response(response[16], input_name=f"user-add-unauthorized.{dummy_zone_name}", record_type="CNAME", record_data="test.com.", error_messages=[f"User \"ok\" is not authorized. Contact zone owner group: {dummy_group_name} at test@test.com to make DNS changes."]) + assert_failed_change_in_error_response(response[17], input_name=f"invalid-ipv4-{parent_zone_name}", record_type="CNAME", record_data="1.2.3.4.", + error_messages=[f'Invalid Cname: "Fqdn(1.2.3.4.)", valid cname should not be IP address']) + finally: clear_recordset_list(to_delete, client) From ff674219a288df923b7baa5fc97fb4550a0b3d3d Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Thu, 22 Sep 2022 12:36:00 +0530 Subject: [PATCH 05/11] typo corrections --- .../test/scala/vinyldns/api/domain/DomainValidationsSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala index a7ad20263..b101282c5 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala @@ -78,7 +78,7 @@ class DomainValidationsSpec validateHostName("asterisk.domain*.name.") shouldBe invalid } - property("Shortests fqdn name should be valid") { + property("Shortest fqdn name should be valid") { val fqdn = Fqdn("a.") validateCName(fqdn) shouldBe valid } From a6329251bf7dae67a09fd4cc2a0967f4c28c0a3a Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Thu, 22 Sep 2022 12:40:45 +0530 Subject: [PATCH 06/11] Resolved conflicts --- .../src/main/scala/vinyldns/api/domain/DomainValidations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala b/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala index 019b41f91..b60011bf6 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala @@ -65,7 +65,7 @@ object DomainValidations { // Cname check - Cname should not be IP address def validateCName(name: Fqdn): ValidatedNel[DomainValidationError, Fqdn] = validateIpv4Address(name.fqdn.dropRight(1)).isValid match { - case true => InvalidCName(name.toString).invalidNel + case true => InvalidIPv4CName(name.toString).invalidNel case false => validateHostName(name.fqdn).map(_ => name) } From 05c676e7e3cccba3e1333d55f735b826ab2a8c22 Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Thu, 22 Sep 2022 12:51:07 +0530 Subject: [PATCH 07/11] update --- .../main/scala/vinyldns/core/domain/SingleChangeError.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/core/src/main/scala/vinyldns/core/domain/SingleChangeError.scala b/modules/core/src/main/scala/vinyldns/core/domain/SingleChangeError.scala index 862f4ae7e..8811478be 100644 --- a/modules/core/src/main/scala/vinyldns/core/domain/SingleChangeError.scala +++ b/modules/core/src/main/scala/vinyldns/core/domain/SingleChangeError.scala @@ -36,7 +36,7 @@ object DomainValidationErrorType extends Enumeration { CnameIsNotUniqueError, UserIsNotAuthorized, UserIsNotAuthorizedError, RecordNameNotUniqueInBatch, RecordInReverseZoneError, HighValueDomainError, MissingOwnerGroupId, ExistingMultiRecordError, NewMultiRecordError, CnameAtZoneApexError, RecordRequiresManualReview, UnsupportedOperation, - DeleteRecordDataDoesNotExist, InvalidCName = Value + DeleteRecordDataDoesNotExist, InvalidIPv4CName = Value // $COVERAGE-OFF$ def from(error: DomainValidationError): DomainValidationErrorType = @@ -73,7 +73,7 @@ object DomainValidationErrorType extends Enumeration { case _: RecordRequiresManualReview => RecordRequiresManualReview case _: UnsupportedOperation => UnsupportedOperation case _: DeleteRecordDataDoesNotExist => DeleteRecordDataDoesNotExist - case _: InvalidCName => InvalidCName + case _: InvalidIPv4CName => InvalidIPv4CName } // $COVERAGE-ON$ } From 3303de2548b3fb9cb237d07e5275f2859985ad83 Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Thu, 22 Sep 2022 13:12:06 +0530 Subject: [PATCH 08/11] update --- .../vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala index 35f10af94..60213d33f 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala @@ -727,7 +727,7 @@ class BatchChangeValidationsSpec ) val result = validateAddChangeInput(change, false) - result should haveInvalid[DomainValidationError](InvalidCName(s"Fqdn($invalidCNAMERecordData.)")) + result should haveInvalid[DomainValidationError](InvalidIPv4CName(s"Fqdn($invalidCNAMERecordData.)")) } property("""validateAddChangeInput: should fail with InvalidLength From 08e2d6b8672f83eee55d7a8222b98c35d1cf8386 Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Thu, 22 Sep 2022 15:27:05 +0530 Subject: [PATCH 09/11] update in tests --- .../api/domain/DomainValidations.scala | 10 +-- .../api/domain/DomainValidationsSpec.scala | 89 +++++++++---------- 2 files changed, 49 insertions(+), 50 deletions(-) diff --git a/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala b/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala index b60011bf6..091b79bcd 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala @@ -63,19 +63,19 @@ object DomainValidations { val MX_PREFERENCE_MAX_VALUE: Int = 65535 // Cname check - Cname should not be IP address - def validateCName(name: Fqdn): ValidatedNel[DomainValidationError, Fqdn] = + def validateCname(name: Fqdn, isReverse: Boolean): ValidatedNel[DomainValidationError, Fqdn] = validateIpv4Address(name.fqdn.dropRight(1)).isValid match { case true => InvalidIPv4CName(name.toString).invalidNel - case false => validateHostName(name.fqdn).map(_ => name) + case false => validateIsReverseCname(name, isReverse) } def validateHostName(name: Fqdn): ValidatedNel[DomainValidationError, Fqdn] = validateHostName(name.fqdn).map(_ => name) - def validateCname(name: Fqdn, isReverse: Boolean): ValidatedNel[DomainValidationError, Fqdn] = - validateCname(name.fqdn, isReverse).map(_ => name) + def validateIsReverseCname(name: Fqdn, isReverse: Boolean): ValidatedNel[DomainValidationError, Fqdn] = + validateIsReverseCname(name.fqdn, isReverse).map(_ => name) - def validateCname(name: String, isReverse: Boolean): ValidatedNel[DomainValidationError, String] = { + def validateIsReverseCname(name: String, isReverse: Boolean): ValidatedNel[DomainValidationError, String] = { isReverse match { case true => val checkRegex = validReverseZoneFQDNRegex diff --git a/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala index e447c2772..68dc9535e 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/DomainValidationsSpec.scala @@ -79,24 +79,23 @@ class DomainValidationsSpec property("Shortest fqdn name should be valid") { val fqdn = Fqdn("a.") - validateCName(fqdn) shouldBe valid + validateCname(fqdn, false) shouldBe valid } property("Ip address in cname should be invalid") { val fqdn = Fqdn("1.2.3.4") - println(validateCName(fqdn)) - validateCName(fqdn) shouldBe invalid + validateCname(fqdn, false) shouldBe invalid } property("Longest fqdn name should be valid") { val fqdn = Fqdn(("a" * 50 + ".") * 5) - validateCName(fqdn) shouldBe valid + validateCname(fqdn, false) shouldBe valid } property("fqdn name should pass property-based testing") { forAll(domainGenerator) { domain: String => val domains= Fqdn(domain) - whenever(validateCName(domains).isValid) { + whenever(validateHostName(domains).isValid) { domains.fqdn.length should be > 0 domains.fqdn.length should be < 256 (domains.fqdn should fullyMatch).regex(validFQDNRegex) @@ -105,25 +104,25 @@ class DomainValidationsSpec } } - property("fqdn names beginning with invalid characters should fail with InvalidDomainName") { - validateCName(Fqdn("/slash.domain.name.")).failWith[InvalidDomainName] - validateCName(Fqdn("-hyphen.domain.name.")).failWith[InvalidDomainName] + property("fqdn names beginning with invalid characters should fail with InvalidCname") { + validateCname(Fqdn("/slash.domain.name."), false).failWith[InvalidCname] + validateCname(Fqdn("-hyphen.domain.name."), false).failWith[InvalidCname] } property("fqdn names with underscores should pass property-based testing") { - validateCName(Fqdn("_underscore.domain.name.")).isValid - validateCName(Fqdn("under_score.domain.name.")).isValid - validateCName(Fqdn("underscore._domain.name.")).isValid + validateCname(Fqdn("_underscore.domain.name."), false).isValid + validateCname(Fqdn("under_score.domain.name."), false).isValid + validateCname(Fqdn("underscore._domain.name."), false).isValid } // For wildcard records. '*' can only be in the beginning followed by '.' and domain name property("fqdn names beginning with asterisk should pass property-based testing") { - validateCName(Fqdn("*.domain.name.")) shouldBe valid - validateCName(Fqdn("aste*risk.domain.name.")) shouldBe invalid - validateCName(Fqdn("*asterisk.domain.name.")) shouldBe invalid - validateCName(Fqdn("asterisk*.domain.name.")) shouldBe invalid - validateCName(Fqdn("asterisk.*domain.name."))shouldBe invalid - validateCName(Fqdn("asterisk.domain*.name.")) shouldBe invalid + validateCname(Fqdn("*.domain.name."), false) shouldBe valid + validateCname(Fqdn("aste*risk.domain.name."),false) shouldBe invalid + validateCname(Fqdn("*asterisk.domain.name."),false) shouldBe invalid + validateCname(Fqdn("asterisk*.domain.name."),false) shouldBe invalid + validateCname(Fqdn("asterisk.*domain.name."),false)shouldBe invalid + validateCname(Fqdn("asterisk.domain*.name."),false) shouldBe invalid } property("Valid Ipv4 addresses should pass property-based testing") { @@ -161,50 +160,50 @@ class DomainValidationsSpec } property("Shortest cname should be valid") { - validateCname("a.",true) shouldBe valid - validateCname("a.",false) shouldBe valid + validateIsReverseCname("a.",true) shouldBe valid + validateIsReverseCname("a.",false) shouldBe valid } property("Longest cname should be valid") { val name = ("a" * 50 + ".") * 5 - validateCname(name,true) shouldBe valid - validateCname(name,false) shouldBe valid + validateIsReverseCname(name,true) shouldBe valid + validateIsReverseCname(name,false) shouldBe valid } property("Cnames with underscores should pass property-based testing") { - validateCname("_underscore.domain.name.",true).isValid - validateCname("under_score.domain.name.",true).isValid - validateCname("underscore._domain.name.",true).isValid - validateCname("_underscore.domain.name.",false).isValid - validateCname("under_score.domain.name.",false).isValid - validateCname("underscore._domain.name.",false).isValid + validateIsReverseCname("_underscore.domain.name.",true).isValid + validateIsReverseCname("under_score.domain.name.",true).isValid + validateIsReverseCname("underscore._domain.name.",true).isValid + validateIsReverseCname("_underscore.domain.name.",false).isValid + validateIsReverseCname("under_score.domain.name.",false).isValid + validateIsReverseCname("underscore._domain.name.",false).isValid } // For wildcard records. '*' can only be in the beginning followed by '.' and domain name property("Cnames beginning with asterisk should pass property-based testing") { - validateCname("*.domain.name.",true) shouldBe valid - validateCname("aste*risk.domain.name.",true) shouldBe invalid - validateCname("*asterisk.domain.name.",true) shouldBe invalid - validateCname("asterisk*.domain.name.",true) shouldBe invalid - validateCname("asterisk.*domain.name.",true) shouldBe invalid - validateCname("asterisk.domain*.name.",true) shouldBe invalid - validateCname("*.domain.name.",false) shouldBe valid - validateCname("aste*risk.domain.name.",false) shouldBe invalid - validateCname("*asterisk.domain.name.",false) shouldBe invalid - validateCname("asterisk*.domain.name.",false) shouldBe invalid - validateCname("asterisk.*domain.name.",false) shouldBe invalid - validateCname("asterisk.domain*.name.",false) shouldBe invalid + validateIsReverseCname("*.domain.name.",true) shouldBe valid + validateIsReverseCname("aste*risk.domain.name.",true) shouldBe invalid + validateIsReverseCname("*asterisk.domain.name.",true) shouldBe invalid + validateIsReverseCname("asterisk*.domain.name.",true) shouldBe invalid + validateIsReverseCname("asterisk.*domain.name.",true) shouldBe invalid + validateIsReverseCname("asterisk.domain*.name.",true) shouldBe invalid + validateIsReverseCname("*.domain.name.",false) shouldBe valid + validateIsReverseCname("aste*risk.domain.name.",false) shouldBe invalid + validateIsReverseCname("*asterisk.domain.name.",false) shouldBe invalid + validateIsReverseCname("asterisk*.domain.name.",false) shouldBe invalid + validateIsReverseCname("asterisk.*domain.name.",false) shouldBe invalid + validateIsReverseCname("asterisk.domain*.name.",false) shouldBe invalid } property("Cname names with forward slash should pass with reverse zone") { - validateCname("/slash.cname.name.",true).isValid - validateCname("slash./cname.name.",true).isValid - validateCname("slash.cname./name.",true).isValid + validateIsReverseCname("/slash.cname.name.",true).isValid + validateIsReverseCname("slash./cname.name.",true).isValid + validateIsReverseCname("slash.cname./name.",true).isValid } property("Cname names with forward slash should fail with forward zone") { - validateCname("/slash.cname.name.",false).failWith[InvalidCname] - validateCname("slash./cname.name.",false).failWith[InvalidCname] - validateCname("slash.cname./name.",false).failWith[InvalidCname] + validateIsReverseCname("/slash.cname.name.",false).failWith[InvalidCname] + validateIsReverseCname("slash./cname.name.",false).failWith[InvalidCname] + validateIsReverseCname("slash.cname./name.",false).failWith[InvalidCname] } } From 068d408e7a52b01b5b88323b01654f91ae1cf131 Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Tue, 7 Mar 2023 16:26:00 +0530 Subject: [PATCH 10/11] Updated the error message --- .../src/test/functional/tests/batch/create_batch_change_test.py | 2 +- .../scala/vinyldns/core/domain/DomainValidationErrors.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/api/src/test/functional/tests/batch/create_batch_change_test.py b/modules/api/src/test/functional/tests/batch/create_batch_change_test.py index 094e66c9f..9e2c698e5 100644 --- a/modules/api/src/test/functional/tests/batch/create_batch_change_test.py +++ b/modules/api/src/test/functional/tests/batch/create_batch_change_test.py @@ -2023,7 +2023,7 @@ def test_cname_recordtype_add_checks(shared_zone_test_context): record_type="CNAME", record_data="test.com.", error_messages=[f"User \"ok\" is not authorized. Contact zone owner group: {dummy_group_name} at test@test.com to make DNS changes."]) assert_failed_change_in_error_response(response[17], input_name=f"invalid-ipv4-{parent_zone_name}", record_type="CNAME", record_data="1.2.3.4.", - error_messages=[f'Invalid Cname: "Fqdn(1.2.3.4.)", valid cname should not be IP address']) + error_messages=[f'Invalid Cname: "Fqdn(1.2.3.4.)", Valid CNAME record data should not be an IP address']) finally: clear_recordset_list(to_delete, client) diff --git a/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala b/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala index ad77e2675..4ec438255 100644 --- a/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala +++ b/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala @@ -54,7 +54,7 @@ final case class InvalidDomainName(param: String) extends DomainValidationError final case class InvalidIPv4CName(param: String) extends DomainValidationError { def message: String = - s"""Invalid Cname: "$param", valid cname should not be IP address""" + s"""Invalid Cname: "$param", Valid CNAME record data should not be an IP address""" } final case class InvalidCname(param: String, isReverseZone: Boolean) extends DomainValidationError { From 844cd66becffb810aa0b3b932d2e3d30fdf27e6e Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Tue, 7 Mar 2023 17:52:24 +0530 Subject: [PATCH 11/11] dummy commit --- .../scala/vinyldns/api/domain/record/RecordSetServiceSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/api/src/test/scala/vinyldns/api/domain/record/RecordSetServiceSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/record/RecordSetServiceSpec.scala index 3d629e43b..726f6703c 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/record/RecordSetServiceSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/record/RecordSetServiceSpec.scala @@ -626,7 +626,7 @@ class RecordSetServiceSpec .getRecordSetsByName(zone.id, newRecord.name) val result = rightResultOf( - underTest.updateRecordSet(newRecord, auth).map(_.asInstanceOf[RecordSetChange]).value + underTest.updateRecordSet(newRecord, auth).map(_.asInstanceOf[RecordSetChange]).value ) result.recordSet.ttl shouldBe newRecord.ttl