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 <[email protected]> Authored: Wed Nov 19 21:53:21 2014 -0800 Committer: Arun Suresh <[email protected]> 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();
