Skip to content

Commit 80fa78e

Browse files
Jake-RaffeJake-Raffe
authored
[DL-17363] Handle 403 responses from get Penalties and get Financial Details (#1060)
1 parent c0917de commit 80fa78e

9 files changed

Lines changed: 134 additions & 85 deletions

File tree

‎app/v1/connectors/httpparsers/FinancialDataHttpParser.scala‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ object FinancialDataHttpParser extends Logging {
3939
errorConnectorLog(s"[FinancialDataHttpParser][read] invalid JSON errors: $errors")(response)
4040
Left(ErrorWrapper(responseCorrelationId, InvalidJson))
4141
}
42+
case FORBIDDEN =>
43+
errorConnectorLog(s"[FinancialDataHttpParser][read] 403 error: ${response.body}")(response)
44+
Left(ErrorWrapper(responseCorrelationId, UnauthorisedError))
4245
case status =>
4346
val mtdErrors = errorHelper(response.json)
4447
errorConnectorLog(s"[FinancialDataHttpParser][read] status: $status with Error $mtdErrors")(response)

‎app/v1/connectors/httpparsers/PenaltiesHttpParser.scala‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ object PenaltiesHttpParser extends Logging {
3939
errorConnectorLog(s"[PenaltiesHttpParser][read] invalid JSON errors: $errors")(response)
4040
Left(ErrorWrapper(responseCorrelationId, InvalidJson))
4141
}
42+
case FORBIDDEN =>
43+
errorConnectorLog(s"[PenaltiesHttpParser][read] 403 error: ${response.body}")(response)
44+
Left(ErrorWrapper(responseCorrelationId, UnauthorisedError))
4245
case status =>
4346
val mtdErrors = errorHelper(response.json)
4447
errorConnectorLog(s"[PenaltiesHttpParser][read] status: $status with Error $mtdErrors")(response)

‎app/v1/controllers/FinancialDataController.scala‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,18 @@ package v1.controllers
1919
import cats.data.EitherT
2020
import cats.implicits._
2121
import config.AppConfig
22-
23-
import javax.inject.{Inject, Singleton}
2422
import play.api.libs.json.Json
25-
import play.api.mvc.{Action, AnyContent, ControllerComponents, Result}
26-
import utils.{EndpointLogContext, IdGenerator, Logging}
23+
import play.api.mvc.{ Action, AnyContent, ControllerComponents, Result }
24+
import utils.{ EndpointLogContext, IdGenerator, Logging }
2725
import v1.audit.AuditEvents
2826
import v1.controllers.requestParsers.FinancialDataRequestParser
2927
import v1.models.audit.AuditResponse
3028
import v1.models.errors._
3129
import v1.models.request.penalties.FinancialRawData
32-
import v1.services.{AuditService, EnrolmentsAuthService, PenaltiesService}
30+
import v1.services.{ AuditService, EnrolmentsAuthService, PenaltiesService }
3331

34-
import scala.concurrent.{ExecutionContext, Future}
32+
import javax.inject.{ Inject, Singleton }
33+
import scala.concurrent.{ ExecutionContext, Future }
3534

3635
@Singleton
3736
class FinancialDataController @Inject()(val authService: EnrolmentsAuthService,
@@ -40,64 +39,65 @@ class FinancialDataController @Inject()(val authService: EnrolmentsAuthService,
4039
auditService: AuditService,
4140
cc: ControllerComponents,
4241
val idGenerator: IdGenerator,
43-
appConfig: AppConfig)
44-
(implicit ec: ExecutionContext)
45-
extends AuthorisedController(cc) with BaseController with Logging {
42+
appConfig: AppConfig)(implicit ec: ExecutionContext)
43+
extends AuthorisedController(cc)
44+
with BaseController
45+
with Logging {
4646

4747
implicit val endpointLogContext: EndpointLogContext =
4848
EndpointLogContext(
4949
controllerName = "FinancialDataController",
5050
endpointName = "retrieveFinancialData"
5151
)
5252

53-
5453
def retrieveFinancialData(vrn: String, chargeReference: String): Action[AnyContent] = authorisedAction(vrn).async { implicit request =>
55-
5654
implicit val correlationId: String = idGenerator.getUid
5755

58-
infoLog(s"${endpointLogContext.toString} correlationId: $correlationId: " +
59-
s"attempting to retrieve financial data for VRN: $vrn")
60-
56+
infoLog(
57+
s"${endpointLogContext.toString} correlationId: $correlationId: " +
58+
s"attempting to retrieve financial data for VRN: $vrn")
6159

6260
val result: EitherT[Future, ErrorWrapper, Result] = {
6361
for {
64-
parsedRequest <- EitherT.fromEither[Future](requestParser.parseRequest(FinancialRawData(vrn, chargeReference)))
62+
parsedRequest <- EitherT.fromEither[Future](requestParser.parseRequest(FinancialRawData(vrn, chargeReference)))
6563
serviceResponse <- EitherT(service.retrieveFinancialData(parsedRequest))
6664
} yield {
6765

68-
infoLog(s"${endpointLogContext.toString} " +
69-
s"Successfully retrieved Financial Data with correlationId : ${serviceResponse.correlationId}")
70-
66+
infoLog(
67+
s"${endpointLogContext.toString} " +
68+
s"Successfully retrieved Financial Data with correlationId : ${serviceResponse.correlationId}")
7169

72-
auditService.auditEvent(AuditEvents.auditFinancialData(serviceResponse.correlationId,
73-
request.userDetails, AuditResponse(OK, Right(Some(Json.toJson(serviceResponse.responseData))))
74-
))
70+
auditService.auditEvent(
71+
AuditEvents.auditFinancialData(serviceResponse.correlationId,
72+
request.userDetails,
73+
AuditResponse(OK, Right(Some(Json.toJson(serviceResponse.responseData))))))
7574

7675
Ok(Json.toJson(serviceResponse.responseData))
7776
.withApiHeaders(serviceResponse.correlationId)
7877
}
7978
}
8079
result.leftMap { errorWrapper: ErrorWrapper =>
8180
val resCorrelationId: String = errorWrapper.correlationId
82-
val leftResult = errorResult(errorWrapper).withApiHeaders(resCorrelationId)
81+
val leftResult = errorResult(errorWrapper).withApiHeaders(resCorrelationId)
8382
warnLog(ControllerError(endpointLogContext, vrn, request, leftResult.header.status, errorWrapper.error.message, resCorrelationId))
8483

85-
auditService.auditEvent(AuditEvents.auditFinancialData(resCorrelationId,
86-
request.userDetails, AuditResponse(httpStatus = leftResult.header.status, Left(errorWrapper.auditErrors))
87-
))
84+
auditService.auditEvent(
85+
AuditEvents.auditFinancialData(resCorrelationId,
86+
request.userDetails,
87+
AuditResponse(httpStatus = leftResult.header.status, Left(errorWrapper.auditErrors))))
8888

8989
leftResult
9090
}.merge
9191
}
9292

9393
private def errorResult(errorWrapper: ErrorWrapper): Result = {
9494
(errorWrapper.error: @unchecked) match {
95-
case VrnFormatError | RuleIncorrectGovTestScenarioError |
96-
FinancialInvalidIdNumber |
97-
FinancialInvalidSearchItem |
98-
MtdError("INVALID_REQUEST", _, _) => BadRequest(Json.toJson(errorWrapper))
95+
case VrnFormatError | RuleIncorrectGovTestScenarioError | FinancialInvalidIdNumber | FinancialInvalidSearchItem |
96+
MtdError("INVALID_REQUEST", _, _) =>
97+
BadRequest(Json.toJson(errorWrapper))
9998
case FinancialNotDataFound => NotFound(Json.toJson(errorWrapper))
100-
case _ => InternalServerError(Json.toJson(errorWrapper))
99+
case UnauthorisedError => Forbidden(Json.toJson(errorWrapper))
100+
case _ => InternalServerError(Json.toJson(errorWrapper))
101101
}
102102
}
103103
}

‎app/v1/controllers/PenaltiesController.scala‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -39,51 +39,52 @@ class PenaltiesController @Inject()(val authService: EnrolmentsAuthService,
3939
auditService: AuditService,
4040
cc: ControllerComponents,
4141
val idGenerator: IdGenerator,
42-
appConfig: AppConfig)
43-
(implicit ec: ExecutionContext)
44-
extends AuthorisedController(cc) with BaseController with Logging {
42+
appConfig: AppConfig)(implicit ec: ExecutionContext)
43+
extends AuthorisedController(cc)
44+
with BaseController
45+
with Logging {
4546

4647
implicit val endpointLogContext: EndpointLogContext =
4748
EndpointLogContext(
4849
controllerName = "PenaltiesController",
4950
endpointName = "retrievePenalties"
5051
)
5152

52-
5353
def retrievePenalties(vrn: String): Action[AnyContent] = authorisedAction(vrn).async { implicit request =>
54-
5554
implicit val correlationId: String = idGenerator.getUid
5655

57-
infoLog(s"${endpointLogContext.toString} correlationId: $correlationId: " +
58-
s"attempting to retrieve penalties for VRN: $vrn")
59-
56+
infoLog(
57+
s"${endpointLogContext.toString} correlationId: $correlationId: " +
58+
s"attempting to retrieve penalties for VRN: $vrn")
6059

6160
val result: EitherT[Future, ErrorWrapper, Result] = {
6261
for {
63-
parsedRequest <- EitherT.fromEither[Future](requestParser.parseRequest(PenaltiesRawData(vrn)))
62+
parsedRequest <- EitherT.fromEither[Future](requestParser.parseRequest(PenaltiesRawData(vrn)))
6463
serviceResponse <- EitherT(service.retrievePenalties(parsedRequest))
6564
} yield {
6665

67-
infoLog(s"${endpointLogContext.toString} " +
68-
s"Successfully retrieved Payments from DES with correlationId : ${serviceResponse.correlationId}")
69-
66+
infoLog(
67+
s"${endpointLogContext.toString} " +
68+
s"Successfully retrieved Payments from DES with correlationId : ${serviceResponse.correlationId}")
7069

71-
auditService.auditEvent(AuditEvents.auditPenalties(serviceResponse.correlationId,
72-
request.userDetails, AuditResponse(OK, Right(Some(Json.toJson(serviceResponse.responseData))))
73-
))
70+
auditService.auditEvent(
71+
AuditEvents.auditPenalties(serviceResponse.correlationId,
72+
request.userDetails,
73+
AuditResponse(OK, Right(Some(Json.toJson(serviceResponse.responseData))))))
7474

7575
Ok(Json.toJson(serviceResponse.responseData))
7676
.withApiHeaders(serviceResponse.correlationId)
7777
}
7878
}
7979
result.leftMap { errorWrapper: ErrorWrapper =>
8080
val resCorrelationId: String = errorWrapper.correlationId
81-
val leftResult = errorResult(errorWrapper).withApiHeaders(resCorrelationId)
81+
val leftResult = errorResult(errorWrapper).withApiHeaders(resCorrelationId)
8282
warnLog(ControllerError(endpointLogContext, vrn, request, leftResult.header.status, errorWrapper.error.message, resCorrelationId))
8383

84-
auditService.auditEvent(AuditEvents.auditPenalties(resCorrelationId,
85-
request.userDetails, AuditResponse(httpStatus = leftResult.header.status, Left(errorWrapper.auditErrors))
86-
))
84+
auditService.auditEvent(
85+
AuditEvents.auditPenalties(resCorrelationId,
86+
request.userDetails,
87+
AuditResponse(httpStatus = leftResult.header.status, Left(errorWrapper.auditErrors))))
8788

8889
leftResult
8990
}.merge
@@ -92,7 +93,8 @@ class PenaltiesController @Inject()(val authService: EnrolmentsAuthService,
9293
private def errorResult(errorWrapper: ErrorWrapper): Result = {
9394
(errorWrapper.error: @unchecked) match {
9495
case PenaltiesInvalidIdValue | RuleIncorrectGovTestScenarioError => BadRequest(Json.toJson(errorWrapper))
95-
case _ => InternalServerError(Json.toJson(errorWrapper))
96+
case UnauthorisedError => Forbidden(Json.toJson(errorWrapper))
97+
case _ => InternalServerError(Json.toJson(errorWrapper))
9698
}
9799
}
98100
}

‎app/v1/services/AuditService.scala‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,17 @@
1616

1717
package v1.services
1818

19-
import javax.inject.{Inject, Singleton}
20-
import play.api.libs.json.{Json, Writes}
21-
import play.api.{Configuration, Logger}
19+
import play.api.Configuration
20+
import play.api.libs.json.{ Json, Writes }
2221
import uk.gov.hmrc.http.HeaderCarrier
2322
import uk.gov.hmrc.play.audit.AuditExtensions
24-
import uk.gov.hmrc.play.audit.http.connector.{AuditConnector, AuditResult}
23+
import uk.gov.hmrc.play.audit.http.connector.{ AuditConnector, AuditResult }
2524
import uk.gov.hmrc.play.audit.model.ExtendedDataEvent
2625
import uk.gov.hmrc.play.bootstrap.config.AppName
2726
import v1.models.audit.AuditEvent
2827

29-
30-
import scala.concurrent.{ExecutionContext, Future}
28+
import javax.inject.{ Inject, Singleton }
29+
import scala.concurrent.{ ExecutionContext, Future }
3130

3231
@Singleton
3332
class AuditService @Inject()(auditConnector: AuditConnector,

‎test/v1/connectors/httpparsers/FinancialDataHttpParserSpec.scala‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,14 @@
1616

1717
package v1.connectors.httpparsers
1818

19-
import play.api.http.Status.{ BAD_REQUEST, CREATED, OK }
19+
import play.api.http.Status.{ BAD_REQUEST, CREATED, FORBIDDEN, OK }
2020
import play.api.libs.json.{ JsValue, Json }
2121
import support.UnitSpec
2222
import uk.gov.hmrc.http.HttpResponse
2323
import v1.connectors.httpparsers.FinancialDataHttpParser.FinancialDataHttpReads
2424
import v1.constants.FinancialDataConstants
2525
import v1.constants.FinancialDataConstants._
26+
import v1.constants.PenaltiesConstants.errorWrapper
2627
import v1.models.errors._
2728
import v1.models.outcomes.ResponseWrapper
2829
import v1.models.response.financialData.FinancialDataResponse
@@ -94,7 +95,17 @@ class FinancialDataHttpParserSpec extends UnitSpec {
9495
}
9596
}
9697

97-
"API response is an error (any non 200/201 status)" must {
98+
"response is FORBIDDEN (403)" must {
99+
"return an UnauthorisedError" in {
100+
val response =
101+
HttpResponse(status = FORBIDDEN, body = "403 - Forbidden", headers = Map("CorrelationId" -> Seq(correlationId)))
102+
val result = FinancialDataHttpReads.read("", "", response)
103+
104+
result shouldBe Left(errorWrapper(MtdError("CLIENT_OR_AGENT_NOT_AUTHORISED", "The client and/or agent is not authorised")))
105+
}
106+
}
107+
108+
"API response is an error (any non 200/201/403 status)" must {
98109
"return the error in an ErrorWrapper (appropriate error handled by .errorHelper)" in {
99110
val error = Json.parse("""
100111
|{

‎test/v1/connectors/httpparsers/PenaltiesHttpParserSpec.scala‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,17 @@ class PenaltiesHttpParserSpec extends UnitSpec {
6666
}
6767
}
6868

69-
"response is an error (any non 200 status)" when {
69+
"response is FORBIDDEN (403)" must {
70+
"return an UnauthorisedError" in {
71+
val response =
72+
HttpResponse(status = FORBIDDEN, body = "403 - Forbidden", headers = Map("CorrelationId" -> Seq(correlationId)))
73+
val result = PenaltiesHttpReads.read("", "", response)
74+
75+
result shouldBe Left(errorWrapper(MtdError("CLIENT_OR_AGENT_NOT_AUTHORISED", "The client and/or agent is not authorised")))
76+
}
77+
}
78+
79+
"response is an error (any non 200 or 403 status)" when {
7080
"return an error in an ErrorWrapper (appropriate error handled by .errorHelper)" in {
7181
val errorJson = buildErrorHip("002", Some("Invalid Tax Regime"))
7282
val httpResponse = buildHttpResponse(BAD_REQUEST, errorJson)

‎test/v1/controllers/FinancialDataControllerSpec.scala‎

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,21 @@ import v1.audit.AuditEvents
2424
import v1.constants.FinancialDataConstants
2525
import v1.mocks.MockIdGenerator
2626
import v1.mocks.requestParsers.MockFinancialDataRequestParser
27-
import v1.mocks.services.{MockAuditService, MockEnrolmentsAuthService, MockPenaltiesService}
28-
import v1.models.audit.{AuditError, AuditResponse}
29-
import v1.models.errors.{FinancialNotDataFound, MtdError, RuleIncorrectGovTestScenarioError, UnexpectedFailure, VrnFormatError}
27+
import v1.mocks.services.{ MockAuditService, MockEnrolmentsAuthService, MockPenaltiesService }
28+
import v1.models.audit.{ AuditError, AuditResponse }
29+
import v1.models.errors._
3030

3131
import scala.concurrent.ExecutionContext.Implicits.global
3232
import scala.concurrent.Future
3333

34-
class FinancialDataControllerSpec extends ControllerBaseSpec with MockEnrolmentsAuthService
35-
with MockPenaltiesService with MockFinancialDataRequestParser with MockAuditService with MockIdGenerator with MockAppConfig {
34+
class FinancialDataControllerSpec
35+
extends ControllerBaseSpec
36+
with MockEnrolmentsAuthService
37+
with MockPenaltiesService
38+
with MockFinancialDataRequestParser
39+
with MockAuditService
40+
with MockIdGenerator
41+
with MockAppConfig {
3642

3743
trait Test {
3844
val hc: HeaderCarrier = HeaderCarrier()
@@ -63,9 +69,11 @@ class FinancialDataControllerSpec extends ControllerBaseSpec with MockEnrolments
6369

6470
MockPenaltiesRequestParser.parse(FinancialDataConstants.rawData)(Right(FinancialDataConstants.financialRequest))
6571

66-
MockPenaltiesService.retrieveFinancialData(FinancialDataConstants.financialRequest)(Right(FinancialDataConstants.wrappedFinancialDataResponse()))
72+
MockPenaltiesService.retrieveFinancialData(FinancialDataConstants.financialRequest)(
73+
Right(FinancialDataConstants.wrappedFinancialDataResponse()))
6774

68-
val result: Future[Result] = controller.retrieveFinancialData(FinancialDataConstants.vrn, FinancialDataConstants.searchItem)(fakeGetRequest)
75+
val result: Future[Result] =
76+
controller.retrieveFinancialData(FinancialDataConstants.vrn, FinancialDataConstants.searchItem)(fakeGetRequest)
6977

7078
status(result) shouldBe OK
7179
contentAsJson(result) shouldBe FinancialDataConstants.testUpstreamFinancialDetails
@@ -83,9 +91,11 @@ class FinancialDataControllerSpec extends ControllerBaseSpec with MockEnrolments
8391

8492
MockPenaltiesRequestParser.parse(FinancialDataConstants.rawData)(Right(FinancialDataConstants.financialRequest))
8593

86-
MockPenaltiesService.retrieveFinancialData(FinancialDataConstants.financialRequest)(Right(FinancialDataConstants.wrappedFinancialDataResponse(FinancialDataConstants.testFinancialDataResponse)))
94+
MockPenaltiesService.retrieveFinancialData(FinancialDataConstants.financialRequest)(
95+
Right(FinancialDataConstants.wrappedFinancialDataResponse(FinancialDataConstants.testFinancialDataResponse)))
8796

88-
val result: Future[Result] = controller.retrieveFinancialData(FinancialDataConstants.vrn, FinancialDataConstants.searchItem)(fakeGetRequest)
97+
val result: Future[Result] =
98+
controller.retrieveFinancialData(FinancialDataConstants.vrn, FinancialDataConstants.searchItem)(fakeGetRequest)
8999

90100
status(result) shouldBe OK
91101
contentAsJson(result) shouldBe FinancialDataConstants.testUpstreamFinancialDetails
@@ -106,6 +116,7 @@ class FinancialDataControllerSpec extends ControllerBaseSpec with MockEnrolments
106116
(VrnFormatError, BAD_REQUEST),
107117
(RuleIncorrectGovTestScenarioError, BAD_REQUEST),
108118
(FinancialNotDataFound, NOT_FOUND),
119+
(UnauthorisedError, FORBIDDEN),
109120
(UnexpectedFailure.mtdError(INTERNAL_SERVER_ERROR, "error"), INTERNAL_SERVER_ERROR)
110121
)
111122

@@ -119,9 +130,11 @@ class FinancialDataControllerSpec extends ControllerBaseSpec with MockEnrolments
119130

120131
MockPenaltiesRequestParser.parse(FinancialDataConstants.rawData)(Right(FinancialDataConstants.financialRequest))
121132

122-
MockPenaltiesService.retrieveFinancialData(FinancialDataConstants.financialRequest)(Left(FinancialDataConstants.errorWrapper(mtdError)))
133+
MockPenaltiesService.retrieveFinancialData(FinancialDataConstants.financialRequest)(
134+
Left(FinancialDataConstants.errorWrapper(mtdError)))
123135

124-
val result: Future[Result] = controller.retrieveFinancialData(FinancialDataConstants.vrn, FinancialDataConstants.searchItem)(fakeGetRequest)
136+
val result: Future[Result] =
137+
controller.retrieveFinancialData(FinancialDataConstants.vrn, FinancialDataConstants.searchItem)(fakeGetRequest)
125138

126139
status(result) shouldBe expectedStatus
127140
contentAsJson(result) shouldBe Json.toJson(mtdError)
@@ -152,11 +165,12 @@ class FinancialDataControllerSpec extends ControllerBaseSpec with MockEnrolments
152165
contentType(result) shouldBe Some("application/json")
153166
header("X-CorrelationId", result) shouldBe Some(FinancialDataConstants.correlationId)
154167

155-
MockedAuditService.verifyAuditEvent(AuditEvents.auditFinancialData(
156-
correlationId = FinancialDataConstants.correlationId,
157-
userDetails = FinancialDataConstants.userDetails,
158-
auditResponse = AuditResponse(BAD_REQUEST, Some(Seq(AuditError(VrnFormatError.code))), None)
159-
))
168+
MockedAuditService.verifyAuditEvent(
169+
AuditEvents.auditFinancialData(
170+
correlationId = FinancialDataConstants.correlationId,
171+
userDetails = FinancialDataConstants.userDetails,
172+
auditResponse = AuditResponse(BAD_REQUEST, Some(Seq(AuditError(VrnFormatError.code))), None)
173+
))
160174
}
161175
}
162176
}

0 commit comments

Comments
 (0)