Skip to content

Commit 173ac69

Browse files
author
Dineshkumar Yadav
committed
Revert "RANGER-5730: Restrict role import createNonExist flag to roles only (#1166)"
This reverts commit 995a45e.
1 parent fb8abbc commit 173ac69

7 files changed

Lines changed: 15 additions & 118 deletions

File tree

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

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -41,25 +41,6 @@ default RangerRole updateRole(RangerRole role, Boolean createNonExistUserGroup,
4141
return updateRole(role, createNonExistUserGroup);
4242
}
4343

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

6445
void deleteRole(String roleName) throws Exception;
6546

‎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
@@ -118,11 +118,6 @@ public RangerRole updateRole(RangerRole role, Boolean createNonExistUserGroupRol
118118

119119
@Override
120120
public RangerRole createRole(RangerRole role, Boolean createNonExistUserGroupRole, Boolean isRefTableCleanupRequired) throws Exception {
121-
return createRole(role, createNonExistUserGroupRole, createNonExistUserGroupRole, isRefTableCleanupRequired);
122-
}
123-
124-
@Override
125-
public RangerRole createRole(RangerRole role, Boolean createNonExistUserGroup, Boolean createNonExistRole, Boolean isRefTableCleanupRequired) throws Exception {
126121
if (LOG.isDebugEnabled()) {
127122
LOG.debug("==> RoleDBStore.createRole()");
128123
}
@@ -142,19 +137,14 @@ public RangerRole createRole(RangerRole role, Boolean createNonExistUserGroup, B
142137
throw new Exception("Cannot create role:[" + role + "]");
143138
}
144139

145-
roleRefUpdater.createNewRoleMappingForRefTable(createdRole, createNonExistUserGroup, createNonExistRole, isRefTableCleanupRequired);
140+
roleRefUpdater.createNewRoleMappingForRefTable(createdRole, createNonExistUserGroupRole, isRefTableCleanupRequired);
146141

147142
roleService.createTransactionLog(createdRole, null, RangerBaseModelService.OPERATION_CREATE_CONTEXT);
148143
return createdRole;
149144
}
150145

151146
@Override
152147
public RangerRole updateRole(RangerRole role, Boolean createNonExistUserGroupRole, Boolean isRefTableCleanupRequired) throws Exception {
153-
return updateRole(role, createNonExistUserGroupRole, createNonExistUserGroupRole, isRefTableCleanupRequired);
154-
}
155-
156-
@Override
157-
public RangerRole updateRole(RangerRole role, Boolean createNonExistUserGroup, Boolean createNonExistRole, Boolean isRefTableCleanupRequired) throws Exception {
158148
XXRole xxRole = daoMgr.getXXRole().findByRoleId(role.getId());
159149
if (xxRole == null) {
160150
throw restErrorUtil.createRESTException("role with id: " + role.getId() + " does not exist");
@@ -177,7 +167,7 @@ public RangerRole updateRole(RangerRole role, Boolean createNonExistUserGroup, B
177167
throw new Exception("Cannot update role:[" + role + "]");
178168
}
179169

180-
roleRefUpdater.createNewRoleMappingForRefTable(updatedRole, createNonExistUserGroup, createNonExistRole, isRefTableCleanupRequired);
170+
roleRefUpdater.createNewRoleMappingForRefTable(updatedRole, createNonExistUserGroupRole, isRefTableCleanupRequired);
181171

182172
roleService.updatePolicyVersions(updatedRole.getId());
183173

‎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 & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -492,12 +492,6 @@ public RESTResponse importRolesFromFile(@Context HttpServletRequest request,
492492
if (updateIfExists == null) {
493493
updateIfExists = false;
494494
}
495-
496-
// For role import, createNonExistUserGroupRole only creates missing nested roles.
497-
// Missing users/groups are never created during import (RANGER-5730).
498-
final Boolean createNonExistUserGroup = Boolean.FALSE;
499-
final Boolean createNonExistRole = Boolean.TRUE.equals(createNonExistUserGroupRole);
500-
501495
List<String> roleNameList = new ArrayList<String>();
502496

503497
roleNameList = getRoleNameList(request, roleNameList);
@@ -543,7 +537,7 @@ public RESTResponse importRolesFromFile(@Context HttpServletRequest request,
543537
}
544538
}
545539
else {
546-
roleStore.updateRole(roleInJson, createNonExistUserGroup, createNonExistRole, true);
540+
roleStore.updateRole(roleInJson, createNonExistUserGroupRole, true);
547541
totalRoleUpdate++;
548542
}
549543
} catch (WebApplicationException excp) {
@@ -562,7 +556,7 @@ public RESTResponse importRolesFromFile(@Context HttpServletRequest request,
562556
ret.setStatusCode(RESTResponse.STATUS_SUCCESS);
563557
} else if (!roleNameList.contains(roleNameInJson) && (!roleNameInJson.isEmpty())) {
564558
try {
565-
roleStore.createRole(roleInJson, createNonExistUserGroup, createNonExistRole, false);
559+
roleStore.createRole(roleInJson, createNonExistUserGroupRole, false);
566560
} catch (WebApplicationException excp) {
567561
throw excp;
568562
} 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
@@ -440,7 +440,7 @@ public void testCreateRole() throws Exception {
440440
Mockito.when(roleService.create(rangerRole)).thenReturn(rangerRole);
441441
Mockito.when(roleService.read(xxRole.getId())).thenReturn(rangerRole);
442442
Mockito.doNothing().when(transactionSynchronizationAdapter).executeOnTransactionCommit(Mockito.any());
443-
Mockito.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean(), Mockito.anyBoolean());
443+
Mockito.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean());
444444
Mockito.doNothing().when(roleService).createTransactionLog( Mockito.any(), Mockito.any(), Mockito.anyInt());
445445

446446
roleDBStore.createRole(rangerRole, true, false);
@@ -469,7 +469,7 @@ public void testUpdateRole() throws Exception {
469469
Mockito.when(xxRoleDao.findByRoleId(rangerRole.getId())).thenReturn(xxRole);
470470
Mockito.doNothing().when(transactionSynchronizationAdapter).executeOnTransactionCommit(Mockito.any());
471471
Mockito.when(roleService.update(rangerRole)).thenReturn(rangerRole);
472-
Mockito.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean(), Mockito.anyBoolean());
472+
Mockito.doNothing().when(roleRefUpdater).createNewRoleMappingForRefTable(Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean());
473473
Mockito.doNothing().when(roleService).updatePolicyVersions(rangerRole.getId());
474474
Mockito.doNothing().when(roleService).createTransactionLog( Mockito.any(), Mockito.any(), Mockito.anyInt());
475475

‎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 & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1434,7 +1434,7 @@ public void test20importRolesFromFile() throws Exception {
14341434

14351435
Mockito.when(searchUtil.getSearchFilter(request, roleService.sortFields)).thenReturn(filter);
14361436
Mockito.when(roleStore.getRoleNames(Mockito.any(SearchFilter.class))).thenReturn(roleList);
1437-
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(false), eq(createNonExistUserGroupRole), eq(false)))
1437+
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(createNonExistUserGroupRole), eq(false)))
14381438
.thenReturn(rangerRole);
14391439

