diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java index ce26a34..2d8e483 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java @@ -2444,7 +2444,7 @@ public void alter_partitions(final String db_name, final String tbl_name, for (Partition tmpPart : new_parts) { Partition oldTmpPart = null; if (olditr.hasNext()) { - oldTmpPart = (Partition) olditr.next(); + oldTmpPart = olditr.next(); } else { throw new InvalidOperationException("failed to alterpartitions"); @@ -3670,7 +3670,7 @@ public PrincipalPrivilegeSet get_table_privilege_set(final String dbName, @Override public boolean grant_role(final String roleName, - final String userName, final PrincipalType principalType, + final String principalName, final PrincipalType principalType, final String grantor, final PrincipalType grantorType, final boolean grantOption) throws MetaException, TException { incrementCounter("add_role_member"); @@ -3679,7 +3679,15 @@ public boolean grant_role(final String roleName, try { RawStore ms = getMS(); Role role = ms.getRole(roleName); - ret = ms.grantRole(role, userName, principalType, grantor, grantorType, grantOption); + if(principalType == PrincipalType.ROLE){ + //check if this grant statement will end up creating a cycle + if(isNewRoleAParent(principalName, roleName)){ + throw new MetaException("Cannot grant role " + principalName + " to " + roleName + + " as " + roleName + " already belongs to the role " + principalName + + ". (no cycles allowed)"); + } + } + ret = ms.grantRole(role, principalName, principalType, grantor, grantorType, grantOption); } catch (MetaException e) { throw e; } catch (Exception e) { @@ -3688,6 +3696,29 @@ public boolean grant_role(final String roleName, return ret; } + + + /** + * Check if newRole is in parent hierarchy of curRole + * @param newRole + * @param curRole + * @return true if newRole is curRole or present in its hierarchy + * @throws MetaException + */ + private boolean isNewRoleAParent(String newRole, String curRole) throws MetaException { + if(newRole.equals(curRole)){ + return true; + } + //do this check recursively on all the parent roles of curRole + List parentRoleMaps = getMS().listRoles(curRole, PrincipalType.ROLE); + for(MRoleMap parentRole : parentRoleMaps){ + if(isNewRoleAParent(newRole, parentRole.getRole().getRoleName())){ + return true; + } + } + return false; + } + public List list_roles(final String principalName, final PrincipalType principalType) throws MetaException, TException { incrementCounter("list_roles"); diff --git a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAccessController.java b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAccessController.java index 906aebb..adbd1a8 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAccessController.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAccessController.java @@ -30,6 +30,7 @@ import org.apache.hadoop.hive.metastore.api.HiveObjectPrivilege; import org.apache.hadoop.hive.metastore.api.HiveObjectRef; import org.apache.hadoop.hive.metastore.api.HiveObjectType; +import org.apache.hadoop.hive.metastore.api.MetaException; import org.apache.hadoop.hive.metastore.api.PrivilegeBag; import org.apache.hadoop.hive.metastore.api.PrivilegeGrantInfo; import org.apache.hadoop.hive.metastore.api.Role; @@ -202,9 +203,11 @@ public void grantRole(List hivePrincipals, List roleNames AuthorizationUtils.getThriftPrincipalType(grantorPrinc.getType()), grantOption ); - } catch (Exception e) { - String msg = "Error granting roles for " + hivePrincipal.getName() + " to role " + roleName - + hivePrincipal.getName(); + } catch (MetaException e) { + throw new HiveAuthorizationPluginException(e.getMessage(), e); + } catch (Exception e) { + String msg = "Error granting roles for " + hivePrincipal.getName() + " to role " + + roleName + ": " + e.getMessage(); throw new HiveAuthorizationPluginException(msg, e); } } diff --git a/ql/src/test/queries/clientnegative/authorization_role_cycles1.q b/ql/src/test/queries/clientnegative/authorization_role_cycles1.q new file mode 100644 index 0000000..c083e2d --- /dev/null +++ b/ql/src/test/queries/clientnegative/authorization_role_cycles1.q @@ -0,0 +1,8 @@ +set hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactory; +-- this is applicable to any security mode as check is in metastore +create role role1; +create role role2; +grant role role1 to role role2; + +-- this will create a cycle +grant role role2 to role role1; \ No newline at end of file diff --git a/ql/src/test/queries/clientnegative/authorization_role_cycles2.q b/ql/src/test/queries/clientnegative/authorization_role_cycles2.q new file mode 100644 index 0000000..bfc07d0 --- /dev/null +++ b/ql/src/test/queries/clientnegative/authorization_role_cycles2.q @@ -0,0 +1,19 @@ +set hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactory; +-- this is applicable to any security mode as check is in metastore + +create role role1; + +create role role2; +grant role role2 to role role1; + +create role role3; +grant role role3 to role role2; + +create role role4; +grant role role4 to role role3; + +create role role5; +grant role role5 to role role4; + +-- this will create a cycle in middle of the hierarchy +grant role role2 to role role4; diff --git a/ql/src/test/results/clientnegative/authorization_role_cycles1.q.out b/ql/src/test/results/clientnegative/authorization_role_cycles1.q.out new file mode 100644 index 0000000..9d2c3be --- /dev/null +++ b/ql/src/test/results/clientnegative/authorization_role_cycles1.q.out @@ -0,0 +1,18 @@ +PREHOOK: query: -- this is applicable to any security mode as check is in metastore +create role role1 +PREHOOK: type: CREATEROLE +POSTHOOK: query: -- this is applicable to any security mode as check is in metastore +create role role1 +POSTHOOK: type: CREATEROLE +PREHOOK: query: create role role2 +PREHOOK: type: CREATEROLE +POSTHOOK: query: create role role2 +POSTHOOK: type: CREATEROLE +PREHOOK: query: grant role role1 to role role2 +PREHOOK: type: GRANT_ROLE +POSTHOOK: query: grant role role1 to role role2 +POSTHOOK: type: GRANT_ROLE +PREHOOK: query: -- this will create a cycle +grant role role2 to role role1 +PREHOOK: type: GRANT_ROLE +FAILED: Execution Error, return code 1 from org.apache.hadoop.hive.ql.exec.DDLTask. org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAuthorizationPluginException: Cannot grant role role1 to role2 as role2 already belongs to the role role1. (no cycles allowed) diff --git a/ql/src/test/results/clientnegative/authorization_role_cycles2.q.out b/ql/src/test/results/clientnegative/authorization_role_cycles2.q.out new file mode 100644 index 0000000..be9a491 --- /dev/null +++ b/ql/src/test/results/clientnegative/authorization_role_cycles2.q.out @@ -0,0 +1,44 @@ +PREHOOK: query: -- this is applicable to any security mode as check is in metastore + +create role role1 +PREHOOK: type: CREATEROLE +POSTHOOK: query: -- this is applicable to any security mode as check is in metastore + +create role role1 +POSTHOOK: type: CREATEROLE +PREHOOK: query: create role role2 +PREHOOK: type: CREATEROLE +POSTHOOK: query: create role role2 +POSTHOOK: type: CREATEROLE +PREHOOK: query: grant role role2 to role role1 +PREHOOK: type: GRANT_ROLE +POSTHOOK: query: grant role role2 to role role1 +POSTHOOK: type: GRANT_ROLE +PREHOOK: query: create role role3 +PREHOOK: type: CREATEROLE +POSTHOOK: query: create role role3 +POSTHOOK: type: CREATEROLE +PREHOOK: query: grant role role3 to role role2 +PREHOOK: type: GRANT_ROLE +POSTHOOK: query: grant role role3 to role role2 +POSTHOOK: type: GRANT_ROLE +PREHOOK: query: create role role4 +PREHOOK: type: CREATEROLE +POSTHOOK: query: create role role4 +POSTHOOK: type: CREATEROLE +PREHOOK: query: grant role role4 to role role3 +PREHOOK: type: GRANT_ROLE +POSTHOOK: query: grant role role4 to role role3 +POSTHOOK: type: GRANT_ROLE +PREHOOK: query: create role role5 +PREHOOK: type: CREATEROLE +POSTHOOK: query: create role role5 +POSTHOOK: type: CREATEROLE +PREHOOK: query: grant role role5 to role role4 +PREHOOK: type: GRANT_ROLE +POSTHOOK: query: grant role role5 to role role4 +POSTHOOK: type: GRANT_ROLE +PREHOOK: query: -- this will create a cycle in middle of the hierarchy +grant role role2 to role role4 +PREHOOK: type: GRANT_ROLE +FAILED: Execution Error, return code 1 from org.apache.hadoop.hive.ql.exec.DDLTask. org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAuthorizationPluginException: Cannot grant role role4 to role2 as role2 already belongs to the role role4. (no cycles allowed)