jprdonnelly/impala
1
0
Fork 0
mirror of https://github.com/apache/impala.git synced 2025-12-30 03:01:44 -05:00
Code Issues Projects Releases Wiki Activity

IMPALA-6650: Add fine-grained DROP privilege

Browse Source 
Add support for executing DROP statements by granting DROP privilege.

These are the new GRANT/REVOKE statements introduced at server,
database, and table scopes.

GRANT DROP on SERVER svr TO ROLE testrole;
GRANT DROP on DATABASE db TO ROLE testrole;
GRANT DROP on TABLE tbl TO ROLE testrole;

REVOKE DROP on SERVER svr FROM ROLE testrole;
REVOKE DROP on DATABASE db FROM ROLE testrole;
REVOKE DROP on TABLE tbl FROM ROLE testrole;

Testing:
- Added new front-end tests
- Ran front-end tests

Cherry-picks: not for 2.x

Change-Id: Iea1dfb0b117cb1725e9656b9a79a9aebee82b5e4
Reviewed-on: http://gerrit.cloudera.org:8080/9911
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Impala Public Jenkins
This commit is contained in:
Fredy Wijaya
2018-04-03 10:58:23 -05:00
committed by Impala Public Jenkins
parent 75e1bd1bcd
commit c9f4fdc6bc
11 changed files with 159 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
common/thrift/CatalogObjects.thrift
View File

@@ -470,7 +470,8 @@ enum TPrivilegeLevel {
SELECT,
REFRESH,
CREATE,
ALTER
ALTER,
DROP
}
// Represents a privilege in an authorization policy. Privileges contain the level

3
fe/.gitignore vendored
View File

@@ -33,6 +33,9 @@ src/test/resources/authz-policy.ini
# Generated minicluster config
src/test/resources/minicluster-conf.xml
# Generated hive-log4j2.properties file
src/test/resources/hive-log4j2.properties
# Generated thrift files
generated-sources

2
fe/src/main/cup/sql-parser.cup
View File

@@ -966,6 +966,8 @@ privilege ::=
{: RESULT = TPrivilegeLevel.CREATE; :}
| KW_ALTER
{: RESULT = TPrivilegeLevel.ALTER; :}
| KW_DROP
{: RESULT = TPrivilegeLevel.DROP; :}
| KW_ALL
{: RESULT = TPrivilegeLevel.ALL; :}
;

7
fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
View File

@@ -85,13 +85,10 @@ public class DropFunctionStmt extends StatementBase {
false);
}
// For now, if authorization is enabled, the user needs ALL on the server
// to drop functions.
// TODO: this is not the right granularity but acceptable for now.
analyzer.registerPrivReq(new PrivilegeRequest(
new AuthorizeableFn(desc_.dbName(), desc_.signatureString()), Privilege.ALL));
new AuthorizeableFn(desc_.dbName(), desc_.signatureString()), Privilege.DROP));
Db db = analyzer.getDb(desc_.dbName(), Privilege.DROP, false);
Db db = analyzer.getDb(desc_.dbName(), false);
if (db == null && !ifExists_) {
throw new AnalysisException(Analyzer.DB_DOES_NOT_EXIST_ERROR_MSG + desc_.dbName());
}

7
fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
View File