14401440
RESTResponse resp = roleRest.importRolesFromFile(request, uploadedInputStream, fileDetail, updateIfExists,
@@ -1464,15 +1464,14 @@ public void test20bimportRolesFromFile() throws Exception {
14641464

14651465
Mockito.when(searchUtil.getSearchFilter(request, roleService.sortFields)).thenReturn(filter);
14661466
Mockito.when(roleStore.getRoleNames(Mockito.any(SearchFilter.class))).thenReturn(roleList);
1467-
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(false), eq(createNonExistUserGroupRole), eq(false)))
1467+
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(createNonExistUserGroupRole), eq(false)))
14681468
.thenReturn(rangerRole);
14691469

14701470
RESTResponse resp = roleRest.importRolesFromFile(request, uploadedInputStream, fileDetail, updateIfExists,
14711471
createNonExistUserGroupRole);
14721472
Assert.assertNotNull(resp);
14731473
Assert.assertEquals(resp.getStatusCode(), RESTResponse.STATUS_SUCCESS);
14741474
Assert.assertEquals(resp.getMsgDesc(), "Total Role Created = 6 , Total Role Unchanged = 1");
1475-
Mockito.verify(roleStore, Mockito.atLeastOnce()).createRole(Mockito.any(RangerRole.class), eq(false), eq(true), eq(false));
14761475
}
14771476

14781477
// import role with updateIfExists=true and createNonExistUserGroupRole=true
@@ -1496,9 +1495,9 @@ public void test20cimportRolesFromFileWithUpdate() throws Exception {
14961495
Mockito.when(searchUtil.getSearchFilter(request, roleService.sortFields)).thenReturn(filter);
14971496
Mockito.when(roleStore.getRoleNames(Mockito.any(SearchFilter.class))).thenReturn(roleList);
14981497
Mockito.when(roleStore.getRole(Mockito.anyString())).thenReturn(rangerRole);
1499-
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(false), eq(createNonExistUserGroupRole), eq(false)))
1498+
Mockito.when(roleStore.createRole(Mockito.any(RangerRole.class), eq(createNonExistUserGroupRole), eq(false)))
15001499
.thenReturn(rangerRole);
1501-
Mockito.when(roleStore.updateRole(Mockito.any(RangerRole.class), eq(false), eq(createNonExistUserGroupRole), eq(true)))
1500+
Mockito.when(roleStore.updateRole(Mockito.any(RangerRole.class), eq(createNonExistUserGroupRole), eq(true)))
15021501
.thenReturn(rangerRole);
15031502

15041503
RESTResponse resp = roleRest.importRolesFromFile(request, uploadedInputStream, fileDetail, updateIfExists,
@@ -1507,8 +1506,6 @@ public void test20cimportRolesFromFileWithUpdate() throws Exception {
15071506
Assert.assertEquals(resp.getStatusCode(), RESTResponse.STATUS_SUCCESS);
15081507
Assert.assertEquals(resp.getMsgDesc(),
15091508
"Total Role Created = 6 , Total Role Updated = 1 , Total Role Unchanged = 0");
1510-
Mockito.verify(roleStore, Mockito.atLeastOnce()).createRole(Mockito.any(RangerRole.class), eq(false), eq(true), eq(false));
1511-
Mockito.verify(roleStore, Mockito.atLeastOnce()).updateRole(Mockito.any(RangerRole.class), eq(false), eq(true), eq(true));
15121509
}
15131510

15141511
// import role throws exceptions

0 commit comments

Comments
 (0)