mneethiraj commented on code in PR #253:
URL: https://github.com/apache/ranger/pull/253#discussion_r1188099205
##########
security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java:
##########
@@ -863,6 +868,49 @@ public void deleteXUserByUserName(@PathParam("userName")
String userName,
xUserMgr.deleteXUser(vxUser.getId(), forceDelete);
}
+
+ /**
+ * Proceed with caution: Force deletes users in bulk from the ranger db,
+ * essentially serves as a cleanup Op for external users.
+ * <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 deleteXUsers() {
+ if (!usersDeletionInProgress){
+ synchronized (XUserREST.class) {
+ usersDeletionInProgress = true;
+ xUserMgr.bulkDeleteUsers();
+ usersDeletionInProgress = false;
+ }
+ return Response.ok("External users deleted
successfully").build();
+ }
+ return Response.ok("Users Deletion in progress, check ranger
admin logs!").build();
Review Comment:
Instead of returning 'ok', consider returning an error code like 409
(conflict).
##########
security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java:
##########
@@ -165,6 +166,10 @@ public class XUserREST {
static final Logger logger = LoggerFactory.getLogger(XUserMgr.class);
+ static volatile boolean usersDeletionInProgress = false;
Review Comment:
Use of static variables wouldn't work in HA deployments, as subsequent
request might be going to a different instance. Why is it critical to avoid
multiple simultaneous calls?
##########
security-admin/src/main/resources/META-INF/jpa_named_queries.xml:
##########
@@ -161,6 +161,20 @@
</query>
</named-query>
+ <named-query name="XXUser.findByUserSource">
Review Comment:
XXUser.findByUserSource => XXUser.getUserIdsForSource
##########
security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java:
##########
@@ -863,6 +868,49 @@ public void deleteXUserByUserName(@PathParam("userName")
String userName,
xUserMgr.deleteXUser(vxUser.getId(), forceDelete);
}
+
+ /**
+ * Proceed with caution: Force deletes users in bulk from the ranger db,
+ * essentially serves as a cleanup Op for external users.
+ * <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 deleteXUsers() {
+ if (!usersDeletionInProgress){
+ synchronized (XUserREST.class) {
+ usersDeletionInProgress = true;
+ xUserMgr.bulkDeleteUsers();
+ usersDeletionInProgress = false;
+ }
+ return Response.ok("External users deleted
successfully").build();
+ }
+ return Response.ok("Users Deletion in progress, check ranger
admin logs!").build();
+ }
+
+ /**
+ * Proceed with caution: Force deletes groups in bulk from the ranger
db,
+ * essentially serves as a cleanup Op for external groups.
+ * <tt>Delete</tt> happens one at a time with immediate commit on the
transaction.
+ */
+ @DELETE
+ @Path("/delete/external/groups")
+ @PreAuthorize("hasRole('ROLE_SYS_ADMIN')")
+ @Produces({ "application/json" })
+ public Response deleteXGroups() {
Review Comment:
Instead of an API specifically to delete all external users, consider a
generic API that let's the call specify the users to delete via query-params -
similar to in searchXUsers():
```
@DELETE
@Path("/groups")
@PreAuthorize("hasRole('ROLE_SYS_ADMIN')")
@Produces({ "application/json" })
public Response deleteGroups(@Context HttpServletRequest request) {
SearchCriteria searchCriteria = searchUtil.extractCommonCriterias(request,
xGroupUserService.sortFields);
// TODO: update searchCriteria with query-params
Long<Long> groupIds = xUserMgr.deleteGroups(searchCriteria);
return Response.ok(groupIds.size() + " external groups deleted
successfully").build();
}
```
##########
security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java:
##########
@@ -2400,6 +2428,32 @@ public synchronized void deleteXUser(Long id, boolean
force) {
}
}
+ public void bulkDeleteUsers(){
Review Comment:
bulkDeleteUsers() => forceDeleteAllExternalUsers()
##########
security-admin/src/main/resources/META-INF/jpa_named_queries.xml:
##########
@@ -161,6 +161,20 @@
</query>
</named-query>
+ <named-query name="XXUser.findByUserSource">
+ <query>
+ SELECT user.id FROM XXUser user, XXPortalUser portalUser
+ WHERE user.name = portalUser.loginId and
portalUser.userSource = :userSource
+ </query>
+ </named-query>
+
+ <named-query name="XXGroup.findByGroupSource">
Review Comment:
XXGroup.findByGroupSource => XXGroup.getGroupIdsForSource
##########
security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java:
##########
@@ -2197,6 +2198,33 @@ public void deleteXGroup(Long id, boolean force) {
}
}
+ public void bulkDeleteGroups(){
Review Comment:
bulkDeleteGroups() => forceDeleteAllExternalGroups()
##########
security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java:
##########
@@ -863,6 +868,49 @@ public void deleteXUserByUserName(@PathParam("userName")
String userName,
xUserMgr.deleteXUser(vxUser.getId(), forceDelete);
}
+
+ /**
+ * Proceed with caution: Force deletes users in bulk from the ranger db,
+ * essentially serves as a cleanup Op for external users.
+ * <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 deleteXUsers() {
Review Comment:
Instead of an API specifically to delete all external users, consider a
generic API that let's the call specify the users to delete via query-params -
similar to in searchXUsers():
```
@DELETE
@Path("/users")
@PreAuthorize("hasRole('ROLE_SYS_ADMIN')")
@Produces({ "application/json" })
public Response deleteUsers(@Context HttpServletRequest request) {
SearchCriteria searchCriteria = searchUtil.extractCommonCriterias(request,
xUserService.sortFields);
// TODO: update searchCriteria with query-params
Long<Long> userIds = xUserMgr.deleteUsers(searchCriteria);
return Response.ok(userIds.size() + " external users deleted
successfully").build();
}
```
##########
security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java:
##########
@@ -863,6 +868,49 @@ public void deleteXUserByUserName(@PathParam("userName")
String userName,
xUserMgr.deleteXUser(vxUser.getId(), forceDelete);
}
+
+ /**
+ * Proceed with caution: Force deletes users in bulk from the ranger db,
+ * essentially serves as a cleanup Op for external users.
+ * <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 deleteXUsers() {
+ if (!usersDeletionInProgress){
+ synchronized (XUserREST.class) {
+ usersDeletionInProgress = true;
+ xUserMgr.bulkDeleteUsers();
+ usersDeletionInProgress = false;
+ }
+ return Response.ok("External users deleted
successfully").build();
+ }
+ return Response.ok("Users Deletion in progress, check ranger
admin logs!").build();
+ }
+
+ /**
+ * Proceed with caution: Force deletes groups in bulk from the ranger
db,
+ * essentially serves as a cleanup Op for external groups.
+ * <tt>Delete</tt> happens one at a time with immediate commit on the
transaction.
+ */
+ @DELETE
+ @Path("/delete/external/groups")
+ @PreAuthorize("hasRole('ROLE_SYS_ADMIN')")
+ @Produces({ "application/json" })
+ public Response deleteXGroups() {
+ if (!groupsDeletionInProgress) {
+ synchronized (XUserREST.class) {
+ groupsDeletionInProgress = true;
+ xUserMgr.bulkDeleteGroups();
+ groupsDeletionInProgress = false;
+ }
+ return Response.ok("External groups deleted
successfully").build();
+ }
+ return Response.ok("Groups Deletion in progress, check ranger
admin logs!").build();
Review Comment:
Instead of returning 'ok', consider returning an error code like 409
(conflict).
--
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]
