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 <[email protected]> Authored: Mon Feb 22 13:14:26 2016 -0800 Committer: Sravya Tirukkovalur <[email protected]> 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(); }
