This is an automated email from the ASF dual-hosted git repository.
vyommani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/master by this push:
new 8ef86d1c3 RANGER-5850: Validate nested role membership for cycles and
avoid rec… (#1324)
8ef86d1c3 is described below
commit 8ef86d1c37e2118071d1c507a61cee295b8ed8e4
Author: Vyom Mani Tiwari <[email protected]>
AuthorDate: Mon Oct 26 16:41:19 2026 +0530
RANGER-5850: Validate nested role membership for cycles and avoid rec…
(#1324)
* RANGER-5850: Validate nested role membership for cycles and avoid
recursive role expansion in RoleREST
* RANGER-5850: addressed the review comments
* RANGER-5850: addressed the review comments
---
.../java/org/apache/ranger/biz/RoleDBStore.java | 104 +++++++++++++++++
.../main/java/org/apache/ranger/db/XXRoleDao.java | 32 ++++++
.../main/java/org/apache/ranger/rest/RoleREST.java | 108 +++++++++++++-----
.../main/resources/META-INF/jpa_named_queries.xml | 4 +
.../org/apache/ranger/biz/TestRoleDBStore.java | 126 +++++++++++++++++++++
.../java/org/apache/ranger/rest/TestRoleREST.java | 61 ++++++++++
6 files changed, 407 insertions(+), 28 deletions(-)
diff --git
a/security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java
b/security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java
index 4d06bfe67..ec9d0526b 100644
--- a/security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java
@@ -29,6 +29,7 @@
import org.apache.ranger.common.UserSessionBase;
import org.apache.ranger.common.db.RangerTransactionSynchronizationAdapter;
import org.apache.ranger.db.RangerDaoManager;
+import org.apache.ranger.db.XXRoleDao;
import org.apache.ranger.entity.XXRole;
import org.apache.ranger.entity.XXRoleRefGroup;
import org.apache.ranger.entity.XXRoleRefUser;
@@ -51,9 +52,12 @@
import org.springframework.stereotype.Component;
import javax.annotation.PostConstruct;
+import javax.ws.rs.WebApplicationException;
+import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.Deque;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -113,6 +117,8 @@ public RangerRole createRole(RangerRole role, Boolean
createNonExistUserGroupRol
throw restErrorUtil.createRESTException("role with name: " +
role.getName() + " already exists", MessageEnums.ERROR_DUPLICATE_OBJECT);
}
+ ensureRoleNotNestedInItself(role);
+
Runnable roleVersionUpdater = new RoleVersionUpdater(daoMgr);
transactionSynchronizationAdapter.executeOnTransactionCommit(roleVersionUpdater);
@@ -149,6 +155,8 @@ public RangerRole updateRole(RangerRole role, Boolean
createNonExistUserGroupRol
oldRole = JsonUtils.jsonToObject(xxRole.getRoleText(),
RangerRole.class);
}
+ ensureNoRoleNestingCycle(role, xxRole, oldRole);
+
Runnable roleVersionUpdater = new RoleVersionUpdater(daoMgr);
transactionSynchronizationAdapter.executeOnTransactionCommit(roleVersionUpdater);
@@ -454,6 +462,102 @@ 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) {
+ XXRoleDao roleDao = daoMgr.getXXRole();
+ 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 memberRoleName = pending.poll();
+
+ if (StringUtils.equals(memberRoleName, role.getName())) {
+ throw createNestingCycleException(role.getName());
+ }
+
+ if (visited.add(memberRoleName)) {
+ pending.addAll(roleDao.getRoleMemberNames(memberRoleName)); //
empty if the role will be created empty
+ }
+ }
+ }
+
+ /**
+ * Rejects an update that would make the role contain itself, directly or
through nested roles.
+ *
+ * Only nested roles added by this update are checked: an edge role -> N
closes a cycle iff N is the role
+ * itself or one of its ancestors. Ancestors are found by walking
x_role_ref_role upwards by id (the same walk
+ * updatePolicyVersions() already performs on every update), so no role
JSON is loaded. Updates that do not add
+ * a nested role (e.g. user/group membership changes) do no extra DB work.
+ */
+ private void ensureNoRoleNestingCycle(RangerRole role, XXRole xxRole,
RangerRole oldRole) {
+ Set<String> addedRoleNames = new HashSet<>();
+
+ for (RangerRole.RoleMember member : role.getRoles()) {
+ if (member != null && StringUtils.isNotEmpty(member.getName())) {
+ addedRoleNames.add(member.getName());
+ }
+ }
+
+ if (oldRole != null) {
+ for (RangerRole.RoleMember member : oldRole.getRoles()) {
+ if (member != null) {
+ addedRoleNames.remove(member.getName());
+ }
+ }
+ }
+
+ if (addedRoleNames.isEmpty()) {
+ return;
+ }
+
+ if (addedRoleNames.contains(role.getName()) ||
addedRoleNames.contains(xxRole.getName())) {
+ throw createNestingCycleException(role.getName());
+ }
+
+ // roles that don't exist yet are created empty, so they can't close a
cycle
+ Set<Long> addedRoleIds = new
HashSet<>(daoMgr.getXXRole().getIdsByRoleNames(addedRoleNames).values());
+
+ if (addedRoleIds.isEmpty()) {
+ return;
+ }
+
+ Set<Long> visited = new HashSet<>();
+ Deque<Long> pending = new ArrayDeque<>();
+
+ pending.add(xxRole.getId());
+
+ while (!pending.isEmpty()) {
+ Long roleId = pending.poll();
+
+ if (addedRoleIds.contains(roleId)) {
+ throw createNestingCycleException(role.getName());
+ }
+
+ if (visited.add(roleId)) {
+
pending.addAll(daoMgr.getXXRoleRefRole().getContainingRoles(roleId));
+ }
+ }
+ }
+
+ private WebApplicationException createNestingCycleException(String
roleName) {
+ LOG.error("Role {} can not be saved: nested roles would make it
contain itself", roleName);
+
+ return restErrorUtil.createRESTException("Invalid role members for
role " + roleName + ": a role can not contain itself, directly or through
nested roles", MessageEnums.INVALID_INPUT_DATA);
+ }
+
private void ensureRoleDeleteAllowed(String roleName) throws Exception {
boolean roleNotInPolicy = ensureRoleNotInPolicy(roleName);
diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXRoleDao.java
b/security-admin/src/main/java/org/apache/ranger/db/XXRoleDao.java
index 78372ee2c..bb6161427 100644
--- a/security-admin/src/main/java/org/apache/ranger/db/XXRoleDao.java
+++ b/security-admin/src/main/java/org/apache/ranger/db/XXRoleDao.java
@@ -19,8 +19,10 @@
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
+import org.apache.ranger.authorization.utils.JsonUtils;
import org.apache.ranger.common.db.BaseDao;
import org.apache.ranger.entity.XXRole;
+import org.apache.ranger.plugin.model.RangerRole;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Service;
@@ -146,4 +148,34 @@ public Map<String, Long>
getIdsByRoleNames(Collection<String> roleNames) {
return Collections.emptyMap();
}
+
+ /**
+ * Returns names of the roles listed as members of the given role, read
from the role's stored definition
+ * (x_role.role_text) with a single query. The stored definition is used
rather than x_role_ref_role because a
+ * reference to a role created with createNonExistUserGroupRole is written
to the ref table only after commit.
+ *
+ * @return member role names; empty if the role does not exist or has no
member roles
+ */
+ public List<String> getRoleMemberNames(String roleName) {
+ List<String> ret = new ArrayList<>();
+
+ if (StringUtils.isNotEmpty(roleName)) {
+ try {
+ String roleText =
getEntityManager().createNamedQuery("XXRole.getRoleTextByName",
String.class).setParameter("roleName", roleName).getSingleResult();
+ RangerRole role = JsonUtils.jsonToObject(roleText,
RangerRole.class);
+
+ if (role != null) {
+ for (RangerRole.RoleMember member : role.getRoles()) {
+ if (member != null &&
StringUtils.isNotEmpty(member.getName())) {
+ ret.add(member.getName());
+ }
+ }
+ }
+ } catch (NoResultException e) {
+ logger.debug("getRoleMemberNames({}): role not found",
roleName);
+ }
+ }
+
+ return ret;
+ }
}
diff --git a/security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java
b/security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java
index 5df0c7ee5..ba0147b34 100644
--- a/security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java
+++ b/security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java
@@ -93,12 +93,16 @@
import java.io.IOException;
import java.io.InputStream;
+import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
+import java.util.Deque;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
import java.util.Set;
@Path("roles")
@@ -1007,15 +1011,16 @@ public List<String> getUserRoles(@PathParam("user")
String userName, @Context Ht
Set<RangerRole> roleList = roleStore.getRoleNames(userName,
userMgr.getGroupsForUser(userName));
+ // one visited set across all of the user's roles: a nested role
shared by several of them is loaded once
+ Set<String> nestedRoleNames = new HashSet<>();
+
for (RangerRole role : roleList) {
ret.add(role.getName());
- Set<String> roleMembers = new HashSet<>();
-
- getRoleMemberNames(roleMembers, role);
-
- ret.addAll(roleMembers);
+ getRoleMemberNames(nestedRoleNames, role);
}
+
+ ret.addAll(nestedRoleNames);
} catch (WebApplicationException excp) {
throw excp;
} catch (Throwable excp) {
@@ -1407,15 +1412,18 @@ private boolean ensureRoleAccess(String username,
Set<String> userGroups, Ranger
}
Set<RangerRole.RoleMember> roleMemberList = new HashSet<>();
-
- getRoleMembers(roleMemberList, role);
+ Map<String, RangerRole> loadedRoles =
getRoleMembers(roleMemberList, role);
for (RangerRole.RoleMember roleMember : roleMemberList) {
if (!roleMember.getIsAdmin()) {
continue;
}
- RangerRole roleMemberObj = roleStore.getRole(roleMember.getName());
+ RangerRole roleMemberObj = loadedRoles.get(roleMember.getName());
// already loaded while expanding
+
+ if (roleMemberObj == null) { // nested role doesn't exist (yet)
+ continue;
+ }
if (getUserNames(roleMemberObj).contains(username)) {
isAccessible = true;
@@ -1447,19 +1455,7 @@ private RangerRole addUsersGroupsAndRoles(RangerRole
role, Set<String> users, Se
}
try {
- // Check for any cycling relationship between roles
- for (String newRole : roles) {
- //get members recursively and check if the grantor role is
already a member
- Set<String> roleMembers = new HashSet<>();
-
- getRoleMemberNames(roleMembers, roleStore.getRole(newRole));
-
- LOG.debug("Role members for {} = {}", newRole, roleMembers);
-
- if (roleMembers.contains(role.getName())) {
- throw new Exception("Invalid role grant");
- }
- }
+ // nesting cycles, including granting a role into itself, are
rejected by RoleDBStore.updateRole()
Set<RangerRole.RoleMember> roleUsers = new HashSet<>();
Set<RangerRole.RoleMember> roleGroups = new HashSet<>();
@@ -1678,19 +1674,75 @@ private Set<String> getRoleNames(RangerRole role) {
return rolenames;
}
+ /*
+ * Adds names of all roles nested, directly or transitively, under the
given role. Iterative with a visited
+ * set: each role is loaded at most once and a cyclic role graph
terminates instead of overflowing the stack.
+ */
private void getRoleMemberNames(Set<String> roleMembers, RangerRole role)
throws Exception {
- for (RangerRole.RoleMember roleMember : role.getRoles()) {
- roleMembers.add(roleMember.getName());
+ Deque<RangerRole> pending = new ArrayDeque<>();
+
+ pending.add(role);
+
+ while (!pending.isEmpty()) {
+ RangerRole current = pending.poll();
+
+ for (RangerRole.RoleMember roleMember : current.getRoles()) {
+ if (roleMembers.add(roleMember.getName())) {
+ RangerRole memberRole =
getRoleIfExists(roleMember.getName());
+
+ if (memberRole != null) {
+ pending.add(memberRole);
+ }
+ }
+ }
+ }
+ }
+
+ /*
+ * Adds all role members nested, directly or transitively, under the given
role, and returns the nested roles
+ * loaded along the way (by name). Iterative: each nested role is loaded
at most once, so a cyclic role graph
+ * terminates instead of overflowing the stack.
+ */
+ private Map<String, RangerRole> getRoleMembers(Set<RangerRole.RoleMember>
roleMembers, RangerRole role) throws Exception {
+ Map<String, RangerRole> loaded = new HashMap<>();
+ Deque<RangerRole> pending = new ArrayDeque<>();
+
+ pending.add(role);
+
+ while (!pending.isEmpty()) {
+ RangerRole current = pending.poll();
+
+ for (RangerRole.RoleMember roleMember : current.getRoles()) {
+ roleMembers.add(roleMember);
+
+ if (!loaded.containsKey(roleMember.getName())) {
+ RangerRole memberRole =
getRoleIfExists(roleMember.getName());
- getRoleMemberNames(roleMembers,
roleStore.getRole(roleMember.getName()));
+ loaded.put(roleMember.getName(), memberRole);
+
+ if (memberRole != null) {
+ pending.add(memberRole);
+ }
+ }
+ }
}
+
+ return loaded;
}
- private void getRoleMembers(Set<RangerRole.RoleMember> roleMembers,
RangerRole role) throws Exception {
- for (RangerRole.RoleMember roleMember : role.getRoles()) {
- roleMembers.add(roleMember);
+ /*
+ * A nested role named in a stored role definition may not exist yet, e.g.
when it is created after commit by
+ * createNonExistUserGroupRole. Skip it instead of failing the whole read.
+ * RoleDBStore.getRole(String) throws WebApplicationException only when
the role is not found (by name, or by id
+ * in roleService.read() if deleted in between); DB failures surface as
persistence exceptions and still propagate.
+ */
+ private RangerRole getRoleIfExists(String roleName) {
+ try {
+ return roleStore.getRole(roleName);
+ } catch (WebApplicationException excp) {
+ LOG.warn("Role {} does not exist; skipping it", roleName);
- getRoleMembers(roleMembers,
roleStore.getRole(roleMember.getName()));
+ return null;
}
}
diff --git a/security-admin/src/main/resources/META-INF/jpa_named_queries.xml
b/security-admin/src/main/resources/META-INF/jpa_named_queries.xml
index 1dcffe9ae..817868964 100755
--- a/security-admin/src/main/resources/META-INF/jpa_named_queries.xml
+++ b/security-admin/src/main/resources/META-INF/jpa_named_queries.xml
@@ -2023,6 +2023,10 @@
<query>SELECT obj.id, obj.name FROM XXRole obj WHERE obj.name IN
:roleNames</query>
</named-query>
+ <named-query name="XXRole.getRoleTextByName">
+ <query>SELECT obj.roleText FROM XXRole obj WHERE obj.name =
:roleName</query>
+ </named-query>
+
<!-- XXRoleRef* -->
<named-query name="XXRoleRefUser.findByRoleId">
diff --git
a/security-admin/src/test/java/org/apache/ranger/biz/TestRoleDBStore.java
b/security-admin/src/test/java/org/apache/ranger/biz/TestRoleDBStore.java
index 9548c14f5..f085ecc0c 100644
--- a/security-admin/src/test/java/org/apache/ranger/biz/TestRoleDBStore.java
+++ b/security-admin/src/test/java/org/apache/ranger/biz/TestRoleDBStore.java
@@ -18,6 +18,7 @@
package org.apache.ranger.biz;
import org.apache.ranger.authorization.hadoop.config.RangerAdminConfig;
+import org.apache.ranger.authorization.utils.JsonUtils;
import org.apache.ranger.common.ContextUtil;
import org.apache.ranger.common.RESTErrorUtil;
import org.apache.ranger.common.RangerConstants;
@@ -592,6 +593,131 @@ public void
testInitStore_setsPredicateUtilAndPopulateFlag() {
}
}
+ @Test
+ public void testCreateRole_selfNestingRejected() throws Exception {
+ RangerRole rangerRole = getRangerRole();
+ XXRoleDao xxRoleDao = Mockito.mock(XXRoleDao.class);
+
+ rangerRole.setRoles(Collections.singletonList(new RoleMember(roleName,
false)));
+
+ Mockito.when(daoMgr.getXXRole()).thenReturn(xxRoleDao);
+ Mockito.when(xxRoleDao.findByRoleName(roleName)).thenReturn(null);
+ Mockito.when(restErrorUtil.createRESTException(Mockito.anyString(),
Mockito.any())).thenReturn(new WebApplicationException());
+
+ Assertions.assertThrows(WebApplicationException.class, () ->
roleDBStore.createRole(rangerRole, false, false));
+
+ Mockito.verify(roleService, Mockito.never()).create(Mockito.any());
+ }
+
+ @Test
+ public void testCreateRole_nestedRoleAlreadyContainingNewRoleRejected()
throws Exception {
+ // roleB (stored, e.g. earlier in the same import) lists test-role,
whose ref row is still pending;
+ // creating test-role containing roleB must be rejected from the
stored definitions
+ RangerRole rangerRole = getRangerRole();
+ XXRoleDao xxRoleDao = Mockito.mock(XXRoleDao.class);
+
+ rangerRole.setRoles(Collections.singletonList(new RoleMember("roleB",
false)));
+
+ Mockito.when(daoMgr.getXXRole()).thenReturn(xxRoleDao);
+ Mockito.when(xxRoleDao.findByRoleName(roleName)).thenReturn(null);
+
Mockito.when(xxRoleDao.getRoleMemberNames("roleB")).thenReturn(Collections.singletonList(roleName));
+ Mockito.when(restErrorUtil.createRESTException(Mockito.anyString(),
Mockito.any())).thenReturn(new WebApplicationException());
+
+ Assertions.assertThrows(WebApplicationException.class, () ->
roleDBStore.createRole(rangerRole, false, false));
+
+ Mockito.verify(roleService, Mockito.never()).create(Mockito.any());
+ }
+
+ @Test
+ public void testUpdateRole_selfNestingRejectedWithoutGraphWalk() throws
Exception {
+ RangerRole rangerRole = getRangerRole();
+ XXRoleDao xxRoleDao = Mockito.mock(XXRoleDao.class);
+
+ rangerRole.setRoles(Collections.singletonList(new RoleMember(roleName,
false)));
+
+ Mockito.when(daoMgr.getXXRole()).thenReturn(xxRoleDao);
+
Mockito.when(xxRoleDao.findByRoleId(rangerRole.getId())).thenReturn(getTestRole());
+ Mockito.when(restErrorUtil.createRESTException(Mockito.anyString(),
Mockito.any())).thenReturn(new WebApplicationException());
+
+ Assertions.assertThrows(WebApplicationException.class, () ->
roleDBStore.updateRole(rangerRole, false, true));
+
+ Mockito.verify(xxRoleDao,
Mockito.never()).getIdsByRoleNames(Mockito.any());
+ Mockito.verify(roleService, Mockito.never()).update(Mockito.any());
+ }
+
+ @Test
+ public void testUpdateRole_addingAncestorAsNestedRoleRejected() throws
Exception {
+ // stored: roleB (id 20) contains test-role (id 9). Update adds roleB
into test-role -> cycle.
+ RangerRole rangerRole = getRangerRole();
+ XXRoleDao xxRoleDao = Mockito.mock(XXRoleDao.class);
+ XXRoleRefRoleDao xxRoleRefRoleDao =
Mockito.mock(XXRoleRefRoleDao.class);
+
+ rangerRole.setRoles(Collections.singletonList(new RoleMember("roleB",
false)));
+
+ Mockito.when(daoMgr.getXXRole()).thenReturn(xxRoleDao);
+ Mockito.when(daoMgr.getXXRoleRefRole()).thenReturn(xxRoleRefRoleDao);
+
Mockito.when(xxRoleDao.findByRoleId(rangerRole.getId())).thenReturn(getTestRole());
+
Mockito.when(xxRoleDao.getIdsByRoleNames(Collections.singleton("roleB"))).thenReturn(Collections.singletonMap("roleB",
20L));
+
Mockito.when(xxRoleRefRoleDao.getContainingRoles(roleId)).thenReturn(Collections.singleton(20L));
+ Mockito.when(restErrorUtil.createRESTException(Mockito.anyString(),
Mockito.any())).thenReturn(new WebApplicationException());
+
+ Assertions.assertThrows(WebApplicationException.class, () ->
roleDBStore.updateRole(rangerRole, false, true));
+
+ Mockito.verify(roleService, Mockito.never()).update(Mockito.any());
+ }
+
+ @Test
+ public void testUpdateRole_unchangedNestedRolesSkipCycleCheck() throws
Exception {
+ // user/group-only updates must not pay for a graph walk
+ RangerRole rangerRole = getRangerRole();
+ XXRole xxRole = getTestRole();
+ XXRoleDao xxRoleDao = Mockito.mock(XXRoleDao.class);
+
+ rangerRole.setRoles(Collections.singletonList(new RoleMember("roleB",
false)));
+ xxRole.setRoleText(JsonUtils.objectToJson(rangerRole));
+
+ Mockito.when(daoMgr.getXXRole()).thenReturn(xxRoleDao);
+
Mockito.when(xxRoleDao.findByRoleId(rangerRole.getId())).thenReturn(xxRole);
+
Mockito.doNothing().when(transactionSynchronizationAdapter).executeOnTransactionCommit(Mockito.any());
+ Mockito.when(roleService.update(rangerRole)).thenReturn(rangerRole);
+
Mockito.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(),
Mockito.anyBoolean(), Mockito.anyBoolean());
+
Mockito.doNothing().when(roleService).updatePolicyVersions(rangerRole.getId());
+
Mockito.doNothing().when(roleService).createTransactionLog(Mockito.any(),
Mockito.any(), Mockito.anyInt());
+
+ roleDBStore.updateRole(rangerRole, false, true);
+
+ Mockito.verify(xxRoleDao,
Mockito.never()).getIdsByRoleNames(Mockito.any());
+ Mockito.verify(daoMgr, Mockito.never()).getXXRoleRefRole();
+ }
+
+ @Test
+ public void testUpdateRole_preexistingAncestorCycleTerminatesAndSucceeds()
throws Exception {
+ // ancestors: 9 -> {30}, 30 -> {31}, 31 -> {30} (existing cycle above
test-role); added roleC (id 20) is unrelated
+ RangerRole rangerRole = getRangerRole();
+ XXRoleDao xxRoleDao = Mockito.mock(XXRoleDao.class);
+ XXRoleRefRoleDao xxRoleRefRoleDao =
Mockito.mock(XXRoleRefRoleDao.class);
+
+ rangerRole.setRoles(Collections.singletonList(new RoleMember("roleC",
false)));
+
+ Mockito.when(daoMgr.getXXRole()).thenReturn(xxRoleDao);
+ Mockito.when(daoMgr.getXXRoleRefRole()).thenReturn(xxRoleRefRoleDao);
+
Mockito.when(xxRoleDao.findByRoleId(rangerRole.getId())).thenReturn(getTestRole());
+
Mockito.when(xxRoleDao.getIdsByRoleNames(Collections.singleton("roleC"))).thenReturn(Collections.singletonMap("roleC",
20L));
+
Mockito.when(xxRoleRefRoleDao.getContainingRoles(roleId)).thenReturn(Collections.singleton(30L));
+
Mockito.when(xxRoleRefRoleDao.getContainingRoles(30L)).thenReturn(Collections.singleton(31L));
+
Mockito.when(xxRoleRefRoleDao.getContainingRoles(31L)).thenReturn(Collections.singleton(30L));
+
Mockito.doNothing().when(transactionSynchronizationAdapter).executeOnTransactionCommit(Mockito.any());
+ Mockito.when(roleService.update(rangerRole)).thenReturn(rangerRole);
+
Mockito.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(),
Mockito.anyBoolean(), Mockito.anyBoolean());
+
Mockito.doNothing().when(roleService).updatePolicyVersions(rangerRole.getId());
+
Mockito.doNothing().when(roleService).createTransactionLog(Mockito.any(),
Mockito.any(), Mockito.anyInt());
+
+ Assertions.assertEquals(rangerRole, roleDBStore.updateRole(rangerRole,
false, true));
+
+ Mockito.verify(xxRoleRefRoleDao,
Mockito.times(1)).getContainingRoles(30L);
+ Mockito.verify(xxRoleRefRoleDao,
Mockito.times(1)).getContainingRoles(31L);
+ }
+
private XXRole getTestRole() {
return new XXRole() {{
setId(TestRoleDBStore.roleId);
diff --git
a/security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java
b/security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java
index 39790f7fb..3012abf29 100644
--- a/security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java
+++ b/security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java
@@ -1623,6 +1623,67 @@ public void test22deleteRoleWithValidationError() {
});
}
+ @Test
+ public void test36GetRoleByIdCyclicRoleDeniedWithoutRecursion() throws
Exception {
+ // roleA <-> roleB cycle; a non-member, non-admin caller must get a
clean denial, not a StackOverflowError
+ RangerRole roleA = createNestingRole("roleA", "roleB", false);
+ RangerRole roleB = createNestingRole("roleB", "roleA", false);
+
+ roleA.setId(roleId);
+
+
Mockito.when(bizUtil.isUserRangerAdmin(Mockito.anyString())).thenReturn(false);
+ Mockito.when(roleStore.getRole(roleId)).thenReturn(roleA);
+ Mockito.when(roleStore.getRole("roleA")).thenReturn(roleA);
+ Mockito.when(roleStore.getRole("roleB")).thenReturn(roleB);
+
Mockito.when(restErrorUtil.createRESTException(Mockito.anyString())).thenReturn(new
WebApplicationException());
+
+ Assertions.assertThrows(WebApplicationException.class, () ->
roleRest.getRole(roleId));
+
+ // each nested role is loaded at most once
+ Mockito.verify(roleStore, Mockito.atMost(1)).getRole("roleA");
+ Mockito.verify(roleStore, Mockito.atMost(1)).getRole("roleB");
+ }
+
+ @Test
+ public void test37GetUserRolesCyclicRoleTerminates() throws Exception {
+ RangerRole roleA = createNestingRole("roleA", "roleB", false);
+ RangerRole roleB = createNestingRole("roleB", "roleA", false);
+
+
Mockito.when(xUserService.getXUserByUserName(Mockito.anyString())).thenReturn(createVXUser());
+ Mockito.when(roleStore.getRoleNames(Mockito.anyString(),
Mockito.any())).thenReturn(new HashSet<>(Collections.singletonList(roleA)));
+ Mockito.when(roleStore.getRole("roleA")).thenReturn(roleA);
+ Mockito.when(roleStore.getRole("roleB")).thenReturn(roleB);
+
+ List<String> returnedRoles = roleRest.getUserRoles(adminLoginID,
Mockito.mock(HttpServletRequest.class));
+
+ Assertions.assertEquals(new HashSet<>(Arrays.asList("roleA",
"roleB")), new HashSet<>(returnedRoles));
+ Mockito.verify(roleStore, Mockito.times(1)).getRole("roleA");
+ Mockito.verify(roleStore, Mockito.times(1)).getRole("roleB");
+ }
+
+ @Test
+ public void test38MissingNestedRoleSkippedOnRead() throws Exception {
+ // roleA lists "pending" (not created yet) with admin option and
roleB; reads must skip it, not fail
+ List<RangerRole.RoleMember> members = new
ArrayList<>(Arrays.asList(new RangerRole.RoleMember("pending", true), new
RangerRole.RoleMember("roleB", true)));
+ RangerRole roleA = new RangerRole("roleA", "roleA",
null, new ArrayList<>(), new ArrayList<>(), members);
+ RangerRole roleB = createRangerRole(adminLoginID,
false);
+
+ roleA.setId(roleId);
+
+
Mockito.when(bizUtil.isUserRangerAdmin(Mockito.anyString())).thenReturn(false);
+ Mockito.when(roleStore.getRole(roleId)).thenReturn(roleA);
+ Mockito.when(roleStore.getRole("pending")).thenThrow(new
WebApplicationException());
+ Mockito.when(roleStore.getRole("roleB")).thenReturn(roleB);
+
+ Assertions.assertEquals(roleA, roleRest.getRole(roleId));
+ }
+
+ private RangerRole createNestingRole(String name, String nestedRoleName,
boolean isAdmin) {
+ List<RangerRole.RoleMember> roles = new
ArrayList<>(Collections.singletonList(new RangerRole.RoleMember(nestedRoleName,
isAdmin)));
+
+ return new RangerRole(name, name, null, new ArrayList<>(), new
ArrayList<>(), roles);
+ }
+
private RangerRole createRole() {
String name = "test-role";
String name2 = "admin";
