sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sra...@apache.org
Subject incubator-sentry git commit: SENTRY-1035: Generic service does not handle group name casing correctly (Sravya Tirukkovalur, Reviewed by: Hao Hao and Lenni Kuff)
Date Mon, 22 Feb 2016 21:14:58 GMT
Repository: incubator-sentry
Updated Branches:
  refs/heads/master e6f6c1fbc -> 03b2f1882


SENTRY-1035: Generic service does not handle group name casing correctly (Sravya Tirukkovalur,
Reviewed by: Hao Hao and Lenni Kuff)

Change-Id: I4479b18676b6e8f5ac044a7e74927092db36a9a8


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

Branch: refs/heads/master
Commit: 03b2f18823e23200d100c1610b5a53c4e641ca46
Parents: e6f6c1f
Author: Sravya Tirukkovalur <sravya@cloudera.com>
Authored: Mon Feb 22 13:14:26 2016 -0800
Committer: Sravya Tirukkovalur <sravya@cloudera.com>
Committed: Mon Feb 22 13:14:26 2016 -0800

----------------------------------------------------------------------
 .../service/persistent/DelegateSentryStore.java | 39 +++++++++++++-------
 .../thrift/SentryGenericPolicyProcessor.java    | 22 ++++++++---
 .../db/generic/tools/TestSentryShellSolr.java   | 36 +++++++++++++++---
 .../sentry/tests/e2e/hive/TestOperations.java   | 35 ++++++++++++++++--
 4 files changed, 104 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/03b2f188/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
