diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java index deb2c8a05b32..8ff4495736d3 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java @@ -25,6 +25,7 @@ import java.io.IOException; import java.nio.file.InvalidPathException; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.Map; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.TableIterator; @@ -73,6 +74,37 @@ public OMBucketDeleteRequest(OMRequest omRequest) { super(omRequest); } + @Override + public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + super.preExecute(ozoneManager); + DeleteBucketRequest deleteBucketRequest = + getOmRequest().getDeleteBucketRequest(); + String volumeName = deleteBucketRequest.getVolumeName(); + String bucketName = deleteBucketRequest.getBucketName(); + + // ACL check during preExecute + if (ozoneManager.getAclsEnabled()) { + try { + checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, + OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.DELETE, + volumeName, bucketName, null); + } catch (IOException ex) { + // Ensure audit log captures preExecute failures + Map auditMap = new LinkedHashMap<>(); + auditMap.put(OzoneConsts.VOLUME, volumeName); + auditMap.put(OzoneConsts.BUCKET, bucketName); + markForAudit(ozoneManager.getAuditLogger(), + buildAuditMessage(OMAction.DELETE_BUCKET, auditMap, ex, + getOmRequest().getUserInfo())); + throw ex; + } + } + + return getOmRequest().toBuilder() + .setUserInfo(getUserIfNotExists(ozoneManager)) + .build(); + } + @Override public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) { final long transactionLogIndex = context.getIndex(); @@ -101,13 +133,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut boolean success = true; OMClientResponse omClientResponse = null; try { - // check Acl - if (ozoneManager.getAclsEnabled()) { - checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, - OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.DELETE, - volumeName, bucketName, null); - } - // acquire lock mergeOmLockDetails( omMetadataManager.getLock().acquireReadLock(VOLUME_LOCK, volumeName)); @@ -265,7 +290,7 @@ private boolean bucketContainsSnapshotInCache( /** * Validates bucket delete requests. - * Handles the cases where an older client attempts to delete a bucket + * Handles the cases where an older client attempts to delete a bucket with * a new bucket layout. * We do not want to allow this to happen, since this would cause the client * to be able to delete buckets it cannot understand. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java index 6d0c90cdca29..619a1b3fc3d8 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.nio.file.InvalidPathException; +import java.util.Map; import java.util.Objects; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; @@ -63,15 +64,40 @@ public OMBucketSetOwnerRequest(OMRequest omRequest) { @Override public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + super.preExecute(ozoneManager); + long modificationTime = Time.now(); OzoneManagerProtocolProtos.SetBucketPropertyRequest.Builder setBucketPropertyRequestBuilder = getOmRequest() .getSetBucketPropertyRequest().toBuilder() .setModificationTime(modificationTime); + SetBucketPropertyRequest setBucketPropertyRequest = + getOmRequest().getSetBucketPropertyRequest(); + BucketArgs bucketArgs = setBucketPropertyRequest.getBucketArgs(); + String volumeName = bucketArgs.getVolumeName(); + String bucketName = bucketArgs.getBucketName(); + + // ACL check during preExecute + if (ozoneManager.getAclsEnabled()) { + try { + checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, + OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, + volumeName, bucketName, null); + } catch (IOException ex) { + // Ensure audit log captures preExecute failures + OmBucketArgs omBucketArgs = OmBucketArgs.getFromProtobuf(bucketArgs); + Map auditMap = omBucketArgs.toAuditMap(); + markForAudit(ozoneManager.getAuditLogger(), + buildAuditMessage(OMAction.SET_OWNER, auditMap, ex, + getOmRequest().getUserInfo())); + throw ex; + } + } + return getOmRequest().toBuilder() .setSetBucketPropertyRequest(setBucketPropertyRequestBuilder) - .setUserInfo(getUserInfo()) + .setUserInfo(getUserIfNotExists(ozoneManager)) .build(); } @@ -109,13 +135,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut boolean acquiredBucketLock = false, success = true; OMClientResponse omClientResponse = null; try { - // check Acl - if (ozoneManager.getAclsEnabled()) { - checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, - OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volumeName, bucketName, null); - } - // acquire lock. mergeOmLockDetails(omMetadataManager.getLock().acquireWriteLock( BUCKET_LOCK, volumeName, bucketName)); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java index a88e5fb73334..6f91bcff9317 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.nio.file.InvalidPathException; import java.util.List; +import java.util.Map; import java.util.Objects; import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension; import org.apache.hadoop.hdds.client.DefaultReplicationConfig; @@ -79,6 +80,8 @@ public OMBucketSetPropertyRequest(OMRequest omRequest) { @Override public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + super.preExecute(ozoneManager); + long modificationTime = Time.now(); OzoneManagerProtocolProtos.SetBucketPropertyRequest.Builder setBucketPropertyRequestBuilder = getOmRequest() @@ -97,9 +100,26 @@ public OMRequest preExecute(OzoneManager ozoneManager) setBucketPropertyRequestBuilder.setBucketArgs(bucketArgsBuilder.build()); } + // ACL check during preExecute + if (ozoneManager.getAclsEnabled()) { + String volumeName = bucketArgs.getVolumeName(); + String bucketName = bucketArgs.getBucketName(); + try { + checkAclPermission(ozoneManager, volumeName, bucketName); + } catch (IOException ex) { + // Ensure audit log captures preExecute failures + OmBucketArgs omBucketArgs = OmBucketArgs.getFromProtobuf(bucketArgs); + Map auditMap = omBucketArgs.toAuditMap(); + markForAudit(ozoneManager.getAuditLogger(), + buildAuditMessage(OMAction.UPDATE_BUCKET, auditMap, ex, + getOmRequest().getUserInfo())); + throw ex; + } + } + return getOmRequest().toBuilder() .setSetBucketPropertyRequest(setBucketPropertyRequestBuilder) - .setUserInfo(getUserInfo()) + .setUserInfo(getUserIfNotExists(ozoneManager)) .build(); } @@ -132,11 +152,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut boolean acquiredBucketLock = false, success = true; OMClientResponse omClientResponse = null; try { - // check Acl - if (ozoneManager.getAclsEnabled()) { - checkAclPermission(ozoneManager, volumeName, bucketName); - } - // acquire lock. mergeOmLockDetails(omMetadataManager.getLock().acquireWriteLock( BUCKET_LOCK, volumeName, bucketName)); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java index 6faac1a24dc3..783d42315df8 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.nio.file.InvalidPathException; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import org.apache.commons.lang3.tuple.Pair; @@ -29,6 +30,7 @@ import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.OzoneConsts; 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; @@ -59,6 +61,49 @@ public OMBucketAclRequest(OMRequest omRequest, AclOp aclOp) { omBucketAclOp = aclOp; } + @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.BUCKET); + ResolvedBucket resolvedBucket = ozoneManager.resolveBucketLink( + Pair.of(objectParser.getVolume(), objectParser.getBucket())); + String volume = resolvedBucket.realVolume(); + String bucket = resolvedBucket.realBucket(); + + checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, + OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, + volume, bucket, null); + } catch (IOException ex) { + // Ensure audit log captures preExecute failures + Map auditMap = new LinkedHashMap<>(); + OzoneObj obj = getObject(); + auditMap.putAll(obj.toAuditMap()); + List acls = getAcls(); + if (acls != null) { + auditMap.put(OzoneConsts.ACL, acls.toString()); + } + // 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 transactionLogIndex = context.getIndex(); @@ -87,12 +132,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut volume = resolvedBucket.realVolume(); bucket = resolvedBucket.realBucket(); - // check Acl - if (ozoneManager.getAclsEnabled()) { - checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, - OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, bucket, null); - } mergeOmLockDetails( omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volume, bucket)); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketSetAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketSetAclRequest.java index 97dca83c1978..d7a02e2a54fe 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketSetAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketSetAclRequest.java @@ -54,6 +54,7 @@ public class OMBucketSetAclRequest extends OMBucketAclRequest { @Override public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + super.preExecute(ozoneManager); long modificationTime = Time.now(); OzoneManagerProtocolProtos.SetAclRequest.Builder setAclRequestBuilder = getOmRequest().getSetAclRequest().toBuilder()