----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/75252/#review227013 -----------------------------------------------------------
agents-common/src/main/java/org/apache/ranger/plugin/model/RangerGds.java Lines 891 (patched) <https://reviews.apache.org/r/75252/#comment315282> This class can be used to update non-GDS policies as well in future. So, please rename RangerGdsGrant => RangerGrant; and move this class to its own file, RangerGrant.java. agents-common/src/main/java/org/apache/ranger/plugin/model/RangerGds.java Lines 894 (patched) <https://reviews.apache.org/r/75252/#comment315280> I suggest replacing principalType and principalName with: private RangerPrincipal principal; agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyHeader.java Lines 39 (patched) <https://reviews.apache.org/r/75252/#comment315281> Following fields should be added as well: - zoneName - policyType security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java Lines 1944 (patched) <https://reviews.apache.org/r/75252/#comment315283> Consider moving rangerPolicy insider the 'if' block at #1954, as it is not used outside this block. security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java Lines 1976 (patched) <https://reviews.apache.org/r/75252/#comment315284> Consider having a single return for a method: { List<RangerPolicyItem> ret = rangerPolicy == null ? null : rangerPolicy.getPolicyItems(); if (CollectionUtils.isNotEmpty(ret)) { String[] principals = searchUtil.getParamMultiValues(request, "principal"); String[] accessTypes = searchUtil.getParamMultiValues(request, "accessType"); if (ArrayUtils.isNotEmpty(principals) || ArrayUtils.isNotEmpty(accessTypes)) { PolicyItemPredicate pred = new PolicyItemPredicate(principals, accessTypes); ret = ret.streams().filter(pred).collect(Collectors.toList()); } } return ret; } private static class PolicyItemPredicate implements Predicate<RangerPolicyItem> { private final Set<String> users; private final Set<String> groups; private final Set<String> roles; private final Set<String> accessTypes; private final boolean hasPrincipalFilter; public PolicyItemPredicate(String[] principals, String[] accessTypes) { if (ArrayUtils.isNotEmpty(principals)) { this.users = new HashSet<>(); this.groups = new HashSet<>(); this.roles = new HashSet<>(); for (String principal : principals) { if (principal.startsWith("u:")) { this.users.add(principal.substring("u:".length())); } else if (principal.startsWith("g:")) { this.groups.add(principal.substring("g:".length())); } else if (principal.startsWith("r:")) { this.roles.add(principal.substring("r:".length())); } else { this.users.add(principal); } } } else { this.users = Collections.emptySet(); this.groups = Collections.emptySet(); this.roles = Collections.emptySet(); } this.hasPrincipalFilter = !this.users.isEmpty() || !groups.isEmpty() || !roles.isEmpty(); this.accessTypes = ArrayUtils.isNotEmpty(accessTypes) ? new HashSet<>(Arrays.asList(accessTypes)) : Collections.emptySet(); } @Override public boolean test(RangerPolicyItem policyItem) { boolean ret = accessTypes.isEmpty() || (policyItem.getAccesses() != null && policyItem.getAccesses().stream().anyMatch(access -> accessTypes.contains(access.getType()))); if (ret) { ret = (policyItem.getUsers() != null && policyItem.getUsers().stream().anyMatch(this::matchUser)) || (policyItem.getGroups() != null && policyItem.getGroups().stream().anyMatch(this::matchGroup)) || (policyItem.getRoles() != null && policyItem.getRoles().stream().anyMatch(this::matchRole)); } return ret; } public Set<String> getUsers() { return users; } public Set<String> getGroups() { return groups; } public Set<String> getRoles() { return roles; } public Set<String> getAccessTypes() { return accessTypes; } public boolean hasPrincipalFilter() { return hasPrincipalFilter; } @Override public boolean test(RangerPolicyItem policyItem) { boolean ret = accessTypes.isEmpty() || hasAccessType(policyItem); if (ret && hasPrincipalFilter) { ret = (policyItem.getUsers() != null && policyItem.getUsers().stream().anyMatch(users::contains)) || (policyItem.getGroups() != null && policyItem.getGroups().stream().anyMatch(groups::contains)) || (policyItem.getRoles() != null && policyItem.getRoles().stream().anyMatch(roles::contains)); } return ret; } public boolean hasAccessType(RangerPolicyItem policyItem) { return policyItem.getAccesses() != null && policyItem.getAccesses().stream().anyMatch(access -> accessTypes.contains(access.getType())); } } security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java Lines 2054 (patched) <https://reviews.apache.org/r/75252/#comment315285> Consider leveraging PolicyItemPredicate suggested earlier to simplify this method, along the following lines: private List<RangerGdsGrant> transformPolicyItemsToGdsGrants(List<RangerPolicyItem> policyItems, PolicyItemPredicate pred) { List<RangerGrant> ret = new ArrayList<RangerGrant>(); for (RangerPolicyItem policyItem : policyItems) { if (!pred.getAccessTypes().isEmpty() && !pred.hasAccessType(policyItem)) { continue; } if (policyItem.getUsers() != null) { policyItem.getUsers().stream().filter(user -> !pred.hasPrincipalFilter() || pred.getUsers().contains(user)).forEach(user -> ret.add(new RangerGrant(PrincipalType.USER, user, policyItem.getAccesses(), policyItem.getConditions()))); } if (policyItem.getGroups() != null) { policyItem.getGroups().stream().filter(group -> !pred.hasPrincipalFilter() || pred.getGroups().contains(group)).forEach(group -> ret.add(new RangerGrant(PrincipalType.GROUP, group, policyItem.getAccesses(), policyItem.getConditions()))); } if (policyItem.getRoles() != null) { policyItem.getRoles().stream().filter(role -> !pred.hasPrincipalFilter() || pred.getRoles().contains(role)).forEach(role -> ret.add(new RangerGrant(PrincipalType.ROLE, role, policyItem.getAccesses(), policyItem.getConditions()))); } } return ret; } - Madhan Neethiraj On Oct. 31, 2024, 7:36 p.m., Radhika Kundam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/75252/ > ----------------------------------------------------------- > > (Updated Oct. 31, 2024, 7:36 p.m.) > > > Review request for ranger, Madhan Neethiraj and Ramesh Mani. > > > Bugs: RANGER-4960 > https://issues.apache.org/jira/browse/RANGER-4960 > > > Repository: ranger > > > Description > ------- > > This Grant API introduces functionality to grant or revoke specific > permissions on datasets to external users, groups, or roles. It includes > support for defining access validity based on policy item conditions. > > Key features include: > 1. Permission Management: Enables granting and revoking access to datasets > for designated users, groups, or roles. > 2. Policy-Based Validity: Allows setting validity periods and conditions > within policy items, defining the scope and duration of access for each user, > group, or role. > > This API enhancement provides flexibility in managing dataset permissions, > improving security and control over data access. > > Attached file for Grant API UseCases. > > Detailed information about Grant API request and response attached to Jira. > > > Diffs > ----- > > agents-common/src/main/java/org/apache/ranger/plugin/model/RangerGds.java > 68216fd45 > > agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyHeader.java > PRE-CREATION > security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java > a6c6746b3 > security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java c66429834 > > security-admin/src/main/java/org/apache/ranger/security/context/RangerAPIList.java > acfce5f0a > > > Diff: https://reviews.apache.org/r/75252/diff/1/ > > > Testing > ------- > > Tested locally. > > > File Attachments > ---------------- > > Grant API UseCases > > https://reviews.apache.org/media/uploaded/files/2024/10/31/9e0f7209-36b8-4779-9336-63052015a117__Grant_API_UseCases.pdf > > > Thanks, > > Radhika Kundam > >
