You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by as...@apache.org on 2014/11/20 06:54:48 UTC

incubator-sentry git commit: SENTRY-526: Fix SentryStore so that Duplicate grant with same privilge but different case is handled correctly

Repository: incubator-sentry
Updated Branches:
  refs/heads/master fa26a0b4f -> a8c878ab5


SENTRY-526: Fix SentryStore so that Duplicate grant with same privilge but different case is handled correctly


Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/a8c878ab
Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/a8c878ab
Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/a8c878ab

Branch: refs/heads/master
Commit: a8c878ab5f1b9fbfb28f7d985946913b155ebd64
Parents: fa26a0b
Author: Arun Suresh <as...@cloudera.com>
Authored: Wed Nov 19 21:53:21 2014 -0800
Committer: Arun Suresh <as...@cloudera.com>
Committed: Wed Nov 19 21:53:50 2014 -0800

----------------------------------------------------------------------
 .../db/service/persistent/SentryStore.java      | 27 +++++++++---------
 .../db/service/persistent/TestSentryStore.java  | 29 ++++++++++++++++++++
 .../e2e/dbprovider/TestDatabaseProvider.java    | 15 ++++++++++
 3 files changed, 58 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a8c878ab/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 d163418..3615661 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
@@ -18,7 +18,6 @@
 
 package org.apache.sentry.provider.db.service.persistent;
 
-import com.codahale.metrics.Gauge;
 import static org.apache.sentry.provider.common.ProviderConstants.AUTHORIZABLE_JOINER;
 import static org.apache.sentry.provider.common.ProviderConstants.KV_JOINER;
 
@@ -68,6 +67,7 @@ import org.apache.sentry.service.thrift.ServiceConstants.PrivilegeScope;
 import org.apache.sentry.service.thrift.ServiceConstants.ServerConfig;
 import org.datanucleus.store.rdbms.exceptions.MissingTableException;
 
