[WG][APB-8880] Agent client check link - refactor and improvements, i…

…nternalAuth (#295)

* [WG][APB-8681][APB-8880] Create agent link, refactor AgentDetails from AA

* [WG][APB-8681][APB-8880] Create agent link, refactor AgentDetails from AA, add test

* [WG][APB-8681][APB-8880] Create agent link, refactor AgentDetails from AA, remove UTR, AA link

* [WG][APB-8681][APB-8880] Create agent link, log parsing errors and return 404not 502

* [WG][APB-8924][APB-8880] use new AA endpoint with internal auth for client to get agent details with checks

* [WG][APB-8924][APB-8880] refactor getAgentDetails

---------

Co-authored-by: wojciech.gradzki <gitwojciech@users.noreply.github.com>

app/uk/gov/hmrc/agentclientrelationships/config/AppConfig.scala

@@ -74,6 +74,8 @@ class AppConfig @Inject() (config: Configuration, servicesConfig: ServicesConfig
 
   val authUrl = servicesConfig.baseUrl("auth")
 
+  val agentAssuranceInternalAuthToken: String = servicesConfig.getString("agent-assurance-internal-auth.token")
+
   val citizenDetailsBaseUrl: String = servicesConfig.baseUrl("citizen-details")
 
   val agentClientAuthorisationUrl = servicesConfig.baseUrl("agent-client-authorisation")

app/uk/gov/hmrc/agentclientrelationships/connectors/AgentAssuranceConnector.scala

@@ -21,6 +21,7 @@ import play.api.libs.json.Json
 import uk.gov.hmrc.agentclientrelationships.config.AppConfig
 import uk.gov.hmrc.agentclientrelationships.model.invitationLink.AgentDetailsDesResponse
 import uk.gov.hmrc.agentclientrelationships.util.HttpAPIMonitor
+import uk.gov.hmrc.agentmtdidentifiers.model.Arn
 import uk.gov.hmrc.http._
 import uk.gov.hmrc.http.client.HttpClientV2
 import uk.gov.hmrc.play.bootstrap.metrics.Metrics
@@ -40,9 +41,15 @@ class AgentAssuranceConnector @Inject() (httpV2: HttpClientV2)(implicit
 
   import uk.gov.hmrc.http.HttpReads.Implicits._
 
-  def getAgentRecordWithChecks(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[AgentDetailsDesResponse] =
+  private def aaHeaders: (String, String) = HeaderNames.authorisation -> appConfig.agentAssuranceInternalAuthToken
+
+  def getAgentRecordWithChecks(arn: Arn)(implicit
+    hc: HeaderCarrier,
+    ec: ExecutionContext
+  ): Future[AgentDetailsDesResponse] =
     httpV2
-      .get(new URL(s"$baseUrl/agent-assurance/agent-record-with-checks"))
+      .get(new URL(s"$baseUrl/agent-assurance/agent-record-with-checks/arn/${arn.value}"))
+      .setHeader(aaHeaders)
       .execute[HttpResponse]
       .map(response =>
         response.status match {

app/uk/gov/hmrc/agentclientrelationships/controllers/InvitationLinkController.scala

@@ -20,7 +20,7 @@ import play.api.Logger
 import play.api.libs.json.Json
 import play.api.mvc.{Action, AnyContent, ControllerComponents}
 import uk.gov.hmrc.agentclientrelationships.model.invitationLink.ValidateLinkFailureResponse
-import uk.gov.hmrc.agentclientrelationships.services.AgentReferenceService
+import uk.gov.hmrc.agentclientrelationships.services.InvitationLinkService
 import uk.gov.hmrc.auth.core.AuthConnector
 import uk.gov.hmrc.play.bootstrap.backend.controller.BackendController
 
@@ -29,7 +29,7 @@ import scala.concurrent.ExecutionContext
 
 @Singleton
 class InvitationLinkController @Inject() (
-  agentReferenceService: AgentReferenceService,
+  agentReferenceService: InvitationLinkService,
   val authConnector: AuthConnector,
   cc: ControllerComponents
 )(implicit ec: ExecutionContext)
@@ -46,9 +46,6 @@ class InvitationLinkController @Inject() (
           case ValidateLinkFailureResponse.AgentSuspended =>
             Logger(getClass).warn(s"Agent is suspended for: $uid")
             Forbidden
-          case ValidateLinkFailureResponse.AgentNameMissing =>
-            Logger(getClass).warn(s"Agent name is missing for: $uid")
-            NotFound
         },
         validLinkResponse => Ok(Json.toJson(validLinkResponse))
       )

app/uk/gov/hmrc/agentclientrelationships/model/invitationLink/AgencyDetails.scala

@@ -16,28 +16,29 @@
 
 package uk.gov.hmrc.agentclientrelationships.model.invitationLink
 
-import play.api.libs.json.{Json, OFormat}
-
-case class BusinessAddress(
-  addressLine1: String,
-  addressLine2: Option[String],
-  addressLine3: Option[String] = None,
-  addressLine4: Option[String] = None,
-  postalCode: Option[String],
-  countryCode: String
-)
-
-object BusinessAddress {
-  implicit val format: OFormat[BusinessAddress] = Json.format
-}
+import play.api.libs.functional.syntax._
+import play.api.libs.json._
 
 case class AgencyDetails(
-  agencyName: Option[String],
-  agencyEmail: Option[String],
-  agencyTelephone: Option[String],
-  agencyAddress: Option[BusinessAddress]
+  agencyName: String,
+  agencyEmail: String
 )
 
 object AgencyDetails {
-  implicit val format: OFormat[AgencyDetails] = Json.format
+
+  private def optionalReads(fieldName: String): Reads[String] = Reads[String] {
+    case JsString(value) => JsSuccess(value)
+    case JsNull          => JsError(s"$fieldName must not be null")
+    case _               => JsError(s"Invalid $fieldName value")
+  }
+
+  private val reads: Reads[AgencyDetails] = (
+    (__ \ "agencyName").read(optionalReads("Agency name")).orElse(Reads.pure("")) and
+      (__ \ "agencyEmail").read(optionalReads("Agency email")).orElse(Reads.pure(""))
+  )(AgencyDetails.apply _)
+
+  private val writes: Writes[AgencyDetails] = Json.writes[AgencyDetails]
+
+  implicit val agencyDetailsFormat: Format[AgencyDetails] = Format(reads, writes)
+
 }

app/uk/gov/hmrc/agentclientrelationships/model/invitationLink/AgentDetailsDesResponse.scala

@@ -16,16 +16,14 @@
 
 package uk.gov.hmrc.agentclientrelationships.model.invitationLink
 
-import play.api.libs.json.{Format, Json}
-import uk.gov.hmrc.agentmtdidentifiers.model.{SuspensionDetails, Utr}
+import play.api.libs.json._
+import uk.gov.hmrc.agentmtdidentifiers.model.SuspensionDetails
 
 case class AgentDetailsDesResponse(
-  uniqueTaxReference: Option[Utr],
-  agencyDetails: Option[AgencyDetails],
+  agencyDetails: AgencyDetails,
   suspensionDetails: Option[SuspensionDetails]
 )
 
 object AgentDetailsDesResponse {
-
-  implicit val format: Format[AgentDetailsDesResponse] = Json.format[AgentDetailsDesResponse]
+  implicit val format: OFormat[AgentDetailsDesResponse] = Json.format[AgentDetailsDesResponse]
 }

app/uk/gov/hmrc/agentclientrelationships/model/invitationLink/ValidateLinkFailureResponse.scala

@@ -24,6 +24,4 @@ object ValidateLinkFailureResponse {
   case object NormalizedAgentNameNotMatched extends ValidateLinkFailureResponse
 
   case object AgentSuspended extends ValidateLinkFailureResponse
-
-  case object AgentNameMissing extends ValidateLinkFailureResponse
 }

app/uk/gov/hmrc/agentclientrelationships/services/AgentReferenceService.scala → app/uk/gov/hmrc/agentclientrelationships/services/InvitationLinkService.scala

@@ -20,14 +20,15 @@ import play.api.Logging
 import uk.gov.hmrc.agentclientrelationships.connectors.AgentAssuranceConnector
 import uk.gov.hmrc.agentclientrelationships.model.invitationLink.{AgentDetailsDesResponse, AgentReferenceRecord, ValidateLinkFailureResponse, ValidateLinkResponse}
 import uk.gov.hmrc.agentclientrelationships.repository.AgentReferenceRepository
+import uk.gov.hmrc.agentmtdidentifiers.model.Arn
 import uk.gov.hmrc.http.HeaderCarrier
 
 import javax.inject.{Inject, Singleton}
 import scala.collection.Seq
 import scala.concurrent.{ExecutionContext, Future}
 
 @Singleton
-class AgentReferenceService @Inject() (
+class InvitationLinkService @Inject() (
   agentReferenceRepository: AgentReferenceRepository,
   agentAssuranceConnector: AgentAssuranceConnector
 )(implicit ec: ExecutionContext)
@@ -43,7 +44,7 @@ class AgentReferenceService @Inject() (
       _ <- EitherT.fromEither[Future](
              validateNormalizedAgentName(agentReferenceRecord.normalisedAgentNames, normalizedAgentName)
            )
-      agentDetailsResponse <- EitherT.right(getAgentDetails)
+      agentDetailsResponse <- EitherT.right(getAgentDetails(agentReferenceRecord.arn))
       _                    <- EitherT.fromEither[Future](checkSuspensionDetails(agentDetailsResponse))
       agencyName           <- EitherT(getAgencyName(agentDetailsResponse))
     } yield ValidateLinkResponse(agentReferenceRecord.arn, agencyName)
@@ -64,8 +65,10 @@ class AgentReferenceService @Inject() (
     if (normalisedAgentNames.contains(normalizedAgentName)) Right(true)
     else Left(ValidateLinkFailureResponse.NormalizedAgentNameNotMatched)
 
-  private def getAgentDetails(implicit hc: HeaderCarrier): Future[AgentDetailsDesResponse] =
-    agentAssuranceConnector.getAgentRecordWithChecks
+  private def getAgentDetails(arn: Arn)(implicit
+    hc: HeaderCarrier
+  ): Future[AgentDetailsDesResponse] =
+    agentAssuranceConnector.getAgentRecordWithChecks(arn)
 
   private def checkSuspensionDetails(
     agentDetailsDesResponse: AgentDetailsDesResponse
@@ -78,9 +81,7 @@ class AgentReferenceService @Inject() (
     agentDetailsDesResponse: AgentDetailsDesResponse
   ): Future[Either[ValidateLinkFailureResponse, String]] =
     Future.successful(
-      agentDetailsDesResponse.agencyDetails
-        .flatMap(_.agencyName)
-        .toRight(ValidateLinkFailureResponse.AgentNameMissing)
+      Right(agentDetailsDesResponse.agencyDetails.agencyName)
     )
 
 }

conf/app.routes

@@ -13,7 +13,8 @@ GET           /client/relationships/service/:service                         @uk
 
 GET           /client/:service/details/:clientId                             @uk.gov.hmrc.agentclientrelationships.controllers.ClientDetailsController.findClientDetails(service: String, clientId: String)
 
-GET           /agent-reference/uid/:uid/:normalizedAgentName                 @uk.gov.hmrc.agentclientrelationships.controllers.InvitationLinkController.validateLink(uid: String, normalizedAgentName: String)
+GET           /agent/agent-reference/uid/:uid/:normalizedAgentName          @uk.gov.hmrc.agentclientrelationships.controllers.InvitationLinkController.validateLink(uid: String, normalizedAgentName: String)
+
 
 # stride endpoints
 GET           /relationships/service/:service/client/:clientIdType/:clientId @uk.gov.hmrc.agentclientrelationships.controllers.RelationshipsController.getRelationships(service: String, clientIdType: String, clientId: String)

conf/application.conf

@@ -213,4 +213,6 @@ agent.trackPage.cache.enabled = false
 
 alt-itsa.enabled = true
 
+agent-assurance-internal-auth.token = "YWdlbnQtYXNzdXJhbmNl" ##base64: agent_assurance
+
 internalServiceHostPatterns = ["^.*\\.service$","^.*\\.mdtp$","^localhost$"]

it/test/uk/gov/hmrc/agentclientrelationships/connectors/AgentAssuranceConnectorISpec.scala

@@ -22,6 +22,7 @@ import play.api.inject.guice.GuiceApplicationBuilder
 import play.api.test.Helpers.{await, defaultAwaitTimeout}
 import uk.gov.hmrc.agentclientrelationships.stubs.AgentAssuranceStubs
 import uk.gov.hmrc.agentclientrelationships.support.{TestData, UnitSpec, WireMockSupport}
+import uk.gov.hmrc.agentmtdidentifiers.model.Arn
 import uk.gov.hmrc.http.{HeaderCarrier, UpstreamErrorResponse}
 
 import scala.concurrent.ExecutionContext.Implicits.global
@@ -40,7 +41,8 @@ class AgentAssuranceConnectorISpec
       .configure(
         "microservice.services.agent-assurance.port" -> wireMockPort,
         "auditing.consumer.baseUri.host"             -> wireMockHost,
-        "auditing.consumer.baseUri.port"             -> wireMockPort
+        "auditing.consumer.baseUri.port"             -> wireMockPort,
+        "agent-assurance-internal-auth.token"        -> "internalAuthToken"
       )
 
   implicit val hc: HeaderCarrier = HeaderCarrier()
@@ -49,16 +51,17 @@ class AgentAssuranceConnectorISpec
 
   "getAgentRecord" should {
     "return the agent record for a given agent" in {
+      val agentARN: Arn = Arn("ABCDE123456")
+      givenAgentRecordFound(agentARN, agentRecordResponse)
 
-      givenAgentRecordFound(agentRecord)
-
-      await(connector.getAgentRecordWithChecks) shouldBe agentRecord
+      await(connector.getAgentRecordWithChecks(agentARN)) shouldBe agentRecord
     }
 
     "throw exception when 502 response" in {
-      givenAgentDetailsErrorResponse(502)
+      val agentARN: Arn = Arn("ABCDE123456")
+      givenAgentDetailsErrorResponse(agentARN, 502)
       intercept[UpstreamErrorResponse] {
-        await(connector.getAgentRecordWithChecks)
+        await(connector.getAgentRecordWithChecks(agentARN))
       }
     }
   }

it/test/uk/gov/hmrc/agentclientrelationships/controllers/ClientDetailsControllerISpec.scala

@@ -21,7 +21,7 @@ import play.api.libs.json.Json
 import play.api.test.FakeRequest
 import play.api.test.Helpers.{await, defaultAwaitTimeout, stubControllerComponents}
 import uk.gov.hmrc.agentclientrelationships.config.AppConfig
-import uk.gov.hmrc.agentclientrelationships.model.{Accepted, EnrolmentKey, PartialAuth}
+import uk.gov.hmrc.agentclientrelationships.model.{EnrolmentKey, PartialAuth}
 import uk.gov.hmrc.agentclientrelationships.repository.{InvitationsEventStoreRepository, InvitationsRepository}
 import uk.gov.hmrc.agentclientrelationships.services.ClientDetailsService
 import uk.gov.hmrc.agentclientrelationships.stubs.ClientDetailsStub

it/test/uk/gov/hmrc/agentclientrelationships/controllers/InvitationLinkControllerISpec.scala

@@ -21,7 +21,7 @@ import play.api.test.Helpers.{await, defaultAwaitTimeout, stubControllerComponen
 import uk.gov.hmrc.agentclientrelationships.config.AppConfig
 import uk.gov.hmrc.agentclientrelationships.model.invitationLink.AgentReferenceRecord
 import uk.gov.hmrc.agentclientrelationships.repository.MongoAgentReferenceRepository
-import uk.gov.hmrc.agentclientrelationships.services.AgentReferenceService
+import uk.gov.hmrc.agentclientrelationships.services.InvitationLinkService
 import uk.gov.hmrc.agentclientrelationships.support.TestData
 import uk.gov.hmrc.auth.core.AuthConnector
 
@@ -38,7 +38,7 @@ class InvitationLinkControllerISpec extends RelationshipsBaseControllerISpec wit
     normalisedAgentNames = Seq(normalizedAgentName, "NormalisedAgentName2")
   )
 
-  val agentReferenceService: AgentReferenceService = app.injector.instanceOf[AgentReferenceService]
+  val agentReferenceService: InvitationLinkService = app.injector.instanceOf[InvitationLinkService]
   val authConnector: AuthConnector = app.injector.instanceOf[AuthConnector]
   implicit val appConfig: AppConfig = app.injector.instanceOf[AppConfig]
   implicit val ec: ExecutionContext = app.injector.instanceOf[ExecutionContext]
@@ -51,49 +51,60 @@ class InvitationLinkControllerISpec extends RelationshipsBaseControllerISpec wit
 
     "return 200 status and valid JSON when agent reference and details are found and agent is not suspended " in {
       givenAuditConnector()
-      givenAgentRecordFound(agentRecord)
+
+      givenAgentRecordFound(arn, agentRecordResponse)
       await(agentReferenceRepo.create(agentReferenceRecord))
 
-      val result = doAgentGetRequest(s"/agent-client-relationships/agent-reference/uid/$uid/$normalizedAgentName")
+      val result =
+        doAgentGetRequest(s"/agent-client-relationships/agent/agent-reference/uid/$uid/$normalizedAgentName")
       result.status shouldBe 200
       result.json shouldBe Json.obj(
         "arn"  -> arn.value,
-        "name" -> agentRecord.agencyDetails.flatMap(_.agencyName)
+        "name" -> agentRecord.agencyDetails.agencyName
       )
     }
 
     "return 404 status when agent reference is not found" in {
       givenAuditConnector()
-      givenAgentRecordFound(agentRecord)
+      givenAgentRecordFound(arn, agentRecordResponse)
 
-      val result = doAgentGetRequest(s"/agent-client-relationships/agent-reference/uid/$uid/$normalizedAgentName")
+      val result =
+        doAgentGetRequest(s"/agent-client-relationships/agent/agent-reference/uid/$uid/$normalizedAgentName")
       result.status shouldBe 404
     }
 
     "return 404 status when normalisedAgentNames is not on agent reference list" in {
       givenAuditConnector()
-      givenAgentRecordFound(agentRecord)
+      givenAgentRecordFound(arn, agentRecordResponse)
       await(agentReferenceRepo.create(agentReferenceRecord.copy(normalisedAgentNames = Seq("DummyNotMatching"))))
 
-      val result = doAgentGetRequest(s"/agent-client-relationships/agent-reference/uid/$uid/$normalizedAgentName")
+      val result = doAgentGetRequest(s"/agent-client-relationships/agent/agent-reference/uid/$uid/$normalizedAgentName")
+      result.status shouldBe 404
+    }
+
+    "return 404 status when agent name is missing" in {
+      givenAuditConnector()
+      givenAgentRecordFound(arn, agentRecordResponseWithNoAgentName)
+
+      val result = doAgentGetRequest(s"/agent-client-relationships/agent/agent-reference/uid/$uid/$normalizedAgentName")
       result.status shouldBe 404
     }
 
     "return 502 status agent details are not found" in {
       givenAuditConnector()
-      givenAgentDetailsErrorResponse(502)
+      givenAgentDetailsErrorResponse(arn, 502)
       await(agentReferenceRepo.create(agentReferenceRecord))
 
-      val result = doAgentGetRequest(s"/agent-client-relationships/agent-reference/uid/$uid/$normalizedAgentName")
+      val result = doAgentGetRequest(s"/agent-client-relationships/agent/agent-reference/uid/$uid/$normalizedAgentName")
       result.status shouldBe 502
     }
 
     "return 403 status when agent is suspended" in {
       givenAuditConnector()
-      givenAgentRecordFound(suspendedAgentRecordOption)
+      givenAgentRecordFound(arn, suspendedAgentRecordResponse)
       await(agentReferenceRepo.create(agentReferenceRecord))
 
-      val result = doAgentGetRequest(s"/agent-client-relationships/agent-reference/uid/$uid/$normalizedAgentName")
+      val result = doAgentGetRequest(s"/agent-client-relationships/agent/agent-reference/uid/$uid/$normalizedAgentName")
       result.status shouldBe 403
     }
   }

it/test/uk/gov/hmrc/agentclientrelationships/controllers/RelationshipsBaseControllerISpec.scala

@@ -99,7 +99,8 @@ trait RelationshipsBaseControllerISpec
         "agent.trackPage.cache.expires"                         -> "1 millis",
         "agent.trackPage.cache.enabled"                         -> true,
         "alt-itsa.enabled"                                      -> true,
-        "mongodb.uri"                                           -> mongoUri
+        "mongodb.uri"                                           -> mongoUri,
+        "agent-assurance-internal-auth.token"                   -> "internalAuthToken"
       )
       .overrides(moduleWithOverrides)
       .configure(additionalConfig)

it/test/uk/gov/hmrc/agentclientrelationships/stubs/AgentAssuranceStubs.scala

@@ -19,23 +19,29 @@ package uk.gov.hmrc.agentclientrelationships.stubs
 import com.github.tomakehurst.wiremock.client.WireMock._
 import com.github.tomakehurst.wiremock.stubbing.StubMapping
 import play.api.libs.json.Json
-import uk.gov.hmrc.agentclientrelationships.model.invitationLink.AgentDetailsDesResponse
+import play.api.test.Helpers.{AUTHORIZATION, USER_AGENT}
+import uk.gov.hmrc.agentclientrelationships.support.TestData
+import uk.gov.hmrc.agentmtdidentifiers.model.Arn
 
-trait AgentAssuranceStubs {
+trait AgentAssuranceStubs extends TestData {
 
-  def givenAgentRecordFound(agentRecord: AgentDetailsDesResponse): StubMapping =
+  def givenAgentRecordFound(arn: Arn, agentRecord: TestAgentDetailsDesResponse): StubMapping =
     stubFor(
-      get(urlEqualTo("/agent-assurance/agent-record-with-checks"))
+      get(urlEqualTo(s"/agent-assurance/agent-record-with-checks/arn/${arn.value}"))
+        .withHeader(AUTHORIZATION, equalTo("internalAuthToken"))
+        .withHeader(USER_AGENT, equalTo("agent-client-relationships"))
         .willReturn(
           aResponse()
             .withStatus(200)
             .withBody(Json.toJson(agentRecord).toString)
         )
     )
 
-  def givenAgentDetailsErrorResponse(status: Int): StubMapping =
+  def givenAgentDetailsErrorResponse(arn: Arn, status: Int): StubMapping =
     stubFor(
-      get(urlEqualTo("/agent-assurance/agent-record-with-checks"))
+      get(urlEqualTo(s"/agent-assurance/agent-record-with-checks/arn/${arn.value}"))
+        .withHeader(AUTHORIZATION, equalTo("internalAuthToken"))
+        .withHeader(USER_AGENT, equalTo("agent-client-relationships"))
         .willReturn(
           aResponse()
             .withStatus(status)