This is an automated email from the ASF dual-hosted git repository. joemcdonnell pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 666c1be66420468936b6b34f6d1fb8f695a72d7d Author: Fredy Wijaya <fwij...@cloudera.com> AuthorDate: Wed Jun 5 17:42:54 2019 -0700 IMPALA-8551: Make the grant/revoke error messages to be more user friendly This patch updates the grant/revoke error messages to be more user friendly, especially when granting/revoking with an invalid principal. Testing: - Added E2E test to test grant/revoke with invalid principal Change-Id: I8995f5dc88b211cd3af415713802cfeac44fe576 Reviewed-on: http://gerrit.cloudera.org:8080/13525 Reviewed-by: Fredy Wijaya <fwij...@cloudera.com> Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com> --- .../ranger/RangerCatalogdAuthorizationManager.java | 12 ++++- tests/authorization/test_ranger.py | 54 ++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 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 7bb6aaf..0b296d0 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 @@ -40,6 +40,8 @@ import org.apache.impala.thrift.TShowRolesParams; import org.apache.impala.thrift.TShowRolesResult; import org.apache.impala.util.ClassUtil; import org.apache.ranger.plugin.util.GrantRevokeRequest; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.Collections; @@ -57,6 +59,8 @@ import java.util.function.Supplier; * Operations not supported by Ranger will throw an {@link UnsupportedFeatureException}. */ public class RangerCatalogdAuthorizationManager implements AuthorizationManager { + private static final Logger LOG = LoggerFactory.getLogger( + RangerCatalogdAuthorizationManager.class); private static final String AUTHZ_CACHE_INVALIDATION_MARKER = "ranger"; private final Supplier<RangerImpalaPlugin> plugin_; @@ -171,7 +175,9 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager } } } catch (Exception e) { - throw new InternalException(e.getMessage()); + LOG.error("Error granting a privilege in Ranger: ", e); + throw new InternalException("Error granting a privilege in Ranger. " + + "Ranger error message: " + e.getMessage()); } } @@ -184,7 +190,9 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager } } } catch (Exception e) { - throw new InternalException(e.getMessage()); + LOG.error("Error revoking a privilege in Ranger: ", e); + throw new InternalException("Error revoking a privilege in Ranger. " + + "Ranger error message: " + e.getMessage()); } } diff --git a/tests/authorization/test_ranger.py b/tests/authorization/test_ranger.py index 83ca5e2..bb1019a 100644 --- a/tests/authorization/test_ranger.py +++ b/tests/authorization/test_ranger.py @@ -548,3 +548,57 @@ class TestRanger(CustomClusterTestSuite): error_msg.format("SHOW ROLE GRANT GROUP"))]: result = self.execute_query_expect_failure(impala_client, statement[0], user=user) assert statement[1] in str(result) + + @CustomClusterTestSuite.with_args( + impalad_args=IMPALAD_ARGS, catalogd_args=CATALOGD_ARGS) + def test_grant_revoke_invalid_principal(self): + """Tests grant/revoke to/from invalid principal should return more readable + error messages.""" + valid_user = "admin" + invalid_user = "invalid_user" + invalid_group = "invalid_group" + # TODO(IMPALA-8640): Create two different Impala clients because the users to + # workaround the bug. + invalid_impala_client = self.create_impala_client() + valid_impala_client = self.create_impala_client() + for statement in ["grant select on table functional.alltypes to user {0}" + .format(getuser()), + "revoke select on table functional.alltypes from user {0}" + .format(getuser())]: + result = self.execute_query_expect_failure(invalid_impala_client, + statement, + user=invalid_user) + if "grant" in statement: + assert "Error granting a privilege in Ranger. Ranger error message: " \ + "HTTP 403 Error: Grantor user invalid_user doesn't exist" in str(result) + else: + assert "Error revoking a privilege in Ranger. Ranger error message: " \ + "HTTP 403 Error: Grantor user invalid_user doesn't exist" in str(result) + + for statement in ["grant select on table functional.alltypes to user {0}" + .format(invalid_user), + "revoke select on table functional.alltypes from user {0}" + .format(invalid_user)]: + result = self.execute_query_expect_failure(valid_impala_client, + statement, + user=valid_user) + if "grant" in statement: + assert "Error granting a privilege in Ranger. Ranger error message: " \ + "HTTP 403 Error: Grantee user invalid_user doesn't exist" in str(result) + else: + assert "Error revoking a privilege in Ranger. Ranger error message: " \ + "HTTP 403 Error: Grantee user invalid_user doesn't exist" in str(result) + + for statement in ["grant select on table functional.alltypes to group {0}" + .format(invalid_group), + "revoke select on table functional.alltypes from group {0}" + .format(invalid_group)]: + result = self.execute_query_expect_failure(valid_impala_client, + statement, + user=valid_user) + if "grant" in statement: + assert "Error granting a privilege in Ranger. Ranger error message: " \ + "HTTP 403 Error: Grantee group invalid_group doesn't exist" in str(result) + else: + assert "Error revoking a privilege in Ranger. Ranger error message: " \ + "HTTP 403 Error: Grantee group invalid_group doesn't exist" in str(result)