sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From amis...@apache.org
Subject [sentry] branch master updated: SENTRY-2492: Consecutive ALL grants get deleted when multiple roles have ALL grants on that object (Kalyan Kumar Kalvagadda reviewed by Arjun Mishra)
Date Thu, 14 Feb 2019 16:25:19 GMT
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();
+  }
 }


Mime
View raw message