[CROSSDATA-2028] Fix returned message when impersonator enabled and c…

…onnection lost (#819)
Stratio · Apr 12, 2019 · ddaca22 · ddaca22
1 parent 8bf0eb5
commit ddaca22
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 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
@@ -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
@@ -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 =>