SENTRY-2432: The case of a username is ignored when determining object ownership (Na Li, reviewed by Fredy Wijaya, Kalyan Kumar Kalvagadda, Arjun Mishra)
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
index f29c455..f5802d7 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
@@ -317,7 +317,7 @@
if (roleNames == null || roleNames.isEmpty()) {
return paramBuilder;
}
- paramBuilder.newChild().addSet("role.roleName == ", roleNames);
+ paramBuilder.newChild().addSet("role.roleName == ", roleNames, true);
paramBuilder.addString("roles.contains(role)");
return paramBuilder;
}
@@ -339,7 +339,7 @@
if (userNames == null || userNames.isEmpty()) {
return paramBuilder;
}
- paramBuilder.newChild().addSet("user.userName == ", userNames);
+ paramBuilder.newChild().addSet("user.userName == ", userNames, false);
paramBuilder.addString("users.contains(user)");
return paramBuilder;
}
@@ -362,7 +362,7 @@
* @param values
* @return this
*/
- QueryParamBuilder addSet(String prefix, Iterable<String> values) {
+ QueryParamBuilder addSet(String prefix, Iterable<String> values, boolean toLowerCase) {
if (values == null) {
return this;
}
@@ -371,7 +371,8 @@
for(String name: values) {
// Append index to the varName
String vName = "var" + paramId.toString();
- addCustomParam(prefix + ":" + vName, vName, name.trim().toLowerCase());
+
+ addCustomParam(prefix + ":" + vName, vName, (toLowerCase) ? name.trim().toLowerCase() : name.trim());
paramId.incrementAndGet();
}
return this;
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 b387a22..869605e 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
@@ -386,7 +386,7 @@
tm.executeTransactionWithRetry(
pm -> {
pm.setDetachAllOnCommit(false); // No need to detach objects
- String trimmedUserName = trimAndLower(userName);
+ String trimmedUserName = userName.trim();
if (getUser(pm, trimmedUserName) != null) {
throw new SentryAlreadyExistsException("User: " + trimmedUserName);
}
@@ -759,13 +759,13 @@
execute(update, pm -> {
pm.setDetachAllOnCommit(false); // No need to detach objects
- String trimmedEntityName = trimAndLower(name);
+
// first do grant check
grantOptionCheck(pm, grantorPrincipal, privilege);
// Alter sentry Role and grant Privilege.
MSentryPrivilege mPrivilege = alterSentryGrantPrivilegeCore(pm, type,
- trimmedEntityName, privilege);
+ name, privilege);
if (mPrivilege != null) {
// update the privilege to be the one actually updated.
@@ -829,6 +829,12 @@
String entityName, TSentryPrivilege privilege)
throws SentryNoSuchObjectException, SentryInvalidInputException {
MSentryPrivilege mPrivilege = null;
+
+ entityName = entityName.trim();
+ if (type.equals(SentryPrincipalType.ROLE)) {
+ entityName = entityName.toLowerCase();
+ }
+
PrivilegePrincipal mEntity = getEntity(pm, entityName, type);
if (mEntity == null) {
if(type == SentryPrincipalType.ROLE) {
@@ -955,11 +961,10 @@
final Update update) throws Exception {
execute(update, pm -> {
pm.setDetachAllOnCommit(false); // No need to detach objects
- String trimmedEntityName = trimAndLower(principalName);
// Alter sentry Role and grant Privilege.
MSentryPrivilege mPrivilege = alterSentryGrantPrivilegeCore(pm, entityType,
- trimmedEntityName, privilege);
+ principalName, privilege);
if (mPrivilege != null) {
// update the privilege to be the one actually updated.
@@ -1024,7 +1029,7 @@
return tm.executeTransaction(
new TransactionBlock<MSentryUser>() {
public MSentryUser execute(PersistenceManager pm) throws Exception {
- String trimmedUserName = trimAndLower(userName);
+ String trimmedUserName = userName.trim();
MSentryUser sentryUser = getUser(pm, trimmedUserName);
if (sentryUser == null) {
if (throwExceptionIfNotExist) {
@@ -1115,11 +1120,11 @@
final Update update) throws Exception {
execute(update, pm -> {
pm.setDetachAllOnCommit(false); // No need to detach objects
- String trimmedEntityName = safeTrimLower(principalName);
+
// first do revoke check
grantOptionCheck(pm, grantorPrincipal, tPrivilege);
- alterSentryRevokePrivilegeCore(pm, type, trimmedEntityName, tPrivilege);
+ alterSentryRevokePrivilegeCore(pm, type, principalName, tPrivilege);
return null;
});
}
@@ -1152,6 +1157,15 @@
private void alterSentryRevokePrivilegeCore(PersistenceManager pm, SentryPrincipalType type,
String entityName, TSentryPrivilege tPrivilege)
throws SentryNoSuchObjectException, SentryInvalidInputException {
+ if (entityName == null) {
+ throw new SentryInvalidInputException("Null entityName");
+ }
+
+ entityName = entityName.trim();
+ if (type.equals(SentryPrincipalType.ROLE)) {
+ entityName = entityName.toLowerCase();
+ }
+
PrivilegePrincipal mEntity = getEntity(pm, entityName, type);
if (mEntity == null) {
if(type == SentryPrincipalType.ROLE) {
@@ -1467,7 +1481,7 @@
private void dropSentryUserCore(PersistenceManager pm, String userName)
throws SentryNoSuchObjectException {
- String lUserName = trimAndLower(userName);
+ String lUserName = userName.trim();
MSentryUser sentryUser = getUser(pm, lUserName);
if (sentryUser == null) {
throw noSuchUser(lUserName);
@@ -3996,7 +4010,7 @@
mSentryRoles = (List<MSentryRole>)query.execute();
} else {
QueryParamBuilder paramBuilder = QueryParamBuilder.newQueryParamBuilder(QueryParamBuilder.Op.OR);
- paramBuilder.addSet("roleName == ", roleNames);
+ paramBuilder.addSet("roleName == ", roleNames, true);
query.setFilter(paramBuilder.toString());
mSentryRoles =
(List<MSentryRole>) query.executeWithMap(paramBuilder.getArguments());
diff --git a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
index 4a9afe3..1568dc1 100644
--- a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
+++ b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
@@ -3483,7 +3483,7 @@
names.add("bar");
names.add("bob");
- paramBuilder.addSet("prefix == ", names);
+ paramBuilder.addSet("prefix == ", names, true);
assertEquals("(prefix == :var0 && prefix == :var1 && prefix == :var2)",
paramBuilder.toString());
params = paramBuilder.getArguments();
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
index d3294f4..29d2256 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
@@ -134,6 +134,60 @@
}
/**
+ * Verify that the user who creases database has owner privilege on this database
+ * and also makes sure that HDFS ACL rules are updated.
+ *
+ * @throws Exception
+ */
+ @Test
+ public void testCreateDatabaseUserNameCase() throws Throwable {
+ dbNames = new String[]{DB1};
+ roles = new String[]{"admin_role", "create_db1"};
+ String USER1_1_UPPERCASE = USER1_1.toUpperCase();
+
+ // create required roles
+ setupUserRoles(roles, statementAdmin);
+
+ statementAdmin.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE");
+
+ // setup privileges for USER1
+ statementAdmin.execute("GRANT CREATE ON SERVER server1" + " TO ROLE create_db1");
+
+ Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1);
+ Statement statementUSER1_1 = connectionUSER1_1.createStatement();
+ Connection connectionUSER1_1_UPPERCASE = hiveServer2.createConnection(USER1_1_UPPERCASE, USER1_1_UPPERCASE);
+ Statement statementUSER1_1_UPPERCASE = connectionUSER1_1_UPPERCASE.createStatement();
+
+ try {
+ // USER1 creates test DB
+ statementUSER1_1.execute("CREATE DATABASE " + DB1);
+
+ // verify privileges created for new database
+ verifyTableOwnerPrivilegeExistForPrincipal(statementUSER1_1, SentryPrincipalType.USER,
+ Lists.newArrayList(USER1_1),
+ DB1, "", 1);
+
+ try {
+ statementUSER1_1_UPPERCASE.execute("CREATE TABLE " + DB1 + "." + tableName1
+ + " (under_col int comment 'the under column')");
+ Assert.fail("Expect creating table to fail for user " + USER1_1_UPPERCASE);
+ } catch (HiveSQLException ex) {
+ LOGGER.info(
+ "Expect creating table to fail for user " + USER1_1_UPPERCASE + ". " + ex.getMessage());
+ }
+ } finally {
+ statementAdmin.close();
+ connection.close();
+
+ statementUSER1_1.close();
+ connectionUSER1_1.close();
+
+ statementUSER1_1_UPPERCASE.close();
+ connectionUSER1_1_UPPERCASE.close();
+ }
+ }
+
+ /**
* Verify that the user who does not creases database has no owner privilege on this database and
* also makes sure that there are not HDFS ACL.
*
@@ -257,7 +311,7 @@
*
* @throws Exception
*/
- @Ignore("Enable the test once HIVE-18031 is in the hiver version integrated with Sentry")
+ @Ignore("Enable the test once HIVE-18031 is in the hive version integrated with Sentry")
@Test
public void testAuthorizeAlterDatabaseSetOwner() throws Throwable {
String ownerRole = "owner_role";
@@ -364,12 +418,92 @@
}
/**
+ * Verify that the user who can call alter database set owner on this table, and the user
+ * case is preserved
+ *
+ * @throws Exception
+ */
+ @Ignore("Enable the test once HIVE-18031 is in the hive version integrated with Sentry")
+ @Test
+ public void testAuthorizeAlterDatabaseSetOwnerUserNameCase() throws Exception {
+ String ownerRole = "owner_role";
+ String allWithGrantRole = "allWithGrant_role";
+ dbNames = new String[]{DB1};
+ roles = new String[]{"admin_role", "create_on_server", ownerRole};
+ String USER1_2_UPPERCASE = USER1_2.toUpperCase();
+
+ // create required roles, and assign them to USERGROUP1
+ setupUserRoles(roles, statementAdmin);
+
+ // create test DB
+ statementAdmin.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE");
+
+ // setup privileges for USER1
+ statementAdmin.execute("GRANT CREATE ON SERVER " + SERVER_NAME + " TO ROLE create_on_server");
+
+ Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1);
+ Statement statementUSER1_1 = connectionUSER1_1.createStatement();
+ Connection connectionUSER1_2_UPPERCASE = hiveServer2.createConnection(USER1_2_UPPERCASE, USER1_2_UPPERCASE);
+ Statement statementUSER1_2_UPPERCASE = connectionUSER1_2_UPPERCASE.createStatement();
+ Connection connectionUSER2_1 = hiveServer2.createConnection(USER2_1, USER2_1);
+ Statement statementUSER2_1 = connectionUSER2_1.createStatement();
+
+ // USER1_1 create database and becomes owner of DB1
+ statementUSER1_1.execute("CREATE DATABASE " + DB1);
+
+ // create role that has all with grant on the table, and assign to USERGROUP2
+ // so USER2_1 can alter DB1 owner
+ statementAdmin.execute("create role " + allWithGrantRole);
+ statementAdmin.execute("grant role " + allWithGrantRole + " to group " + USERGROUP2);
+ statementAdmin.execute("GRANT ALL ON DATABASE " + DB1 + " to role " +
+ allWithGrantRole + " with grant option");
+
+ try {
+ // user2_1 having all with grant on this DB and can issue command: alter database set owner
+ // alter database set owner to a user USER1_2
+ statementUSER2_1
+ .execute("ALTER DATABASE " + DB1 + " SET OWNER USER " + USER1_2);
+
+ // verify privileges is transferred to user USER1_2
+ verifyTableOwnerPrivilegeExistForPrincipal(statementAdmin, SentryPrincipalType.USER,
+ Lists.newArrayList(USER1_2),
+ DB1, "", 1);
+
+ // verify that another user whose name differ from USER1_2 only in case cannot share the
+ // owner privilege with USER1_2
+ try {
+ statementUSER1_2_UPPERCASE.execute("CREATE TABLE " + DB1 + "." + tableName1
+ + " (under_col int comment 'the under column')");
+ Assert.fail("Expect creating table to fail for user " + USER1_2_UPPERCASE);
+ } catch (HiveSQLException ex) {
+ LOGGER.info(
+ "Expect creating table to fail for user " + USER1_2_UPPERCASE + ". " + ex.getMessage());
+ }
+ } finally {
+ statementAdmin.execute("drop role " + allWithGrantRole);
+
+ statementAdmin.close();
+ connection.close();
+
+ statementUSER1_1.close();
+ connectionUSER1_1.close();
+
+ statementUSER1_2_UPPERCASE.close();
+ connectionUSER1_2_UPPERCASE.close();
+
+ statementUSER2_1.close();
+ connectionUSER2_1.close();
+ }
+ }
+
+
+ /**
* Verify that if the same user is owner of both DB and table, after alter DB's owner,
* the table owner is still that user
*
* @throws Exception
*/
- @Ignore("Enable the test once HIVE-18031 is in the hiver version integrated with Sentry")
+ @Ignore("Enable the test once HIVE-18031 is in the hive version integrated with Sentry")
@Test
public void testAlterDBNotDropTableOwnerSameOwner() throws Exception {
String allWithGrantRole = "allWithGrant_role";
@@ -440,7 +574,7 @@
*
* @throws Exception
*/
- @Ignore("Enable the test once HIVE-18031 is in the hiver version integrated with Sentry")
+ @Ignore("Enable the test once HIVE-18031 is in the hive version integrated with Sentry")
@Test
public void testAlterDBNotDropTableOwnerDifferentOwner() throws Exception {
String allWithGrantRole = "allWithGrant_role";
@@ -630,6 +764,66 @@
}
/**
+ * Verify that the user who creases table has owner privilege on this table, and owner case
+ * is preserved
+ *
+ * @throws Exception
+ */
+ @Test
+ public void testCreateTableUserNameCase() throws Exception {
+ dbNames = new String[]{DB1};
+ roles = new String[]{"admin_role", "create_db1"};
+ String USER1_1_UPPERCASE = USER1_1.toUpperCase();
+
+ // create required roles
+ setupUserRoles(roles, statementAdmin);
+
+ // create test DB
+ statementAdmin.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE");
+ statementAdmin.execute("CREATE DATABASE " + DB1);
+
+ // setup privileges for USER1
+ statementAdmin.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE create_db1");
+ statementAdmin.execute("USE " + DB1);
+
+ Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1);
+ Statement statementUSER1_1 = connectionUSER1_1.createStatement();
+ Connection connectionUSER1_1_UPPERCASE = hiveServer2.createConnection(USER1_1_UPPERCASE, USER1_1_UPPERCASE);
+ Statement statementUSER1_1_UPPERCASE = connectionUSER1_1_UPPERCASE.createStatement();
+
+ try {
+ // USER1 create table
+ statementUSER1_1.execute("CREATE TABLE " + DB1 + "." + tableName1
+ + " (under_col int comment 'the under column')");
+
+ // verify privileges created for new table
+ verifyTableOwnerPrivilegeExistForPrincipal(statementUSER1_1, SentryPrincipalType.USER,
+ Lists.newArrayList(USER1_1),
+ DB1, tableName1, 1);
+
+ // verify that USER1_1_UPPERCASE does not have privilege on this table
+ try {
+ statementUSER1_1_UPPERCASE
+ .execute("INSERT INTO TABLE " + DB1 + "." + tableName1 + " VALUES (35)");
+ Assert.fail("Expect inserting table to fail for user " + USER1_1_UPPERCASE);
+ } catch (HiveSQLException ex) {
+ LOGGER.info(
+ "Expect inserting table to fail for user " + USER1_1_UPPERCASE + ". " + ex
+ .getMessage());
+ }
+ } finally {
+ statementAdmin.close();
+ connection.close();
+
+ statementUSER1_1.close();
+ connectionUSER1_1.close();
+
+ statementUSER1_1_UPPERCASE.close();
+ connectionUSER1_1_UPPERCASE.close();
+ }
+ }
+
+ /**
* Verify that no owner privilege is created on table created by an admin user
*
* @throws Exception
@@ -701,7 +895,7 @@
*
* @throws Exception
*/
- @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry")
+ @Ignore("Enable the test once HIVE-18762 is in the hive version integrated with Sentry")
@Test
public void testAlterTable() throws Throwable {
dbNames = new String[]{DB1};
@@ -782,12 +976,99 @@
}
/**
+ * Verify that the owner privilege is updated when the ownership is changed, and its user name
+ * case is preserved
+ *
+ * @throws Exception
+ */
+ @Ignore("Enable the test once HIVE-18762 is in the hive version integrated with Sentry")
+ @Test
+ public void testAlterTableUserNameCase() throws Exception {
+ dbNames = new String[]{DB1};
+ String allWithGrantRole = "allWithGrant_role";
+ roles = new String[]{"admin_role", "create_db1", "owner_role"};
+ String USER1_2_UPPERCASE = USER1_2.toUpperCase();
+
+ // create required roles
+ setupUserRoles(roles, statementAdmin);
+
+ // create test DB
+ statementAdmin.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE");
+ statementAdmin.execute("CREATE DATABASE " + DB1);
+
+ // setup privileges for USER1
+ statementAdmin.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE create_db1");
+ statementAdmin.execute("USE " + DB1);
+
+ Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1);
+ Statement statementUSER1_1 = connectionUSER1_1.createStatement();
+ Connection connectionUSER1_2_UPPERCASE = hiveServer2.createConnection(USER1_2_UPPERCASE, USER1_2_UPPERCASE);
+ Statement statementUSER1_2_UPPERCASE = connectionUSER1_2_UPPERCASE.createStatement();
+ Connection connectionUSER2_1 = hiveServer2.createConnection(USER2_1, USER2_1);
+ Statement statementUSER2_1 = connectionUSER2_1.createStatement();
+
+ // USER1 create table
+ statementUSER1_1.execute("CREATE TABLE " + DB1 + "." + tableName1
+ + " (under_col int comment 'the under column')");
+
+ // create role that has all with grant on the table, and assign to USERGROUP2
+ // so USER2_1 can alter DB1.tableName1 owner
+ statementAdmin.execute("create role " + allWithGrantRole);
+ statementAdmin.execute("grant role " + allWithGrantRole + " to group " + USERGROUP2);
+ statementAdmin.execute("GRANT ALL ON DATABASE " + DB1 + " to role " +
+ allWithGrantRole + " with grant option");
+
+ // verify privileges created for new table
+ verifyTableOwnerPrivilegeExistForPrincipal(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1),
+ DB1, tableName1, 1);
+
+ try {
+ // user2_1 having all with grant on this DB and can issue command: alter table set owner
+ // to set owner to USER1_2
+ statementUSER2_1
+ .execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER USER " + USER1_2);
+
+ // verify privileges is transferred to user USER1_2
+ verifyTableOwnerPrivilegeExistForPrincipal(statementAdmin, SentryPrincipalType.USER,
+ Lists.newArrayList(USER1_2),
+ DB1, tableName1, 1);
+
+ // verify that another user whose name differ from USER1_2 only in case cannot share the
+ // owner privilege with USER1_2
+ try {
+ statementUSER1_2_UPPERCASE
+ .execute("INSERT INTO TABLE " + DB1 + "." + tableName1 + " VALUES (35)");
+ Assert.fail("Expect inserting table to fail for user " + USER1_2_UPPERCASE);
+ } catch (HiveSQLException ex) {
+ LOGGER.info(
+ "Expect inserting table to fail for user " + USER1_2_UPPERCASE + ". " + ex
+ .getMessage());
+ }
+ } finally {
+
+ statementAdmin.execute("drop role " + allWithGrantRole);
+
+ statementAdmin.close();
+ connection.close();
+
+ statementUSER1_1.close();
+ connectionUSER1_1.close();
+
+ statementUSER1_2_UPPERCASE.close();
+ connectionUSER1_2_UPPERCASE.close();
+
+ statementUSER2_1.close();
+ connectionUSER2_1.close();
+ }
+ }
+
+ /**
* Verify that the owner privilege is updated when the ownership is changed when DB name
* is not explicitly specified
*
* @throws Exception
*/
- @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry")
+ @Ignore("Enable the test once HIVE-18762 is in the hive version integrated with Sentry")
@Test
public void testAlterTableWithoutDB() throws Exception {
dbNames = new String[]{DB1};
@@ -865,7 +1146,7 @@
*
* @throws Exception
*/
- @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry")
+ @Ignore("Enable the test once HIVE-18762 is in the hive version integrated with Sentry")
@Test
public void testAlterTableNegativeWithoutDB() throws Exception {
dbNames = new String[]{DB1};
@@ -950,7 +1231,7 @@
*
* @throws Exception
*/
- @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry")
+ @Ignore("Enable the test once HIVE-18762 is in the hive version integrated with Sentry")
@Test
public void testAuthorizeAlterTableSetOwner() throws Exception {
String ownerRole = "owner_role";
@@ -1069,7 +1350,7 @@
* Verify that no owner privilege is granted when the ownership is changed to sentry admin user
* @throws Exception
*/
- @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry")
+ @Ignore("Enable the test once HIVE-18762 is in the hive version integrated with Sentry")
@Test
public void testAlterTableAdmin() throws Exception {
dbNames = new String[]{DB1};
