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: dev-unsubscr...@ranger.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to