Skip to content

Commit b149e09

Browse files
author
Dineshkumar Yadav
committed
Revert "RANGER-5730: Restrict role import createNonExist flag to roles only (#1142)"
This reverts commit 8dba041.
1 parent a93f268 commit b149e09

7 files changed

Lines changed: 15 additions & 119 deletions

File tree

‎agents-common/src/main/java/org/apache/ranger/plugin/store/RoleStore.java‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -40,26 +40,6 @@ default RangerRole updateRole(RangerRole role, Boolean createNonExistUserGroup,
4040
return updateRole(role, createNonExistUserGroup);
4141
}
4242

43-
/**
44-
* Create role with separate controls for creating missing users/groups vs nested roles.
45-
* Default implementation combines the flags for stores that do not support the split.
46-
*/
47-
default RangerRole createRole(RangerRole role, Boolean createNonExistUserGroup, Boolean createNonExistRole, Boolean isRefTableCleanupRequired) throws Exception {
48-
boolean combined = Boolean.TRUE.equals(createNonExistUserGroup) || Boolean.TRUE.equals(createNonExistRole);
49-
50-
return createRole(role, combined, isRefTableCleanupRequired);
51-
}
52-
53-
/**
54-
* Update role with separate controls for creating missing users/groups vs nested roles.
55-
* Default implementation combines the flags for stores that do not support the split.
56-
*/
57-
default RangerRole updateRole(RangerRole role, Boolean createNonExistUserGroup, Boolean createNonExistRole, Boolean isRefTableCleanupRequired) throws Exception {
58-
boolean combined = Boolean.TRUE.equals(createNonExistUserGroup) || Boolean.TRUE.equals(createNonExistRole);
59-
60-
return updateRole(role, combined, isRefTableCleanupRequired);
61-
}
62-
6343
void deleteRole(String roleName) throws Exception;
6444

6545
void deleteRole(Long roleId) throws Exception;

