mneethiraj commented on code in PR #253:
URL: https://github.com/apache/ranger/pull/253#discussion_r1192692660
##########
security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java:
##########
@@ -863,6 +864,61 @@ public void deleteXUserByUserName(@PathParam("userName")
String userName,
xUserMgr.deleteXUser(vxUser.getId(), forceDelete);
}
+
+ /**
+ * Proceed with <tt>caution</tt>: Force deletes users from the ranger
db,
+ * <tt>Delete</tt> happens one at a time with immediate commit on the
transaction.
+ */
+ @DELETE
+ @Path("/delete/external/users")
Review Comment:
I suggest replacing `external` in the URI with `force', since this API is
not specific to external users i.e., userSource query-param.
##########
security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java:
##########
@@ -863,6 +864,61 @@ public void deleteXUserByUserName(@PathParam("userName")
String userName,
xUserMgr.deleteXUser(vxUser.getId(), forceDelete);
}
+
+ /**
+ * Proceed with <tt>caution</tt>: Force deletes users from the ranger
db,
+ * <tt>Delete</tt> happens one at a time with immediate commit on the
transaction.
+ */
+ @DELETE
+ @Path("/delete/external/users")
+ @PreAuthorize("hasRole('ROLE_SYS_ADMIN')")
+ @Produces({ "application/json" })
+ public Response forceDeleteUsers(@Context HttpServletRequest request) {
+ SearchCriteria searchCriteria = new SearchCriteria();
+ searchUtil.extractString(
+ request, searchCriteria, "name", "User
name",null);
+ searchUtil.extractString(
+ request, searchCriteria, "emailAddress", "Email
Address", null);
+ searchUtil.extractInt(
+ request, searchCriteria, "userSource", "User
Source");
+ searchUtil.extractInt(
+ request, searchCriteria, "isVisible", "User
Visibility");
+ searchUtil.extractInt(
+ request, searchCriteria, "status", "User
Status");
+ searchUtil.extractString(
+ request, searchCriteria, "syncSource", "Sync
Source", null);
+ searchUtil.extractRoleString(
+ request, searchCriteria, "userRole", "Role",
null);
+
+ VXUserList userList = xUserMgr.searchXUsers(searchCriteria);
+ xUserMgr.forceDeleteUsers(userList.getList());
+ return Response.ok(userList.getListSize() + " users deleted
successfully").build();
+ }
+
+ /**
+ * Proceed with <tt>caution</tt>: Force deletes groups from the ranger
db,
+ * <tt>Delete</tt> happens one at a time with immediate commit on the
transaction.
+ */
+ @DELETE
+ @Path("/delete/external/groups")
Review Comment:
I suggest replacing `external` in the URI with `force', since this API is
not specific to external group i.e., groupSource query-param.
##########
security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java:
##########
@@ -2400,6 +2426,31 @@ public synchronized void deleteXUser(Long id, boolean
force) {
}
}
+ public void forceDeleteUsers(List<VXUser> users){
+ long startTime = Time.now();
+ for(VXUser user: users){
+ Long userId = user.getId();
+ TransactionTemplate txTemplate = new
TransactionTemplate(txManager);
+
txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW);
+ try {
+ txTemplate.execute(new
TransactionCallback<Object>() {
+ @Override
+ public Object
doInTransaction(TransactionStatus status) {
+ deleteXUser(userId, true);
+ return null;
+ }
+ });
+ } catch (Throwable ex) {
+ logger.error("forceDeleteUsers(): Failed to
delete user id: " + userId + " ", ex);
+ throw new RuntimeException(ex);
Review Comment:
This exception would cause delete operation to be aborted on the first
failure, right? The response to the client wouldn't show that few users have
been deleted. I would suggest the following changes:
- instead of aborting on the first failure, this method should attempt to
delete subsequent users as well
- the return from this method should include list of users successfully
deleted, and the list of users who couldn't be deleted
Similar updates in forceDeleteGroups() as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
