From c26906e587fe3ac709feff0b2e16d6ff37b3e5dd Mon Sep 17 00:00:00 2001 From: Paul Hodgson Date: Mon, 23 Nov 2020 14:12:37 +0000 Subject: [PATCH] Mibm 168 (#118) * [MIBM-168][PH] obfuscate journey details to enable logging * [MIBM-168][PH] add some info and warn level logging --- .../CannotUseServiceController.scala | 8 +- .../CheckYourAnswersController.scala | 33 ++++---- .../DeclarationConfirmationController.scala | 3 +- .../DeclarationJourneyActionProvider.scala | 35 ++++++--- .../DeclarationJourneyController.scala | 16 +++- .../controllers/EoriNumberController.scala | 22 +++--- .../GoodsOverThresholdController.scala | 17 +++-- .../InvalidRequestController.scala | 2 + .../PaymentCalculationController.scala | 27 ++++--- .../PurchaseDetailsController.scala | 26 +++---- .../controllers/ReviewGoodsController.scala | 28 +++---- .../model/adresslookup/Address.scala | 14 ++-- .../model/core/SessionId.scala | 20 ++++- .../DeclarationJourneyRepository.scala | 6 +- .../utils/DeclarationJourneyLogger.scala | 76 +++++++++++++++++++ .../utils/Obfuscator.scala | 23 ++++++ .../model/core/DeclarationSpec.scala | 15 ++++ .../utils/ObfuscatorSpec.scala | 35 +++++++++ 18 files changed, 301 insertions(+), 105 deletions(-) create mode 100644 app/uk/gov/hmrc/merchandiseinbaggage/utils/DeclarationJourneyLogger.scala create mode 100644 app/uk/gov/hmrc/merchandiseinbaggage/utils/Obfuscator.scala create mode 100644 test/uk/gov/hmrc/merchandiseinbaggage/utils/ObfuscatorSpec.scala diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/CannotUseServiceController.scala b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/CannotUseServiceController.scala index 50b008eef..6f69a8f65 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/CannotUseServiceController.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/CannotUseServiceController.scala @@ -19,6 +19,7 @@ package uk.gov.hmrc.merchandiseinbaggage.controllers import javax.inject.{Inject, Singleton} import play.api.mvc.{Action, AnyContent, MessagesControllerComponents} import uk.gov.hmrc.merchandiseinbaggage.config.AppConfig +import uk.gov.hmrc.merchandiseinbaggage.controllers.DeclarationJourneyController.goodsDestinationUnansweredMessage import uk.gov.hmrc.merchandiseinbaggage.views.html.CannotUseServiceView @Singleton @@ -28,8 +29,9 @@ class CannotUseServiceController @Inject()(override val controllerComponents: Me extends DeclarationJourneyController { val onPageLoad: Action[AnyContent] = actionProvider.journeyAction { implicit request => - request.declarationJourney.maybeGoodsDestination.fold(actionProvider.invalidRequest) { goodsDestination => - Ok(view(request.declarationJourney.declarationType, goodsDestination)) - } + request.declarationJourney.maybeGoodsDestination + .fold(actionProvider.invalidRequest(goodsDestinationUnansweredMessage)) { goodsDestination => + Ok(view(request.declarationJourney.declarationType, goodsDestination)) + } } } diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/CheckYourAnswersController.scala b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/CheckYourAnswersController.scala index 3c965e093..ce13186d8 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/CheckYourAnswersController.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/CheckYourAnswersController.scala @@ -22,6 +22,7 @@ import reactivemongo.api.commands.UpdateWriteResult import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse} import uk.gov.hmrc.merchandiseinbaggage.config.AppConfig import uk.gov.hmrc.merchandiseinbaggage.connectors.{MibConnector, PaymentConnector} +import uk.gov.hmrc.merchandiseinbaggage.controllers.DeclarationJourneyController.incompleteMessage import uk.gov.hmrc.merchandiseinbaggage.forms.CheckYourAnswersForm.form import uk.gov.hmrc.merchandiseinbaggage.model.api.{Declaration, PayApiRequestBuilder} import uk.gov.hmrc.merchandiseinbaggage.model.core.DeclarationType.{Export, Import} @@ -44,32 +45,26 @@ class CheckYourAnswersController @Inject()(override val controllerComponents: Me extends DeclarationJourneyUpdateController with PayApiRequestBuilder { val onPageLoad: Action[AnyContent] = actionProvider.journeyAction.async { implicit request => - request.declarationJourney.declarationIfRequiredAndComplete.fold(actionProvider.invalidRequestF){ declaration => - calculationService.paymentCalculation(declaration.declarationGoods).map { paymentCalculations => - if(declaration.declarationType == Import - && paymentCalculations.totalGbpValue.value > declaration.goodsDestination.threshold.value) { - Redirect(routes.GoodsOverThresholdController.onPageLoad()) - } else { - val taxDue = paymentCalculations.totalTaxDue - Ok(page(form, declaration, taxDue)) + request.declarationJourney.declarationIfRequiredAndComplete + .fold(actionProvider.invalidRequestF(incompleteMessage)) { declaration => + calculationService.paymentCalculation(declaration.declarationGoods).map { paymentCalculations => + if (declaration.declarationType == Import + && paymentCalculations.totalGbpValue.value > declaration.goodsDestination.threshold.value) { + Redirect(routes.GoodsOverThresholdController.onPageLoad()) + } else Ok(page(form, declaration, paymentCalculations.totalTaxDue)) } } - } } val onSubmit: Action[AnyContent] = actionProvider.journeyAction.async { implicit request => request.declarationJourney.declarationIfRequiredAndComplete - .fold(actionProvider.invalidRequestF)(declaration => declarationConfirmation(declaration)) - } + .fold(actionProvider.invalidRequestF(incompleteMessage))(declaration => declarationConfirmation(declaration)) + } val addMoreGoods: Action[AnyContent] = actionProvider.journeyAction.async { implicit request => val updatedGoodsEntries: Seq[GoodsEntry] = request.declarationJourney.goodsEntries.entries :+ GoodsEntry.empty - repo.upsert( - request.declarationJourney.copy( - goodsEntries = GoodsEntries(updatedGoodsEntries) - ) - ).map { _ => + repo.upsert(request.declarationJourney.copy(goodsEntries = GoodsEntries(updatedGoodsEntries))).map { _ => Redirect(routes.GoodsTypeQuantityController.onPageLoad(updatedGoodsEntries.size)) } } @@ -85,14 +80,14 @@ class CheckYourAnswersController @Inject()(override val controllerComponents: Me (implicit request: DeclarationJourneyRequest[AnyContent]): Future[Result] = for { persist <- mibConnector.persistDeclaration(declaration) - pay <- createPaymentSession(declaration.declarationGoods) - _ <- resetJourney(persist) + pay <- createPaymentSession(declaration.declarationGoods) + _ <- resetJourney(persist) } yield Redirect(connector.extractUrl(pay).nextUrl.value) private def createPaymentSession(goods: DeclarationGoods)(implicit headerCarrier: HeaderCarrier): Future[HttpResponse] = for { payApiRequest <- buildRequest(goods, calculationService.paymentCalculation) - response <- connector.createPaymentSession(payApiRequest) + response <- connector.createPaymentSession(payApiRequest) } yield response private def resetAndRedirect(declarationId: DeclarationId) diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/DeclarationConfirmationController.scala b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/DeclarationConfirmationController.scala index fa1163f49..f17687e3a 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/DeclarationConfirmationController.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/DeclarationConfirmationController.scala @@ -20,6 +20,7 @@ import javax.inject.Inject import play.api.mvc.{Action, AnyContent, MessagesControllerComponents} import uk.gov.hmrc.merchandiseinbaggage.config.AppConfig import uk.gov.hmrc.merchandiseinbaggage.connectors.MibConnector +import uk.gov.hmrc.merchandiseinbaggage.controllers.DeclarationJourneyController.incompleteMessage import uk.gov.hmrc.merchandiseinbaggage.views.html.DeclarationConfirmationView import scala.concurrent.ExecutionContext @@ -33,7 +34,7 @@ class DeclarationConfirmationController @Inject()( extends DeclarationJourneyController { val onPageLoad: Action[AnyContent] = actionProvider.journeyAction.async { implicit request => - request.declarationJourney.declarationId.fold(actionProvider.invalidRequestF) { declarationId => + request.declarationJourney.declarationId.fold(actionProvider.invalidRequestF(incompleteMessage)) { declarationId => connector.findDeclaration(declarationId).map(declaration => Ok(view(declaration))) } } diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/DeclarationJourneyActionProvider.scala b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/DeclarationJourneyActionProvider.scala index d83f91ecf..c5f4b8f28 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/DeclarationJourneyActionProvider.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/DeclarationJourneyActionProvider.scala @@ -22,6 +22,7 @@ import play.api.mvc._ import uk.gov.hmrc.http.SessionKeys import uk.gov.hmrc.merchandiseinbaggage.model.core.SessionId import uk.gov.hmrc.merchandiseinbaggage.repositories.DeclarationJourneyRepository +import uk.gov.hmrc.merchandiseinbaggage.utils.DeclarationJourneyLogger import scala.concurrent.{ExecutionContext, Future} @@ -33,21 +34,29 @@ class DeclarationJourneyActionProvider @Inject()(defaultActionBuilder: DefaultAc def goodsAction(idx: Int): ActionBuilder[DeclarationGoodsRequest, AnyContent] = defaultActionBuilder andThen journeyActionRefiner andThen goodsActionRefiner(idx) - def invalidRequest: Result = Redirect(routes.InvalidRequestController.onPageLoad()) + def invalidRequest(warnMessage: String)(implicit request: RequestHeader): Result = { + DeclarationJourneyLogger.warn( + s"$warnMessage so redirecting to ${routes.InvalidRequestController.onPageLoad()}")(request) + Redirect(routes.InvalidRequestController.onPageLoad()) + } - def invalidRequestF: Future[Result] = Future.successful(invalidRequest) + def invalidRequestF(warningMessage: String)(implicit request: RequestHeader): Future[Result] = { + Future.successful(invalidRequest(warningMessage)) + } private def journeyActionRefiner: ActionRefiner[Request, DeclarationJourneyRequest] = new ActionRefiner[Request, DeclarationJourneyRequest] { override protected def refine[A](request: Request[A]): Future[Either[Result, DeclarationJourneyRequest[A]]] = { - request.session.get(SessionKeys.sessionId) match { - case None => Future successful Left(invalidRequest) + case None => Future successful Left(invalidRequest("Session Id not found")(request)) case Some(sessionId) => - repo.findBySessionId(SessionId(sessionId)).map{ - case Some(declarationJourney) => Right(new DeclarationJourneyRequest(declarationJourney, request)) - case _ => Left(invalidRequest) + repo.findBySessionId(SessionId(sessionId)).map { + case Some(declarationJourney) => + val declarationJourneyRequest = new DeclarationJourneyRequest(declarationJourney, request) + DeclarationJourneyLogger.info("journeyActionRefiner success")(declarationJourneyRequest) + Right(declarationJourneyRequest) + case _ => Left(invalidRequest("Persisted declaration journey not found")(request)) } } } @@ -59,10 +68,14 @@ class DeclarationJourneyActionProvider @Inject()(defaultActionBuilder: DefaultAc new ActionRefiner[DeclarationJourneyRequest, DeclarationGoodsRequest] { override protected def refine[A](request: DeclarationJourneyRequest[A]): Future[Either[Result, DeclarationGoodsRequest[A]]] = { - request.declarationJourney.goodsEntries.entries.lift(idx - 1) match { - case None => Future successful Left(invalidRequest) - case Some(goodsEntry) => Future successful Right( new DeclarationGoodsRequest(request, goodsEntry)) - } + Future successful (request.declarationJourney.goodsEntries.entries.lift(idx - 1) match { + case None => + Left(invalidRequest(s"Goods entry not found for index $idx")(request)) + case Some(goodsEntry) => + val declarationJourneyRequest = new DeclarationGoodsRequest(request, goodsEntry) + DeclarationJourneyLogger.info("goodsActionRefiner success")(declarationJourneyRequest) + Right(declarationJourneyRequest) + }) } override protected def executionContext: ExecutionContext = ec diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/DeclarationJourneyController.scala b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/DeclarationJourneyController.scala index f3e3b7b02..06cceea3d 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/DeclarationJourneyController.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/DeclarationJourneyController.scala @@ -20,6 +20,7 @@ import play.api.i18n.Messages import play.api.mvc._ import uk.gov.hmrc.merchandiseinbaggage.model.core.{DeclarationJourney, GoodsEntry} import uk.gov.hmrc.merchandiseinbaggage.repositories.DeclarationJourneyRepository +import uk.gov.hmrc.merchandiseinbaggage.utils.DeclarationJourneyLogger import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendBaseController import scala.concurrent.{ExecutionContext, Future} @@ -30,6 +31,12 @@ trait DeclarationJourneyController extends FrontendBaseController { val onPageLoad: Action[AnyContent] } +object DeclarationJourneyController { + val incompleteMessage = "declaration journey is not required and complete" + val goodsDestinationUnansweredMessage = "goods destination is unanswered" + val goodsDeclarationIncompleteMessage = "goods declaration is incomplete" +} + trait DeclarationJourneyUpdateController extends DeclarationJourneyController { val onSubmit: Action[AnyContent] @@ -48,10 +55,15 @@ trait IndexedDeclarationJourneyController extends FrontendBaseController { def onPageLoad(idx: Int): Action[AnyContent] - def withGoodsCategory(goodsEntry: GoodsEntry)(f: String => Future[Result]): Future[Result] = + def withGoodsCategory(goodsEntry: GoodsEntry) + (f: String => Future[Result]) + (implicit request: DeclarationGoodsRequest[AnyContent]): Future[Result] = goodsEntry.maybeCategoryQuantityOfGoods match { case Some(c) => f(c.category) - case None => Future successful Redirect(routes.InvalidRequestController.onPageLoad()) + case None => + DeclarationJourneyLogger.warn( + s"Goods category not found so redirecting to ${routes.InvalidRequestController.onPageLoad()}") + Future successful Redirect(routes.InvalidRequestController.onPageLoad()) } } diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/EoriNumberController.scala b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/EoriNumberController.scala index 70b386d29..d9e88261b 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/EoriNumberController.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/EoriNumberController.scala @@ -37,8 +37,10 @@ class EoriNumberController @Inject()( private val backButtonUrl: Call = routes.CustomsAgentController.onPageLoad() + private val invalidRequestMessage = "maybeIsACustomsAgent is unanswered" + val onPageLoad: Action[AnyContent] = actionProvider.journeyAction { implicit request => - request.declarationJourney.maybeIsACustomsAgent.fold(actionProvider.invalidRequest) { isAgent => + request.declarationJourney.maybeIsACustomsAgent.fold(actionProvider.invalidRequest(invalidRequestMessage)) { isAgent => val preparedForm = request.declarationJourney.maybeEori.fold(form)(e => form.fill(e.value)) Ok(view(preparedForm, isAgent, backButtonUrl)) @@ -46,16 +48,14 @@ class EoriNumberController @Inject()( } val onSubmit: Action[AnyContent] = actionProvider.journeyAction.async { implicit request => - request.declarationJourney.maybeIsACustomsAgent.fold(actionProvider.invalidRequestF) { isAgent => - form - .bindFromRequest() - .fold( - formWithErrors => - Future.successful(BadRequest(view(formWithErrors, isAgent, backButtonUrl))), - eori => - persistAndRedirect( - request.declarationJourney.copy(maybeEori = Some(Eori(eori))), routes.TravellerDetailsController.onPageLoad()) - ) + request.declarationJourney.maybeIsACustomsAgent.fold(actionProvider.invalidRequestF(invalidRequestMessage)) { isAgent => + form.bindFromRequest().fold( + formWithErrors => + Future.successful(BadRequest(view(formWithErrors, isAgent, backButtonUrl))), + eori => + persistAndRedirect( + request.declarationJourney.copy(maybeEori = Some(Eori(eori))), routes.TravellerDetailsController.onPageLoad()) + ) } } } diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/GoodsOverThresholdController.scala b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/GoodsOverThresholdController.scala index b752bd0e1..3948ad7ca 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/GoodsOverThresholdController.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/GoodsOverThresholdController.scala @@ -19,6 +19,7 @@ package uk.gov.hmrc.merchandiseinbaggage.controllers import javax.inject.{Inject, Singleton} import play.api.mvc.{Action, AnyContent, MessagesControllerComponents} import uk.gov.hmrc.merchandiseinbaggage.config.AppConfig +import uk.gov.hmrc.merchandiseinbaggage.controllers.DeclarationJourneyController.{goodsDeclarationIncompleteMessage, goodsDestinationUnansweredMessage} import uk.gov.hmrc.merchandiseinbaggage.service.CalculationService import uk.gov.hmrc.merchandiseinbaggage.views.html.GoodsOverThresholdView @@ -33,13 +34,15 @@ class GoodsOverThresholdController @Inject()(override val controllerComponents: extends DeclarationJourneyController { val onPageLoad: Action[AnyContent] = actionProvider.journeyAction.async { implicit request => - request.declarationJourney.goodsEntries.declarationGoodsIfComplete.fold(actionProvider.invalidRequestF) { goods => - request.declarationJourney.maybeGoodsDestination.fold(actionProvider.invalidRequestF) { destination => - for { - paymentCalculations <- calculationService.paymentCalculation(goods) - rates <- calculationService.getConversionRates(goods) - } yield Ok(view(destination, paymentCalculations.totalGbpValue, rates)) + request.declarationJourney.goodsEntries.declarationGoodsIfComplete + .fold(actionProvider.invalidRequestF(goodsDeclarationIncompleteMessage)) { goods => + request.declarationJourney.maybeGoodsDestination + .fold(actionProvider.invalidRequestF(goodsDestinationUnansweredMessage)) { destination => + for { + paymentCalculations <- calculationService.paymentCalculation(goods) + rates <- calculationService.getConversionRates(goods) + } yield Ok(view(destination, paymentCalculations.totalGbpValue, rates)) + } } - } } } diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/InvalidRequestController.scala b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/InvalidRequestController.scala index be9605dc4..05c6334f2 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/InvalidRequestController.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/InvalidRequestController.scala @@ -19,6 +19,7 @@ package uk.gov.hmrc.merchandiseinbaggage.controllers import javax.inject.{Inject, Singleton} import play.api.mvc.{Action, AnyContent, MessagesControllerComponents} import uk.gov.hmrc.merchandiseinbaggage.config.AppConfig +import uk.gov.hmrc.merchandiseinbaggage.utils.DeclarationJourneyLogger import uk.gov.hmrc.merchandiseinbaggage.views.html.InvalidRequestView import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendBaseController @@ -31,6 +32,7 @@ class InvalidRequestController @Inject()(override val controllerComponents: Mess extends FrontendBaseController { def onPageLoad(): Action[AnyContent] = Action { implicit request => + DeclarationJourneyLogger.warn(s"User was directed to ${routes.InvalidRequestController.onPageLoad()}") Ok(view()) } } diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/PaymentCalculationController.scala b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/PaymentCalculationController.scala index c7a2fc824..bb4a5a8c9 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/PaymentCalculationController.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/PaymentCalculationController.scala @@ -19,6 +19,7 @@ package uk.gov.hmrc.merchandiseinbaggage.controllers import javax.inject.{Inject, Singleton} import play.api.mvc.{Action, AnyContent, Call, MessagesControllerComponents} import uk.gov.hmrc.merchandiseinbaggage.config.AppConfig +import uk.gov.hmrc.merchandiseinbaggage.controllers.DeclarationJourneyController.{goodsDeclarationIncompleteMessage, goodsDestinationUnansweredMessage} import uk.gov.hmrc.merchandiseinbaggage.service.CalculationService import uk.gov.hmrc.merchandiseinbaggage.views.html.PaymentCalculationView @@ -35,18 +36,20 @@ class PaymentCalculationController @Inject()(override val controllerComponents: private val backButtonUrl: Call = routes.ReviewGoodsController.onPageLoad() val onPageLoad: Action[AnyContent] = actionProvider.journeyAction.async { implicit request => - request.declarationJourney.goodsEntries.declarationGoodsIfComplete.fold(actionProvider.invalidRequestF) { goods => - request.declarationJourney.maybeGoodsDestination.fold(actionProvider.invalidRequestF) { destination => - for { - paymentCalculations <- calculationService.paymentCalculation(goods) - rates <- calculationService.getConversionRates(goods) - } yield { - if(paymentCalculations.totalGbpValue.value > destination.threshold.value) - Redirect(routes.GoodsOverThresholdController.onPageLoad()) - else - Ok(view(paymentCalculations, rates, routes.CustomsAgentController.onPageLoad(), backButtonUrl)) - } + request.declarationJourney.goodsEntries.declarationGoodsIfComplete + .fold(actionProvider.invalidRequestF(goodsDeclarationIncompleteMessage)) { goods => + request.declarationJourney.maybeGoodsDestination + .fold(actionProvider.invalidRequestF(goodsDestinationUnansweredMessage)) { destination => + for { + paymentCalculations <- calculationService.paymentCalculation(goods) + rates <- calculationService.getConversionRates(goods) + } yield { + if (paymentCalculations.totalGbpValue.value > destination.threshold.value) + Redirect(routes.GoodsOverThresholdController.onPageLoad()) + else + Ok(view(paymentCalculations, rates, routes.CustomsAgentController.onPageLoad(), backButtonUrl)) + } + } } - } } } diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/PurchaseDetailsController.scala b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/PurchaseDetailsController.scala index 3605eaf19..3c8a6ca87 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/PurchaseDetailsController.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/PurchaseDetailsController.scala @@ -52,20 +52,18 @@ class PurchaseDetailsController @Inject()( def onSubmit(idx: Int): Action[AnyContent] = actionProvider.goodsAction(idx).async { implicit request: DeclarationGoodsRequest[AnyContent] => withGoodsCategory(request.goodsEntry) { category => connector.getCurrencies().flatMap { currencyPeriod => - form - .bindFromRequest() - .fold( - formWithErrors => - Future.successful(BadRequest(view(formWithErrors, idx, category, currencyPeriod.currencies, backButtonUrl(idx)))), - purchaseDetailsInput => - currencyPeriod.currencies.find(_.currencyCode == purchaseDetailsInput.currency) - .fold(actionProvider.invalidRequestF) { currency => - persistAndRedirect( - request.goodsEntry.copy(maybePurchaseDetails = Some(PurchaseDetails(purchaseDetailsInput.price, currency))), - idx, - routes.ReviewGoodsController.onPageLoad()) - } - ) + form.bindFromRequest().fold( + formWithErrors => + Future.successful(BadRequest(view(formWithErrors, idx, category, currencyPeriod.currencies, backButtonUrl(idx)))), + purchaseDetailsInput => + currencyPeriod.currencies.find(_.currencyCode == purchaseDetailsInput.currency) + .fold(actionProvider.invalidRequestF(s"currency [$purchaseDetailsInput.currency] not found")) { currency => + persistAndRedirect( + request.goodsEntry.copy(maybePurchaseDetails = Some(PurchaseDetails(purchaseDetailsInput.price, currency))), + idx, + routes.ReviewGoodsController.onPageLoad()) + } + ) } } } diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/ReviewGoodsController.scala b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/ReviewGoodsController.scala index ff3ae40d3..2a1cf528c 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/controllers/ReviewGoodsController.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/controllers/ReviewGoodsController.scala @@ -19,6 +19,7 @@ package uk.gov.hmrc.merchandiseinbaggage.controllers import javax.inject.{Inject, Singleton} import play.api.mvc.{Action, AnyContent, Call, MessagesControllerComponents} import uk.gov.hmrc.merchandiseinbaggage.config.AppConfig +import uk.gov.hmrc.merchandiseinbaggage.controllers.DeclarationJourneyController.goodsDeclarationIncompleteMessage import uk.gov.hmrc.merchandiseinbaggage.forms.ReviewGoodsForm.form import uk.gov.hmrc.merchandiseinbaggage.model.core.YesNo._ import uk.gov.hmrc.merchandiseinbaggage.model.core.{GoodsEntries, GoodsEntry} @@ -39,33 +40,26 @@ class ReviewGoodsController @Inject()(override val controllerComponents: Message routes.PurchaseDetailsController.onPageLoad(request.declarationJourney.goodsEntries.entries.size) val onPageLoad: Action[AnyContent] = actionProvider.journeyAction { implicit request => - request.declarationJourney.goodsEntries.declarationGoodsIfComplete.fold(actionProvider.invalidRequest) { goods => - Ok(view(form, goods, backButtonUrl)) - } + request.declarationJourney.goodsEntries.declarationGoodsIfComplete + .fold(actionProvider.invalidRequest(goodsDeclarationIncompleteMessage)) { goods => + Ok(view(form, goods, backButtonUrl)) + } } val onSubmit: Action[AnyContent] = actionProvider.journeyAction.async { implicit request => - request.declarationJourney.goodsEntries.declarationGoodsIfComplete.fold(actionProvider.invalidRequestF) { goods => - form - .bindFromRequest() - .fold( + request.declarationJourney.goodsEntries.declarationGoodsIfComplete + .fold(actionProvider.invalidRequestF(goodsDeclarationIncompleteMessage)) { goods => + form.bindFromRequest().fold( formWithErrors => Future.successful(BadRequest(view(formWithErrors, goods, backButtonUrl))), declareMoreGoods => if (declareMoreGoods == Yes) { val updatedGoodsEntries = request.declarationJourney.goodsEntries.entries :+ GoodsEntry.empty - repo.upsert( - request.declarationJourney.copy( - goodsEntries = GoodsEntries(updatedGoodsEntries) - ) - ).map { _ => + repo.upsert(request.declarationJourney.copy(goodsEntries = GoodsEntries(updatedGoodsEntries))).map { _ => Redirect(routes.GoodsTypeQuantityController.onPageLoad(updatedGoodsEntries.size)) } - } - else { - Future.successful(Redirect(routes.PaymentCalculationController.onPageLoad())) - } + } else Future.successful(Redirect(routes.PaymentCalculationController.onPageLoad())) ) - } + } } } diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/model/adresslookup/Address.scala b/app/uk/gov/hmrc/merchandiseinbaggage/model/adresslookup/Address.scala index 77d9c9b31..acd55990e 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/model/adresslookup/Address.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/model/adresslookup/Address.scala @@ -17,15 +17,19 @@ package uk.gov.hmrc.merchandiseinbaggage.model.adresslookup import play.api.libs.json._ +import uk.gov.hmrc.merchandiseinbaggage.utils.Obfuscator.{maybeObfuscate, obfuscate} -case class Country(code: String, name: Option[String]) +case class Country(code: String, name: Option[String]) { + lazy val obfuscated: Country = Country(obfuscate(code), maybeObfuscate(name)) +} -case class Address(lines: Seq[String], postcode: Option[String], country: Country) +case class Address(lines: Seq[String], postcode: Option[String], country: Country) { + lazy val obfuscated: Address = + Address(lines.map(line => obfuscate(line)), maybeObfuscate(postcode), country.obfuscated) +} object Address { - implicit val formatCountry: OFormat[Country] = Json.format[Country] - implicit val formatAddressLookupAddress: OFormat[Address] = - Json.format[Address] + implicit val formatAddressLookupAddress: OFormat[Address] = Json.format[Address] } diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/model/core/SessionId.scala b/app/uk/gov/hmrc/merchandiseinbaggage/model/core/SessionId.scala index d9b96707a..f164c758a 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/model/core/SessionId.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/model/core/SessionId.scala @@ -26,10 +26,11 @@ import play.api.libs.functional.syntax._ import play.api.libs.json.{Format, Json, OFormat} import uk.gov.hmrc.merchandiseinbaggage.model.Enum import uk.gov.hmrc.merchandiseinbaggage.model.adresslookup.Address -import uk.gov.hmrc.merchandiseinbaggage.model.api.{Declaration, JourneyDetails, JourneyInSmallVehicle, JourneyOnFootViaVehiclePort, JourneyViaFootPassengerOnlyPort, PurchaseDetails} +import uk.gov.hmrc.merchandiseinbaggage.model.api._ import uk.gov.hmrc.merchandiseinbaggage.model.calculation.CalculationRequest import uk.gov.hmrc.merchandiseinbaggage.model.core.GoodsDestinations.{GreatBritain, NorthernIreland} import uk.gov.hmrc.merchandiseinbaggage.model.core.YesNo.{No, Yes} +import uk.gov.hmrc.merchandiseinbaggage.utils.Obfuscator.{maybeObfuscate, obfuscate} import scala.collection.immutable @@ -108,13 +109,17 @@ object GoodsEntries { case class Name(firstName: String, lastName: String) { override val toString: String = s"$firstName $lastName" + + lazy val obfuscated: Name = Name(obfuscate(firstName), obfuscate(lastName)) } object Name { implicit val format: OFormat[Name] = Json.format[Name] } -case class Email(email: String, confirmation: String) +case class Email(email: String, confirmation: String) { + lazy val obfuscated: Email = Email(obfuscate(email), obfuscate(confirmation)) +} object Email { implicit val format: OFormat[Email] = Json.format[Email] @@ -122,6 +127,8 @@ object Email { case class Eori(value: String) { override val toString: String = value + + lazy val obfuscated: Eori = Eori(obfuscate(value)) } object Eori { @@ -154,6 +161,15 @@ case class DeclarationJourney(sessionId: SessionId, maybeRegistrationNumber: Option[String] = None, declarationId: Option[DeclarationId] = None ) { + lazy val obfuscated: DeclarationJourney = + this.copy( + maybeNameOfPersonCarryingTheGoods = maybeNameOfPersonCarryingTheGoods.map(_.obfuscated), + maybeEmailAddress = maybeEmailAddress.map(_.obfuscated), + maybeCustomsAgentName = maybeObfuscate(maybeCustomsAgentName), + maybeCustomsAgentAddress = maybeCustomsAgentAddress.map(_.obfuscated), + maybeEori = maybeEori.map(_.obfuscated), + maybeRegistrationNumber = maybeObfuscate(maybeRegistrationNumber) + ) val maybeCustomsAgent: Option[CustomsAgent] = for { diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/repositories/DeclarationJourneyRepository.scala b/app/uk/gov/hmrc/merchandiseinbaggage/repositories/DeclarationJourneyRepository.scala index 6d87ea59c..6d727cfc8 100644 --- a/app/uk/gov/hmrc/merchandiseinbaggage/repositories/DeclarationJourneyRepository.scala +++ b/app/uk/gov/hmrc/merchandiseinbaggage/repositories/DeclarationJourneyRepository.scala @@ -56,7 +56,11 @@ class DeclarationJourneyRepository @Inject()(mongo: () => DB, @Named("declaratio find(query).map(_.headOption) } - def deleteAll(): Future[Unit] = super.removeAll().map(_ => ()) + def deleteAll(): Future[Unit] = { + logger.warn("DeclarationJourneyRepository.deleteAll() called" ) + + super.removeAll().map(_ => ()) + } implicit val jsObjectWriter: OWrites[JsObject] = new OWrites[JsObject] { override def writes(o: JsObject): JsObject = o diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/utils/DeclarationJourneyLogger.scala b/app/uk/gov/hmrc/merchandiseinbaggage/utils/DeclarationJourneyLogger.scala new file mode 100644 index 000000000..613cadc40 --- /dev/null +++ b/app/uk/gov/hmrc/merchandiseinbaggage/utils/DeclarationJourneyLogger.scala @@ -0,0 +1,76 @@ +/* + * Copyright 2020 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 uk.gov.hmrc.merchandiseinbaggage.utils + +import play.api.Logger +import play.api.libs.json.Json.{prettyPrint, toJson} +import play.api.mvc.{Request, RequestHeader} +import uk.gov.hmrc.http.CookieNames.deviceID +import uk.gov.hmrc.http.HeaderCarrier +import uk.gov.hmrc.merchandiseinbaggage.controllers.DeclarationJourneyRequest +import uk.gov.hmrc.play.HeaderCarrierConverter +import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendHeaderCarrierProvider + +object DeclarationJourneyLogger { + private val logger: Logger = Logger("DeclarationJourneyLogger") + + def info(message: => String)(implicit request: RequestHeader): Unit = logMessage(message, Info) + + def warn(message: => String)(implicit request: RequestHeader): Unit = logMessage(message, Warn) + + private sealed trait LogLevel + + private case object Info extends LogLevel + + private case object Warn extends LogLevel + + def logMessage(message: => String, level: LogLevel)(implicit request: RequestHeader): Unit = { + lazy val richMessage = makeRichMessage(message) + level match { + case Info => logger.info(richMessage) + case Warn => logger.warn(richMessage) + } + } + + def headerCarrier(implicit request: Request[_]): HeaderCarrier = HcProvider.headerCarrier + + private def sessionId(implicit r: RequestHeader) = { + val hc = r match { + case r: Request[_] => headerCarrier(r) + case r => HeaderCarrierConverter.fromHeadersAndSession(r.headers) + } + s"sessionId: [${hc.sessionId.map(_.value).getOrElse("")}]" + } + + private def deviceId(implicit r: RequestHeader) = s"deviceId: [${r.cookies.find(_.name == deviceID).map(_.value).getOrElse("")}]" + + private def context(implicit r: RequestHeader) = s"context: [${r.method} ${r.path}]] $sessionId $deviceId" + + private def obfuscatedDeclarationJourney(declarationJourneyRequest: DeclarationJourneyRequest[_]) = + s"declarationJourney: [${prettyPrint(toJson(declarationJourneyRequest.declarationJourney.obfuscated))}]" + + private def makeRichMessage(message: String)(implicit request: RequestHeader): String = request match { + case declarationJourneyRequest: DeclarationJourneyRequest[_] => + s"$message ${obfuscatedDeclarationJourney(declarationJourneyRequest)} $context" + case r => + s"$message declarationJourney: [] $context" + } +} + +private object HcProvider extends FrontendHeaderCarrierProvider { + def headerCarrier(implicit request: Request[_]): HeaderCarrier = hc(request) +} \ No newline at end of file diff --git a/app/uk/gov/hmrc/merchandiseinbaggage/utils/Obfuscator.scala b/app/uk/gov/hmrc/merchandiseinbaggage/utils/Obfuscator.scala new file mode 100644 index 000000000..c33137680 --- /dev/null +++ b/app/uk/gov/hmrc/merchandiseinbaggage/utils/Obfuscator.scala @@ -0,0 +1,23 @@ +/* + * Copyright 2020 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 uk.gov.hmrc.merchandiseinbaggage.utils + +object Obfuscator { + def obfuscate(string: String): String = string.flatMap(_=> "*") + + def maybeObfuscate(maybeString: Option[String]): Option[String] = maybeString.map(string => obfuscate(string)) +} diff --git a/test/uk/gov/hmrc/merchandiseinbaggage/model/core/DeclarationSpec.scala b/test/uk/gov/hmrc/merchandiseinbaggage/model/core/DeclarationSpec.scala index 3c9343d72..a71b6cb49 100644 --- a/test/uk/gov/hmrc/merchandiseinbaggage/model/core/DeclarationSpec.scala +++ b/test/uk/gov/hmrc/merchandiseinbaggage/model/core/DeclarationSpec.scala @@ -26,6 +26,7 @@ import uk.gov.hmrc.merchandiseinbaggage.model.core.YesNo.{No, Yes} import uk.gov.hmrc.merchandiseinbaggage.model.currencyconversion.Currency import uk.gov.hmrc.merchandiseinbaggage.{BaseSpec, CoreTestData} import Declaration._ +import uk.gov.hmrc.merchandiseinbaggage.model.adresslookup.{Address, Country} class DeclarationSpec extends BaseSpec with CoreTestData { private val completedNonCustomsAgentJourney = completedDeclarationJourney.copy(maybeIsACustomsAgent = Some(No)) @@ -142,6 +143,20 @@ class DeclarationSpec extends BaseSpec with CoreTestData { parse(toJson(completedDeclarationJourney).toString()).validate[DeclarationJourney].get mustBe completedDeclarationJourney } + "be obfuscated" in { + completedDeclarationJourney.obfuscated.maybeNameOfPersonCarryingTheGoods mustBe Some(Name("*****", "****")) + completedDeclarationJourney.obfuscated.maybeEmailAddress mustBe Some(Email("***********", "***********")) + completedDeclarationJourney.obfuscated.maybeCustomsAgentName mustBe Some("**********") + completedDeclarationJourney.obfuscated.maybeCustomsAgentAddress mustBe + Some( + Address( + lines = Seq("*************", "**********"), + postcode = Some("*******"), + country = Country("**", Some("**************")))) + completedDeclarationJourney.obfuscated.maybeEori mustBe Some(Eori("**************")) + completedDeclarationJourney.obfuscated.maybeRegistrationNumber mustBe Some("******") + } + "have a customs agent" when { "the user has provided all the required details" in { completedDeclarationJourney.maybeCustomsAgent mustBe diff --git a/test/uk/gov/hmrc/merchandiseinbaggage/utils/ObfuscatorSpec.scala b/test/uk/gov/hmrc/merchandiseinbaggage/utils/ObfuscatorSpec.scala new file mode 100644 index 000000000..a642b1ae0 --- /dev/null +++ b/test/uk/gov/hmrc/merchandiseinbaggage/utils/ObfuscatorSpec.scala @@ -0,0 +1,35 @@ +/* + * Copyright 2020 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 uk.gov.hmrc.merchandiseinbaggage.utils + +import uk.gov.hmrc.merchandiseinbaggage.BaseSpec +import uk.gov.hmrc.merchandiseinbaggage.utils.Obfuscator.obfuscate + +class ObfuscatorSpec extends BaseSpec { + "obfuscate should" should { + "return a string of `*` characters of the same length as the input" in { + obfuscate("1") mustBe "*" + obfuscate("12") mustBe "**" + } + + "return an empty string" when { + "the input is empty" in { + obfuscate("") mustBe "" + } + } + } +}