This is an automated email from the ASF dual-hosted git repository.
dineshkumar-yadav pushed a commit to branch ranger-2.10
in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/ranger-2.10 by this push:
new 162e2d92a Revert "RANGER-5730: Restrict role import createNonExist
flag to roles only (#1166)" (#1275)
162e2d92a is described below
commit 162e2d92a0990dd6cc060b5492931498bf0e32ea
Author: Dineshkumar Yadav <[email protected]>
AuthorDate: Sat Oct 3 16:17:04 2026 +0530
Revert "RANGER-5730: Restrict role import createNonExist flag to roles only
(#1166)" (#1275)
This reverts commit 995a45e8cf0fe3cacc93dffeb551c901a1a0fd46.
Co-authored-by: Dineshkumar Yadav <[email protected]>
---
.../org/apache/ranger/plugin/store/RoleStore.java | 19 ---------
.../java/org/apache/ranger/biz/RoleDBStore.java | 14 +------
.../java/org/apache/ranger/biz/RoleRefUpdater.java | 26 +++---------
.../main/java/org/apache/ranger/rest/RoleREST.java | 10 +----
.../org/apache/ranger/biz/TestRoleDBStore.java | 4 +-
.../org/apache/ranger/biz/TestRoleRefUpdater.java | 49 ----------------------
.../java/org/apache/ranger/rest/TestRoleREST.java | 11 ++---
7 files changed, 15 insertions(+), 118 deletions(-)
diff --git
a/agents-common/src/main/java/org/apache/ranger/plugin/store/RoleStore.java
b/agents-common/src/main/java/org/apache/ranger/plugin/store/RoleStore.java
index 719f762a0..0b100c3aa 100644
--- a/agents-common/src/main/java/org/apache/ranger/plugin/store/RoleStore.java
+++ b/agents-common/src/main/java/org/apache/ranger/plugin/store/RoleStore.java
@@ -41,25 +41,6 @@ default RangerRole updateRole(RangerRole role, Boolean
createNonExistUserGroup,
return updateRole(role, createNonExistUserGroup);
}
- /**
- * Create role with separate controls for creating missing users/groups vs
nested roles.
- * Default implementation combines the flags for stores that do not
support the split.
- */
- default RangerRole createRole(RangerRole role, Boolean
createNonExistUserGroup, Boolean createNonExistRole, Boolean
isRefTableCleanupRequired) throws Exception {
- boolean combined = Boolean.TRUE.equals(createNonExistUserGroup) ||
Boolean.TRUE.equals(createNonExistRole);
-
- return createRole(role, combined, isRefTableCleanupRequired);
- }
-
- /**
- * Update role with separate controls for creating missing users/groups vs
nested roles.
- * Default implementation combines the flags for stores that do not
support the split.
- */
- default RangerRole updateRole(RangerRole role, Boolean
createNonExistUserGroup, Boolean createNonExistRole, Boolean
isRefTableCleanupRequired) throws Exception {
- boolean combined = Boolean.TRUE.equals(createNonExistUserGroup) ||
Boolean.TRUE.equals(createNonExistRole);
-
- return updateRole(role, combined, isRefTableCleanupRequired);
- }
void deleteRole(String roleName) throws Exception;
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 a3b8b6cf2..4e98b5d2a 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
@@ -118,11 +118,6 @@ public RangerRole updateRole(RangerRole role, Boolean
createNonExistUserGroupRol
@Override
public RangerRole createRole(RangerRole role, Boolean
createNonExistUserGroupRole, Boolean isRefTableCleanupRequired) throws
Exception {
- return createRole(role, createNonExistUserGroupRole,
createNonExistUserGroupRole, isRefTableCleanupRequired);
- }
-
- @Override
- public RangerRole createRole(RangerRole role, Boolean
createNonExistUserGroup, Boolean createNonExistRole, Boolean
isRefTableCleanupRequired) throws Exception {
if (LOG.isDebugEnabled()) {
LOG.debug("==> RoleDBStore.createRole()");
}
@@ -142,7 +137,7 @@ public RangerRole createRole(RangerRole role, Boolean
createNonExistUserGroup, B
throw new Exception("Cannot create role:[" + role + "]");
}
- roleRefUpdater.createNewRoleMappingForRefTable(createdRole,
createNonExistUserGroup, createNonExistRole, isRefTableCleanupRequired);
+ roleRefUpdater.createNewRoleMappingForRefTable(createdRole,
createNonExistUserGroupRole, isRefTableCleanupRequired);
roleService.createTransactionLog(createdRole, null,
RangerBaseModelService.OPERATION_CREATE_CONTEXT);
return createdRole;
@@ -150,11 +145,6 @@ public RangerRole createRole(RangerRole role, Boolean
createNonExistUserGroup, B
@Override
public RangerRole updateRole(RangerRole role, Boolean
createNonExistUserGroupRole, Boolean isRefTableCleanupRequired) throws
Exception {
- return updateRole(role, createNonExistUserGroupRole,
createNonExistUserGroupRole, isRefTableCleanupRequired);
- }
-
- @Override
- public RangerRole updateRole(RangerRole role, Boolean
createNonExistUserGroup, Boolean createNonExistRole, Boolean
isRefTableCleanupRequired) throws Exception {
XXRole xxRole = daoMgr.getXXRole().findByRoleId(role.getId());
if (xxRole == null) {
throw restErrorUtil.createRESTException("role with id: " +
role.getId() + " does not exist");
@@ -177,7 +167,7 @@ public RangerRole updateRole(RangerRole role, Boolean
createNonExistUserGroup, B
throw new Exception("Cannot update role:[" + role + "]");
}
- roleRefUpdater.createNewRoleMappingForRefTable(updatedRole,
createNonExistUserGroup, createNonExistRole, isRefTableCleanupRequired);
+ roleRefUpdater.createNewRoleMappingForRefTable(updatedRole,
createNonExistUserGroupRole, isRefTableCleanupRequired);
roleService.updatePolicyVersions(updatedRole.getId());
diff --git
a/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java
b/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java
index 2bb3f2ecf..82fa68f14 100644
--- a/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java
@@ -84,21 +84,7 @@ public RangerDaoManager getRangerDaoManager() {
return daoMgr;
}
- /**
- * Creates role-user/group/role ref mappings.
- * When {@code createNonExistUserGroupRole} is true, missing users,
groups, and nested roles may be created
- * (existing create/update role API behavior).
- */
public void createNewRoleMappingForRefTable(RangerRole rangerRole, Boolean
createNonExistUserGroupRole, Boolean isRefTableCleanupRequired) {
- createNewRoleMappingForRefTable(rangerRole,
createNonExistUserGroupRole, createNonExistUserGroupRole,
isRefTableCleanupRequired);
- }
-
- /**
- * Creates role-user/group/role ref mappings with separate controls for
creating missing users/groups vs nested roles.
- * Role import uses createNonExistUserGroup=false and
createNonExistRole=true so nested roles can be created
- * without forcing creation of missing users or groups.
- */
- public void createNewRoleMappingForRefTable(RangerRole rangerRole, Boolean
createNonExistUserGroup, Boolean createNonExistRole, Boolean
isRefTableCleanupRequired) {
if (rangerRole == null) {
return;
}
@@ -123,13 +109,11 @@ public void createNewRoleMappingForRefTable(RangerRole
rangerRole, Boolean creat
roleRoles.add(role.getName());
}
- if (Boolean.TRUE.equals(isRefTableCleanupRequired)) {
+ if (isRefTableCleanupRequired) {
cleanupRefTablesForUpdate(rangerRole, roleUsers, roleGroups,
roleRoles);
}
- final boolean adminAccess = xaBizUtil.checkAdminAccess();
- final boolean isCreateNonExistentUGs =
Boolean.TRUE.equals(createNonExistUserGroup) && adminAccess;
- final boolean isCreateNonExistentRoles =
Boolean.TRUE.equals(createNonExistRole) && adminAccess;
+ final boolean isCreateNonExistentUGRs = createNonExistUserGroupRole &&
xaBizUtil.checkAdminAccess();
if (CollectionUtils.isNotEmpty(roleUsers)) {
LOG.debug("New user entries to be inserted into x_role_ref_user
for role ID {}: {}", roleId, roleUsers);
@@ -151,7 +135,7 @@ public void createNewRoleMappingForRefTable(RangerRole
rangerRole, Boolean creat
if (userRef != null) {
xxRoleRefUsers.add(userRef);
}
- } else if (isCreateNonExistentUGs) {
+ } else if (isCreateNonExistentUGRs) {
rangerTransactionSynchronizationAdapter.executeOnTransactionCommit(associator);
} else {
throw restErrorUtil.createRESTException("user with name: "
+ userName + " does not exist ", MessageEnums.INVALID_INPUT_DATA);
@@ -181,7 +165,7 @@ public void createNewRoleMappingForRefTable(RangerRole
rangerRole, Boolean creat
if (groupRef != null) {
xxRoleRefGroups.add(groupRef);
}
- } else if (isCreateNonExistentUGs) {
+ } else if (isCreateNonExistentUGRs) {
rangerTransactionSynchronizationAdapter.executeOnTransactionCommit(associator);
} else {
throw restErrorUtil.createRESTException("Group with name:
" + groupName + " does not exist ", MessageEnums.INVALID_INPUT_DATA);
@@ -211,7 +195,7 @@ public void createNewRoleMappingForRefTable(RangerRole
rangerRole, Boolean creat
if (roleRef != null) {
xxRoleRefRoles.add(roleRef);
}
- } else if (isCreateNonExistentRoles) {
+ } else if (isCreateNonExistentUGRs) {
rangerTransactionSynchronizationAdapter.executeOnTransactionCommit(associator);
} else {
throw restErrorUtil.createRESTException("Role with name: "
+ subRoleName + " does not exist ", MessageEnums.INVALID_INPUT_DATA);
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 ffe8aa96e..226aa7911 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
@@ -492,12 +492,6 @@ public RESTResponse importRolesFromFile(@Context
HttpServletRequest request,
if (updateIfExists == null) {
updateIfExists = false;
}
-
- // For role import, createNonExistUserGroupRole only
creates missing nested roles.
- // Missing users/groups are never created during import
(RANGER-5730).
- final Boolean createNonExistUserGroup = Boolean.FALSE;
- final Boolean createNonExistRole =
Boolean.TRUE.equals(createNonExistUserGroupRole);
-
List<String> roleNameList = new ArrayList<String>();
roleNameList = getRoleNameList(request, roleNameList);
@@ -543,7 +537,7 @@ public RESTResponse importRolesFromFile(@Context
HttpServletRequest request,
}
}
else {
-
roleStore.updateRole(roleInJson, createNonExistUserGroup,
createNonExistRole, true);
+
roleStore.updateRole(roleInJson, createNonExistUserGroupRole,
true);
totalRoleUpdate++;
}
} catch (WebApplicationException excp) {
@@ -562,7 +556,7 @@ public RESTResponse importRolesFromFile(@Context
HttpServletRequest request,
ret.setStatusCode(RESTResponse.STATUS_SUCCESS);
} else if
(!roleNameList.contains(roleNameInJson) && (!roleNameInJson.isEmpty())) {
try {
-
roleStore.createRole(roleInJson, createNonExistUserGroup, createNonExistRole,
false);
+
roleStore.createRole(roleInJson, createNonExistUserGroupRole, false);
} catch
(WebApplicationException excp) {
throw excp;
} catch
(Throwable excp) {
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 4fea6518a..d0e25e3d1 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
@@ -440,7 +440,7 @@ public void testCreateRole() throws Exception {
Mockito.when(roleService.create(rangerRole)).thenReturn(rangerRole);
Mockito.when(roleService.read(xxRole.getId())).thenReturn(rangerRole);
Mockito.doNothing().when(transactionSynchronizationAdapter).executeOnTransactionCommit(Mockito.any());
-
Mockito.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(),
Mockito.anyBoolean(), Mockito.anyBoolean(), Mockito.anyBoolean());
+
Mockito.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(),
Mockito.anyBoolean(), Mockito.anyBoolean());
Mockito.doNothing().when(roleService).createTransactionLog(
Mockito.any(), Mockito.any(), Mockito.anyInt());
roleDBStore.createRole(rangerRole, true, false);
@@ -469,7 +469,7 @@ public void testUpdateRole() throws Exception {
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.anyBoolean());
+
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());
diff --git
a/security-admin/src/test/java/org/apache/ranger/biz/TestRoleRefUpdater.java
b/security-admin/src/test/java/org/apache/ranger/biz/TestRoleRefUpdater.java
index 89146fae6..800fa1a32 100644
--- a/security-admin/src/test/java/org/apache/ranger/biz/TestRoleRefUpdater.java
+++ b/security-admin/src/test/java/org/apache/ranger/biz/TestRoleRefUpdater.java
@@ -195,55 +195,6 @@ public void
test03_createNewRoleMapping_missingPrincipalThrowsUnlessCreateAllowe
verify(adapter,
times(1)).executeOnTransactionCommit(any(Runnable.class));
}
- @Test
- public void
test05a_createNewRoleMapping_createRoleOnlyDoesNotCreateMissingUser() throws
Exception {
- RoleRefUpdater updater = new RoleRefUpdater();
- RangerDaoManager dao = mock(RangerDaoManager.class);
- XXUserDao xUserDao = mock(XXUserDao.class);
- RESTErrorUtil rest = mock(RESTErrorUtil.class);
- RangerBizUtil biz = mock(RangerBizUtil.class);
-
- setField(updater, RoleRefUpdater.class, "daoMgr", dao);
- setField(updater, RoleRefUpdater.class, "restErrorUtil", rest);
- setField(updater, RoleRefUpdater.class, "xaBizUtil", biz);
-
- when(dao.getXXUser()).thenReturn(xUserDao);
-
when(xUserDao.getIdsByUserNames(anySet())).thenReturn(Collections.emptyMap());
- when(biz.checkAdminAccess()).thenReturn(true);
-
- RuntimeException expected = new RuntimeException("missing user");
- when(rest.createRESTException(anyString(), any())).thenThrow(expected);
-
- // Missing user with createNonExistUserGroup=false,
createNonExistRole=true -> must fail (no user create)
- RangerRole roleWithMissingUser = buildRole(9L,
Collections.singletonList("missingUser"), Collections.emptyList(),
- Collections.emptyList());
- Assertions.assertThrows(RuntimeException.class,
- () ->
updater.createNewRoleMappingForRefTable(roleWithMissingUser, false, true,
false));
- }
-
- @Test
- public void
test05b_createNewRoleMapping_createRoleOnlyCreatesMissingNestedRole() throws
Exception {
- RoleRefUpdater updater = new RoleRefUpdater();
- RangerDaoManager dao = mock(RangerDaoManager.class);
- XXRoleDao xRoleDao = mock(XXRoleDao.class);
- RangerTransactionSynchronizationAdapter adapter =
mock(RangerTransactionSynchronizationAdapter.class);
- RangerBizUtil biz = mock(RangerBizUtil.class);
-
- setField(updater, RoleRefUpdater.class, "daoMgr", dao);
- setField(updater, RoleRefUpdater.class,
"rangerTransactionSynchronizationAdapter", adapter);
- setField(updater, RoleRefUpdater.class, "xaBizUtil", biz);
-
- when(dao.getXXRole()).thenReturn(xRoleDao);
-
when(xRoleDao.getIdsByRoleNames(anySet())).thenReturn(Collections.emptyMap());
- when(biz.checkAdminAccess()).thenReturn(true);
-
- // Missing nested role with createNonExistUserGroup=false,
createNonExistRole=true -> schedule role create only
- RangerRole roleWithMissingNestedRole = buildRole(9L,
Collections.emptyList(), Collections.emptyList(),
- Collections.singletonList("missingRole"));
- updater.createNewRoleMappingForRefTable(roleWithMissingNestedRole,
false, true, false);
- verify(adapter,
times(1)).executeOnTransactionCommit(any(Runnable.class));
- }
-
@Test
public void test04_cleanupRefTablesForUpdate_selectivePrincipalCleanup()
throws Exception {
RoleRefUpdater updater = new RoleRefUpdater();
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 a1a5cc112..4536d5e01 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
@@ -1434,7 +1434,7 @@ public void test20importRolesFromFile() throws Exception {
Mockito.when(searchUtil.getSearchFilter(request,
roleService.sortFields)).thenReturn(filter);
Mockito.when(roleStore.getRoleNames(Mockito.any(SearchFilter.class))).thenReturn(roleList);
-
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(false),
eq(createNonExistUserGroupRole), eq(false)))
+
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class),
eq(createNonExistUserGroupRole), eq(false)))
.thenReturn(rangerRole);
RESTResponse resp = roleRest.importRolesFromFile(request,
uploadedInputStream, fileDetail, updateIfExists,
@@ -1464,7 +1464,7 @@ public void test20bimportRolesFromFile() throws Exception
{
Mockito.when(searchUtil.getSearchFilter(request,
roleService.sortFields)).thenReturn(filter);
Mockito.when(roleStore.getRoleNames(Mockito.any(SearchFilter.class))).thenReturn(roleList);
-
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(false),
eq(createNonExistUserGroupRole), eq(false)))
+
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class),
eq(createNonExistUserGroupRole), eq(false)))
.thenReturn(rangerRole);
RESTResponse resp = roleRest.importRolesFromFile(request,
uploadedInputStream, fileDetail, updateIfExists,
@@ -1472,7 +1472,6 @@ public void test20bimportRolesFromFile() throws Exception
{
Assert.assertNotNull(resp);
Assert.assertEquals(resp.getStatusCode(),
RESTResponse.STATUS_SUCCESS);
Assert.assertEquals(resp.getMsgDesc(), "Total Role Created = 6
, Total Role Unchanged = 1");
- Mockito.verify(roleStore,
Mockito.atLeastOnce()).createRole(Mockito.any(RangerRole.class), eq(false),
eq(true), eq(false));
}
// import role with updateIfExists=true and
createNonExistUserGroupRole=true
@@ -1496,9 +1495,9 @@ public void test20cimportRolesFromFileWithUpdate() throws
Exception {
Mockito.when(searchUtil.getSearchFilter(request,
roleService.sortFields)).thenReturn(filter);
Mockito.when(roleStore.getRoleNames(Mockito.any(SearchFilter.class))).thenReturn(roleList);
Mockito.when(roleStore.getRole(Mockito.anyString())).thenReturn(rangerRole);
-
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(false),
eq(createNonExistUserGroupRole), eq(false)))
+
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class),
eq(createNonExistUserGroupRole), eq(false)))
.thenReturn(rangerRole);
-
Mockito.when(roleStore.updateRole(Mockito.any(RangerRole.class), eq(false),
eq(createNonExistUserGroupRole), eq(true)))
+
Mockito.when(roleStore.updateRole(Mockito.any(RangerRole.class),
eq(createNonExistUserGroupRole), eq(true)))
.thenReturn(rangerRole);
RESTResponse resp = roleRest.importRolesFromFile(request,
uploadedInputStream, fileDetail, updateIfExists,
@@ -1507,8 +1506,6 @@ public void test20cimportRolesFromFileWithUpdate() throws
Exception {
Assert.assertEquals(resp.getStatusCode(),
RESTResponse.STATUS_SUCCESS);
Assert.assertEquals(resp.getMsgDesc(),
"Total Role Created = 6 , Total Role Updated =
1 , Total Role Unchanged = 0");
- Mockito.verify(roleStore,
Mockito.atLeastOnce()).createRole(Mockito.any(RangerRole.class), eq(false),
eq(true), eq(false));
- Mockito.verify(roleStore,
Mockito.atLeastOnce()).updateRole(Mockito.any(RangerRole.class), eq(false),
eq(true), eq(true));
}
// import role throws exceptions
