You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by pr...@apache.org on 2014/09/05 20:55:17 UTC
git commit: SENTRY-318: Allow all users Show GRANT as long as they
have the grant on that role. (Sravya Tirukkovalur via Prasad Mujumdar)
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 <pr...@cloudera.com>
Authored: Fri Sep 5 11:55:07 2014 -0700
Committer: Prasad Mujumdar <pr...@cloudera.com>
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);