maheshk114 commented on code in PR #3264:
URL: https://github.com/apache/hive/pull/3264#discussion_r867661240
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java:
##########
@@ -454,13 +456,24 @@ private static Set<SQLPrivTypeGrant> getPrivilegesFromFS(
if (FileUtils.isOwnerOfFileHierarchy(fs, fileStatus, userName, recurse)) {
privs.add(SQLPrivTypeGrant.OWNER_PRIV);
}
- if (FileUtils.isActionPermittedForFileHierarchy(fs, fileStatus, userName,
FsAction.WRITE, recurse)) {
+ UserGroupInformation ugi = Utils.getUGI();
+ String currentUser = ugi.getShortUserName();
+ FileSystem fsAsUser = null;
+ UserGroupInformation proxyUser = null;
+ if (userName != null && !userName.equals(currentUser)) {
+ proxyUser = UserGroupInformation.createProxyUser(userName,
UserGroupInformation.getLoginUser());
+ fsAsUser = FileUtils.getFsAsUser(fs, proxyUser);
+ }
+ if (FileUtils.isActionPermittedForFileHierarchy(fs, fileStatus, userName,
FsAction.WRITE, recurse, fsAsUser)) {
privs.add(SQLPrivTypeGrant.INSERT_NOGRANT);
privs.add(SQLPrivTypeGrant.DELETE_NOGRANT);
}
- if (FileUtils.isActionPermittedForFileHierarchy(fs, fileStatus, userName,
FsAction.READ, recurse)) {
+ if (FileUtils.isActionPermittedForFileHierarchy(fs, fileStatus, userName,
FsAction.READ, recurse, fsAsUser)) {
privs.add(SQLPrivTypeGrant.SELECT_NOGRANT);
}
+ if (proxyUser != null) {
+ FileSystem.closeAllForUGI(proxyUser);
Review Comment:
Should not it be done for failure cases also ?
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java:
##########
@@ -5008,11 +5009,25 @@ static private boolean needToCopy(final HiveConf conf,
Path srcf, Path destf, Fi
boolean isOwned = FileUtils.isOwnerOfFileHierarchy(srcFs, srcs,
configuredOwner, false);
if (configuredOwner.equals(runningUser)) {
// Check if owner has write permission, else it will have to copy
+ UserGroupInformation ugi = Utils.getUGI();
+ String currentUser = ugi.getShortUserName();
+ FileSystem fsAsUser = null;
+ UserGroupInformation proxyUser = null;
+ if (configuredOwner != null && !configuredOwner.equals(currentUser))
{
+ proxyUser = UserGroupInformation.createProxyUser(configuredOwner,
UserGroupInformation.getLoginUser());
Review Comment:
This block is used more than once. Can be moved to a common block.
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java:
##########
@@ -5008,11 +5009,25 @@ static private boolean needToCopy(final HiveConf conf,
Path srcf, Path destf, Fi
boolean isOwned = FileUtils.isOwnerOfFileHierarchy(srcFs, srcs,
configuredOwner, false);
if (configuredOwner.equals(runningUser)) {
// Check if owner has write permission, else it will have to copy
+ UserGroupInformation ugi = Utils.getUGI();
+ String currentUser = ugi.getShortUserName();
+ FileSystem fsAsUser = null;
+ UserGroupInformation proxyUser = null;
+ if (configuredOwner != null && !configuredOwner.equals(currentUser))
{
+ proxyUser = UserGroupInformation.createProxyUser(configuredOwner,
UserGroupInformation.getLoginUser());
+ fsAsUser = FileUtils.getFsAsUser(srcFs, proxyUser);
+ }
if (!(isOwned &&
FileUtils.isActionPermittedForFileHierarchy(
- srcFs, srcs, configuredOwner, FsAction.WRITE, false))) {
+ srcFs, srcs, configuredOwner, FsAction.WRITE, false,
fsAsUser))) {
+ if (proxyUser != null) {
+ FileSystem.closeAllForUGI(proxyUser);
Review Comment:
This can be done in a final block
##########
common/src/java/org/apache/hadoop/hive/common/FileUtils.java:
##########
@@ -408,10 +408,23 @@ public static FileStatus
getPathOrParentThatExists(FileSystem fs, Path path) thr
return getPathOrParentThatExists(fs, parentPath);
}
- public static void checkFileAccessWithImpersonation(final FileSystem fs,
final FileStatus stat,
- final FsAction action, final String user)
- throws IOException, AccessControlException, InterruptedException,
Exception {
- checkFileAccessWithImpersonation(fs, stat, action, user, null);
+ public static void checkFileAccessWithImpersonation(final FileSystem fs,
final FileStatus stat, final FsAction action,
+ final String user) throws IOException, AccessControlException,
InterruptedException, Exception {
+ UserGroupInformation ugi = Utils.getUGI();
+ String currentUser = ugi.getShortUserName();
+ UserGroupInformation proxyUser = null;
+ FileSystem fsAsUser = null;
+ try {
+ if (user != null && !user.equals(currentUser)) {
+ proxyUser = UserGroupInformation.createProxyUser(user,
UserGroupInformation.getLoginUser());
+ fsAsUser = FileUtils.getFsAsUser(fs, proxyUser);
+ }
+ checkFileAccessWithImpersonation(fs, stat, action, user, null, fsAsUser);
Review Comment:
instead of fsAsUser, pass currentUser and do proxy user creation in
checkFileAccessWithImpersonation method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
