MTDSA-8886: Investigate Play 28 upgrade (#56)

* MTDSA-8886: Update non-play plugins.

* MTDSA-8886: Bump sbt version to see if anything breaks.

* MTDSA-8886: Bump everything to latest Play 28 version available. Copy out code from domain library as it does not have a Play 28 version and isn't going to.

* MTDSA-8886: Remove dependency comments.

* MTDSA-8886: Remove build.sbt deprecations.

* MTDSA-8886: Remove build.sbt deprecations.

* MTDSA-8886: whoops

* MTDSA-8886: play 28 alignments

* MTDSA-8886: play 28 alignments

* MTDSA-8886: play 28 alignments

* MTDSA-8886: play 28 alignments

* MTDSA-8886: Fix unit testing for DES connectors for new version of HTTP verbs. Some minor renaming for clarity.

* MTDSA-8886: Add environment config option for DES to selectively allow certain headers to be sent to DES/ our stub.

* MTDSA-8886: Remove warnings

* MTDSA-8886: Remove deprecations (and some extra warnings)

* MTDSA-8886: Some renamings and other minor refactoring.

* MTDSA-8886: Remove unused `whitelisting`.

* MTDSA-8886: Some streamlining and coverage refactoring.

* MTDSA-8886: Review comments

app/config/AppConfig.scala

@@ -23,45 +23,46 @@ import uk.gov.hmrc.play.bootstrap.config.ServicesConfig
 import javax.inject.{Inject, Singleton}
 
 trait AppConfig {
-
-  def desBaseUrl: String
-
+  // MTD ID Lookup Config
   def mtdIdBaseUrl: String
 
+  // DES Config
+  def desBaseUrl: String
   def desEnv: String
-
   def desToken: String
+  def desEnvironmentHeaders: Option[Seq[String]]
 
-  def apiGatewayContext: String
+  // Business Rule Config
+  def minimumPermittedTaxYear: Int
 
+  // API Config
+  def apiGatewayContext: String
+  def confidenceLevelConfig: ConfidenceLevelConfig
   def apiStatus(version: String): String
-
   def featureSwitch: Option[Configuration]
-
   def endpointsEnabled(version: String): Boolean
-
-  def minimumPermittedTaxYear: Int
-
-  def confidenceLevelConfig: ConfidenceLevelConfig
 }
 
 @Singleton
 class AppConfigImpl @Inject()(config: ServicesConfig, configuration: Configuration) extends AppConfig {
+  // MTD ID Lookup Config
+  val mtdIdBaseUrl: String = config.baseUrl(serviceName ="mtd-id-lookup")
 
-  val mtdIdBaseUrl: String = config.baseUrl("mtd-id-lookup")
+  // DES Config
   val desBaseUrl: String = config.baseUrl("des")
   val desEnv: String = config.getString("microservice.services.des.env")
   val desToken: String = config.getString("microservice.services.des.token")
-  val apiGatewayContext: String = config.getString("api.gateway.context")
+  val desEnvironmentHeaders: Option[Seq[String]] = configuration.getOptional[Seq[String]]("microservice.services.des.environmentHeaders")
+
+  // Business rule Config
   val minimumPermittedTaxYear: Int = config.getInt("minimumPermittedTaxYear")
 
+  // API Config
+  val apiGatewayContext: String = config.getString("api.gateway.context")
+  val confidenceLevelConfig: ConfidenceLevelConfig = configuration.get[ConfidenceLevelConfig](s"api.confidence-level-check")
   def apiStatus(version: String): String = config.getString(s"api.$version.status")
-
   def featureSwitch: Option[Configuration] = configuration.getOptional[Configuration](s"feature-switch")
-
   def endpointsEnabled(version: String): Boolean = config.getBoolean(s"api.$version.endpoints.enabled")
-
-  val confidenceLevelConfig: ConfidenceLevelConfig = configuration.get[ConfidenceLevelConfig](s"api.confidence-level-check")
 }
 
 trait FixedConfig {

app/definition/ApiDefinition.scala

@@ -20,12 +20,6 @@ import play.api.libs.json.{Format, Json, OFormat}
 import uk.gov.hmrc.auth.core.ConfidenceLevel
 import utils.enums.Enums
 
-case class Access(`type`: String, whitelistedApplicationIds: Seq[String])
-
-object Access {
-  implicit val formatAccess: OFormat[Access] = Json.format[Access]
-}
-
 case class Parameter(name: String, required: Boolean = false)
 
 object Parameter {
@@ -52,7 +46,7 @@ object APIStatus extends Enumeration {
   val parser: PartialFunction[String, APIStatus] = Enums.parser[APIStatus]
 }
 
-case class APIVersion(version: String, access: Option[Access] = None, status: APIStatus, endpointsEnabled: Boolean) {
+case class APIVersion(version: String, status: APIStatus, endpointsEnabled: Boolean) {
 
   require(version.nonEmpty, "version is required")
 }

app/definition/ApiDefinitionFactory.scala

@@ -16,7 +16,7 @@
 
 package definition
 
-import config.{AppConfig, FeatureSwitch}
+import config.AppConfig
 import definition.Versions._
 import play.api.Logger
 import uk.gov.hmrc.auth.core.ConfidenceLevel
@@ -56,7 +56,6 @@ class ApiDefinitionFactory @Inject()(appConfig: AppConfig) {
         versions = Seq(
           APIVersion(
             version = VERSION_1,
-            access = buildWhiteListingAccess(),
             status = buildAPIStatus(VERSION_1),
             endpointsEnabled = appConfig.endpointsEnabled(VERSION_1)
           )
@@ -72,9 +71,4 @@ class ApiDefinitionFactory @Inject()(appConfig: AppConfig) {
         APIStatus.ALPHA
       }
   }
-
-  private[definition] def buildWhiteListingAccess(): Option[Access] = {
-    val featureSwitch = FeatureSwitch(appConfig.featureSwitch)
-    if (featureSwitch.isWhiteListingEnabled) Some(Access("PRIVATE", featureSwitch.whiteListedApplicationIds)) else None
-  }
 }

app/definition/Versions.scala

@@ -18,20 +18,16 @@ package definition
 
 import play.api.http.HeaderNames.ACCEPT
 import play.api.mvc.RequestHeader
-import uk.gov.hmrc.http.HeaderCarrier
 
 object Versions {
   val VERSION_1 = "1.0"
   val VERSION_2 = "2.0"
 
   private val versionRegex = """application\/vnd.hmrc.(\d.\d)\+json""".r
 
-  def getFromRequest(implicit hc: HeaderCarrier): Option[String] =
-    getFrom(hc.headers)
-
   def getFromRequest(request: RequestHeader): Option[String] =
     getFrom(request.headers.headers)
 
-  private def getFrom(headers: Seq[(String, String)]) =
+  private def getFrom(headers: Seq[(String, String)]): Option[String] =
     headers.collectFirst { case (ACCEPT, versionRegex(ver)) => ver }
 }

app/utils/ErrorHandler.scala

@@ -24,7 +24,7 @@ import play.api.mvc.Results._
 import play.api.mvc._
 import uk.gov.hmrc.auth.core.AuthorisationException
 import uk.gov.hmrc.http._
-import uk.gov.hmrc.play.HeaderCarrierConverter
+import uk.gov.hmrc.play.http.HeaderCarrierConverter
 import uk.gov.hmrc.play.audit.http.connector.AuditConnector
 import uk.gov.hmrc.play.bootstrap.config.HttpAuditEvent
 import uk.gov.hmrc.play.bootstrap.backend.http.JsonErrorHandler
@@ -33,19 +33,17 @@ import v1.models.errors._
 import scala.concurrent._
 
 @Singleton
-class ErrorHandler @Inject()(
-                              config: Configuration,
-                              auditConnector: AuditConnector,
-                              httpAuditEvent: HttpAuditEvent
-                            )(implicit ec: ExecutionContext) extends JsonErrorHandler(auditConnector, httpAuditEvent, config) {
+class ErrorHandler @Inject()(config: Configuration,
+                             auditConnector: AuditConnector,
+                             httpAuditEvent: HttpAuditEvent)(implicit ec: ExecutionContext)
+  extends JsonErrorHandler(auditConnector, httpAuditEvent, config) {
 
   import httpAuditEvent.dataEvent
 
   private val logger: Logger = Logger(this.getClass)
 
   override def onClientError(request: RequestHeader, statusCode: Int, message: String): Future[Result] = {
-
-    implicit val headerCarrier: HeaderCarrier = HeaderCarrierConverter.fromHeadersAndSession(request.headers, Some(request.session))
+    implicit val headerCarrier: HeaderCarrier = HeaderCarrierConverter.fromRequestAndSession(request, request.session)
 
     logger.warn(s"[ErrorHandler][onClientError] error in version 1, for (${request.method}) [${request.uri}] with status:" +
       s" $statusCode and message: $message")
@@ -79,7 +77,7 @@ class ErrorHandler @Inject()(
   }
 
   override def onServerError(request: RequestHeader, ex: Throwable): Future[Result] = {
-    implicit val headerCarrier: HeaderCarrier = HeaderCarrierConverter.fromHeadersAndSession(request.headers, Some(request.session))
+    implicit val headerCarrier: HeaderCarrier = HeaderCarrierConverter.fromRequestAndSession(request, request.session)
 
     logger.warn(s"[ErrorHandler][onServerError] Internal server error in version 1, for (${request.method}) [${request.uri}] -> ", ex)
 

app/v1/connectors/AmendDisclosuresConnector.scala

@@ -25,7 +25,7 @@ import scala.concurrent.{ExecutionContext, Future}
 
 @Singleton
 class AmendDisclosuresConnector @Inject()(val http: HttpClient,
-                                          val appConfig: AppConfig) extends BaseDesConnector {
+                                          val appConfig: AppConfig) extends BaseDownstreamConnector {
 
   def amendDisclosures(request: AmendDisclosuresRequest)(
     implicit hc: HeaderCarrier,

app/v1/connectors/BaseDesConnector.scala → app/v1/connectors/BaseDownstreamConnector.scala

@@ -19,43 +19,39 @@ package v1.connectors
 import config.AppConfig
 import play.api.Logger
 import play.api.libs.json.Writes
-import uk.gov.hmrc.http.logging.Authorization
-import uk.gov.hmrc.http.{HeaderCarrier, HttpReads}
-import uk.gov.hmrc.http.HttpClient
+import uk.gov.hmrc.http.{HeaderCarrier, HttpClient, HttpReads}
 
 import scala.concurrent.{ExecutionContext, Future}
 
-trait BaseDesConnector {
+trait BaseDownstreamConnector {
   val http: HttpClient
   val appConfig: AppConfig
 
-  val logger = Logger(this.getClass)
-
-  private[connectors] def desHeaderCarrier(implicit hc: HeaderCarrier): HeaderCarrier =
-    hc.copy(authorization = Some(Authorization(s"Bearer ${appConfig.desToken}")))
-      .withExtraHeaders("Environment" -> appConfig.desEnv)
-
-  def post[Body: Writes, Resp](body: Body, uri: DesUri[Resp])(implicit ec: ExecutionContext,
-                                                              hc: HeaderCarrier,
-                                                              httpReads: HttpReads[DesOutcome[Resp]],
-                                                              correlationId: String): Future[DesOutcome[Resp]] = {
-
-    def doPost(implicit hc: HeaderCarrier): Future[DesOutcome[Resp]] = {
-      http.POST(s"${appConfig.desBaseUrl}/${uri.value}", body)
-    }
-
-    doPost(desHeaderCarrier(hc.withExtraHeaders("CorrelationId" -> correlationId)))
-  }
+  val logger: Logger = Logger(this.getClass)
+
+  private def downstreamHeaderCarrier(additionalHeaders: Seq[String] = Seq.empty)(implicit hc: HeaderCarrier,
+                                                                                  correlationId: String): HeaderCarrier =
+    HeaderCarrier(
+      extraHeaders = hc.extraHeaders ++
+        // Contract headers
+        Seq(
+          "Authorization" -> s"Bearer ${appConfig.desToken}",
+          "Environment" -> appConfig.desEnv,
+          "CorrelationId" -> correlationId
+        ) ++
+        // Other headers (i.e Gov-Test-Scenario, Content-Type)
+        hc.headers(additionalHeaders ++ appConfig.desEnvironmentHeaders.getOrElse(Seq.empty))
+    )
 
   def get[Resp](uri: DesUri[Resp])(implicit ec: ExecutionContext,
                                    hc: HeaderCarrier,
                                    httpReads: HttpReads[DesOutcome[Resp]],
                                    correlationId: String): Future[DesOutcome[Resp]] = {
 
     def doGet(implicit hc: HeaderCarrier): Future[DesOutcome[Resp]] =
-      http.GET(s"${appConfig.desBaseUrl}/${uri.value}")
+      http.GET(url = s"${appConfig.desBaseUrl}/${uri.value}")
 
-    doGet(desHeaderCarrier(hc.withExtraHeaders("CorrelationId" -> correlationId)))
+    doGet(downstreamHeaderCarrier())
   }
 
   def delete[Resp](uri: DesUri[Resp])(implicit ec: ExecutionContext,
@@ -64,10 +60,10 @@ trait BaseDesConnector {
                                       correlationId: String): Future[DesOutcome[Resp]] = {
 
     def doDelete(implicit hc: HeaderCarrier): Future[DesOutcome[Resp]] = {
-      http.DELETE(s"${appConfig.desBaseUrl}/${uri.value}")
+      http.DELETE(url = s"${appConfig.desBaseUrl}/${uri.value}")
     }
 
-    doDelete(desHeaderCarrier(hc.withExtraHeaders("CorrelationId" -> correlationId)))
+    doDelete(downstreamHeaderCarrier())
   }
 
   def put[Body: Writes, Resp](body: Body, uri: DesUri[Resp])(implicit ec: ExecutionContext,
@@ -76,9 +72,9 @@ trait BaseDesConnector {
                                                              correlationId: String): Future[DesOutcome[Resp]] = {
 
     def doPut(implicit hc: HeaderCarrier): Future[DesOutcome[Resp]] = {
-      http.PUT(s"${appConfig.desBaseUrl}/${uri.value}", body)
+      http.PUT(url = s"${appConfig.desBaseUrl}/${uri.value}", body)
     }
 
-    doPut(desHeaderCarrier(hc.withExtraHeaders("CorrelationId" -> correlationId)))
+    doPut(downstreamHeaderCarrier(Seq("Content-Type")))
   }
-}
+}

app/v1/connectors/DeleteRetrieveConnector.scala

@@ -26,7 +26,7 @@ import scala.concurrent.{ExecutionContext, Future}
 
 @Singleton
 class DeleteRetrieveConnector @Inject()(val http: HttpClient,
-                                        val appConfig: AppConfig) extends BaseDesConnector {
+                                        val appConfig: AppConfig) extends BaseDownstreamConnector {
 
   def delete()(implicit hc: HeaderCarrier,
                ec: ExecutionContext,

app/v1/connectors/httpparsers/StandardDesHttpParser.scala

@@ -28,7 +28,7 @@ object StandardDesHttpParser extends HttpParser {
 
   case class SuccessCode(status: Int) extends AnyVal
 
-  val logger = Logger(getClass)
+  val logger: Logger = Logger(getClass)
 
   // Return Right[DesResponse[Unit]] as success response has no body - no need to assign it a value
   implicit def readsEmpty(implicit successCode: SuccessCode = SuccessCode(NO_CONTENT)): HttpReads[DesOutcome[Unit]] =

app/v1/controllers/requestParsers/AmendDisclosuresRequestParser.scala

@@ -17,7 +17,7 @@
 package v1.controllers.requestParsers
 
 import javax.inject.Inject
-import uk.gov.hmrc.domain.Nino
+import v1.models.domain.Nino
 import v1.controllers.requestParsers.validators.AmendDisclosuresValidator
 import v1.models.request.disclosures.{AmendDisclosuresRawData, AmendDisclosuresRequest, AmendDisclosuresRequestBody}
 

app/v1/controllers/requestParsers/DeleteRetrieveRequestParser.scala

@@ -17,7 +17,7 @@
 package v1.controllers.requestParsers
 
 import javax.inject.Inject
-import uk.gov.hmrc.domain.Nino
+import v1.models.domain.Nino
 import v1.controllers.requestParsers.validators.DeleteRetrieveValidator
 import v1.models.request.{DeleteRetrieveRawData, DeleteRetrieveRequest}
 

app/v1/controllers/requestParsers/validators/validations/NinoValidation.scala

@@ -16,7 +16,7 @@
 
 package v1.controllers.requestParsers.validators.validations
 
-import uk.gov.hmrc.domain.Nino
+import v1.models.domain.Nino
 import v1.models.errors.{MtdError, NinoFormatError}
 
 object NinoValidation {

app/v1/models/domain/Nino.scala

@@ -0,0 +1,41 @@
+/*
+ * Copyright 2021 HM Revenue & Customs
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package v1.models.domain
+
+case class Nino(nino: String) {
+  require(Nino.isValid(nino), s"$nino is not a valid nino.")
+
+  private val LengthWithoutSuffix: Int = 8
+  def value: String = nino
+  val name = "nino"
+  def formatted: String = value.grouped(2).mkString(" ")
+  def withoutSuffix: String = value.take(LengthWithoutSuffix)
+}
+
+object Nino extends (String => Nino) {
+  private val validNinoFormat = "[[A-Z]&&[^DFIQUV]][[A-Z]&&[^DFIQUVO]] ?\\d{2} ?\\d{2} ?\\d{2} ?[A-D]{1}"
+  private val invalidPrefixes = List("BG", "GB", "NK", "KN", "TN", "NT", "ZZ")
+
+  private def hasValidPrefix(nino: String) = !invalidPrefixes.exists(nino.startsWith)
+  def isValid(nino: String): Boolean = nino != null && hasValidPrefix(nino) && nino.matches(validNinoFormat)
+
+  private[domain] val validFirstCharacters = ('A' to 'Z').filterNot(List('D', 'F', 'I', 'Q', 'U', 'V').contains).map(_.toString)
+  private[domain] val validSecondCharacters = ('A' to 'Z').filterNot(List('D', 'F', 'I', 'O', 'Q', 'U', 'V').contains).map(_.toString)
+  val validPrefixes: Seq[String] = validFirstCharacters.flatMap(a => validSecondCharacters.map(a + _)).filterNot(invalidPrefixes.contains(_))
+  val validSuffixes: Seq[String] = ('A' to 'D').map(_.toString)
+}
+