Repository: sentry
Updated Branches:
  refs/heads/master f0f9d620a -> e0d99fef5


SENTRY-2308: Create privilege on table has no use case (Sergio Pena, reviewed 
by Na Li, Arjun Mishra)


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

Branch: refs/heads/master
Commit: e0d99fef5c23e34613a0d90705fa61346816cb78
Parents: f0f9d62
Author: Sergio Pena <sergio.p...@cloudera.com>
Authored: Fri Aug 17 12:15:24 2018 -0500
Committer: Sergio Pena <sergio.p...@cloudera.com>
Committed: Fri Aug 17 12:16:06 2018 -0500

----------------------------------------------------------------------
 .../SentryHiveAuthorizationTaskFactoryImpl.java |  58 +++++++--
 .../authz/DefaultSentryAccessController.java    |   8 ++
 .../e2e/hive/TestAllowedGrantPrivileges.java    | 125 +++++++++++++++++++
 .../hive/TestDescribeMetadataPrivileges.java    |   5 +
 4 files changed, 189 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/e0d99fef/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
----------------------------------------------------------------------
diff --git 
a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
 
b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
index 2c662e5..aab9374 100644
--- 
a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
+++ 
b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
@@ -42,7 +42,6 @@ import org.apache.hadoop.hive.ql.plan.GrantDesc;
 import org.apache.hadoop.hive.ql.plan.GrantRevokeRoleDDL;
 import org.apache.hadoop.hive.ql.plan.PrincipalDesc;
 import org.apache.hadoop.hive.ql.plan.PrivilegeDesc;
-import org.apache.hadoop.hive.ql.plan.PrivilegeObjectDesc;
 import org.apache.hadoop.hive.ql.plan.RevokeDesc;
 import org.apache.hadoop.hive.ql.plan.RoleDDLDesc;
 import org.apache.hadoop.hive.ql.plan.ShowGrantDesc;
@@ -51,6 +50,7 @@ import 
org.apache.hadoop.hive.ql.security.authorization.PrivilegeRegistry;
 import org.apache.hadoop.hive.ql.security.authorization.PrivilegeType;
 import org.apache.hadoop.hive.ql.session.SessionState;
 import org.apache.sentry.core.model.db.AccessConstants;
+import org.apache.sentry.core.model.db.DBModelAuthorizable.AuthorizableType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -151,16 +151,60 @@ public class SentryHiveAuthorizationTaskFactoryImpl 
implements HiveAuthorization
         throw new SemanticException(msg);
       }
     }
+
+    // Throws an exception if the action is not allowed to be granted on the 
object
+    checkAllowedPrivileges(privilegeObj, privilegeDesc);
+
     GrantDesc grantDesc = new GrantDesc(privilegeObj, privilegeDesc,
         principalDesc, userName, PrincipalType.USER, grantOption);
     return createTask(new DDLWork(inputs, outputs, grantDesc));
   }
+
+  /**
+   * Checks if a privilege is allowed to be granted in an object. It throws an 
exception
+   * if it is not allowed.
+   *
+   * @param privilegeObj The privilege object to get the authorizable type
+   * @param privilegeDesc The list of privileges to check
+   * @throws SemanticException If one or more of privileges are not allowed on 
the object
+   */
+  private void checkAllowedPrivileges(SentryHivePrivilegeObjectDesc 
privilegeObj,
+    List<PrivilegeDesc> privilegeDesc) throws SemanticException {
+    AuthorizableType authorizableType = null;
+
+    // Get the scope (or authorizable type) of the grant privilege
+    if (privilegeObj.getUri()) {
+      authorizableType = AuthorizableType.URI;
+    } else if (privilegeObj.getTable()) {
+      authorizableType = AuthorizableType.Table;
+    } else if (privilegeObj.getDatabase()) {
+      authorizableType = AuthorizableType.Db;
+    } else if (privilegeObj.getServer()) {
+      authorizableType = AuthorizableType.Server;
+    }
+
+    for (PrivilegeDesc privilege : privilegeDesc) {
+      PrivilegeType privType = privilege.getPrivilege().getPriv();
+
+      if (authorizableType == AuthorizableType.Table) {
+        // If a privilege comes with columns, then we treated this as column 
privileges
+        if (privilege.getColumns() != null && 
!privilege.getColumns().isEmpty() && privType != PrivilegeType.SELECT) {
+          String msg = SentryHiveConstants.PRIVILEGE_NOT_SUPPORTED + privType 
+ " on Column";
+          throw new SemanticException(msg);
+        } else if (privType == PrivilegeType.CREATE) {
+          String msg = SentryHiveConstants.PRIVILEGE_NOT_SUPPORTED + privType 
+ " on Table";
+          throw new SemanticException(msg);
+        }
+      }
+    }
+  }
+
   @Override
   public Task<? extends Serializable> createRevokeTask(ASTNode ast, 
HashSet<ReadEntity> inputs,
       HashSet<WriteEntity> outputs) throws SemanticException {
     List<PrivilegeDesc> privilegeDesc = analyzePrivilegeListDef((ASTNode) 
ast.getChild(0));
     List<PrincipalDesc> principalDesc = analyzePrincipalListDef((ASTNode) 
ast.getChild(1));
-    PrivilegeObjectDesc privilegeObj = null;
+    SentryHivePrivilegeObjectDesc privilegeObj = null;
     if (ast.getChildCount() > 2) {
       ASTNode astChild = (ASTNode) ast.getChild(2);
       privilegeObj = analyzePrivilegeObject(astChild);
@@ -174,6 +218,10 @@ public class SentryHiveAuthorizationTaskFactoryImpl 
implements HiveAuthorization
         throw new SemanticException(msg);
       }
     }