index fcd40e8..34d3fea 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
@@ -74,7 +74,7 @@ public class DelegateSentryStore implements SentryStoreLayer {
     this.conf = conf;
     //delegated old sentryStore
     this.delegate = new SentryStore(conf);
-    adminGroups = ImmutableSet.copyOf(toTrimedLower(Sets.newHashSet(conf.getStrings(
+    adminGroups = ImmutableSet.copyOf(toTrimmed(Sets.newHashSet(conf.getStrings(
         ServerConfig.ADMIN_GROUPS, new String[]{}))));
   }
 
@@ -113,7 +113,7 @@ public class DelegateSentryStore implements SentryStoreLayer {
       throws SentryNoSuchObjectException {
     boolean rollbackTransaction = true;
     PersistenceManager pm = null;
-    role = toTrimedLower(role);
+    role = toTrimmedLower(role);
     try {
       pm = openTransaction();
       Query query = pm.newQuery(MSentryRole.class);
@@ -161,7 +161,7 @@ public class DelegateSentryStore implements SentryStoreLayer {
   public CommitContext alterRoleGrantPrivilege(String component, String role,
       PrivilegeObject privilege, String grantorPrincipal)
       throws SentryUserException {
-    role = toTrimedLower(role);
+    role = toTrimmedLower(role);
     PersistenceManager pm = null;
     boolean rollbackTransaction = true;
     try{
@@ -192,7 +192,7 @@ public class DelegateSentryStore implements SentryStoreLayer {
   public CommitContext alterRoleRevokePrivilege(String component,
       String role, PrivilegeObject privilege, String grantorPrincipal)
       throws SentryUserException {
-    role = toTrimedLower(role);
+    role = toTrimmedLower(role);
     PersistenceManager pm = null;
     boolean rollbackTransaction = true;
     try{
@@ -241,7 +241,7 @@ public class DelegateSentryStore implements SentryStoreLayer {
     try {
       pm = openTransaction();
 
-      privilegeOperator.renamePrivilege(toTrimedLower(component), toTrimedLower(service),
+      privilegeOperator.renamePrivilege(toTrimmedLower(component), toTrimmedLower(service),
           oldAuthorizables, newAuthorizables, requestor, pm);
 
       CommitContext commitContext = commitUpdateTransaction(pm);
@@ -296,7 +296,7 @@ public class DelegateSentryStore implements SentryStoreLayer {
           + " has no grant!");
     }
     //admin group check
-    if (!Sets.intersection(adminGroups, toTrimedLower(groups)).isEmpty()) {
+    if (!Sets.intersection(adminGroups, toTrimmed(groups)).isEmpty()) {
       return;
     }
     //privilege grant option check
@@ -323,7 +323,7 @@ public class DelegateSentryStore implements SentryStoreLayer {
   @Override
   public Set<String> getGroupsByRoles(String component, Set<String> roles)
       throws SentryUserException {
-    roles = toTrimedLower(roles);
+    roles = toTrimmedLower(roles);
     Set<String> groupNames = Sets.newHashSet();
     if (roles.size() == 0) {
       return groupNames;
@@ -372,7 +372,7 @@ public class DelegateSentryStore implements SentryStoreLayer {
       pm = openTransaction();
       Set<MSentryRole> mRoles = Sets.newHashSet();
       for (String role : roles) {
-        MSentryRole mRole = getRole(toTrimedLower(role), pm);
+        MSentryRole mRole = getRole(toTrimmedLower(role), pm);
         if (mRole != null) {
           mRoles.add(mRole);
         }
@@ -393,15 +393,15 @@ public class DelegateSentryStore implements SentryStoreLayer {
     Preconditions.checkNotNull(component);
     Preconditions.checkNotNull(service);
 
-    component = toTrimedLower(component);
-    service = toTrimedLower(service);
+    component = toTrimmedLower(component);
+    service = toTrimmedLower(service);
 
     Set<PrivilegeObject> privileges = Sets.newHashSet();
     PersistenceManager pm = null;
     try {
       pm = openTransaction();
       //CaseInsensitive roleNames
-      roles = toTrimedLower(roles);
+      roles = toTrimmedLower(roles);
 
       if (groups != null) {
         roles.addAll(delegate.getRoleNamesForGroups(groups));
@@ -470,13 +470,13 @@ public class DelegateSentryStore implements SentryStoreLayer {
 
   private Set<TSentryGroup> toTSentryGroups(Set<String> groups) {
     Set<TSentryGroup> tSentryGroups = Sets.newHashSet();
-    for (String group : toTrimedLower(groups)) {
+    for (String group : groups) {
       tSentryGroups.add(new TSentryGroup(group));
     }
     return tSentryGroups;
   }
 
-  private Set<String> toTrimedLower(Set<String> s) {
+  private Set<String> toTrimmedLower(Set<String> s) {
     if (s == null) {
       return new HashSet<String>();
     }
@@ -487,7 +487,18 @@ public class DelegateSentryStore implements SentryStoreLayer {
     return result;
   }
 
-  private String toTrimedLower(String s) {
+  private Set<String> toTrimmed(Set<String> s) {
+    if (s == null) {
+      return new HashSet<String>();
+    }
+    Set<String> result = Sets.newHashSet();
+    for (String v : s) {
+      result.add(v.trim());
+    }
+    return result;
+  }
+
+  private String toTrimmedLower(String s) {
     if (s == null) {
       return "";
     }

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/03b2f188/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java
b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java
index d07331e..69f275d 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java
@@ -82,7 +82,7 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService.
     this.store = createStore(conf);
     this.handerInvoker = new NotificationHandlerInvoker(createHandlers(conf));
     this.conf = conf;
-    adminGroups = ImmutableSet.copyOf(toTrimedLower(Sets.newHashSet(conf.getStrings(
+    adminGroups = ImmutableSet.copyOf((Sets.newHashSet(conf.getStrings(
         ServerConfig.ADMIN_GROUPS, new String[]{}))));
   }
 
@@ -91,7 +91,7 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService.
     this.store = store;
     this.handerInvoker = new NotificationHandlerInvoker(createHandlers(conf));
     this.conf = conf;
-    adminGroups = ImmutableSet.copyOf(toTrimedLower(Sets.newHashSet(conf.getStrings(
+    adminGroups = ImmutableSet.copyOf(toTrimmed(Sets.newHashSet(conf.getStrings(
         ServerConfig.ADMIN_GROUPS, new String[]{}))));
   }
 
@@ -105,7 +105,7 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService.
     }
   }
 
-  private Set<String> toTrimedLower(Set<String> s) {
+  private Set<String> toTrimmedLower(Set<String> s) {
     if (null == s) {
       return new HashSet<String>();
     }
@@ -116,7 +116,18 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService.
     return result;
   }
 
-  private String toTrimedLower(String s) {
+  private Set<String> toTrimmed(Set<String> s) {
+    if (null == s) {
+      return new HashSet<String>();
+    }
+    Set<String> result = Sets.newHashSet();
+    for (String v : s) {
+      result.add(v.trim());
+    }
+    return result;
+  }
+
+  private String toTrimmedLower(String s) {
     if (Strings.isNullOrEmpty(s)){
       return "";
     }
@@ -128,7 +139,6 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService.
   }
 
   private boolean inAdminGroups(Set<String> requestorGroups) {
-    requestorGroups = toTrimedLower(requestorGroups);
     if (Sets.intersection(adminGroups, requestorGroups).isEmpty()) {
       return false;
     }
@@ -625,7 +635,7 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService.
       @Override
       public Response<Set<String>> handle() throws Exception {
         validateClientVersion(request.getProtocol_version());
-        Set<String> activeRoleNames = toTrimedLower(request.getRoleSet().getRoles());
+        Set<String> activeRoleNames = toTrimmedLower(request.getRoleSet().getRoles());
         Set<String> roleNamesForGroups = store.getRolesByGroups(request.getComponent(),
request.getGroups());
         Set<String> rolesToQuery = request.getRoleSet().isAll() ? roleNamesForGroups
: Sets.intersection(activeRoleNames, roleNamesForGroups);
         Set<PrivilegeObject> privileges = store.getPrivilegesByProvider(request.getComponent(),

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/03b2f188/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java
b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java
index f1a87a8..37cc966 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java
@@ -127,11 +127,10 @@ public class TestSentryShellSolr extends SentryGenericServiceIntegrationBase
{
     runTestAsSubject(new TestOperation() {
       @Override
       public void runTestAsSubject() throws Exception {
-        // Must lower case group names, see SENTRY-1035
-        final boolean lowerCaseGroupNames = true;
-        String TEST_GROUP_1 = lowerCaseGroupNames ? "testgroup1" : "testGroup1";
-        String TEST_GROUP_2 = lowerCaseGroupNames ? "testgroup2" : "testGroup2";
-        String TEST_GROUP_3 = lowerCaseGroupNames ? "testgroup3" : "testGroup3";
+        // Group names are case sensitive - mixed case names should work
+        String TEST_GROUP_1 = "testGroup1";
+        String TEST_GROUP_2 = "testGroup2";
+        String TEST_GROUP_3 = "testGroup3";
 
         // create the role for test
         client.createRole(requestorName, TEST_ROLE_NAME_1, SOLR);
@@ -198,6 +197,33 @@ public class TestSentryShellSolr extends SentryGenericServiceIntegrationBase
{
     });
   }
 
+  @Test
+  public void testCaseSensitiveGroupName() throws Exception {
+    runTestAsSubject(new TestOperation() {
+      @Override
+      public void runTestAsSubject() throws Exception {
+
+        // create the role for test
+        client.createRole(requestorName, TEST_ROLE_NAME_1, SOLR);
+        // add role to a group (lower case)
+        String[] args = { "-arg", "-r", TEST_ROLE_NAME_1, "-g", "group1", "-conf",
+            confPath.getAbsolutePath() };
+        SentryShellSolr.main(args);
+
+        // validate the roles when group name is same case as above
+        args = new String[] { "-lr", "-g", "group1", "-conf", confPath.getAbsolutePath()
};
+        SentryShellSolr sentryShell = new SentryShellSolr();
+        Set<String> roleNames = getShellResultWithOSRedirect(sentryShell, args, true);
+        validateRoleNames(roleNames, TEST_ROLE_NAME_1);
+
+        // roles should be empty when group name is different case than above
+        args = new String[] { "-lr", "-g", "GROUP1", "-conf", confPath.getAbsolutePath()
};
+        roleNames = getShellResultWithOSRedirect(sentryShell, args, true);
+        validateRoleNames(roleNames);
+      }
+      });
+    }
+
   public static String grant(boolean shortOption) {
     return shortOption ? "-gpr" : "--grant_privilege_role";
   }

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/03b2f188/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java
b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java
index 438030b..6ca09c9 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java
@@ -132,6 +132,35 @@ public class TestOperations extends AbstractTestWithStaticConfiguration
{
 
   }
 
+  @Test
+  public void testInsertInto() throws Exception{
+    File dataFile;
+    dataFile = new File(dataDir, SINGLE_TYPE_DATA_FILE_NAME);
+    FileOutputStream to = new FileOutputStream(dataFile);
+    Resources.copy(Resources.getResource(SINGLE_TYPE_DATA_FILE_NAME), to);
+    to.close();
+
+    adminCreate(DB1, null);
+    policyFile
+        .addPermissionsToRole("all_db1", privileges.get("all_db1"))
+        .addPermissionsToRole("all_uri", "server=server1->uri=file://" + dataDir)
+        .addRolesToGroup(USERGROUP1, "all_db1", "all_uri");
+
+
+    writePolicyFile(policyFile);
+
+    Connection connection = context.createConnection(USER1_1);
+    Statement statement = context.createStatement(connection);
+    statement.execute("Use " + DB1);
+    statement.execute("create table bar (key int)");
+    statement.execute("load data local inpath '" + dataFile.getPath() + "' into table bar");
+    statement.execute("create table foo (key int) partitioned by (part int) stored as parquet");
+    statement.execute("insert into table foo PARTITION(part=1) select key from bar");
+
+    statement.close();
+    connection.close();
+  }
+
   /* Test all operations that require create on Database alone
   1. Create table : HiveOperation.CREATETABLE
   */
@@ -294,7 +323,7 @@ public class TestOperations extends AbstractTestWithStaticConfiguration
{
   }
 
   private void assertSemanticException(Statement stmt, String command) throws SQLException{
-    context.assertSentrySemanticException(stmt,command, semanticException);
+    context.assertSentrySemanticException(stmt, command, semanticException);
   }
 
   /*
@@ -987,7 +1016,7 @@ public class TestOperations extends AbstractTestWithStaticConfiguration
{
 
     Connection connection = context.createConnection(USER1_1);
     Statement statement = context.createStatement(connection);
-    assertSemanticException(statement, "insert overwrite directory '" + location + "' select
* from " + DB1 + ".tb1" );
+    assertSemanticException(statement, "insert overwrite directory '" + location + "' select
* from " + DB1 + ".tb1");
     statement.execute("insert overwrite table " + DB2 + ".tb2 select * from " + DB1 + ".tb1");
     statement.close();
     connection.close();
@@ -995,7 +1024,7 @@ public class TestOperations extends AbstractTestWithStaticConfiguration
{
     connection = context.createConnection(USER2_1);
     statement = context.createStatement(connection);
     statement.execute("insert overwrite directory '" + location + "' select * from " + DB1
+ ".tb1" );
-    assertSemanticException(statement,"insert overwrite table " + DB2 + ".tb2 select * from
" + DB1 + ".tb1");
+    assertSemanticException(statement, "insert overwrite table " + DB2 + ".tb2 select * from
" + DB1 + ".tb1");
     statement.close();
     connection.close();
   }


Mime
View raw message