From adb933a783086579a3e9152f59268e712897f028 Mon Sep 17 00:00:00 2001 From: Aravindh-Raju Date: Wed, 12 Oct 2022 11:37:35 +0530 Subject: [PATCH] Address PR comments --- .../api/config/DottedHostsConfig.scala | 10 +- .../api/domain/record/RecordSetService.scala | 46 ++++----- .../domain/record/RecordSetValidations.scala | 37 ++++---- .../vinyldns/api/VinylDNSTestHelpers.scala | 4 +- .../domain/record/RecordSetServiceSpec.scala | 93 +++++++++++++------ .../record/RecordSetValidationsSpec.scala | 32 +++++-- .../scala/vinyldns/core/TestZoneData.scala | 1 + 7 files changed, 142 insertions(+), 81 deletions(-) diff --git a/modules/api/src/main/scala/vinyldns/api/config/DottedHostsConfig.scala b/modules/api/src/main/scala/vinyldns/api/config/DottedHostsConfig.scala index e28f35f3b..2c0721ac7 100644 --- a/modules/api/src/main/scala/vinyldns/api/config/DottedHostsConfig.scala +++ b/modules/api/src/main/scala/vinyldns/api/config/DottedHostsConfig.scala @@ -19,13 +19,13 @@ 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], allowedDotsLimit: Int) -final case class DottedHostsConfig(authConfigs: List[AuthConfigs]) +final case class ZoneAuthConfigs(zone: String, allowedUserList: List[String], allowedGroupList: List[String], allowedRecordType: List[String], allowedDotsLimit: Int) +final case class DottedHostsConfig(zoneAuthConfigs: List[ZoneAuthConfigs]) object DottedHostsConfig { implicit val configReader: ConfigReader[DottedHostsConfig] = - ConfigReader.forProduct1[DottedHostsConfig, List[AuthConfigs]]( + ConfigReader.forProduct1[DottedHostsConfig, List[ZoneAuthConfigs]]( "allowed-settings", - )(authConfigs => - DottedHostsConfig(authConfigs)) + )(zoneAuthConfigs => + DottedHostsConfig(zoneAuthConfigs)) } diff --git a/modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetService.scala b/modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetService.scala index 6913fe7a1..cd699261d 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetService.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetService.scala @@ -28,7 +28,7 @@ import vinyldns.core.queue.MessageQueue import cats.data._ import cats.effect.IO import org.xbill.DNS.ReverseMap -import vinyldns.api.config.{AuthConfigs, DottedHostsConfig, HighValueDomainConfig} +import vinyldns.api.config.{ZoneAuthConfigs, DottedHostsConfig, HighValueDomainConfig} import vinyldns.api.domain.DomainValidations.{validateIpv4Address, validateIpv6Address} import vinyldns.api.domain.access.AccessValidationsAlgebra import vinyldns.core.domain.record.NameSort.NameSort @@ -91,7 +91,7 @@ class RecordSetService( def addRecordSet(recordSet: RecordSet, auth: AuthPrincipal): Result[ZoneCommandResult] = for { zone <- getZone(recordSet.zoneId) - authZones = dottedHostsConfig.authConfigs.map(x => x.zone) + authZones = dottedHostsConfig.zoneAuthConfigs.map(x => x.zone) change <- RecordSetChangeGenerator.forAdd(recordSet, zone, Some(auth)).toResult // because changes happen to the RS in forAdd itself, converting 1st and validating on that rsForValidations = change.recordSet @@ -127,7 +127,8 @@ class RecordSetService( approvedNameServers, recordFqdnDoesNotAlreadyExist, allowedZoneList, - isRecordTypeAndUserAllowed + isRecordTypeAndUserAllowed, + allowedDotsLimit ).toResult _ <- if(allowedZoneList.contains(zone.name)) checkAllowedDots(allowedDotsLimit, rsForValidations, zone).toResult else ().toResult _ <- if(allowedZoneList.contains(zone.name)) isNotApexEndsWithDot(rsForValidations, zone).toResult else ().toResult @@ -160,7 +161,7 @@ class RecordSetService( validateRecordLookupAgainstDnsBackend ) _ <- noCnameWithNewName(rsForValidations, existingRecordsWithName, zone).toResult - authZones = dottedHostsConfig.authConfigs.map(x => x.zone) + authZones = dottedHostsConfig.zoneAuthConfigs.map(x => x.zone) allowedZoneList <- getAllowedZones(authZones).toResult[Set[String]] isInAllowedUsers = checkIfInAllowedUsers(zone, dottedHostsConfig, auth) isUserInAllowedGroups <- checkIfInAllowedGroups(zone, dottedHostsConfig, auth).toResult[Boolean] @@ -178,6 +179,7 @@ class RecordSetService( recordFqdnDoesNotAlreadyExist, allowedZoneList, isRecordTypeAndUserAllowed, + allowedDotsLimit ).toResult _ <- if(existing.name == rsForValidations.name) ().toResult else if(allowedZoneList.contains(zone.name)) checkAllowedDots(allowedDotsLimit, rsForValidations, zone).toResult else ().toResult _ <- if(allowedZoneList.contains(zone.name)) isNotApexEndsWithDot(rsForValidations, zone).toResult else ().toResult @@ -244,16 +246,16 @@ 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 configZones = config.zoneAuthConfigs.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 + config.zoneAuthConfigs.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 + config.zoneAuthConfigs.filter(x => zoneName.matches(x.zone.replace("*", "[A-Za-z0-9.]*"))).head.allowedDotsLimit } else { 0 @@ -262,14 +264,14 @@ class RecordSetService( // 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) + val configZones = config.zoneAuthConfigs.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){ - val users = config.authConfigs.flatMap { - x: AuthConfigs => + val users = config.zoneAuthConfigs.flatMap { + x: ZoneAuthConfigs => if (x.zone == zoneName) x.allowedUserList else List.empty } if(users.contains(auth.signedInUser.userName)){ @@ -280,8 +282,8 @@ class RecordSetService( } } else if(isContainWildcardZone){ - val users = config.authConfigs.flatMap { - x: AuthConfigs => + val users = config.zoneAuthConfigs.flatMap { + x: ZoneAuthConfigs => if (x.zone.contains("*")) { val wildcardZone = x.zone.replace("*", "[A-Za-z0-9.]*") if (zoneName.matches(wildcardZone)) x.allowedUserList else List.empty @@ -301,14 +303,14 @@ class RecordSetService( // Check if user is allowed to create dotted hosts using the record types present in dotted hosts config def checkIfInAllowedRecordType(zone: Zone, config: DottedHostsConfig, rs: RecordSet): Boolean = { - val configZones = config.authConfigs.map(x => x.zone) + val configZones = config.zoneAuthConfigs.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){ - val rType = config.authConfigs.flatMap { - x: AuthConfigs => + val rType = config.zoneAuthConfigs.flatMap { + x: ZoneAuthConfigs => if (x.zone == zoneName) x.allowedRecordType else List.empty } if(rType.contains(rs.typ.toString)){ @@ -319,8 +321,8 @@ class RecordSetService( } } else if(isContainWildcardZone){ - val rType = config.authConfigs.flatMap { - x: AuthConfigs => + val rType = config.zoneAuthConfigs.flatMap { + x: ZoneAuthConfigs => if (x.zone.contains("*")) { val wildcardZone = x.zone.replace("*", "[A-Za-z0-9.]*") if (zoneName.matches(wildcardZone)) x.allowedRecordType else List.empty @@ -340,20 +342,20 @@ class RecordSetService( // Check if user is allowed to create dotted hosts using the groups present in dotted hosts config def checkIfInAllowedGroups(zone: Zone, config: DottedHostsConfig, auth: AuthPrincipal): IO[Boolean] = { - val configZones = config.authConfigs.map(x => x.zone) + val configZones = config.zoneAuthConfigs.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) val groups = if(isContainNormalZone){ - config.authConfigs.flatMap { - x: AuthConfigs => + config.zoneAuthConfigs.flatMap { + x: ZoneAuthConfigs => if (x.zone == zoneName) x.allowedGroupList else List.empty } } else if(isContainWildcardZone){ - config.authConfigs.flatMap { - x: AuthConfigs => + config.zoneAuthConfigs.flatMap { + x: ZoneAuthConfigs => if (x.zone.contains("*")) { val wildcardZone = x.zone.replace("*", "[A-Za-z0-9.]*") if (zoneName.matches(wildcardZone)) x.allowedGroupList else List.empty diff --git a/modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetValidations.scala b/modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetValidations.scala index 7fd3ca050..4a625e0cc 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetValidations.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetValidations.scala @@ -97,7 +97,8 @@ object RecordSetValidations { existingRecordSet: Option[RecordSet] = None, recordFqdnDoesNotExist: Boolean, dottedHostZoneConfig: Set[String], - isRecordTypeAndUserAllowed: Boolean + isRecordTypeAndUserAllowed: Boolean, + allowedDotsLimit: Int = 0 ): Either[Throwable, Unit] = { val zoneName = if(zone.name.takeRight(1) != ".") zone.name + "." else zone.name @@ -105,7 +106,7 @@ object RecordSetValidations { val isDomainAllowed = dottedHostZoneConfig.contains(zoneName) // Check if record set contains dot and if it is in zone which is allowed to have dotted records from dotted hosts config - if(newRecordSet.name.contains(".") && isDomainAllowed && newRecordSet.name != zone.name) { + if(allowedDotsLimit != 0 && newRecordSet.name.contains(".") && isDomainAllowed && newRecordSet.name != zone.name) { if(!isRecordTypeAndUserAllowed){ isUserAndRecordTypeAuthorized(newRecordSet, zone, existingRecordSet, recordFqdnDoesNotExist, isRecordTypeAndUserAllowed) } @@ -175,16 +176,17 @@ object RecordSetValidations { approvedNameServers: List[Regex], recordFqdnDoesNotExist: Boolean, dottedHostZoneConfig: Set[String], - isRecordTypeAndUserAllowed: Boolean + isRecordTypeAndUserAllowed: Boolean, + allowedDotsLimit: Int = 0 ): Either[Throwable, Unit] = newRecordSet.typ match { - case CNAME => cnameValidations(newRecordSet, existingRecordsWithName, zone, existingRecordSet, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed) - case NS => nsValidations(newRecordSet, zone, existingRecordSet, approvedNameServers, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed) - case SOA => soaValidations(newRecordSet, zone, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed) + case CNAME => cnameValidations(newRecordSet, existingRecordsWithName, zone, existingRecordSet, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed, allowedDotsLimit) + case NS => nsValidations(newRecordSet, zone, existingRecordSet, approvedNameServers, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed, allowedDotsLimit) + case SOA => soaValidations(newRecordSet, zone, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed, allowedDotsLimit) case PTR => ptrValidations(newRecordSet, zone) case SRV | TXT | NAPTR => ().asRight // SRV, TXT and NAPTR do not go through dotted host check - case DS => dsValidations(newRecordSet, existingRecordsWithName, zone, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed) - case _ => checkForDot(newRecordSet, zone, existingRecordSet, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed) + case DS => dsValidations(newRecordSet, existingRecordsWithName, zone, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed, allowedDotsLimit) + case _ => checkForDot(newRecordSet, zone, existingRecordSet, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed, allowedDotsLimit) } def typeSpecificDeleteValidations(recordSet: RecordSet, zone: Zone): Either[Throwable, Unit] = @@ -208,7 +210,8 @@ object RecordSetValidations { existingRecordSet: Option[RecordSet] = None, recordFqdnDoesNotExist: Boolean, dottedHostZoneConfig: Set[String], - isRecordTypeAndUserAllowed: Boolean + isRecordTypeAndUserAllowed: Boolean, + allowedDotsLimit: Int = 0 ): Either[Throwable, Unit] = { // cannot create a cname record if a record with the same exists val noRecordWithName = { @@ -241,7 +244,7 @@ object RecordSetValidations { ) _ <- noRecordWithName _ <- RDataWithConsecutiveDots - _ <- checkForDot(newRecordSet, zone, existingRecordSet, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed) + _ <- checkForDot(newRecordSet, zone, existingRecordSet, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed, allowedDotsLimit) } yield () } @@ -252,7 +255,8 @@ object RecordSetValidations { zone: Zone, recordFqdnDoesNotExist: Boolean, dottedHostZoneConfig: Set[String], - isRecordTypeAndUserAllowed: Boolean + isRecordTypeAndUserAllowed: Boolean, + allowedDotsLimit: Int = 0 ): Either[Throwable, Unit] = { // see https://tools.ietf.org/html/rfc4035#section-2.4 val nsChecks = existingRecordsWithName.find(_.typ == NS) match { @@ -265,7 +269,7 @@ object RecordSetValidations { } for { - _ <- checkForDot(newRecordSet, zone, None, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed) + _ <- checkForDot(newRecordSet, zone, None, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed, allowedDotsLimit) _ <- isNotOrigin( newRecordSet, zone, @@ -282,10 +286,11 @@ object RecordSetValidations { approvedNameServers: List[Regex], recordFqdnDoesNotExist: Boolean, dottedHostZoneConfig: Set[String], - isRecordTypeAndUserAllowed: Boolean + isRecordTypeAndUserAllowed: Boolean, + allowedDotsLimit: Int = 0 ): Either[Throwable, Unit] = { // TODO kept consistency with old validation. Not sure why NS could be dotted in reverse specifically - val isNotDottedHost = if (!zone.isReverse) checkForDot(newRecordSet, zone, None, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed) else ().asRight + val isNotDottedHost = if (!zone.isReverse) checkForDot(newRecordSet, zone, None, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed, allowedDotsLimit) else ().asRight for { _ <- isNotDottedHost @@ -307,9 +312,9 @@ object RecordSetValidations { } yield () } - def soaValidations(newRecordSet: RecordSet, zone: Zone, recordFqdnDoesNotExist: Boolean, dottedHostZoneConfig: Set[String], isRecordTypeAndUserAllowed: Boolean): Either[Throwable, Unit] = + def soaValidations(newRecordSet: RecordSet, zone: Zone, recordFqdnDoesNotExist: Boolean, dottedHostZoneConfig: Set[String], isRecordTypeAndUserAllowed: Boolean, allowedDotsLimit: Int = 0): Either[Throwable, Unit] = // TODO kept consistency with old validation. in theory if SOA always == zone name, no special case is needed here - if (!zone.isReverse) checkForDot(newRecordSet, zone, None, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed) else ().asRight + if (!zone.isReverse) checkForDot(newRecordSet, zone, None, recordFqdnDoesNotExist, dottedHostZoneConfig, isRecordTypeAndUserAllowed, allowedDotsLimit) else ().asRight def ptrValidations(newRecordSet: RecordSet, zone: Zone): Either[Throwable, Unit] = // TODO we don't check for PTR as dotted...not sure why diff --git a/modules/api/src/test/scala/vinyldns/api/VinylDNSTestHelpers.scala b/modules/api/src/test/scala/vinyldns/api/VinylDNSTestHelpers.scala index 08c4d28e1..94eabc111 100644 --- a/modules/api/src/test/scala/vinyldns/api/VinylDNSTestHelpers.scala +++ b/modules/api/src/test/scala/vinyldns/api/VinylDNSTestHelpers.scala @@ -18,7 +18,7 @@ package vinyldns.api import com.comcast.ip4s.IpAddress import org.joda.time.DateTime -import vinyldns.api.config.{AuthConfigs, BatchChangeConfig, DottedHostsConfig, HighValueDomainConfig, LimitsConfig, ManualReviewConfig, ScheduledChangesConfig} +import vinyldns.api.config.{ZoneAuthConfigs, BatchChangeConfig, DottedHostsConfig, HighValueDomainConfig, LimitsConfig, ManualReviewConfig, ScheduledChangesConfig} import vinyldns.api.domain.batch.V6DiscoveryNibbleBoundaries import vinyldns.core.domain.record._ import vinyldns.core.domain.zone._ @@ -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"), 3), AuthConfigs("abc.zone.recordsets.",List("locked"),List("dummy"),List("CNAME"), 3), AuthConfigs("xyz.",List("super"),List("xyz"),List("CNAME"), 3))) + val dottedHostsConfig: DottedHostsConfig = DottedHostsConfig(List(ZoneAuthConfigs("dotted.xyz.",List("xyz"),List("dummy"),List("CNAME"), 3), ZoneAuthConfigs("abc.zone.recordsets.",List("locked"),List("dummy"),List("CNAME"), 3), ZoneAuthConfigs("xyz.",List("super"),List("xyz"),List("CNAME"), 3), ZoneAuthConfigs("dot.xyz.",List("super"),List("xyz"),List("CNAME"), 0))) val emptyDottedHostsConfig: DottedHostsConfig = DottedHostsConfig(List.empty) 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 9f613b7e3..6891fbddd 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 @@ -24,7 +24,7 @@ import org.scalatestplus.mockito.MockitoSugar import org.scalatest.matchers.should.Matchers import org.scalatest.wordspec.AnyWordSpec import org.scalatest.BeforeAndAfterEach -import vinyldns.api.config.{AuthConfigs, DottedHostsConfig} +import vinyldns.api.config.{ZoneAuthConfigs, DottedHostsConfig} import vinyldns.api.{ResultHelpers, VinylDNSTestHelpers} import vinyldns.api.domain.access.AccessValidations import vinyldns.api.domain.record.RecordSetHelpers._ @@ -128,14 +128,14 @@ class RecordSetServiceSpec ) def getDottedHostsConfigGroupsAllowed(zone: Zone, config: DottedHostsConfig): List[String] = { - val configZones = config.authConfigs.map(x => x.zone) + val configZones = config.zoneAuthConfigs.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-z.]*")) val isContainWildcardZone = dottedZoneConfig.exists(x => zoneName.substring(0, zoneName.length - 1).matches(x)) val isContainNormalZone = configZones.contains(zoneName) val groups = if (isContainWildcardZone || isContainNormalZone) { - config.authConfigs.flatMap { - x: AuthConfigs => + config.zoneAuthConfigs.flatMap { + x: ZoneAuthConfigs => if (x.zone.contains("*")) { val wildcardZone = x.zone.replace("*", "[A-Za-z.]*") if (zoneName.substring(0, zoneName.length - 1).matches(wildcardZone)) x.allowedGroupList else List.empty @@ -150,7 +150,7 @@ class RecordSetServiceSpec groups } - val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.authConfigs.map(x => x.zone) + val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.zoneAuthConfigs.map(x => x.zone) val dottedHostsConfigGroupsAllowed: List[String] = getDottedHostsConfigGroupsAllowed(okZone, VinylDNSTestHelpers.dottedHostsConfig) @@ -164,7 +164,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(okZone.id, record.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -234,7 +234,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(okZone.id, record.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -304,7 +304,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(okZone.id, record.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -350,7 +350,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(okZone.id, record.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -389,7 +389,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(okZone.id, record.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -447,7 +447,7 @@ class RecordSetServiceSpec doReturn(IO.pure(Some(okGroup))) .when(mockGroupRepo) .getGroup(okGroup.id) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -521,7 +521,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(okZone.id, record.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -560,7 +560,7 @@ class RecordSetServiceSpec val record = cname.copy(name = "new.name", zoneId = dottedZone.id, status = RecordSetStatus.Active) - val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.authConfigs.map(x => x.zone) + val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.zoneAuthConfigs.map(x => x.zone) val dottedHostsConfigGroupsAllowed: List[String] = getDottedHostsConfigGroupsAllowed(dottedZone, VinylDNSTestHelpers.dottedHostsConfig) @@ -571,7 +571,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(dottedZone.id, record.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -604,7 +604,7 @@ class RecordSetServiceSpec val record = cname.copy(name = "new.name", zoneId = xyzZone.id, status = RecordSetStatus.Active) - val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.authConfigs.map(x => x.zone) + val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.zoneAuthConfigs.map(x => x.zone) val dottedHostsConfigGroupsAllowed: List[String] = getDottedHostsConfigGroupsAllowed(xyzZone, VinylDNSTestHelpers.dottedHostsConfig) @@ -644,6 +644,47 @@ class RecordSetServiceSpec result.recordSet.name shouldBe record.name } + "fail if the record is dotted and zone, user, record type is allowed but number of dots allowed in config is 0" in { + val record = + cname.copy(name = "new.name", zoneId = dotZone.id, status = RecordSetStatus.Active) + + val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.zoneAuthConfigs.map(x => x.zone) + + val dottedHostsConfigGroupsAllowed: List[String] = getDottedHostsConfigGroupsAllowed(dottedZone, VinylDNSTestHelpers.dottedHostsConfig) + + doReturn(IO.pure(Some(dotZone))).when(mockZoneRepo).getZone(dotZone.id) + doReturn(IO.pure(List())) + .when(mockRecordRepo) + .getRecordSets(dotZone.id, record.name, record.typ) + doReturn(IO.pure(List())) + .when(mockRecordRepo) + .getRecordSetsByName(dotZone.id, record.name) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) + .when(mockZoneRepo) + .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) + doReturn(IO.pure(Set())) + .when(mockZoneRepo) + .getZonesByFilters(Set.empty) + doReturn(IO.pure(None)) + .when(mockZoneRepo) + .getZoneByName(record.name + "." + dotZone.name) + doReturn(IO.pure(List())) + .when(mockRecordRepo) + .getRecordSetsByFQDNs(Set(record.name + "." + dotZone.name)) + doReturn(IO.pure(Set())) + .when(mockZoneRepo) + .getZonesByFilters(record.name.split('.').map(x => x + "." + dotZone.name).toSet) + doReturn(IO.pure(Set(dummyGroup))) + .when(mockGroupRepo) + .getGroupsByName(dottedHostsConfigGroupsAllowed.toSet) + doReturn(IO.pure(ListUsersResults(listOfDummyUsers.toSeq, None))) + .when(mockUserRepo) + .getUsers(dummyGroup.memberIds, None, None) + + // fails as no.of.dots allowed for the zone in the config is 0 + val result = leftResultOf(underTest.addRecordSet(record, xyzAuth).value) + result shouldBe an[InvalidRequest] + } "fail if the record is dotted and user, record type is in allowed dotted hosts config except zone" in { val record = cname.copy(name = "new.name", zoneId = okZone.id, status = RecordSetStatus.Active) @@ -654,7 +695,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(okZone.id, record.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -684,7 +725,7 @@ class RecordSetServiceSpec val record = cname.copy(name = "new.name", zoneId = abcZone.id, status = RecordSetStatus.Active) - val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.authConfigs.map(x => x.zone) + val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.zoneAuthConfigs.map(x => x.zone) val dottedHostsConfigGroupsAllowed: List[String] = getDottedHostsConfigGroupsAllowed(abcZone, VinylDNSTestHelpers.dottedHostsConfig) @@ -725,8 +766,8 @@ class RecordSetServiceSpec val record = aaaa.copy(name = "new.name", zoneId = dottedZone.id, status = RecordSetStatus.Active) - val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.authConfigs.map { - case y:AuthConfigs => y.zone + val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.zoneAuthConfigs.map { + case y:ZoneAuthConfigs => y.zone } val dottedHostsConfigGroupsAllowed: List[String] = getDottedHostsConfigGroupsAllowed(dottedZone, VinylDNSTestHelpers.dottedHostsConfig) @@ -738,7 +779,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(dottedZone.id, record.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -779,7 +820,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(okZone.id, newRecord.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -836,7 +877,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(okZone.id, newRecord.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -896,7 +937,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(okZone.id, newRecord.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -939,7 +980,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(okZone.id, newRecord.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -982,7 +1023,7 @@ class RecordSetServiceSpec doReturn(IO.pure(List())) .when(mockRecordRepo) .getRecordSetsByName(okZone.id, newRecord.name) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) @@ -1138,7 +1179,7 @@ class RecordSetServiceSpec doReturn(IO.pure(Some(oneUserDummyGroup))) .when(mockGroupRepo) .getGroup(oneUserDummyGroup.id) - doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone))) + doReturn(IO.pure(Set(dottedZone, abcZone, xyzZone, dotZone))) .when(mockZoneRepo) .getZonesByNames(dottedHostsConfigZonesAllowed.toSet) doReturn(IO.pure(Set())) diff --git a/modules/api/src/test/scala/vinyldns/api/domain/record/RecordSetValidationsSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/record/RecordSetValidationsSpec.scala index 10fd0a99e..e0a03905e 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/record/RecordSetValidationsSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/record/RecordSetValidationsSpec.scala @@ -44,7 +44,7 @@ class RecordSetValidationsSpec import RecordSetValidations._ - val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.authConfigs.map(x => x.zone) + val dottedHostsConfigZonesAllowed: List[String] = VinylDNSTestHelpers.dottedHostsConfig.zoneAuthConfigs.map(x => x.zone) "RecordSetValidations" should { "validRecordTypes" should { @@ -232,18 +232,30 @@ class RecordSetValidationsSpec } "return a success for any new record with dotted hosts in forward zones if it satisfies dotted hosts configs" in { - val record = typeSpecificValidations(dottedARecord.copy(zoneId = dottedZone.id), List(), dottedZone, None, Nil, true, dottedHostsConfigZonesAllowed.toSet, true) + // Zone, User, Record Type and Number of dots are all satisfied + val record = typeSpecificValidations(dottedARecord.copy(typ = CNAME, zoneId = dottedZone.id), List(), dottedZone, None, Nil, true, dottedHostsConfigZonesAllowed.toSet, true, 5) record should be(right) } - "return a failure for any new record with dotted hosts in forward zones (CNAME) if it satisfies dotted hosts configs" in { - val record = typeSpecificValidations(dottedARecord.copy(typ = CNAME, zoneId = dottedZone.id), List(), dottedZone, None, Nil, true, dottedHostsConfigZonesAllowed.toSet, true) - record should be(right) + "return a failure for any new record with dotted hosts if no.of.dots allowed is 0" in { + // Zone, User, Record Type and Number of dots are all satisfied + leftValue( + typeSpecificValidations(dottedARecord.copy(typ = CNAME, zoneId = dottedZone.id), List(), dottedZone, None, Nil, true, dottedHostsConfigZonesAllowed.toSet, true, 0) + ) shouldBe an[InvalidRequest] } - "return a failure for any new record with dotted hosts in forward zones (NS) if it satisfies dotted hosts configs" in { - val record = typeSpecificValidations(dottedARecord.copy(typ = NS, zoneId = dottedZone.id), List(), dottedZone, None, Nil, true, dottedHostsConfigZonesAllowed.toSet, true) - record should be(right) + "return a failure for any new record with dotted hosts in forward zones (A record) if it doesn't satisfy dotted hosts configs" in { + // 'A' record is not allowed in the config + leftValue( + typeSpecificValidations(dottedARecord.copy(zoneId = dottedZone.id), List(), dottedZone, None, Nil, true, dottedHostsConfigZonesAllowed.toSet, false, 5) + ) shouldBe an[InvalidRequest] + } + + "return a failure for any new record with dotted hosts in forward zones (NS record) if it doesn't satisfy dotted hosts configs" in { + // 'NS' record is not allowed in the config + leftValue( + typeSpecificValidations(dottedARecord.copy(typ = NS, zoneId = dottedZone.id), List(), dottedZone, None, Nil, true, dottedHostsConfigZonesAllowed.toSet, false, 5) + ) shouldBe an[InvalidRequest] } "return a success for any existing record with dotted hosts in forward zones" in { @@ -445,7 +457,7 @@ class RecordSetValidationsSpec } "return ok if the DS is dotted and zone, user, record type is allowed in dotted hosts config" in { val record = - dsValidations(ds.copy(name = "dotted.trial", zoneId = dottedZone.id), List(matchingNs), dottedZone, true, dottedHostsConfigZonesAllowed.toSet, true) + dsValidations(ds.copy(name = "dotted.trial", zoneId = dottedZone.id), List(matchingNs), dottedZone, true, dottedHostsConfigZonesAllowed.toSet, true, 5) record should be(right) } "return an InvalidRequest if the DS is dotted and zone, user, record type is allowed in dotted hosts config but has a conflict with existing record or zone" in { @@ -508,7 +520,7 @@ class RecordSetValidationsSpec } "return ok if the CNAME is dotted and zone, user, record type is allowed in dotted hosts config" in { val record = - cnameValidations(cname.copy(name = "dot.ted", zoneId = dottedZone.id), List(), dottedZone, None, true, dottedHostsConfigZonesAllowed.toSet, true) + cnameValidations(cname.copy(name = "dot.ted", zoneId = dottedZone.id), List(), dottedZone, None, true, dottedHostsConfigZonesAllowed.toSet, true, 5) record should be(right) } "return an InvalidRequest if the CNAME is dotted and zone, user, record type is allowed in dotted hosts config but has a conflict with existing record or zone" in { diff --git a/modules/core/src/test/scala/vinyldns/core/TestZoneData.scala b/modules/core/src/test/scala/vinyldns/core/TestZoneData.scala index 34c1495f1..df08e8a0a 100644 --- a/modules/core/src/test/scala/vinyldns/core/TestZoneData.scala +++ b/modules/core/src/test/scala/vinyldns/core/TestZoneData.scala @@ -35,6 +35,7 @@ object TestZoneData { connection = testConnection ) val dottedZone: Zone = Zone("dotted.xyz.", "dotted@xyz.com", adminGroupId = xyzGroup.id) + val dotZone: Zone = Zone("dot.xyz.", "dotted@xyz.com", adminGroupId = xyzGroup.id) val abcZone: Zone = Zone("abc.zone.recordsets.", "test@test.com", adminGroupId = abcGroup.id) val xyzZone: Zone = Zone("xyz.", "abc@xyz.com", adminGroupId = xyzGroup.id) val zoneIp4: Zone = Zone("0.162.198.in-addr.arpa.", "test@test.com", adminGroupId = abcGroup.id)