Skip to content

Commit

Permalink
Review comments: refactor for readability
Browse files Browse the repository at this point in the history
  • Loading branch information
@fateh288
fateh288 committed Nov 4, 2024
1 parent 4e54841 commit 0406897
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 309 deletions.
1 change: 0 additions & 1 deletion ...src/main/java/org/apache/ranger/authorization/hadoop/constants/RangerHadoopConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public class RangerHadoopConstants {
public static final boolean HBASE_UPDATE_RANGER_POLICIES_ON_GRANT_REVOKE_DEFAULT_VALUE = true;
public static final String HBASE_COLUMN_AUTH_OPTIMIZATION = "ranger.plugin.hbase.column.auth.optimization";


public static final String KNOX_ACCESS_VERIFIER_CLASS_NAME_PROP = "knox.authorization.verifier.classname";
public static final String KNOX_ACCESS_VERIFIER_CLASS_NAME_DEFAULT_VALUE = "org.apache.ranger.pdp.knox.RangerAuthorizer";

Expand Down
2 changes: 1 addition & 1 deletion agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ default Map<String, ResourceElementMatchingScope> getResourceElementMatchingScop
return Collections.emptyMap();
}

enum ResourceMatchingScope { SELF, SELF_OR_DESCENDANTS, SELF_AND_ALL_DESCENDANTS}
enum ResourceMatchingScope { SELF, SELF_OR_DESCENDANTS, SELF_AND_ALL_DESCENDANTS }

enum ResourceElementMatchingScope { SELF, SELF_OR_CHILD, SELF_OR_PREFIX }

Expand Down
53 changes: 26 additions & 27 deletions .../src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,8 @@ public void evaluate(RangerAccessRequest request, RangerAccessResult result) {
matchType = resourceMatcher != null ? resourceMatcher.getMatchType(request.getResource(), request.getResourceElementMatchingScopes(), request.getContext()) : RangerPolicyResourceMatcher.MatchType.NONE;
}

final boolean isMatchedResource;
isMatchedResource = isMatchForResourceMatchingScope(request.getResourceMatchingScope(), matchType, request.isAccessTypeAny());

