mgaido91 commented on a change in pull request #141: [LIVY-551] Add "doAs" 
impersonation support
URL: https://github.com/apache/incubator-livy/pull/141#discussion_r251421684
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/server/AccessManager.scala
 ##########
 @@ -98,46 +99,51 @@ private[livy] class AccessManager(conf: LivyConf) extends 
Logging {
   def isAccessControlOn: Boolean = aclsOn
 
   /**
-   * Checks that the requesting user can impersonate the target user.
-   * If the user does not have permission to impersonate, then throws an 
`AccessControlException`.
-   *
-   * @return The user that should be impersonated. That can be the target user 
if defined, the
-   *         request's user - which may not be defined - otherwise, or `None` 
if impersonation is
-   *         disabled.
+   * Checks that the request user can impersonate the target user.
+   * If impersonation is enabled and the user does not have permission to 
impersonate
+   * then throws an `AccessControlException`. If impersonation is disabled 
returns false
    */
   def checkImpersonation(
-      target: Option[String],
       requestUser: String,
-      livyConf: LivyConf): Option[String] = {
-    if (livyConf.getBoolean(LivyConf.IMPERSONATION_ENABLED)) {
-      if (!target.forall(hasSuperAccess(_, requestUser))) {
-        throw new AccessControlException(
-          s"User '$requestUser' not allowed to impersonate '$target'.")
+      impersonatedUser: String): Boolean = {
+    if (conf.getBoolean(LivyConf.IMPERSONATION_ENABLED)) {
+      if (hasSuperAccess(requestUser, impersonatedUser)) {
+        return true
       }
-      target.orElse(Option(requestUser))
-    } else {
-      None
+      throw new AccessControlException(
+        s"User '$requestUser' not allowed to impersonate '$impersonatedUser'.")
     }
+    false
   }
 
   /**
-   * Check that the requesting user has admin access to resources owned by the 
given target user.
+   * Check that the request user has is able to impersonate the given user.
    */
-  def hasSuperAccess(target: String, requestUser: String): Boolean = {
-    requestUser == target || checkSuperUser(requestUser)
+  def hasSuperAccess(
+      requestUser: String,
+      impersonatedUser: String): Boolean = {
+    requestUser == impersonatedUser || checkSuperUser(requestUser)
   }
 
   /**
-   * Check that the request's user has modify access to resources owned by the 
given target user.
+   * Check that the request user has modify access to the given session.
    */
-  def hasModifyAccess(target: String, requestUser: String): Boolean = {
-    requestUser == target || checkModifyPermissions(requestUser)
+  def hasModifyAccess(
 
 Review comment:
   nit: one line

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to