‎security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,6 @@ public RangerRole updateRole(RangerRole role, Boolean createNonExistUserGroupRol
105105

106106
@Override
107107
public RangerRole createRole(RangerRole role, Boolean createNonExistUserGroupRole, Boolean isRefTableCleanupRequired) throws Exception {
108-
return createRole(role, createNonExistUserGroupRole, createNonExistUserGroupRole, isRefTableCleanupRequired);
109-
}
110-
111-
@Override
112-
public RangerRole createRole(RangerRole role, Boolean createNonExistUserGroup, Boolean createNonExistRole, Boolean isRefTableCleanupRequired) throws Exception {
113108
LOG.debug("==> RoleDBStore.createRole()");
114109

115110
XXRole xxRole = daoMgr.getXXRole().findByRoleName(role.getName());
@@ -129,7 +124,7 @@ public RangerRole createRole(RangerRole role, Boolean createNonExistUserGroup, B
129124
throw new Exception("Cannot create role:[" + role + "]");
130125
}
131126

132-
roleRefUpdater.createNewRoleMappingForRefTable(createdRole, createNonExistUserGroup, createNonExistRole, isRefTableCleanupRequired);
127+
roleRefUpdater.createNewRoleMappingForRefTable(createdRole, createNonExistUserGroupRole, isRefTableCleanupRequired);
133128

134129
roleService.createTransactionLog(createdRole, null, RangerBaseModelService.OPERATION_CREATE_CONTEXT);
135130

@@ -138,11 +133,6 @@ public RangerRole createRole(RangerRole role, Boolean createNonExistUserGroup, B
138133

139134
@Override
140135
public RangerRole updateRole(RangerRole role, Boolean createNonExistUserGroupRole, Boolean isRefTableCleanupRequired) throws Exception {
141-
return updateRole(role, createNonExistUserGroupRole, createNonExistUserGroupRole, isRefTableCleanupRequired);
142-
}
143-
144-
@Override
145-
public RangerRole updateRole(RangerRole role, Boolean createNonExistUserGroup, Boolean createNonExistRole, Boolean isRefTableCleanupRequired) throws Exception {
146136
XXRole xxRole = daoMgr.getXXRole().findByRoleId(role.getId());
147137

148138
if (xxRole == null) {
@@ -169,7 +159,7 @@ public RangerRole updateRole(RangerRole role, Boolean createNonExistUserGroup, B
169159
throw new Exception("Cannot update role:[" + role + "]");
170160
}
171161

172-
roleRefUpdater.createNewRoleMappingForRefTable(updatedRole, createNonExistUserGroup, createNonExistRole, isRefTableCleanupRequired);
162+
roleRefUpdater.createNewRoleMappingForRefTable(updatedRole, createNonExistUserGroupRole, isRefTableCleanupRequired);
173163

174164
roleService.updatePolicyVersions(updatedRole.getId());
175165

‎security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java‎

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -84,21 +84,7 @@ public RangerDaoManager getRangerDaoManager() {
8484
return daoMgr;
8585
}
8686

87-
/**
88-
* Creates role-user/group/role ref mappings.
89-
* When {@code createNonExistUserGroupRole} is true, missing users, groups, and nested roles may be created
90-
* (existing create/update role API behavior).
91-
*/
9287
public void createNewRoleMappingForRefTable(RangerRole rangerRole, Boolean createNonExistUserGroupRole, Boolean isRefTableCleanupRequired) {
93-
createNewRoleMappingForRefTable(rangerRole, createNonExistUserGroupRole, createNonExistUserGroupRole, isRefTableCleanupRequired);
94-
}
95-
96-
/**
97-
* Creates role-user/group/role ref mappings with separate controls for creating missing users/groups vs nested roles.
98-
* Role import uses createNonExistUserGroup=false and createNonExistRole=true so nested roles can be created
99-
* without forcing creation of missing users or groups.
100-
*/
101-
public void createNewRoleMappingForRefTable(RangerRole rangerRole, Boolean createNonExistUserGroup, Boolean createNonExistRole, Boolean isRefTableCleanupRequired) {
10288
if (rangerRole == null) {
10389
return;
10490
}
@@ -123,13 +109,11 @@ public void createNewRoleMappingForRefTable(RangerRole rangerRole, Boolean creat
123109
roleRoles.add(role.getName());
124110
}
125111

126-
if (Boolean.TRUE.equals(isRefTableCleanupRequired)) {
112+
if (isRefTableCleanupRequired) {
127113
cleanupRefTablesForUpdate(rangerRole, roleUsers, roleGroups, roleRoles);
128114
}
129115

130-
final boolean adminAccess = xaBizUtil.checkAdminAccess();
131-
final boolean isCreateNonExistentUGs = Boolean.TRUE.equals(createNonExistUserGroup) && adminAccess;
132-
final boolean isCreateNonExistentRoles = Boolean.TRUE.equals(createNonExistRole) && adminAccess;
116+
final boolean isCreateNonExistentUGRs = createNonExistUserGroupRole && xaBizUtil.checkAdminAccess();
133117

134118
if (CollectionUtils.isNotEmpty(roleUsers)) {
135119
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
151135
if (userRef != null) {
152136
xxRoleRefUsers.add(userRef);
153137
}
154-
} else if (isCreateNonExistentUGs) {
138+
} else if (isCreateNonExistentUGRs) {
155139
rangerTransactionSynchronizationAdapter.executeOnTransactionCommit(associator);
156140
} else {
157141
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
181165
if (groupRef != null) {
182166
xxRoleRefGroups.add(groupRef);
183167
}
184-
} else if (isCreateNonExistentUGs) {
168+
} else if (isCreateNonExistentUGRs) {
185169
rangerTransactionSynchronizationAdapter.executeOnTransactionCommit(associator);
186170
} else {
187171
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
211195
if (roleRef != null) {
212196
xxRoleRefRoles.add(roleRef);
213197
}
214-
} else if (isCreateNonExistentRoles) {
198+
} else if (isCreateNonExistentUGRs) {
215199
rangerTransactionSynchronizationAdapter.executeOnTransactionCommit(associator);
216200
} else {
217201
throw restErrorUtil.createRESTException("Role with name: " + subRoleName + " does not exist ", MessageEnums.INVALID_INPUT_DATA);

‎security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -491,11 +491,6 @@ public RESTResponse importRolesFromFile(@Context HttpServletRequest request, @Fo
491491
updateIfExists = false;
492492
}
493493

494-
// For role import, createNonExistUserGroupRole only creates missing nested roles.
495-
// Missing users/groups are never created during import (RANGER-5730).
496-
final Boolean createNonExistUserGroup = Boolean.FALSE;
497-
final Boolean createNonExistRole = Boolean.TRUE.equals(createNonExistUserGroupRole);
498-
499494
List<String> roleNameList = getRoleNameList(request, new ArrayList<>());
500495
String fileName = fileDetail.getFileName();
501496
int totalRoleCreate = 0;
@@ -535,7 +530,7 @@ public RESTResponse importRolesFromFile(@Context HttpServletRequest request, @Fo
535530

536531
LOG.debug("Ignoring Roles from provided role in Json file... {}", roleNameInJson);
537532
} else {
538-
roleStore.updateRole(roleInJson, createNonExistUserGroup, createNonExistRole, true);
533+
roleStore.updateRole(roleInJson, createNonExistUserGroupRole, true);
539534

540535
totalRoleUpdate++;
541536
}
@@ -555,7 +550,7 @@ public RESTResponse importRolesFromFile(@Context HttpServletRequest request, @Fo
555550
ret.setStatusCode(RESTResponse.STATUS_SUCCESS);
556551
} else if (!roleNameList.contains(roleNameInJson) && (!roleNameInJson.isEmpty())) {
557552
try {
558-
roleStore.createRole(roleInJson, createNonExistUserGroup, createNonExistRole, false);
553+
roleStore.createRole(roleInJson, createNonExistUserGroupRole, false);
559554
} catch (WebApplicationException excp) {
560555
throw excp;
561556
} catch (Throwable excp) {

‎security-admin/src/test/java/org/apache/ranger/biz/TestRoleDBStore.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ public void testCreateRole() throws Exception {
435435
Mockito.when(roleService.create(rangerRole)).thenReturn(rangerRole);
436436
Mockito.when(roleService.read(xxRole.getId())).thenReturn(rangerRole);
437437
Mockito.doNothing().when(transactionSynchronizationAdapter).executeOnTransactionCommit(Mockito.any());
438-
Mockito.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean(), Mockito.anyBoolean());
438+
Mockito.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean());
439439
Mockito.doNothing().when(roleService).createTransactionLog(Mockito.any(), Mockito.any(), Mockito.anyInt());
440440

441441
roleDBStore.createRole(rangerRole, true, false);
@@ -462,7 +462,7 @@ public void testUpdateRole() throws Exception {
462462
Mockito.when(xxRoleDao.findByRoleId(rangerRole.getId())).thenReturn(xxRole);
463463
Mockito.doNothing().when(transactionSynchronizationAdapter).executeOnTransactionCommit(Mockito.any());
464464
Mockito.when(roleService.update(rangerRole)).thenReturn(rangerRole);
465-
Mockito.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean(), Mockito.anyBoolean());
465+
Mockito.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean());
466466
Mockito.doNothing().when(roleService).updatePolicyVersions(rangerRole.getId());
467467
Mockito.doNothing().when(roleService).createTransactionLog(Mockito.any(), Mockito.any(), Mockito.anyInt());
468468

‎security-admin/src/test/java/org/apache/ranger/biz/TestRoleRefUpdater.java‎

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -195,55 +195,6 @@ public void test03_createNewRoleMapping_missingPrincipalThrowsUnlessCreateAllowe
195195
verify(adapter, times(1)).executeOnTransactionCommit(any(Runnable.class));
196196
}
197197

198-
@Test
199-
public void test05a_createNewRoleMapping_createRoleOnlyDoesNotCreateMissingUser() throws Exception {
200-
RoleRefUpdater updater = new RoleRefUpdater();
201-
RangerDaoManager dao = mock(RangerDaoManager.class);
202-
XXUserDao xUserDao = mock(XXUserDao.class);
203-
RESTErrorUtil rest = mock(RESTErrorUtil.class);
204-
RangerBizUtil biz = mock(RangerBizUtil.class);
205-
206-
setField(updater, RoleRefUpdater.class, "daoMgr", dao);
207-
setField(updater, RoleRefUpdater.class, "restErrorUtil", rest);
208-
setField(updater, RoleRefUpdater.class, "xaBizUtil", biz);
209-
210-
when(dao.getXXUser()).thenReturn(xUserDao);
211-
when(xUserDao.getIdsByUserNames(anySet())).thenReturn(Collections.emptyMap());
212-
when(biz.checkAdminAccess()).thenReturn(true);
213-
214-
RuntimeException expected = new RuntimeException("missing user");
215-
when(rest.createRESTException(anyString(), any())).thenThrow(expected);
216-
217-
// Missing user with createNonExistUserGroup=false, createNonExistRole=true -> must fail (no user create)
218-
RangerRole roleWithMissingUser = buildRole(9L, Collections.singletonList("missingUser"), Collections.emptyList(),
219-
Collections.emptyList());
220-
Assertions.assertThrows(RuntimeException.class,
221-
() -> updater.createNewRoleMappingForRefTable(roleWithMissingUser, false, true, false));
222-
}
223-
224-
@Test
225-
public void test05b_createNewRoleMapping_createRoleOnlyCreatesMissingNestedRole() throws Exception {
226-
RoleRefUpdater updater = new RoleRefUpdater();
227-
RangerDaoManager dao = mock(RangerDaoManager.class);
228-
XXRoleDao xRoleDao = mock(XXRoleDao.class);
229-
RangerTransactionSynchronizationAdapter adapter = mock(RangerTransactionSynchronizationAdapter.class);
230-
RangerBizUtil biz = mock(RangerBizUtil.class);
231-
232-
setField(updater, RoleRefUpdater.class, "daoMgr", dao);
233-
setField(updater, RoleRefUpdater.class, "rangerTransactionSynchronizationAdapter", adapter);
234-
setField(updater, RoleRefUpdater.class, "xaBizUtil", biz);
235-
236-
when(dao.getXXRole()).thenReturn(xRoleDao);
237-
when(xRoleDao.getIdsByRoleNames(anySet())).thenReturn(Collections.emptyMap());
238-
when(biz.checkAdminAccess()).thenReturn(true);
239-
240-
// Missing nested role with createNonExistUserGroup=false, createNonExistRole=true -> schedule role create only
241-
RangerRole roleWithMissingNestedRole = buildRole(9L, Collections.emptyList(), Collections.emptyList(),
242-
Collections.singletonList("missingRole"));
243-
updater.createNewRoleMappingForRefTable(roleWithMissingNestedRole, false, true, false);
244-
verify(adapter, times(1)).executeOnTransactionCommit(any(Runnable.class));
245-
}
246-
247198
@Test
248199
public void test04_cleanupRefTablesForUpdate_selectivePrincipalCleanup() throws Exception {
249200
RoleRefUpdater updater = new RoleRefUpdater();

‎security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1500,7 +1500,7 @@ public void test20importRolesFromFile() throws Exception {
15001500

15011501
Mockito.when(searchUtil.getSearchFilter(request, roleService.sortFields)).thenReturn(filter);
15021502
Mockito.when(roleStore.getRoleNames(Mockito.any(SearchFilter.class))).thenReturn(roleList);
1503-
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(false), eq(createNonExistUserGroupRole), eq(isRefTableCleanupRequired))).thenReturn(rangerRole);
1503+
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(createNonExistUserGroupRole), eq(isRefTableCleanupRequired))).thenReturn(rangerRole);
15041504

15051505
RESTResponse resp = roleRest.importRolesFromFile(request, uploadedInputStream, fileDetail, updateIfExists, createNonExistUserGroupRole);
15061506
Assertions.assertNotNull(resp);
@@ -1528,14 +1528,12 @@ public void test20bimportRolesFromFile() throws Exception {
15281528

15291529
Mockito.when(searchUtil.getSearchFilter(request, roleService.sortFields)).thenReturn(filter);
15301530
Mockito.when(roleStore.getRoleNames(Mockito.any(SearchFilter.class))).thenReturn(roleList);
1531-
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(false), eq(createNonExistUserGroupRole), eq(isRefTableCleanupRequired))).thenReturn(rangerRole);
1531+
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(createNonExistUserGroupRole), eq(isRefTableCleanupRequired))).thenReturn(rangerRole);
15321532

15331533
RESTResponse resp = roleRest.importRolesFromFile(request, uploadedInputStream, fileDetail, updateIfExists, createNonExistUserGroupRole);
15341534
Assertions.assertNotNull(resp);
15351535
Assertions.assertEquals(resp.getStatusCode(), RESTResponse.STATUS_SUCCESS);
15361536
Assertions.assertEquals(resp.getMsgDesc(), "Total Role Created = 6 , Total Role Unchanged = 1");
1537-
// Import with flag=true must request role creation only (not users/groups)
1538-
Mockito.verify(roleStore, Mockito.atLeastOnce()).createRole(Mockito.any(RangerRole.class), eq(false), eq(true), eq(false));
15391537
}
15401538

15411539
// import role with updateIfExists=true and createNonExistUserGroupRole=true
@@ -1558,15 +1556,13 @@ public void test20cimportRolesFromFileWithUpdate() throws Exception {
15581556
Mockito.when(searchUtil.getSearchFilter(request, roleService.sortFields)).thenReturn(filter);
15591557
Mockito.when(roleStore.getRoleNames(Mockito.any(SearchFilter.class))).thenReturn(roleList);
15601558
Mockito.when(roleStore.getRole(Mockito.anyString())).thenReturn(rangerRole);
1561-
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(false), eq(createNonExistUserGroupRole), eq(false))).thenReturn(rangerRole);
1562-
Mockito.when(roleStore.updateRole(Mockito.any(RangerRole.class), eq(false), eq(createNonExistUserGroupRole), eq(true))).thenReturn(rangerRole);
1559+
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(createNonExistUserGroupRole), eq(false))).thenReturn(rangerRole);
1560+
Mockito.when(roleStore.updateRole(Mockito.any(RangerRole.class), eq(createNonExistUserGroupRole), eq(true))).thenReturn(rangerRole);
15631561

15641562
RESTResponse resp = roleRest.importRolesFromFile(request, uploadedInputStream, fileDetail, updateIfExists, createNonExistUserGroupRole);
15651563
Assertions.assertNotNull(resp);
15661564
Assertions.assertEquals(resp.getStatusCode(), RESTResponse.STATUS_SUCCESS);
15671565
Assertions.assertEquals(resp.getMsgDesc(), "Total Role Created = 6 , Total Role Updated = 1 , Total Role Unchanged = 0");
1568-
Mockito.verify(roleStore, Mockito.atLeastOnce()).createRole(Mockito.any(RangerRole.class), eq(false), eq(true), eq(false));
1569-
Mockito.verify(roleStore, Mockito.atLeastOnce()).updateRole(Mockito.any(RangerRole.class), eq(false), eq(true), eq(true));
15701566
}
15711567

15721568
// import role throws exceptions

0 commit comments

Comments
 (0)