From 7f65dc658234e1c0c4a70d0075dde3bda1ddb915 Mon Sep 17 00:00:00 2001 From: Aravindh-Raju Date: Mon, 2 May 2022 13:42:43 +0530 Subject: [PATCH 01/32] Validate group fields --- .../api/domain/membership/MembershipProtocol.scala | 2 ++ .../api/domain/membership/MembershipService.scala | 12 ++++++++++++ .../scala/vinyldns/api/route/MembershipRouting.scala | 1 + .../core/src/main/scala/vinyldns/core/Messages.scala | 2 ++ 4 files changed, 17 insertions(+) diff --git a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipProtocol.scala b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipProtocol.scala index 6db87bb29..1b39e1b50 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipProtocol.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipProtocol.scala @@ -153,6 +153,8 @@ final case class GroupNotFoundError(msg: String) extends Throwable(msg) final case class GroupAlreadyExistsError(msg: String) extends Throwable(msg) +final case class GroupValidationError(msg: String) extends Throwable(msg) + final case class UserNotFoundError(msg: String) extends Throwable(msg) final case class InvalidGroupError(msg: String) extends Throwable(msg) diff --git a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipService.scala b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipService.scala index 5d24297e6..92dbbf3e0 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipService.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipService.scala @@ -57,6 +57,7 @@ class MembershipService( val adminMembers = inputGroup.adminUserIds val nonAdminMembers = inputGroup.memberIds.diff(adminMembers) for { + _ <- groupValidation(newGroup) _ <- hasMembersAndAdmins(newGroup).toResult _ <- groupWithSameNameDoesNotExist(newGroup.name) _ <- usersExist(newGroup.memberIds) @@ -76,6 +77,7 @@ class MembershipService( for { existingGroup <- getExistingGroup(groupId) newGroup = existingGroup.withUpdates(name, email, description, memberIds, adminUserIds) + _ <- groupValidation(newGroup) _ <- canEditGroup(existingGroup, authPrincipal).toResult addedAdmins = newGroup.adminUserIds.diff(existingGroup.adminUserIds) // new non-admin members ++ admins converted to non-admins @@ -263,6 +265,16 @@ class MembershipService( .orFail(GroupNotFoundError(s"Group with ID $groupId was not found")) .toResult[Group] + // Validate group details. Group name and email cannot be empty + def groupValidation(group: Group): Result[Unit] = { + Option(group) match { + case Some(value) if value.name.isEmpty || value.email.isEmpty => + GroupValidationError(GroupValidationErrorMsg).asLeft + case _ => + ().asRight + } + }.toResult + def groupWithSameNameDoesNotExist(name: String): Result[Unit] = groupRepo .getGroupByName(name) diff --git a/modules/api/src/main/scala/vinyldns/api/route/MembershipRouting.scala b/modules/api/src/main/scala/vinyldns/api/route/MembershipRouting.scala index 6611a7346..5704c12a5 100644 --- a/modules/api/src/main/scala/vinyldns/api/route/MembershipRouting.scala +++ b/modules/api/src/main/scala/vinyldns/api/route/MembershipRouting.scala @@ -45,6 +45,7 @@ class MembershipRoute( case GroupNotFoundError(msg) => complete(StatusCodes.NotFound, msg) case NotAuthorizedError(msg) => complete(StatusCodes.Forbidden, msg) case GroupAlreadyExistsError(msg) => complete(StatusCodes.Conflict, msg) + case GroupValidationError(msg) => complete(StatusCodes.BadRequest, msg) case InvalidGroupError(msg) => complete(StatusCodes.BadRequest, msg) case UserNotFoundError(msg) => complete(StatusCodes.NotFound, msg) case InvalidGroupRequestError(msg) => complete(StatusCodes.BadRequest, msg) diff --git a/modules/core/src/main/scala/vinyldns/core/Messages.scala b/modules/core/src/main/scala/vinyldns/core/Messages.scala index fcaff7c2b..771488300 100644 --- a/modules/core/src/main/scala/vinyldns/core/Messages.scala +++ b/modules/core/src/main/scala/vinyldns/core/Messages.scala @@ -79,4 +79,6 @@ object Messages { val NotAuthorizedErrorMsg = "User \"%s\" is not authorized. Contact %s owner group: %s at %s to make DNS changes." + // Error displayed when group name or email is empty + val GroupValidationErrorMsg = "Group name and email cannot be empty." } From d9a4d7828d9deaa2a7f75fe73051aa8d4b3f7ab5 Mon Sep 17 00:00:00 2001 From: Aravindh-Raju Date: Mon, 2 May 2022 15:32:12 +0530 Subject: [PATCH 02/32] Add tests --- .../membership/MembershipServiceSpec.scala | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/modules/api/src/test/scala/vinyldns/api/domain/membership/MembershipServiceSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/membership/MembershipServiceSpec.scala index d1e8fd3fa..ac13446d0 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/membership/MembershipServiceSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/membership/MembershipServiceSpec.scala @@ -114,6 +114,7 @@ class MembershipServiceSpec "create a new group" should { "save the group and add the members when the group is valid" in { doReturn(IO.pure(Some(okUser))).when(mockUserRepo).getUser("ok") + doReturn(().toResult).when(underTest).groupValidation(groupInfo) doReturn(().toResult).when(underTest).groupWithSameNameDoesNotExist(groupInfo.name) doReturn(().toResult).when(underTest).usersExist(groupInfo.memberIds) doReturn(IO.pure(okGroup)).when(mockGroupRepo).save(any[DB], any[Group]) @@ -141,6 +142,7 @@ class MembershipServiceSpec "save the groupChange in the groupChangeRepo" in { doReturn(IO.pure(Some(okUser))).when(mockUserRepo).getUser("ok") + doReturn(().toResult).when(underTest).groupValidation(groupInfo) doReturn(().toResult).when(underTest).groupWithSameNameDoesNotExist(groupInfo.name) doReturn(().toResult).when(underTest).usersExist(groupInfo.memberIds) doReturn(IO.pure(okGroup)).when(mockGroupRepo).save(any[DB], any[Group]) @@ -168,7 +170,7 @@ class MembershipServiceSpec adminUserIds = Set(okUserInfo.id, dummyUserInfo.id) ) val expectedMembersAdded = Set(okUserInfo.id, dummyUserInfo.id) - + doReturn(().toResult).when(underTest).groupValidation(info) doReturn(().toResult).when(underTest).groupWithSameNameDoesNotExist(info.name) doReturn(().toResult).when(underTest).usersExist(any[Set[String]]) doReturn(IO.pure(okGroup)).when(mockGroupRepo).save(any[DB], any[Group]) @@ -196,6 +198,7 @@ class MembershipServiceSpec "set the current user as a member" in { val info = groupInfo.copy(memberIds = Set.empty, adminUserIds = Set.empty) doReturn(IO.pure(Some(okUser))).when(mockUserRepo).getUser("ok") + doReturn(().toResult).when(underTest).groupValidation(info) doReturn(().toResult).when(underTest).groupWithSameNameDoesNotExist(info.name) doReturn(().toResult).when(underTest).usersExist(Set(okAuth.userId)) doReturn(IO.pure(okGroup)).when(mockGroupRepo).save(any[DB], any[Group]) @@ -224,6 +227,7 @@ class MembershipServiceSpec "return an error if users do not exist" in { doReturn(IO.pure(Some(okUser))).when(mockUserRepo).getUser("ok") + doReturn(().toResult).when(underTest).groupValidation(groupInfo) doReturn(().toResult).when(underTest).groupWithSameNameDoesNotExist(groupInfo.name) doReturn(result(UserNotFoundError("fail"))) .when(underTest) @@ -239,6 +243,7 @@ class MembershipServiceSpec "return an error if fail while saving the group" in { doReturn(IO.pure(Some(okUser))).when(mockUserRepo).getUser("ok") + doReturn(().toResult).when(underTest).groupValidation(groupInfo) doReturn(().toResult).when(underTest).groupWithSameNameDoesNotExist(groupInfo.name) doReturn(().toResult).when(underTest).usersExist(groupInfo.memberIds) doReturn(IO.raiseError(new RuntimeException("fail"))).when(mockGroupRepo).save(any[DB], any[Group]) @@ -253,6 +258,7 @@ class MembershipServiceSpec "return an error if fail while adding the members" in { doReturn(IO.pure(Some(okUser))).when(mockUserRepo).getUser("ok") + doReturn(().toResult).when(underTest).groupValidation(groupInfo) doReturn(().toResult).when(underTest).groupWithSameNameDoesNotExist(groupInfo.name) doReturn(().toResult).when(underTest).usersExist(groupInfo.memberIds) doReturn(IO.pure(okGroup)).when(mockGroupRepo).save(any[DB], any[Group]) @@ -264,6 +270,20 @@ class MembershipServiceSpec val error = leftResultOf(underTest.createGroup(groupInfo, okAuth).value) error shouldBe a[RuntimeException] } + + "return an error if group name and/or email is empty" in { + doReturn(IO.pure(Some(okUser))).when(mockUserRepo).getUser("ok") + doReturn(result(GroupValidationError("fail"))) + .when(underTest) + .groupValidation(groupInfo.copy(name = "", email = "")) + + val error = leftResultOf(underTest.createGroup(groupInfo.copy(name = "", email = ""), okAuth).value) + error shouldBe a[GroupValidationError] + + verify(mockGroupRepo, never()).save(any[DB], any[Group]) + verify(mockMembershipRepo, never()) + .saveMembers(any[DB], anyString, any[Set[String]], isAdmin = anyBoolean) + } } "update an existing group" should { @@ -388,6 +408,31 @@ class MembershipServiceSpec error shouldBe a[GroupAlreadyExistsError] } + "return an error if group name and/or email is empty" in { + doReturn(IO.pure(Some(existingGroup))) + .when(mockGroupRepo) + .getGroup(existingGroup.id) + doReturn(().toResult).when(underTest).usersExist(any[Set[String]]) + doReturn(result(GroupValidationError("fail"))) + .when(underTest) + .groupValidation(existingGroup.copy(name = "", email = "")) + + val error = leftResultOf( + underTest + .updateGroup( + updatedInfo.id, + name = "", + email = "", + updatedInfo.description, + updatedInfo.memberIds, + updatedInfo.adminUserIds, + okAuth + ) + .value + ) + error shouldBe a[GroupValidationError] + } + "return an error if the group is not found" in { doReturn(IO.pure(None)).when(mockGroupRepo).getGroup(existingGroup.id) From ad0a37236a48fafd02d35cce9a7cba8ab66cc640 Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Mon, 23 May 2022 13:12:33 +0530 Subject: [PATCH 03/32] Added forward slash validation for forward and reverse zone --- .../api/domain/DomainValidations.scala | 34 +++++++++++++++++-- .../domain/batch/BatchChangeValidations.scala | 18 +++++++--- .../batch/BatchChangeValidationsSpec.scala | 17 ++++++++++ 3 files changed, 63 insertions(+), 6 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..73fa6cfac 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/DomainValidations.scala @@ -27,8 +27,10 @@ import scala.util.matching.Regex Object to house common domain validations */ object DomainValidations { + val validReverseFQDNRegex: 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 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 + """^(?:([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 = """^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$""".r val validIpv6Regex: Regex = @@ -60,7 +62,35 @@ object DomainValidations { def validateHostName(name: Fqdn): ValidatedNel[DomainValidationError, Fqdn] = validateHostName(name.fqdn).map(_ => name) + def validateReverseHostName(name: Fqdn): ValidatedNel[DomainValidationError, Fqdn] = + validateReverseHostName(name.fqdn).map(_ => name) + def validateHostName(name: String): ValidatedNel[DomainValidationError, String] = { + /* + Label rules are as follows (from RFC 952; detailed in RFC 1034): + - Starts with a letter, or digit, or underscore or asterisk (as of RFC 1123) + - Interior contains letter, digit or hyphen, or underscore + - Ends with a letter or digit, or underscore + All possible labels permutations: + - A single letter/digit: [0-9a-zA-Z]{1} + - A combination of 1-63 letters/digits: [0-9a-zA-Z]{1,63} + - A single letter/digit followed by up to 61 letters, digits, hyphens or slashes + and ending with a letter/digit:[0-9a-zA-Z]{1}[0-9a-zA-Z\-]{0,61}[0-9a-zA-Z]{1} + - A wildcard and dot character (*.) followed by up to 60 letters, digits, hyphens + and ending with a letter/digit:[*.]{2}[0-9a-zA-Z\-_]{0,60}[0-9a-zA-Z_]{1} + A valid domain name is a series of one or more