Merge pull request #88 from hmrc/APSR-1253

APSR-1253 - Add mdtp to allowed domains for url validation
hmrc · Sep 17, 2020 · 19e6f3e · 19e6f3e
2 parents 5c22cf2 + 70eefc2
commit 19e6f3e
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 22 deletions.
diff --git a/app/uk/gov/hmrc/apisubscriptionfields/controller/SubscriptionFieldsController.scala b/app/uk/gov/hmrc/apisubscriptionfields/controller/SubscriptionFieldsController.scala
@@ -90,9 +90,11 @@ class SubscriptionFieldsController @Inject()(cc: ControllerComponents, service:
       else {
         service.upsert(clientId, apiContext, apiVersion, payload.fields).map( _ match {
           case NotFoundSubsFieldsUpsertResponse                             => BadRequest(Json.toJson("reason" -> "field definitions not found")) // TODO
-          case FailedValidationSubsFieldsUpsertResponse(fieldErrorMessages) => BadRequest(Json.toJson(fieldErrorMessages))
+          case FailedValidationSubsFieldsUpsertResponse(fieldErrorMessages) => 
+            BadRequest(Json.toJson(fieldErrorMessages))
           case SuccessfulSubsFieldsUpsertResponse(response, true)           => Created(Json.toJson(response))
-          case SuccessfulSubsFieldsUpsertResponse(response, false)          => Ok(Json.toJson(response))
+          case SuccessfulSubsFieldsUpsertResponse(response, false)          => 
+            Ok(Json.toJson(response))
         })
         .recover(recovery)
       }

diff --git a/app/uk/gov/hmrc/apisubscriptionfields/model/Model.scala b/app/uk/gov/hmrc/apisubscriptionfields/model/Model.scala
@@ -19,8 +19,8 @@ package uk.gov.hmrc.apisubscriptionfields.model
 import cats.data.{NonEmptyList => NEL}
 import uk.gov.hmrc.apisubscriptionfields.model.FieldDefinitionType.FieldDefinitionType
 import org.apache.commons.validator.routines.UrlValidator
-import eu.timepit.refined._
 import Types._
+import org.apache.commons.validator.routines.DomainValidator
 
 sealed trait ValidationRule {
   def validate(value: FieldValue): Boolean = {
@@ -36,9 +36,11 @@ case class RegexValidationRule(regex: RegexExpr) extends ValidationRule {
 
 // Taken from: https://stackoverflow.com/a/5078838
 case object UrlValidationRule extends ValidationRule {
+  DomainValidator.updateTLDOverride(DomainValidator.ArrayType.GENERIC_PLUS, Array("mdtp"));
+  private val schemes = Array("http","https")
+  private lazy val urlValidator = new org.apache.commons.validator.routines.UrlValidator(schemes, UrlValidator.ALLOW_LOCAL_URLS)
+
   def validateAgainstRule(value: FieldValue): Boolean = {
-    val schemes = Array("http","https")
-    val urlValidator = new org.apache.commons.validator.routines.UrlValidator(schemes, UrlValidator.ALLOW_LOCAL_URLS)
     urlValidator.isValid(value)
   }
 }

diff --git a/test/uk/gov/hmrc/apisubscriptionfields/model/ModelSpec.scala b/test/uk/gov/hmrc/apisubscriptionfields/model/ModelSpec.scala
@@ -19,7 +19,6 @@ package uk.gov.hmrc.apisubscriptionfields.model
 import uk.gov.hmrc.apisubscriptionfields.SubscriptionFieldsTestData
 import uk.gov.hmrc.apisubscriptionfields.FieldDefinitionTestData
 import uk.gov.hmrc.apisubscriptionfields.HmrcSpec
-import org.scalatest.FunSpec
 import org.scalatest.Matchers
 
 class ModelSpec extends HmrcSpec with SubscriptionFieldsTestData with FieldDefinitionTestData with ValidationRuleTestData {
@@ -42,24 +41,28 @@ class ModelSpec extends HmrcSpec with SubscriptionFieldsTestData with FieldDefin
   }
 }
 
-class UrlValidationRuleSpec extends FunSpec with ValidationRuleTestData with Matchers {
-  describe("pass for a matching value") {
-    UrlValidationRule.validate(validUrl) shouldBe true
-  }
+class UrlValidationRuleSpec extends HmrcSpec with ValidationRuleTestData with Matchers {
+  "url validation rule" should {
+    "pass for a matching value" in {
+      UrlValidationRule.validate(validUrl) shouldBe true
+    }
 
-  describe("pass for localhost") {
-    UrlValidationRule.validate(localValidUrl) shouldBe true
-  }
+    "pass for localhost" in {
+      UrlValidationRule.validate(localValidUrl) shouldBe true
+    }
 
-  describe("return true when the value is blank") {
-    UrlValidationRule.validate("") shouldBe true
-  }
+    "return true when the value is blank" in {
+      UrlValidationRule.validate("") shouldBe true
+    }
 
-  describe("invalid urls") {
-   invalidUrls.map(invalidUrl =>  {
-     it(s"$invalidUrl should not be valid") {
-       UrlValidationRule.validate(invalidUrl) shouldBe false
-     }
-   })
+    "invalid urls" in {
+      invalidUrls.map(invalidUrl =>  {
+        UrlValidationRule.validate(invalidUrl) shouldBe false
+      })
+    }
+
+    "handles internal mdtp domains in url" in {
+     UrlValidationRule.validate("https://who-cares.mdtp/pathy/mcpathface") shouldBe true
+    }
   }
 }