sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From linaataus...@apache.org
Subject sentry git commit: SENTRY-2255: alter table set owner command can be executed only by user with proper privilege (Na Li, reviewed by Sergio Pena, Kalyan Kumar Kalvagadda)
Date Wed, 08 Aug 2018 20:29:17 GMT
Repository: sentry
Updated Branches:
  refs/heads/master fb7bb7bc6 -> c30d56111


SENTRY-2255: alter table set owner command can be executed only by user with proper privilege
(Na Li, reviewed by Sergio Pena, Kalyan Kumar Kalvagadda)


Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/c30d5611
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/c30d5611
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/c30d5611

Branch: refs/heads/master
Commit: c30d5611194d852df76ea34d3b6c3dd3676ccf34
Parents: fb7bb7b
Author: lina.li <lina.li@cloudera.com>
Authored: Wed Aug 8 15:27:42 2018 -0500
Committer: lina.li <lina.li@cloudera.com>
Committed: Wed Aug 8 15:27:42 2018 -0500

----------------------------------------------------------------------
 .../binding/hive/authz/HiveAuthzBinding.java    |   6 +-
 .../binding/hive/authz/HiveAuthzPrivileges.java |  24 +-
 .../hive/authz/HiveAuthzPrivilegesMap.java      |  12 +-
 .../sentry/policy/common/CommonPrivilege.java   |  31 +-
 .../provider/common/AuthorizationProvider.java  |  16 ++
 .../common/NoAuthorizationProvider.java         |   6 +
 .../common/ResourceAuthorizationProvider.java   |  26 +-
 .../db/service/persistent/SentryStore.java      |   8 +
 .../e2e/dbprovider/TestOwnerPrivileges.java     | 282 +++++++++++++++++--
 .../tests/e2e/hdfs/TestHDFSIntegrationBase.java |   1 +
 10 files changed, 380 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
