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 f86f52ecf4c..5205c5cdc1b 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,6 +40,26 @@ 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 4d06bfe67bd..1d410377a8d 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,6 +105,11 @@ 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()); @@ -124,7 +129,7 @@ public RangerRole createRole(RangerRole role, Boolean createNonExistUserGroupRol throw new Exception("Cannot create role:[" + role + "]"); } - roleRefUpdater.createNewRoleMappingForRefTable(createdRole, createNonExistUserGroupRole, isRefTableCleanupRequired); + roleRefUpdater.createNewRoleMappingForRefTable(createdRole, createNonExistUserGroup, createNonExistRole, isRefTableCleanupRequired); roleService.createTransactionLog(createdRole, null, RangerBaseModelService.OPERATION_CREATE_CONTEXT); @@ -133,6 +138,11 @@ public RangerRole createRole(RangerRole role, Boolean createNonExistUserGroupRol @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) { @@ -159,7 +169,7 @@ public RangerRole updateRole(RangerRole role, Boolean createNonExistUserGroupRol throw new Exception("Cannot update role:[" + role + "]"); } - roleRefUpdater.createNewRoleMappingForRefTable(updatedRole, createNonExistUserGroupRole, isRefTableCleanupRequired); + roleRefUpdater.createNewRoleMappingForRefTable(updatedRole, createNonExistUserGroup, createNonExistRole, 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 82fa68f149b..2bb3f2ecf77 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,7 +84,21 @@ 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; } @@ -109,11 +123,13 @@ public void createNewRoleMappingForRefTable(RangerRole rangerRole, Boolean creat roleRoles.add(role.getName()); } - if (isRefTableCleanupRequired) { + if (Boolean.TRUE.equals(isRefTableCleanupRequired)) { cleanupRefTablesForUpdate(rangerRole, roleUsers, roleGroups, roleRoles); } - final boolean isCreateNonExistentUGRs = createNonExistUserGroupRole && xaBizUtil.checkAdminAccess(); + final boolean adminAccess = xaBizUtil.checkAdminAccess(); + final boolean isCreateNonExistentUGs = Boolean.TRUE.equals(createNonExistUserGroup) && adminAccess; + final boolean isCreateNonExistentRoles = Boolean.TRUE.equals(createNonExistRole) && adminAccess; if (CollectionUtils.isNotEmpty(roleUsers)) { LOG.debug("New user entries to be inserted into x_role_ref_user for role ID {}: {}", roleId, roleUsers); @@ -135,7 +151,7 @@ public void createNewRoleMappingForRefTable(RangerRole rangerRole, Boolean creat if (userRef != null) { xxRoleRefUsers.add(userRef); } - } else if (isCreateNonExistentUGRs) { + } else if (isCreateNonExistentUGs) { rangerTransactionSynchronizationAdapter.executeOnTransactionCommit(associator); } else { throw restErrorUtil.createRESTException("user with name: " + userName + " does not exist ", MessageEnums.INVALID_INPUT_DATA); @@ -165,7 +181,7 @@ public void createNewRoleMappingForRefTable(RangerRole rangerRole, Boolean creat if (groupRef != null) { xxRoleRefGroups.add(groupRef); } - } else if (isCreateNonExistentUGRs) { + } else if (isCreateNonExistentUGs) { rangerTransactionSynchronizationAdapter.executeOnTransactionCommit(associator); } else { throw restErrorUtil.createRESTException("Group with name: " + groupName + " does not exist ", MessageEnums.INVALID_INPUT_DATA); @@ -195,7 +211,7 @@ public void createNewRoleMappingForRefTable(RangerRole rangerRole, Boolean creat if (roleRef != null) { xxRoleRefRoles.add(roleRef); } - } else if (isCreateNonExistentUGRs) { + } else if (isCreateNonExistentRoles) { 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 b92b221fe4b..df19ce43161 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 @@ -481,6 +481,11 @@ 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 roleNameList = getRoleNameList(request, new ArrayList<>()); String fileName = fileDetail.getFileName(); int totalRoleCreate = 0; @@ -520,7 +525,7 @@ public RESTResponse importRolesFromFile(@Context HttpServletRequest request, @Fo LOG.debug("Ignoring Roles from provided role in Json file... {}", roleNameInJson); } else { - roleStore.updateRole(roleInJson, createNonExistUserGroupRole, true); + roleStore.updateRole(roleInJson, createNonExistUserGroup, createNonExistRole, true); totalRoleUpdate++; } @@ -540,7 +545,7 @@ public RESTResponse importRolesFromFile(@Context HttpServletRequest request, @Fo ret.setStatusCode(RESTResponse.STATUS_SUCCESS); } else if (!roleNameList.contains(roleNameInJson) && (!roleNameInJson.isEmpty())) { try { - roleStore.createRole(roleInJson, createNonExistUserGroupRole, false); + roleStore.createRole(roleInJson, createNonExistUserGroup, createNonExistRole, 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 9548c14f5d9..a7293e4f2f8 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.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(), Mockito.anyBoolean(), 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.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(), Mockito.anyBoolean(), 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 800fa1a32fe..89146fae67c 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,6 +195,55 @@ 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 b7de4172423..d81ad8afbb5 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 @@ -1214,7 +1214,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(createNonExistUserGroupRole), eq(isRefTableCleanupRequired))).thenReturn(rangerRole); + Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(false), eq(createNonExistUserGroupRole), eq(isRefTableCleanupRequired))).thenReturn(rangerRole); RESTResponse resp = roleRest.importRolesFromFile(request, uploadedInputStream, fileDetail, updateIfExists, createNonExistUserGroupRole); Assertions.assertNotNull(resp); @@ -1242,12 +1242,14 @@ 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(createNonExistUserGroupRole), eq(isRefTableCleanupRequired))).thenReturn(rangerRole); + Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(false), 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 @@ -1270,13 +1272,15 @@ 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(createNonExistUserGroupRole), eq(false))).thenReturn(rangerRole); - Mockito.when(roleStore.updateRole(Mockito.any(RangerRole.class), eq(createNonExistUserGroupRole), eq(true))).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); 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