IMPALA-3143: Fix permission check when user belongs to supergroup This commit fixes an issue where a permission check warning is incorrectly generated when a user that belongs to the supergroup issues a create database statement. The fix is to also check if the user belongs to the supergroup when checking for permissions and ACLs.
Testing: This change was tested manually by creating an HDFS directory D that belonged to user/group A and running a CREATE DATABASE foo LOCALTION 'D' statement as a user B that belongs to the 'supergroup' group; the test verified that when the supergroup is taken into account, no permision check warning is issued. Change-Id: I2bb703ac70280c62db404db2c07d87139aa5ae0d Reviewed-on: http://gerrit.cloudera.org:8080/3164 Reviewed-by: Alex Behm <[email protected]> Tested-by: Internal Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/a0ad1868 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/a0ad1868 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/a0ad1868 Branch: refs/heads/master Commit: a0ad1868bda902fd914bc2be39eb9629a6eceb76 Parents: 6c8dc1b Author: Dimitris Tsirogiannis <[email protected]> Authored: Fri May 20 20:49:21 2016 -0700 Committer: Tim Armstrong <[email protected]> Committed: Tue May 24 20:41:09 2016 -0700 ---------------------------------------------------------------------- .../com/cloudera/impala/util/FsPermissionChecker.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a0ad1868/fe/src/main/java/com/cloudera/impala/util/FsPermissionChecker.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/com/cloudera/impala/util/FsPermissionChecker.java b/fe/src/main/java/com/cloudera/impala/util/FsPermissionChecker.java index 18d0534..056fdf0 100644 --- a/fe/src/main/java/com/cloudera/impala/util/FsPermissionChecker.java +++ b/fe/src/main/java/com/cloudera/impala/util/FsPermissionChecker.java @@ -24,10 +24,10 @@ import java.util.List; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.permission.AclEntry; import org.apache.hadoop.fs.permission.AclEntryType; import org.apache.hadoop.fs.permission.AclStatus; -import org.apache.hadoop.fs.permission.AclStatus; import org.apache.hadoop.fs.permission.AclEntryScope; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; @@ -36,7 +36,8 @@ import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.hdfs.protocol.AclException; -import org.apache.hadoop.ipc.RemoteException; +import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_PERMISSIONS_SUPERUSERGROUP_DEFAULT; +import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_PERMISSIONS_SUPERUSERGROUP_KEY; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -50,10 +51,13 @@ import com.google.common.collect.Lists; public class FsPermissionChecker { private final static Logger LOG = LoggerFactory.getLogger(FsPermissionChecker.class); private final static FsPermissionChecker instance_; + private final static Configuration CONF; protected final String user_; private final Set<String> groups_ = new HashSet<String>(); + private final String supergroup_; static { + CONF = new Configuration(); try { instance_ = new FsPermissionChecker(); } catch (IOException e) { @@ -65,10 +69,12 @@ public class FsPermissionChecker { private FsPermissionChecker() throws IOException { UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); groups_.addAll(Arrays.asList(ugi.getGroupNames())); - // Todo: should add HDFS supergroup as well + supergroup_ = CONF.get(DFS_PERMISSIONS_SUPERUSERGROUP_KEY, + DFS_PERMISSIONS_SUPERUSERGROUP_DEFAULT); user_ = ugi.getShortUserName(); } + private boolean isSuperUser() { return groups_.contains(supergroup_); } private static List<AclEntryType> ACL_TYPE_PRIORITY = ImmutableList.of(AclEntryType.USER, AclEntryType.GROUP, AclEntryType.OTHER); @@ -198,6 +204,7 @@ public class FsPermissionChecker { * permissions. */ public boolean checkPermissions(FsAction action) { + if (FsPermissionChecker.this.isSuperUser()) return true; Boolean aclPerms = checkAcls(action); if (aclPerms != null) return aclPerms;
