This is an automated email from the ASF dual-hosted git repository.
pradeepagrawal8184 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 4cfd669a2 Revert "RANGER-5730: Restrict role import createNonExist
flag to roles only (#1142)" (#1274)
4cfd669a2 is described below
commit 4cfd669a23586525ab2adac73ee96b9c472655c1
Author: Dineshkumar Yadav <[email protected]>
AuthorDate: Thu Oct 1 15:33:29 2026 +0530
Revert "RANGER-5730: Restrict role import createNonExist flag to roles only
(#1142)" (#1274)
This reverts commit 8dba0414abdff996f08fdd9a59bd3fb7958f4c87.
Co-authored-by: Dineshkumar Yadav <[email protected]>
---
.../org/apache/ranger/plugin/store/RoleStore.java | 20 ---------
.../java/org/apache/ranger/biz/RoleDBStore.java | 14 +------
.../java/org/apache/ranger/biz/RoleRefUpdater.java | 26 +++---------
.../main/java/org/apache/ranger/rest/RoleREST.java | 9 +---
.../org/apache/ranger/biz/TestRoleDBStore.java | 4 +-
.../org/apache/ranger/biz/TestRoleRefUpdater.java | 49 ----------------------
.../java/org/apache/ranger/rest/TestRoleREST.java | 12 ++----
7 files changed, 15 insertions(+), 119 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 5205c5cdc..f86f52ecf 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
@@ -40,26 +40,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;
void deleteRole(Long roleId) 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 1d410377a..4d06bfe67 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
@@ -105,11 +105,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 {
LOG.debug("==> RoleDBStore.createRole()");
XXRole xxRole = daoMgr.getXXRole().findByRoleName(role.getName());
@@ -129,7 +124,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);
@@ -138,11 +133,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) {
@@ -169,7 +159,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 b6e1a9c1a..5df0c7ee5 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
@@ -491,11 +491,6 @@ public RESTResponse importRolesFromFile(@Context
HttpServletRequest request, @Fo
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 = getRoleNameList(request, new
ArrayList<>());
String fileName = fileDetail.getFileName();
int totalRoleCreate = 0;
@@ -535,7 +530,7 @@ public RESTResponse importRolesFromFile(@Context
HttpServletRequest request, @Fo
LOG.debug("Ignoring Roles from
provided role in Json file... {}", roleNameInJson);
} else {
-
roleStore.updateRole(roleInJson, createNonExistUserGroup, createNonExistRole,
true);
+
roleStore.updateRole(roleInJson, createNonExistUserGroupRole, true);
totalRoleUpdate++;
}
@@ -555,7 +550,7 @@ public RESTResponse importRolesFromFile(@Context
HttpServletRequest request, @Fo
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 a7293e4f2..9548c14f5 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
@@ -435,7 +435,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);
@@ -462,7 +462,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 2656dbb0d..39790f7fb 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
@@ -1500,7 +1500,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(isRefTableCleanupRequired))).thenReturn(rangerRole);
+ Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class),
eq(createNonExistUserGroupRole),
eq(isRefTableCleanupRequired))).thenReturn(rangerRole);
RESTResponse resp = roleRest.importRolesFromFile(request,
uploadedInputStream, fileDetail, updateIfExists, createNonExistUserGroupRole);
Assertions.assertNotNull(resp);
@@ -1528,14 +1528,12 @@ 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(isRefTableCleanupRequired))).thenReturn(rangerRole);
+ Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class),
eq(createNonExistUserGroupRole),
eq(isRefTableCleanupRequired))).thenReturn(rangerRole);
RESTResponse resp = roleRest.importRolesFromFile(request,
uploadedInputStream, fileDetail, updateIfExists, createNonExistUserGroupRole);
Assertions.assertNotNull(resp);
Assertions.assertEquals(resp.getStatusCode(),
RESTResponse.STATUS_SUCCESS);
Assertions.assertEquals(resp.getMsgDesc(), "Total Role Created = 6 ,
Total Role Unchanged = 1");
- // Import with flag=true must request role creation only (not
users/groups)
- Mockito.verify(roleStore,
Mockito.atLeastOnce()).createRole(Mockito.any(RangerRole.class), eq(false),
eq(true), eq(false));
}
// import role with updateIfExists=true and
createNonExistUserGroupRole=true
@@ -1558,15 +1556,13 @@ 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))).thenReturn(rangerRole);
- Mockito.when(roleStore.updateRole(Mockito.any(RangerRole.class),
eq(false), eq(createNonExistUserGroupRole), eq(true))).thenReturn(rangerRole);
+ Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class),
eq(createNonExistUserGroupRole), eq(false))).thenReturn(rangerRole);
+ Mockito.when(roleStore.updateRole(Mockito.any(RangerRole.class),
eq(createNonExistUserGroupRole), eq(true))).thenReturn(rangerRole);
RESTResponse resp = roleRest.importRolesFromFile(request,
uploadedInputStream, fileDetail, updateIfExists, createNonExistUserGroupRole);
Assertions.assertNotNull(resp);
Assertions.assertEquals(resp.getStatusCode(),
RESTResponse.STATUS_SUCCESS);
Assertions.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
