From 9014570a375a643b04c8125f433d25df8c1106be Mon Sep 17 00:00:00 2001 From: Michael Ly Date: Wed, 7 Aug 2019 10:33:18 -0400 Subject: [PATCH] Implement domains requiring manual review (via batch change interface) (#779) * Implement domains requiring review. * Update configs. * Update tests. --- docker/api/docker.conf | 15 ++++ .../zones/1.9.e.f.c.c.7.2.9.6.d.f.ip6.arpa | 1 + docker/bind9/zones/2.0.192.in-addr.arpa | 1 + .../batch/approve_batch_change_test.py | 10 ++- .../batch/create_batch_change_test.py | 46 ++++++++++ .../api/src/main/resources/application.conf | 15 ++++ .../scala/vinyldns/api/VinylDNSConfig.scala | 7 ++ .../api/domain/batch/BatchChangeService.scala | 12 ++- .../domain/batch/BatchChangeValidations.scala | 41 +++++++-- .../domain/zone/ZoneRecordValidations.scala | 24 ++++- .../api/src/test/resources/application.conf | 15 ++++ .../batch/BatchChangeValidationsSpec.scala | 87 ++++++++++++------- .../core/domain/DomainValidationErrors.scala | 9 ++ .../core/domain/SingleChangeError.scala | 3 +- 14 files changed, 238 insertions(+), 48 deletions(-) diff --git a/docker/api/docker.conf b/docker/api/docker.conf index 2de292dc9..048e67db7 100644 --- a/docker/api/docker.conf +++ b/docker/api/docker.conf @@ -207,6 +207,21 @@ vinyldns { ] } + # FQDNs / IPs that require manual review upon submission in batch change interface + # domain-list used for all record types except PTR + # ip-list used exclusively for PTR records + manual-review-domains = { + domain-list = [ + "needs-review.*" + ] + ip-list = [ + "192.0.2.254", + "192.0.2.255", + "fd69:27cc:fe91:0:0:0:ffff:1", + "fd69:27cc:fe91:0:0:0:ffff:2" + ] + } + # types of unowned records that users can access in shared zones shared-approved-types = ["A", "AAAA", "CNAME", "PTR", "TXT"] diff --git a/docker/bind9/zones/1.9.e.f.c.c.7.2.9.6.d.f.ip6.arpa b/docker/bind9/zones/1.9.e.f.c.c.7.2.9.6.d.f.ip6.arpa index 3bee39117..8f3b4ee4a 100755 --- a/docker/bind9/zones/1.9.e.f.c.c.7.2.9.6.d.f.ip6.arpa +++ b/docker/bind9/zones/1.9.e.f.c.c.7.2.9.6.d.f.ip6.arpa @@ -9,3 +9,4 @@ $ttl 38400 4.2.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0 IN PTR www.vinyldns. 5.2.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0 IN PTR mail.vinyldns. 0.0.0.0.f.f.f.f.0.0.0.0.0.0.0.0.0.0.0.0 IN PTR high.value.domain.ip6. +2.0.0.0.f.f.f.f.0.0.0.0.0.0.0.0.0.0.0.0 IN PTR needs.review.domain.ip6. diff --git a/docker/bind9/zones/2.0.192.in-addr.arpa b/docker/bind9/zones/2.0.192.in-addr.arpa index 8ddd1d882..d81c79f72 100644 --- a/docker/bind9/zones/2.0.192.in-addr.arpa +++ b/docker/bind9/zones/2.0.192.in-addr.arpa @@ -12,3 +12,4 @@ $ttl 38400 194 IN CNAME 194.192/30.2.0.192.in-addr.arpa. 195 IN CNAME 195.192/30.2.0.192.in-addr.arpa. 253 IN PTR high.value.domain.ip4. +255 IN PTR needs.review.domain.ip4 diff --git a/modules/api/functional_test/live_tests/batch/approve_batch_change_test.py b/modules/api/functional_test/live_tests/batch/approve_batch_change_test.py index 0f716c964..e83d7554e 100644 --- a/modules/api/functional_test/live_tests/batch/approve_batch_change_test.py +++ b/modules/api/functional_test/live_tests/batch/approve_batch_change_test.py @@ -11,7 +11,8 @@ def test_approve_pending_batch_change_success(shared_zone_test_context): approver = shared_zone_test_context.support_user_client batch_change_input = { "changes": [ - get_change_A_AAAA_json("test-approve-success.not.loaded.", address="4.3.2.1") + get_change_A_AAAA_json("test-approve-success.not.loaded.", address="4.3.2.1"), + get_change_A_AAAA_json("needs-review.not.loaded.", address="4.3.2.1"), ], "ownerGroupId": shared_zone_test_context.ok_group['id'] } @@ -25,6 +26,8 @@ def test_approve_pending_batch_change_success(shared_zone_test_context): assert_that(get_batch['approvalStatus'], is_('PendingReview')) assert_that(get_batch['changes'][0]['status'], is_('NeedsReview')) assert_that(get_batch['changes'][0]['validationErrors'][0]['errorType'], is_('ZoneDiscoveryError')) + assert_that(get_batch['changes'][1]['status'], is_('NeedsReview')) + assert_that(get_batch['changes'][1]['validationErrors'][0]['errorType'], is_('RecordRequiresManualReview')) # need to create the zone so the change can succeed zone = { @@ -43,12 +46,13 @@ def test_approve_pending_batch_change_success(shared_zone_test_context): to_delete = [(change['zoneId'], change['recordSetId']) for change in completed_batch['changes']] assert_that(completed_batch['status'], is_('Complete')) - assert_that(completed_batch['changes'][0]['status'], is_('Complete')) + for change in completed_batch['changes']: + assert_that(change['status'], is_('Complete')) + assert_that(len(change['validationErrors']), is_(0)) assert_that(completed_batch['approvalStatus'], is_('ManuallyApproved')) assert_that(completed_batch['reviewerId'], is_('support-user-id')) assert_that(completed_batch['reviewerUserName'], is_('support-user')) assert_that(completed_batch, has_key('reviewTimestamp')) - assert_that(len(completed_batch['changes'][0]['validationErrors']), is_(0)) finally: clear_zoneid_rsid_tuple_list(to_delete, client) if to_disconnect is not None: diff --git a/modules/api/functional_test/live_tests/batch/create_batch_change_test.py b/modules/api/functional_test/live_tests/batch/create_batch_change_test.py index be5924987..acad7156b 100644 --- a/modules/api/functional_test/live_tests/batch/create_batch_change_test.py +++ b/modules/api/functional_test/live_tests/batch/create_batch_change_test.py @@ -882,6 +882,52 @@ def test_create_batch_change_with_high_value_domain_fails(shared_zone_test_conte assert_that(response[12], is_not(has_key("errors"))) +@pytest.mark.manual_batch_review +def test_create_batch_change_with_domains_requiring_review_succeeds(shared_zone_test_context): + """ + Test creating a batch change with an input name requiring review is accepted + """ + + rejecter = shared_zone_test_context.support_user_client + client = shared_zone_test_context.ok_vinyldns_client + batch_change_input = { + "ownerGroupId": shared_zone_test_context.ok_group['id'], + "comments": "this is optional", + "changes": [ + get_change_A_AAAA_json("needs-review-add.ok."), + get_change_A_AAAA_json("needs-review-update.ok.", change_type="DeleteRecordSet"), + get_change_A_AAAA_json("needs-review-update.ok."), + get_change_A_AAAA_json("needs-review-delete.ok.", change_type="DeleteRecordSet"), + get_change_PTR_json("192.0.2.254"), + get_change_PTR_json("192.0.2.255", change_type="DeleteRecordSet"), # 255 exists already + get_change_PTR_json("192.0.2.255"), + get_change_PTR_json("192.0.2.255", change_type="DeleteRecordSet"), + get_change_PTR_json("fd69:27cc:fe91:0:0:0:ffff:1"), + get_change_PTR_json("fd69:27cc:fe91:0:0:0:ffff:2", change_type="DeleteRecordSet"), # ffff:2 exists already + get_change_PTR_json("fd69:27cc:fe91:0:0:0:ffff:2"), + get_change_PTR_json("fd69:27cc:fe91:0:0:0:ffff:2", change_type="DeleteRecordSet"), + + get_change_A_AAAA_json("i-can-be-touched.ok.", address="1.1.1.1") + ] + } + response = None + + try: + response = client.create_batch_change(batch_change_input, status=202) + get_batch = client.get_batch_change(response['id']) + assert_that(get_batch['status'], is_('PendingReview')) + assert_that(get_batch['approvalStatus'], is_('PendingReview')) + for i in xrange(1, 11): + assert_that(get_batch['changes'][i]['status'], is_('NeedsReview')) + assert_that(get_batch['changes'][i]['validationErrors'][0]['errorType'], is_('RecordRequiresManualReview')) + assert_that(get_batch['changes'][12]['validationErrors'], empty()) + + finally: + # Clean up so data doesn't change + if response: + rejecter.reject_batch_change(response['id'], status=200) + + def test_create_batch_change_with_invalid_record_type_fails(shared_zone_test_context): """ Test creating a batch change with invalid record type fails diff --git a/modules/api/src/main/resources/application.conf b/modules/api/src/main/resources/application.conf index fa96c82c1..3b790ef87 100644 --- a/modules/api/src/main/resources/application.conf +++ b/modules/api/src/main/resources/application.conf @@ -113,6 +113,21 @@ vinyldns { ] } + # FQDNs / IPs that require manual review upon submission in batch change interface + # domain-list used for all record types except PTR + # ip-list used exclusively for PTR records + manual-review-domains = { + domain-list = [ + "needs-review.*" + ] + ip-list = [ + "192.0.2.254", + "192.0.2.255", + "fd69:27cc:fe91:0:0:0:ffff:1", + "fd69:27cc:fe91:0:0:0:ffff:2" + ] + } + # types of unowned records that users can access in shared zones shared-approved-types = ["A", "AAAA", "CNAME", "PTR", "TXT"] diff --git a/modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala b/modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala index feada0fba..34b5286b1 100644 --- a/modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala +++ b/modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala @@ -137,4 +137,11 @@ object VinylDNSConfig { lazy val scheduledChangesEnabled: Boolean = vinyldnsConfig .as[Option[Boolean]]("scheduled-changes-enabled") .getOrElse(false) + + lazy val domainListRequiringManualReview: List[Regex] = + ZoneRecordValidations.toCaseIgnoredRegexList( + getOptionalStringList("manual-review-domains.domain-list")) + + lazy val ipListRequiringManualReview: List[IpAddress] = + getOptionalStringList("manual-review-domains.ip-list").flatMap(ip => IpAddress(ip)) } diff --git a/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeService.scala b/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeService.scala index 4807c0ad2..aea4d2c61 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeService.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeService.scala @@ -91,7 +91,10 @@ class BatchChangeService( auth: AuthPrincipal, allowManualReview: Boolean): BatchResult[BatchChange] = for { - validationOutput <- applyBatchChangeValidationFlow(batchChangeInput, auth) + validationOutput <- applyBatchChangeValidationFlow( + batchChangeInput, + auth, + isApproved = false) changeForConversion <- buildResponse( batchChangeInput, validationOutput.validatedChanges, @@ -107,11 +110,12 @@ class BatchChangeService( def applyBatchChangeValidationFlow( batchChangeInput: BatchChangeInput, - auth: AuthPrincipal): BatchResult[BatchValidationFlowOutput] = + auth: AuthPrincipal, + isApproved: Boolean): BatchResult[BatchValidationFlowOutput] = for { existingGroup <- getOwnerGroup(batchChangeInput.ownerGroupId) _ <- validateBatchChangeInput(batchChangeInput, existingGroup, auth) - inputValidatedSingleChanges = validateInputChanges(batchChangeInput.changes) + inputValidatedSingleChanges = validateInputChanges(batchChangeInput.changes, isApproved) zoneMap <- getZonesForRequest(inputValidatedSingleChanges).toBatchResult changesWithZones = zoneDiscovery(inputValidatedSingleChanges, zoneMap) recordSets <- getExistingRecordSets(changesWithZones, zoneMap).toBatchResult @@ -152,7 +156,7 @@ class BatchChangeService( reviewInfo = BatchChangeReviewInfo( authPrincipal.userId, approveBatchChangeInput.reviewComment) - validationOutput <- applyBatchChangeValidationFlow(asInput, requesterAuth) + validationOutput <- applyBatchChangeValidationFlow(asInput, requesterAuth, isApproved = true) changeForConversion <- buildResponseForApprover( batchChange, asInput, 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 aed35d31c..11d972138 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 @@ -37,7 +37,9 @@ trait BatchChangeValidationsAlgebra { existingGroup: Option[Group], authPrincipal: AuthPrincipal): BatchResult[Unit] - def validateInputChanges(input: List[ChangeInput]): ValidatedBatch[ChangeInput] + def validateInputChanges( + input: List[ChangeInput], + isApproved: Boolean): ValidatedBatch[ChangeInput] def validateChangesWithContext( changes: ValidatedBatch[ChangeForValidation], @@ -143,16 +145,20 @@ class BatchChangeValidations( /* input validations */ - def validateInputChanges(input: List[ChangeInput]): ValidatedBatch[ChangeInput] = + def validateInputChanges( + input: List[ChangeInput], + isApproved: Boolean): ValidatedBatch[ChangeInput] = input.map { - case a: AddChangeInput => validateAddChangeInput(a).map(_ => a) - case d: DeleteChangeInput => validateInputName(d).map(_ => d) + case a: AddChangeInput => validateAddChangeInput(a, isApproved).map(_ => a) + case d: DeleteChangeInput => validateInputName(d, isApproved).map(_ => d) } - def validateAddChangeInput(addChangeInput: AddChangeInput): SingleValidation[Unit] = { + def validateAddChangeInput( + addChangeInput: AddChangeInput, + isApproved: Boolean): SingleValidation[Unit] = { val validTTL = addChangeInput.ttl.map(validateTTL(_).asUnit).getOrElse(().valid) val validRecord = validateRecordData(addChangeInput.record) - val validInput = validateInputName(addChangeInput) + val validInput = validateInputName(addChangeInput, isApproved) validTTL |+| validRecord |+| validInput } @@ -170,7 +176,7 @@ class BatchChangeValidations( InvalidBatchRecordType(other.toString, SupportedBatchChangeRecordTypes.get).invalidNel[Unit] } - def validateInputName(change: ChangeInput): SingleValidation[Unit] = { + def validateInputName(change: ChangeInput, isApproved: Boolean): SingleValidation[Unit] = { val typedChecks = change.typ match { case A | AAAA | MX => validateHostName(change.inputName).asUnit |+| notInReverseZone(change) @@ -181,7 +187,7 @@ class BatchChangeValidations( case other => InvalidBatchRecordType(other.toString, SupportedBatchChangeRecordTypes.get).invalidNel[Unit] } - typedChecks |+| isNotHighValueDomain(change) + typedChecks |+| isNotHighValueDomain(change) |+| doesNotRequireManualReview(change, isApproved) } def validatePtrIp(ip: String): SingleValidation[Unit] = { @@ -489,6 +495,25 @@ class BatchChangeValidations( change.inputName) } + def doesNotRequireManualReview( + change: ChangeInput, + isApproved: Boolean): SingleValidation[Unit] = + if (isApproved) { + // If we are reviewing, don't need to check whether DNS change needs review + ().validNel + } else { + change.typ match { + case RecordType.PTR => + ZoneRecordValidations.ipDoesNotRequireManualReview( + VinylDNSConfig.ipListRequiringManualReview, + change.inputName) + case _ => + ZoneRecordValidations.domainDoesNotRequireManualReview( + VinylDNSConfig.domainListRequiringManualReview, + change.inputName) + } + } + def ownerGroupProvidedIfNeeded( change: AddChangeForValidation, existingRecord: Option[RecordSet], diff --git a/modules/api/src/main/scala/vinyldns/api/domain/zone/ZoneRecordValidations.scala b/modules/api/src/main/scala/vinyldns/api/domain/zone/ZoneRecordValidations.scala index 95dea65b9..eb6e933a8 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/zone/ZoneRecordValidations.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/zone/ZoneRecordValidations.scala @@ -20,7 +20,11 @@ import cats.implicits._ import cats.data._ import com.comcast.ip4s.IpAddress import com.comcast.ip4s.interop.cats.implicits._ -import vinyldns.core.domain.{DomainValidationError, HighValueDomainError} +import vinyldns.core.domain.{ + DomainValidationError, + HighValueDomainError, + RecordRequiresManualReview +} import vinyldns.core.domain.record.{NSData, RecordSet} import scala.util.matching.Regex @@ -76,4 +80,22 @@ object ZoneRecordValidations { } else { HighValueDomainError(ip).invalidNel } + + def domainDoesNotRequireManualReview( + regexList: List[Regex], + fqdn: String): ValidatedNel[DomainValidationError, Unit] = + if (!isStringInRegexList(regexList, fqdn)) { + ().validNel + } else { + RecordRequiresManualReview(fqdn).invalidNel + } + + def ipDoesNotRequireManualReview( + regexList: List[IpAddress], + ip: String): ValidatedNel[DomainValidationError, Unit] = + if (!isIpInIpList(regexList, ip)) { + ().validNel + } else { + RecordRequiresManualReview(ip).invalidNel + } } diff --git a/modules/api/src/test/resources/application.conf b/modules/api/src/test/resources/application.conf index 2146f72d6..62e75c127 100644 --- a/modules/api/src/test/resources/application.conf +++ b/modules/api/src/test/resources/application.conf @@ -41,6 +41,21 @@ vinyldns { ] } + # FQDNs / IPs that require manual review upon submission in batch change interface + # domain-list used for all record types except PTR + # ip-list used exclusively for PTR records + manual-review-domains = { + domain-list = [ + "needs-review.*" + ] + ip-list = [ + "192.0.2.254", + "192.0.2.255", + "fd69:27cc:fe91:0:0:0:ffff:1", + "fd69:27cc:fe91:0:0:0:ffff:2" + ] + } + # types of unowned records that users can access in shared zones shared-approved-types = ["A", "AAAA", "CNAME", "PTR", "TXT"] 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 52561fcf7..d79df9ca0 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 @@ -196,7 +196,7 @@ class BatchChangeValidationsSpec property("validateInputChanges: should succeed if all inputs are good") { forAll(listOfN(3, validAChangeGen)) { input: List[ChangeInput] => - val result = validateInputChanges(input) + val result = validateInputChanges(input, false) result.map(_ shouldBe valid) } } @@ -370,7 +370,9 @@ class BatchChangeValidationsSpec val invalidIpv6Input = AddChangeInput("testbad.example.com.", RecordType.AAAA, ttl, AAAAData("invalidIpv6:123")) val result = - validateInputChanges(List(goodInput, goodAAAAInput, invalidDomainNameInput, invalidIpv6Input)) + validateInputChanges( + List(goodInput, goodAAAAInput, invalidDomainNameInput, invalidIpv6Input), + false) result(0) shouldBe valid result(1) shouldBe valid result(2) should haveInvalid[DomainValidationError](InvalidDomainName("invalidDomainName$.")) @@ -384,9 +386,9 @@ class BatchChangeValidationsSpec val changeIpV6 = AddChangeInput("fd69:27cc:fe91:0:0:0:0:ffff", RecordType.PTR, ttl, PTRData("test.")) - val resultA = validateInputName(changeA) - val resultIpV4 = validateInputName(changeIpV4) - val resultIpV6 = validateInputName(changeIpV6) + val resultA = validateInputName(changeA, false) + val resultIpV4 = validateInputName(changeIpV4, false) + val resultIpV6 = validateInputName(changeIpV6, false) resultA should haveInvalid[DomainValidationError]( HighValueDomainError("high-value-domain.foo.")) @@ -395,10 +397,33 @@ class BatchChangeValidationsSpec HighValueDomainError("fd69:27cc:fe91:0:0:0:0:ffff")) } + property("""validateInputName: should fail with a RecordRequiresManualReview + |if inputName is matches domain requiring manual review""".stripMargin) { + val changeA = AddChangeInput("needs-review.foo.", RecordType.A, ttl, AData("1.1.1.1")) + val changeIpV4 = AddChangeInput("192.0.2.254", RecordType.PTR, ttl, PTRData("test.")) + val changeIpV6 = + AddChangeInput("fd69:27cc:fe91:0:0:0:ffff:1", RecordType.PTR, ttl, PTRData("test.")) + + val resultA = validateInputName(changeA, false) + val resultIpV4 = validateInputName(changeIpV4, false) + val resultIpV6 = validateInputName(changeIpV6, false) + + resultA should haveInvalid[DomainValidationError]( + RecordRequiresManualReview("needs-review.foo.")) + resultIpV4 should haveInvalid[DomainValidationError](RecordRequiresManualReview("192.0.2.254")) + resultIpV6 should haveInvalid[DomainValidationError]( + RecordRequiresManualReview("fd69:27cc:fe91:0:0:0:ffff:1")) + } + + property("doesNotRequireManualReview: should succeed if user is reviewing") { + val changeA = AddChangeInput("needs-review.foo.", RecordType.A, ttl, AData("1.1.1.1")) + validateInputName(changeA, true) should beValid(()) + } + property("""validateInputName: should fail with a DomainValidationError for deletes |if validateHostName fails for an invalid domain name""".stripMargin) { val change = DeleteChangeInput("invalidDomainName$", RecordType.A) - val result = validateInputName(change) + val result = validateInputName(change, false) result should haveInvalid[DomainValidationError](InvalidDomainName("invalidDomainName$.")) } @@ -406,22 +431,22 @@ class BatchChangeValidationsSpec |if validateHostName fails for an invalid domain name length""".stripMargin) { val invalidDomainName = Random.alphanumeric.take(256).mkString val change = DeleteChangeInput(invalidDomainName, RecordType.AAAA) - val result = validateInputName(change) - result should (haveInvalid[DomainValidationError](InvalidDomainName(s"$invalidDomainName.")) - .and(haveInvalid[DomainValidationError](InvalidLength(s"$invalidDomainName.", 2, 255)))) + val result = validateInputName(change, false) + result should haveInvalid[DomainValidationError](InvalidDomainName(s"$invalidDomainName.")) + .and(haveInvalid[DomainValidationError](InvalidLength(s"$invalidDomainName.", 2, 255))) } property("""validateInputName: PTR should fail with InvalidIPAddress for deletes |if inputName is not a valid ipv4 or ipv6 address""".stripMargin) { val invalidIp = "invalidIp.111" val change = DeleteChangeInput(invalidIp, RecordType.PTR) - val result = validateInputName(change) + val result = validateInputName(change, false) result should haveInvalid[DomainValidationError](InvalidIPAddress(invalidIp)) } property("validateAddChangeInput: should succeed if single addChangeInput is good for A Record") { forAll(validAChangeGen) { input: AddChangeInput => - val result = validateAddChangeInput(input) + val result = validateAddChangeInput(input, false) result shouldBe valid } } @@ -429,7 +454,7 @@ class BatchChangeValidationsSpec property( "validateAddChangeInput: should succeed if single addChangeInput is good for AAAA Record") { forAll(validAAAAChangeGen) { input: AddChangeInput => - val result = validateAddChangeInput(input) + val result = validateAddChangeInput(input, false) result shouldBe valid } } @@ -437,7 +462,7 @@ class BatchChangeValidationsSpec property("""validateAddChangeInput: should fail with a DomainValidationError |if validateHostName fails for an invalid domain name""".stripMargin) { val change = AddChangeInput("invalidDomainName$", RecordType.A, ttl, AData("1.1.1.1")) - val result = validateAddChangeInput(change) + val result = validateAddChangeInput(change, false) result should haveInvalid[DomainValidationError](InvalidDomainName("invalidDomainName$.")) } @@ -445,7 +470,7 @@ class BatchChangeValidationsSpec |if validateHostName fails for an invalid domain name length""".stripMargin) { val invalidDomainName = Random.alphanumeric.take(256).mkString val change = AddChangeInput(invalidDomainName, RecordType.A, ttl, AData("1.1.1.1")) - val result = validateAddChangeInput(change) + val result = validateAddChangeInput(change, false) result should haveInvalid[DomainValidationError](InvalidDomainName(s"$invalidDomainName.")) .and(haveInvalid[DomainValidationError](InvalidLength(s"$invalidDomainName.", 2, 255))) } @@ -455,7 +480,7 @@ class BatchChangeValidationsSpec forAll(choose[Long](0, 29)) { invalidTTL: Long => val change = AddChangeInput("test.comcast.com.", RecordType.A, Some(invalidTTL), AData("1.1.1.1")) - val result = validateAddChangeInput(change) + val result = validateAddChangeInput(change, false) result should haveInvalid[DomainValidationError]( InvalidTTL(invalidTTL, DomainValidations.TTL_MIN_LENGTH, DomainValidations.TTL_MAX_LENGTH)) } @@ -465,7 +490,7 @@ class BatchChangeValidationsSpec |if validateRecordData fails for an invalid ipv4 address""".stripMargin) { val invalidIpv4 = "invalidIpv4:123" val change = AddChangeInput("test.comcast.com.", RecordType.A, ttl, AData(invalidIpv4)) - val result = validateAddChangeInput(change) + val result = validateAddChangeInput(change, false) result should haveInvalid[DomainValidationError](InvalidIpv4Address(invalidIpv4)) } @@ -473,14 +498,14 @@ class BatchChangeValidationsSpec |if validateRecordData fails for an invalid ipv6 address""".stripMargin) { val invalidIpv6 = "invalidIpv6:123" val change = AddChangeInput("test.comcast.com.", RecordType.AAAA, ttl, AAAAData(invalidIpv6)) - val result = validateAddChangeInput(change) + val result = validateAddChangeInput(change, false) result should haveInvalid[DomainValidationError](InvalidIpv6Address(invalidIpv6)) } property("validateAddChangeInput: should fail if A inputName includes a reverse zone address") { val invalidInputName = "test.1.2.3.in-addr.arpa." val badAChange = AddChangeInput(invalidInputName, RecordType.A, ttl, AData("1.1.1.1")) - val result = validateAddChangeInput(badAChange) + val result = validateAddChangeInput(badAChange, false) result should haveInvalid[DomainValidationError]( RecordInReverseZoneError(invalidInputName, RecordType.A.toString)) } @@ -489,7 +514,7 @@ class BatchChangeValidationsSpec val invalidInputName = "test.1.2.3.ip6.arpa." val badAAAAChange = AddChangeInput(invalidInputName, RecordType.AAAA, ttl, AAAAData("1:2:3:4:5:6:7:8")) - val result = validateAddChangeInput(badAAAAChange) + val result = validateAddChangeInput(badAAAAChange, false) result should haveInvalid[DomainValidationError]( RecordInReverseZoneError(invalidInputName, RecordType.AAAA.toString)) } @@ -499,7 +524,7 @@ class BatchChangeValidationsSpec val invalidCNAMERecordData = "$$$" val change = AddChangeInput("test.comcast.com.", RecordType.CNAME, ttl, CNAMEData(invalidCNAMERecordData)) - val result = validateAddChangeInput(change) + val result = validateAddChangeInput(change, false) result should haveInvalid[DomainValidationError](InvalidDomainName(s"$invalidCNAMERecordData.")) } @@ -509,7 +534,7 @@ class BatchChangeValidationsSpec val invalidCNAMERecordData = "s" * 256 val change = AddChangeInput("test.comcast.com.", RecordType.CNAME, ttl, CNAMEData(invalidCNAMERecordData)) - val result = validateAddChangeInput(change) + val result = validateAddChangeInput(change, false) result should haveInvalid[DomainValidationError]( InvalidLength(s"$invalidCNAMERecordData.", 2, 255)) @@ -519,7 +544,7 @@ class BatchChangeValidationsSpec |if inputName is not a valid ipv4 or ipv6 address""".stripMargin) { val invalidIp = "invalidip.111." val change = AddChangeInput(invalidIp, RecordType.PTR, ttl, PTRData("test.comcast.com")) - val result = validateAddChangeInput(change) + val result = validateAddChangeInput(change, false) result should haveInvalid[DomainValidationError](InvalidIPAddress(invalidIp)) } @@ -527,7 +552,7 @@ class BatchChangeValidationsSpec property("validateAddChangeInput: should fail with InvalidDomainName for invalid PTR record data") { val invalidPTRDname = "*invalidptrdname" val change = AddChangeInput("4.5.6.7", RecordType.PTR, ttl, PTRData(invalidPTRDname)) - val result = validateAddChangeInput(change) + val result = validateAddChangeInput(change, false) result should haveInvalid[DomainValidationError](InvalidDomainName(s"$invalidPTRDname.")) } @@ -1420,13 +1445,13 @@ class BatchChangeValidationsSpec property("validateAddChangeInput: should succeed for a valid TXT addChangeInput") { val input = AddChangeInput("txt.ok.", RecordType.TXT, ttl, TXTData("test")) - val result = validateAddChangeInput(input) + val result = validateAddChangeInput(input, false) result shouldBe valid } property("validateAddChangeInput: should fail for a TXT addChangeInput with empty TXTData") { val input = AddChangeInput("txt.ok.", RecordType.TXT, ttl, TXTData("")) - val result = validateAddChangeInput(input) + val result = validateAddChangeInput(input, false) result should haveInvalid[DomainValidationError](InvalidLength("", 1, 64764)) } @@ -1434,21 +1459,21 @@ class BatchChangeValidationsSpec "validateAddChangeInput: should fail for a TXT addChangeInput with TXTData that is too many characters") { val txtData = "x" * 64765 val input = AddChangeInput("txt.ok.", RecordType.TXT, ttl, TXTData(txtData)) - val result = validateAddChangeInput(input) + val result = validateAddChangeInput(input, false) result should haveInvalid[DomainValidationError](InvalidLength(txtData, 1, 64764)) } property("validateAddChangeInput: should succeed for a valid MX addChangeInput") { val input = AddChangeInput("mx.ok.", RecordType.MX, ttl, MXData(1, "foo.bar.")) - val result = validateAddChangeInput(input) + val result = validateAddChangeInput(input, false) result shouldBe valid } property("validateAddChangeInput: should fail for a MX addChangeInput with invalid preference") { val inputSmall = AddChangeInput("mx.ok.", RecordType.MX, ttl, MXData(-1, "foo.bar.")) val inputLarge = AddChangeInput("mx.ok.", RecordType.MX, ttl, MXData(1000000, "foo.bar.")) - val resultSmall = validateAddChangeInput(inputSmall) - val resultLarge = validateAddChangeInput(inputLarge) + val resultSmall = validateAddChangeInput(inputSmall, false) + val resultLarge = validateAddChangeInput(inputLarge, false) resultSmall should haveInvalid[DomainValidationError]( InvalidMxPreference( @@ -1464,14 +1489,14 @@ class BatchChangeValidationsSpec property("validateAddChangeInput: should fail for a MX addChangeInput with invalid exchange") { val input = AddChangeInput("mx.ok.", RecordType.MX, ttl, MXData(1, "foo$.bar.")) - val result = validateAddChangeInput(input) + val result = validateAddChangeInput(input, false) result should haveInvalid[DomainValidationError](InvalidDomainName("foo$.bar.")) } property( "validateAddChangeInput: should fail for a MX addChangeInput with invalid preference and exchange") { val input = AddChangeInput("mx.ok.", RecordType.MX, ttl, MXData(-1, "foo$.bar.")) - val result = validateAddChangeInput(input) + val result = validateAddChangeInput(input, false) result should haveInvalid[DomainValidationError]( InvalidMxPreference( -1, 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 f00da25f2..11a7bcb25 100644 --- a/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala +++ b/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala @@ -172,4 +172,13 @@ final case class NewMultiRecordError(changeName: String, changeType: RecordType) final case class CnameAtZoneApexError(zoneName: String) extends DomainValidationError { def message: String = s"""CNAME cannot be the same name as zone "$zoneName".""" } + +final case class RecordRequiresManualReview(fqdn: String, fatal: Boolean = false) + extends DomainValidationError(fatal) { + def message: String = + s"""Record set with name "$fqdn" is configured to require manual review, but manual review is + |not enabled.""".stripMargin + .replaceAll("\n", " ") +} + // $COVERAGE-ON$ 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 7c5870433..cfb0d5b08 100644 --- a/modules/core/src/main/scala/vinyldns/core/domain/SingleChangeError.scala +++ b/modules/core/src/main/scala/vinyldns/core/domain/SingleChangeError.scala @@ -34,7 +34,7 @@ object DomainValidationErrorType extends Enumeration { InvalidBatchRecordType, ZoneDiscoveryError, RecordAlreadyExists, RecordDoesNotExist, CnameIsNotUniqueError, UserIsNotAuthorized, RecordNameNotUniqueInBatch, RecordInReverseZoneError, HighValueDomainError, MissingOwnerGroupId, ExistingMultiRecordError, NewMultiRecordError, - CnameAtZoneApexError = Value + CnameAtZoneApexError, RecordRequiresManualReview = Value // $COVERAGE-OFF$ def from(error: DomainValidationError): DomainValidationErrorType = @@ -66,6 +66,7 @@ object DomainValidationErrorType extends Enumeration { case _: ExistingMultiRecordError => ExistingMultiRecordError case _: NewMultiRecordError => NewMultiRecordError case _: CnameAtZoneApexError => CnameAtZoneApexError + case _: RecordRequiresManualReview => RecordRequiresManualReview } // $COVERAGE-ON$ }