mneethiraj commented on code in PR #1324:
URL: https://github.com/apache/ranger/pull/1324#discussion_r4354373611


##########
security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java:
##########
@@ -454,6 +461,117 @@ private void ensureRoleNameUpdateAllowed(String roleName) 
throws Exception {
         }
     }
 
+    /**
+     * Rejects a new role whose nested roles contain it, directly or 
transitively.
+     *
+     * The ref table can't be used here: when an earlier save referenced a 
not-yet-existing role with
+     * createNonExistUserGroupRole=true (e.g. earlier in the same import), its 
x_role_ref_role row is only written
+     * after commit, so that edge exists only in the stored role JSON. So walk 
the new role's nested roles downwards
+     * using their stored definitions. Each role is loaded at most once; a 
role with no nested roles costs nothing.
+     */
+    private void ensureRoleNotNestedInItself(RangerRole role) {
+        Set<String>   visited = new HashSet<>();
+        Deque<String> pending = new ArrayDeque<>();
+
+        for (RangerRole.RoleMember member : role.getRoles()) {
+            if (member != null && StringUtils.isNotEmpty(member.getName())) {
+                pending.add(member.getName());
+            }
+        }
+
+        while (!pending.isEmpty()) {
+            String nestedRoleName = pending.poll();
+
+            if (StringUtils.equals(nestedRoleName, role.getName())) {
+                throw createNestingCycleException(role.getName());
+            }
+
+            if (!visited.add(nestedRoleName)) {
+                continue;
+            }
+
+            XXRole xxNestedRole = 
daoMgr.getXXRole().findByRoleName(nestedRoleName);

Review Comment:
   Consider replacing 493  to 508 with a new method in XXRoleDao to retrieve 
the list of refereced roles:
   
   ```
     XXRoleDao roleDao = daoMgr.getXXRole();
   
     while (!pending.isEmpty()) {
       ...
       Collection<String> refRoleNames = 
roleDao.getRoleMemberNames(memberRoleName);
   
       pending.add(refRoleNames);
     }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to