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