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

 ##########
 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] = {
 
 Review comment:
   I don't really see a use case for the conf instances being different. At the 
call sites, it's just a matter of sub-optimal APIs (e.g. `BatchSession.create` 
being a static method and thus not having any state associated, like a 
configuration object).

----------------------------------------------------------------
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