From ddaca223adfd6f18df8befc5cf5d7d272fb30f3f Mon Sep 17 00:00:00 2001
From: "Roque Vassal'lo @ StratioBD"
 <32131830+rvassallo-stratio@users.noreply.github.com>
Date: Fri, 12 Apr 2019 11:32:37 +0200
Subject: [PATCH] [CROSSDATA-2028] Fix returned message when impersonator
 enabled and connection lost (#819)

---
 .../stratio/crossdata/server/HttpRoutes.scala | 23 ++++++++----
 .../crossdata/server/actors/ServerActor.scala | 36 ++++++++++---------
 2 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/server/src/main/scala/com/stratio/crossdata/server/HttpRoutes.scala b/server/src/main/scala/com/stratio/crossdata/server/HttpRoutes.scala
index 5baaa53d4..20094ea18 100644
--- a/server/src/main/scala/com/stratio/crossdata/server/HttpRoutes.scala
+++ b/server/src/main/scala/com/stratio/crossdata/server/HttpRoutes.scala
@@ -231,13 +231,18 @@ abstract class HttpRoutes(
             if (verifiedUser) {
 
               // Authorize impersonation if needed
-              val authorizeSessionImpersonation: Either[Option[SessionID], String] = (cmd, queryOpt, impersonatedUserOpt) match {
+              val authorizeSessionImpersonation: Either[Option[SessionID], (StatusCode, Either[String, FailureServerReply])] = (cmd, queryOpt, impersonatedUserOpt) match {
                 case (sessionCommand: SessionCommand, Some(_), Some(impersonatedUser)) =>
                   Try(
-                    Await.result((serverActor ? GetImpersonatedUserSession(sessionCommand.session.id, impersonatedUser)).map {
-                      case sessionId: SessionID => Left(Some(sessionId))
-                      case failureMessage: String => Right(failureMessage)
-                    }, 5 seconds)
+                    Await.result(
+                      (serverActor ? GetImpersonatedUserSession(sessionCommand.requestId, sessionCommand.session.id, impersonatedUser))
+                        .map {
+                          case sessionId: SessionID => Left(Some(sessionId))
+                          case failureMessage: String => Right((StatusCodes.Unauthorized, Left(failureMessage)))
+                          case reply: FailureServerReply => Right((StatusCodes.InternalServerError, Right(reply)))
+                        },
+                      5 seconds
+                    )
                   ) match {
                     case Success(l: Left[_, _]) => l
                     case Success(r: Right[_, _]) => r
@@ -370,8 +375,12 @@ abstract class HttpRoutes(
                     }
                 }
               } else {
-                logger.info(s"Authorization error: ${authorizeSessionImpersonation.right.get}")
-                completeWithErrorResult(s"Authorization error: ${authorizeSessionImpersonation.right.get}", StatusCodes.Unauthorized)
+                if(authorizeSessionImpersonation.right.get._2.isLeft) {
+                  completeWithErrorResult(authorizeSessionImpersonation.right.get._2.left.get, authorizeSessionImpersonation.right.get._1)
+                } else {
+                  completeWithResponse(authorizeSessionImpersonation.right.get._1 -> authorizeSessionImpersonation.right.get._2.right.get)
+                }
+
               }
             } else {
               logger.info(s"Authentication error: ${user.getOrElse("<empty>")} not allowed")
diff --git a/server/src/main/scala/com/stratio/crossdata/server/actors/ServerActor.scala b/server/src/main/scala/com/stratio/crossdata/server/actors/ServerActor.scala
index ef82d2b3d..1f4e8c4bf 100644
--- a/server/src/main/scala/com/stratio/crossdata/server/actors/ServerActor.scala
+++ b/server/src/main/scala/com/stratio/crossdata/server/actors/ServerActor.scala
@@ -50,7 +50,7 @@ object ServerActor {
 
   case class GetUserId(sessionId: UUID)
 
-  case class GetImpersonatedUserSession(sessionId: UUID, user: String)
+  case class GetImpersonatedUserSession(requestId: UUID, sessionId: UUID, user: String)
 
   case class JobId(sessionId: UUID, queryId: UUID)
 
@@ -284,30 +284,34 @@ class ServerActor(cluster: Cluster, sessionProvider: XDSessionProvider, serverCo
     case GetUserId(session) =>
       sender ! sessionProvider.getUserId(session)
 
-    case GetImpersonatedUserSession(sessionId, user) =>
-      val serviceUserId = sessionProvider.getUserId(sessionId)
-      val authorized = securityManager.forall {
-        manager =>
-          serviceUserId.exists {
-            serviceUser =>
+    case GetImpersonatedUserSession(requestId, sessionId, user) =>
+      val originalSender = sender()
+      sessionProvider.session(sessionId) match {
+        case Success(s) =>
+          val authorized = securityManager.forall {
+            manager =>
               manager.authorize(
                 userId = user,
                 resource = Resource(
                   Seq(sys.env.getOrElse(Resource.CrossdataClusterNameEnvVar, "unknown")),
                   ServiceImpersonationResource,
-                  serviceUser
+                  s.user
                 ),
                 action = Other
               )
           }
-      }
-      if(authorized) {
-        val userSessionId = sessionProvider.getChildSessionId(sessionId, user)
-        sender ! SessionID(userSessionId)
-        // Remove session from clean-pending session list
-        State.removeSessionFromCleanPendingList(userSessionId)
-      } else {
-        sender ! s"User $user cannot use ${serviceUserId.map(x => s"$x's").getOrElse("this")} connection"
+          if(authorized) {
+            val userSessionId = sessionProvider.getChildSessionId(sessionId, user)
+            originalSender ! SessionID(userSessionId)
+            // Remove session from clean-pending session list
+            State.removeSessionFromCleanPendingList(userSessionId)
+          } else {
+            logger.info(s"Authorization error: User $user cannot use ${s.user}'s connection")
+            originalSender ! s"Authorization error: User $user cannot use ${s.user}'s connection"
+          }
+        case Failure(th) =>
+          logger.warn(s"Received impersonation message with an unknown sessionId $sessionId", th)
+          originalSender ! FailureServerReply(requestId, s"Unable to recover the session $sessionId. Cause: ${th.getMessage}")
       }
 
     case SparkExecutorStatus =>