+import com.codahale.metrics.Gauge;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
@@ -599,21 +599,22 @@ public class SentryStore {
 
   private List<MSentryPrivilege> getMSentryPrivileges(TSentryPrivilege tPriv, PersistenceManager pm) {
     Query query = pm.newQuery(MSentryPrivilege.class);
-    StringBuilder filters = new StringBuilder("this.serverName == \"" + toNULLCol(tPriv.getServerName()) + "\" ");
+    StringBuilder filters = new StringBuilder("this.serverName == \""
+          + toNULLCol(safeTrimLower(tPriv.getServerName())) + "\" ");
     if (!isNULL(tPriv.getDbName())) {
-      filters.append("&& this.dbName == \"" + toNULLCol(tPriv.getDbName()) + "\" ");
+      filters.append("&& this.dbName == \"" + toNULLCol(safeTrimLower(tPriv.getDbName())) + "\" ");
       if (!isNULL(tPriv.getTableName())) {
-        filters.append("&& this.tableName == \"" + toNULLCol(tPriv.getTableName()) + "\" ");
+        filters.append("&& this.tableName == \"" + toNULLCol(safeTrimLower(tPriv.getTableName())) + "\" ");
         if (!isNULL(tPriv.getColumnName())) {
-          filters.append("&& this.columnName == \"" + toNULLCol(tPriv.getColumnName()) + "\" ");
+          filters.append("&& this.columnName == \"" + toNULLCol(safeTrimLower(tPriv.getColumnName())) + "\" ");
         }
       }
     }
     // if db is null, uri is not null
     else if (!isNULL(tPriv.getURI())){
-      filters.append("&& this.URI == \"" + toNULLCol(tPriv.getURI()) + "\" ");
+      filters.append("&& this.URI == \"" + toNULLCol(safeTrim(tPriv.getURI())) + "\" ");
     }
-    filters.append("&& this.action == \"" + toNULLCol(tPriv.getAction().toLowerCase()) + "\"");
+    filters.append("&& this.action == \"" + toNULLCol(safeTrimLower(tPriv.getAction())) + "\"");
 
     query.setFilter(filters.toString());
     List<MSentryPrivilege> privileges = (List<MSentryPrivilege>) query.execute();
@@ -622,13 +623,13 @@ public class SentryStore {
 
   private MSentryPrivilege getMSentryPrivilege(TSentryPrivilege tPriv, PersistenceManager pm) {
     Query query = pm.newQuery(MSentryPrivilege.class);
-    query.setFilter("this.serverName == \"" + toNULLCol(tPriv.getServerName()) + "\" "
-				+ "&& this.dbName == \"" + toNULLCol(tPriv.getDbName()) + "\" "
-				+ "&& this.tableName == \"" + toNULLCol(tPriv.getTableName()) + "\" "
-				+ "&& this.columnName == \"" + toNULLCol(tPriv.getColumnName()) + "\" "
-				+ "&& this.URI == \"" + toNULLCol(tPriv.getURI()) + "\" "
+    query.setFilter("this.serverName == \"" + toNULLCol(safeTrimLower(tPriv.getServerName())) + "\" "
+				+ "&& this.dbName == \"" + toNULLCol(safeTrimLower(tPriv.getDbName())) + "\" "
+				+ "&& this.tableName == \"" + toNULLCol(safeTrimLower(tPriv.getTableName())) + "\" "
+				+ "&& this.columnName == \"" + toNULLCol(safeTrimLower(tPriv.getColumnName())) + "\" "
+				+ "&& this.URI == \"" + toNULLCol(safeTrim(tPriv.getURI())) + "\" "
 				+ "&& this.grantOption == grantOption "
-				+ "&& this.action == \"" + toNULLCol(tPriv.getAction().toLowerCase()) + "\"");
+				+ "&& this.action == \"" + toNULLCol(safeTrimLower(tPriv.getAction())) + "\"");
     query.declareParameters("Boolean grantOption");
     query.setUnique(true);
     Boolean grantOption = null;

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a8c878ab/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
index 60cec1f..70917b7 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
@@ -743,6 +743,35 @@ public class TestSentryStore {
   }
 
   @Test
+  public void testGrantDuplicatePrivilege() throws Exception {
+    String roleName = "test-privilege";
+    String grantor = "g1";
+    String server = "server1";
+    String db = "db1";
+    String table = "tbl1";
+    long seqId = sentryStore.createSentryRole(roleName).getSequenceId();
+    TSentryPrivilege privilege = new TSentryPrivilege();
+    privilege.setPrivilegeScope("TABLE");
+    privilege.setServerName(server);
+    privilege.setDbName(db);
+    privilege.setTableName(table);
+    privilege.setAction(AccessConstants.ALL);
+    privilege.setCreateTime(System.currentTimeMillis());
+    assertEquals(seqId + 1, sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName, privilege)
+        .getSequenceId());
+    assertEquals(seqId + 2, sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName, privilege)
+        .getSequenceId());
+    privilege.setServerName("Server1");
+    privilege.setDbName("DB1");
+    privilege.setTableName("TBL1");
+    assertEquals(seqId + 3, sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName, privilege)
+        .getSequenceId());
+    MSentryRole role = sentryStore.getMSentryRoleByName(roleName);
+    Set<MSentryPrivilege> privileges = role.getPrivileges();
+    assertEquals(privileges.toString(), 1, privileges.size());
+  }
+
+  @Test
   public void testListSentryPrivilegesForProvider() throws Exception {
     String roleName1 = "list-privs-r1", roleName2 = "list-privs-r2";
     String groupName1 = "list-privs-g1", groupName2 = "list-privs-g2";

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a8c878ab/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 7fa7600..3189955 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
@@ -198,6 +198,21 @@ public class TestDatabaseProvider extends AbstractTestWithStaticConfiguration {
     connection.close();
   }
 
+  @Test
+  public void testGrantDuplicateonDb() throws Exception {
+
+    Connection connection = context.createConnection(ADMIN1);
+    Statement statement = context.createStatement(connection);
+    statement.execute("CREATE ROLE user_role");
+
+    // Grant only SELECT on Database
+    statement.execute("GRANT SELECT ON DATABASE db1 TO ROLE user_role");
+    statement.execute("GRANT SELECT ON DATABASE DB1 TO ROLE user_role");
+    statement.execute("GRANT SELECT ON DATABASE Db1 TO ROLE user_role");
+    statement.close();
+    connection.close();
+  }
+
   private File doSetupForGrantDbTests() throws Exception {
     super.setupAdmin();