+
+    // Throws an exception if the action is not allowed to be granted on the 
object
+    checkAllowedPrivileges(privilegeObj, privilegeDesc);
+
     RevokeDesc revokeDesc = new RevokeDesc(privilegeDesc, principalDesc, 
privilegeObj);
     return createTask(new DDLWork(inputs, outputs, revokeDesc));
   }
@@ -398,11 +446,7 @@ public class SentryHiveAuthorizationTaskFactoryImpl 
implements HiveAuthorization
       if (privilegeDef.getChildCount() > 1) {
         cols = BaseSemanticAnalyzer.getColumnNames((ASTNode) 
privilegeDef.getChild(1));
       }
-      // Columns accept only SELECT privileges
-      if (cols != null && !privObj.getPriv().equals(PrivilegeType.SELECT)) {
-        String msg = SentryHiveConstants.PRIVILEGE_NOT_SUPPORTED + 
privObj.getPriv() + " on Column";
-        throw new SemanticException(msg);
-      }
+
       PrivilegeDesc privilegeDesc = new PrivilegeDesc(privObj, cols);
       ret.add(privilegeDesc);
     }

http://git-wip-us.apache.org/repos/asf/sentry/blob/e0d99fef/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
----------------------------------------------------------------------
diff --git 
a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
 
