mneethiraj commented on code in PR #1324:
URL: https://github.com/apache/ranger/pull/1324#discussion_r4354035187
##########
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();
Review Comment:
`nestedRoleName` => `memberRoleName`
##########
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 499 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]
