From 70eefc2f05bf193cc2ee45d91c4d538708e9f60c Mon Sep 17 00:00:00 2001
From: Andy Spaven <16537059+AndySpaven@users.noreply.github.com>
Date: Thu, 17 Sep 2020 10:50:39 +0100
Subject: [PATCH] APSR-1253 - Add mdtp to allowed domains for url validation

Co-authored-by: Anjum Abbas <khaakpaayali@gmail.com>
Co-authored-by: Pete Slater <pslater@equalexperts.com>
---
 .../SubscriptionFieldsController.scala        |  6 ++-
 .../apisubscriptionfields/model/Model.scala   |  8 ++--
 .../model/ModelSpec.scala                     | 37 ++++++++++---------
 3 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/app/uk/gov/hmrc/apisubscriptionfields/controller/SubscriptionFieldsController.scala b/app/uk/gov/hmrc/apisubscriptionfields/controller/SubscriptionFieldsController.scala
index f61c607..ae4379a 100644
--- 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
index af35b50..4a22751 100644
--- 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
index 1c6d5a3..c71c383 100644
--- 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
+    }
   }
 }
\ No newline at end of file