This is an automated email from the ASF dual-hosted git repository. dineshkumar-yadav pushed a commit to branch sandbox/dineshkumar-yadav/revert-ranger-2.10-RANGER-5730 in repository https://gitbox.apache.org/repos/asf/ranger.git
commit 173ac69945626c09169c7d86b3f305f638b63cdf Author: Dineshkumar Yadav <[email protected]> AuthorDate: Thu Oct 1 13:10:24 2026 +0530 Revert "RANGER-5730: Restrict role import createNonExist flag to roles only (#1166)" This reverts commit 995a45e8cf0fe3cacc93dffeb551c901a1a0fd46. --- .../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
