Address PR comments

modules/api/src/it/resources/application.conf

@@ -172,6 +172,7 @@ vinyldns {
       allowed-user-list = ["testuser"]
       allowed-group-list = ["dummy-group"]
       allowed-record-type = ["AAAA"]
+      allowed-dots-limit = 3
       },
       {
       # for wildcard zones. Settings will be applied to all matching zones
@@ -179,6 +180,7 @@ vinyldns {
       allowed-user-list = ["professor", "testuser"]
       allowed-group-list = ["testing-group"]
       allowed-record-type = ["A", "CNAME"]
+      allowed-dots-limit = 3
       }
     ]
   }

modules/api/src/main/resources/reference.conf

@@ -100,6 +100,7 @@ vinyldns {
       allowed-user-list = ["ok"]
       allowed-group-list = ["dummy-group"]
       allowed-record-type = ["CNAME"]
+      allowed-dots-limit = 3
       },
       {
       # for wildcard zones. Settings will be applied to all matching zones
@@ -107,6 +108,7 @@ vinyldns {
       allowed-user-list = ["sharedZoneUser"]
       allowed-group-list = ["history-group1"]
       allowed-record-type = ["A"]
+      allowed-dots-limit = 3
       }
     ]
   }

modules/api/src/main/scala/vinyldns/api/config/DottedHostsConfig.scala

@@ -19,7 +19,7 @@ package vinyldns.api.config
 import pureconfig.ConfigReader
 import pureconfig.generic.auto._
 