b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
index beca2f8..5db796d 100644
--- 
a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
+++ 
b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
@@ -522,6 +522,14 @@ public class DefaultSentryAccessController extends 
SentryHiveAccessController {
                       action, grantOp);
                 }
               } else {
+                // Table does not accept CREATE privileges
+                if (action.equalsIgnoreCase(AccessConstants.CREATE)) {
+                  String msg =
+                    SentryHiveConstants.PRIVILEGE_NOT_SUPPORTED + 
privilege.getName()
+                      + " on Table";
+                  throw new HiveAuthzPluginException(msg);
+                }
+
                 if (isGrant) {
                   sentryClient.grantTablePrivilege(grantorName, roleName, 
serverName,
                       hivePrivObject.getDbname(), 
hivePrivObject.getObjectName(), action, grantOp);

http://git-wip-us.apache.org/repos/asf/sentry/blob/e0d99fef/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestAllowedGrantPrivileges.java
----------------------------------------------------------------------
diff --git 
a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestAllowedGrantPrivileges.java
 
b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestAllowedGrantPrivileges.java
new file mode 100644
index 0000000..79c075c
--- /dev/null
+++ 
b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestAllowedGrantPrivileges.java
@@ -0,0 +1,125 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sentry.tests.e2e.hive;
+
+import com.google.common.collect.Sets;
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Set;
+import org.apache.sentry.core.model.db.DBModelAction;
+import org.apache.sentry.core.model.db.DBModelAuthorizable.AuthorizableType;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import static 
org.apache.sentry.core.model.db.DBModelAuthorizable.AuthorizableType.Server;
+import static 
org.apache.sentry.core.model.db.DBModelAuthorizable.AuthorizableType.Db;
+import static 
org.apache.sentry.core.model.db.DBModelAuthorizable.AuthorizableType.Table;
+import static 
org.apache.sentry.core.model.db.DBModelAuthorizable.AuthorizableType.Column;
+import static org.junit.Assert.fail;
+
+@RunWith(Parameterized.class)
+public class TestAllowedGrantPrivileges extends 
AbstractTestWithStaticConfiguration  {
+  private static Connection adminCon;
+  private static Statement adminStmt;
+  private static final String SERVER1 = "server1";
+
+  private DBModelAction action;
+  private Set<AuthorizableType> allowedScope;
+
+  @Parameterized.Parameters
+  public static Collection getPrivileges() {
+    return Arrays.asList(new Object[][] {
+      { DBModelAction.ALL,     Sets.newHashSet(Server, Db, Table) },
+      { DBModelAction.CREATE,  Sets.newHashSet(Server, Db) },
+      { DBModelAction.SELECT,  Sets.newHashSet(Server, Db, Table, Column) },
+      { DBModelAction.INSERT,  Sets.newHashSet(Server, Db, Table) },
+      { DBModelAction.ALTER,   Sets.newHashSet(Server, Db, Table) },
+      { DBModelAction.DROP,    Sets.newHashSet(Server, Db, Table) },
+      { DBModelAction.INDEX,   Sets.newHashSet(Server, Db, Table) },
+      { DBModelAction.LOCK,    Sets.newHashSet(Server, Db, Table) }
+    });
+  }
+
+  @BeforeClass
+  public static void setupTestStaticConfiguration() throws Exception{
+    useSentryService = true;
+    AbstractTestWithStaticConfiguration.setupTestStaticConfiguration();
+    setupAdmin();
+
+    adminCon = context.createConnection(ADMIN1);
+    adminStmt = context.createStatement(adminCon);
+  }
+
+  @AfterClass
+  public static void destroy() throws SQLException {
+    adminStmt.close();
+    adminCon.close();
+  }
+
+  public TestAllowedGrantPrivileges(DBModelAction action, 
Set<AuthorizableType> allowedScope) {
+    this.action = action;
+    this.allowedScope = allowedScope;
+  }
+
+  @Before
+  public void setup() throws Exception {
+    adminStmt.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE");
+    adminStmt.execute("CREATE DATABASE " + DB1);
+    adminStmt.execute("CREATE TABLE " + DB1 + "." + TBL1 + " (id int)");
+    adminStmt.execute("CREATE ROLE role1");
+  }
+
+  private void verifyGrantCommand(String command, AuthorizableType scope) {
+    try {
+      adminStmt.execute(command);
+      if (!allowedScope.contains(scope)) {
+        fail(String.format("GRANT %s should NOT be allowed on the %s scope.", 
action, scope));
+      }
+    } catch (Exception e) {
+      if (allowedScope.contains(scope)) {
+        fail(String.format("GRANT %s should be allowed on the %s scope.", 
action, scope));
+      }
+    }
+  }
+
+  @Test
+  public void testGrantOnServer() {
+    verifyGrantCommand("GRANT " + action + " ON SERVER " + SERVER1 + " TO ROLE 
role1", Server);
+  }
+
+  @Test
+  public void testGrantOnDatabase() {
+    verifyGrantCommand("GRANT " + action + " ON DATABASE " + DB1 + " TO ROLE 
role1", Db);
+  }
+
+  @Test
+  public void testGrantOnTable() {
+    verifyGrantCommand("GRANT " + action + " ON TABLE " + DB1 + "." + TBL1 + " 
TO ROLE role1", Table);
+  }
+
+  @Test
+  public void testGrantOnColumn() {
+    verifyGrantCommand("GRANT " + action + "(id) ON TABLE " + DB1 + "." + TBL1 
+ " TO ROLE role1", Column);
+  }
+}

http://git-wip-us.apache.org/repos/asf/sentry/blob/e0d99fef/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestDescribeMetadataPrivileges.java
----------------------------------------------------------------------
diff --git 
a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestDescribeMetadataPrivileges.java
 
b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestDescribeMetadataPrivileges.java
index 54f4f2f..e0a0134 100644
--- 
a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestDescribeMetadataPrivileges.java
+++ 
b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestDescribeMetadataPrivileges.java
@@ -95,6 +95,11 @@ public class TestDescribeMetadataPrivileges extends 
AbstractTestWithStaticConfig
   @Test
   public void testDescribeTableWithGrantOnTable() throws Exception {
     if (action != null) {
+      if (action == DBModelAction.CREATE) {
+        // CREATE is not supported at the table level
+        return;
+      }
+      
       adminStmt.execute("GRANT " + action + " ON TABLE " + DB1 + "." + TBL1 + 
" TO ROLE role1");
     }
 

Reply via email to