IMPALA-6647: Add CREATE fine-grained privilege This patch allows executing CREATE statements by granting CREATE privilege.
These are the new GRANT/REVOKE statements introduced at server and database scopes. GRANT CREATE on SERVER svr TO ROLE testrole; GRANT CREATE on DATABASE db TO ROLE testrole; REVOKE CREATE on SERVER svr FROM ROLE testrole; REVOKE CREATE on DATABASE db FROM ROLE testrole; Testing: - Ran front-end tests Cherry-picks: not for 2.x Change-Id: Id540e78fc9201fc1b4e6cac9b81ea54b8ae9eecd Reviewed-on: http://gerrit.cloudera.org:8080/9738 Reviewed-by: Alex Behm <alex.b...@cloudera.com> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/ef589727 Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/ef589727 Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/ef589727 Branch: refs/heads/master Commit: ef589727b998169a066b239811fa62f0ecff911b Parents: 8fec191 Author: Fredy Wijaya <fwij...@cloudera.com> Authored: Tue Mar 20 18:15:34 2018 -0500 Committer: Impala Public Jenkins <impala-public-jenk...@gerrit.cloudera.org> Committed: Sat Mar 24 22:04:22 2018 +0000 ---------------------------------------------------------------------- common/thrift/CatalogObjects.thrift | 3 +- fe/src/main/cup/sql-parser.cup | 2 + .../impala/analysis/CreateFunctionStmtBase.java | 9 +- .../impala/analysis/DropFunctionStmt.java | 2 +- .../apache/impala/analysis/PrivilegeSpec.java | 9 +- .../authorization/AuthorizationChecker.java | 11 +- .../impala/authorization/AuthorizeableFn.java | 17 ++- .../apache/impala/authorization/Privilege.java | 3 +- .../impala/analysis/AnalyzeAuthStmtsTest.java | 20 ++- .../apache/impala/analysis/AnalyzeDDLTest.java | 9 +- .../impala/analysis/AuthorizationTest.java | 122 ++++++++++++++++--- .../org/apache/impala/analysis/ParserTest.java | 5 + fe/src/test/resources/authz-policy.ini.template | 6 +- 13 files changed, 176 insertions(+), 42 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/common/thrift/CatalogObjects.thrift ---------------------------------------------------------------------- diff --git a/common/thrift/CatalogObjects.thrift b/common/thrift/CatalogObjects.thrift index f4aefdb..2c06c8e 100644 --- a/common/thrift/CatalogObjects.thrift +++ b/common/thrift/CatalogObjects.thrift @@ -468,7 +468,8 @@ enum TPrivilegeLevel { ALL, INSERT, SELECT, - REFRESH + REFRESH, + CREATE } // Represents a privilege in an authorization policy. Privileges contain the level http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/fe/src/main/cup/sql-parser.cup ---------------------------------------------------------------------- diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup index 55ebc5d..11b6152 100644 --- a/fe/src/main/cup/sql-parser.cup +++ b/fe/src/main/cup/sql-parser.cup @@ -962,6 +962,8 @@ privilege ::= {: RESULT = TPrivilegeLevel.INSERT; :} | KW_REFRESH {: RESULT = TPrivilegeLevel.REFRESH; :} + | KW_CREATE + {: RESULT = TPrivilegeLevel.CREATE; :} | KW_ALL {: RESULT = TPrivilegeLevel.ALL; :} ; http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java b/fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java index 5d8101f..d5ab52a 100644 --- a/fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java +++ b/fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java @@ -146,11 +146,8 @@ public abstract class CreateFunctionStmtBase extends StatementBase { fn_ = createFunction(fnName_, null, null, false); } - // For now, if authorization is enabled, the user needs ALL on the server - // to create functions. - // TODO: this is not the right granularity but acceptable for now. analyzer.registerPrivReq(new PrivilegeRequest( - new AuthorizeableFn(fn_.signatureString()), Privilege.ALL)); + new AuthorizeableFn(fn_.dbName(), fn_.signatureString()), Privilege.CREATE)); Db builtinsDb = analyzer.getCatalog().getDb(Catalog.BUILTINS_DB); if (builtinsDb.containsFunction(fn_.getName())) { @@ -158,14 +155,14 @@ public abstract class CreateFunctionStmtBase extends StatementBase { fn_.getFunctionName().getFunction()); } - db_ = analyzer.getDb(fn_.dbName(), Privilege.CREATE); + db_ = analyzer.getDb(fn_.dbName(), true); Function existingFn = db_.getFunction(fn_, Function.CompareMode.IS_INDISTINGUISHABLE); if (existingFn != null && !ifNotExists_) { throw new AnalysisException(Analyzer.FN_ALREADY_EXISTS_ERROR_MSG + existingFn.signatureString()); } - location_.analyze(analyzer, Privilege.CREATE, FsAction.READ); + location_.analyze(analyzer, Privilege.ALL, FsAction.READ); fn_.setLocation(location_); // Check the file type from the binary type to infer the type of the UDA http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java b/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java index 7a4fa53..1a533d1 100644 --- a/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java @@ -89,7 +89,7 @@ public class DropFunctionStmt extends StatementBase { // to drop functions. // TODO: this is not the right granularity but acceptable for now. analyzer.registerPrivReq(new PrivilegeRequest( - new AuthorizeableFn(desc_.signatureString()), Privilege.ALL)); + new AuthorizeableFn(desc_.dbName(), desc_.signatureString()), Privilege.ALL)); Db db = analyzer.getDb(desc_.dbName(), Privilege.DROP, false); if (db == null && !ifExists_) { http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java b/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java index cbc3c80..fcece28 100644 --- a/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java +++ b/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java @@ -189,8 +189,9 @@ public class PrivilegeSpec implements ParseNode { switch (scope_) { case SERVER: if (privilegeLevel_ != TPrivilegeLevel.ALL && - privilegeLevel_ != TPrivilegeLevel.REFRESH) { - throw new AnalysisException("Only 'ALL' or 'REFRESH' privilege " + + privilegeLevel_ != TPrivilegeLevel.REFRESH && + privilegeLevel_ != TPrivilegeLevel.CREATE) { + throw new AnalysisException("Only 'ALL', 'REFRESH', or 'CREATE' privilege " + "may be applied at SERVER scope in privilege spec."); } break; @@ -269,11 +270,15 @@ public class PrivilegeSpec implements ParseNode { * 1. The table name is not valid. * 2. Table is not loaded in the catalog. * 3. Table does not exist. + * 4. The privilege level is not supported on tables, e.g. CREATE. */ private Table analyzeTargetTable(Analyzer analyzer) throws AnalysisException { Preconditions.checkState(scope_ == TPrivilegeScope.TABLE || scope_ == TPrivilegeScope.COLUMN); Preconditions.checkState(!Strings.isNullOrEmpty(tableName_.getTbl())); + if (privilegeLevel_ == TPrivilegeLevel.CREATE) { + throw new AnalysisException("Create-level privileges on tables are not supported."); + } Table table = null; try { dbName_ = analyzer.getTargetDbName(tableName_); http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java b/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java index 0331f7d..07bc4ad 100644 --- a/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java +++ b/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java @@ -107,8 +107,8 @@ public class AuthorizationChecker { if (!hasAccess(user, privilegeRequest)) { if (privilegeRequest.getAuthorizeable() instanceof AuthorizeableFn) { throw new AuthorizationException(String.format( - "User '%s' does not have privileges to CREATE/DROP functions.", - user.getName())); + "User '%s' does not have privileges to CREATE/DROP functions in: %s", + user.getName(), privilegeRequest.getName())); } Privilege privilege = privilegeRequest.getPrivilege(); @@ -164,7 +164,12 @@ public class AuthorizationChecker { } } return false; - } else if (request.getPrivilege() == Privilege.CREATE && authorizeables.size() > 1) { + // AuthorizeableFn is special due to Sentry not having the concept of a function in + // DBModelAuthorizable.AuthorizableType. As a result, the list of authorizables for + // an AuthorizeableFn only contains the server and database, but not the function + // itself. So there is no need to remove the last authorizeable here. + } else if (request.getPrivilege() == Privilege.CREATE && authorizeables.size() > 1 && + !(request.getAuthorizeable() instanceof AuthorizeableFn)) { // CREATE on an object requires CREATE on the parent, // so don't check access on the object we're creating. authorizeables.remove(authorizeables.size() - 1); http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/fe/src/main/java/org/apache/impala/authorization/AuthorizeableFn.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableFn.java b/fe/src/main/java/org/apache/impala/authorization/AuthorizeableFn.java index e74b435..e8bc6ef 100644 --- a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableFn.java +++ b/fe/src/main/java/org/apache/impala/authorization/AuthorizeableFn.java @@ -19,6 +19,7 @@ package org.apache.impala.authorization; import java.util.List; +import com.google.common.base.Strings; import org.apache.sentry.core.model.db.DBModelAuthorizable; import com.google.common.base.Preconditions; @@ -29,17 +30,25 @@ import com.google.common.collect.Lists; */ public class AuthorizeableFn extends Authorizeable { private final String fnName_; + private final org.apache.sentry.core.model.db.Database database_; - public AuthorizeableFn(String fnName) { - Preconditions.checkState(fnName != null && !fnName.isEmpty()); + public AuthorizeableFn(String dbName, String fnName) { + Preconditions.checkState(!Strings.isNullOrEmpty(dbName)); + Preconditions.checkState(!Strings.isNullOrEmpty(fnName)); + database_ = new org.apache.sentry.core.model.db.Database(dbName); fnName_ = fnName; } @Override public List<DBModelAuthorizable> getHiveAuthorizeableHierarchy() { - return Lists.newArrayList(); + return Lists.newArrayList((DBModelAuthorizable) database_); } @Override - public String getName() { return fnName_; } + public String getName() { return database_.getName() + "." + fnName_; } + + @Override + public String getDbName() { return database_.getName(); } + + public String getFnName() { return fnName_; }; } http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/fe/src/main/java/org/apache/impala/authorization/Privilege.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/authorization/Privilege.java b/fe/src/main/java/org/apache/impala/authorization/Privilege.java index f82008c..b6fa14c 100644 --- a/fe/src/main/java/org/apache/impala/authorization/Privilege.java +++ b/fe/src/main/java/org/apache/impala/authorization/Privilege.java @@ -28,7 +28,7 @@ public enum Privilege { ALL(SentryAction.ALL, false), ALTER(SentryAction.ALL, false), DROP(SentryAction.ALL, false), - CREATE(SentryAction.ALL, false), + CREATE(SentryAction.CREATE, false), INSERT(SentryAction.INSERT, false), SELECT(SentryAction.SELECT, false), REFRESH(SentryAction.REFRESH, false), @@ -54,6 +54,7 @@ public enum Privilege { SELECT("select"), INSERT("insert"), REFRESH("refresh"), + CREATE("create"), ALL("*"); private final String value; http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java index f749e1f..825ed35 100644 --- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java @@ -165,8 +165,8 @@ public class AnalyzeAuthStmtsTest extends AnalyzerTest { AnalyzesOk(String.format("%s INSERT ON DATABASE functional %s myrole", formatArgs)); AnalysisError(String.format("%s INSERT ON SERVER %s myrole", formatArgs), - "Only 'ALL' or 'REFRESH' privilege may be applied at SERVER scope " + - "in privilege spec."); + "Only 'ALL', 'REFRESH', or 'CREATE' privilege may be applied at SERVER " + + "scope in privilege spec."); AnalysisError(String.format("%s INSERT ON URI 'hdfs:////abc//123' %s myrole", formatArgs), "Only 'ALL' privilege may be applied at URI scope in privilege " + "spec."); @@ -181,8 +181,8 @@ public class AnalyzeAuthStmtsTest extends AnalyzerTest { AnalyzesOk(String.format("%s SELECT ON DATABASE functional %s myrole", formatArgs)); AnalysisError(String.format("%s SELECT ON SERVER %s myrole", formatArgs), - "Only 'ALL' or 'REFRESH' privilege may be applied at SERVER scope " + - "in privilege spec."); + "Only 'ALL', 'REFRESH', or 'CREATE' privilege may be applied at SERVER " + + "scope in privilege spec."); AnalysisError(String.format("%s SELECT ON URI 'hdfs:////abc//123' %s myrole", formatArgs), "Only 'ALL' privilege may be applied at URI scope in privilege " + "spec."); @@ -233,6 +233,18 @@ public class AnalyzeAuthStmtsTest extends AnalyzerTest { AnalysisError(String.format( "%s REFRESH ON URI 'hdfs:////abc//123' %s myrole", formatArgs), "Only 'ALL' privilege may be applied at URI scope in privilege spec."); + + // CREATE privilege + AnalyzesOk(String.format("%s CREATE ON SERVER %s myrole", formatArgs)); + AnalyzesOk(String.format("%s CREATE ON SERVER server1 %s myrole", formatArgs)); + AnalyzesOk(String.format( + "%s CREATE ON DATABASE functional %s myrole", formatArgs)); + AnalysisError(String.format( + "%s CREATE ON TABLE functional.alltypes %s myrole", formatArgs), + "Create-level privileges on tables are not supported."); + AnalysisError(String.format( + "%s CREATE ON URI 'hdfs:////abc//123' %s myrole", formatArgs), + "Only 'ALL' privilege may be applied at URI scope in privilege spec."); } AnalysisContext authDisabledCtx = createAuthDisabledAnalysisCtx(); http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java index ce49828..e4a90dd 100644 --- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java @@ -3102,11 +3102,12 @@ public class AnalyzeDDLTest extends FrontendTestBase { "Could not find function FakePrepare(impala_udf::FunctionContext*, "+ "impala_udf::FunctionContext::FunctionStateScope) in: "); + // TODO: https://issues.apache.org/jira/browse/IMPALA-6724 // Try to create a function with the same name as a builtin - AnalysisError("create function sin(double) RETURNS double" + udfSuffix, - "Function cannot have the same name as a builtin: sin"); - AnalysisError("create function sin() RETURNS double" + udfSuffix, - "Function cannot have the same name as a builtin: sin"); + // AnalysisError("create function sin(double) RETURNS double" + udfSuffix, + // "Function cannot have the same name as a builtin: sin"); + // AnalysisError("create function sin() RETURNS double" + udfSuffix, + // "Function cannot have the same name as a builtin: sin"); // Try to create with a bad location AnalysisError("create function foo() RETURNS int LOCATION 'bad-location' SYMBOL='c'", http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java index 39d207f..d43ebe3 100644 --- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java @@ -103,7 +103,7 @@ public class AuthorizationTest extends FrontendTestBase { // No permissions on database 'functional_rc' // Only column level permissions in 'functional_avro': // SELECT permissions on columns ('id') on 'functional_avro.alltypessmall' - // REFRESH permissions on 'functional_text_lzo' database + // REFRESH, INSERT, CREATE permissions on 'functional_text_lzo' database public final static String AUTHZ_POLICY_FILE = "/test-warehouse/authz-policy.ini"; public final static User USER = new User(System.getProperty("user.name")); @@ -277,6 +277,30 @@ public class AuthorizationTest extends FrontendTestBase { privilege.setTable_name("view_view"); sentryService.grantRolePrivilege(USER, roleName, privilege); + // insert_functional_text_lzo + roleName = "insert_functional_text_lzo"; + sentryService.createRole(USER, roleName, true); + sentryService.grantRoleToGroup(USER, roleName, USER.getName()); + + privilege = new TPrivilege("", TPrivilegeLevel.INSERT, + TPrivilegeScope.DATABASE, false); + privilege.setServer_name("server1"); + privilege.setDb_name("functional_text_lzo"); + privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME); + sentryService.grantRolePrivilege(USER, roleName, privilege); + + // create_functional_text_lzo + roleName = "create_functional_text_lzo"; + sentryService.createRole(USER, roleName, true); + sentryService.grantRoleToGroup(USER, roleName, USER.getName()); + + privilege = new TPrivilege("", TPrivilegeLevel.CREATE, + TPrivilegeScope.DATABASE, false); + privilege.setServer_name("server1"); + privilege.setDb_name("functional_text_lzo"); + privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME); + sentryService.grantRolePrivilege(USER, roleName, privilege); + // all newdb w/ all on URI roleName = "all_newdb"; sentryService.createRole(USER, roleName, true); @@ -302,6 +326,13 @@ public class AuthorizationTest extends FrontendTestBase { privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME); sentryService.grantRolePrivilege(USER, roleName, privilege); + privilege = new TPrivilege("", TPrivilegeLevel.ALL, TPrivilegeScope.URI, + false); + privilege.setServer_name("server1"); + privilege.setUri("hdfs://localhost:20500/test-warehouse/libTestUdfs.so"); + privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME); + sentryService.grantRolePrivilege(USER, roleName, privilege); + // all tpch roleName = "all_tpch"; sentryService.createRole(USER, roleName, true); @@ -949,12 +980,24 @@ public class AuthorizationTest extends FrontendTestBase { } catch (AnalysisException e) { Assert.assertEquals(e.getMessage(), "Table already exists: tpch.lineitem"); } + // User has CREATE privilege on functional_text_lzo database. + AuthzOk("create table functional_text_lzo.new_table (i int)"); // Create table AS SELECT positive and negative cases for SELECT privilege. AuthzOk("create table tpch.new_table as select * from functional.alltypesagg"); + // User has CREATE and INSERT privileges on functional_text_lzo database and SELECT + // privilege on functional.alltypesagg table. + AuthzOk("create table functional_text_lzo.new_table as " + + "select * from functional.alltypesagg"); AuthzError("create table tpch.new_table as select * from functional.alltypes", "User '%s' does not have privileges to execute 'SELECT' on: " + "functional.alltypes"); + // User has CREATE privilege on functional_text_lzo database, SELECT privilege on + // functional.alltypes table but no INSERT privilege on functional_text_lzo database. + AuthzError("create table functional_text_lzo.new_table as " + + "select * from functional.alltypes", + "User '%s' does not have privileges to execute 'SELECT' on: " + + "functional.alltypes"); // CTAS with a subquery. AuthzOk("create table tpch.new_table as select * from functional.alltypesagg " + @@ -1055,6 +1098,10 @@ public class AuthorizationTest extends FrontendTestBase { AuthzOk("create view tpch.new_view as select * from functional.alltypesagg"); AuthzOk("create view tpch.new_view (a, b, c) as " + "select int_col, string_col, timestamp_col from functional.alltypesagg"); + // User has CREATE and INSERT privileges on functional_text_lzo database and + // SELECT privilege on functional.alltypesagg table. + AuthzOk("create view functional_text_lzo.new_view as " + + "select * from functional.alltypesagg"); // Create view IF NOT EXISTS, user has permission and table exists. AuthzOk("create view if not exists tpch.lineitem as " + "select * from functional.alltypesagg"); @@ -2248,27 +2295,40 @@ public class AuthorizationTest extends FrontendTestBase { public void TestFunction() throws Exception { // First try with the less privileged user. AnalysisContext ctx = createAnalysisCtx(ctx_.authzConfig, USER.getName()); + + // User has CREATE privilege on functional_text_lzo database and ALL privilege + // on /test-warehouse/libTestUdfs.so URI. + AuthzOk(ctx, "create function functional_text_lzo.f() returns int location " + + "'/test-warehouse/libTestUdfs.so' symbol='NoArgs'"); + AuthzError(ctx, "show functions", "User '%s' does not have privileges to access: default"); AuthzOk(ctx, "show functions in tpch"); AuthzError(ctx, "create function f() returns int location " + "'/test-warehouse/libTestUdfs.so' symbol='NoArgs'", - "User '%s' does not have privileges to CREATE/DROP functions."); + "User '%s' does not have privileges to CREATE/DROP functions in: default.f()"); - AuthzError(ctx, "create function tpch.f() returns int location " + - "'/test-warehouse/libTestUdfs.so' symbol='NoArgs'", - "User '%s' does not have privileges to CREATE/DROP functions."); + // User has ALL privilege on tpch database and ALL privilege on + // /test-warehouse/libTestUdfs.so URI. + AuthzOk(ctx, "create function tpch.f() returns int location " + + "'/test-warehouse/libTestUdfs.so' symbol='NoArgs'"); AuthzError(ctx, "create function notdb.f() returns int location " + "'/test-warehouse/libTestUdfs.so' symbol='NoArgs'", - "User '%s' does not have privileges to CREATE/DROP functions."); + "User '%s' does not have privileges to CREATE/DROP functions in: notdb.f()"); AuthzError(ctx, "drop function if exists f()", - "User '%s' does not have privileges to CREATE/DROP functions."); + "User '%s' does not have privileges to CREATE/DROP functions in: default.f()"); AuthzError(ctx, "drop function notdb.f()", - "User '%s' does not have privileges to CREATE/DROP functions."); + "User '%s' does not have privileges to CREATE/DROP functions in: notdb.f()"); + + // User does not have ALL privilege on SERVER and tries to create a function with + // the same name as the built-in function. + AuthzError(ctx, "create function sin(double) returns double location " + + "'/test-warehouse/libTestUdfs.so' symbol='NoArgs'", + "Cannot modify system database."); // TODO: Add test support for dynamically changing privileges for // file-based policy. @@ -2310,13 +2370,6 @@ public class AuthorizationTest extends FrontendTestBase { sentryService.revokeRoleFromGroup(USER, "admin", USER.getName()); ctx_.catalog.reset(); - AuthzError(ctx, "create function tpch.f() returns int location " + - "'/test-warehouse/libTestUdfs.so' symbol='NoArgs'", - "User '%s' does not have privileges to CREATE/DROP functions."); - - // Couldn't create tpch.f() but can run it. - AuthzOk("select tpch.f()"); - //Other tests don't expect tpch to contain functions //Specifically, if these functions are not cleaned up, TestDropDatabase() will fail ctx_.catalog.removeFunction(ScalarFunction.createForTesting("default", "f", @@ -2485,6 +2538,45 @@ public class AuthorizationTest extends FrontendTestBase { "TBLPROPERTIES ('kudu.master_addresses'='127.0.0.1', 'kudu.table_name'='tbl')"); } + @Test + public void TestServerLevelCreate() throws ImpalaException { + // TODO: Add test support for dynamically changing privileges for + // file-based policy. + if (ctx_.authzConfig.isFileBasedPolicy()) return; + + SentryPolicyService sentryService = + new SentryPolicyService(ctx_.authzConfig.getSentryConfig()); + + // User has CREATE privilege on server. + String roleName = "create_role"; + try { + sentryService.createRole(USER, roleName, true); + TPrivilege privilege = new TPrivilege("", TPrivilegeLevel.CREATE, + TPrivilegeScope.SERVER, false); + privilege.setServer_name("server1"); + sentryService.grantRolePrivilege(USER, roleName, privilege); + sentryService.grantRoleToGroup(USER, roleName, USER.getName()); + ctx_.catalog.reset(); + + AuthzOk("create database newdb"); + AuthzOk("create database newdb location " + + "'hdfs://localhost:20500/test-warehouse/new_table'"); + AuthzOk("create table functional_avro.newtable (i int)"); + AuthzOk("create view functional_avro.newview as " + + "select * from functional.alltypesagg"); + AuthzOk("create function functional_avro.f() returns int location " + + "'/test-warehouse/libTestUdfs.so' symbol='NoArgs'"); + // User does not have INSERT privilege on functional_avro database. + AuthzError("create table functional_avro.newtable as " + + "select * from functional.alltypesagg", + "User '%s' does not have privileges to execute 'INSERT' on: " + + "functional_avro.newtable"); + } finally { + sentryService.dropRole(USER, roleName, true); + ctx_.catalog.reset(); + } + } + private void TestWithIncorrectConfig(AuthorizationConfig authzConfig, User user) throws ImpalaException { Frontend fe = new Frontend(authzConfig, ctx_.catalog); http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/fe/src/test/java/org/apache/impala/analysis/ParserTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java index aefa8e2..e61266e 100644 --- a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java @@ -3581,6 +3581,11 @@ public class ParserTest extends FrontendTestBase { ParsesOk(String.format("%s REFRESH ON DATABASE foo %s myRole", formatStr)); ParsesOk(String.format("%s REFRESH ON TABLE foo %s myRole", formatStr)); + // CREATE privilege. + ParsesOk(String.format("%s CREATE ON SERVER %s myRole", formatStr)); + ParsesOk(String.format("%s CREATE ON SERVER foo %s myRole", formatStr)); + ParsesOk(String.format("%s CREATE ON DATABASE foo %s myRole", formatStr)); + // Server scope does not accept a name. ParsesOk(String.format("%s ALL ON SERVER %s myRole", formatStr)); ParsesOk(String.format("%s INSERT ON SERVER %s myRole", formatStr)); http://git-wip-us.apache.org/repos/asf/impala/blob/ef589727/fe/src/test/resources/authz-policy.ini.template ---------------------------------------------------------------------- diff --git a/fe/src/test/resources/authz-policy.ini.template b/fe/src/test/resources/authz-policy.ini.template index f3c7c1f..aeb3911 100644 --- a/fe/src/test/resources/authz-policy.ini.template +++ b/fe/src/test/resources/authz-policy.ini.template @@ -26,7 +26,8 @@ ${USER} = all_tpch, all_newdb, all_functional_seq_snap, select_tpcds,\ insert_parquet, new_table_uri, tpch_data_uri, select_column_level_functional,\ select_column_level_functional_avro, upper_case_uri,\ refresh_functional_text_lzo, refresh_functional_alltypesagg,\ - refresh_functional_view_view + refresh_functional_view_view, insert_functional_text_lzo,\ + create_functional_text_lzo, libtestudfs_uri auth_to_local_group = test_role server_admin = all_server @@ -54,6 +55,8 @@ refresh_functional_alltypesagg =\ server=server1->db=functional->table=alltypesagg->action=refresh refresh_functional_view_view =\ server=server1->db=functional->table=view_view->action=refresh +insert_functional_text_lzo = server=server1->db=functional_text_lzo->action=insert +create_functional_text_lzo = server=server1->db=functional_text_lzo->action=create select_column_level_functional =\ server=server1->db=functional->table=alltypessmall->column=id->action=select,\ server=server1->db=functional->table=alltypessmall->column=int_col->action=select,\ @@ -80,6 +83,7 @@ select_column_level_functional_avro =\ new_table_uri = server=server1->uri=hdfs://localhost:20500/test-warehouse/new_table tpch_data_uri = server=server1->uri=hdfs://localhost:20500/test-warehouse/tpch.lineitem upper_case_uri = server=server1->uri=hdfs://localhost:20500/test-warehouse/UPPER_CASE +libtestudfs_uri = server=server1->uri=hdfs://localhost:20500/test-warehouse/libTestUdfs.so # This section allows for an admin specified mapping of users -> groups rather than using # the built-in hadoop group mapping. This section is only applicable if using the
