Repository: incubator-sentry Updated Branches: refs/heads/master 467915c61 -> 416ca0644
SENTRY-318: Allow all users Show GRANT as long as they have the grant on that role. (Sravya Tirukkovalur via Prasad Mujumdar) Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/416ca064 Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/416ca064 Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/416ca064 Branch: refs/heads/master Commit: 416ca064404ef85ff5dd7de5a302592ca2569aa4 Parents: 467915c Author: Prasad Mujumdar <[email protected]> Authored: Fri Sep 5 11:55:07 2014 -0700 Committer: Prasad Mujumdar <[email protected]> Committed: Fri Sep 5 11:55:07 2014 -0700 ---------------------------------------------------------------------- .../db/service/persistent/SentryStore.java | 41 ++--------- .../thrift/SentryPolicyStoreProcessor.java | 23 ++++-- .../e2e/dbprovider/TestDatabaseProvider.java | 74 ++++++++++++-------- .../TestDbSentryOnFailureHookLoading.java | 5 +- 4 files changed, 68 insertions(+), 75 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/416ca064/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java index 33600e9..718306d 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java @@ -776,11 +776,8 @@ public class SentryStore { /** * Gets sentry privilege objects for criteria from the persistence layer - * @param roleName : roleName to look up - * @param serverName : serverName (required) - * @param uri : URI (optional) - * @param dbName : dbName (optional if tableName is null else required) - * @param tableName : tableName (optional) + * @param roleNames : roleNames to look up (required) + * @param authHierarchy : filter push down based on auth hierarchy (optional) * @return : Set of thrift sentry privilege objects * @throws SentryNoSuchObjectException */ @@ -861,37 +858,7 @@ public class SentryStore { return convertToTSentryRoles(roleSet); } - private SetMultimap<String, String> getRoleToPrivilegeMap(Set<String> groups) { - SetMultimap<String, String> result = HashMultimap.create(); - boolean rollbackTransaction = true; - PersistenceManager pm = null; - try { - pm = openTransaction(); - Query query = pm.newQuery(MSentryGroup.class); - query.setFilter("this.groupName == t"); - query.declareParameters("java.lang.String t"); - query.setUnique(true); - for (String group : groups) { - MSentryGroup sentryGroup = (MSentryGroup) query.execute(group.trim()); - if (sentryGroup != null) { - for (MSentryRole role : sentryGroup.getRoles()) { - for (MSentryPrivilege privilege : role.getPrivileges()) { - result.put(role.getRoleName(), toAuthorizable(privilege)); - } - } - } - } - rollbackTransaction = false; - commitTransaction(pm); - return result; - } finally { - if (rollbackTransaction) { - rollbackTransaction(pm); - } - } - } - - private Set<String> getRoleNamesForGroups(Set<String> groups) { + public Set<String> getRoleNamesForGroups(Set<String> groups) { Set<String> result = new HashSet<String>(); boolean rollbackTransaction = true; PersistenceManager pm = null; @@ -928,7 +895,7 @@ public class SentryStore { for (String group : groups) { MSentryGroup sentryGroup = (MSentryGroup) query.execute(group.trim()); if (sentryGroup != null) { - result = sentryGroup.getRoles(); + result.addAll(sentryGroup.getRoles()); } } return result; http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/416ca064/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java index f227a02..070c494 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java @@ -126,10 +126,15 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { return result; } - private void authorize(String requestorUser, Set<String> requestorGroups) - throws SentryAccessDeniedException { + private boolean inAdminGroups(Set<String> requestorGroups) { requestorGroups = toTrimedLower(requestorGroups); if (Sets.intersection(adminGroups, requestorGroups).isEmpty()) { + return false; + } else return true; + } + private void authorize(String requestorUser, Set<String> requestorGroups) + throws SentryAccessDeniedException { + if (!inAdminGroups(requestorGroups)) { String msg = "User: " + requestorUser + " is part of " + requestorGroups + " which does not, intersect admin groups " + adminGroups; LOGGER.warn(msg); @@ -369,12 +374,16 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { TListSentryPrivilegesResponse response = new TListSentryPrivilegesResponse(); TSentryResponseStatus status; Set<TSentryPrivilege> privilegeSet = new HashSet<TSentryPrivilege>(); + String subject = request.getRequestorUserName(); try { - //TODO: Handle authorization for metadata queries - // Shall we allow only admin users to list privileges - // or allow all users as long as user is granted this role? - authorize(request.getRequestorUserName(), - getRequestorGroups(request.getRequestorUserName())); + Set<String> groups = getRequestorGroups(subject); + Boolean admin = inAdminGroups(groups); + if(!admin) { + Set<String> roleNamesForGroups = toTrimedLower(sentryStore.getRoleNamesForGroups(groups)); + if(!roleNamesForGroups.contains(request.getRoleName().trim().toLowerCase())) { + throw new SentryAccessDeniedException("Access denied to " + subject); + } + } if (request.isSetAuthorizableHierarchy()) { TSentryAuthorizable authorizableHierarchy = request.getAuthorizableHierarchy(); privilegeSet = sentryStore.getTSentryPrivileges(Sets.newHashSet(request.getRoleName()), authorizableHierarchy); http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/416ca064/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java index 8b83859..066e909 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java @@ -1392,38 +1392,52 @@ public class TestDatabaseProvider extends AbstractTestWithStaticConfiguration { assertResultSize(resultSet, 0); statement.execute("CREATE ROLE role2"); statement.execute("GRANT SELECT ON TABLE t1 TO ROLE role1"); + statement.execute("GRANT ROLE role1 to GROUP " + USERGROUP1); - resultSet = statement.executeQuery("SHOW GRANT ROLE role1"); - ResultSetMetaData resultSetMetaData = resultSet.getMetaData(); - //| database | table | partition | column | principal_name | - // principal_type | privilege | grant_option | grant_time | grantor | - assertThat(resultSetMetaData.getColumnCount(), is(10)); - assertThat(resultSetMetaData.getColumnName(1), equalToIgnoringCase("database")); - assertThat(resultSetMetaData.getColumnName(2), equalToIgnoringCase("table")); - assertThat(resultSetMetaData.getColumnName(3), equalToIgnoringCase("partition")); - assertThat(resultSetMetaData.getColumnName(4), equalToIgnoringCase("column")); - assertThat(resultSetMetaData.getColumnName(5), equalToIgnoringCase("principal_name")); - assertThat(resultSetMetaData.getColumnName(6), equalToIgnoringCase("principal_type")); - assertThat(resultSetMetaData.getColumnName(7), equalToIgnoringCase("privilege")); - assertThat(resultSetMetaData.getColumnName(8), equalToIgnoringCase("grant_option")); - assertThat(resultSetMetaData.getColumnName(9), equalToIgnoringCase("grant_time")); - assertThat(resultSetMetaData.getColumnName(10), equalToIgnoringCase("grantor")); - - while ( resultSet.next()) { - assertThat(resultSet.getString(1), equalToIgnoringCase("default")); - assertThat(resultSet.getString(2), equalToIgnoringCase("t1")); - assertThat(resultSet.getString(3), equalToIgnoringCase(""));//partition - assertThat(resultSet.getString(4), equalToIgnoringCase(""));//column - assertThat(resultSet.getString(5), equalToIgnoringCase("role1"));//principalName - assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType - assertThat(resultSet.getString(7), equalToIgnoringCase("select")); - assertThat(resultSet.getBoolean(8), is(new Boolean("False")));//grantOption - //Create time is not tested - //assertThat(resultSet.getLong(9), is(new Long(0))); - assertThat(resultSet.getString(10), equalToIgnoringCase(ADMIN1));//grantor + String[] users = {ADMIN1, USER1_1}; + for (String user:users) { + connection = context.createConnection(user); + statement = context.createStatement(connection); + resultSet = statement.executeQuery("SHOW GRANT ROLE role1"); + ResultSetMetaData resultSetMetaData = resultSet.getMetaData(); + //| database | table | partition | column | principal_name | + // principal_type | privilege | grant_option | grant_time | grantor | + assertThat(resultSetMetaData.getColumnCount(), is(10)); + assertThat(resultSetMetaData.getColumnName(1), equalToIgnoringCase("database")); + assertThat(resultSetMetaData.getColumnName(2), equalToIgnoringCase("table")); + assertThat(resultSetMetaData.getColumnName(3), equalToIgnoringCase("partition")); + assertThat(resultSetMetaData.getColumnName(4), equalToIgnoringCase("column")); + assertThat(resultSetMetaData.getColumnName(5), equalToIgnoringCase("principal_name")); + assertThat(resultSetMetaData.getColumnName(6), equalToIgnoringCase("principal_type")); + assertThat(resultSetMetaData.getColumnName(7), equalToIgnoringCase("privilege")); + assertThat(resultSetMetaData.getColumnName(8), equalToIgnoringCase("grant_option")); + assertThat(resultSetMetaData.getColumnName(9), equalToIgnoringCase("grant_time")); + assertThat(resultSetMetaData.getColumnName(10), equalToIgnoringCase("grantor")); + + while ( resultSet.next()) { + assertThat(resultSet.getString(1), equalToIgnoringCase("default")); + assertThat(resultSet.getString(2), equalToIgnoringCase("t1")); + assertThat(resultSet.getString(3), equalToIgnoringCase(""));//partition + assertThat(resultSet.getString(4), equalToIgnoringCase(""));//column + assertThat(resultSet.getString(5), equalToIgnoringCase("role1"));//principalName + assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType + assertThat(resultSet.getString(7), equalToIgnoringCase("select")); + assertThat(resultSet.getBoolean(8), is(new Boolean("False")));//grantOption + //Create time is not tested + //assertThat(resultSet.getLong(9), is(new Long(0))); + assertThat(resultSet.getString(10), equalToIgnoringCase(ADMIN1));//grantor + } + statement.close(); + connection.close(); } - statement.close(); - connection.close(); + + //Negative test case + connection = context.createConnection(USER2_1); + statement = context.createStatement(connection); + context.assertSentryException(statement, "SHOW GRANT ROLE role1", + SentryAccessDeniedException.class.getSimpleName()); + + } /** http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/416ca064/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java index 7ffe534..1af8baa 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java @@ -184,7 +184,10 @@ public class TestDbSentryOnFailureHookLoading extends AbstractTestWithDbProvider verifyFailureHook(statement, "SHOW GRANT role role1", HiveOperation.SHOW_GRANT, null, null, true); - //Grant privilege on table doesnt expose db and table objects + //Should pass as user1_1 is granted role all_db1 + statement.execute("SHOW GRANT role all_db1"); + + //Grant privilege on table doesnt expose db and table objects verifyFailureHook(statement, "GRANT ALL ON TABLE tab1 TO ROLE admin_role", HiveOperation.GRANT_PRIVILEGE, null, null, true);