if (isMatchedResource) {
final boolean isMatchedResource = isMatchForResourceMatchingScope(request.getResourceMatchingScope(), matchType, request.isAccessTypeAny());
if ( isMatchedResource ) {
//Evaluate Policy Level Custom Conditions, if any and allowed then go ahead for policyItem level evaluation
if (matchPolicyCustomConditions(request)) {
if (!result.getIsAuditedDetermined()) {
Expand All @@ -260,29 +258,7 @@ public void evaluate(RangerAccessRequest request, RangerAccessResult result) {
LOG.debug("<== RangerDefaultPolicyEvaluator.evaluate(policyId=" + getPolicyId() + ", " + request + ", " + result + ")");
}
}
private boolean isMatchForResourceMatchingScope(final RangerAccessRequest.ResourceMatchingScope scope, final RangerPolicyResourceMatcher.MatchType matchType, boolean isAnyMatch) {
if (isAnyMatch){
return matchType != RangerPolicyResourceMatcher.MatchType.NONE;
}
boolean ret = false;
if (scope!=null){
switch (scope) {
case SELF_OR_DESCENDANTS: {
ret = matchType != RangerPolicyResourceMatcher.MatchType.NONE;
break;
}
case SELF_AND_ALL_DESCENDANTS: {
ret = matchType != RangerPolicyResourceMatcher.MatchType.NONE;
break;
}
default: {
ret = matchType == RangerPolicyResourceMatcher.MatchType.SELF || matchType == RangerPolicyResourceMatcher.MatchType.SELF_AND_ALL_DESCENDANTS;
break;
}
}
}
return ret;
}

@Override
public boolean isMatch(RangerAccessResource resource, Map<String, Object> evalContext) {
if(LOG.isDebugEnabled()) {
Expand Down Expand Up @@ -1528,4 +1504,27 @@ private List<RangerConditionEvaluator> createPolicyConditionEvaluators(RangerPol
return ret;
}

private boolean isMatchForResourceMatchingScope(final RangerAccessRequest.ResourceMatchingScope scope, final RangerPolicyResourceMatcher.MatchType matchType, boolean isAnyMatch) {
boolean ret = false;
if (isAnyMatch){
ret = matchType != RangerPolicyResourceMatcher.MatchType.NONE;
}
else if (scope!=null) {
switch (scope) {
case SELF_OR_DESCENDANTS: {
ret = matchType != RangerPolicyResourceMatcher.MatchType.NONE;
break;
}
case SELF_AND_ALL_DESCENDANTS: {
ret = matchType != RangerPolicyResourceMatcher.MatchType.NONE;
break;
}
default: {
ret = matchType == RangerPolicyResourceMatcher.MatchType.SELF || matchType == RangerPolicyResourceMatcher.MatchType.SELF_AND_ALL_DESCENDANTS;
break;
}
}
}
return ret;
}
}
6 changes: 3 additions & 3 deletions hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/AuthorizationSession.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ boolean isNameSpaceOperation() {
StringUtils.equals(_operation, "getUserPermissionForNamespace");
}

private RangerAccessResource createHBaseResource(){
private RangerAccessResource createHBaseResource() {
// TODO get this via a factory instead
RangerAccessResourceImpl resource = new RangerHBaseResource();
// policy engine should deal sensibly with null/empty values, if any
Expand All @@ -181,7 +181,7 @@ private RangerAccessResource createHBaseResource(){
resource.setValue(RangerHBaseResource.KEY_COLUMN, _column);
return resource;
}
private RangerAccessRequest createRangerRequest(){
private RangerAccessRequest createRangerRequest() {
RangerAccessResource resource = createHBaseResource();
String user = _userUtils.getUserAsString(_user);
RangerAccessRequestImpl request = new RangerAccessRequestImpl(resource, _access, user, _groups, null);
Expand Down Expand Up @@ -378,7 +378,7 @@ AuthorizationSession resourceMatchingScope(RangerAccessRequest.ResourceMatchingS
_resourceMatchingScope = scope;
return this;
}
public boolean getPropertyIsColumnAuthOptimizationEnabled(){
public boolean getPropertyIsColumnAuthOptimizationEnabled() {
return _authorizer.getPropertyIsColumnAuthOptimizationEnabled();
}
}
17 changes: 8 additions & 9 deletions ...t/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ public class RangerAuthorizationCoprocessor implements AccessControlService.Inte
final HbaseAuthUtils _authUtils = _factory.getAuthUtils();
private static volatile RangerHBasePlugin hbasePlugin = null;


public void setColumnAuthOptimizationEnabled(boolean enable) throws Exception {
if (hbasePlugin!=null) {
hbasePlugin.setColumnAuthOptimizationEnabled(enable);
Expand Down Expand Up @@ -270,13 +269,13 @@ Map<String, Set<String>> getColumnFamilies(Map<byte[], ? extends Collection<?>>
if (CollectionUtils.isEmpty(columnCollection)) {
// family points to null map, OK.
// if column auth disabled, then also empty set is fine
if (LOG.isDebugEnabled()){
if (LOG.isDebugEnabled()) {
LOG.debug("RangerAuthorizationCoprocessor getColumnFamilies: columns are empty. " +
"Setting columns to emptySet in familyMap");
}
result.put(family, Collections.<String> emptySet());
} else {
if (LOG.isDebugEnabled()){
if (LOG.isDebugEnabled()) {
LOG.debug("RangerAuthorizationCoprocessor getColumnFamilies: columns exist");
}
Iterator<String> columnIterator = new ColumnIterator(columnCollection);
Expand Down Expand Up @@ -338,7 +337,7 @@ public String toString() {

ColumnFamilyAccessResult evaluateAccess(ObserverContext<?> ctx, String operation, Action action, final RegionCoprocessorEnvironment env,
final Map<byte[], ? extends Collection<?>> familyMap, String commandStr) throws AccessDeniedException {
if (LOG.isDebugEnabled()){
if (LOG.isDebugEnabled()) {
LOG.debug("evaluateAccess: isColumnAuthOptimizationEnabled="+hbasePlugin.getPropertyIsColumnAuthOptimizationEnabled());
}

Expand Down Expand Up @@ -443,7 +442,7 @@ ColumnFamilyAccessResult evaluateAccess(ObserverContext<?> ctx, String operation
if (columns == null || columns.isEmpty()) {
// family points to null map, OK.
// if column auth disabled, then also empty set is fine
if (LOG.isDebugEnabled()){
if (LOG.isDebugEnabled()) {
LOG.debug("RangerAuthorizationCoprocessor evaluateAccess: (No tags found for resource, "+
" all policies are * at column level for resource and "+
RangerHadoopConstants.HBASE_COLUMN_AUTH_OPTIMIZATION +
Expand Down Expand Up @@ -524,7 +523,7 @@ ColumnFamilyAccessResult evaluateAccess(ObserverContext<?> ctx, String operation
" Skipping Family level check, will do finer level access check.");
}
boolean isColumnAuthShortCircuitingEnabled = hbasePlugin.getPropertyIsColumnAuthOptimizationEnabled();
if (isColumnAuthShortCircuitingEnabled){
if (isColumnAuthShortCircuitingEnabled) {
session.column(null)
.resourceMatchingScope(RangerAccessRequest.ResourceMatchingScope.SELF_AND_ALL_DESCENDANTS)
.buildRequest()
Expand All @@ -533,10 +532,10 @@ ColumnFamilyAccessResult evaluateAccess(ObserverContext<?> ctx, String operation
AuthzAuditEvent auditEvent = auditHandler.getAndDiscardMostRecentEvent();
// reset ResourceMatchingScope to SELF
session.resourceMatchingScope(RangerAccessRequest.ResourceMatchingScope.SELF);
if (LOG.isDebugEnabled()){
if (LOG.isDebugEnabled()) {
LOG.debug("evaluateAccess: isColumnAuthShortCircuitingEnabled=true, isColumnFamilyAndDescendantsAuthorized={}",isColumnFamilyAndDescendantsAuthorized);
}
if (isColumnFamilyAndDescendantsAuthorized){
if (isColumnFamilyAndDescendantsAuthorized) {
familesFullyAuthorized.add(family);
if (auditEvent != null) {
if (LOG.isDebugEnabled()) {
Expand Down Expand Up @@ -1953,7 +1952,7 @@ public RangerAccessResult isAccessAllowed(RangerAccessRequest request, RangerAcc
return ret;
}
@Override
public void setPolicies(ServicePolicies policies){
public void setPolicies(ServicePolicies policies) {
super.setPolicies(policies);
this.serviceConfigs = policies.getServiceConfig();
this.isColumnAuthOptimizationEnabled = Boolean.parseBoolean(this.serviceConfigs.get(RangerHadoopConstants.HBASE_COLUMN_AUTH_OPTIMIZATION));
Expand Down
10 changes: 4 additions & 6 deletions ...-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,12 @@ public ReturnCode filterKeyValue(Cell kv) throws IOException {
boolean authCheckNeeded = false;
if (family == null) {
LOG.warn("filterKeyValue: Unexpected - null/empty family! Access denied!");
}
else if (_familiesAccessDenied.contains(family)) {
} else if (_familiesAccessDenied.contains(family)) {
LOG.debug("filterKeyValue: family found in access denied families cache. Access denied.");
}
else if (_session.getPropertyIsColumnAuthOptimizationEnabled() && _familiesFullyAuthorized.contains(family)){
} else if (_session.getPropertyIsColumnAuthOptimizationEnabled() && _familiesFullyAuthorized.contains(family)){
LOG.debug("filterKeyValue: ColumnAuthOptimizationEnabled and family found in fully authorized families cache. Column authorization is not required");
result = ReturnCode.INCLUDE;
}
else if (_columnsAccessAllowed.containsKey(family)) {
} else if (_columnsAccessAllowed.containsKey(family)) {
LOG.debug("filterKeyValue: family found in column level access results cache.");
if (_columnsAccessAllowed.get(family).contains(column)) {
LOG.debug("filterKeyValue: family/column found in column level access results cache. Access allowed.");
Expand All @@ -114,6 +111,7 @@ else if (_columnsAccessAllowed.containsKey(family)) {
} else {
LOG.warn("filterKeyValue: Unexpected - alien family encountered that wasn't seen by pre-hook! Access Denied.!");
}

if (authCheckNeeded) {
LOG.debug("filterKeyValue: Checking authorization...");
_session.columnFamily(family)
Expand Down
Loading

0 comments on commit 0406897

Please sign in to comment.