vyommani commented on code in PR #1201:
URL: https://github.com/apache/ranger/pull/1201#discussion_r4011997616


##########
hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java:
##########
@@ -1434,13 +1434,34 @@ boolean canSkipAccessCheck(User user, final String 
operation, String access, fin
 
     /* ---- EndpointObserver implementation ---- */
 
-    boolean isAccessForMetadataRead(String access, String table) {
+    boolean isAccessForMetadataRead(String access, String table, User user) {
         if (authUtils.isReadAccess(access) && isSpecialTable(table)) {
-            LOG.debug("isAccessForMetadataRead: Metadata tables read: access 
allowed!");
+            if (StringUtils.equals(table, "hbase:acl")) {

Review Comment:
   This duplicates the system-or-super-user predicate already in 
requireSystemOrSuperUser(). Can we extract it
   boolean isSystemOrSuperUser(User user) {
       if (user == null) return false;
       if (userUtils.isSuperUser(user)) return true;
       try {
           User loginUser = User.getCurrent();
           return loginUser != null && Objects.equals(loginUser.getShortName(), 
user.getShortName());
       } catch (IOException excp) {
           LOG.warn("isSystemOrSuperUser: unable to obtain login user; treating 
[{}] as non-system", user.getShortName(), excp);
           return false;
       }
   }
   and have both callers use it? Then this method reduces to if 
(ACL_TABLE_NAME.equals(table) && !isSystemOrSuperUser(user)) return false;. 
Also use PermissionStorage.ACL_TABLE_NAME.getNameAsString() instead of the 
literal



-- 
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]

Reply via email to