mneethiraj commented on code in PR #988:
URL: https://github.com/apache/ranger/pull/988#discussion_r3342789955
##########
hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java:
##########
@@ -2843,11 +2846,187 @@ private RangerResourceACLs
getRangerResourceACLs(HivePrivilegeObject hiveObject,
ret = hivePlugin.getResourceACLs(request);
+ if (ret != null && principal != null && request != null) {
+ filterAclsByValiditySchedule(ret, principal, request);
+ }
+
LOG.debug("<== RangerHivePolicyProvider.getRangerResourceACLs:[{}],
principal=[{}], Computed ACLS:[{}]", hiveObject, principal, ret);
return ret;
}
+ private void filterAclsByValiditySchedule(RangerResourceACLs acls,
HivePrincipal principal, RangerAccessRequest request) {
+ if (acls == null || principal == null || request == null) {
+ return;
+ }
+
+ Map<String, Map<String, RangerResourceACLs.AccessResult>>
principalACLs = null;
+ switch (principal.getType()) {
+ case USER:
+ principalACLs = acls.getUserACLs();
+ break;
+ case GROUP:
+ principalACLs = acls.getGroupACLs();
+ break;
+ case ROLE:
+ principalACLs = acls.getRoleACLs();
+ break;
+ default:
+ break;
+ }
+
+ if (principalACLs != null) {
+ Map<String, RangerResourceACLs.AccessResult> accessMap =
principalACLs.get(principal.getName());
+ if (accessMap != null) {
+ Map<String, Collection<String>> impliedGrants = null;
+ Map<String, Collection<String>> gdsImpliedGrants = null;
+
+ if (hivePlugin != null) {
+ RangerServiceDef serviceDef = hivePlugin.getServiceDef();
+ if (serviceDef != null) {
Review Comment:
Consider replacing this `if` block by initializing `impliedGrants` in line
2881 with:
```
final Map<String, Collection<String>> impliedGrants =
hivePlugin.getServiceDefHelper() != null ?
hivePlugin.getServiceDefHelper().getImpliedAccessGrants() :
Collections.emptyMap();
```
##########
hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java:
##########
@@ -2843,11 +2846,187 @@ private RangerResourceACLs
getRangerResourceACLs(HivePrivilegeObject hiveObject,
ret = hivePlugin.getResourceACLs(request);
+ if (ret != null && principal != null && request != null) {
+ filterAclsByValiditySchedule(ret, principal, request);
+ }
+
LOG.debug("<== RangerHivePolicyProvider.getRangerResourceACLs:[{}],
principal=[{}], Computed ACLS:[{}]", hiveObject, principal, ret);
return ret;
}
+ private void filterAclsByValiditySchedule(RangerResourceACLs acls,
HivePrincipal principal, RangerAccessRequest request) {
+ if (acls == null || principal == null || request == null) {
+ return;
+ }
+
+ Map<String, Map<String, RangerResourceACLs.AccessResult>>
principalACLs = null;
+ switch (principal.getType()) {
+ case USER:
+ principalACLs = acls.getUserACLs();
+ break;
+ case GROUP:
+ principalACLs = acls.getGroupACLs();
+ break;
+ case ROLE:
+ principalACLs = acls.getRoleACLs();
+ break;
+ default:
+ break;
+ }
+
+ if (principalACLs != null) {
+ Map<String, RangerResourceACLs.AccessResult> accessMap =
principalACLs.get(principal.getName());
+ if (accessMap != null) {
+ Map<String, Collection<String>> impliedGrants = null;
+ Map<String, Collection<String>> gdsImpliedGrants = null;
+
+ if (hivePlugin != null) {
Review Comment:
`hivePlugin` will not null here given the caller already references it at
line 2847. I suggest removing the `if` at 2884.
##########
hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java:
##########
@@ -2843,11 +2846,187 @@ private RangerResourceACLs
getRangerResourceACLs(HivePrivilegeObject hiveObject,
ret = hivePlugin.getResourceACLs(request);
+ if (ret != null && principal != null && request != null) {
+ filterAclsByValiditySchedule(ret, principal, request);
+ }
+
LOG.debug("<== RangerHivePolicyProvider.getRangerResourceACLs:[{}],
principal=[{}], Computed ACLS:[{}]", hiveObject, principal, ret);
return ret;
}
+ private void filterAclsByValiditySchedule(RangerResourceACLs acls,
HivePrincipal principal, RangerAccessRequest request) {
+ if (acls == null || principal == null || request == null) {
+ return;
+ }
+
+ Map<String, Map<String, RangerResourceACLs.AccessResult>>
principalACLs = null;
+ switch (principal.getType()) {
+ case USER:
+ principalACLs = acls.getUserACLs();
+ break;
+ case GROUP:
+ principalACLs = acls.getGroupACLs();
+ break;
+ case ROLE:
+ principalACLs = acls.getRoleACLs();
+ break;
+ default:
+ break;
+ }
+
+ if (principalACLs != null) {
+ Map<String, RangerResourceACLs.AccessResult> accessMap =
principalACLs.get(principal.getName());
+ if (accessMap != null) {
+ Map<String, Collection<String>> impliedGrants = null;
+ Map<String, Collection<String>> gdsImpliedGrants = null;
+
+ if (hivePlugin != null) {
+ RangerServiceDef serviceDef = hivePlugin.getServiceDef();
+ if (serviceDef != null) {
+ impliedGrants =
ServiceDefUtil.getExpandedImpliedGrants(serviceDef);
+ }
+
+ GdsPolicyEngine gdsPolicyEngine =
hivePlugin.getGdsPolicyEngine();
+ if (gdsPolicyEngine != null &&
gdsPolicyEngine.getGdsInfo() != null) {
+ RangerServiceDef gdsServiceDef =
gdsPolicyEngine.getGdsInfo().getGdsServiceDef();
Review Comment:
Consider updating `GdsPolicyEngine` to expose `RangerServiceDefHelper` and
use it to initialize `gdsImpliedGrants` in line 2882.
```
final Map<String, Collection<String>> gdsImpliedGrants = gdsPolicyEngine !=
null && gdsPolicyEngine.getServiceDefHelper() != null ?
gdsPolicyEngine.getServiceDefHelper().getImpliedAccessGrants() :
Collections.emptyMap();
```
##########
hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java:
##########
@@ -2843,11 +2846,187 @@ private RangerResourceACLs
getRangerResourceACLs(HivePrivilegeObject hiveObject,
ret = hivePlugin.getResourceACLs(request);
+ if (ret != null && principal != null && request != null) {
+ filterAclsByValiditySchedule(ret, principal, request);
+ }
+
LOG.debug("<== RangerHivePolicyProvider.getRangerResourceACLs:[{}],
principal=[{}], Computed ACLS:[{}]", hiveObject, principal, ret);
return ret;
}
+ private void filterAclsByValiditySchedule(RangerResourceACLs acls,
HivePrincipal principal, RangerAccessRequest request) {
+ if (acls == null || principal == null || request == null) {
+ return;
+ }
+
+ Map<String, Map<String, RangerResourceACLs.AccessResult>>
principalACLs = null;
+ switch (principal.getType()) {
+ case USER:
+ principalACLs = acls.getUserACLs();
+ break;
+ case GROUP:
+ principalACLs = acls.getGroupACLs();
+ break;
+ case ROLE:
+ principalACLs = acls.getRoleACLs();
+ break;
+ default:
+ break;
+ }
+
+ if (principalACLs != null) {
+ Map<String, RangerResourceACLs.AccessResult> accessMap =
principalACLs.get(principal.getName());
+ if (accessMap != null) {
+ Map<String, Collection<String>> impliedGrants = null;
+ Map<String, Collection<String>> gdsImpliedGrants = null;
+
+ if (hivePlugin != null) {
+ RangerServiceDef serviceDef = hivePlugin.getServiceDef();
+ if (serviceDef != null) {
+ impliedGrants =
ServiceDefUtil.getExpandedImpliedGrants(serviceDef);
+ }
+
+ GdsPolicyEngine gdsPolicyEngine =
hivePlugin.getGdsPolicyEngine();
+ if (gdsPolicyEngine != null &&
gdsPolicyEngine.getGdsInfo() != null) {
+ RangerServiceDef gdsServiceDef =
gdsPolicyEngine.getGdsInfo().getGdsServiceDef();
+ if (gdsServiceDef != null) {
+ gdsImpliedGrants =
ServiceDefUtil.getExpandedImpliedGrants(gdsServiceDef);
+ }
+ }
+ }
+
+ Map<RangerPolicy.RangerPolicyItem, Boolean>
policyItemActiveCache = new HashMap<>();
+ List<String> keysToRemove = new ArrayList<>();
Review Comment:
`keysToRemove` ==> `permissionsToRemove`
##########
hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java:
##########
@@ -2843,11 +2846,187 @@ private RangerResourceACLs
getRangerResourceACLs(HivePrivilegeObject hiveObject,
ret = hivePlugin.getResourceACLs(request);
+ if (ret != null && principal != null && request != null) {
+ filterAclsByValiditySchedule(ret, principal, request);
+ }
+
LOG.debug("<== RangerHivePolicyProvider.getRangerResourceACLs:[{}],
principal=[{}], Computed ACLS:[{}]", hiveObject, principal, ret);
return ret;
}
+ private void filterAclsByValiditySchedule(RangerResourceACLs acls,
HivePrincipal principal, RangerAccessRequest request) {
+ if (acls == null || principal == null || request == null) {
+ return;
+ }
+
+ Map<String, Map<String, RangerResourceACLs.AccessResult>>
principalACLs = null;
+ switch (principal.getType()) {
+ case USER:
+ principalACLs = acls.getUserACLs();
+ break;
+ case GROUP:
+ principalACLs = acls.getGroupACLs();
+ break;
+ case ROLE:
+ principalACLs = acls.getRoleACLs();
+ break;
+ default:
+ break;
+ }
+
+ if (principalACLs != null) {
+ Map<String, RangerResourceACLs.AccessResult> accessMap =
principalACLs.get(principal.getName());
+ if (accessMap != null) {
+ Map<String, Collection<String>> impliedGrants = null;
+ Map<String, Collection<String>> gdsImpliedGrants = null;
+
+ if (hivePlugin != null) {
+ RangerServiceDef serviceDef = hivePlugin.getServiceDef();
+ if (serviceDef != null) {
+ impliedGrants =
ServiceDefUtil.getExpandedImpliedGrants(serviceDef);
+ }
+
+ GdsPolicyEngine gdsPolicyEngine =
hivePlugin.getGdsPolicyEngine();
+ if (gdsPolicyEngine != null &&
gdsPolicyEngine.getGdsInfo() != null) {
+ RangerServiceDef gdsServiceDef =
gdsPolicyEngine.getGdsInfo().getGdsServiceDef();
+ if (gdsServiceDef != null) {
+ gdsImpliedGrants =
ServiceDefUtil.getExpandedImpliedGrants(gdsServiceDef);
+ }
+ }
+ }
+
+ Map<RangerPolicy.RangerPolicyItem, Boolean>
policyItemActiveCache = new HashMap<>();
+ List<String> keysToRemove = new ArrayList<>();
+ for (Map.Entry<String, RangerResourceACLs.AccessResult> entry
: accessMap.entrySet()) {
+ RangerResourceACLs.AccessResult accessResult =
entry.getValue();
+ if (accessResult != null && accessResult.getResult() ==
RangerPolicyEvaluator.ACCESS_CONDITIONAL) {
+ RangerPolicy policy = accessResult.getPolicy();
+ if (policy != null) {
+ boolean hasActiveItem = false;
+
+ List<RangerPolicy.RangerPolicyItem> policyItems =
new ArrayList<>();
+ if
(CollectionUtils.isNotEmpty(policy.getPolicyItems())) {
+ policyItems.addAll(policy.getPolicyItems());
+ }
+ if
(CollectionUtils.isNotEmpty(policy.getDenyPolicyItems())) {
+
policyItems.addAll(policy.getDenyPolicyItems());
+ }
+ if
(CollectionUtils.isNotEmpty(policy.getAllowExceptions())) {
+
policyItems.addAll(policy.getAllowExceptions());
+ }
+ if
(CollectionUtils.isNotEmpty(policy.getDenyExceptions())) {
+ policyItems.addAll(policy.getDenyExceptions());
+ }
+
+ for (RangerPolicy.RangerPolicyItem policyItem :
policyItems) {
+ if (isPolicyItemApplicable(policyItem,
principal, entry.getKey(), impliedGrants, gdsImpliedGrants)) {
+ Boolean isActive =
policyItemActiveCache.get(policyItem);
+ if (isActive == null) {
+ isActive =
isPolicyItemValidityActive(policyItem, request);
+ policyItemActiveCache.put(policyItem,
isActive);
+ }
+ if (isActive) {
+ hasActiveItem = true;
+ break;
+ }
+ }
+ }
+ if (!hasActiveItem) {
+ keysToRemove.add(entry.getKey());
Review Comment:
@rameeshm - if multiple policies grant this permission to the principal,
removing the permission based on just one policy (line 2904) will result in
incorrect ACLs.
--
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]
