PIN-4999 Resolved PR issues

pagopa · Jun 25, 2024 · 20166c6 · 20166c6
1 parent ecf6225
commit 20166c6
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 21 deletions.
diff --git a/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/ClientApiHandlers.scala b/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/ClientApiHandlers.scala
@@ -125,7 +125,7 @@ object ClientApiHandlers extends AkkaResponses {
     result match {
       case Success(s)                                  => success(s)
       case Failure(ex: OrganizationNotAllowedOnClient) => forbidden(ex, logMessage)
-      case Failure(ex: KeyOperationNotAllowedOnClient) => forbidden(ex, logMessage)
+      case Failure(ex: UserIsNotMemberOfClient)        => forbidden(ex, logMessage)
       case Failure(ex: ClientKeyNotFound)              => notFound(ex, logMessage)
       case Failure(ex)                                 => internalServerError(ex, logMessage)
     }

diff --git a/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/ClientApiServiceImpl.scala b/src/main/scala/it/pagopa/interop/authorizationprocess/api/impl/ClientApiServiceImpl.scala
@@ -230,19 +230,18 @@ final case class ClientApiServiceImpl(
     val operationLabel: String = s"Deleting Key $keyId of Client $clientId"
     logger.info(operationLabel)
 
-    def checkRole(client: PersistentClient): Future[Unit] = for {
-      roles           <- getUserRolesFuture(contexts)
+    def assertUserIsMemberOfClient(client: PersistentClient): Future[Unit] = for {
       requesterUserId <- getUidFutureUUID(contexts)
       _               <-
-        if (roles.contains(SECURITY_ROLE) && client.users.filter(_ == requesterUserId).isEmpty)
-          Future.failed(KeyOperationNotAllowedOnClient(client.id))
+        if (!client.users.contains(requesterUserId))
+          Future.failed(UserIsNotMemberOfClient(client.id, requesterUserId))
         else Future.unit
     } yield ()
 
     val result: Future[Unit] = for {
       clientUuid <- clientId.toFutureUUID
       client     <- authorizationManagementService.getClient(clientUuid)
-      _          <- checkRole(client)
+      _          <- assertUserIsMemberOfClient(client)
       _          <- assertIsClientConsumer(client).toFuture
       _          <- authorizationManagementService.deleteKey(clientUuid, keyId)(contexts)
     } yield ()

diff --git a/src/main/scala/it/pagopa/interop/authorizationprocess/error/AuthorizationProcessErrors.scala b/src/main/scala/it/pagopa/interop/authorizationprocess/error/AuthorizationProcessErrors.scala
@@ -64,9 +64,6 @@ object AuthorizationProcessErrors {
         s"The number of the keys ${size.toString} for the client ${clientId.toString} exceed maximun allowed"
       )
 
-  final case class KeyOperationNotAllowedOnClient(clientId: UUID)
-      extends ComponentError(
-        "0025",
-        s"Role is not allowed to perform operation on the key for the client ${clientId.toString}"
-      )
+  final case class UserIsNotMemberOfClient(clientId: UUID, userId: UUID)
+      extends ComponentError("0025", s"User $userId is not a member of client ${clientId.toString}")
 }
diff --git a/src/test/scala/it/pagopa/interop/authorizationprocess/KeyOperationSpec.scala b/src/test/scala/it/pagopa/interop/authorizationprocess/KeyOperationSpec.scala
@@ -9,7 +9,7 @@ import it.pagopa.interop.authorizationprocess.api.impl.{ClientApiServiceImpl, ke
 import it.pagopa.interop.authorizationprocess.error.AuthorizationProcessErrors.{ClientKeyNotFound, ClientNotFound}
 import it.pagopa.interop.authorizationprocess.model._
 import it.pagopa.interop.selfcare.v2.client.model.UserResource
-import it.pagopa.interop.commons.utils.USER_ROLES
+import it.pagopa.interop.commons.utils.{UID, USER_ROLES}
 import it.pagopa.interop.commons.jwt.{ADMIN_ROLE, SECURITY_ROLE}
 import it.pagopa.interop.authorizationprocess.util.{CustomMatchers, SpecUtilsWithImplicit}
 import it.pagopa.interop.commons.cqrs.service.ReadModelService
@@ -277,13 +277,13 @@ class KeyOperationSpec
   }
 
   "Delete key" should {
-    "succeed" in {
+    "succeed if role is Admin" in {
       implicit val contexts: Seq[(String, String)] =
         Seq(
           "bearer"         -> bearerToken,
           USER_ROLES       -> ADMIN_ROLE,
           "organizationId" -> consumerId.toString,
-          "uid"            -> userId.toString,
+          UID              -> userId.toString,
           "selfcareId"     -> selfcareId.toString
         )
       val kid                                      = "some-kid"
@@ -292,7 +292,7 @@ class KeyOperationSpec
         .getClient(_: UUID)(_: ExecutionContext, _: ReadModelService))
         .expects(*, *, *)
         .once()
-        .returns(Future.successful(persistentClient))
+        .returns(Future.successful(persistentClient.copy(id = client.id, users = Set(userId))))
 
       (mockAuthorizationManagementService
         .deleteKey(_: UUID, _: String)(_: Seq[(String, String)]))
@@ -311,7 +311,7 @@ class KeyOperationSpec
           "bearer"         -> bearerToken,
           USER_ROLES       -> SECURITY_ROLE,
           "organizationId" -> consumerId.toString,
-          "uid"            -> userId.toString,
+          UID              -> userId.toString,
           "selfcareId"     -> selfcareId.toString
         )
       val kid                                      = "some-kid"
@@ -320,7 +320,7 @@ class KeyOperationSpec
         .getClient(_: UUID)(_: ExecutionContext, _: ReadModelService))
         .expects(*, *, *)
         .once()
-        .returns(Future.successful(persistentClient.copy(users = Set(userId))))
+        .returns(Future.successful(persistentClient.copy(id = client.id, users = Set(userId))))
 
       (mockAuthorizationManagementService
         .deleteKey(_: UUID, _: String)(_: Seq[(String, String)]))
@@ -338,7 +338,7 @@ class KeyOperationSpec
           "bearer"         -> bearerToken,
           USER_ROLES       -> SECURITY_ROLE,
           "organizationId" -> consumerId.toString,
-          "uid"            -> userId.toString,
+          UID              -> userId.toString,
           "selfcareId"     -> selfcareId.toString
         )
       val kid                                      = "some-kid"
@@ -347,20 +347,48 @@ class KeyOperationSpec
         .getClient(_: UUID)(_: ExecutionContext, _: ReadModelService))
         .expects(*, *, *)
         .once()
