From a44e442d1d2dfc3ac186256292051d40e31c04c4 Mon Sep 17 00:00:00 2001 From: Riccardo Torsoli <122275960+nttdata-rtorsoli@users.noreply.github.com> Date: Thu, 20 Jun 2024 17:01:58 +0200 Subject: [PATCH] PIN-4985 BKE - Auth-process: rid of route GET -> /clients/{clientId}/users/{userId}/keys (#244) Co-authored-by: nttdata-rtorsoli --- .../resources/interface-specification.yml | 47 --------------- .../api/impl/UserApiHandlers.scala | 23 -------- .../api/impl/UserApiMarshallerImpl.scala | 13 ----- .../api/impl/UserApiServiceImpl.scala | 53 ----------------- .../server/impl/Dependencies.scala | 13 +---- .../server/impl/Main.scala | 9 +-- .../UserOperationSpec.scala | 57 +------------------ .../authz/UserpiAuthzSpec.scala | 44 -------------- .../authorizationprocess/util/SpecUtils.scala | 2 - 9 files changed, 5 insertions(+), 256 deletions(-) delete mode 100644 src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/UserApiHandlers.scala delete mode 100644 src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/UserApiMarshallerImpl.scala delete mode 100644 src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/UserApiServiceImpl.scala delete mode 100644 src/test/scala/it/pagopa/interop/authorizationprocess/authz/UserpiAuthzSpec.scala diff --git a/src/main/resources/interface-specification.yml b/src/main/resources/interface-specification.yml index 98e6dce4..ece7710b 100644 --- a/src/main/resources/interface-specification.yml +++ b/src/main/resources/interface-specification.yml @@ -22,11 +22,6 @@ tags: externalDocs: description: Find out more url: http://swagger.io - - name: user - description: Get security information - externalDocs: - description: Find out more - url: http://swagger.io - name: health description: Verify service status externalDocs: @@ -652,48 +647,6 @@ paths: application/problem+json: schema: $ref: '#/components/schemas/Problem' - '/clients/{clientId}/users/{userId}/keys': - parameters: - - $ref: '#/components/parameters/CorrelationIdHeader' - - name: clientId - in: path - description: ID of the client holding the key - required: true - schema: - type: string - format: uuid - - name: userId - in: path - required: true - description: ID of the User that the added keys MUST belong to - schema: - type: string - format: uuid - get: - tags: - - user - summary: Returns a set of keys by user ID and client ID. - description: 'Given an user and a client it returns its corresponding set of keys, if any' - operationId: getClientUserKeys - responses: - '200': - description: returns the corresponding array of keys - content: - application/json: - schema: - $ref: '#/components/schemas/Keys' - '401': - description: Unauthorized - content: - application/problem+json: - schema: - $ref: '#/components/schemas/Problem' - '404': - description: Client id not found - content: - application/problem+json: - schema: - $ref: '#/components/schemas/Problem' '/clients/{clientId}/purposes': parameters: - $ref: '#/components/parameters/CorrelationIdHeader' diff --git a/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/UserApiHandlers.scala b/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/UserApiHandlers.scala deleted file mode 100644 index 47108387..00000000 --- a/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/UserApiHandlers.scala +++ /dev/null @@ -1,23 +0,0 @@ -package it.pagopa.interop.authorizationprocess.api.impl - -import akka.http.scaladsl.server.Route -import com.typesafe.scalalogging.LoggerTakingImplicit -import it.pagopa.interop.authorizationprocess.error.AuthorizationProcessErrors.ClientNotFound -import it.pagopa.interop.commons.logging.ContextFieldsToLog -import it.pagopa.interop.commons.utils.errors.AkkaResponses - -import scala.util.{Failure, Success, Try} -import it.pagopa.interop.authorizationprocess.error.AuthorizationProcessErrors.OrganizationNotAllowedOnClient - -object UserApiHandlers extends AkkaResponses { - - def getClientUserKeysResponse[T](logMessage: String)( - success: T => Route - )(result: Try[T])(implicit contexts: Seq[(String, String)], logger: LoggerTakingImplicit[ContextFieldsToLog]): Route = - result match { - case Success(s) => success(s) - case Failure(ex: OrganizationNotAllowedOnClient) => forbidden(ex, logMessage) - case Failure(ex: ClientNotFound) => notFound(ex, logMessage) - case Failure(ex) => internalServerError(ex, logMessage) - } -} diff --git a/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/UserApiMarshallerImpl.scala b/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/UserApiMarshallerImpl.scala deleted file mode 100644 index d14dfb0c..00000000 --- a/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/UserApiMarshallerImpl.scala +++ /dev/null @@ -1,13 +0,0 @@ -package it.pagopa.interop.authorizationprocess.api.impl - -import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport -import akka.http.scaladsl.marshalling.ToEntityMarshaller -import it.pagopa.interop.authorizationprocess.api.UserApiMarshaller -import it.pagopa.interop.authorizationprocess.model._ - -case object UserApiMarshallerImpl extends UserApiMarshaller with SprayJsonSupport { - - override implicit def toEntityMarshallerProblem: ToEntityMarshaller[Problem] = entityMarshallerProblem - - override implicit def toEntityMarshallerKeys: ToEntityMarshaller[Keys] = sprayJsonMarshaller[Keys] -} diff --git a/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/UserApiServiceImpl.scala b/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/UserApiServiceImpl.scala deleted file mode 100644 index 8756cbff..00000000 --- a/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/UserApiServiceImpl.scala +++ /dev/null @@ -1,53 +0,0 @@ -package it.pagopa.interop.authorizationprocess.api.impl - -import akka.http.scaladsl.marshalling.ToEntityMarshaller -import akka.http.scaladsl.server.Directives.onComplete -import akka.http.scaladsl.server.Route -import cats.syntax.all._ -import com.typesafe.scalalogging.{Logger, LoggerTakingImplicit} -import it.pagopa.interop.authorizationprocess.api.UserApiService -import it.pagopa.interop.authorizationprocess.model._ -import it.pagopa.interop.authorizationprocess.service._ -import it.pagopa.interop.authorizationprocess.api.impl.UserApiHandlers._ -import it.pagopa.interop.commons.jwt.{ADMIN_ROLE, SECURITY_ROLE, SUPPORT_ROLE, authorize} -import it.pagopa.interop.commons.logging.{CanLogContextFields, ContextFieldsToLog} -import it.pagopa.interop.selfcare.v2.client.model.{Problem => _} -import it.pagopa.interop.commons.utils.TypeConversions._ -import it.pagopa.interop.authorizationprocess.common.Adapters._ -import it.pagopa.interop.commons.cqrs.service.ReadModelService -import it.pagopa.interop.authorizationprocess.common.AuthorizationUtils._ - -import scala.concurrent.{ExecutionContext, Future} - -final case class UserApiServiceImpl( - authorizationManagementService: AuthorizationManagementService, - selfcareV2Service: SelfcareV2Service -)(implicit ec: ExecutionContext, readModel: ReadModelService) - extends UserApiService { - - implicit val logger: LoggerTakingImplicit[ContextFieldsToLog] = - Logger.takingImplicit[ContextFieldsToLog](this.getClass) - - override def getClientUserKeys(clientId: String, userId: String)(implicit - contexts: Seq[(String, String)], - toEntityMarshallerKeys: ToEntityMarshaller[Keys], - toEntityMarshallerProblem: ToEntityMarshaller[Problem] - ): Route = authorize(ADMIN_ROLE, SECURITY_ROLE, SUPPORT_ROLE) { - val operationLabel: String = s"Getting client keys $clientId for user $userId" - logger.info(operationLabel) - - val result: Future[Keys] = for { - clientUuid <- clientId.toFutureUUID - client <- authorizationManagementService.getClient(clientUuid) - _ <- assertIsClientConsumer(client).toFuture - userUuid <- userId.toFutureUUID - clientKeys <- authorizationManagementService.getClientKeys(client.id) - apiKeys = clientKeys.filter(userUuid.some == _.userId).map(_.toApi) - keys <- apiKeys.traverse(_.toFuture) - } yield Keys(keys = keys) - - onComplete(result) { - getClientUserKeysResponse[Keys](operationLabel)(getClientUserKeys200) - } - } -} diff --git a/src/main/scala/it/pagopa/interop/authorizationprocess/server/impl/Dependencies.scala b/src/main/scala/it/pagopa/interop/authorizationprocess/server/impl/Dependencies.scala index 9bf03679..c7a9dbdb 100644 --- a/src/main/scala/it/pagopa/interop/authorizationprocess/server/impl/Dependencies.scala +++ b/src/main/scala/it/pagopa/interop/authorizationprocess/server/impl/Dependencies.scala @@ -19,9 +19,7 @@ import it.pagopa.interop.authorizationprocess.api.impl.{ HealthApiMarshallerImpl, HealthServiceApiImpl } -import it.pagopa.interop.authorizationprocess.api.impl.UserApiServiceImpl -import it.pagopa.interop.authorizationprocess.api.impl.UserApiMarshallerImpl -import it.pagopa.interop.authorizationprocess.api.{ClientApi, HealthApi, UserApi} +import it.pagopa.interop.authorizationprocess.api.{ClientApi, HealthApi} import it.pagopa.interop.selfcare.v2.client.api.{InstitutionsApi, UsersApi} import it.pagopa.interop.authorizationprocess.common.system.ApplicationConfiguration import it.pagopa.interop.authorizationprocess.api.impl.serviceCode @@ -97,15 +95,6 @@ trait Dependencies { jwtReader.OAuth2JWTValidatorAsContexts ) - def userApi(jwtReader: JWTReader, blockingEc: ExecutionContextExecutor)(implicit - actorSystem: ActorSystem[_], - ec: ExecutionContext - ): UserApi = new UserApi( - UserApiServiceImpl(authorizationManagementService(blockingEc), selfcareV2Service()), - UserApiMarshallerImpl, - jwtReader.OAuth2JWTValidatorAsContexts(Logger.takingImplicit[ContextFieldsToLog]("OAuth2JWTValidatorAsContexts")) - ) - def getJwtValidator(): Future[JWTReader] = JWTConfiguration.jwtReader .loadKeyset() .map(keyset => diff --git a/src/main/scala/it/pagopa/interop/authorizationprocess/server/impl/Main.scala b/src/main/scala/it/pagopa/interop/authorizationprocess/server/impl/Main.scala index cca55658..d6b55461 100644 --- a/src/main/scala/it/pagopa/interop/authorizationprocess/server/impl/Main.scala +++ b/src/main/scala/it/pagopa/interop/authorizationprocess/server/impl/Main.scala @@ -32,12 +32,9 @@ object Main extends App with CORSSupport with Dependencies { val serverBinding = for { jwtReader <- getJwtValidator() - controller = new Controller( - clientApi(jwtReader, blockingEc), - healthApi, - userApi(jwtReader, blockingEc), - validationExceptionToRoute.some - )(actorSystem.classicSystem) + controller = new Controller(clientApi(jwtReader, blockingEc), healthApi, validationExceptionToRoute.some)( + actorSystem.classicSystem + ) binding <- Http()(actorSystem.classicSystem) .newServerAt("0.0.0.0", ApplicationConfiguration.serverPort) .bind(corsHandler(controller.routes)) diff --git a/src/test/scala/it/pagopa/interop/authorizationprocess/UserOperationSpec.scala b/src/test/scala/it/pagopa/interop/authorizationprocess/UserOperationSpec.scala index e9f33e05..4a62649d 100644 --- a/src/test/scala/it/pagopa/interop/authorizationprocess/UserOperationSpec.scala +++ b/src/test/scala/it/pagopa/interop/authorizationprocess/UserOperationSpec.scala @@ -3,7 +3,7 @@ package it.pagopa.interop.authorizationprocess import cats.syntax.all._ import akka.http.scaladsl.model.StatusCodes import akka.http.scaladsl.testkit.ScalatestRouteTest -import it.pagopa.interop.authorizationprocess.api.impl.{ClientApiServiceImpl, UserApiServiceImpl} +import it.pagopa.interop.authorizationprocess.api.impl.ClientApiServiceImpl import it.pagopa.interop.authorizationprocess.api.impl.ClientApiMarshallerImpl._ import it.pagopa.interop.authorizationprocess.error.AuthorizationProcessErrors.{InstitutionNotFound, ClientNotFound} import it.pagopa.interop.authorizationprocess.model._ @@ -21,12 +21,6 @@ import scala.concurrent.{ExecutionContext, Future} class UserOperationSpec extends AnyWordSpecLike with MockFactory with SpecUtilsWithImplicit with ScalatestRouteTest { - val serviceUser: UserApiServiceImpl = - UserApiServiceImpl(mockAuthorizationManagementService, mockSelfcareV2Service)( - ExecutionContext.global, - mockReadModel - ) - val service: ClientApiServiceImpl = ClientApiServiceImpl( mockAuthorizationManagementService, mockAgreementManagementService, @@ -273,53 +267,4 @@ class UserOperationSpec extends AnyWordSpecLike with MockFactory with SpecUtilsW } } } - - "User retrieve keys" should { - "succeed" in { - - (mockAuthorizationManagementService - .getClient(_: UUID)(_: ExecutionContext, _: ReadModelService)) - .expects(persistentClient.id, *, *) - .once() - .returns(Future.successful(persistentClient.copy(users = Set(userId)))) - - (mockAuthorizationManagementService - .getClientKeys(_: UUID)(_: ExecutionContext, _: ReadModelService)) - .expects(persistentClient.id, *, *) - .once() - .returns(Future.successful(Seq(persistentKey))) - - Get() ~> serviceUser.getClientUserKeys(persistentClient.id.toString, userId.toString) ~> check { - status shouldEqual StatusCodes.OK - } - } - - "fail if the caller is not the client consumer" in { - - (mockAuthorizationManagementService - .getClient(_: UUID)(_: ExecutionContext, _: ReadModelService)) - .expects(persistentClient.id, *, *) - .once() - .returns(Future.successful(persistentClient.copy(consumerId = UUID.randomUUID()))) - - Get() ~> serviceUser.getClientUserKeys(persistentClient.id.toString, UUID.randomUUID().toString) ~> check { - status shouldEqual StatusCodes.Forbidden - responseAs[Problem].errors.head.code shouldEqual "007-0008" - } - } - - "fail if client does not exist" in { - (mockAuthorizationManagementService - .getClient(_: UUID)(_: ExecutionContext, _: ReadModelService)) - .expects(persistentClient.id, *, *) - .once() - .returns(Future.failed(ClientNotFound(persistentClient.id))) - - Get() ~> serviceUser.getClientUserKeys(persistentClient.id.toString, userId.toString) ~> check { - status shouldEqual StatusCodes.NotFound - responseAs[Problem].errors.head.code shouldEqual "007-0010" - } - } - - } } diff --git a/src/test/scala/it/pagopa/interop/authorizationprocess/authz/UserpiAuthzSpec.scala b/src/test/scala/it/pagopa/interop/authorizationprocess/authz/UserpiAuthzSpec.scala deleted file mode 100644 index c4a9aa14..00000000 --- a/src/test/scala/it/pagopa/interop/authorizationprocess/authz/UserpiAuthzSpec.scala +++ /dev/null @@ -1,44 +0,0 @@ -package it.pagopa.interop.authorizationprocess.authz - -import it.pagopa.interop.authorizationprocess.api.impl.UserApiMarshallerImpl._ -import it.pagopa.interop.authorizationprocess.api.impl.UserApiServiceImpl -import it.pagopa.interop.authorizationprocess.service._ -import it.pagopa.interop.authorizationprocess.util.FakeDependencies._ -import it.pagopa.interop.authorizationprocess.util.{AuthorizedRoutes, AuthzScalatestRouteTest} -import it.pagopa.interop.commons.cqrs.service.{MongoDbReadModelService, ReadModelService} -import it.pagopa.interop.commons.cqrs.model.ReadModelConfig -import org.scalamock.scalatest.MockFactory -import org.scalatest.wordspec.AnyWordSpecLike - -import scala.concurrent.ExecutionContext - -class UserApiAuthzSpec extends AnyWordSpecLike with MockFactory with AuthzScalatestRouteTest { - - val fakeAgreementManagementService: AgreementManagementService = new FakeAgreementManagementService() - val fakeCatalogManagementService: CatalogManagementService = new FakeCatalogManagementService() - val fakeAuthorizationManagementService: AuthorizationManagementService = new FakeAuthorizationManagementService() - val fakeSelfcareV2Service: SelfcareV2Service = new FakeSelfcareV2Service() - val fakePurposeManagementService: PurposeManagementService = new FakePurposeManagementService() - implicit val fakeReadModel: ReadModelService = new MongoDbReadModelService( - ReadModelConfig( - "mongodb://localhost/?socketTimeoutMS=1&serverSelectionTimeoutMS=1&connectTimeoutMS=1&&autoReconnect=false&keepAlive=false", - "db" - ) - ) - - val service: UserApiServiceImpl = - UserApiServiceImpl(fakeAuthorizationManagementService, fakeSelfcareV2Service)( - ExecutionContext.global, - fakeReadModel - ) - - "User api authorization spec" should { - "accept authorized roles for getClientUserKeys" in { - val endpoint = AuthorizedRoutes.endpoints("getClientUserKeys") - validateAuthorization( - endpoint, - { implicit c: Seq[(String, String)] => service.getClientUserKeys("test", "test") } - ) - } - } -} diff --git a/src/test/scala/it/pagopa/interop/authorizationprocess/util/SpecUtils.scala b/src/test/scala/it/pagopa/interop/authorizationprocess/util/SpecUtils.scala index 30badd93..f2038ab7 100644 --- a/src/test/scala/it/pagopa/interop/authorizationprocess/util/SpecUtils.scala +++ b/src/test/scala/it/pagopa/interop/authorizationprocess/util/SpecUtils.scala @@ -416,8 +416,6 @@ trait SpecUtils extends SprayJsonSupport { self: MockFactory => val clientApiMarshaller: ClientApiMarshallerImpl.type = ClientApiMarshallerImpl - val userApiMarshaller: UserApiMarshallerImpl.type = UserApiMarshallerImpl - implicit def fromResponseUnmarshallerClientRequest: FromEntityUnmarshaller[Client] = sprayJsonUnmarshaller[Client]