Merge pull request #88 from hmrc/CIR-1651

CIR 1651

app/config/InvalidJsonErrorHandler.scala

@@ -0,0 +1,112 @@
+/*
+ * Copyright 2024 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 config
+
+import play.api.Configuration
+import play.api.http.Status.{BAD_REQUEST, NOT_FOUND}
+import play.api.libs.json.Json
+import play.api.libs.json.Json.toJson
+import play.api.mvc.Results.{BadRequest, NotFound, Status}
+import play.api.mvc.{RequestHeader, Result}
+import uk.gov.hmrc.http.HeaderCarrier
+import uk.gov.hmrc.play.audit.http.connector.AuditConnector
+import uk.gov.hmrc.play.bootstrap.backend.http.ErrorResponse
+import uk.gov.hmrc.play.bootstrap.config.HttpAuditEvent
+
+import javax.inject.Inject
+import scala.concurrent.{ExecutionContext, Future}
+
+class InvalidJsonErrorHandler @Inject()(
+                                         auditConnector: AuditConnector, httpAuditEvent: HttpAuditEvent, configuration: Configuration
+                                       )(implicit ec: ExecutionContext)
+  extends uk.gov.hmrc.play.bootstrap.backend.http.JsonErrorHandler(auditConnector, httpAuditEvent, configuration) {
+  import httpAuditEvent._
+
+  override def onClientError(request: RequestHeader, statusCode: Int, message: String): Future[Result] = {
+    implicit val headerCarrier: HeaderCarrier = hc(request)
+    val result = statusCode match {
+      case NOT_FOUND =>
+        auditConnector.sendEvent(
+          dataEvent(
+            eventType       = "ResourceNotFound",
+            transactionName = "Resource Endpoint Not Found",
+            request         = request,
+            detail          = Map.empty
+          )
+        )
+        NotFound(toJson(ErrorResponse(NOT_FOUND, "URI not found", requested = Some(request.path))))
+
+      case BAD_REQUEST =>
+        auditConnector.sendEvent(
+          dataEvent(
+            eventType       = "ServerValidationError",
+            transactionName = "Request bad format exception",
+            request         = request,
+            detail          = Map.empty
+          )
+        )
+        def constructErrorMessage(input: String): String = {
+          val unrecognisedTokenJsonError = "^Invalid Json: Unrecognized token '(.*)':.*".r
+          val invalidJson                = "^(?s)Invalid Json:.*".r
+          val jsonValidationError        = "^Json validation error.*".r
+          val booleanParsingError        = "^Cannot parse parameter .* as Boolean: should be true, false, 0 or 1$".r
+          val missingParameterError      = "^Missing parameter:.*".r
+          val characterParseError        = "^Cannot parse parameter .* with value '(.*)' as Char: .* must be exactly one digit in length.$".r
+          val parameterParseError        = "^Cannot parse parameter .* as .*: For input string: \"(.*)\"$".r
+          input match {
+            case unrecognisedTokenJsonError(toBeRedacted) => input.replace(toBeRedacted, "REDACTED")
+            case invalidJson()
+                 | jsonValidationError()
+                 | booleanParsingError()
+                 | missingParameterError()                  => filterJsonExceptionMsg(input)
+            case characterParseError(toBeRedacted)        => input.replace(toBeRedacted, "REDACTED")
+            case parameterParseError(toBeRedacted)        => input.replace(toBeRedacted, "REDACTED")
+            case _                                        => "bad request, cause: REDACTED"
+          }
+        }
+        val msg =
+          if (suppress4xxErrorMessages) "Bad request"
+          else constructErrorMessage(message)
+
+        BadRequest(toJson(ErrorResponse(BAD_REQUEST, msg)))
+
+      case _ =>
+        auditConnector.sendEvent(
+          dataEvent(
+            eventType       = "ClientError",
+            transactionName = s"A client error occurred, status: $statusCode",
+            request         = request,
+            detail          = Map.empty
+          )
+        )
+
+        val msg =
+          if (suppress4xxErrorMessages) "Other error"
+          else message
+
+        Status(statusCode)(toJson(ErrorResponse(statusCode, msg)))
+    }
+    Future.successful(result)
+  }
+
+  private def filterJsonExceptionMsg(msg: String): String = {
+    msg.indexOf("at [Source") match {
+      case -1 => msg
+      case x => msg.substring(0, x)
+    }
+  }
+}

app/connectors/DownstreamConnector.scala

@@ -38,10 +38,10 @@ class DownstreamConnector @Inject()(httpClient: HttpClient) extends Logging {
     logger.info(s"Forwarding to downstream url: $url")
 
     // TODO: Check if the context path is present - if so remove before forwarding
-    val called = (request.method, request.headers.get(CONTENT_TYPE).map(_.toLowerCase())) match {
-      case ("POST", Some("application/json")) =>
+    val called = request.method match {
+      case "POST" =>
         httpClient.POST[Option[JsValue], HttpResponse](url = url, body = Some(request.body), onwardHeaders)
-      case ("GET", _)                         =>
+      case "GET"  =>
         httpClient.GET[HttpResponse](url = url, onwardHeaders)
     }
 

app/controllers/AddressSearchController.scala

@@ -21,12 +21,11 @@ import config.AppConfig
 import connectors.DownstreamConnector
 import model._
 import model.address.{AddressRecord, NonUKAddress, Postcode}
-import model.request.{LookupByCountryRequest, LookupByPostTownRequest, LookupByPostcodeRequest, LookupByUprnRequest}
-import model.response.ErrorResponse
+import model.request._
 import org.apache.pekko.actor.ActorSystem
 import org.apache.pekko.stream.Materializer
 import play.api.Logging
-import play.api.http.HeaderNames
+import play.api.http.{HeaderNames, MimeTypes}
 import play.api.libs.json._
 import play.api.mvc._
 import play.api.mvc.request.RequestTarget
@@ -37,109 +36,102 @@ import uk.gov.hmrc.play.http.HeaderCarrierConverter
 
 import javax.inject.Inject
 import scala.concurrent.{ExecutionContext, Future}
-import scala.util.{Failure, Success, Try}
 
 class AddressSearchController @Inject()(connector: DownstreamConnector, auditConnector: AuditConnector, cc: ControllerComponents, val configHelper: AppConfig)(implicit ec: ExecutionContext)
   extends BackendController(cc) with Logging with AccessChecker {
   private val actorSystem = ActorSystem("AddressSearchController")
   private implicit val materializer: Materializer = Materializer.createMaterializer(actorSystem)
 
-  import ErrorResponse.Implicits._
-
-  private def withValidJson[T: Reads](request: Request[String], doSearch: (Request[String], T) => Future[Result]): Future[Result] = {
-    Try(Json.parse(request.body)) match {
-      case Success(json) =>
-        json.validate[T] match {
-          case JsSuccess(requestDetails, _) =>
-            doSearch(request, requestDetails)
-          case JsError(errors)              =>
-            Future.successful(BadRequest(JsError.toJson(errors)))
-        }
-      case Failure(_)    => Future.successful(BadRequest(Json.toJson(ErrorResponse.invalidJson)))
-    }
-  }
-
-  def searchByPostcode(): Action[String] = accessCheckedAction(parse.tolerantText) {
-    request: Request[String] =>
-      withValidJson[LookupByPostcodeRequest](request, searchByPostcode)
+  def searchByPostcode(): Action[LookupByPostcodeRequest] = accessCheckedAction(parse.json[LookupByPostcodeRequest]) {
+    implicit request: Request[LookupByPostcodeRequest] =>
+      searchByPostcode(request)
   }
 
-  def searchByUprn(): Action[String] = accessCheckedAction(parse.tolerantText) {
-    request: Request[String] =>
-      withValidJson[LookupByUprnRequest](request, searchByUprn)
+  def searchByUprn(): Action[LookupByUprnRequest] = accessCheckedAction(parse.json[LookupByUprnRequest]) {
+    implicit request: Request[LookupByUprnRequest] =>
+      searchByUprn(request)
   }
 
-  def searchByPostTown(): Action[String] = accessCheckedAction(parse.tolerantText) {
-    request: Request[String] =>
-      withValidJson[LookupByPostTownRequest](request, searchByTown)
+  def searchByPostTown(): Action[LookupByPostTownRequest] = accessCheckedAction(parse.json[LookupByPostTownRequest]) {
+    implicit request: Request[LookupByPostTownRequest] =>
+      searchByTown(request)
   }
 
   def supportedCountries(): Action[AnyContent] = Action.async {
     request: Request[AnyContent] =>
-      forwardIfAllowed[JsValue](request.map(r => JsObject.empty), _ => ())
+      forwardIfAllowed[JsValue, JsValue](request.map(r => JsObject.empty), _ => ())
   }
 
-  def searchByCountry(countryCode: String): Action[String] = accessCheckedAction(parse.tolerantText) {
-    request =>
+  def searchByCountry(countryCode: String): Action[LookupByCountryFilterRequest] = accessCheckedAction(parse.json[LookupByCountryFilterRequest]) {
+    implicit request: Request[LookupByCountryFilterRequest] =>
       implicit val hc: HeaderCarrier = HeaderCarrierConverter.fromRequest(request)
-      val newRequest =
+      val newRequest: Request[LookupByCountryRequest] =
         request.withTarget(RequestTarget("/country/lookup", "/country/lookup", request.queryString))
           .withBody(addCountryTo(request.body, countryCode.toLowerCase))
 
 
-      withValidJson[LookupByCountryRequest](newRequest, searchByCountry)
+      searchByCountry(newRequest)
   }
 
-  private[controllers] def searchByUprn(request: Request[String], uprn: LookupByUprnRequest): Future[Result] = {
+  private[controllers] def searchByUprn(request: Request[LookupByUprnRequest]): Future[Result] = {
     implicit val hc: HeaderCarrier = HeaderCarrierConverter.fromRequest(request)
     val userAgent = request.headers.get(HeaderNames.USER_AGENT)
 
+    val uprn: LookupByUprnRequest = request.body
+
     import model.address.AddressRecord.formats._
 
-    forwardIfAllowed[List[AddressRecord]](request.map(rb => Json.parse(rb)),
+    forwardIfAllowed[LookupByUprnRequest, List[AddressRecord]](request,
       addresses => auditAddressSearch(userAgent, addresses, uprn = Some(uprn.uprn))
     )
   }
 
-  private[controllers] def searchByPostcode[A](request: Request[String], postcode: LookupByPostcodeRequest): Future[Result] = {
+  private[controllers] def searchByPostcode[A](request: Request[LookupByPostcodeRequest]): Future[Result] = {
     implicit val hc: HeaderCarrier = HeaderCarrierConverter.fromRequest(request)
     val userAgent = request.headers.get(HeaderNames.USER_AGENT)
 
+    val postcode: LookupByPostcodeRequest = request.body
+
     import model.address.AddressRecord.formats._
 
-    forwardIfAllowed[List[AddressRecord]](request.map(rb => Json.parse(rb)),
+    forwardIfAllowed[LookupByPostcodeRequest, List[AddressRecord]](request,
       addresses => auditAddressSearch(userAgent, addresses, postcode = Some(postcode.postcode), filter = postcode.filter))
   }
 
-  private[controllers] def searchByTown[A](request: Request[String], posttown: LookupByPostTownRequest): Future[Result] = {
+  private[controllers] def searchByTown[A](request: Request[LookupByPostTownRequest]): Future[Result] = {
     implicit val hc: HeaderCarrier = HeaderCarrierConverter.fromRequest(request)
     val userAgent = request.headers.get(HeaderNames.USER_AGENT)
 
+    val posttown: LookupByPostTownRequest = request.body
+
     import model.address.AddressRecord.formats._
 
-    forwardIfAllowed[List[AddressRecord]](request.map(rb => Json.parse(rb)),
+    forwardIfAllowed[LookupByPostTownRequest, List[AddressRecord]](request,
       addresses => auditAddressSearch(userAgent, addresses, posttown = Some(posttown.posttown.toUpperCase), filter = posttown.filter))
   }
 
-  private[controllers] def searchByCountry[A](request: Request[String], country: LookupByCountryRequest)(implicit hc: HeaderCarrier): Future[Result] = {
+  private[controllers] def searchByCountry[A](request: Request[LookupByCountryRequest])(implicit hc: HeaderCarrier): Future[Result] = {
     implicit val hc: HeaderCarrier = HeaderCarrierConverter.fromRequest(request)
     val userAgent = request.headers.get(HeaderNames.USER_AGENT)
 
+    val country: LookupByCountryRequest = request.body
+
     import model.address.NonUKAddress._
 
-    forwardIfAllowed[List[NonUKAddress]](request.map(rb => Json.parse(rb)),
+    forwardIfAllowed[LookupByCountryRequest, List[NonUKAddress]](request,
       addresses => auditNonUKAddressSearch(userAgent, country = country.country, filter = Option(country.filter), nonUKAddresses = addresses))
   }
 
-  private def addCountryTo(body: String, country: String): String = {
-    val newBody = Json.parse(body).as[JsObject] + (("country", JsString(country)))
-    newBody.toString()
+  private def addCountryTo(body: LookupByCountryFilterRequest, country: String): LookupByCountryRequest = {
+    LookupByCountryRequest(country, body.filter)
   }
 
   private def url(path: String) = s"${configHelper.addressSearchApiBaseUrl}$path"
 
-  private def forwardIfAllowed[Resp:Reads](request: Request[JsValue], auditFn: Resp => Unit): Future[Result] = {
-    connector.forward(request, url(request.target.uri.toString), configHelper.addressSearchApiAuthToken)
+  private def forwardIfAllowed[Req: Writes, Resp:Reads](request: Request[Req], auditFn: Resp => Unit): Future[Result] = {
+    val newHeadersMap = request.headers.toSimpleMap ++ Map(HeaderNames.CONTENT_TYPE -> MimeTypes.JSON)
+    val jsonRequest = request.withHeaders(Headers(newHeadersMap.toSeq: _*))
+    connector.forward(jsonRequest.map((r: Req) => Json.toJson(r)), url(jsonRequest.target.uri.toString), configHelper.addressSearchApiAuthToken)
       .flatMap(res => res.body.consumeData.map(d => res.header.status -> d))
       .map { case (s, bs) => s -> bs.utf8String }
       .map { case (s, res) => s -> Json.parse(res) }

app/model/request.scala

@@ -74,6 +74,13 @@ object request {
     implicit val writes: Writes[LookupByPostTownRequest] = Json.writes[LookupByPostTownRequest]
   }
 
+  case class LookupByCountryFilterRequest(filter: String)
+
+  object LookupByCountryFilterRequest {
+    implicit val reads: Reads[LookupByCountryFilterRequest] = Json.reads[LookupByCountryFilterRequest]
+    implicit val writes: Writes[LookupByCountryFilterRequest] = Json.writes[LookupByCountryFilterRequest]
+  }
+
   case class LookupByCountryRequest(country: String, filter: String)
 
   object LookupByCountryRequest {

app/util/package.scala

@@ -1,5 +1,5 @@
 /*
- * Copyright 2023 HM Revenue & Customs
+ * Copyright 2024 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.

conf/application.conf

@@ -22,7 +22,7 @@ header.x-origin = "X-LOCALHOST-Origin"
 play.modules.enabled += "uk.gov.hmrc.play.bootstrap.HttpClientModule"
 
 # Json error handler
-play.http.errorHandler = "uk.gov.hmrc.play.bootstrap.backend.http.JsonErrorHandler"
+play.http.errorHandler = "config.InvalidJsonErrorHandler"
 
 # Address Lookup module
 play.modules.enabled += "Module"

it/test/suites/CountryLookupSuiteV2.scala

@@ -20,15 +20,17 @@ import com.codahale.metrics.SharedMetricRegistries
 import it.helper.AppServerTestApi
 import model.address.NonUKAddress
 import model.{NonUKAddressSearchAuditEvent, NonUKAddressSearchAuditEventMatchedAddress, NonUKAddressSearchAuditEventRequestDetails}
+import org.apache.pekko.util.ByteString
 import org.mockito.ArgumentMatchers.{any, eq => meq}
 import org.mockito.Mockito
 import org.scalatest.wordspec.AnyWordSpec
 import org.scalatestplus.mockito.MockitoSugar.mock
 import org.scalatestplus.play.guice.GuiceOneServerPerSuite
 import play.api.Application
+import play.api.http.{HeaderNames, MimeTypes}
 import play.api.inject.guice.GuiceApplicationBuilder
 import play.api.libs.json.{JsValue, Json}
-import play.api.libs.ws.WSClient
+import play.api.libs.ws.{InMemoryBody, WSClient}
 import play.api.test.Helpers.{await, defaultAwaitTimeout}
 import play.inject.Bindings
 import uk.gov.hmrc.play.audit.http.connector.AuditConnector
@@ -133,7 +135,13 @@ class CountryLookupSuiteV2()
 
       "return forbidden when the user-agent is absent" in {
         val path = "/country/GB/lookup"
-        val response = await(wsClient.url(appEndpoint + path).withMethod("POST").withBody("""{"filter":"FX1 4AB"}""").execute())
+        val response = await(
+          wsClient
+            .url(appEndpoint + path)
+            .withMethod("POST")
+            .withHttpHeaders(HeaderNames.CONTENT_TYPE -> MimeTypes.JSON)
+            .withBody("""{"filter":"FX1 4AB"}""")
+            .execute())
         response.status shouldBe FORBIDDEN
       }
 
@@ -153,6 +161,22 @@ class CountryLookupSuiteV2()
         val response = request("GET", "/country/GB/lookup?postcode=FX1+9PY", headerOrigin -> "xxx")
         response.status shouldBe METHOD_NOT_ALLOWED
       }
+
+      "give an unsupported media type response for bermuda with text content-type" in {
+        // Have to define this as the withHttpHeaders doesn't seem to be replacing existing headers.
+        def pst(path: String, body: String): play.api.libs.ws.WSResponse = {
+          val wsBody = InMemoryBody(ByteString(body.trim))
+          val req = newRequest("POST", path)
+            .withHttpHeaders("Content-Type" -> "plain/text")
+            .withBody(wsBody)
+
+          await(req.execute("POST"))
+        }
+
+        val response = pst(path = "/country/BM/lookup", body = """{"filter":"HM02"}""")
+
+        response.status shouldBe UNSUPPORTED_MEDIA_TYPE
+      }
     }
   }
 }