whbing commented on PR #4799:
URL: https://github.com/apache/ozone/pull/4799#issuecomment-1572272705

   > > Parent ACL checking logic redefined in 
[HDDS-8717](https://github.com/apache/ozone/pull/4439/files#diff-c41e5b8d16771255aaf9b64fa5565417426d1bff8875cb45d1965334adad8806).
 Most cases just need READ permission for volume
   > 
   > Thanks @whbing for the change, looks good to me according to the statement 
above that now key writes don't check volume acls and it is always set to Read 
to be at par with Ranger.
   > 
   > However in the native acl types I see NONE as a type which means no 
permission for user, `enum ACLType { READ, WRITE, CREATE, LIST, DELETE, 
READ_ACL, WRITE_ACL, ALL, NONE;` If I set acl this at volume, this would be 
ignored as for any key write it doesn't check volume acl which makes setting 
NONE useless. Not sure what could be long term solution here but we might need 
to re-evaluate & document native authorizer behaviour and whats expected.
   
   In fact, the volume's `READ` acl is checked for most operations for `bucket` 
or `key` operation. (Just `CREATE` bucket need volume's `WRITE` acl). 
   Acls for volume will always be checked  in 
`OzoneNativeAuthorizer#checkAccess` as follow:
   
   ```bash
   // OzoneNativeAuthorizer#checkAccess
   switch (objInfo.getResourceType()) {
       case VOLUME:
         ...
         boolean volumeAccess =  isOwner ||
             volumeManager.checkAccess(objInfo, context);
         return volumeAccess;
       case BUCKET:
         ...
         return (bucketAccess
             && volumeManager.checkAccess(objInfo, parentContext));
       case KEY:
         ...
         return (keyAccess
             && prefixManager.checkAccess(objInfo, parentContext)
             && bucketManager.checkAccess(objInfo, parentContext)
             && volumeManager.checkAccess(objInfo, parentVolContext));
       case PREFIX:
         return (prefixAccess
             && bucketManager.checkAccess(objInfo, parentContext)
             && volumeManager.checkAccess(objInfo, parentVolContext));
       default:
         throw new OMException("Unexpected object type:" +
             objInfo.getResourceType(), INVALID_REQUEST);
   ```
   `volumeManager.checkAccess(objInfo, parentVolContext))` will check volume's 
acl even if `objInfo.getResourceType()` is KEY. If NONE acl in a volume (or a 
bucket, a key), will permission denied.


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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to