From 5291e8cf4ed1177521a5329640afc625da603d1b Mon Sep 17 00:00:00 2001 From: Sergey Soldatov Date: Thu, 21 May 2026 12:58:55 -0700 Subject: [PATCH] HDDS-13855. Move ACL check in Key and Prefix requests to preExecute Move ACL authorization checks for key operations (delete-keys, rename-keys) and key/prefix ACL operations (add, remove, set ACL, including FSO variants) from validateAndUpdateCache to preExecute. For bulk operations (OMKeysDeleteRequest, OMKeysRenameRequest), keys that fail the ACL check are collected in new proto fields (aclDeniedKeys / aclDeniedRenameKeys) and removed from the batch so the remaining permitted keys can still be processed. Proto changes: add aclDeniedKeys to DeleteKeyArgs and aclDeniedRenameKeys to RenameKeysArgs. Co-authored-by: Cursor --- .../src/main/proto/OmClientProtocol.proto | 2 + .../om/request/key/OMKeysDeleteRequest.java | 91 ++++++++++++---- .../om/request/key/OMKeysRenameRequest.java | 100 ++++++++++++++---- .../om/request/key/acl/OMKeyAclRequest.java | 48 +++++++-- .../key/acl/OMKeyAclRequestWithFSO.java | 49 +++++++-- .../key/acl/OMKeyAddAclRequestWithFSO.java | 10 +- .../key/acl/OMKeyRemoveAclRequestWithFSO.java | 10 +- .../key/acl/OMKeySetAclRequestWithFSO.java | 10 +- .../key/acl/prefix/OMPrefixAclRequest.java | 49 +++++++-- 9 files changed, 300 insertions(+), 69 deletions(-) diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index 22c71cc29852..022660107921 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -1367,6 +1367,7 @@ message RenameKeysArgs { required string volumeName = 1; required string bucketName = 2; repeated RenameKeysMap renameKeysMap = 3; + repeated RenameKeysMap aclDeniedRenameKeys = 4; } message RenameKeysMap { @@ -1400,6 +1401,7 @@ message DeleteKeyArgs { required string volumeName = 1; required string bucketName = 2; repeated string keys = 3; + repeated string aclDeniedKeys = 4; } message DeleteKeyError { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java index e8a17d2e74fe..952069f38b89 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java @@ -88,6 +88,65 @@ public OMKeysDeleteRequest(OMRequest omRequest, BucketLayout bucketLayout) { super(omRequest, bucketLayout); } + @Override + public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + super.preExecute(ozoneManager); + + DeleteKeysRequest deleteKeysRequest = getOmRequest().getDeleteKeysRequest(); + DeleteKeyArgs deleteKeyArgs = deleteKeysRequest.getDeleteKeys(); + + String volumeName = deleteKeyArgs.getVolumeName(); + String bucketName = deleteKeyArgs.getBucketName(); + List keys = deleteKeyArgs.getKeysList(); + + ResolvedBucket resolvedBucketObj = ozoneManager.resolveBucketLink( + Pair.of(volumeName, bucketName)); + String resolvedVolume = resolvedBucketObj.realVolume(); + String resolvedBucket = resolvedBucketObj.realBucket(); + + String volumeOwner = getVolumeOwner(ozoneManager.getMetadataManager(), resolvedVolume); + + List keysPassingAcl = new ArrayList<>(); + List aclDeniedKeys = new ArrayList<>(); + if (ozoneManager.getAclsEnabled()) { + for (String keyName : keys) { + try { + checkKeyAcls(ozoneManager, resolvedVolume, resolvedBucket, keyName, + IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY, + volumeOwner); + keysPassingAcl.add(keyName); + } catch (IOException ex) { + LOG.warn("ACL check failed for key {} during preExecute: {}", + keyName, ex.getMessage()); + aclDeniedKeys.add(keyName); + Map auditMap = new LinkedHashMap<>(); + auditMap.put(VOLUME, resolvedVolume); + auditMap.put(BUCKET, resolvedBucket); + auditMap.put(KEY, keyName); + markForAudit(ozoneManager.getAuditLogger(), + buildAuditMessage(DELETE_KEYS, auditMap, ex, + getOmRequest().getUserInfo())); + } + } + } else { + keysPassingAcl.addAll(keys); + } + + DeleteKeyArgs.Builder modifiedArgs = deleteKeyArgs.toBuilder() + .clearKeys() + .addAllKeys(keysPassingAcl) + .clearAclDeniedKeys() + .addAllAclDeniedKeys(aclDeniedKeys); + + DeleteKeysRequest.Builder modifiedRequest = deleteKeysRequest.toBuilder() + .setDeleteKeys(modifiedArgs); + + return getOmRequest().toBuilder() + .setDeleteKeysRequest(modifiedRequest) + .setUserInfo(getUserIfNotExists(ozoneManager)) + .build(); + } + @Override @SuppressWarnings("methodlength") public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) { final long trxnLogIndex = context.getIndex(); @@ -147,7 +206,13 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut acquiredLock = getOmLockDetails().isLockAcquired(); // Validate bucket and volume exists or not. validateBucketAndVolume(omMetadataManager, volumeName, bucketName); - String volumeOwner = getVolumeOwner(omMetadataManager, volumeName); + + for (String deniedKey : deleteKeyArgs.getAclDeniedKeysList()) { + deleteStatus = false; + unDeletedKeys.addKeys(deniedKey); + keyToError.put(deniedKey, + new ErrorInfo(OMException.ResultCodes.ACCESS_DENIED.name(), "ACL check failed")); + } for (indexFailed = 0; indexFailed < length; indexFailed++) { String keyName = deleteKeyArgs.getKeys(indexFailed); @@ -166,25 +231,11 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut continue; } - try { - // check Acl - long startNanosDeleteKeysAclCheckLatency = Time.monotonicNowNanos(); - checkKeyAcls(ozoneManager, volumeName, bucketName, keyName, - IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY, - volumeOwner); - perfMetrics.setDeleteKeysAclCheckLatencyNs(Time.monotonicNowNanos() - startNanosDeleteKeysAclCheckLatency); - OzoneFileStatus fileStatus = getOzoneKeyStatus( - ozoneManager, omMetadataManager, volumeName, bucketName, keyName); - addKeyToAppropriateList(omKeyInfoList, omKeyInfo, dirList, - fileStatus); - deleteKeysInfo.add(omKeyInfo); - } catch (Exception ex) { - deleteStatus = false; - LOG.error("Acl check failed for Key: {}", objectKey, ex); - deleteKeys.remove(keyName); - unDeletedKeys.addKeys(keyName); - keyToError.put(keyName, new ErrorInfo(OMException.ResultCodes.ACCESS_DENIED.name(), "ACL check failed")); - } + OzoneFileStatus fileStatus = getOzoneKeyStatus( + ozoneManager, omMetadataManager, volumeName, bucketName, keyName); + addKeyToAppropriateList(omKeyInfoList, omKeyInfo, dirList, + fileStatus); + deleteKeysInfo.add(omKeyInfo); } OmBucketInfo omBucketInfo = diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index 3da0849f44f4..8b710e6254a6 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -17,8 +17,12 @@ package org.apache.hadoop.ozone.om.request.key; +import static org.apache.hadoop.ozone.OzoneConsts.BUCKET; +import static org.apache.hadoop.ozone.OzoneConsts.DST_KEY; import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP; +import static org.apache.hadoop.ozone.OzoneConsts.SRC_KEY; import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP; +import static org.apache.hadoop.ozone.OzoneConsts.VOLUME; import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.LeveledResource.BUCKET_LOCK; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME; @@ -77,6 +81,76 @@ public OMKeysRenameRequest(OMRequest omRequest, BucketLayout bucketLayout) { super(omRequest, bucketLayout); } + @Override + public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + super.preExecute(ozoneManager); + + RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest(); + RenameKeysArgs renameKeysArgs = renameKeysRequest.getRenameKeysArgs(); + + String volumeName = renameKeysArgs.getVolumeName(); + String bucketName = renameKeysArgs.getBucketName(); + + ResolvedBucket resolvedBucketObj = ozoneManager.resolveBucketLink( + Pair.of(volumeName, bucketName)); + String resolvedVolume = resolvedBucketObj.realVolume(); + String resolvedBucket = resolvedBucketObj.realBucket(); + + String volumeOwner = getVolumeOwner(ozoneManager.getMetadataManager(), resolvedVolume); + + List keyPairsPassingAcl = new ArrayList<>(); + List aclDeniedPairs = new ArrayList<>(); + if (ozoneManager.getAclsEnabled()) { + for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) { + String fromKeyName = renameKey.getFromKeyName(); + String toKeyName = renameKey.getToKeyName(); + + if (fromKeyName.isEmpty() || toKeyName.isEmpty()) { + keyPairsPassingAcl.add(renameKey); + continue; + } + + try { + checkKeyAcls(ozoneManager, resolvedVolume, resolvedBucket, fromKeyName, + IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY, + volumeOwner); + checkKeyAcls(ozoneManager, resolvedVolume, resolvedBucket, toKeyName, + IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY, + volumeOwner); + keyPairsPassingAcl.add(renameKey); + } catch (IOException ex) { + LOG.warn("ACL check failed for rename {} -> {} during preExecute: {}", + fromKeyName, toKeyName, ex.getMessage()); + aclDeniedPairs.add(renameKey); + Map auditMap = new LinkedHashMap<>(); + auditMap.put(VOLUME, resolvedVolume); + auditMap.put(BUCKET, resolvedBucket); + auditMap.put(SRC_KEY, fromKeyName); + auditMap.put(DST_KEY, toKeyName); + markForAudit(ozoneManager.getAuditLogger(), + buildAuditMessage(OMAction.RENAME_KEYS, auditMap, ex, + getOmRequest().getUserInfo())); + } + } + } else { + keyPairsPassingAcl.addAll(renameKeysArgs.getRenameKeysMapList()); + } + + RenameKeysArgs.Builder modifiedArgs = renameKeysArgs.toBuilder() + .clearRenameKeysMap() + .addAllRenameKeysMap(keyPairsPassingAcl) + .clearAclDeniedRenameKeys() + .addAllAclDeniedRenameKeys(aclDeniedPairs); + + RenameKeysRequest.Builder modifiedRequest = renameKeysRequest.toBuilder() + .setRenameKeysArgs(modifiedArgs); + + return getOmRequest().toBuilder() + .setRenameKeysRequest(modifiedRequest) + .setUserInfo(getUserIfNotExists(ozoneManager)) + .build(); + } + @Override @SuppressWarnings("methodlength") public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) { @@ -125,7 +199,12 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut // Validate bucket and volume exists or not. validateBucketAndVolume(omMetadataManager, volumeName, bucketName); - String volumeOwner = getVolumeOwner(omMetadataManager, volumeName); + + for (RenameKeysMap deniedPair : renameKeysArgs.getAclDeniedRenameKeysList()) { + renameStatus = false; + unRenamedKeys.add(deniedPair); + } + for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) { fromKeyName = renameKey.getFromKeyName(); @@ -142,25 +221,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut continue; } - try { - // check Acls to see if user has access to perform delete operation - // on old key and create operation on new key - checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName, - IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY, - volumeOwner); - checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName, - IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY, - volumeOwner); - } catch (Exception ex) { - renameStatus = false; - unRenamedKeys.add( - unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName) - .build()); - LOG.error("Acl check failed for fromKeyName {} toKeyName {}", - fromKeyName, toKeyName, ex); - continue; - } - // Check if toKey exists String fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java index 35e06de92ee2..02b92aa6a3f9 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java @@ -21,11 +21,13 @@ import java.io.IOException; import java.nio.file.InvalidPathException; +import java.util.LinkedHashMap; import java.util.Map; import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.audit.AuditLogger; +import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.om.ResolvedBucket; @@ -61,6 +63,46 @@ public OMKeyAclRequest(OMRequest omRequest) { super(omRequest); } + @Override + public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + OMRequest omRequest = super.preExecute(ozoneManager); + + // ACL check during preExecute + if (ozoneManager.getAclsEnabled()) { + try { + ObjectParser objectParser = new ObjectParser(getPath(), + ObjectType.KEY); + ResolvedBucket resolvedBucket = ozoneManager.resolveBucketLink( + Pair.of(objectParser.getVolume(), objectParser.getBucket())); + String volume = resolvedBucket.realVolume(); + String bucket = resolvedBucket.realBucket(); + String key = objectParser.getKey(); + + checkAcls(ozoneManager, OzoneObj.ResourceType.KEY, + OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, + volume, bucket, key); + } catch (IOException ex) { + // Ensure audit log captures preExecute failures + Map auditMap = new LinkedHashMap<>(); + OzoneObj obj = getObject(); + auditMap.putAll(obj.toAuditMap()); + // Determine which action based on request type + OMAction action = OMAction.SET_ACL; + if (omRequest.hasAddAclRequest()) { + action = OMAction.ADD_ACL; + } else if (omRequest.hasRemoveAclRequest()) { + action = OMAction.REMOVE_ACL; + } + markForAudit(ozoneManager.getAuditLogger(), + buildAuditMessage(action, auditMap, ex, + omRequest.getUserInfo())); + throw ex; + } + } + + return omRequest; + } + @Override public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) { final long trxnLogIndex = context.getIndex(); @@ -87,12 +129,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut bucket = resolvedBucket.realBucket(); key = objectParser.getKey(); - // check Acl - if (ozoneManager.getAclsEnabled()) { - checkAcls(ozoneManager, OzoneObj.ResourceType.KEY, - OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, bucket, key); - } mergeOmLockDetails( omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volume, bucket)); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequestWithFSO.java index f9c8602e0a43..b087f996e7ab 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequestWithFSO.java @@ -22,11 +22,13 @@ import java.io.IOException; import java.nio.file.InvalidPathException; +import java.util.LinkedHashMap; import java.util.Map; import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; +import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.om.ResolvedBucket; @@ -42,6 +44,7 @@ import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.om.response.key.acl.OMKeyAclResponseWithFSO; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.OzoneObj; @@ -56,6 +59,46 @@ public OMKeyAclRequestWithFSO(OzoneManagerProtocolProtos.OMRequest omReq, setBucketLayout(bucketLayout); } + @Override + public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + OMRequest omRequest = super.preExecute(ozoneManager); + + // ACL check during preExecute + if (ozoneManager.getAclsEnabled()) { + try { + ObjectParser objectParser = new ObjectParser(getPath(), + OzoneManagerProtocolProtos.OzoneObj.ObjectType.KEY); + ResolvedBucket resolvedBucket = ozoneManager.resolveBucketLink( + Pair.of(objectParser.getVolume(), objectParser.getBucket())); + String volume = resolvedBucket.realVolume(); + String bucket = resolvedBucket.realBucket(); + String key = objectParser.getKey(); + + checkAcls(ozoneManager, OzoneObj.ResourceType.KEY, + OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, + volume, bucket, key); + } catch (IOException ex) { + // Ensure audit log captures preExecute failures + Map auditMap = new LinkedHashMap<>(); + OzoneObj obj = getObject(); + auditMap.putAll(obj.toAuditMap()); + // Determine which action based on request type + OMAction action = OMAction.SET_ACL; + if (omRequest.hasAddAclRequest()) { + action = OMAction.ADD_ACL; + } else if (omRequest.hasRemoveAclRequest()) { + action = OMAction.REMOVE_ACL; + } + markForAudit(ozoneManager.getAuditLogger(), + buildAuditMessage(action, auditMap, ex, + omRequest.getUserInfo())); + throw ex; + } + } + + return omRequest; + } + @Override public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) { final long trxnLogIndex = context.getIndex(); @@ -81,12 +124,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut bucket = resolvedBucket.realBucket(); key = objectParser.getKey(); - // check Acl - if (ozoneManager.getAclsEnabled()) { - checkAcls(ozoneManager, OzoneObj.ResourceType.KEY, - OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, bucket, key); - } mergeOmLockDetails(omMetadataManager.getLock() .acquireWriteLock(BUCKET_LOCK, volume, bucket)); lockAcquired = getOmLockDetails().isLockAcquired(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAddAclRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAddAclRequestWithFSO.java index 049f9a8384e3..e8e557c80a3e 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAddAclRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAddAclRequestWithFSO.java @@ -55,13 +55,17 @@ public class OMKeyAddAclRequestWithFSO extends OMKeyAclRequestWithFSO { @Override public OzoneManagerProtocolProtos.OMRequest preExecute( OzoneManager ozoneManager) throws IOException { + // Call parent's preExecute to perform ACL checks + OzoneManagerProtocolProtos.OMRequest omRequest = + super.preExecute(ozoneManager); + long modificationTime = Time.now(); OzoneManagerProtocolProtos.AddAclRequest.Builder addAclRequestBuilder = - getOmRequest().getAddAclRequest().toBuilder() + omRequest.getAddAclRequest().toBuilder() .setModificationTime(modificationTime); - return getOmRequest().toBuilder().setAddAclRequest(addAclRequestBuilder) - .setUserInfo(getUserInfo()).build(); + return omRequest.toBuilder().setAddAclRequest(addAclRequestBuilder) + .build(); } public OMKeyAddAclRequestWithFSO( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyRemoveAclRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyRemoveAclRequestWithFSO.java index 02dd83874d69..8dce6235ea71 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyRemoveAclRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyRemoveAclRequestWithFSO.java @@ -55,14 +55,18 @@ public class OMKeyRemoveAclRequestWithFSO extends OMKeyAclRequestWithFSO { @Override public OzoneManagerProtocolProtos.OMRequest preExecute( OzoneManager ozoneManager) throws IOException { + // Call parent's preExecute to perform ACL checks + OzoneManagerProtocolProtos.OMRequest omRequest = + super.preExecute(ozoneManager); + long modificationTime = Time.now(); OzoneManagerProtocolProtos.RemoveAclRequest.Builder removeAclRequestBuilder = - getOmRequest().getRemoveAclRequest().toBuilder() + omRequest.getRemoveAclRequest().toBuilder() .setModificationTime(modificationTime); - return getOmRequest().toBuilder() - .setRemoveAclRequest(removeAclRequestBuilder).setUserInfo(getUserInfo()) + return omRequest.toBuilder() + .setRemoveAclRequest(removeAclRequestBuilder) .build(); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeySetAclRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeySetAclRequestWithFSO.java index 8d0d4db80baf..590d761b130a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeySetAclRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeySetAclRequestWithFSO.java @@ -56,13 +56,17 @@ public class OMKeySetAclRequestWithFSO extends OMKeyAclRequestWithFSO { @Override public OzoneManagerProtocolProtos.OMRequest preExecute( OzoneManager ozoneManager) throws IOException { + // Call parent's preExecute to perform ACL checks + OzoneManagerProtocolProtos.OMRequest omRequest = + super.preExecute(ozoneManager); + long modificationTime = Time.now(); OzoneManagerProtocolProtos.SetAclRequest.Builder setAclRequestBuilder = - getOmRequest().getSetAclRequest().toBuilder() + omRequest.getSetAclRequest().toBuilder() .setModificationTime(modificationTime); - return getOmRequest().toBuilder().setSetAclRequest(setAclRequestBuilder) - .setUserInfo(getUserInfo()).build(); + return omRequest.toBuilder().setSetAclRequest(setAclRequestBuilder) + .build(); } public OMKeySetAclRequestWithFSO( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/prefix/OMPrefixAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/prefix/OMPrefixAclRequest.java index 5549c1f21b9f..8bef94a3c194 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/prefix/OMPrefixAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/prefix/OMPrefixAclRequest.java @@ -21,10 +21,12 @@ import java.io.IOException; import java.nio.file.InvalidPathException; +import java.util.LinkedHashMap; import java.util.Map; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.audit.AuditLogger; +import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OMMetrics; import org.apache.hadoop.ozone.om.OzoneManager; @@ -50,6 +52,45 @@ public OMPrefixAclRequest(OMRequest omRequest) { super(omRequest); } + @Override + public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + OMRequest omRequest = super.preExecute(ozoneManager); + + // ACL check during preExecute + if (ozoneManager.getAclsEnabled()) { + try { + PrefixManagerImpl prefixManager = + (PrefixManagerImpl) ozoneManager.getPrefixManager(); + OzoneObj resolvedPrefixObj = prefixManager.getResolvedPrefixObj(getOzoneObj()); + prefixManager.validateOzoneObj(getOzoneObj()); + validatePrefixPath(resolvedPrefixObj.getPath()); + + checkAcls(ozoneManager, OzoneObj.ResourceType.PREFIX, + OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, + resolvedPrefixObj.getVolumeName(), resolvedPrefixObj.getBucketName(), + resolvedPrefixObj.getPrefixName()); + } catch (IOException ex) { + // Ensure audit log captures preExecute failures + Map auditMap = new LinkedHashMap<>(); + OzoneObj obj = getOzoneObj(); + auditMap.putAll(obj.toAuditMap()); + // Determine which action based on request type + OMAction action = OMAction.SET_ACL; + if (omRequest.hasAddAclRequest()) { + action = OMAction.ADD_ACL; + } else if (omRequest.hasRemoveAclRequest()) { + action = OMAction.REMOVE_ACL; + } + markForAudit(ozoneManager.getAuditLogger(), + buildAuditMessage(action, auditMap, ex, + omRequest.getUserInfo())); + throw ex; + } + } + + return omRequest; + } + @Override public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) { final long trxnLogIndex = context.getIndex(); @@ -76,14 +117,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut validatePrefixPath(resolvedPrefixObj.getPath()); prefixPath = resolvedPrefixObj.getPath(); - // check Acl - if (ozoneManager.getAclsEnabled()) { - checkAcls(ozoneManager, OzoneObj.ResourceType.PREFIX, - OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - resolvedPrefixObj.getVolumeName(), resolvedPrefixObj.getBucketName(), - resolvedPrefixObj.getPrefixName()); - } - mergeOmLockDetails(omMetadataManager.getLock() .acquireWriteLock(PREFIX_LOCK, prefixPath)); lockAcquired = getOmLockDetails().isLockAcquired();