> On March 22, 2023, 7:35 p.m., Madhan Neethiraj wrote:
> > security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
> > Line 832 (original), 833 (patched)
> > <https://reviews.apache.org/r/74349/diff/1/?file=2274970#file2274970line833>
> >
> > Instead of introducing RangerUsersAndGroups, I suggest to use existing
> > RangerRole. This class also enables:
> > - adding roles to a role
> > - adding both admin and non-admin role members
> >
> >
> > Also, I would recommend not changing signature of existing public
> > methods - to avoid breaking clients using these methods. Instead, consider
> > adding new methods.
Hi Madhan,
Do we need to keep both old and new behaviour like this
Or something different ?
In PublicAPIsv2.java
@PUT
@Path("/api/roles/{id}/addUsersAndGroups")
@Consumes({ "application/json" })
@Produces({ "application/json" })
public RangerRole addUsersAndGroupsByRoleId(@PathParam("id") Long
roleId, RangerRole rangerRole, @DefaultValue("false") @QueryParam("isAdmin")
Boolean isAdmin, @Context HttpServletRequest request) {
if (logger.isDebugEnabled()) {
logger.debug("==> PublicAPIsv2.addUsersAndGroups(id=" +
roleId + ", rangerRole=" + rangerRole + ", isAdmin=" + isAdmin + ")");
}
RangerRole rangerRole =
roleREST.addUsersAndGroupsByRoleId(roleId, rangerUsersAndGroups, isAdmin);
if (logger.isDebugEnabled()) {
logger.debug("<== PublicAPIsv2.addUsersAndGroups(id=" +
roleId + ", rangerUsersAndGroups=" + rangerUsersAndGroups + ", isAdmin=" +
isAdmin + ")");
}
return rangerRole;
}
public RangerRole addUsersAndGroups(Long roleId, List<String> users,
List<String> groups, Boolean isAdmin) {
return roleREST.addUsersAndGroups(roleId, users, groups,
isAdmin);
}
In RoleREST.java
@PUT
@Path("/roles/{id}/addUsersAndGroups")
@Consumes({ "application/json" })
@Produces({ "application/json" })
public RangerRole addUsersAndGroupsByRoleId(@PathParam("id") Long roleId,
RangerRole rangerRole, @DefaultValue("false") @QueryParam("isAdmin") Boolean
isAdmin) {
if (LOG.isDebugEnabled()) {
LOG.debug("==> addUsersAndGroups(id=" + roleId + ", rangerRole=" +
rangerRole + ", isAdmin=" + isAdmin + ")");
}
//Fetch the groups and users from rangerRole and pass
List<String> users = new ArrayList<>();
List<String> groups = new ArrayList<>();
if (CollectionUtils.isEmpty(rangerRole.getUsers())) {
users = rangerRole.getUsers().stream().map(roleMember ->
roleMember.getName()).collect(Collectors.toList());
}
if (CollectionUtils.isEmpty(rangerRole.getGroups())) {
groups = rangerRole.getGroups().stream().map(roleMember ->
roleMember.getName()).collect(Collectors.toList());
}
RangerRole rangerRole = addUsersAndGroups(roleId, users, groups,
isAdmin);
if (LOG.isDebugEnabled()) {
LOG.debug("<== addUsersAndGroups(id=" + roleId + ", rangerRole=" +
rangerRole + ", isAdmin=" + isAdmin + ")");
}
return rangerRole;
}
public RangerRole addUsersAndGroups(Long roleId, List<String> users,
List<String> groups, Boolean isAdmin) {
if (LOG.isDebugEnabled()) {
LOG.debug("==> addUsersAndGroups(id=" + roleId + ", users=" +
Arrays.toString(users.toArray()) + ", groups=" +
Arrays.toString(groups.toArray()) + ", isAdmin=" + isAdmin + ")");
}
- Ramachandran
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225293
-----------------------------------------------------------
On March 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated March 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj,
> Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj,
> Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String>
> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of
> URI )
> Via Request payload of List<String> users, List<String> groups, Boolean
> isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users,
> groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String>
> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of
> URI )
> Via Request payload of List<String> users, List<String> groups
> (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String>
> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of
> URI )
> Via Request payload of List<String> users, List<String> groups
> (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java
> PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
> 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java
> 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java
> 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
