mneethiraj commented on code in PR #417:
URL: https://github.com/apache/ranger/pull/417#discussion_r1866737222
##########
agents-common/src/main/java/org/apache/ranger/authorization/hadoop/constants/RangerHadoopConstants.java:
##########
@@ -51,6 +51,7 @@ public class RangerHadoopConstants {
public static final String
HBASE_UPDATE_RANGER_POLICIES_ON_GRANT_REVOKE_PROP =
"xasecure.hbase.update.xapolicies.on.grant.revoke";
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";
Review Comment:
`ranger.plugin.hbase.column.auth.optimization` =>
`ranger.plugin.hbase.column.auth.optimized`
##########
hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java:
##########
@@ -90,19 +91,28 @@ public class RangerAuthorizationCoprocessor implements
AccessControlService.Inte
private static final String GROUP_PREFIX = "@";
private UserProvider userProvider;
- private RegionCoprocessorEnvironment regionEnv;
+ private RegionCoprocessorEnvironment regionEnv;
private Map<InternalScanner, String> scannerOwners = new
MapMaker().weakKeys().makeMap();
/** if we should check EXEC permissions */
private boolean shouldCheckExecPermission;
-
+
/*
* These are package level only for testability and aren't meant to be
exposed outside via getters/setters or made available to derived classes.
*/
final HbaseFactory _factory = HbaseFactory.getInstance();
final HbaseUserUtils _userUtils = _factory.getUserUtils();
final HbaseAuthUtils _authUtils = _factory.getAuthUtils();
private static volatile RangerHBasePlugin hbasePlugin = null;
-
+
+ public void setColumnAuthOptimizationEnabled(boolean enable) throws
Exception {
+ if (hbasePlugin!=null) {
Review Comment:
Assign RangerAuthorizationCoprocessor. hbasePlugin to a local variable
before accessing it:
```
RangerHBasePlugin plugn = RangerAuthorizationCoprocessor.hbasePlugin;
if (plugin != null) {
plugin. setColumnAuthOptimizationEnabled(enable);
}
```
##########
hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java:
##########
@@ -491,14 +515,52 @@ ColumnFamilyAccessResult
evaluateAccess(ObserverContext<?> ctx, String operation
session.resourceMatchingScope(RangerAccessRequest.ResourceMatchingScope.SELF);
session.ignoreDescendantDeny(true);
} else {
- LOG.debug("evaluateAccess: columns collection
not empty. Skipping Family level check, will do finer level access check.");
+ boolean isColumnAuthShortCircuitingEnabled =
hbasePlugin.getPropertyIsColumnAuthOptimizationEnabled();
Review Comment:
To be consistent, rename: `isColumnAuthShortCircuitingEnabled` =>
`isColumnAuthOptimizationEnabled`
##########
hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java:
##########
@@ -491,14 +515,52 @@ ColumnFamilyAccessResult
evaluateAccess(ObserverContext<?> ctx, String operation
session.resourceMatchingScope(RangerAccessRequest.ResourceMatchingScope.SELF);
session.ignoreDescendantDeny(true);
} else {
- LOG.debug("evaluateAccess: columns collection
not empty. Skipping Family level check, will do finer level access check.");
+ boolean isColumnAuthShortCircuitingEnabled =
hbasePlugin.getPropertyIsColumnAuthOptimizationEnabled();
+ if(LOG.isDebugEnabled()) {
+ LOG.debug("evaluateAccess: columns
collection not empty." +
+ " Skipping Family level
check, will do finer level access check for columns.");
+ }
+ if (isColumnAuthShortCircuitingEnabled) {
+ session.column(null)
+ .buildRequest()
+ .authorize();
+
+ boolean isColumnFamilyAuthorized =
session.isAuthorized();
+
+ //check if column family fully
authorized i.e. no deny for columns
+ session.column(null)
+
.resourceMatchingScope(RangerAccessRequest.ResourceMatchingScope.SELF_OR_DESCENDANTS)
+
.ignoreDescendantDeny(false)
+ .buildRequest()
+ .authorize();
+
+ boolean
isColumnFamilyAndDescendantsAuthorized = session.isAuthorized() &&
isColumnFamilyAuthorized;
+ AuthzAuditEvent auditEvent =
auditHandler.getAndDiscardMostRecentEvent();
+ // reset ResourceMatchingScope to SELF,
ignoreDescendantDeny to true
+
session.resourceMatchingScope(RangerAccessRequest.ResourceMatchingScope.SELF)
+
.ignoreDescendantDeny(true);
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("evaluateAccess:
isColumnAuthShortCircuitingEnabled=true,
isColumnFamilyAndDescendantsAuthorized={}",isColumnFamilyAndDescendantsAuthorized);
+ }
+ if
(isColumnFamilyAndDescendantsAuthorized) {
+
familiesFullyAuthorized.add(family);
+ if (auditEvent != null) {
+ if
(LOG.isDebugEnabled()) {
+
LOG.debug("evaluateAccess: isColumnAuthShortCircuitingEnabled=true, adding to
familiesFullyAuthorized set");
Review Comment:
Include `family` in the log message, to be more useful.
```
LOG.debug("evaluateAccess: isColumnAuthOptimizationEnabled ={}, adding
family {} to familiesFullyAuthorized", isColumnAuthOptimizationEnabled, family);
```
##########
hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java:
##########
@@ -491,14 +515,52 @@ ColumnFamilyAccessResult
evaluateAccess(ObserverContext<?> ctx, String operation
session.resourceMatchingScope(RangerAccessRequest.ResourceMatchingScope.SELF);
session.ignoreDescendantDeny(true);
} else {
- LOG.debug("evaluateAccess: columns collection
not empty. Skipping Family level check, will do finer level access check.");
+ boolean isColumnAuthShortCircuitingEnabled =
hbasePlugin.getPropertyIsColumnAuthOptimizationEnabled();
+ if(LOG.isDebugEnabled()) {
+ LOG.debug("evaluateAccess: columns
collection not empty." +
+ " Skipping Family level
check, will do finer level access check for columns.");
+ }
+ if (isColumnAuthShortCircuitingEnabled) {
+ session.column(null)
+ .buildRequest()
+ .authorize();
+
+ boolean isColumnFamilyAuthorized =
session.isAuthorized();
+
+ //check if column family fully
authorized i.e. no deny for columns
+ session.column(null)
Review Comment:
Can the second authz, starting with line 531, be skipped when
`isColumnFamilyAuthorized` is `false`?
```
boolean isColumnFamilyAndDescendantsAuthorized = false;
if (session.isAuthorized()) {
session.column(null)
.resourceMatchingScope(RangerAccessRequest.ResourceMatchingScope.SELF_OR_DESCENDANTS)
.ignoreDescendantDeny(false)
.buildRequest()
.authorize();
isColumnFamilyAndDescendantsAuthorized = session.isAuthorized();
}
--
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]