You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2019/06/13 06:37:18 UTC
[impala] 04/07: IMPALA-8588: Fix revoke grant option with Ranger
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 5b0c098b080515cc928984a46ddc973021964f90
Author: Austin Nobis <an...@cloudera.com>
AuthorDate: Tue May 28 10:44:59 2019 -0700
IMPALA-8588: Fix revoke grant option with Ranger
Previously, the REVOKE GRANT OPTION type statements would not only
revoke the grant option, but also the privilege as well. The behavior
has been updated to only revoke the grant option. In Ranger UI, this is
seen as the delegate admin option. Examples:
REVOKE SELECT ON DATABASE <database> FROM USER <user>
This will revoke the SELECT privilege on the database resource, but
if there are other privileges on that resource the grant option will
remain for those privileges.
REVOKE GRANT OPTION FOR SELECT ON DATABASE <database> FROM USER <user>
This will revoke the grant option for all privileges on this database
resource. It will not revoke the SELECT privilege on the resource.
Testing:
- Ran all FE tests
- Ran all E2E tests
- Updated test_ranger to test behavior for when REVOKE GRANT OPTION
statements are submitted.
Change-Id: Iddfccb442c3be3c266dbc2d8ae85c5674c534d7c
Reviewed-on: http://gerrit.cloudera.org:8080/13450
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../ranger/RangerCatalogdAuthorizationManager.java | 32 ++++++++++++----------
.../authorization/AuthorizationTestBase.java | 12 ++++----
tests/authorization/test_ranger.py | 29 ++++++++++++++++----
3 files changed, 47 insertions(+), 26 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
index 0b296d0..0b1527f 100644
--- a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
+++ b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
@@ -122,8 +122,8 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
public void grantPrivilegeToUser(User requestingUser, TGrantRevokePrivParams params,
TDdlExecResponse response) throws ImpalaException {
List<GrantRevokeRequest> requests = createGrantRevokeRequests(
- requestingUser.getName(), params.getPrincipal_name(), Collections.emptyList(),
- plugin_.get().getClusterName(), params.getPrivileges());
+ requestingUser.getName(), true, params.getPrincipal_name(),
+ Collections.emptyList(), plugin_.get().getClusterName(), params.getPrivileges());
grantPrivilege(requests);
refreshAuthorization(response);
@@ -133,8 +133,8 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
public void revokePrivilegeFromUser(User requestingUser, TGrantRevokePrivParams params,
TDdlExecResponse response) throws ImpalaException {
List<GrantRevokeRequest> requests = createGrantRevokeRequests(
- requestingUser.getName(), params.getPrincipal_name(), Collections.emptyList(),
- plugin_.get().getClusterName(), params.getPrivileges());
+ requestingUser.getName(), false, params.getPrincipal_name(),
+ Collections.emptyList(), plugin_.get().getClusterName(), params.getPrivileges());
revokePrivilege(requests);
refreshAuthorization(response);
@@ -144,7 +144,7 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
public void grantPrivilegeToGroup(User requestingUser, TGrantRevokePrivParams params,
TDdlExecResponse response) throws ImpalaException {
List<GrantRevokeRequest> requests = createGrantRevokeRequests(
- requestingUser.getName(), null,
+ requestingUser.getName(), true, null,
Collections.singletonList(params.getPrincipal_name()),
plugin_.get().getClusterName(), params.getPrivileges());
@@ -156,7 +156,7 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
public void revokePrivilegeFromGroup(User requestingUser, TGrantRevokePrivParams params,
TDdlExecResponse response) throws ImpalaException {
List<GrantRevokeRequest> requests = createGrantRevokeRequests(
- requestingUser.getName(), null,
+ requestingUser.getName(), false, null,
Collections.singletonList(params.getPrincipal_name()),
plugin_.get().getClusterName(), params.getPrivileges());
@@ -236,13 +236,14 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
}
public static List<GrantRevokeRequest> createGrantRevokeRequests(String grantor,
- String user, List<String> groups, String clusterName, List<TPrivilege> privileges) {
+ boolean isGrant, String user, List<String> groups, String clusterName,
+ List<TPrivilege> privileges) {
List<GrantRevokeRequest> requests = new ArrayList<>();
for (TPrivilege p: privileges) {
Function<Map<String, String>, GrantRevokeRequest> createRequest = (resource) ->
createGrantRevokeRequest(grantor, user, groups, clusterName, p.has_grant_opt,
- p.privilege_level, resource);
+ isGrant, p.privilege_level, resource);
// Ranger Impala service definition defines 3 resources:
// [DB -> Table -> Column]
@@ -271,22 +272,25 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
}
private static GrantRevokeRequest createGrantRevokeRequest(String grantor, String user,
- List<String> groups, String clusterName, boolean withGrantOpt,
+ List<String> groups, String clusterName, boolean withGrantOpt, boolean isGrant,
TPrivilegeLevel level, Map<String, String> resource) {
GrantRevokeRequest request = new GrantRevokeRequest();
request.setGrantor(grantor);
if (user != null) request.getUsers().add(user);
if (!groups.isEmpty()) request.getGroups().addAll(groups);
- request.setDelegateAdmin((withGrantOpt) ? Boolean.TRUE : Boolean.FALSE);
+ request.setDelegateAdmin(isGrant == withGrantOpt);
request.setEnableAudit(Boolean.TRUE);
request.setReplaceExistingPermissions(Boolean.FALSE);
request.setClusterName(clusterName);
request.setResource(resource);
- if (level == TPrivilegeLevel.INSERT) {
- request.getAccessTypes().add(RangerAuthorizationChecker.UPDATE_ACCESS_TYPE);
- } else {
- request.getAccessTypes().add(level.name().toLowerCase());
+ // For revoke grant option, omit the privilege
+ if (!(!isGrant && withGrantOpt)) {
+ if (level == TPrivilegeLevel.INSERT) {
+ request.getAccessTypes().add(RangerAuthorizationChecker.UPDATE_ACCESS_TYPE);
+ } else {
+ request.getAccessTypes().add(level.name().toLowerCase());
+ }
}
return request;
diff --git a/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java b/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
index bd3b8cc..b559ba1 100644
--- a/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
+++ b/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
@@ -200,14 +200,14 @@ public abstract class AuthorizationTestBase extends FrontendTestBase {
}
protected abstract class WithRanger implements WithPrincipal {
- private final List<GrantRevokeRequest> requests = new ArrayList<>();
+ private final List<GrantRevokeRequest> grants = new ArrayList<>();
private final RangerCatalogdAuthorizationManager authzManager =
new RangerCatalogdAuthorizationManager(() -> rangerImpalaPlugin_, null);
@Override
public void init(TPrivilege[]... privileges) throws ImpalaException {
for (TPrivilege[] privilege : privileges) {
- requests.addAll(buildRequest(Arrays.asList(privilege))
+ grants.addAll(buildRequest(Arrays.asList(privilege))
.stream()
.peek(r -> {
r.setResource(updateUri(r.getResource()));
@@ -218,7 +218,7 @@ public abstract class AuthorizationTestBase extends FrontendTestBase {
}).collect(Collectors.toList()));
}
- authzManager.grantPrivilege(requests);
+ authzManager.grantPrivilege(grants);
rangerImpalaPlugin_.refreshPoliciesAndTags();
}
@@ -229,7 +229,7 @@ public abstract class AuthorizationTestBase extends FrontendTestBase {
@Override
public void cleanUp() throws ImpalaException {
- authzManager.revokePrivilege(requests);
+ authzManager.revokePrivilege(grants);
}
@Override
@@ -240,7 +240,7 @@ public abstract class AuthorizationTestBase extends FrontendTestBase {
@Override
protected List<GrantRevokeRequest> buildRequest(List<TPrivilege> privileges) {
return RangerCatalogdAuthorizationManager.createGrantRevokeRequests(
- RANGER_ADMIN.getName(), USER.getName(), Collections.emptyList(),
+ RANGER_ADMIN.getName(), true, USER.getName(), Collections.emptyList(),
rangerImpalaPlugin_.getClusterName(), privileges);
}
}
@@ -251,7 +251,7 @@ public abstract class AuthorizationTestBase extends FrontendTestBase {
List<String> groups = Collections.singletonList(System.getProperty("user.name"));
return RangerCatalogdAuthorizationManager.createGrantRevokeRequests(
- RANGER_ADMIN.getName(), null, groups,
+ RANGER_ADMIN.getName(), true, null, groups,
rangerImpalaPlugin_.getClusterName(), privileges);
}
}
diff --git a/tests/authorization/test_ranger.py b/tests/authorization/test_ranger.py
index bb1019a..4f3ac87 100644
--- a/tests/authorization/test_ranger.py
+++ b/tests/authorization/test_ranger.py
@@ -124,25 +124,44 @@ class TestRanger(CustomClusterTestSuite):
"grant select on database {0} to user {1} with "
"grant option".format(unique_database, user1),
user=ADMIN)
+ self.execute_query_expect_success(admin_client,
+ "grant insert on database {0} to user {1} with "
+ "grant option".format(unique_database, user1),
+ user=ADMIN)
# Verify user 1 has with_grant privilege on unique_database
result = self.execute_query("show grant user {0} on database {1}"
.format(user1, unique_database))
TestRanger._check_privileges(result, [
+ ["USER", user1, unique_database, "", "", "", "*", "insert", "true"],
["USER", user1, unique_database, "", "", "", "*", "select", "true"],
+ ["USER", user1, unique_database, "*", "*", "", "", "insert", "true"],
["USER", user1, unique_database, "*", "*", "", "", "select", "true"]])
+ # Revoke select privilege and check grant option is still present
+ self.execute_query_expect_success(admin_client,
+ "revoke select on database {0} from user {1}"
+ .format(unique_database, user1), user=ADMIN)
+ result = self.execute_query("show grant user {0} on database {1}"
+ .format(user1, unique_database))
+ TestRanger._check_privileges(result, [
+ ["USER", user1, unique_database, "", "", "", "*", "insert", "true"],
+ ["USER", user1, unique_database, "*", "*", "", "", "insert", "true"]])
+
# Revoke privilege granting from user 1
- self.execute_query_expect_success(admin_client, "revoke grant option for select "
+ self.execute_query_expect_success(admin_client, "revoke grant option for insert "
"on database {0} from user {1}"
.format(unique_database, user1), user=ADMIN)
# User 1 can no longer grant privileges on unique_database
+ # In ranger it is currently not possible to revoke grant for a single access type
result = self.execute_query("show grant user {0} on database {1}"
.format(user1, unique_database))
- TestRanger._check_privileges(result, [])
+ TestRanger._check_privileges(result, [
+ ["USER", user1, unique_database, "", "", "", "*", "insert", "false"],
+ ["USER", user1, unique_database, "*", "*", "", "", "insert", "false"]])
finally:
- admin_client.execute("revoke grant option for select on database {0} from user {1}"
+ admin_client.execute("revoke insert on database {0} from user {1}"
.format(unique_database, user1), user=ADMIN)
admin_client.execute("drop database if exists {0} cascade".format(unique_database),
user=ADMIN)
@@ -177,9 +196,6 @@ class TestRanger(CustomClusterTestSuite):
# Test USER inherits privileges for their GROUP
self._test_show_grant_user_group(admin_client, user, group, unique_db)
-
- # Test that show grant without ON a resource fails
-
finally:
admin_client.execute("drop database if exists {0} cascade".format(unique_db),
user=ADMIN)
@@ -237,6 +253,7 @@ class TestRanger(CustomClusterTestSuite):
["USER", user, "*", "", "", "", "*", "all", "false"],
["USER", user, "*", "*", "*", "", "", "all", "false"]])
finally:
+ admin_client.execute("revoke all on server from user {0}".format(user))
for privilege in privileges:
admin_client.execute("revoke {0} on server from user {1}".format(privilege, user))