IMPALA-9222: Speed up show tables/DBs if the user has access to parent db/server
Currently we always do the auth check for tables/dbs individually.
If the user has privileges higher in the hierarchy then it is not
necessary to do these checks as they will all succeed.
This change adds a higher level check before the individual checks.
The optimization is only enabled for Sentry, as Ranger has deny
policies, so server/db level access does not guarantee db/table level
access.
Testing:
- the existing auth related test coverage seems enough
- there are no tests for deny policies yet - adding them seems a bigger
task so I created a follow up jira: IMPALA-9252
Change-Id: Ic1f5c5d1cf447a9f1cec46c45272f250b8580826
Reviewed-on: http://gerrit.cloudera.org:8080/14867
Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java
index f01d787..fdce34f 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -868,7 +868,11 @@
User user) throws ImpalaException {
FeCatalog catalog = getCatalog();
List<String> tblNames = catalog.getTableNames(dbName, matcher);
- if (authzFactory_.getAuthorizationConfig().isEnabled()) {
+
+ boolean needsAuthChecks = authzFactory_.getAuthorizationConfig().isEnabled()
+ && !userHasAccessForWholeDb(user, dbName);
+
+ if (needsAuthChecks) {
List<Future<Boolean>> pendingCheckTasks = Lists.newArrayList();
Iterator<String> iter = tblNames.iterator();
while (iter.hasNext()) {
@@ -1010,9 +1014,12 @@
public List<? extends FeDb> getDbs(PatternMatcher matcher, User user)
throws InternalException {
List<? extends FeDb> dbs = getCatalog().getDbs(matcher);
- // If authorization is enabled, filter out the databases the user does not
- // have permissions on.
- if (authzFactory_.getAuthorizationConfig().isEnabled()) {
+
+ boolean needsAuthChecks = authzFactory_.getAuthorizationConfig().isEnabled()
+ && !userHasAccessForWholeServer(user);
+
+ // Filter out the databases the user does not have permissions on.
+ if (needsAuthChecks) {
Iterator<? extends FeDb> iter = dbs.iterator();
List<Future<Boolean>> pendingCheckTasks = Lists.newArrayList();
while (iter.hasNext()) {
@@ -1040,8 +1047,8 @@
}
PrivilegeRequestBuilder builder = new PrivilegeRequestBuilder(
- authzFactory_.getAuthorizableFactory())
- .anyOf(minPrivilegeSetForShowStmts_);
+ authzFactory_.getAuthorizableFactory())
+ .anyOf(minPrivilegeSetForShowStmts_);
if (tblName == null) {
// Check database
builder = builder.onAnyColumn(dbName, owner);
@@ -1054,6 +1061,46 @@
}
/**
+ * Check whether the whole server is accessible to given user.
+ */
+ private boolean userHasAccessForWholeServer(User user)
+ throws InternalException {
+ if (authEngineSupportsDenyRules()) return false;
+ PrivilegeRequestBuilder builder = new PrivilegeRequestBuilder(
+ authzFactory_.getAuthorizableFactory()).anyOf(minPrivilegeSetForShowStmts_)
+ .onServer(authzFactory_.getAuthorizationConfig().getServerName());
+ return authzChecker_.get().hasAnyAccess(user, builder.buildSet());
+ }
+
+ /**
+ * Check whether the whole database is accessible to given user.
+ */
+ private boolean userHasAccessForWholeDb(User user, String dbName)
+ throws InternalException, DatabaseNotFoundException {
+ if (authEngineSupportsDenyRules()) return false;
+ // FeDb is needed to respect ownership in Ranger, dbName would be enough for
+ // the privilege request otherwise.
+ FeDb db = getCatalog().getDb(dbName);
+ if (db == null) {
+ throw new DatabaseNotFoundException("Database '" + dbName + "' not found");
+ }
+ PrivilegeRequestBuilder builder = new PrivilegeRequestBuilder(
+ authzFactory_.getAuthorizableFactory()).anyOf(minPrivilegeSetForShowStmts_)
+ .onDb(db);
+ return authzChecker_.get().hasAnyAccess(user, builder.buildSet());
+ }
+
+ /**
+ * Returns whether the authorization engine supports deny rules. If it does,
+ * then a privilege on a higher level object does not imply privilege on lower
+ * level objects in the hierarchy.
+ */
+ private boolean authEngineSupportsDenyRules() {
+ // TODO: could check config for Ranger and return true if deny rules are disabled
+ return !authzFactory_.getAuthorizationConfig().getProviderName().equals("sentry");
+ }
+
+ /**
* Returns all data sources that match the pattern. If pattern is null,
* matches all data sources.
*/
