From 6b36e41e75995ceb0e5b7ffd295e340ea951e318 Mon Sep 17 00:00:00 2001 From: Chris Cook <114592674+ChrisCookOC@users.noreply.github.com> Date: Mon, 13 May 2024 12:08:46 +0100 Subject: [PATCH 1/4] Auth to get enrolment --- app/config/FrontendAppConfig.scala | 5 +- .../actions/IdentifierAction.scala | 47 ++++++----- app/models/EnrolmentConfig.scala | 32 ++++++++ app/models/requests/IdentifierRequest.scala | 2 +- app/views/UnauthorisedView.scala.html | 2 +- conf/application.conf | 5 ++ .../CategoryGuidanceControllerSpec.scala | 16 ++++ .../CommodityCodeControllerSpec.scala | 16 ++++ .../CountryOfOriginControllerSpec.scala | 16 ++++ test/controllers/HomePageControllerSpec.scala | 16 ++++ test/controllers/actions/AuthActionSpec.scala | 4 +- .../actions/DataRetrievalActionSpec.scala | 4 +- .../actions/FakeIdentifierAction.scala | 2 +- .../actions/SessionActionSpec.scala | 77 ------------------- .../forms/CommodityCodeFormProviderSpec.scala | 16 ++++ .../CountryOfOriginFormProviderSpec.scala | 16 ++++ 16 files changed, 166 insertions(+), 110 deletions(-) create mode 100644 app/models/EnrolmentConfig.scala delete mode 100644 test/controllers/actions/SessionActionSpec.scala diff --git a/app/config/FrontendAppConfig.scala b/app/config/FrontendAppConfig.scala index db339d8db..b0213b5d5 100644 --- a/app/config/FrontendAppConfig.scala +++ b/app/config/FrontendAppConfig.scala @@ -17,10 +17,10 @@ package config import com.google.inject.{Inject, Singleton} +import models.EnrolmentConfig import play.api.Configuration import play.api.i18n.Lang import play.api.mvc.RequestHeader - import uk.gov.hmrc.http.StringContextOps @Singleton class FrontendAppConfig @Inject() (configuration: Configuration) { @@ -53,4 +53,7 @@ class FrontendAppConfig @Inject() (configuration: Configuration) { val countdown: Int = configuration.get[Int]("timeout-dialog.countdown") val cacheTtl: Int = configuration.get[Int]("mongodb.timeToLiveInSeconds") + + val tgpEnrolmentIdentifier: EnrolmentConfig = configuration.get[EnrolmentConfig]("enrolment-config") + } diff --git a/app/controllers/actions/IdentifierAction.scala b/app/controllers/actions/IdentifierAction.scala index cd0ab0c86..76f618fad 100644 --- a/app/controllers/actions/IdentifierAction.scala +++ b/app/controllers/actions/IdentifierAction.scala @@ -20,11 +20,13 @@ import com.google.inject.Inject import config.FrontendAppConfig import controllers.routes import models.requests.IdentifierRequest +import play.api.Logging import play.api.mvc.Results._ import play.api.mvc._ import uk.gov.hmrc.auth.core._ import uk.gov.hmrc.auth.core.retrieve.v2.Retrievals -import uk.gov.hmrc.http.{HeaderCarrier, UnauthorizedException} +import uk.gov.hmrc.auth.core.retrieve.~ +import uk.gov.hmrc.http.HeaderCarrier import uk.gov.hmrc.play.http.HeaderCarrierConverter import scala.concurrent.{ExecutionContext, Future} @@ -36,39 +38,34 @@ class AuthenticatedIdentifierAction @Inject()( config: FrontendAppConfig, val parser: BodyParsers.Default ) - (implicit val executionContext: ExecutionContext) extends IdentifierAction with AuthorisedFunctions { + (implicit val executionContext: ExecutionContext) + extends IdentifierAction + with AuthorisedFunctions + with Logging { override def invokeBlock[A](request: Request[A], block: IdentifierRequest[A] => Future[Result]): Future[Result] = { implicit val hc: HeaderCarrier = HeaderCarrierConverter.fromRequestAndSession(request, request.session) - authorised().retrieve(Retrievals.internalId) { - _.map { - internalId => block(IdentifierRequest(request, internalId)) - }.getOrElse(throw new UnauthorizedException("Unable to retrieve internal Id")) - } recover { + authorised(Enrolment(config.tgpEnrolmentIdentifier.key)) + .retrieve(Retrievals.internalId and Retrievals.authorisedEnrolments) { + x => {println("CCCCC " + x) + x match { + case Some(internalId) ~ authorisedEnrolments => + authorisedEnrolments + .getEnrolment(config.tgpEnrolmentIdentifier.key) + .flatMap(_.getIdentifier(config.tgpEnrolmentIdentifier.identifier)) match { + case Some(enrolment) => + block(IdentifierRequest(request, internalId, enrolment.value)) + case None => throw InsufficientEnrolments("Unable to retrieve Enrolment") + } + }}} recover { case _: NoActiveSession => + logger.info(s"No Active Session. Redirect to $config.loginContinueUrl") Redirect(config.loginUrl, Map("continue" -> Seq(config.loginContinueUrl))) case _: AuthorisationException => + logger.info("Authorisation failure: No enrolments found for TGP. Redirecting to UnauthorisedController") Redirect(routes.UnauthorisedController.onPageLoad) } } } - -class SessionIdentifierAction @Inject()( - val parser: BodyParsers.Default - ) - (implicit val executionContext: ExecutionContext) extends IdentifierAction { - - override def invokeBlock[A](request: Request[A], block: IdentifierRequest[A] => Future[Result]): Future[Result] = { - - implicit val hc: HeaderCarrier = HeaderCarrierConverter.fromRequestAndSession(request, request.session) - - hc.sessionId match { - case Some(session) => - block(IdentifierRequest(request, session.value)) - case None => - Future.successful(Redirect(routes.JourneyRecoveryController.onPageLoad())) - } - } -} diff --git a/app/models/EnrolmentConfig.scala b/app/models/EnrolmentConfig.scala new file mode 100644 index 000000000..8ddb03b97 --- /dev/null +++ b/app/models/EnrolmentConfig.scala @@ -0,0 +1,32 @@ +/* + * 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 models + +import play.api.{ConfigLoader, Configuration} + +case class EnrolmentConfig(key: String, identifier: String) + +object EnrolmentConfig { + + implicit lazy val configLoader: ConfigLoader[EnrolmentConfig] = ConfigLoader { config => prefix => + val enrolmentConfig = Configuration(config).get[Configuration](prefix) + val key = enrolmentConfig.get[String]("enrolment-key") + val id = enrolmentConfig.get[String]("enrolment-identifier") + + EnrolmentConfig(key, id) + } +} diff --git a/app/models/requests/IdentifierRequest.scala b/app/models/requests/IdentifierRequest.scala index dbbbc9f64..2b7d98e29 100644 --- a/app/models/requests/IdentifierRequest.scala +++ b/app/models/requests/IdentifierRequest.scala @@ -18,4 +18,4 @@ package models.requests import play.api.mvc.{Request, WrappedRequest} -case class IdentifierRequest[A] (request: Request[A], userId: String) extends WrappedRequest[A](request) +case class IdentifierRequest[A] (request: Request[A], userId: String, eori: String) extends WrappedRequest[A](request) diff --git a/app/views/UnauthorisedView.scala.html b/app/views/UnauthorisedView.scala.html index ba9a424d3..98528a873 100644 --- a/app/views/UnauthorisedView.scala.html +++ b/app/views/UnauthorisedView.scala.html @@ -29,5 +29,5 @@
@messages("unauthorised.p1")
-@messages("unauthorised.p2.part1") @messages("unauthorised.p2.linkText") @messages("unauthorised.p2.part2")
+@messages("unauthorised.p2.part1") @messages("unauthorised.p2.linkText") @messages("unauthorised.p2.part2")
} diff --git a/conf/application.conf b/conf/application.conf index e671f1f93..4407d6d20 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -84,3 +84,8 @@ tracking-consent-frontend { features { welsh-translation: true } + +enrolment-config { + enrolment-key = "HMRC-CUS-ORG" + enrolment-identifier = "EORINumber" +} diff --git a/test/controllers/CategoryGuidanceControllerSpec.scala b/test/controllers/CategoryGuidanceControllerSpec.scala index e7c872a69..5a2e1815f 100644 --- a/test/controllers/CategoryGuidanceControllerSpec.scala +++ b/test/controllers/CategoryGuidanceControllerSpec.scala @@ -1,3 +1,19 @@ +/* + * 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 controllers import base.SpecBase diff --git a/test/controllers/CommodityCodeControllerSpec.scala b/test/controllers/CommodityCodeControllerSpec.scala index 7d38953fa..44089e356 100644 --- a/test/controllers/CommodityCodeControllerSpec.scala +++ b/test/controllers/CommodityCodeControllerSpec.scala @@ -1,3 +1,19 @@ +/* + * 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 controllers import base.SpecBase diff --git a/test/controllers/CountryOfOriginControllerSpec.scala b/test/controllers/CountryOfOriginControllerSpec.scala index 9ff3fed20..3ab732f2c 100644 --- a/test/controllers/CountryOfOriginControllerSpec.scala +++ b/test/controllers/CountryOfOriginControllerSpec.scala @@ -1,3 +1,19 @@ +/* + * 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 controllers import base.SpecBase diff --git a/test/controllers/HomePageControllerSpec.scala b/test/controllers/HomePageControllerSpec.scala index 2868a9e53..a53523943 100644 --- a/test/controllers/HomePageControllerSpec.scala +++ b/test/controllers/HomePageControllerSpec.scala @@ -1,3 +1,19 @@ +/* + * 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 controllers import base.SpecBase diff --git a/test/controllers/actions/AuthActionSpec.scala b/test/controllers/actions/AuthActionSpec.scala index ce8a79da5..5cf6b6926 100644 --- a/test/controllers/actions/AuthActionSpec.scala +++ b/test/controllers/actions/AuthActionSpec.scala @@ -20,7 +20,7 @@ import base.SpecBase import com.google.inject.Inject import config.FrontendAppConfig import controllers.routes -import play.api.mvc.{BodyParsers, Results} +import play.api.mvc.{Action, AnyContent, BodyParsers, Results} import play.api.test.FakeRequest import play.api.test.Helpers._ import uk.gov.hmrc.auth.core._ @@ -34,7 +34,7 @@ import scala.concurrent.{ExecutionContext, Future} class AuthActionSpec extends SpecBase { class Harness(authAction: IdentifierAction) { - def onPageLoad() = authAction { _ => Results.Ok } + def onPageLoad(): Action[AnyContent] = authAction { _ => Results.Ok } } "Auth Action" - { diff --git a/test/controllers/actions/DataRetrievalActionSpec.scala b/test/controllers/actions/DataRetrievalActionSpec.scala index 9602c56ea..693cab094 100644 --- a/test/controllers/actions/DataRetrievalActionSpec.scala +++ b/test/controllers/actions/DataRetrievalActionSpec.scala @@ -43,7 +43,7 @@ class DataRetrievalActionSpec extends SpecBase with MockitoSugar { when(sessionRepository.get("id")) thenReturn Future(None) val action = new Harness(sessionRepository) - val result = action.callTransform(IdentifierRequest(FakeRequest(), "id")).futureValue + val result = action.callTransform(IdentifierRequest(FakeRequest(), "id", "eori")).futureValue result.userAnswers must not be defined } @@ -57,7 +57,7 @@ class DataRetrievalActionSpec extends SpecBase with MockitoSugar { when(sessionRepository.get("id")) thenReturn Future(Some(UserAnswers("id"))) val action = new Harness(sessionRepository) - val result = action.callTransform(new IdentifierRequest(FakeRequest(), "id")).futureValue + val result = action.callTransform(new IdentifierRequest(FakeRequest(), "id", "eori")).futureValue result.userAnswers mustBe defined } diff --git a/test/controllers/actions/FakeIdentifierAction.scala b/test/controllers/actions/FakeIdentifierAction.scala index 09b5380f7..04a22a2d7 100644 --- a/test/controllers/actions/FakeIdentifierAction.scala +++ b/test/controllers/actions/FakeIdentifierAction.scala @@ -25,7 +25,7 @@ import scala.concurrent.{ExecutionContext, Future} class FakeIdentifierAction @Inject()(bodyParsers: PlayBodyParsers) extends IdentifierAction { override def invokeBlock[A](request: Request[A], block: IdentifierRequest[A] => Future[Result]): Future[Result] = - block(IdentifierRequest(request, "id")) + block(IdentifierRequest(request, "id", "eori")) override def parser: BodyParser[AnyContent] = bodyParsers.default diff --git a/test/controllers/actions/SessionActionSpec.scala b/test/controllers/actions/SessionActionSpec.scala deleted file mode 100644 index 991427622..000000000 --- a/test/controllers/actions/SessionActionSpec.scala +++ /dev/null @@ -1,77 +0,0 @@ -/* - * 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 controllers.actions - -import base.SpecBase -import play.api.mvc.{Action, AnyContent, BodyParsers, Results} -import play.api.test.FakeRequest -import play.api.test.Helpers._ -import uk.gov.hmrc.http.SessionKeys - -import scala.concurrent.ExecutionContext.Implicits.global - -class SessionActionSpec extends SpecBase { - - class Harness(action: IdentifierAction) { - def onPageLoad(): Action[AnyContent] = action { _ => Results.Ok } - } - - "Session Action" - { - - "when there is no active session" - { - - "must redirect to the session expired page" in { - - val application = applicationBuilder(userAnswers = None).build() - - running(application){ - val bodyParsers = application.injector.instanceOf[BodyParsers.Default] - - val sessionAction = new SessionIdentifierAction(bodyParsers) - - val controller = new Harness(sessionAction) - - val result = controller.onPageLoad()(FakeRequest()) - - status(result) mustBe SEE_OTHER - redirectLocation(result).value must startWith(controllers.routes.JourneyRecoveryController.onPageLoad().url) - } - } - } - - "when there is an active session" - { - - "must perform the action" in { - - val application = applicationBuilder(userAnswers = None).build() - - running(application) { - val bodyParsers = application.injector.instanceOf[BodyParsers.Default] - - val sessionAction = new SessionIdentifierAction(bodyParsers) - - val controller = new Harness(sessionAction) - - val request = FakeRequest().withSession(SessionKeys.sessionId -> "foo") - - val result = controller.onPageLoad()(request) - status(result) mustBe OK - } - } - } - } -} diff --git a/test/forms/CommodityCodeFormProviderSpec.scala b/test/forms/CommodityCodeFormProviderSpec.scala index 6502aeb41..36262abd0 100644 --- a/test/forms/CommodityCodeFormProviderSpec.scala +++ b/test/forms/CommodityCodeFormProviderSpec.scala @@ -1,3 +1,19 @@ +/* + * 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 forms import forms.behaviours.StringFieldBehaviours diff --git a/test/forms/CountryOfOriginFormProviderSpec.scala b/test/forms/CountryOfOriginFormProviderSpec.scala index 49633a68c..04af7672e 100644 --- a/test/forms/CountryOfOriginFormProviderSpec.scala +++ b/test/forms/CountryOfOriginFormProviderSpec.scala @@ -1,3 +1,19 @@ +/* + * 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 forms import forms.behaviours.StringFieldBehaviours From e082686ddad42b6605df8102c486ddd07961318a Mon Sep 17 00:00:00 2001 From: Chris Cook <114592674+ChrisCookOC@users.noreply.github.com> Date: Mon, 13 May 2024 12:11:20 +0100 Subject: [PATCH 2/4] Remove debug code --- app/controllers/actions/IdentifierAction.scala | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/controllers/actions/IdentifierAction.scala b/app/controllers/actions/IdentifierAction.scala index 76f618fad..299f05b4e 100644 --- a/app/controllers/actions/IdentifierAction.scala +++ b/app/controllers/actions/IdentifierAction.scala @@ -49,8 +49,6 @@ class AuthenticatedIdentifierAction @Inject()( authorised(Enrolment(config.tgpEnrolmentIdentifier.key)) .retrieve(Retrievals.internalId and Retrievals.authorisedEnrolments) { - x => {println("CCCCC " + x) - x match { case Some(internalId) ~ authorisedEnrolments => authorisedEnrolments .getEnrolment(config.tgpEnrolmentIdentifier.key) @@ -59,7 +57,7 @@ class AuthenticatedIdentifierAction @Inject()( block(IdentifierRequest(request, internalId, enrolment.value)) case None => throw InsufficientEnrolments("Unable to retrieve Enrolment") } - }}} recover { + } recover { case _: NoActiveSession => logger.info(s"No Active Session. Redirect to $config.loginContinueUrl") Redirect(config.loginUrl, Map("continue" -> Seq(config.loginContinueUrl))) From 61af9cbe5ba44df4fd12920726af269a74383ea9 Mon Sep 17 00:00:00 2001 From: Chris Cook <114592674+ChrisCookOC@users.noreply.github.com> Date: Mon, 13 May 2024 16:58:03 +0100 Subject: [PATCH 3/4] Enforce agent affinity group --- app/controllers/actions/IdentifierAction.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/actions/IdentifierAction.scala b/app/controllers/actions/IdentifierAction.scala index 299f05b4e..f45c9c709 100644 --- a/app/controllers/actions/IdentifierAction.scala +++ b/app/controllers/actions/IdentifierAction.scala @@ -47,7 +47,9 @@ class AuthenticatedIdentifierAction @Inject()( implicit val hc: HeaderCarrier = HeaderCarrierConverter.fromRequestAndSession(request, request.session) - authorised(Enrolment(config.tgpEnrolmentIdentifier.key)) + val predicates = Enrolment(config.tgpEnrolmentIdentifier.key) and AffinityGroup.Agent + + authorised(predicates) .retrieve(Retrievals.internalId and Retrievals.authorisedEnrolments) { case Some(internalId) ~ authorisedEnrolments => authorisedEnrolments From a201da5abe4174047882b3572f250523ee701424 Mon Sep 17 00:00:00 2001 From: Chris Cook <114592674+ChrisCookOC@users.noreply.github.com> Date: Mon, 13 May 2024 17:06:00 +0100 Subject: [PATCH 4/4] Enforce not agent affinity group --- app/controllers/actions/IdentifierAction.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/actions/IdentifierAction.scala b/app/controllers/actions/IdentifierAction.scala index f45c9c709..7cfbebe53 100644 --- a/app/controllers/actions/IdentifierAction.scala +++ b/app/controllers/actions/IdentifierAction.scala @@ -47,7 +47,7 @@ class AuthenticatedIdentifierAction @Inject()( implicit val hc: HeaderCarrier = HeaderCarrierConverter.fromRequestAndSession(request, request.session) - val predicates = Enrolment(config.tgpEnrolmentIdentifier.key) and AffinityGroup.Agent + val predicates = Enrolment(config.tgpEnrolmentIdentifier.key) and (AffinityGroup.Organisation or AffinityGroup.Individual) authorised(predicates) .retrieve(Retrievals.internalId and Retrievals.authorisedEnrolments) {