-        .returns(Future.successful(persistentClient))
+        .returns(Future.successful(persistentClient.copy(id = client.id)))
+
+      Get() ~> service.deleteClientKeyById(client.id.toString, kid) ~> check {
+        status shouldEqual StatusCodes.Forbidden
+      }
+    }
+    "fail if role is Admin and UID is not a member" in {
+      implicit val contexts: Seq[(String, String)] =
+        Seq(
+          "bearer"         -> bearerToken,
+          USER_ROLES       -> ADMIN_ROLE,
+          "organizationId" -> consumerId.toString,
+          UID              -> userId.toString,
+          "selfcareId"     -> selfcareId.toString
+        )
+      val kid                                      = "some-kid"
+
+      (mockAuthorizationManagementService
+        .getClient(_: UUID)(_: ExecutionContext, _: ReadModelService))
+        .expects(*, *, *)
+        .once()
+        .returns(Future.successful(persistentClient.copy(id = client.id)))
 
       Get() ~> service.deleteClientKeyById(client.id.toString, kid) ~> check {
         status shouldEqual StatusCodes.Forbidden
       }
     }
 
     "fail if client or key do not exist" in {
-      val kid = "some-kid"
+      implicit val contexts: Seq[(String, String)] = Seq(
+        "bearer"         -> bearerToken,
+        USER_ROLES       -> SECURITY_ROLE,
+        "organizationId" -> consumerId.toString,
+        UID              -> userId.toString,
+        "selfcareId"     -> selfcareId.toString
+      )
+      val kid                                      = "some-kid"
       (mockAuthorizationManagementService
         .getClient(_: UUID)(_: ExecutionContext, _: ReadModelService))
         .expects(*, *, *)
         .once()
-        .returns(Future.successful(persistentClient))
+        .returns(Future.successful(persistentClient.copy(id = client.id, users = Set(userId))))
 
       (mockAuthorizationManagementService
         .deleteKey(_: UUID, _: String)(_: Seq[(String, String)]))