----------------------------------------------------------------------
diff --git a/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
b/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
index f1cbbb6..6a1556f 100644
--- a/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
+++ b/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
@@ -338,7 +338,8 @@ public class HiveAuthzBinding {
       for (List<DBModelAuthorizable> inputHierarchy : inputHierarchyList) {
         if (getAuthzType(inputHierarchy).equals(key)) {
           found = true;
-          if (!authProvider.hasAccess(subject, inputHierarchy, entry.getValue(), activeRoleSet))
{
+          if (!authProvider.hasAccess(subject, inputHierarchy, entry.getValue(),
+              stmtAuthPrivileges.getGrantOption(), activeRoleSet)) {
             throw new AuthorizationException("User " + subject.getName() +
                 " does not have privileges for " + hiveOp.name());
           }
@@ -362,7 +363,8 @@ public class HiveAuthzBinding {
       for (List<DBModelAuthorizable> outputHierarchy : outputHierarchyList) {
         if (getAuthzType(outputHierarchy).equals(key)) {
           found = true;
-          if (!authProvider.hasAccess(subject, outputHierarchy, entry.getValue(), activeRoleSet))
{
+          if (!authProvider.hasAccess(subject, outputHierarchy, entry.getValue(),
+              stmtAuthPrivileges.getGrantOption(), activeRoleSet)) {
             throw new AuthorizationException("User " + subject.getName() +
                 " does not have privileges for " + hiveOp.name());
           }

http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java
----------------------------------------------------------------------
diff --git a/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java
b/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java
index f164b30..c37ce64 100644
--- a/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java
+++ b/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java
@@ -68,6 +68,7 @@ public class HiveAuthzPrivileges {
         new HashMap<AuthorizableType,EnumSet<DBModelAction>>();
     private HiveOperationType operationType;
     private HiveOperationScope operationScope;
+    private boolean grantOption = false;
 
     public AuthzPrivilegeBuilder addInputObjectPriviledge(AuthorizableType inputObjectType,
EnumSet<DBModelAction> inputPrivilege) {
       inputPrivileges.put(inputObjectType, inputPrivilege);
@@ -94,6 +95,11 @@ public class HiveAuthzPrivileges {
       return this;
     }
 
+    public AuthzPrivilegeBuilder setGrantOption(boolean requireGrantOption) {
+      this.grantOption = requireGrantOption;
+      return this;
+    }
+
     public HiveAuthzPrivileges build() {
       if (operationScope.equals(HiveOperationScope.UNKNOWN)) {
         throw new UnsupportedOperationException("Operation scope is not set");
@@ -103,7 +109,8 @@ public class HiveAuthzPrivileges {
         throw new UnsupportedOperationException("Operation scope is not set");
       }
 
-      return new HiveAuthzPrivileges(inputPrivileges, outputPrivileges, operationType, operationScope);
+      return new HiveAuthzPrivileges(inputPrivileges, outputPrivileges, operationType,
+          operationScope, grantOption);
     }
   }
 
@@ -113,14 +120,22 @@ public class HiveAuthzPrivileges {
       new HashMap<AuthorizableType,EnumSet<DBModelAction>>();
   private final HiveOperationType operationType;
   private final HiveOperationScope operationScope;
+  private final boolean grantOption;
 
   protected HiveAuthzPrivileges(Map<AuthorizableType,EnumSet<DBModelAction>>
inputPrivileges,
       Map<AuthorizableType,EnumSet<DBModelAction>> outputPrivileges, HiveOperationType
operationType,
       HiveOperationScope operationScope) {
+    this(inputPrivileges, outputPrivileges, operationType, operationScope, false);
+  }
+
+  protected HiveAuthzPrivileges(Map<AuthorizableType,EnumSet<DBModelAction>>
inputPrivileges,
+      Map<AuthorizableType,EnumSet<DBModelAction>> outputPrivileges, HiveOperationType
operationType,
+      HiveOperationScope operationScope, boolean requireGrantOption) {
     this.inputPrivileges.putAll(inputPrivileges);
     this.outputPrivileges.putAll(outputPrivileges);
     this.operationScope = operationScope;
     this.operationType = operationType;
+    this.grantOption = requireGrantOption;
   }
 
   /**
@@ -138,6 +153,13 @@ public class HiveAuthzPrivileges {
   }
 
   /**
+   * @return the grantOption
+   */
+  public boolean getGrantOption() {
+    return grantOption;
+  }
+
+  /**
    * @return the operationType
    */
   public HiveOperationType getOperationType() {

http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
----------------------------------------------------------------------
diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
index 9350af0..d976512 100644
--- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
+++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
@@ -112,6 +112,15 @@ public class HiveAuthzPrivilegesMap {
         setOperationType(HiveOperationType.DDL).
         build();
 
+    /* TODO: once HIVE-18762 is in the hiver version integrated with Sentry, uncomment this
block
+    HiveAuthzPrivileges alterTableSetOwnerPrivilege = new HiveAuthzPrivileges.AuthzPrivilegeBuilder().
+        addInputObjectPriviledge(AuthorizableType.Table, EnumSet.of(DBModelAction.ALL)).
+        setOperationScope(HiveOperationScope.TABLE).
+        setOperationType(HiveOperationType.DDL).
+        setGrantOption(true).
+        build();
+        */
+
     // input required privilege from Hive: SELECT on column level and DELETE on table level
     // output required privilege from Hive: INSERT on table level
     // Sentry makes it more restrictive, and requires ALL at input, INSERT and ALTER at output
@@ -270,7 +279,8 @@ public class HiveAuthzPrivilegesMap {
     hiveAuthzStmtPrivMap.put(HiveOperation.ALTERVIEW_PROPERTIES, alterTablePrivilege);
     hiveAuthzStmtPrivMap.put(HiveOperation.ALTERVIEW_AS, createViewPrivilege);
     hiveAuthzStmtPrivMap.put(HiveOperation.ALTERVIEW_RENAME, alterTableRenamePrivilege);
-
+    // TODO: once HIVE-18762 is in the hiver version integrated with Sentry, uncomment next
line
+    // hiveAuthzStmtPrivMap.put(HiveOperation.ALTERTABLE_OWNER, alterTableSetOwnerPrivilege);
 
     hiveAuthzStmtPrivMap.put(HiveOperation.ALTERTABLE_DROPPARTS, dropPartitionPrivilege);
     hiveAuthzStmtPrivMap.put(HiveOperation.ALTERTABLE_ADDPARTS, addPartitionPrivilege);

http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
----------------------------------------------------------------------
diff --git a/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
b/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
index ab55609..61d442a 100644
--- a/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
+++ b/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
@@ -35,6 +35,7 @@ import java.util.List;
 public class CommonPrivilege implements Privilege {
 
   private ImmutableList<KeyValue> parts;
+  private boolean grantOption = false;
 
   public CommonPrivilege(String privilegeStr) {
     privilegeStr = Strings.nullToEmpty(privilegeStr).trim();
@@ -52,6 +53,14 @@ public class CommonPrivilege implements Privilege {
     if (parts.isEmpty()) {
       throw new AssertionError("Should never occur: " + privilegeStr);
     }
+
+    // check if grant option is present
+    KeyValue lastPart = parts.get(parts.size() - 1);
+    if (lastPart.getKey().equalsIgnoreCase(SentryConstants.GRANT_OPTION)) {
+      grantOption = lastPart.getValue().equalsIgnoreCase("true");
+      parts.remove(parts.size() - 1);
+    }
+
     this.parts = ImmutableList.copyOf(parts);
   }
 
@@ -62,7 +71,13 @@ public class CommonPrivilege implements Privilege {
       return false;
     }
 
-    List<KeyValue> otherParts = ((CommonPrivilege) privilege).getParts();
+    CommonPrivilege requiredPrivilege = (CommonPrivilege) privilege;
+    if ((requiredPrivilege.grantOption == true) && (this.grantOption == false)) {
+      // the required privilege wp needs grant option, but this privilege does not have grant
option
+      return false;
+    }
+
+    List<KeyValue> otherParts = requiredPrivilege.getParts();
     if(parts.equals(otherParts)) {
       return true;
     }
@@ -106,7 +121,7 @@ public class CommonPrivilege implements Privilege {
     // all of the other parts are wildcards
     for (; index < parts.size(); index++) {
       KeyValue part = parts.get(index);
-      if (!SentryConstants.PRIVILEGE_WILDCARD_VALUE.equals(part.getValue())) {
+      if (!isPrivilegeActionAll(part, model.getBitFieldActionFactory())) {
         return false;
       }
     }
@@ -114,6 +129,18 @@ public class CommonPrivilege implements Privilege {
     return true;
   }
 
+  /**
+   * Check if the action part in a privilege is ALL. Owner privilege is
+   * treated as ALL for authorization
+   * @param actionPart it must be the action of a privilege
+   * @return true if the action is ALL; false otherwise
+   */
+  private boolean isPrivilegeActionAll(KeyValue actionPart,
+      BitFieldActionFactory bitFieldActionFactory) {
+    return impliesAction(actionPart.getValue(), SentryConstants.PRIVILEGE_WILDCARD_VALUE,
+        bitFieldActionFactory);
+  }
+
   @Override
   public List<KeyValue> getAuthorizable() {
     List<KeyValue> authorizable = new ArrayList<>();

http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
index 73fcda8..aecfe5b 100644
--- a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
+++ b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
@@ -53,6 +53,22 @@ public interface AuthorizationProvider {
       Set<? extends Action> actions, ActiveRoleSet roleSet);
 
   /***
+   * Returns validate subject privileges on given Authorizable object
+   *
+   * @param subject: UserID to validate privileges
+   * @param authorizableHierarchy : List of object according to namespace hierarchy.
+   *        eg. Server->Db->Table or Server->Function
+   *        The privileges will be validated from the higher to lower scope
+   * @param actions : Privileges to validate
+   * @param requireGrantOption: true: require grant option of matching privilege; false:
otherwise
+   * @param roleSet : Roles which should be used when obtaining privileges
+   * @return
+   *        True if the subject is authorized to perform requested action on the given object
+   */
+  boolean hasAccess(Subject subject, List<? extends Authorizable> authorizableHierarchy,
+      Set<? extends Action> actions, boolean requireGrantOption, ActiveRoleSet roleSet);
+
+  /***
    * Get the GroupMappingService used by the AuthorizationProvider
    *
    * @return GroupMappingService used by the AuthorizationProvider

http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java
b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java
index be0830d..61400ca 100644
--- a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java
+++ b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java
@@ -38,6 +38,12 @@ public class NoAuthorizationProvider implements AuthorizationProvider {
   }
 
   @Override
+  public boolean hasAccess(Subject subject, List<? extends Authorizable> authorizableHierarchy,
+      Set<? extends Action> actions, boolean grantOption, ActiveRoleSet roleSet) {
+    return false;
+  }
+
+  @Override
   public GroupMappingService getGroupMapping() {
     return noGroupMappingService;
   }

http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
index a9b98f3..1e1aa63 100644
--- a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
+++ b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
@@ -18,6 +18,7 @@ package org.apache.sentry.provider.common;
 
 import static org.apache.sentry.core.common.utils.SentryConstants.AUTHORIZABLE_JOINER;
 import static org.apache.sentry.core.common.utils.SentryConstants.AUTHORIZABLE_SPLITTER;
+import static org.apache.sentry.core.common.utils.SentryConstants.GRANT_OPTION;
 import static org.apache.sentry.core.common.utils.SentryConstants.KV_JOINER;
 import static org.apache.sentry.core.common.utils.SentryConstants.PRIVILEGE_NAME;
 
@@ -83,6 +84,21 @@ public abstract class ResourceAuthorizationProvider implements AuthorizationProv
   @Override
   public boolean hasAccess(Subject subject, List<? extends Authorizable> authorizableHierarchy,
       Set<? extends Action> actions, ActiveRoleSet roleSet) {
+    return hasAccess(subject, authorizableHierarchy, actions, false, roleSet);
+  }
+
+  /***
+   * @param subject: UserID to validate privileges
+   * @param authorizableHierarchy : List of object according to namespace hierarchy.
+   *        eg. Server->Db->Table or Server->Function
+   *        The privileges will be validated from the higher to lower scope
+   * @param actions : Privileges to validate
+   * @return
+   *        True if the subject is authorized to perform requested action on the given object
+   */
+  @Override
+  public boolean hasAccess(Subject subject, List<? extends Authorizable> authorizableHierarchy,
+      Set<? extends Action> actions, boolean requireGrantOption, ActiveRoleSet roleSet)
{
     if(LOGGER.isDebugEnabled()) {
       LOGGER.debug("Authorization Request for " + subject + " " +
           authorizableHierarchy + " and " + actions);
@@ -94,13 +110,13 @@ public abstract class ResourceAuthorizationProvider implements AuthorizationProv
     Preconditions.checkArgument(!actions.isEmpty(), "Actions cannot be empty");
     Preconditions.checkNotNull(roleSet, "ActiveRoleSet cannot be null");
     boolean hasAccess = false;
-    hasAccess = doHasAccess(subject, authorizableHierarchy, actions, roleSet);
+    hasAccess = doHasAccess(subject, authorizableHierarchy, actions, requireGrantOption,
roleSet);
     return hasAccess;
   }
 
   private boolean doHasAccess(Subject subject,
       List<? extends Authorizable> authorizables, Set<? extends Action> actions,
-      ActiveRoleSet roleSet) {
+      boolean requireGrantOption, ActiveRoleSet roleSet) {
     Set<String> groups;
     try {
       groups = getGroups(subject);
@@ -113,7 +129,7 @@ public abstract class ResourceAuthorizationProvider implements AuthorizationProv
     for (Authorizable authorizable : authorizables) {
       hierarchy.add(KV_JOINER.join(authorizable.getTypeName(), authorizable.getName()));
     }
-    List<String> requestPrivileges = buildPermissions(authorizables, actions);
+    List<String> requestPrivileges = buildPermissions(authorizables, actions, requireGrantOption);
     Iterable<Privilege> privileges = getPrivileges(groups, users, roleSet,
         authorizables.toArray(new Authorizable[0]));
     lastFailedPrivileges.get().clear();
@@ -213,7 +229,7 @@ public abstract class ResourceAuthorizationProvider implements AuthorizationProv
   }
 
   private List<String> buildPermissions(List<? extends Authorizable> authorizables,
-      Set<? extends Action> actions) {
+      Set<? extends Action> actions, boolean requireGrantOption) {
     List<String> hierarchy = new ArrayList<String>();
     List<String> requestedPermissions = new ArrayList<String>();
 
@@ -225,6 +241,8 @@ public abstract class ResourceAuthorizationProvider implements AuthorizationProv
       String requestPermission = AUTHORIZABLE_JOINER.join(hierarchy);
       requestPermission = AUTHORIZABLE_JOINER.join(requestPermission,
           KV_JOINER.join(PRIVILEGE_NAME, action.getValue()));
+      requestPermission = AUTHORIZABLE_JOINER.join(requestPermission,
+          KV_JOINER.join(GRANT_OPTION, requireGrantOption));
       requestedPermissions.add(requestPermission);
     }
     return requestedPermissions;

http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
index 621ce89..5644181 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
@@ -2419,6 +2419,14 @@ public class SentryStore implements SentryStoreInterface {
       .add(KV_JOINER.join(SentryConstants.PRIVILEGE_NAME.toLowerCase(),
           privilege.getAction()));
     }
+
+    if (privilege.getGrantOption()) {
+      // include grant option field when it is true
+      authorizable
+          .add(KV_JOINER.join(SentryConstants.GRANT_OPTION.toLowerCase(),
+              privilege.getGrantOption()));
+    }
+
     return AUTHORIZABLE_JOINER.join(authorizable);
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
index 8a10214..2ecf8fe 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
@@ -31,9 +31,11 @@ import java.util.Set;
 
 import org.apache.sentry.tests.e2e.hdfs.TestHDFSIntegrationBase;
 import org.apache.sentry.tests.e2e.hive.StaticUserGroup;
+import org.apache.sentry.service.common.ServiceConstants.SentryPrincipalType;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.BeforeClass;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import com.google.common.collect.Lists;
@@ -52,6 +54,8 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase {
       USER1_1 = StaticUserGroup.USER1_1,
       USER1_2 = StaticUserGroup.USER1_2,
       USERGROUP1 = StaticUserGroup.USERGROUP1,
+      USERGROUP2 = StaticUserGroup.USERGROUP2,
+      USER2_1 = StaticUserGroup.USER2_1,
       DB1 = "db_1";
 
   private final static String renameTag = "_new";
@@ -100,7 +104,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase {
     statementUSER1_1.execute("CREATE DATABASE " + DB1);
 
     // verify privileges created for new database
-    verifyTablePrivilegeExistForUser(statementUSER1_1, Lists.newArrayList(USER1_1),
+    verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1),
         DB1, null, 1);
 
     // verify that user has all privilege on this database, i.e., "OWNER" means "ALL"
@@ -146,7 +150,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase {
     // verify user user1_2 has no privileges created for new database
     Connection connectionUSER1_2 = hiveServer2.createConnection(USER1_2, USER1_2);
     Statement statementUSER1_2 = connectionUSER1_2.createStatement();
-    verifyTablePrivilegeExistForUser(statementUSER1_2, Lists.newArrayList(USER1_2),
+    verifyTableOwnerPrivilegeExistForEntity(statementUSER1_2, SentryPrincipalType.USER, Lists.newArrayList(USER1_2),
         DB1, null, 0);
 
     // verify that user user1_2 does not have any privilege on this database except create
@@ -187,7 +191,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase {
     statement.execute("CREATE DATABASE " + DB1);
 
     // verify no privileges created for new database
-    verifyTablePrivilegeExistForUser(statement, Lists.newArrayList(admin),
+    verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.USER, Lists.newArrayList(admin),
         DB1, null, 0);
 
     statement.close();
@@ -220,7 +224,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase {
     statementUSER1_1.execute("DROP DATABASE " + DB1 + " CASCADE");
 
     // verify owner privileges created for new database no longer exists
-    verifyTablePrivilegeExistForUser(statementUSER1_1, Lists.newArrayList(USER1_1),
+    verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1),
         DB1, null, 0);
 
     statement.close();
@@ -260,7 +264,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase {
 
 
     // verify privileges created for new table
-    verifyTablePrivilegeExistForUser(statementUSER1_1, Lists.newArrayList(USER1_1),
+    verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1),
         DB1, tableName1, 1);
 
     // verify that user has all privilege on this table, i.e., "OWNER" means "ALL"
@@ -312,7 +316,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase {
     // verify user1_2 does not have privileges on table created by user1_1
     Connection connectionUSER1_2 = hiveServer2.createConnection(USER1_2, USER1_2);
     Statement statementUSER1_2 = connectionUSER1_2.createStatement();
-    verifyTablePrivilegeExistForUser(statementUSER1_2, Lists.newArrayList(USER1_2),
+    verifyTableOwnerPrivilegeExistForEntity(statementUSER1_2, SentryPrincipalType.USER, Lists.newArrayList(USER1_2),
         DB1, tableName1, 0);
 
     // verify that user user1_2 does not have any privilege on this table
@@ -369,7 +373,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase {
         + " (under_col int comment 'the under column')");
 
     // verify no owner privileges created for new table
-    verifyTablePrivilegeExistForUser(statement, Lists.newArrayList(admin),
+    verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.USER, Lists.newArrayList(admin),
         DB1, tableName1, 0);
 
     statement.close();
@@ -404,17 +408,234 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase {
     statementUSER1_1.execute("DROP TABLE " + DB1 + "." + tableName1);
 
     // verify privileges created for new table
-    verifyTablePrivilegeExistForUser(statementUSER1_1, Lists.newArrayList(USER1_1),
+    verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1),
         DB1, tableName1, 0);
 
     statement.close();
     connection.close();
   }
 
-  
+  /**
+   * Verify that the owner privilege is updated when the ownership is changed
+   *
+   * @throws Exception
+   */
+  @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry")
+  @Test
+  public void testAlterTable() throws Exception {
+    dbNames = new String[]{DB1};
+    roles = new String[]{"admin_role", "create_db1", "owner_role"};
+
+    // create required roles
+    setupUserRoles(roles, statement);
+
+    // create test DB
+    statement.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE");
+    statement.execute("CREATE DATABASE " + DB1);
+
+    // setup privileges for USER1
+    statement.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE create_db1");
+    statement.execute("USE " + DB1);
+
+    // USER1 create table
+    Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1);
+    Statement statementUSER1_1 = connectionUSER1_1.createStatement();
+    statementUSER1_1.execute("CREATE TABLE " + DB1 + "." + tableName1
+        + " (under_col int comment 'the under column')");
+
+
+    // verify privileges created for new table
+    verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1),
+        DB1, tableName1, 1);
+
+    // verify that user has all privilege on this table, i.e., "OWNER" means "ALL"
+    // for authorization
+    statementUSER1_1.execute("INSERT INTO TABLE " + DB1 + "." + tableName1 + " VALUES (35)");
+
+    // Changing the owner to a role
+    statementUSER1_1.execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER ROLE "
+
+        "owner_role");
 
-  // TODO: once hive supports alter table set owner, need to add testing cases for owner
-  // privilege associated with role
+    // alter table rename is not blocked for notification processing in upstream due to
+    // hive bug HIVE-18783, which is fixed in Hive 2.4.0 and 3.0
+    Thread.sleep(WAIT_BEFORE_TESTVERIFY);
+
+    // Verify that old owner does not have owner privilege
+    verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1),
+        DB1, tableName1, 0);
+    // Verify that new owner has owner privilege
+
+    verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.ROLE, Lists.newArrayList("owner_role"),
+        DB1, tableName1, 1);
+
+
+    // Changing the owner to a user
+    statementUSER1_1.execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER USER "
+
+        USER1_1);
+
+    // Verify that old owner does not have owner privilege
+    verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.ROLE, Lists.newArrayList("owner_role"),
+        DB1, tableName1, 0);
+
+    // Verify that new owner has owner privilege
+    verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1),
+        DB1, tableName1, 1);
+
+    statement.close();
+    connection.close();
+
+    statementUSER1_1.close();
+    connectionUSER1_1.close();
+  }
+
+  /**
+   * Verify that the user who can call alter table set owner on this table
+   *
+   * @throws Exception
+   */
+  @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry")
+  @Test
+  public void testAuthorizeAlterTableSetOwner() throws Exception {
+    String ownerRole = "owner_role";
+    String allWithGrantRole = "allWithGrant_role";
+    dbNames = new String[]{DB1};
+    roles = new String[]{"admin_role", "create_db1", ownerRole};
+
+    // create required roles, and assign them to USERGROUP1
+    setupUserRoles(roles, statement);
+
+    // create test DB
+    statement.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE");
+    statement.execute("CREATE DATABASE " + DB1);
+
+    // setup privileges for USER1
+    statement.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE create_db1");
+    statement.execute("USE " + DB1);
+
+    // USER1_1 create table
+    Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1);
+    Statement statementUSER1_1 = connectionUSER1_1.createStatement();
+    statementUSER1_1.execute("CREATE TABLE " + DB1 + "." + tableName1
+        + " (under_col int comment 'the under column')");
+
+    // owner issues alter table set owner
+    if (!ownerPrivilegeGrantEnabled) {
+      try {
+        statementUSER1_1
+            .execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER ROLE " + ownerRole);
+        Assert.fail("Expect altering table set owner to fail for owner without grant option");
+      } catch (Exception ex) {
+        // owner without grant option cannot issue this command
+      }
+    }
+
+    // admin issues alter table set owner
+    try {
+      statement.execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER ROLE " + ownerRole);
+      Assert.fail("Expect altering table set owner to fail for admin");
+    } catch (Exception ex) {
+      // admin does not have grant option, so cannot issue this command
+    }
+
+    Connection connectionUSER2_1 = hiveServer2.createConnection(USER2_1, USER2_1);
+    Statement statementUSER2_1 = connectionUSER2_1.createStatement();
+
+    try {
+      // create role that has all with grant on the table
+      statement.execute("create role " + allWithGrantRole);
+      statement.execute("grant role " + allWithGrantRole + " to group " + USERGROUP2);
+      statement.execute("grant all on table " + DB1 + "." + tableName1 + " to role " +
+          allWithGrantRole + " with grant option");
+
+      // cannot issue command on a different table
+      try {
+        statementUSER2_1.execute("ALTER TABLE " + DB1 + ".non_exit_table" + " SET OWNER ROLE
" + ownerRole);
+        Assert.fail("Expect altering table set owner to fail on non-exist table");
+      } catch (Exception ex) {
+        // admin does not have grant option, so cannot issue this command
+      }
+
+      // user2_1 having all with grant on this table and can issue command: alter table set
owner
+      // alter table set owner to a role
+      statementUSER2_1
+          .execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER ROLE " + ownerRole);
+
+      // verify privileges is transferred to role owner_role, which is associated with USERGROUP1,
+      // therefore to USER1_1
+      verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.ROLE,
+          Lists.newArrayList(ownerRole),
+          DB1, tableName1, 1);
+
+      // alter table set owner to user USER1_1 and verify privileges is transferred to USER
USER1_1
+      statementUSER2_1
+          .execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER USER " + USER1_1);
+      verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.USER,
+          Lists.newArrayList(USER1_1), DB1, tableName1, 1);
+
+      // alter table set owner to user USER2_1, who already has explicit all with grant
+      statementUSER2_1
+          .execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER USER " + USER2_1);
+      verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.USER,
+          Lists.newArrayList(USER2_1),
+          DB1, tableName1, 1);
+
+    } finally {
+      statement.execute("drop role " + allWithGrantRole);
+
+      statement.close();
+      connection.close();
+
+      statementUSER1_1.close();
+      connectionUSER1_1.close();
+
+      statementUSER2_1.close();
+      connectionUSER2_1.close();
+    }
+  }
+
+  /**
+   * Verify that no owner privilege is granted when the ownership is changed to sentry admin
user
+   * @throws Exception
+   */
+  @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry")
+  @Test
+  public void testAlterTableAdmin() throws Exception {
+    dbNames = new String[]{DB1};
+    roles = new String[]{"admin_role", "create_db1"};
+
+    // create required roles
+    setupUserRoles(roles, statement);
+
+    // create test DB
+    statement.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE");
+    statement.execute("CREATE DATABASE " + DB1);
+
+    // setup privileges for USER1
+    statement.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE create_db1");
+    statement.execute("USE " + DB1);
+
+    // USER1 create table
+    Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1);
+    Statement statementUSER1_1 = connectionUSER1_1.createStatement();
+    statementUSER1_1.execute("CREATE TABLE " + DB1 + "." + tableName1
+        + " (under_col int comment 'the under column')");
+
+    // verify owner privileges created for new table
+    verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.USER, Lists.newArrayList(USER1_1),
+        DB1, tableName1, 1);
+
+    // Changing the owner to an admin user
+    statementUSER1_1.execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER USER "
+
+        admin);
+
+    // verify no owner privileges to the new owner as the owner is admin user
+
+    verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.USER, Lists.newArrayList(admin),
+        DB1, tableName1, 0);
+
+    statement.close();
+    connection.close();
+  }
 
   // Create test roles
   private void setupUserRoles(String[] roles, Statement statement) throws Exception {
@@ -428,36 +649,54 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase {
   }
 
   // verify given table is part of every user in the list
-  private void verifyTablePrivilegeExistForUser(Statement statement,
-      List<String> users, String dbName, String tableName, int expectedResultCount)
throws Exception {
+  // verify that each entity in the list has owner privilege on the given database or table
+  protected void verifyTableOwnerPrivilegeExistForEntity(Statement statement, SentryPrincipalType
entityType,
+      List<String> entities, String dbName, String tableName, int expectedResultCount)
throws Exception {
 
-    for (String userName : users) {
+    for (String entity : entities) {
       String command;
 
       if (tableName == null) {
-        command = "SHOW GRANT USER " + userName + " ON DATABASE " + dbName;
+        command = "SHOW GRANT " + entityType.toString() + " " + entity + " ON DATABASE "
+ dbName;
       } else {
-        command = "SHOW GRANT USER " + userName + " ON TABLE " + dbName + "." + tableName;
+        command = "SHOW GRANT " + entityType.toString() + " " + entity + " ON TABLE " + dbName
+ "." + tableName;
       }
 
       ResultSet resultSet = statement.executeQuery(command);
 
       int resultSize = 0;
       while(resultSet.next()) {
-        resultSize ++;
+
+        String actionValue = resultSet.getString(7);
+        if (!actionValue.equalsIgnoreCase("owner")) {
+          // only check owner privilege, and skip other privileges
+          continue;
+        }
 
         assertThat(resultSet.getString(1), equalToIgnoringCase(dbName)); // db name
 
+        String tableNameValue = resultSet.getString(2);
         if (tableName != null) {
-          assertThat(resultSet.getString(2), equalToIgnoringCase(tableName)); // table name
+          if (!tableNameValue.equalsIgnoreCase(tableName)) {
+            // it is possible the entity has owner privilege on both DB and table
+            // only check the owner privilege on table
+            continue;
+          }
+        } else {
+          if (!tableNameValue.equalsIgnoreCase("")) {
+            // it is possible the entity has owner privilege on both DB and table
+            // only check the owner privilege on db
+            continue;
+          }
         }
 
         assertThat(resultSet.getString(3), equalToIgnoringCase(""));//partition
         assertThat(resultSet.getString(4), equalToIgnoringCase(""));//column
-        assertThat(resultSet.getString(5), equalToIgnoringCase(userName));//principalName
-        assertThat(resultSet.getString(6), equalToIgnoringCase("user"));//principalType
-        assertThat(resultSet.getString(7), equalToIgnoringCase("owner"));
+        assertThat(resultSet.getString(5), equalToIgnoringCase(entity));//principalName
+        assertThat(resultSet.getString(6), equalToIgnoringCase(entityType.toString()));//principalType
         assertThat(resultSet.getBoolean(8), is(ownerPrivilegeGrantEnabled));//grantOption
+
+        resultSize ++;
       }
 
       assertEquals(expectedResultCount, resultSize);
@@ -465,5 +704,4 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase {
       resultSet.close();
     }
   }
-
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
index 4588963..f5e4db8 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
@@ -622,6 +622,7 @@ public abstract class TestHDFSIntegrationBase {
         hiveConf.set("hive.metastore.uris", "thrift://localhost:" + hmsPort);
         hiveConf.set("hive.metastore.pre.event.listeners", "org.apache.sentry.binding.metastore.MetastoreAuthzBinding");
         hiveConf.set("hive.metastore.transactional.event.listeners", "org.apache.hive.hcatalog.listener.DbNotificationListener");
+        hiveConf.set("hive.metastore.event.listeners", "org.apache.sentry.binding.metastore.SentrySyncHMSNotificationsPostEventListener");
         hiveConf.set("hive.metastore.event.message.factory", "org.apache.sentry.binding.metastore.messaging.json.SentryJSONMessageFactory");
         hiveConf.set("hive.security.authorization.task.factory", "org.apache.sentry.binding.hive.SentryHiveAuthorizationTaskFactoryImpl");
         hiveConf.set("hive.server2.session.hook", "org.apache.sentry.binding.hive.HiveAuthzBindingSessionHook");


Mime
View raw message