You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ranger.apache.org by di...@apache.org on 2026/10/01 08:50:33 UTC

(ranger) branch sandbox/dineshkumar-yadav/revert-ranger-2.10-RANGER-5730 created (now 173ac6994)

This is an automated email from the ASF dual-hosted git repository.

dineshkumar-yadav pushed a change to branch sandbox/dineshkumar-yadav/revert-ranger-2.10-RANGER-5730
in repository https://gitbox.apache.org/repos/asf/ranger.git


      at 173ac6994 Revert "RANGER-5730: Restrict role import createNonExist flag to roles only (#1166)"

This branch includes the following new commits:

     new 173ac6994 Revert "RANGER-5730: Restrict role import createNonExist flag to roles only (#1166)"

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(ranger) 01/01: Revert "RANGER-5730: Restrict role import createNonExist flag to roles only (#1166)"

Posted by di...@apache.org.
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 <di...@apache.org>
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