You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by "vyommani (via GitHub)" <gi...@apache.org> on 2026/10/21 08:04:15 UTC
[PR] RANGER-5850: Validate nested role membership for cycles and avoid rec… [ranger]
vyommani opened a new pull request, #1324:
URL: https://github.com/apache/ranger/pull/1324
## What changes were proposed in this pull request?
Roles can contain other roles, but nothing stops a role from ending up inside itself, directly or through other roles. Several RoleREST helpers also expand nested roles recursively, without tracking roles already visited. When a cycle is present, requests that expand it fail.
Validation on save (RoleDBStore)
Create: reject a role whose nested roles contain it. The check walks the stored definitions of the nested roles, loading each role once, so it also catches references that are only written after commit (createNonExistUserGroupRole, for example within an import).
Update: check only the nested roles the update adds. Reject the update if one of them is the role itself or one of its ancestors, found by walking x_role_ref_role ids upward with a visited set. An update that adds no nested role does no extra work.
Grant: relies on updateRole for this check, including granting a role into itself. The separate recursive check in addUsersGroupsAndRoles is removed.
Role expansion (RoleREST)
getRoleMemberNames and getRoleMembers are now iterative, with a visited set. Each nested role is loaded once, and a cycle already present in the data terminates.
ensureRoleAccess reuses the roles loaded during expansion instead of fetching them again. The access decision is unchanged.
getUserRoles shares one visited set across all of the user’s roles, so a nested role common to several of them is loaded once.
A nested role named in a role definition that doesn’t exist (yet) is skipped instead of failing the request.
Behaviour changes
Saving a role that would contain itself now fails with: “a role can not contain itself, directly or through nested roles.”
Existing data with cycles can still be read, and those roles can still be edited.
## How was this patch tested?
TestRoleDBStore:
self-nesting rejected on create and on update;
a create rejected when an existing nested role already contains the new role;
adding an ancestor as a nested role rejected;
an update with unchanged nested roles doing no extra lookups;
an existing cycle above the role terminating and the update succeeding.
TestRoleREST:
GET /roles/{id} on a cyclic role returning a denial, with each role loaded at most once;
getUserRoles on a cyclic role terminating;
a missing nested role skipped on read.
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5850: Validate nested role membership for cycles and avoid rec… [ranger]
Posted by "mneethiraj (via GitHub)" <gi...@apache.org>.
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5850: Validate nested role membership for cycles and avoid rec… [ranger]
Posted by "mneethiraj (via GitHub)" <gi...@apache.org>.
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org