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"); }