xiaoyuyao commented on a change in pull request #24: HDDS 2181. Ozone Manager 
should send correct ACL type in ACL requests to Authorizer
URL: https://github.com/apache/hadoop-ozone/pull/24#discussion_r335097267
 
 

 ##########
 File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java
 ##########
 @@ -77,25 +80,52 @@ public boolean checkAccess(IOzoneObj ozObject, 
RequestContext context)
           "configured to work with OzoneObjInfo type only.", INVALID_REQUEST);
     }
 
+    // For CREATE and DELETE acl requests, the parents need to be checked
+    // for WRITE acl. If Key create request is received, then we need to
+    // check if user has WRITE acl set on Bucket and Volume. In all other cases
+    // the parents also need to be checked for the same acl type.
+    if (isACLTypeCreate || isACLTypeDelete) {
+      parentContext = RequestContext.newBuilder()
+        .setClientUgi(context.getClientUgi())
+        .setIp(context.getIp())
+        .setAclType(context.getAclType())
+        .setAclRights(ACLType.WRITE)
+        .build();
+    } else {
+      parentContext = context;
+    }
+
     switch (objInfo.getResourceType()) {
     case VOLUME:
       LOG.trace("Checking access for volume: {}", objInfo);
       return volumeManager.checkAccess(objInfo, context);
     case BUCKET:
       LOG.trace("Checking access for bucket: {}", objInfo);
-      return (bucketManager.checkAccess(objInfo, context)
-          && volumeManager.checkAccess(objInfo, context));
+      // Skip bucket access check for CREATE acl since
+      // bucket will not exist at the time of creation
+      boolean bucketAccess = isACLTypeCreate
+          || bucketManager.checkAccess(objInfo, context);
+      return (bucketAccess
+          && volumeManager.checkAccess(objInfo, parentContext));
     case KEY:
       LOG.trace("Checking access for Key: {}", objInfo);
-      return (keyManager.checkAccess(objInfo, context)
-          && prefixManager.checkAccess(objInfo, context)
-          && bucketManager.checkAccess(objInfo, context)
-          && volumeManager.checkAccess(objInfo, context));
+      // Skip key access check for CREATE acl since
+      // key will not exist at the time of creation
+      boolean keyAccess = isACLTypeCreate
 
 Review comment:
   NIT: I think we can move the ACLType.CREATE special handling logic inside 
keyManager.checkAccess() where we have another special handling for 
ACLType.WRITE. This way, the implementation inside OzoneNativeAuthorizer will 
be much cleaner. 

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


With regards,
Apache Git Services

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

Reply via email to