-final case class AuthConfigs(zone: String, allowedUserList: List[String], allowedGroupList: List[String], allowedRecordType: List[String])
+final case class AuthConfigs(zone: String, allowedUserList: List[String], allowedGroupList: List[String], allowedRecordType: List[String], allowedDotsLimit: Int)
 final case class DottedHostsConfig(authConfigs: List[AuthConfigs])
 
 object DottedHostsConfig {

modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetService.scala

@@ -118,6 +118,7 @@ class RecordSetService(
       isAllowedUser = isInAllowedUsers || isUserInAllowedGroups
       isRecordTypeAllowed = checkIfInAllowedRecordType(zone, dottedHostsConfig, rsForValidations)
       isRecordTypeAndUserAllowed = isAllowedUser && isRecordTypeAllowed
+      allowedDotsLimit = getAllowedDotsLimit(zone, dottedHostsConfig)
       recordFqdnDoesNotAlreadyExist <- recordFQDNDoesNotExist(rsForValidations, zone).toResult[Boolean]
       _ <- typeSpecificValidations(
         rsForValidations,
@@ -129,6 +130,7 @@ class RecordSetService(
         allowedZoneList,
         isRecordTypeAndUserAllowed
       ).toResult
+      _ <- checkAllowedDots(allowedDotsLimit, rsForValidations, zone).toResult
       _ <- messageQueue.send(change).toResult[Unit]
     } yield change
 
@@ -237,6 +239,24 @@ class RecordSetService(
     }
   }
 
+  // Check if user is allowed to create dotted hosts using the users present in dotted hosts config
+  def getAllowedDotsLimit(zone: Zone, config: DottedHostsConfig): Int = {
+    val configZones = config.authConfigs.map(x => x.zone)
+    val zoneName = if(zone.name.takeRight(1) != ".") zone.name + "." else zone.name
+    val dottedZoneConfig = configZones.filter(_.contains("*")).map(_.replace("*", "[A-Za-z0-9.]*"))
+    val isContainWildcardZone = dottedZoneConfig.exists(x => zoneName.matches(x))
+    val isContainNormalZone = configZones.contains(zoneName)
+    if(isContainNormalZone){
+      config.authConfigs.filter(x => x.zone == zoneName).head.allowedDotsLimit
+    }
+    else if(isContainWildcardZone){
+      config.authConfigs.filter(x => zoneName.matches(x.zone.replace("*", "[A-Za-z0-9.]*"))).head.allowedDotsLimit
+    }
+    else {
+      0
+    }
+  }
+
   // Check if user is allowed to create dotted hosts using the users present in dotted hosts config
   def checkIfInAllowedUsers(zone: Zone, config: DottedHostsConfig, auth: AuthPrincipal): Boolean = {
     val configZones = config.authConfigs.map(x => x.zone)

modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetValidations.scala

@@ -352,6 +352,19 @@ object RecordSetValidations {
       .leftMap(errors => InvalidRequest(errors.toList.map(_.message).mkString(", ")))
   }
 
+  def checkAllowedDots(allowedDotsLimit: Int, recordSet: RecordSet, zone: Zone): Either[Throwable, Unit] = {
+    ensuring(
+      InvalidRequest(
+        s"RecordSet with name ${recordSet.name} has more dots than that is allowed in config for this zone " +
+          s"which is, 'allowed-dots-limit = $allowedDotsLimit'."
+      )
+    )(
+      recordSet.name.count(_ == '.') <= allowedDotsLimit || (recordSet.name.count(_ == '.') == 1 &&
+        recordSet.name.takeRight(1) == ".") || recordSet.name == zone.name ||
+        (recordSet.typ.toString == "PTR" || recordSet.typ.toString == "SRV" || recordSet.typ.toString == "TXT" || recordSet.typ.toString == "NAPTR")
+    )
+  }
+
   def canUseOwnerGroup(
       ownerGroupId: Option[String],
       group: Option[Group],

modules/api/src/test/functional/tests/recordsets/create_recordset_test.py

@@ -557,6 +557,28 @@ def test_create_dotted_a_record_succeeds_if_all_dotted_hosts_config_satisfied(sh
             client.wait_until_recordset_change_status(delete_result, "Complete")
 
 
+def test_create_dotted_a_record_fails_if_all_dotted_hosts_config_not_satisfied(shared_zone_test_context):
+    """
+    Test that creating a A record set with dotted host record name fails
+    Here the zone, user (in group) and record type is allowed.
+    But the record name has more dots than the number of dots allowed for this zone. Hence the test fails
+    The 'allowed-dots-limit' config from dotted-hosts config is not satisfied. Config present in reference.conf
+    """
+    client = shared_zone_test_context.history_client
+    zone = shared_zone_test_context.dummy_zone
+    dotted_host_a_record = {
+        "zoneId": zone["id"],
+        "name": "dot.ted.trial.test.host",
+        "type": "A",
+        "ttl": 500,
+        "records": [{"address": "127.0.0.1"}]
+    }
+
+    error = client.create_recordset(dotted_host_a_record, status=422)
+    assert_that(error, is_("RecordSet with name " + dotted_host_a_record["name"] + " has more dots than that is "
+                           "allowed in config for this zone which is, 'allowed-dots-limit = 3'."))
+
+
 def test_create_dotted_a_record_apex_succeeds(shared_zone_test_context):
     """
     Test that creating an apex A record set containing dots succeeds.
@@ -654,7 +676,8 @@ def test_create_dotted_cname_record_succeeds_if_all_dotted_hosts_config_satisfie
         assert_that(dotted_cname_record["name"], is_(dotted_host_cname_record["name"]))
     finally:
         if dotted_cname_record:
-            delete_result = client.delete_recordset(dotted_cname_record["zoneId"], dotted_cname_record["id"], status=202)
+            delete_result = client.delete_recordset(dotted_cname_record["zoneId"], dotted_cname_record["id"],
+                                                    status=202)
             client.wait_until_recordset_change_status(delete_result, "Complete")
 
 
@@ -760,7 +783,8 @@ def test_create_cname_with_existing_record_with_name_fails(shared_zone_test_cont
         a_record = client.wait_until_recordset_change_status(a_create, "Complete")["recordSet"]
 
         error = client.create_recordset(cname_rs, status=409)
-        assert_that(error, is_(f'RecordSet with name duplicate-test-name already exists in zone {zone["name"]}, CNAME record cannot use duplicate name'))
+        assert_that(error,
+                    is_(f'RecordSet with name duplicate-test-name already exists in zone {zone["name"]}, CNAME record cannot use duplicate name'))
     finally:
         if a_record:
             delete_result = client.delete_recordset(a_record["zoneId"], a_record["id"], status=202)
@@ -803,7 +827,8 @@ def test_create_record_with_existing_cname_fails(shared_zone_test_context):
         cname_record = client.wait_until_recordset_change_status(cname_create, "Complete")["recordSet"]
 
         error = client.create_recordset(a_rs, status=409)
-        assert_that(error, is_(f'RecordSet with name duplicate-test-name and type CNAME already exists in zone {zone["name"]}'))
+        assert_that(error,
+                    is_(f'RecordSet with name duplicate-test-name and type CNAME already exists in zone {zone["name"]}'))
     finally:
         if cname_record:
             delete_result = client.delete_recordset(cname_record["zoneId"], cname_record["id"], status=202)
@@ -1800,7 +1825,8 @@ def test_create_high_value_domain_fails(shared_zone_test_context):
     }
 
     error = client.create_recordset(new_rs, status=422)
-    assert_that(error, is_(f'Record name "high-value-domain.{zone["name"]}" is configured as a High Value Domain, so it cannot be modified.'))
+    assert_that(error,
+                is_(f'Record name "high-value-domain.{zone["name"]}" is configured as a High Value Domain, so it cannot be modified.'))
 
 
 def test_create_high_value_domain_fails_case_insensitive(shared_zone_test_context):
@@ -1822,7 +1848,8 @@ def test_create_high_value_domain_fails_case_insensitive(shared_zone_test_contex
     }
 
     error = client.create_recordset(new_rs, status=422)
-    assert_that(error, is_(f'Record name "hIgH-vAlUe-dOmAiN.{zone["name"]}" is configured as a High Value Domain, so it cannot be modified.'))
+    assert_that(error,
+                is_(f'Record name "hIgH-vAlUe-dOmAiN.{zone["name"]}" is configured as a High Value Domain, so it cannot be modified.'))
 
 
 def test_create_high_value_domain_fails_for_ip4_ptr(shared_zone_test_context):
@@ -1843,7 +1870,8 @@ def test_create_high_value_domain_fails_for_ip4_ptr(shared_zone_test_context):
     }
 
     error_ptr = client.create_recordset(ptr, status=422)
-    assert_that(error_ptr, is_(f'Record name "{shared_zone_test_context.ip4_classless_prefix}.252" is configured as a High Value Domain, so it cannot be modified.'))
+    assert_that(error_ptr,
+                is_(f'Record name "{shared_zone_test_context.ip4_classless_prefix}.252" is configured as a High Value Domain, so it cannot be modified.'))
 
 
 def test_create_high_value_domain_fails_for_ip6_ptr(shared_zone_test_context):
@@ -1864,7 +1892,8 @@ def test_create_high_value_domain_fails_for_ip6_ptr(shared_zone_test_context):
     }
 
     error_ptr = client.create_recordset(ptr, status=422)
-    assert_that(error_ptr, is_(f'Record name "{shared_zone_test_context.ip6_prefix}:0000:0000:0000:0000:ffff" is configured as a High Value Domain, so it cannot be modified.'))
+    assert_that(error_ptr,
+                is_(f'Record name "{shared_zone_test_context.ip6_prefix}:0000:0000:0000:0000:ffff" is configured as a High Value Domain, so it cannot be modified.'))
 
 
 def test_create_with_owner_group_in_private_zone_by_admin_passes(shared_zone_test_context):
@@ -1931,7 +1960,8 @@ def test_create_with_owner_group_in_private_zone_by_acl_passes(shared_zone_test_
     finally:
         clear_ok_acl_rules(shared_zone_test_context)
         if create_rs:
-            delete_result = shared_zone_test_context.ok_vinyldns_client.delete_recordset(zone["id"], create_rs["id"], status=202)
+            delete_result = shared_zone_test_context.ok_vinyldns_client.delete_recordset(zone["id"], create_rs["id"],
+                                                                                         status=202)
             shared_zone_test_context.ok_vinyldns_client.wait_until_recordset_change_status(delete_result, "Complete")
 
 
@@ -1957,8 +1987,11 @@ def test_create_with_owner_group_in_shared_zone_by_acl_passes(shared_zone_test_c
     finally:
         clear_shared_zone_acl_rules(shared_zone_test_context)
         if create_rs:
-            delete_result = shared_zone_test_context.shared_zone_vinyldns_client.delete_recordset(zone["id"], create_rs["id"], status=202)
+            delete_result = shared_zone_test_context.shared_zone_vinyldns_client.delete_recordset(zone["id"],
-            shared_zone_test_context.shared_zone_vinyldns_client.wait_until_recordset_change_status(delete_result, "Complete")
+                                                                                                  create_rs["id"],
+                                                                                                  status=202)
+            shared_zone_test_context.shared_zone_vinyldns_client.wait_until_recordset_change_status(delete_result,
+                                                                                                    "Complete")
 
 
 def test_create_in_shared_zone_without_owner_group_id_succeeds(shared_zone_test_context):
@@ -2012,10 +2045,12 @@ def test_create_in_shared_zone_by_unassociated_user_fails_if_record_type_is_not_
     zone = shared_zone_test_context.shared_zone
     group = shared_zone_test_context.dummy_group
 
-    record_json = create_recordset(zone, "test_shared_not_approved_record_type", "MX", [{"preference": 3, "exchange": "mx"}])
+    record_json = create_recordset(zone, "test_shared_not_approved_record_type", "MX",
+                                   [{"preference": 3, "exchange": "mx"}])
     record_json["ownerGroupId"] = group["id"]
     error = client.create_recordset(record_json, status=403)
-    assert_that(error, is_(f'User dummy does not have access to create test-shared-not-approved-record-type.{zone["name"]}'))
+    assert_that(error,
+                is_(f'User dummy does not have access to create test-shared-not-approved-record-type.{zone["name"]}'))
 
 
 def test_create_with_not_found_owner_group_fails(shared_zone_test_context):
@@ -2054,7 +2089,8 @@ def test_create_ds_success(shared_zone_test_context):
     zone = shared_zone_test_context.ds_zone
     record_data = [
         {"keytag": 60485, "algorithm": 5, "digesttype": 1, "digest": "2BB183AF5F22588179A53B0A98631FAD1A292118"},
-        {"keytag": 60485, "algorithm": 5, "digesttype": 2, "digest": "D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A"}
+        {"keytag": 60485, "algorithm": 5, "digesttype": 2,
+         "digest": "D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A"}
     ]
     record_json = create_recordset(zone, "dskey", "DS", record_data, ttl=3600)
     result_rs = None
@@ -2096,7 +2132,8 @@ def test_create_ds_unknown_algorithm(shared_zone_test_context):
     """
     client = shared_zone_test_context.ok_vinyldns_client
     zone = shared_zone_test_context.ds_zone
-    record_data = [{"keytag": 60485, "algorithm": 0, "digesttype": 1, "digest": "2BB183AF5F22588179A53B0A98631FAD1A292118"}]
+    record_data = [
+        {"keytag": 60485, "algorithm": 0, "digesttype": 1, "digest": "2BB183AF5F22588179A53B0A98631FAD1A292118"}]
     record_json = create_recordset(zone, "dskey", "DS", record_data)
     errors = client.create_recordset(record_json, status=400)["errors"]
     assert_that(errors, contains_inanyorder("Algorithm 0 is not a supported DNSSEC algorithm"))
@@ -2108,7 +2145,8 @@ def test_create_ds_unknown_digest_type(shared_zone_test_context):
     """
     client = shared_zone_test_context.ok_vinyldns_client
     zone = shared_zone_test_context.ds_zone
-    record_data = [{"keytag": 60485, "algorithm": 5, "digesttype": 0, "digest": "2BB183AF5F22588179A53B0A98631FAD1A292118"}]
+    record_data = [
+        {"keytag": 60485, "algorithm": 5, "digesttype": 0, "digest": "2BB183AF5F22588179A53B0A98631FAD1A292118"}]
     record_json = create_recordset(zone, "dskey", "DS", record_data)
     errors = client.create_recordset(record_json, status=400)["errors"]
     assert_that(errors, contains_inanyorder("Digest Type 0 is not a supported DS record digest type"))
@@ -2120,10 +2158,12 @@ def test_create_ds_no_ns_fails(shared_zone_test_context):
     """
     client = shared_zone_test_context.ok_vinyldns_client
     zone = shared_zone_test_context.ds_zone
-    record_data = [{"keytag": 60485, "algorithm": 5, "digesttype": 1, "digest": "2BB183AF5F22588179A53B0A98631FAD1A292118"}]
+    record_data = [
+        {"keytag": 60485, "algorithm": 5, "digesttype": 1, "digest": "2BB183AF5F22588179A53B0A98631FAD1A292118"}]
     record_json = create_recordset(zone, "no-ns-exists", "DS", record_data, ttl=3600)
     error = client.create_recordset(record_json, status=422)
-    assert_that(error, is_(f'DS record [no-ns-exists] is invalid because there is no NS record with that name in the zone [{zone["name"]}]'))
+    assert_that(error,
+                is_(f'DS record [no-ns-exists] is invalid because there is no NS record with that name in the zone [{zone["name"]}]'))
 
 
 def test_create_apex_ds_fails(shared_zone_test_context):
@@ -2145,7 +2185,9 @@ def test_create_dotted_ds_fails(shared_zone_test_context):
     """
     client = shared_zone_test_context.ok_vinyldns_client
     zone = shared_zone_test_context.ds_zone
-    record_data = [{"keytag": 60485, "algorithm": 5, "digesttype": 1, "digest": "2BB183AF5F22588179A53B0A98631FAD1A292118"}]
+    record_data = [
+        {"keytag": 60485, "algorithm": 5, "digesttype": 1, "digest": "2BB183AF5F22588179A53B0A98631FAD1A292118"}]
     record_json = create_recordset(zone, "dotted.ds", "DS", record_data, ttl=100)
     error = client.create_recordset(record_json, status=422)
-    assert_that(error, is_(f'Record with name dotted.ds and type DS is a dotted host which is not allowed in zone {zone["name"]}'))
+    assert_that(error,
+                is_(f'Record with name dotted.ds and type DS is a dotted host which is not allowed in zone {zone["name"]}'))

modules/api/src/test/scala/vinyldns/api/VinylDNSTestHelpers.scala

@@ -40,7 +40,7 @@ trait VinylDNSTestHelpers {
 
   val approvedNameServers: List[Regex] = List(new Regex("some.test.ns."))
 
-  val dottedHostsConfig: DottedHostsConfig = DottedHostsConfig(List(AuthConfigs("dotted.xyz.",List("xyz"),List("dummy"),List("CNAME")), AuthConfigs("abc.zone.recordsets.",List("locked"),List("dummy"),List("CNAME")), AuthConfigs("xyz.",List("super"),List("xyz"),List("CNAME"))))
+  val dottedHostsConfig: DottedHostsConfig = DottedHostsConfig(List(AuthConfigs("dotted.xyz.",List("xyz"),List("dummy"),List("CNAME"), 3), AuthConfigs("abc.zone.recordsets.",List("locked"),List("dummy"),List("CNAME"), 3), AuthConfigs("xyz.",List("super"),List("xyz"),List("CNAME"), 3)))
 
   val emptyDottedHostsConfig: DottedHostsConfig = DottedHostsConfig(List.empty)
 

modules/docs/src/main/mdoc/operator/config-api.md

@@ -551,6 +551,7 @@ Note the following:
 5. If the user is either in `allowed-user-list` or `allowed-group-list`, they are allowed to create a dotted host. It is
 not necessary for the user to be in both `allowed-user-list` and `allowed-group-list`.
 6. The record types which are allowed while creating a dotted host is added to the `allowed-record-type`.
+7. The number of dots allowed in a record name for a zone is given in `allowed-dots-limit`.
 
 ```yaml
 # approved zones, individual users, users in groups and record types that are allowed for dotted hosts
@@ -561,6 +562,7 @@ dotted-hosts = {
       allowed-user-list = ["testuser"]
       allowed-group-list = ["dummy-group"]
       allowed-record-type = ["AAAA"]
+      allowed-dots-limit = 3
       },
       {
       # for wildcard zones. Settings will be applied to all matching zones
@@ -568,6 +570,7 @@ dotted-hosts = {
       allowed-user-list = ["professor", "testuser"]
       allowed-group-list = ["testing-group"]
       allowed-record-type = ["A", "CNAME"]
+      allowed-dots-limit = 3
       }
    ]
 }
@@ -771,6 +774,7 @@ dotted-hosts = {
         allowed-user-list = ["testuser"]
         allowed-group-list = ["dummy-group"]
         allowed-record-type = ["AAAA"]
+        allowed-dots-limit = 3
         },
         {
         # for wildcard zones. Settings will be applied to all matching zones
@@ -778,6 +782,7 @@ dotted-hosts = {
         allowed-user-list = ["professor", "testuser"]
         allowed-group-list = ["testing-group"]
         allowed-record-type = ["A", "CNAME"]
+        allowed-dots-limit = 3
         }
      ]
   }