This is an automated email from the ASF dual-hosted git repository.

jshao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-livy.git


The following commit(s) were added to refs/heads/master by this push:
     new ae96170  [LIVY-592][SERVER] Allow proxy user to view or modify his 
session
ae96170 is described below

commit ae961700d71fd1669dec4f396c8990d3d8c93759
Author: zixu <z...@microsoft.com>
AuthorDate: Thu Aug 22 10:41:29 2019 +0800

    [LIVY-592][SERVER] Allow proxy user to view or modify his session
    
    ## What changes were proposed in this pull request?
    This patch is ported from 
https://github.com/apache/incubator-livy/pull/172, with adding some unit tests.
    
    Allow proxy user to view or modify his session. Here is the link to the 
corresponding Jira
    https://issues.apache.org/jira/browse/LIVY-592
    
    ## How was this patch tested?
    New unit tests and existing unit tests.
    
    Author: zixu <z...@microsoft.com>
    Author: yihengwang <yihengw...@tencent.com>
    
    Closes #202 from yiheng/fix_592.
---
 .../scala/org/apache/livy/server/AccessManager.scala   | 18 ++++++++++++------
 .../scala/org/apache/livy/server/SessionServlet.scala  |  8 ++++++--
 .../apache/livy/server/batch/BatchSessionServlet.scala |  4 +++-
 .../server/interactive/InteractiveSessionServlet.scala |  4 +++-
 .../org/apache/livy/server/SessionServletSpec.scala    | 17 ++++++++++++++++-
 .../apache/livy/server/batch/BatchServletSpec.scala    |  1 +
 6 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/server/src/main/scala/org/apache/livy/server/AccessManager.scala 
b/server/src/main/scala/org/apache/livy/server/AccessManager.scala
index 3ccbfa1..f4cdde7 100644
--- a/server/src/main/scala/org/apache/livy/server/AccessManager.scala
+++ b/server/src/main/scala/org/apache/livy/server/AccessManager.scala
@@ -127,16 +127,22 @@ private[livy] class AccessManager(conf: LivyConf) extends 
Logging {
   }
 
   /**
-   * Check that the request's user has modify access to resources owned by the 
given target user.
+   * Check that the request's user has modify access to resources whose owner 
is the given target
+   * user or whose proxy user is the given proxy user.
    */
