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_r251421028
########## 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: mmmh...now you are using the `conf` passed when building the `AccessManager`, which may be different from the one which was used before this patch. Despite this one seems a cleaner approach, I wonder if there may be some behavioral changes due to this. cc @vanzin for his opinion. ---------------------------------------------------------------- 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