This is an automated email from the ASF dual-hosted git repository.
amishra pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sentry.git
The following commit(s) were added to refs/heads/master by this push:
new 83c9748 SENTRY-2492: Consecutive ALL grants get deleted when multiple roles have
ALL grants on that object (Kalyan Kumar Kalvagadda reviewed by Arjun Mishra)
83c9748 is described below
commit 83c9748ebf03eb552f1d7287c69bdad5b8e24c17
Author: amishra <amishra@cloudera.com>
AuthorDate: Thu Feb 14 10:17:28 2019 -0600
SENTRY-2492: Consecutive ALL grants get deleted when multiple roles have ALL grants on
that object (Kalyan Kumar Kalvagadda reviewed by Arjun Mishra)
---
.../db/service/persistent/SentryStore.java | 8 +-
.../tests/e2e/dbprovider/TestDatabaseProvider.java | 90 ++++++++++++++++++++++
2 files changed, 96 insertions(+), 2 deletions(-)
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
index 0b17867..1d97ff6 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
@@ -881,9 +881,13 @@ public class SentryStore implements SentryStoreInterface {
mPrivilege = getMSentryPrivilege(privilege, pm);
if (mPrivilege == null) {
mPrivilege = convertToMSentryPrivilege(privilege);
+ mPrivilege.appendPrincipal(mEntity);
+ pm.makePersistent(mPrivilege);
+ } else {
+ mEntity.appendPrivilege(mPrivilege);
+ pm.makePersistent(mEntity);
}
- mPrivilege.appendPrincipal(mEntity);
- pm.makePersistent(mPrivilege);
+
return mPrivilege;
}
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 6a7c1f3..9cbadbf 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
@@ -2274,4 +2274,94 @@ public class TestDatabaseProvider extends AbstractTestWithStaticConfiguration
{
validateReturnedResult(expectedResult, returnedResult);
connection.close();
}
+
+ /**
+ * When two roles r1, r2 had ALL grant on an entity like a database,
+ * another ALL grant to the same database to one of the roles r1
+ * caused it to get deleted on role r1
+ * @throws Exception
+ */
+ @Test
+ public void testGrantConsecutiveALL() throws Exception {
+
+ // setup db objects needed by the test
+ Connection connection = context.createConnection(ADMIN1);
+ Statement statement = context.createStatement(connection);
+ statement.execute("DROP DATABASE IF EXISTS db1 CASCADE");
+ statement.execute("CREATE DATABASE db1");
+ statement.execute("CREATE ROLE role1");
+ statement.execute("CREATE ROLE role2");
+ statement.execute("GRANT ALL ON DATABASE db1 TO ROLE role1");
+ statement.execute("GRANT ALL ON DATABASE db1 TO ROLE role2");
+ ResultSet resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
+ assertResultSize(resultSet, 1);
+ assertThat(resultSet.getString(1), equalToIgnoringCase("db1"));//the value should be
db1
+ assertThat(resultSet.getString(5), equalToIgnoringCase("role1"));//principalName
+ assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType
+ assertThat(resultSet.getString(7), equalToIgnoringCase("*"));
+
+ resultSet = statement.executeQuery("SHOW GRANT ROLE role2");
+ assertResultSize(resultSet, 1);
+ assertThat(resultSet.getString(1), equalToIgnoringCase("db1"));//the value should be
db1
+ assertThat(resultSet.getString(5), equalToIgnoringCase("role2"));//principalName
+ assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType
+ assertThat(resultSet.getString(7), equalToIgnoringCase("*"));
+
+ statement.execute("GRANT ALL ON DATABASE db1 TO ROLE role1");
+ resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
+ assertResultSize(resultSet, 1);
+ assertThat(resultSet.getString(1), equalToIgnoringCase("db1"));//the value should be
db1
+ assertThat(resultSet.getString(5), equalToIgnoringCase("role1"));//principalName
+ assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType
+ assertThat(resultSet.getString(7), equalToIgnoringCase("*"));
+
+ connection.close();
+ }
+
+ @Test
+ public void testGrantConsecutiveALLWithMulitpleInitialGrants() throws Exception {
+
+ // setup db objects needed by the test
+ Connection connection = context.createConnection(ADMIN1);
+ Statement statement = context.createStatement(connection);
+ statement.execute("DROP DATABASE IF EXISTS db1 CASCADE");
+ statement.execute("CREATE DATABASE db1");
+ statement.execute("CREATE ROLE role1");
+ statement.execute("CREATE ROLE role2");
+ statement.execute("GRANT SELECT ON DATABASE db1 TO ROLE role1");
+ statement.execute("GRANT INSERT ON DATABASE db1 TO ROLE role1");
+ statement.execute("GRANT ALL ON DATABASE db1 TO ROLE role2");
+ ResultSet resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
+ assertResultSize(resultSet, 2);
+ int i = 1;
+ resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
+ while(resultSet.next()) {
+ assertThat(resultSet.getString(1), equalToIgnoringCase("db1"));//the value should be
db1
+ assertThat(resultSet.getString(5), equalToIgnoringCase("role1"));//principalName
+ assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType
+ if (i%2 != 0) {
+ assertThat(resultSet.getString(7), equalToIgnoringCase("INSERT"));
+ } else{
+ assertThat(resultSet.getString(7), equalToIgnoringCase("SELECT"));
+ }
+ i++;
+ }
+
+ resultSet = statement.executeQuery("SHOW GRANT ROLE role2");
+ assertResultSize(resultSet, 1);
+ assertThat(resultSet.getString(1), equalToIgnoringCase("db1"));//the value should be
db1
+ assertThat(resultSet.getString(5), equalToIgnoringCase("role2"));//principalName
+ assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType
+ assertThat(resultSet.getString(7), equalToIgnoringCase("*"));
+
+ statement.execute("GRANT ALL ON DATABASE db1 TO ROLE role1");
+ resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
+ assertResultSize(resultSet, 1);
+ assertThat(resultSet.getString(1), equalToIgnoringCase("db1"));//the value should be
db1
+ assertThat(resultSet.getString(5), equalToIgnoringCase("role1"));//principalName
+ assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType
+ assertThat(resultSet.getString(7), equalToIgnoringCase("*"));
+
+ connection.close();
+ }
}
|