-  def hasModifyAccess(target: String, requestUser: String): Boolean = {
-    requestUser == target || checkModifyPermissions(requestUser)
+  def hasModifyAccess(target: String, requestUser: String, proxyUser: String = 
""): Boolean = {
+    requestUser == target  ||
+    proxyUser == requestUser ||
+    checkModifyPermissions(requestUser)
   }
 
   /**
-   * Check that the request's user has view access to resources owned by the 
given target user.
+   * Check that the request's user has view access to resources whose owner is 
the given target
+   * user or whose proxy user is the given proxy user.
    */
-  def hasViewAccess(target: String, requestUser: String): Boolean = {
-    requestUser == target || checkViewPermissions(requestUser)
+  def hasViewAccess(target: String, requestUser: String, proxyUser: String = 
""): Boolean = {
+    requestUser == target ||
+    proxyUser == requestUser ||
+    checkViewPermissions(requestUser)
   }
 }
diff --git a/server/src/main/scala/org/apache/livy/server/SessionServlet.scala 
b/server/src/main/scala/org/apache/livy/server/SessionServlet.scala
index 412af92..1fc27a5 100644
--- a/server/src/main/scala/org/apache/livy/server/SessionServlet.scala
+++ b/server/src/main/scala/org/apache/livy/server/SessionServlet.scala
@@ -207,7 +207,7 @@ abstract class SessionServlet[S <: Session, R <: 
RecoveryMetadata](
 
   private def doWithSession(fn: (S => Any),
       allowAll: Boolean,
-      checkFn: Option[(String, String) => Boolean]): Any = {
+      checkFn: Option[(String, String, String) => Boolean]): Any = {
     val idOrNameParam: String = params("id")
     val session = if (idOrNameParam.forall(_.isDigit)) {
       val sessionId = idOrNameParam.toInt
@@ -218,7 +218,11 @@ abstract class SessionServlet[S <: Session, R <: 
RecoveryMetadata](
     }
     session match {
       case Some(session) =>
-        if (allowAll || checkFn.map(_(session.owner, 
effectiveUser(request))).getOrElse(false)) {
+        if (allowAll ||
+            checkFn.map(_(session.owner,
+                          effectiveUser(request),
+                          session.proxyUser.getOrElse("")))
+                   .getOrElse(false)) {
           fn(session)
         } else {
           Forbidden()
diff --git 
a/server/src/main/scala/org/apache/livy/server/batch/BatchSessionServlet.scala 
b/server/src/main/scala/org/apache/livy/server/batch/BatchSessionServlet.scala
index 1bd52f4..0bf6799 100644
--- 
a/server/src/main/scala/org/apache/livy/server/batch/BatchSessionServlet.scala
+++ 
b/server/src/main/scala/org/apache/livy/server/batch/BatchSessionServlet.scala
@@ -60,7 +60,9 @@ class BatchSessionServlet(
       session: BatchSession,
       req: HttpServletRequest): Any = {
     val logs =
-      if (accessManager.hasViewAccess(session.owner, effectiveUser(req))) {
+      if (accessManager.hasViewAccess(session.owner,
+                                      effectiveUser(req),
+                                      session.proxyUser.getOrElse(""))) {
         val lines = session.logLines()
 
         val size = 10
diff --git 
a/server/src/main/scala/org/apache/livy/server/interactive/InteractiveSessionServlet.scala
 
b/server/src/main/scala/org/apache/livy/server/interactive/InteractiveSessionServlet.scala
index 9ad1a24..3b70087 100644
--- 
a/server/src/main/scala/org/apache/livy/server/interactive/InteractiveSessionServlet.scala
+++ 
b/server/src/main/scala/org/apache/livy/server/interactive/InteractiveSessionServlet.scala
@@ -67,7 +67,9 @@ class InteractiveSessionServlet(
       session: InteractiveSession,
       req: HttpServletRequest): Any = {
     val logs =
-      if (accessManager.hasViewAccess(session.owner, effectiveUser(req))) {
+      if (accessManager.hasViewAccess(session.owner,
+                                      effectiveUser(req),
+                                      session.proxyUser.getOrElse(""))) {
         Option(session.logLines())
           .map { lines =>
             val size = 10
diff --git 
a/server/src/test/scala/org/apache/livy/server/SessionServletSpec.scala 
b/server/src/test/scala/org/apache/livy/server/SessionServletSpec.scala
index 02fffa4..cdd1783 100644
--- a/server/src/test/scala/org/apache/livy/server/SessionServletSpec.scala
+++ b/server/src/test/scala/org/apache/livy/server/SessionServletSpec.scala
@@ -70,7 +70,10 @@ object SessionServletSpec {
       override protected def clientSessionView(
           session: Session,
           req: HttpServletRequest): Any = {
-        val logs = if (accessManager.hasViewAccess(session.owner, 
effectiveUser(req))) {
+        val hasViewAccess = accessManager.hasViewAccess(session.owner,
+                                                        effectiveUser(req),
+                                                        
session.proxyUser.getOrElse(""))
+        val logs = if (hasViewAccess) {
           session.logLines()
         } else {
           Nil
@@ -294,6 +297,12 @@ class AclsEnabledSessionServletSpec extends 
BaseSessionServletSpec[Session, Reco
           assert(res.logs === IndexedSeq("log"))
         }
 
+        // LIVY-592: Proxy user cannot view its session log
+        // Proxy user should be able to see its session log
+        jget[MockSessionView](s"/${res.id}", headers = aliceHeaders) { res =>
+          assert(res.logs === IndexedSeq("log"))
+        }
+
         delete(res.id, adminHeaders, SC_OK)
       }
     }
@@ -310,6 +319,12 @@ class AclsEnabledSessionServletSpec extends 
BaseSessionServletSpec[Session, Reco
         delete(res.id, viewUserHeaders, SC_FORBIDDEN)
         delete(res.id, modifyUserHeaders, SC_OK)
       }
+
+      // LIVY-592: Proxy user cannot view its session log
+      // Proxy user should be able to modify its session
+      jpost[MockSessionView]("/?doAs=alice", Map(), headers = adminHeaders) { 
res =>
+        delete(res.id, aliceHeaders, SC_OK)
+      }
     }
 
     it("should not allow regular users to impersonate others") {
diff --git 
a/server/src/test/scala/org/apache/livy/server/batch/BatchServletSpec.scala 
b/server/src/test/scala/org/apache/livy/server/batch/BatchServletSpec.scala
index ed29800..9920586 100644
--- a/server/src/test/scala/org/apache/livy/server/batch/BatchServletSpec.scala
+++ b/server/src/test/scala/org/apache/livy/server/batch/BatchServletSpec.scala
@@ -76,6 +76,7 @@ class BatchServletSpec extends 
BaseSessionServletSpec[BatchSession, BatchRecover
     when(session.appId).thenReturn(Some(appId))
     when(session.appInfo).thenReturn(appInfo)
     when(session.logLines()).thenReturn(log)
+    when(session.proxyUser).thenReturn(None)
 
     val req = mock[HttpServletRequest]
 

Reply via email to