ivandika3 commented on code in PR #10328:
URL: https://github.com/apache/ozone/pull/10328#discussion_r3323523933
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java:
##########
@@ -53,6 +55,43 @@ public abstract class OMVolumeAclRequest extends
OMVolumeRequest {
omVolumeAclOp = aclOp;
}
+ @Override
+ public OzoneManagerProtocolProtos.OMRequest preExecute(OzoneManager
ozoneManager)
+ throws IOException {
+ OzoneManagerProtocolProtos.OMRequest omRequest =
super.preExecute(ozoneManager);
Review Comment:
Not directly related to this patch, but seems there are cases where
`super.preExecute` is called and there are cases where `super.preExecute` is
not called (e.g. `OMKeyDeleteRequest#preExecute`) which seems to be a code
smell. We can check whether we need to call `super.preExecute` every time.
Also found a possible security issue where
`OmClientRequest#getUserInfoNotExists` might user an admin user (OM starter
user) privilege if the client does not specify any user info. I don't think
normal clients will gain admin user currently since both Hadoop RPC and gRPC
clients should already have the user info. However, I think it's best to for
`getUserInfoNotExists` to not fallback to the admin user since if we make any
changes in `getUserInfo` that causes userInfo's remoteAddress and userInfo's
username to not be set, it might cause cause privilege escalations.
--
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]