@@ -191,9 +191,10 @@ public class PrivilegeSpec implements ParseNode {
if (privilegeLevel_ != TPrivilegeLevel.ALL &&
privilegeLevel_ != TPrivilegeLevel.REFRESH &&
privilegeLevel_ != TPrivilegeLevel.CREATE &&
privilegeLevel_ != TPrivilegeLevel.ALTER) {
throw new AnalysisException("Only 'ALL', 'REFRESH', 'CREATE', or 'ALTER' " +
"privilege may be applied at SERVER scope in privilege spec.");
privilegeLevel_ != TPrivilegeLevel.ALTER &&
privilegeLevel_ != TPrivilegeLevel.DROP) {
throw new AnalysisException("Only 'ALL', 'REFRESH', 'CREATE', 'ALTER', or " +
"'DROP' privilege may be applied at SERVER scope in privilege spec.");
}
break;
case DATABASE:

6
fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
View File

@@ -105,13 +105,13 @@ public class AuthorizationChecker {
Preconditions.checkNotNull(privilegeRequest);
if (!hasAccess(user, privilegeRequest)) {
Privilege privilege = privilegeRequest.getPrivilege();
if (privilegeRequest.getAuthorizeable() instanceof AuthorizeableFn) {
throw new AuthorizationException(String.format(
"User '%s' does not have privileges to CREATE/DROP functions in: %s",
user.getName(), privilegeRequest.getName()));
"User '%s' does not have privileges to %s functions in: %s",
user.getName(), privilege, privilegeRequest.getName()));
}
Privilege privilege = privilegeRequest.getPrivilege();
if (EnumSet.of(Privilege.ANY, Privilege.ALL, Privilege.VIEW_METADATA)
.contains(privilege)) {
throw new AuthorizationException(String.format(

3
fe/src/main/java/org/apache/impala/authorization/Privilege.java
View File

@@ -27,7 +27,7 @@ import org.apache.sentry.core.common.Action;
public enum Privilege {
ALL(SentryAction.ALL, false),
ALTER(SentryAction.ALTER, false),
DROP(SentryAction.ALL, false),
DROP(SentryAction.DROP, false),
CREATE(SentryAction.CREATE, false),
INSERT(SentryAction.INSERT, false),
SELECT(SentryAction.SELECT, false),
@@ -56,6 +56,7 @@ public enum Privilege {
REFRESH("refresh"),
CREATE("create"),
ALTER("alter"),
DROP("drop"),
ALL("*");
private final String value;

18
fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
View File

@@ -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', 'REFRESH', 'CREATE', or 'ALTER' privilege may be applied at " +
"SERVER scope in privilege spec.");
"Only 'ALL', 'REFRESH', 'CREATE', 'ALTER', or 'DROP' 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', 'REFRESH', 'CREATE', or 'ALTER' privilege may be applied at " +
"SERVER scope in privilege spec.");
"Only 'ALL', 'REFRESH', 'CREATE', 'ALTER', or 'DROP' 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.");
@@ -255,6 +255,16 @@ public class AnalyzeAuthStmtsTest extends AnalyzerTest {
AnalysisError(String.format(
"%s ALTER ON URI 'hdfs:////abc/123' %s myrole", formatArgs),
"Only 'ALL' privilege may be applied at URI scope in privilege spec.");
// DROP privilege
AnalyzesOk(String.format("%s DROP ON SERVER %s myrole", formatArgs));
AnalyzesOk(String.format("%s DROP ON SERVER server1 %s myrole", formatArgs));
AnalyzesOk(String.format("%s DROP ON DATABASE functional %s myrole", formatArgs));
AnalyzesOk(String.format(
"%s DROP ON TABLE functional.alltypes %s myrole", formatArgs));
AnalysisError(String.format(
"%s DROP ON URI 'hdfs:////abc/123' %s myrole", formatArgs),
"Only 'ALL' privilege may be applied at URI scope in privilege spec.");
}
AnalysisContext authDisabledCtx = createAuthDisabledAnalysisCtx();

128
fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
View File

@@ -40,6 +40,7 @@ import org.apache.impala.catalog.AuthorizationException;
import org.apache.impala.catalog.Db;
import org.apache.impala.catalog.ImpaladCatalog;
import org.apache.impala.catalog.ScalarFunction;
import org.apache.impala.catalog.ScalarType;
import org.apache.impala.catalog.Type;
import org.apache.impala.common.AnalysisException;
import org.apache.impala.common.FrontendTestBase;
@@ -87,11 +88,11 @@ public class AuthorizationTest extends FrontendTestBase {
// ALL permission on 'tpch' database and 'newdb' database
// ALL permission on 'functional_seq_snap' database
// SELECT permissions on all tables in 'tpcds' database
// SELECT, REFRESH permissions on 'functional.alltypesagg' (no INSERT permissions)
// SELECT, REFRESH, DROP permissions on 'functional.alltypesagg'
// ALTER permissions on 'functional.alltypeserror'
// SELECT permissions on 'functional.complex_view' (no INSERT permissions)
// SELECT, REFRESH permissions on 'functional.view_view' (no INSERT permissions)
// ALTER permission on 'functional.alltypes_view'
// SELECT permissions on 'functional.complex_view'
// SELECT, REFRESH permissions on 'functional.view_view'
// ALTER, DROP permissions on 'functional.alltypes_view'
// SELECT permissions on columns ('id', 'int_col', and 'year') on
// 'functional.alltypessmall' (no SELECT permissions on 'functional.alltypessmall')
// SELECT permissions on columns ('id', 'int_struct_col', 'struct_array_col',
@@ -105,7 +106,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, INSERT, CREATE, ALTER permissions on 'functional_text_lzo' database
// REFRESH, INSERT, CREATE, ALTER, DROP 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"));
@@ -267,6 +268,18 @@ public class AuthorizationTest extends FrontendTestBase {
privilege.setTable_name("alltypesagg");
sentryService.grantRolePrivilege(USER, roleName, privilege);
// drop_functional_alltypesagg
roleName = "drop_functional_alltypesagg";
sentryService.createRole(USER, roleName, true);
sentryService.grantRoleToGroup(USER, roleName, USER.getName());
privilege = new TPrivilege("", TPrivilegeLevel.DROP,
TPrivilegeScope.TABLE, false);
privilege.setServer_name("server1");
privilege.setDb_name("functional");
privilege.setTable_name("alltypesagg");
sentryService.grantRolePrivilege(USER, roleName, privilege);
// refresh_functional_view_view
roleName = "refresh_functional_view_view";
sentryService.createRole(USER, roleName, true);
@@ -279,6 +292,18 @@ public class AuthorizationTest extends FrontendTestBase {
privilege.setTable_name("view_view");
sentryService.grantRolePrivilege(USER, roleName, privilege);
// drop_functional_alltypes_view
roleName = "drop_functional_alltypes_view";
sentryService.createRole(USER, roleName, true);
sentryService.grantRoleToGroup(USER, roleName, USER.getName());
privilege = new TPrivilege("", TPrivilegeLevel.DROP,
TPrivilegeScope.TABLE, false);
privilege.setServer_name("server1");
privilege.setDb_name("functional");
privilege.setTable_name("alltypes_view");
sentryService.grantRolePrivilege(USER, roleName, privilege);
// insert_functional_text_lzo
roleName = "insert_functional_text_lzo";
sentryService.createRole(USER, roleName, true);
@@ -315,6 +340,18 @@ public class AuthorizationTest extends FrontendTestBase {
privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
sentryService.grantRolePrivilege(USER, roleName, privilege);
// drop_functional_text_lzo
roleName = "drop_functional_text_lzo";
sentryService.createRole(USER, roleName, true);
sentryService.grantRoleToGroup(USER, roleName, USER.getName());
privilege = new TPrivilege("", TPrivilegeLevel.DROP, 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);
// alter_functional_alltypeserror
roleName = "alter_functional_alltypeserror";
sentryService.createRole(USER, roleName, true);
@@ -1244,6 +1281,15 @@ public class AuthorizationTest extends FrontendTestBase {
AuthzOk("drop database if exists newdb");
AuthzOk("drop database if exists newdb cascade");
AuthzOk("drop database if exists newdb restrict");
// User has DROP privilege on functional_text_lzo database.
AuthzOk("drop database functional_text_lzo");
AuthzOk("drop database functional_text_lzo cascade");
AuthzOk("drop database functional_text_lzo restrict");
AuthzOk("drop database if exists functional_text_lzo");
AuthzOk("drop database if exists functional_text_lzo cascade");
AuthzOk("drop database if exists functional_text_lzo restrict");
// User has permission, database does not exists, IF EXISTS not specified.
try {
AuthzOk("drop database newdb");
@@ -1294,6 +1340,9 @@ public class AuthorizationTest extends FrontendTestBase {
AuthzOk("drop table tpch.lineitem");
AuthzOk("drop table if exists tpch.lineitem");
// User has DROP privilege on functional.alltypesagg table.
AuthzOk("drop table functional.alltypesagg");
// Drop table (user does not have permission).
AuthzError("drop table functional.alltypes",
"User '%s' does not have privileges to execute 'DROP' on: functional.alltypes");
@@ -1330,10 +1379,13 @@ public class AuthorizationTest extends FrontendTestBase {
AuthzOk("drop view functional_seq_snap.alltypes_view");
AuthzOk("drop view if exists functional_seq_snap.alltypes_view");
// Drop view (user does not have permission).
AuthzError("drop view functional.alltypes_view",
// User has DROP privilege on functional.alltypes_view view.
AuthzOk("drop view functional.alltypes_view");
// User does not have DROP privilege on functional.alltypes_view_sub view.
AuthzError("drop view functional.alltypes_view_sub",
"User '%s' does not have privileges to execute 'DROP' on: functional.alltypes");
AuthzError("drop view if exists functional.alltypes_view",
AuthzError("drop view if exists functional.alltypes_view_sub",
"User '%s' does not have privileges to execute 'DROP' on: functional.alltypes");
// Drop view with unqualified table name.
@@ -2412,7 +2464,7 @@ public class AuthorizationTest extends FrontendTestBase {
AuthzError(ctx, "create function f() returns int location " +
"'/test-warehouse/libTestUdfs.so' symbol='NoArgs'",
"User '%s' does not have privileges to CREATE/DROP functions in: default.f()");
"User '%s' does not have privileges to CREATE functions in: default.f()");
// User has ALL privilege on tpch database and ALL privilege on
// /test-warehouse/libTestUdfs.so URI.
@@ -2421,19 +2473,31 @@ public class AuthorizationTest extends FrontendTestBase {
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 in: notdb.f()");
"User '%s' does not have privileges to CREATE functions in: notdb.f()");
// User has DROP privilege on functional_text_lzo database.
try {
ctx_.catalog.addFunction(ScalarFunction.createForTesting("functional_text_lzo",
"f", new ArrayList<Type>(), Type.INT, "/dummy", "dummy.class", null,
null, TFunctionBinaryType.NATIVE));
AuthzOk("drop function functional_text_lzo.f()");
} finally {
ctx_.catalog.removeFunction(ScalarFunction.createForTesting("functional_text_lzo",
"f", new ArrayList<Type>(), Type.INT, "/dummy", "dummy.class", null,
null, TFunctionBinaryType.NATIVE));
}
AuthzError(ctx, "drop function if exists f()",
"User '%s' does not have privileges to CREATE/DROP functions in: default.f()");
"User '%s' does not have privileges to DROP functions in: default.f()");
AuthzError(ctx, "drop function notdb.f()",
"User '%s' does not have privileges to CREATE/DROP functions in: notdb.f()");
"User '%s' does not have privileges to 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'",
"User '%s' does not have privileges to CREATE/DROP functions in: " +
"User '%s' does not have privileges to CREATE functions in: " +
"default.sin(DOUBLE)");
// User tries to create a function in the system database.
AuthzError(ctx, "create function _impala_builtins.sin(double) returns double " +
@@ -2446,7 +2510,7 @@ public class AuthorizationTest extends FrontendTestBase {
AuthzError(ctx, "drop function _impala_builtins.sin(double)",
"Cannot modify system database.");
AuthzError(ctx, "drop function sin(double)",
"User '%s' does not have privileges to CREATE/DROP functions in: " +
"User '%s' does not have privileges to DROP functions in: " +
"default.sin(DOUBLE)");
// TODO: Add test support for dynamically changing privileges for
@@ -2737,6 +2801,42 @@ public class AuthorizationTest extends FrontendTestBase {
}
}
@Test
public void TestServerLevelDrop() 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 DROP privilege on server.
String roleName = "drop_role";
try {
sentryService.createRole(USER, roleName, true);
TPrivilege privilege = new TPrivilege("", TPrivilegeLevel.DROP,
TPrivilegeScope.SERVER, false);
privilege.setServer_name("server1");
sentryService.grantRolePrivilege(USER, roleName, privilege);
sentryService.grantRoleToGroup(USER, roleName, USER.getName());
ctx_.catalog.reset();
AuthzOk("drop database functional");
AuthzOk("drop table functional.alltypes");
AuthzOk("drop view functional.alltypes_view_sub");
ctx_.catalog.addFunction(ScalarFunction.createForTesting("functional",
"f", new ArrayList<Type>(), Type.INT, "/dummy", "dummy.class", null,
null, TFunctionBinaryType.NATIVE));
AuthzOk("drop function functional.f()");
} finally {
sentryService.dropRole(USER, roleName, true);
ctx_.catalog.reset();
ctx_.catalog.addFunction(ScalarFunction.createForTesting("functional",
"f", new ArrayList<Type>(), Type.INT, "/dummy", "dummy.class", null,
null, TFunctionBinaryType.NATIVE));
}
}
private void TestWithIncorrectConfig(AuthorizationConfig authzConfig, User user)
throws ImpalaException {
Frontend fe = new Frontend(authzConfig, ctx_.catalog);

6
fe/src/test/java/org/apache/impala/analysis/ParserTest.java
View File

@@ -3592,6 +3592,12 @@ public class ParserTest extends FrontendTestBase {
ParsesOk(String.format("%s ALTER ON DATABASE foo %s myRole", formatStr));
ParsesOk(String.format("%s ALTER ON TABLE foo %s myRole", formatStr));
// DROP privilege.
ParsesOk(String.format("%s DROP ON SERVER %s myRole", formatStr));
ParsesOk(String.format("%s DROP ON SERVER foo %s myRole", formatStr));
ParsesOk(String.format("%s DROP ON DATABASE foo %s myRole", formatStr));
ParsesOk(String.format("%s DROP ON TABLE 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));

8
fe/src/test/resources/authz-policy.ini.template
View File

@@ -29,7 +29,8 @@ ${USER} = all_tpch, all_newdb, all_functional_seq_snap, select_tpcds,\
refresh_functional_view_view, insert_functional_text_lzo,\
create_functional_text_lzo, alter_functional_text_lzo,\
alter_functional_alltypeserror, alter_functional_alltypes_view,\
libtestudfs_uri
libtestudfs_uri, drop_functional_text_lzo, drop_functional_alltypesagg,\
drop_functional_alltypes_view
auth_to_local_group = test_role
server_admin = all_server
@@ -55,15 +56,20 @@ insert_parquet = server=server1->db=functional_parquet->table=*->action=insert
refresh_functional_text_lzo = server=server1->db=functional_text_lzo->action=refresh
refresh_functional_alltypesagg =\
server=server1->db=functional->table=alltypesagg->action=refresh
drop_functional_alltypesagg =\
server=server1->db=functional->table=alltypesagg->action=drop
alter_functional_alltypeserror =\
server=server1->db=functional->table=alltypeserror->action=alter
refresh_functional_view_view =\
server=server1->db=functional->table=view_view->action=refresh
drop_functional_alltypes_view =\
server=server1->db=functional->table=alltypes_view->action=drop
alter_functional_alltypes_view =\
server=server1->db=functional->table=alltypes_view->action=alter
insert_functional_text_lzo = server=server1->db=functional_text_lzo->action=insert
create_functional_text_lzo = server=server1->db=functional_text_lzo->action=create
alter_functionl_text_lzo = server=server1->db=functional_text_lzo->action=alter
drop_functional_text_lzo = server=server1->db=functional_text_lzo->action=drop
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,\