Skip to content

Ranger 5730: Restrict role import createNonExist flag to roles only #1142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
eoinmcdonnell113 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Ranger 5730: Restrict role import createNonExist flag to roles only #1142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions agents-common/src/main/java/org/apache/ranger/plugin/store/RoleStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 12 additions & 2 deletions security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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);

Expand All @@ -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) {
Expand All @@ -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());

Expand Down
26 changes: 21 additions & 5 deletions security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 7 additions & 2 deletions security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> roleNameList = getRoleNameList(request, new ArrayList<>());
String fileName = fileDetail.getFileName();
int totalRoleCreate = 0;
Expand Down Expand Up @@ -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++;
}
Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions security-admin/src/test/java/org/apache/ranger/biz/TestRoleDBStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());

Expand Down
49 changes: 49 additions & 0 deletions security-admin/src/test/java/org/apache/ranger/biz/TestRoleRefUpdater.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
12 changes: 8 additions & 4 deletions security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading