[ 
https://issues.apache.org/jira/browse/HDDS-1927?focusedWorklogId=298294&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-298294
 ]

ASF GitHub Bot logged work on HDDS-1927:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 20/Aug/19 23:05
            Start Date: 20/Aug/19 23:05
    Worklog Time Spent: 10m 
      Work Description: xiaoyuyao commented on pull request #1263: HDDS-1927. 
Consolidate add/remove Acl into OzoneAclUtil class. Contri…
URL: https://github.com/apache/hadoop/pull/1263#discussion_r315938903
 
 

 ##########
 File path: 
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java
 ##########
 @@ -0,0 +1,311 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.helpers;
+
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import 
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
+import org.apache.hadoop.ozone.security.acl.RequestContext;
+
+import java.util.ArrayList;
+import java.util.BitSet;
+import java.util.Collection;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS;
+import static org.apache.hadoop.ozone.OzoneAcl.AclScope.DEFAULT;
+import static 
org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.GROUP;
+import static 
org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.USER;
+import static 
org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.ALL;
+import static 
org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.NONE;
+
+/**
+ * Helper class for ozone acls operations.
+ */
+public final class OzoneAclUtil {
+
+  private OzoneAclUtil(){
+  }
+
+  /**
+   * Helper function to get access acl list for current user.
+   *
+   * @param userName
+   * @param userGroups
+   * @return list of OzoneAcls
+   * */
+  public static List<OzoneAcl> getAclList(String userName,
+      List<String> userGroups, ACLType userRights, ACLType groupRights) {
+
+    List<OzoneAcl> listOfAcls = new ArrayList<>();
+
+    // User ACL.
+    listOfAcls.add(new OzoneAcl(USER, userName, userRights, ACCESS));
+    if(userGroups != null) {
+      // Group ACLs of the User.
+      userGroups.forEach((group) -> listOfAcls.add(
+          new OzoneAcl(GROUP, group, groupRights, ACCESS)));
+    }
+    return listOfAcls;
+  }
+
+  /**
+   * Check if acl right requested for given RequestContext exist
+   * in provided acl list.
+   * Acl validation rules:
+   * 1. If user/group has ALL bit set than all user should have all rights.
+   * 2. If user/group has NONE bit set than user/group will not have any right.
+   * 3. For all other individual rights individual bits should be set.
+   *
+   * @param acls
+   * @param context
+   * @return return true if acl list contains right requsted in context.
+   * */
+  public static boolean checkAclRight(List<OzoneAcl> acls,
+      RequestContext context) throws OMException {
+    String[] userGroups = context.getClientUgi().getGroupNames();
+    String userName = context.getClientUgi().getUserName();
+    ACLType aclToCheck = context.getAclRights();
+    for (OzoneAcl a : acls) {
+      if(checkAccessInAcl(a, userGroups, userName, aclToCheck)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  private static boolean checkAccessInAcl(OzoneAcl a, String[] groups,
+      String username, ACLType aclToCheck) {
+    BitSet rights = a.getAclBitSet();
+    switch (a.getType()) {
+    case USER:
+      if (a.getName().equals(username)) {
+        return checkIfAclBitIsSet(aclToCheck, rights);
+      }
+      break;
+    case GROUP:
+      for (String grp : groups) {
+        if (a.getName().equals(grp)) {
+          return checkIfAclBitIsSet(aclToCheck, rights);
+        }
+      }
+      break;
+
+    default:
+      return checkIfAclBitIsSet(aclToCheck, rights);
+    }
+    return false;
+  }
+
+  /**
+   * Check if acl right requested for given RequestContext exist
+   * in provided acl list.
+   * Acl validation rules:
+   * 1. If user/group has ALL bit set than all user should have all rights.
+   * 2. If user/group has NONE bit set than user/group will not have any right.
+   * 3. For all other individual rights individual bits should be set.
+   *
+   * @param acls
+   * @param context
+   * @return return true if acl list contains right requsted in context.
+   * */
+  public static boolean checkAclRights(List<OzoneAcl> acls,
+      RequestContext context) throws OMException {
+    String[] userGroups = context.getClientUgi().getGroupNames();
+    String userName = context.getClientUgi().getUserName();
+    ACLType aclToCheck = context.getAclRights();
+    for (OzoneAcl acl : acls) {
+      if (checkAccessInAcl(acl, userGroups, userName, aclToCheck)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  /**
+   * Helper function to check if bit for given acl is set.
+   * @param acl
+   * @param bitset
+   * @return True of acl bit is set else false.
+   * */
+  public static boolean checkIfAclBitIsSet(IAccessAuthorizer.ACLType acl,
+      BitSet bitset) {
+    if (bitset == null) {
+      return false;
+    }
+
+    return ((bitset.get(acl.ordinal())
+        || bitset.get(ALL.ordinal()))
+        && !bitset.get(NONE.ordinal()));
+  }
+
+  /**
+   * Helper function to find and return all DEFAULT acls in input list with
+   * scope changed to ACCESS.
+   * @param acls
+   *
+   * @return list of default Acls.
+   * */
+  public static Collection<OzoneAclInfo> getDefaultAclsProto(
+      List<OzoneAcl> acls) {
+    return acls.stream().filter(a -> a.getAclScope() == DEFAULT)
+        .map(OzoneAcl::toProtobufWithAccessType).collect(Collectors.toList());
+  }
+
+  /**
+   * Helper function to find and return all DEFAULT acls in input list with
+   * scope changed to ACCESS.
+   * @param acls
+   *
+   * @return list of default Acls.
+   * */
+  public static List<OzoneAcl> getDefaultAcls(List<OzoneAcl> acls) {
+    return acls.stream().filter(a -> a.getAclScope() == DEFAULT)
+        .collect(Collectors.toList());
+  }
+
+  /**
+   * Helper function to inherit default ACL as access ACL for child object.
+   * 1. deep copy of OzoneAcl to avoid unexpected parent default ACL change
+   * 2. merge inherited access ACL with existing access ACL via
+   * OzoneUtils.addAcl().
+   * @param acls
+   * @param parentAcls
+   * @return true if acls inherited DEFAULT acls from parentAcls successfully,
+   * false otherwise.
+   */
+  public static boolean inheritDefaultAcls(List<OzoneAcl> acls,
+      List<OzoneAcl> parentAcls) {
+    List<OzoneAcl> inheritedAcls = null;
+    if (parentAcls != null && !parentAcls.isEmpty()) {
+      inheritedAcls = parentAcls.stream()
+          .filter(a -> a.getAclScope() == DEFAULT)
+          .map(acl -> new OzoneAcl(acl.getType(), acl.getName(),
+              acl.getAclBitSet(), OzoneAcl.AclScope.ACCESS))
+          .collect(Collectors.toList());
+    }
+    if (inheritedAcls != null && !inheritedAcls.isEmpty()) {
+      inheritedAcls.stream().forEach(acl -> addAcl(acls, acl));
+      return true;
+    }
+    return false;
+  }
+
+  /**
+   * Convert a list of OzoneAclInfo(protoc) to list of OzoneAcl(java).
+   * @param protoAcls
+   * @return list of OzoneAcl.
+   */
+  public static List<OzoneAcl> fromProtobuf(List<OzoneAclInfo> protoAcls) {
+    return protoAcls.stream().map(acl->OzoneAcl.fromProtobuf(acl))
+        .collect(Collectors.toList());
+  }
+
+  /**
+   * Convert a list of OzoneAcl(java) to list of OzoneAclInfo(protoc).
+   * @param protoAcls
+   * @return list of OzoneAclInfo.
+   */
+  public static List<OzoneAclInfo> toProtobuf(List<OzoneAcl> protoAcls) {
+    return protoAcls.stream().map(acl->OzoneAcl.toProtobuf(acl))
+        .collect(Collectors.toList());
+  }
+
+  /**
+   * Add an OzoneAcl to existing list of OzoneAcls.
+   * @param acl
+   * @param existingAcls
+   * @return true if current OzoneAcls are changed, false otherwise.
+   */
+  public static boolean addAcl(List<OzoneAcl> existingAcls, OzoneAcl acl) {
+    if (existingAcls == null || acl == null) {
+      return false;
+    }
+
+    for (OzoneAcl a: existingAcls) {
+      if (a.getName().equals(acl.getName()) &&
+          a.getType().equals(acl.getType()) &&
+          a.getAclScope().equals(acl.getAclScope())) {
+        BitSet current = a.getAclBitSet();
+        BitSet original = (BitSet) current.clone();
 
 Review comment:
   The code is a rewrite of the original along with better unit test coverage. 
It is much simpler to maintain compare with those being removed in multiple 
places. The new unit tests added in TestOzoneAclUtil covers cases that are 
missed from the much heavier tests in TestOzoneRpcClientAbstract.
   
 
----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 298294)
    Time Spent: 7h 10m  (was: 7h)

> Consolidate add/remove Acl into OzoneAclUtil class
> --------------------------------------------------
>
>                 Key: HDDS-1927
>                 URL: https://issues.apache.org/jira/browse/HDDS-1927
>             Project: Hadoop Distributed Data Store
>          Issue Type: Bug
>            Reporter: Bharat Viswanadham
>            Assignee: Xiaoyu Yao
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 7h 10m
>  Remaining Estimate: 0h
>
> This Jira is created based on @xiaoyu comment on HDDS-1884
> Can we abstract these add/remove logic into common AclUtil class as we can 
> see similar logic in both bucket manager and key manager? For example,
> public static boolean addAcl(List existingAcls, OzoneAcl newAcl)
> public static boolean removeAcl(List existingAcls, OzoneAcl newAcl)
>  
> But to do this, we need both OmKeyInfo and OMBucketInfo to use list of 
> OzoneAcl/OzoneAclInfo.
> This Jira is to do that refactor, and also address above comment to move 
> common logic to AclUtils.



--
This message was sent by Atlassian Jira
(v8.3.2#803003)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to