diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMHALeaderSpecificACLEnforcement.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMHALeaderSpecificACLEnforcement.java index d84e0e863f1d..91e9d0628e01 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMHALeaderSpecificACLEnforcement.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMHALeaderSpecificACLEnforcement.java @@ -22,23 +22,28 @@ import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERMISSION_DENIED; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.concurrent.TimeoutException; +import java.util.function.BooleanSupplier; import org.apache.commons.lang3.RandomStringUtils; +import org.apache.hadoop.hdds.client.OzoneQuota; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.utils.IOUtils; import org.apache.hadoop.ozone.MiniOzoneCluster; import org.apache.hadoop.ozone.MiniOzoneHAClusterImpl; +import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.client.BucketArgs; import org.apache.hadoop.ozone.client.ObjectStore; import org.apache.hadoop.ozone.client.OzoneBucket; @@ -49,8 +54,11 @@ import org.apache.hadoop.ozone.client.VolumeArgs; import org.apache.hadoop.ozone.client.io.OzoneOutputStream; import org.apache.hadoop.ozone.om.exceptions.OMException; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.OzoneNativeAuthorizer; +import org.apache.hadoop.ozone.security.acl.OzoneObj; +import org.apache.hadoop.ozone.security.acl.OzoneObjInfo; import org.apache.hadoop.security.UserGroupInformation; import org.apache.ozone.test.GenericTestUtils; import org.junit.jupiter.api.AfterAll; @@ -70,13 +78,13 @@ public class TestOMHALeaderSpecificACLEnforcement { private static final String OM_SERVICE_ID = "om-service-test-admin"; private static final int NUM_OF_OMS = 3; - private static final String TEST_USER = "testuser-" + + private static final String TEST_USER = "testuser-" + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); - private static final String TEST_VOLUME = "testvol-" + + private static final String TEST_VOLUME = "testvol-" + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); - private static final String ADMIN_VOLUME = "adminvol-" + + private static final String ADMIN_VOLUME = "adminvol-" + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); - private static final String TEST_BUCKET = "testbucket-" + + private static final String TEST_BUCKET = "testbucket-" + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); private MiniOzoneHAClusterImpl cluster; @@ -90,10 +98,10 @@ public void init() throws Exception { // Create test user testUserUgi = UserGroupInformation.createUserForTesting(TEST_USER, new String[]{"testgroup"}); adminUserUgi = UserGroupInformation.getCurrentUser(); - + // Set up and start the cluster setupCluster(); - + // Create admin volume that will be used for bucket permission testing theLeaderOM = cluster.getOMLeader(); createAdminVolume(); @@ -112,14 +120,14 @@ public void restoreLeadership() throws IOException, InterruptedException, Timeou OzoneManager currentLeader = cluster.getOMLeader(); if (!currentLeader.getOMNodeId().equals(theLeaderOM.getOMNodeId())) { currentLeader.transferLeadership(theLeaderOM.getOMNodeId()); - GenericTestUtils.waitFor(() -> { + BooleanSupplier leadershipCheck = () -> { try { - OzoneManager currentLeaderCheck = cluster.getOMLeader(); - return !currentLeaderCheck.getOMNodeId().equals(currentLeader.getOMNodeId()); + return !cluster.getOMLeader().getOMNodeId().equals(currentLeader.getOMNodeId()); } catch (Exception e) { return false; } - }, 1000, 30000); + }; + GenericTestUtils.waitFor(leadershipCheck, 1000, 30000); } } @@ -136,26 +144,24 @@ public void testOMHAAdminPrivilegesAfterLeadershipChange() throws Exception { // Step 1: Get the current leader OM OzoneManager currentLeader = cluster.getOMLeader(); String leaderNodeId = currentLeader.getOMNodeId(); - + // Step 2: Add test user as admin only to the current leader OM addAdminToSpecificOM(currentLeader, TEST_USER); - + // Verify admin was added - assertTrue(currentLeader.getOmAdminUsernames().contains(TEST_USER), - "Test user should be admin on leader OM"); - + assertThat(currentLeader.getOmAdminUsernames()).contains(TEST_USER); + // Step 3: Test volume and bucket creation as test user (should succeed) testVolumeAndBucketCreationAsUser(true); - + // Step 4: Force leadership transfer to another OM node OzoneManager newLeader = transferLeadershipToAnotherNode(currentLeader); - assertNotEquals(leaderNodeId, newLeader.getOMNodeId(), + assertNotEquals(leaderNodeId, newLeader.getOMNodeId(), "Leadership should have transferred to a different node"); - + // Step 5: Verify test user is NOT admin on new leader - assertTrue(!newLeader.getOmAdminUsernames().contains(TEST_USER), - "Test user should NOT be admin on new leader OM"); - + assertThat(newLeader.getOmAdminUsernames()).doesNotContain(TEST_USER); + // Step 6: Test volume and bucket creation as test user (should fail) testVolumeAndBucketCreationAsUser(false); } @@ -167,16 +173,16 @@ private void setupCluster() throws Exception { OzoneConfiguration conf = createBaseConfiguration(); conf.setClass(OZONE_ACL_AUTHORIZER_CLASS, OzoneNativeAuthorizer.class, IAccessAuthorizer.class); - + // Build HA cluster MiniOzoneHAClusterImpl.Builder builder = MiniOzoneCluster.newHABuilder(conf); builder.setOMServiceId(OM_SERVICE_ID) .setNumOfOzoneManagers(NUM_OF_OMS) .setNumDatanodes(3); - + cluster = builder.build(); cluster.waitForClusterToBeReady(); - + // Create client client = OzoneClientFactory.getRpcClient(OM_SERVICE_ID, conf); } @@ -186,14 +192,14 @@ private void setupCluster() throws Exception { */ private OzoneConfiguration createBaseConfiguration() throws IOException { OzoneConfiguration conf = new OzoneConfiguration(); - + // Enable ACL for proper permission testing conf.setBoolean(OZONE_ACL_ENABLED, true); - + // Set current user as initial admin (needed for cluster setup) String currentUser = adminUserUgi.getShortUserName(); conf.set(OZONE_ADMINISTRATORS, currentUser); - + return conf; } @@ -204,12 +210,12 @@ private OzoneConfiguration createBaseConfiguration() throws IOException { */ private void createAdminVolume() throws Exception { ObjectStore adminObjectStore = client.getObjectStore(); - + // Create volume as admin user VolumeArgs volumeArgs = VolumeArgs.newBuilder() .setOwner(adminUserUgi.getShortUserName()) .build(); - + adminObjectStore.createVolume(ADMIN_VOLUME, volumeArgs); } @@ -220,64 +226,64 @@ private void createAdminVolume() throws Exception { private void addAdminToSpecificOM(OzoneManager om, String username) throws Exception { // Get current admin users String currentAdmins = String.join(",", om.getOmAdminUsernames()); - + // Add the new user to admin list String newAdmins = currentAdmins + "," + username; - + // Reconfigure the OM to add the new admin om.getReconfigurationHandler().reconfigureProperty(OZONE_ADMINISTRATORS, newAdmins); } /** * Tests volume and bucket creation as the test user. - * + * * @param shouldSucceed true if operations should succeed, false if they should fail */ private void testVolumeAndBucketCreationAsUser(boolean shouldSucceed) throws Exception { // Switch to test user context UserGroupInformation.setLoginUser(testUserUgi); - + try (OzoneClient userClient = OzoneClientFactory.getRpcClient(OM_SERVICE_ID, cluster.getConf())) { ObjectStore userObjectStore = userClient.getObjectStore(); - + if (shouldSucceed) { // Test volume creation (should succeed) VolumeArgs volumeArgs = VolumeArgs.newBuilder() .setOwner(TEST_USER) .build(); - + userObjectStore.createVolume(TEST_VOLUME, volumeArgs); OzoneVolume volume = userObjectStore.getVolume(TEST_VOLUME); assertNotNull(volume, "Volume should be created successfully"); assertEquals(TEST_VOLUME, volume.getName()); - + // Test bucket creation (should succeed) BucketArgs bucketArgs = BucketArgs.newBuilder() .build(); - + volume.createBucket(TEST_BUCKET, bucketArgs); OzoneBucket bucket = volume.getBucket(TEST_BUCKET); assertNotNull(bucket, "Bucket should be created successfully"); assertEquals(TEST_BUCKET, bucket.getName()); - + } else { // Test volume creation (should fail) VolumeArgs volumeArgs = VolumeArgs.newBuilder() .setOwner(TEST_USER) .build(); - + String newVolumeName = "failtest-" + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); OMException volumeException = assertThrows(OMException.class, () -> { userObjectStore.createVolume(newVolumeName, volumeArgs); }, "Volume creation should fail for non-admin user"); assertEquals(PERMISSION_DENIED, volumeException.getResult()); - + // Test bucket creation (should fail) - use admin-created volume if (volumeExists(userObjectStore, ADMIN_VOLUME)) { OzoneVolume adminVolume = userObjectStore.getVolume(ADMIN_VOLUME); BucketArgs bucketArgs = BucketArgs.newBuilder().build(); String newBucketName = "failtest-" + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); - + OMException bucketException = assertThrows(OMException.class, () -> { adminVolume.createBucket(newBucketName, bucketArgs); }, "Bucket creation should fail for non-admin user in admin-owned volume"); @@ -335,8 +341,7 @@ public void testKeySetTimesAclEnforcementAfterLeadershipChange() throws Exceptio addAdminToSpecificOM(currentLeader, TEST_USER); // Verify admin was added - assertTrue(currentLeader.getOmAdminUsernames().contains(TEST_USER), - "Test user should be admin on leader OM"); + assertThat(currentLeader.getOmAdminUsernames()).contains(TEST_USER); // Switch to test user and try setTimes as admin (should succeed) UserGroupInformation.setLoginUser(testUserUgi); @@ -358,8 +363,7 @@ public void testKeySetTimesAclEnforcementAfterLeadershipChange() throws Exceptio OzoneManager newLeader = transferLeadershipToAnotherNode(currentLeader); assertNotEquals(leaderNodeId, newLeader.getOMNodeId(), "Leadership should have transferred to a different node"); - assertFalse(newLeader.getOmAdminUsernames().contains(TEST_USER), - "Test user should NOT be admin on new leader OM"); + assertThat(newLeader.getOmAdminUsernames()).doesNotContain(TEST_USER); long anotherMtime = System.currentTimeMillis() + 10000; OMException exception = assertThrows(OMException.class, () -> { @@ -373,6 +377,615 @@ public void testKeySetTimesAclEnforcementAfterLeadershipChange() throws Exceptio } } + /** + * Tests that setQuota ACL check is enforced in preExecute and is leader-specific. + */ + @Test + public void testVolumeSetQuotaAclEnforcementAfterLeadershipChange() throws Exception { + ObjectStore adminObjectStore = client.getObjectStore(); + String testVolume = "quotavol-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + + // Create volume as admin + VolumeArgs volumeArgs = VolumeArgs.newBuilder() + .setOwner(adminUserUgi.getShortUserName()) + .build(); + adminObjectStore.createVolume(testVolume, volumeArgs); + + // Add test user as admin on current leader + OzoneManager currentLeader = cluster.getOMLeader(); + addAdminToSpecificOM(currentLeader, TEST_USER); + assertThat(currentLeader.getOmAdminUsernames()).contains(TEST_USER); + + // Test user should be able to set quota as admin + UserGroupInformation.setLoginUser(testUserUgi); + try (OzoneClient userClient = OzoneClientFactory.getRpcClient(OM_SERVICE_ID, cluster.getConf())) { + ObjectStore userObjectStore = userClient.getObjectStore(); + OzoneVolume userVolume = userObjectStore.getVolume(testVolume); + + OzoneQuota quota1 = OzoneQuota.getOzoneQuota(100L * 1024 * 1024 * 1024, 1000); + userVolume.setQuota(quota1); // Set quota to 100GB + + // Transfer leadership + OzoneManager newLeader = transferLeadershipToAnotherNode(currentLeader); + assertThat(newLeader.getOmAdminUsernames()).doesNotContain(TEST_USER); + + // Should fail on new leader + OzoneQuota quota2 = OzoneQuota.getOzoneQuota(200L * 1024 * 1024 * 1024, 2000); + OMException exception = assertThrows(OMException.class, () -> { + userVolume.setQuota(quota2); + }, "setQuota should fail for non-admin user on new leader"); + assertEquals(PERMISSION_DENIED, exception.getResult()); + } finally { + UserGroupInformation.setLoginUser(adminUserUgi); + } + } + + /** + * Tests that setOwner ACL check is enforced in preExecute and is leader-specific. + */ + @Test + public void testVolumeSetOwnerAclEnforcementAfterLeadershipChange() throws Exception { + ObjectStore adminObjectStore = client.getObjectStore(); + String testVolume = "ownervol-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + + // Create volume as admin + VolumeArgs volumeArgs = VolumeArgs.newBuilder() + .setOwner(adminUserUgi.getShortUserName()) + .build(); + adminObjectStore.createVolume(testVolume, volumeArgs); + + // Add test user as admin on current leader + OzoneManager currentLeader = cluster.getOMLeader(); + addAdminToSpecificOM(currentLeader, TEST_USER); + assertThat(currentLeader.getOmAdminUsernames()).contains(TEST_USER); + + // Test user should be able to change owner as admin + UserGroupInformation.setLoginUser(testUserUgi); + try (OzoneClient userClient = OzoneClientFactory.getRpcClient(OM_SERVICE_ID, cluster.getConf())) { + ObjectStore userObjectStore = userClient.getObjectStore(); + OzoneVolume userVolume = userObjectStore.getVolume(testVolume); + + userVolume.setOwner("newowner"); + + // Transfer leadership + OzoneManager newLeader = transferLeadershipToAnotherNode(currentLeader); + assertThat(newLeader.getOmAdminUsernames()).doesNotContain(TEST_USER); + + // Should fail on new leader + OMException exception = assertThrows(OMException.class, () -> { + userVolume.setOwner("anothernewowner"); + }, "setOwner should fail for non-admin user on new leader"); + assertEquals(PERMISSION_DENIED, exception.getResult()); + } finally { + UserGroupInformation.setLoginUser(adminUserUgi); + } + } + + /** + * Tests that deleteVolume ACL check is enforced in preExecute and is leader-specific. + */ + @Test + public void testVolumeDeleteAclEnforcementAfterLeadershipChange() throws Exception { + ObjectStore adminObjectStore = client.getObjectStore(); + String testVolume1 = "delvol1-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String testVolume2 = "delvol2-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + + // Create volumes as admin + VolumeArgs volumeArgs = VolumeArgs.newBuilder() + .setOwner(adminUserUgi.getShortUserName()) + .build(); + adminObjectStore.createVolume(testVolume1, volumeArgs); + adminObjectStore.createVolume(testVolume2, volumeArgs); + + // Add test user as admin on current leader + OzoneManager currentLeader = cluster.getOMLeader(); + addAdminToSpecificOM(currentLeader, TEST_USER); + assertThat(currentLeader.getOmAdminUsernames()).contains(TEST_USER); + + // Test user should be able to delete volume as admin + UserGroupInformation.setLoginUser(testUserUgi); + try (OzoneClient userClient = OzoneClientFactory.getRpcClient(OM_SERVICE_ID, cluster.getConf())) { + ObjectStore userObjectStore = userClient.getObjectStore(); + + userObjectStore.deleteVolume(testVolume1); + + // Transfer leadership + OzoneManager newLeader = transferLeadershipToAnotherNode(currentLeader); + assertThat(newLeader.getOmAdminUsernames()).doesNotContain(TEST_USER); + + // Should fail on new leader + OMException exception = assertThrows(OMException.class, () -> { + userObjectStore.deleteVolume(testVolume2); + }, "deleteVolume should fail for non-admin user on new leader"); + assertEquals(PERMISSION_DENIED, exception.getResult()); + } finally { + UserGroupInformation.setLoginUser(adminUserUgi); + } + } + + /** + * Tests that setBucketProperty ACL check is enforced in preExecute and is leader-specific. + */ + @Test + public void testBucketSetPropertyAclEnforcementAfterLeadershipChange() throws Exception { + ObjectStore adminObjectStore = client.getObjectStore(); + String testVolume = "bucketpropvol-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String testBucket = "bucketprop-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + + // Create volume and bucket as admin + VolumeArgs volumeArgs = VolumeArgs.newBuilder() + .setOwner(adminUserUgi.getShortUserName()) + .build(); + adminObjectStore.createVolume(testVolume, volumeArgs); + OzoneVolume adminVolume = adminObjectStore.getVolume(testVolume); + + BucketArgs bucketArgs = BucketArgs.newBuilder().build(); + adminVolume.createBucket(testBucket, bucketArgs); + + // Add test user as admin on current leader + OzoneManager currentLeader = cluster.getOMLeader(); + addAdminToSpecificOM(currentLeader, TEST_USER); + assertThat(currentLeader.getOmAdminUsernames()).contains(TEST_USER); + + // Test user should be able to set bucket properties as admin + UserGroupInformation.setLoginUser(testUserUgi); + try (OzoneClient userClient = OzoneClientFactory.getRpcClient(OM_SERVICE_ID, cluster.getConf())) { + ObjectStore userObjectStore = userClient.getObjectStore(); + OzoneVolume userVolume = userObjectStore.getVolume(testVolume); + OzoneBucket userBucket = userVolume.getBucket(testBucket); + + // Set versioning + userBucket.setVersioning(true); + + // Transfer leadership + OzoneManager newLeader = transferLeadershipToAnotherNode(currentLeader); + assertThat(newLeader.getOmAdminUsernames()).doesNotContain(TEST_USER); + + // Should fail on new leader + OMException exception = assertThrows(OMException.class, () -> { + userBucket.setVersioning(false); + }, "setBucketProperty should fail for non-admin user on new leader"); + assertEquals(PERMISSION_DENIED, exception.getResult()); + } finally { + UserGroupInformation.setLoginUser(adminUserUgi); + } + } + + /** + * Tests that setBucketOwner ACL check is enforced in preExecute and is leader-specific. + */ + @Test + public void testBucketSetOwnerAclEnforcementAfterLeadershipChange() throws Exception { + ObjectStore adminObjectStore = client.getObjectStore(); + String testVolume = "bucketownervol-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String testBucket = "bucketowner-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + + // Create volume and bucket as admin + VolumeArgs volumeArgs = VolumeArgs.newBuilder() + .setOwner(adminUserUgi.getShortUserName()) + .build(); + adminObjectStore.createVolume(testVolume, volumeArgs); + OzoneVolume adminVolume = adminObjectStore.getVolume(testVolume); + + BucketArgs bucketArgs = BucketArgs.newBuilder().build(); + adminVolume.createBucket(testBucket, bucketArgs); + + // Add test user as admin on current leader + OzoneManager currentLeader = cluster.getOMLeader(); + addAdminToSpecificOM(currentLeader, TEST_USER); + assertThat(currentLeader.getOmAdminUsernames()).contains(TEST_USER); + + // Test user should be able to set bucket owner as admin + UserGroupInformation.setLoginUser(testUserUgi); + try (OzoneClient userClient = OzoneClientFactory.getRpcClient(OM_SERVICE_ID, cluster.getConf())) { + ObjectStore userObjectStore = userClient.getObjectStore(); + OzoneVolume userVolume = userObjectStore.getVolume(testVolume); + OzoneBucket userBucket = userVolume.getBucket(testBucket); + + // Set new owner + userBucket.setOwner("newowner"); + + // Transfer leadership + OzoneManager newLeader = transferLeadershipToAnotherNode(currentLeader); + assertThat(newLeader.getOmAdminUsernames()).doesNotContain(TEST_USER); + + // Should fail on new leader + OMException exception = assertThrows(OMException.class, () -> { + userBucket.setOwner("anothernewowner"); + }, "setBucketOwner should fail for non-admin user on new leader"); + assertEquals(PERMISSION_DENIED, exception.getResult()); + } finally { + UserGroupInformation.setLoginUser(adminUserUgi); + } + } + + /** + * Tests that deleteBucket ACL check is enforced in preExecute and is leader-specific. + */ + @Test + public void testBucketDeleteAclEnforcementAfterLeadershipChange() throws Exception { + ObjectStore adminObjectStore = client.getObjectStore(); + String testVolume = "bucketdelvol-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String testBucket1 = "bucketdel1-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String testBucket2 = "bucketdel2-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + + // Create volume and buckets as admin + VolumeArgs volumeArgs = VolumeArgs.newBuilder() + .setOwner(adminUserUgi.getShortUserName()) + .build(); + adminObjectStore.createVolume(testVolume, volumeArgs); + OzoneVolume adminVolume = adminObjectStore.getVolume(testVolume); + + BucketArgs bucketArgs = BucketArgs.newBuilder().build(); + adminVolume.createBucket(testBucket1, bucketArgs); + adminVolume.createBucket(testBucket2, bucketArgs); + + // Add test user as admin on current leader + OzoneManager currentLeader = cluster.getOMLeader(); + addAdminToSpecificOM(currentLeader, TEST_USER); + assertThat(currentLeader.getOmAdminUsernames()).contains(TEST_USER); + + // Test user should be able to delete bucket as admin + UserGroupInformation.setLoginUser(testUserUgi); + try (OzoneClient userClient = OzoneClientFactory.getRpcClient(OM_SERVICE_ID, cluster.getConf())) { + ObjectStore userObjectStore = userClient.getObjectStore(); + OzoneVolume userVolume = userObjectStore.getVolume(testVolume); + + userVolume.deleteBucket(testBucket1); + + // Transfer leadership + OzoneManager newLeader = transferLeadershipToAnotherNode(currentLeader); + assertThat(newLeader.getOmAdminUsernames()).doesNotContain(TEST_USER); + + // Should fail on new leader + OMException exception = assertThrows(OMException.class, () -> { + userVolume.deleteBucket(testBucket2); + }, "deleteBucket should fail for non-admin user on new leader"); + assertEquals(PERMISSION_DENIED, exception.getResult()); + } finally { + UserGroupInformation.setLoginUser(adminUserUgi); + } + } + + /** + * Tests that deleteKeys (bulk) ACL check is enforced in preExecute and is leader-specific. + * + *

This test verifies that when using the bulk deleteKeys API: + *

+ * + *

The test flow: + *

    + *
  1. Create test volume, bucket, and multiple keys as admin
    2. + *
  2. Add test user as admin on current leader
    3. + *
  3. Test user successfully deletes first key using bulk API
    4. + *
  4. Transfer leadership to node where test user is NOT admin
    5. + *
  5. Test user attempts to delete second key - operation succeeds but key is not deleted
    6. + *
  6. Verify the key still exists (was filtered out due to ACL failure)
    7. + *
+ */ + @Test + public void testKeysDeleteAclEnforcementAfterLeadershipChange() throws Exception { + ObjectStore adminObjectStore = client.getObjectStore(); + String testVolume = "keysdelvol-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String testBucket = "keysdel-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String keyName1 = "key1-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String keyName2 = "key2-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + + // Step 1: Create volume, bucket, and keys as admin + VolumeArgs volumeArgs = VolumeArgs.newBuilder() + .setOwner(adminUserUgi.getShortUserName()) + .build(); + adminObjectStore.createVolume(testVolume, volumeArgs); + OzoneVolume adminVolume = adminObjectStore.getVolume(testVolume); + + BucketArgs bucketArgs = BucketArgs.newBuilder().build(); + adminVolume.createBucket(testBucket, bucketArgs); + OzoneBucket adminBucket = adminVolume.getBucket(testBucket); + + // Create test keys + try (OzoneOutputStream out = adminBucket.createKey(keyName1, 0)) { + out.write("test data 1".getBytes(UTF_8)); + } + try (OzoneOutputStream out = adminBucket.createKey(keyName2, 0)) { + out.write("test data 2".getBytes(UTF_8)); + } + + // Step 2: Add test user as admin on current leader + OzoneManager currentLeader = cluster.getOMLeader(); + String originalLeaderNodeId = currentLeader.getOMNodeId(); + addAdminToSpecificOM(currentLeader, TEST_USER); + assertThat(currentLeader.getOmAdminUsernames()).contains(TEST_USER); + + // Step 3: Test user deletes first key successfully using bulk API + UserGroupInformation.setLoginUser(testUserUgi); + try (OzoneClient userClient = OzoneClientFactory.getRpcClient(OM_SERVICE_ID, cluster.getConf())) { + ObjectStore userObjectStore = userClient.getObjectStore(); + OzoneVolume userVolume = userObjectStore.getVolume(testVolume); + OzoneBucket userBucket = userVolume.getBucket(testBucket); + + // Use bulk deleteKeys API (this supports ACL filtering) + userBucket.deleteKeys(Collections.singletonList(keyName1)); + + // Verify key1 was deleted + OMException ex1 = assertThrows(OMException.class, () -> userBucket.getKey(keyName1), + "Key1 should be deleted"); + assertEquals(OMException.ResultCodes.KEY_NOT_FOUND, ex1.getResult()); + + // Step 4: Transfer leadership to another node where test user is NOT admin + OzoneManager newLeader = transferLeadershipToAnotherNode(currentLeader); + assertNotEquals(originalLeaderNodeId, newLeader.getOMNodeId(), + "Leadership should have transferred to a different node"); + assertThat(newLeader.getOmAdminUsernames()).doesNotContain(TEST_USER); + + // Step 5: Attempt to delete second key - should not throw but key should not be deleted + // The bulk deleteKeys API filters out keys that fail ACL check in preExecute + userBucket.deleteKeys(Collections.singletonList(keyName2)); + } finally { + UserGroupInformation.setLoginUser(adminUserUgi); + } + + // Step 6: Verify key2 still exists (was filtered out due to ACL failure) + OzoneKey key2 = adminBucket.getKey(keyName2); + assertNotNull(key2, "Key2 should still exist after being filtered out by ACL check"); + assertEquals(keyName2, key2.getName()); + } + + /** + * Tests that renameKeys (bulk) ACL check is enforced in preExecute and is leader-specific. + * + *

This test verifies that when using the bulk renameKeys API: + *

+ * + *

The test flow: + *

    + *
  1. Create test volume, bucket, and multiple keys as admin with LEGACY bucket layout
    2. + *
  2. Add test user as admin on current leader
    3. + *
  3. Test user successfully renames first key using bulk API
    4. + *
  4. Transfer leadership to node where test user is NOT admin
    5. + *
  5. Test user attempts to rename second key - operation succeeds but key is not renamed
    6. + *
  6. Verify the key still has original name (was filtered out due to ACL failure)
    7. + *
+ * + *

Note: This test uses LEGACY bucket layout because the bulk renameKeys API is deprecated + * and not supported for FILE_SYSTEM_OPTIMIZED layouts. + */ + @Test + public void testKeysRenameAclEnforcementAfterLeadershipChange() throws Exception { + ObjectStore adminObjectStore = client.getObjectStore(); + String testVolume = "keysrenamevol-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String testBucket = "keysrename-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String keyName1 = "key1-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String keyName2 = "key2-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String newKeyName1 = "newkey1-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String newKeyName2 = "newkey2-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + + // Step 1: Create volume, bucket with LEGACY layout, and keys as admin + VolumeArgs volumeArgs = VolumeArgs.newBuilder() + .setOwner(adminUserUgi.getShortUserName()) + .build(); + adminObjectStore.createVolume(testVolume, volumeArgs); + OzoneVolume adminVolume = adminObjectStore.getVolume(testVolume); + + // Use LEGACY bucket layout since bulk renameKeys is not supported for FSO + BucketArgs bucketArgs = BucketArgs.newBuilder() + .setBucketLayout(BucketLayout.LEGACY) + .build(); + adminVolume.createBucket(testBucket, bucketArgs); + OzoneBucket adminBucket = adminVolume.getBucket(testBucket); + + // Create test keys + try (OzoneOutputStream out = adminBucket.createKey(keyName1, 0)) { + out.write("test data 1".getBytes(UTF_8)); + } + try (OzoneOutputStream out = adminBucket.createKey(keyName2, 0)) { + out.write("test data 2".getBytes(UTF_8)); + } + + // Step 2: Add test user as admin on current leader + OzoneManager currentLeader = cluster.getOMLeader(); + String originalLeaderNodeId = currentLeader.getOMNodeId(); + addAdminToSpecificOM(currentLeader, TEST_USER); + assertThat(currentLeader.getOmAdminUsernames()).contains(TEST_USER); + + // Step 3: Test user renames first key successfully using bulk API + UserGroupInformation.setLoginUser(testUserUgi); + try (OzoneClient userClient = OzoneClientFactory.getRpcClient(OM_SERVICE_ID, cluster.getConf())) { + ObjectStore userObjectStore = userClient.getObjectStore(); + OzoneVolume userVolume = userObjectStore.getVolume(testVolume); + OzoneBucket userBucket = userVolume.getBucket(testBucket); + + // Use bulk renameKeys API (this supports ACL filtering) + Map renameMap1 = new HashMap<>(); + renameMap1.put(keyName1, newKeyName1); + userBucket.renameKeys(renameMap1); + + // Verify key1 was renamed + OMException ex1 = assertThrows(OMException.class, () -> userBucket.getKey(keyName1), + "Original key1 should not exist after rename"); + assertEquals(OMException.ResultCodes.KEY_NOT_FOUND, ex1.getResult()); + assertNotNull(userBucket.getKey(newKeyName1), "Renamed key1 should exist"); + + // Step 4: Transfer leadership to another node where test user is NOT admin + OzoneManager newLeader = transferLeadershipToAnotherNode(currentLeader); + assertNotEquals(originalLeaderNodeId, newLeader.getOMNodeId(), + "Leadership should have transferred to a different node"); + assertThat(newLeader.getOmAdminUsernames()).doesNotContain(TEST_USER); + + // Step 5: Attempt to rename second key - should not throw but key should not be renamed + // The bulk renameKeys API filters out key pairs that fail ACL check in preExecute + Map renameMap2 = new HashMap<>(); + renameMap2.put(keyName2, newKeyName2); + userBucket.renameKeys(renameMap2); + } finally { + UserGroupInformation.setLoginUser(adminUserUgi); + } + + // Step 6: Verify key2 still has original name (was filtered out due to ACL failure) + OzoneKey key2 = adminBucket.getKey(keyName2); + assertNotNull(key2, "Original key2 should still exist after being filtered out by ACL check"); + assertEquals(keyName2, key2.getName()); + + // Verify new key name doesn't exist + OMException ex2 = assertThrows(OMException.class, () -> adminBucket.getKey(newKeyName2), + "New key name should not exist after ACL filtering"); + assertEquals(OMException.ResultCodes.KEY_NOT_FOUND, ex2.getResult()); + } + + /** + * Tests that key ACL operations (addAcl, removeAcl, setAcl) are enforced in preExecute + * and are leader-specific. Uses FILE_SYSTEM_OPTIMIZED bucket layout. + * + *

This test verifies that ACL operations on keys work correctly across leadership changes: + *

+ */ + @Test + public void testKeyAclOperationsEnforcementAfterLeadershipChangeWithFSO() throws Exception { + ObjectStore adminObjectStore = client.getObjectStore(); + String testVolume = "keyaclvol-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String testBucket = "keyaclbucket-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + String keyName = "keyacl-" + + RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT); + + // Step 1: Create volume, bucket with FSO layout, and key as admin + VolumeArgs volumeArgs = VolumeArgs.newBuilder() + .setOwner(adminUserUgi.getShortUserName()) + .build(); + adminObjectStore.createVolume(testVolume, volumeArgs); + OzoneVolume adminVolume = adminObjectStore.getVolume(testVolume); + + // Use FILE_SYSTEM_OPTIMIZED bucket layout + BucketArgs bucketArgs = BucketArgs.newBuilder() + .setBucketLayout(BucketLayout.FILE_SYSTEM_OPTIMIZED) + .build(); + adminVolume.createBucket(testBucket, bucketArgs); + OzoneBucket adminBucket = adminVolume.getBucket(testBucket); + + // Create a key as admin (so test user is NOT the owner) + try (OzoneOutputStream out = adminBucket.createKey(keyName, 0)) { + out.write("test data for ACL operations".getBytes(UTF_8)); + } + + OzoneKey key = adminBucket.getKey(keyName); + assertNotNull(key, "Key should be created successfully"); + + // Create OzoneObj for ACL operations + OzoneObj keyObj = OzoneObjInfo.Builder.newBuilder() + .setVolumeName(testVolume) + .setBucketName(testBucket) + .setKeyName(keyName) + .setResType(OzoneObj.ResourceType.KEY) + .setStoreType(OzoneObj.StoreType.OZONE) + .build(); + + int originalAclCount = adminObjectStore.getAcl(keyObj).size(); + + // Step 2: Add test user as admin on current leader + OzoneManager currentLeader = cluster.getOMLeader(); + String leaderNodeId = currentLeader.getOMNodeId(); + addAdminToSpecificOM(currentLeader, TEST_USER); + assertThat(currentLeader.getOmAdminUsernames()).contains(TEST_USER); + + // Step 3: Test user performs ACL operations as admin (should succeed) + UserGroupInformation.setLoginUser(testUserUgi); + try (OzoneClient userClient = OzoneClientFactory.getRpcClient(OM_SERVICE_ID, cluster.getConf())) { + ObjectStore userObjectStore = userClient.getObjectStore(); + + // Add ACL - should succeed + OzoneAcl addAcl = OzoneAcl.parseAcl("user:anotheruser:rw[ACCESS]"); + boolean addResult = userObjectStore.addAcl(keyObj, addAcl); + assertThat(addResult).isTrue(); + + // Verify ACL was added + List acls = userObjectStore.getAcl(keyObj); + assertThat(acls).hasSize(originalAclCount + 1); + assertThat(acls).contains(addAcl); + + // Set ACL - should succeed + OzoneAcl setAcl = OzoneAcl.parseAcl("user:setuser:rwx[ACCESS]"); + boolean setResult = userObjectStore.setAcl(keyObj, Collections.singletonList(setAcl)); + assertThat(setResult).isTrue(); + + // Verify ACL was set (replaced all previous ACLs) + acls = userObjectStore.getAcl(keyObj); + assertThat(acls).hasSize(1); + assertThat(acls).contains(setAcl); + + // Step 4: Transfer leadership to another node where test user is NOT admin + OzoneManager newLeader = transferLeadershipToAnotherNode(currentLeader); + assertNotEquals(leaderNodeId, newLeader.getOMNodeId(), + "Leadership should have transferred to a different node"); + assertThat(newLeader.getOmAdminUsernames()).doesNotContain(TEST_USER); + + // Step 5: Try ACL operations on new leader - should fail with PERMISSION_DENIED + OzoneAcl anotherAcl = OzoneAcl.parseAcl("user:yetanotheruser:r[ACCESS]"); + + // Add ACL should fail + OMException addException = assertThrows(OMException.class, () -> { + userObjectStore.addAcl(keyObj, anotherAcl); + }, "addAcl should fail for non-admin user on new leader"); + assertEquals(PERMISSION_DENIED, addException.getResult(), + "Should get PERMISSION_DENIED when ACL check fails in preExecute"); + + // Remove ACL should fail + OMException removeException = assertThrows(OMException.class, () -> { + userObjectStore.removeAcl(keyObj, setAcl); + }, "removeAcl should fail for non-admin user on new leader"); + assertEquals(PERMISSION_DENIED, removeException.getResult(), + "Should get PERMISSION_DENIED when ACL check fails in preExecute"); + + // Set ACL should fail + OzoneAcl newSetAcl = OzoneAcl.parseAcl("user:failuser:w[ACCESS]"); + OMException setException = assertThrows(OMException.class, () -> { + userObjectStore.setAcl(keyObj, Collections.singletonList(newSetAcl)); + }, "setAcl should fail for non-admin user on new leader"); + assertEquals(PERMISSION_DENIED, setException.getResult(), + "Should get PERMISSION_DENIED when ACL check fails in preExecute"); + + } finally { + UserGroupInformation.setLoginUser(adminUserUgi); + } + + // Step 6: Verify the ACLs remain unchanged (operations were rejected in preExecute) + List finalAcls = adminObjectStore.getAcl(keyObj); + assertThat(finalAcls).hasSize(1); + assertThat(finalAcls).contains(OzoneAcl.parseAcl("user:setuser:rwx[ACCESS]")); + } + /** * Helper method to check if volume exists. */ @@ -387,41 +1000,41 @@ private boolean volumeExists(ObjectStore store, String volumeName) { /** * Transfers leadership from current leader to another OM node. - * + * * @param currentLeader the current leader OM * @return the new leader OM after transfer */ private OzoneManager transferLeadershipToAnotherNode(OzoneManager currentLeader) throws Exception { // Get list of all OMs List omList = new ArrayList<>(cluster.getOzoneManagersList()); - + // Remove current leader from list omList.remove(currentLeader); - + // Select the first alternative OM as target OzoneManager targetOM = omList.get(0); String targetNodeId = targetOM.getOMNodeId(); - + // Transfer leadership currentLeader.transferLeadership(targetNodeId); - + // Wait for leadership transfer to complete - GenericTestUtils.waitFor(() -> { + BooleanSupplier leadershipTransferCheck = () -> { try { - OzoneManager currentLeaderCheck = cluster.getOMLeader(); - return !currentLeaderCheck.getOMNodeId().equals(currentLeader.getOMNodeId()); + return !cluster.getOMLeader().getOMNodeId().equals(currentLeader.getOMNodeId()); } catch (Exception e) { return false; } - }, 1000, 30000); - + }; + GenericTestUtils.waitFor(leadershipTransferCheck, 1000, 30000); + // Verify leadership change cluster.waitForLeaderOM(); OzoneManager newLeader = cluster.getOMLeader(); - - assertEquals(targetNodeId, newLeader.getOMNodeId(), + + assertEquals(targetNodeId, newLeader.getOMNodeId(), "Leadership should have transferred to target OM"); - + return newLeader; } } 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 3c5f028bb5d9..ff7c06f89832 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)); @@ -118,7 +143,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut acquiredBucketLock = getOmLockDetails().isLockAcquired(); // No need to check volume exists here, as bucket cannot be created - // with out volume creation. Check if bucket exists + // without volume creation. Check if bucket exists String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName); OmBucketInfo omBucketInfo = @@ -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..d1755c03f0fc 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)); @@ -301,12 +316,12 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager, OMException.ResultCodes.QUOTA_ERROR); } } - + // avoid iteration of other bucket if quota set is less than previous set if (quotaInBytes < dbBucketInfo.getQuotaInBytes()) { return true; } - + List bucketList = metadataManager.listBuckets( omVolumeArgs.getVolume(), null, null, Integer.MAX_VALUE, false); for (OmBucketInfo bucketInfo : bucketList) { @@ -342,7 +357,7 @@ public boolean checkQuotaNamespaceValid(OmVolumeArgs omVolumeArgs, if (quotaInNamespace < OzoneConsts.QUOTA_RESET || quotaInNamespace == 0) { return false; } - + if (quotaInNamespace != OzoneConsts.QUOTA_RESET && quotaInNamespace < dbBucketInfo.getTotalBucketNamespace()) { throw new OMException("Cannot update bucket quota. NamespaceQuota " + 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() 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 2f8562642982..27e2b25fc9a7 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,64 @@ 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(); + + // Resolve bucket link + ResolvedBucket resolvedBucketObj = ozoneManager.resolveBucketLink( + Pair.of(volumeName, bucketName)); + String resolvedVolume = resolvedBucketObj.realVolume(); + String resolvedBucket = resolvedBucketObj.realBucket(); + + // ACL check during preExecute - filter out keys that fail ACL check + List keysPassingAcl = new ArrayList<>(); + if (ozoneManager.getAclsEnabled()) { + for (String keyName : keys) { + try { + checkKeyAcls(ozoneManager, resolvedVolume, resolvedBucket, keyName, + IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY); + keysPassingAcl.add(keyName); + } catch (IOException ex) { + // Log ACL failure but continue processing other keys + LOG.warn("ACL check failed for key {} during preExecute, key will be skipped: {}", + keyName, ex.getMessage()); + // Audit the failure + 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 { + // ACLs not enabled, all keys pass + keysPassingAcl.addAll(keys); + } + + // Build modified request with only keys that passed ACL check + DeleteKeyArgs.Builder modifiedArgs = deleteKeyArgs.toBuilder() + .clearKeys() + .addAllKeys(keysPassingAcl); + + 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,8 +205,8 @@ 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); + // ACL check already done in preExecute, keys in the request have passed ACL for (indexFailed = 0; indexFailed < length; indexFailed++) { String keyName = deleteKeyArgs.getKeys(indexFailed); String objectKey = @@ -166,25 +224,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..6c17b31f4e87 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(); + + // Resolve bucket link + ResolvedBucket resolvedBucketObj = ozoneManager.resolveBucketLink( + Pair.of(volumeName, bucketName)); + String resolvedVolume = resolvedBucketObj.realVolume(); + String resolvedBucket = resolvedBucketObj.realBucket(); + + // ACL check during preExecute - filter out key pairs that fail ACL check + List keyPairsPassingAcl = new ArrayList<>(); + if (ozoneManager.getAclsEnabled()) { + for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) { + String fromKeyName = renameKey.getFromKeyName(); + String toKeyName = renameKey.getToKeyName(); + + // Skip empty key names - they will be handled in validateAndUpdateCache + if (fromKeyName.isEmpty() || toKeyName.isEmpty()) { + keyPairsPassingAcl.add(renameKey); + continue; + } + + try { + // Check ACLs: DELETE permission on source key, CREATE permission on destination key + checkKeyAcls(ozoneManager, resolvedVolume, resolvedBucket, fromKeyName, + IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY); + checkKeyAcls(ozoneManager, resolvedVolume, resolvedBucket, toKeyName, + IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY); + keyPairsPassingAcl.add(renameKey); + } catch (IOException ex) { + // Log ACL failure but continue processing other key pairs + LOG.warn("ACL check failed for rename {} -> {} during preExecute, key pair will be skipped: {}", + fromKeyName, toKeyName, ex.getMessage()); + // Audit the failure + 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 { + // ACLs not enabled, all key pairs pass + keyPairsPassingAcl.addAll(renameKeysArgs.getRenameKeysMapList()); + } + + // Build modified request with only key pairs that passed ACL check + RenameKeysArgs.Builder modifiedArgs = renameKeysArgs.toBuilder() + .clearRenameKeysMap() + .addAllRenameKeysMap(keyPairsPassingAcl); + + 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,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut // Validate bucket and volume exists or not. validateBucketAndVolume(omMetadataManager, volumeName, bucketName); - String volumeOwner = getVolumeOwner(omMetadataManager, volumeName); + + // ACL check already done in preExecute, key pairs in the request have passed ACL for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) { fromKeyName = renameKey.getFromKeyName(); @@ -142,25 +217,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(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java index ba32fe542c98..d90f67cac814 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java @@ -22,9 +22,12 @@ import java.io.IOException; import java.nio.file.InvalidPathException; +import java.util.LinkedHashMap; +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; +import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OMMetrics; @@ -57,6 +60,36 @@ public OMVolumeDeleteRequest(OMRequest omRequest) { super(omRequest); } + @Override + public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + super.preExecute(ozoneManager); + DeleteVolumeRequest deleteVolumeRequest = + getOmRequest().getDeleteVolumeRequest(); + Objects.requireNonNull(deleteVolumeRequest); + String volume = deleteVolumeRequest.getVolumeName(); + + // ACL check during preExecute + if (ozoneManager.getAclsEnabled()) { + try { + checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, + OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.DELETE, volume, + null, null); + } catch (IOException ex) { + // Ensure audit log captures preExecute failures + Map auditMap = new LinkedHashMap<>(); + auditMap.put(OzoneConsts.VOLUME, volume); + markForAudit(ozoneManager.getAuditLogger(), + buildAuditMessage(OMAction.DELETE_VOLUME, 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(); @@ -80,13 +113,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut String owner = null; OMClientResponse omClientResponse = null; try { - // check Acl - if (ozoneManager.getAclsEnabled()) { - checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, - OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.DELETE, volume, - null, null); - } - mergeOmLockDetails(omMetadataManager.getLock().acquireWriteLock( VOLUME_LOCK, volume)); acquiredVolumeLock = getOmLockDetails().isLockAcquired(); @@ -169,6 +195,4 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut } return omClientResponse; } - } - diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java index 02c3b7874e99..d4c5d155fe84 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.nio.file.InvalidPathException; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; @@ -61,15 +62,39 @@ public OMVolumeSetOwnerRequest(OMRequest omRequest) { @Override public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + super.preExecute(ozoneManager); long modificationTime = Time.now(); SetVolumePropertyRequest.Builder setPropertyRequestBuilder = getOmRequest() .getSetVolumePropertyRequest().toBuilder() .setModificationTime(modificationTime); + SetVolumePropertyRequest setVolumePropertyRequest = + getOmRequest().getSetVolumePropertyRequest(); + String volume = setVolumePropertyRequest.getVolumeName(); + + // ACL check during preExecute + if (ozoneManager.getAclsEnabled()) { + try { + checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, + OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, + volume, null, null); + } catch (IOException ex) { + // Ensure audit log captures preExecute failures + Map auditMap = new LinkedHashMap<>(); + auditMap.put(OzoneConsts.VOLUME, volume); + auditMap.put(OzoneConsts.OWNER, + setVolumePropertyRequest.getOwnerName()); + markForAudit(ozoneManager.getAuditLogger(), + buildAuditMessage(OMAction.SET_OWNER, auditMap, ex, + getOmRequest().getUserInfo())); + throw ex; + } + } + return getOmRequest().toBuilder() .setSetVolumePropertyRequest(setPropertyRequestBuilder) - .setUserInfo(getUserInfo()) + .setUserInfo(getUserIfNotExists(ozoneManager)) .build(); } @@ -108,13 +133,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut String oldOwner = null; OMClientResponse omClientResponse = null; try { - // check Acl - if (ozoneManager.getAclsEnabled()) { - checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, - OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, null, null); - } - long maxUserVolumeCount = ozoneManager.getMaxUserVolumeCount(); OzoneManagerStorageProtos.PersistedUserVolumeInfo oldOwnerVolumeList; OzoneManagerStorageProtos.PersistedUserVolumeInfo newOwnerVolumeList; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java index c93e8cbeb6c3..8c8144a3b0f1 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.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 java.util.Objects; @@ -63,15 +64,39 @@ public OMVolumeSetQuotaRequest(OMRequest omRequest) { @Override public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + super.preExecute(ozoneManager); long modificationTime = Time.now(); - SetVolumePropertyRequest.Builder setPropertyRequestBuilde = getOmRequest() + SetVolumePropertyRequest.Builder setPropertyRequestBuilder = getOmRequest() .getSetVolumePropertyRequest().toBuilder() .setModificationTime(modificationTime); + SetVolumePropertyRequest setVolumePropertyRequest = + getOmRequest().getSetVolumePropertyRequest(); + String volume = setVolumePropertyRequest.getVolumeName(); + + // ACL check during preExecute + if (ozoneManager.getAclsEnabled()) { + try { + checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, + OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE, volume, + null, null); + } catch (IOException ex) { + // Ensure audit log captures preExecute failures + Map auditMap = new LinkedHashMap<>(); + auditMap.put(OzoneConsts.VOLUME, volume); + auditMap.put(OzoneConsts.QUOTA_IN_BYTES, + String.valueOf(setVolumePropertyRequest.getQuotaInBytes())); + markForAudit(ozoneManager.getAuditLogger(), + buildAuditMessage(OMAction.SET_QUOTA, auditMap, ex, + getOmRequest().getUserInfo())); + throw ex; + } + } + return getOmRequest().toBuilder() - .setSetVolumePropertyRequest(setPropertyRequestBuilde) - .setUserInfo(getUserInfo()) + .setSetVolumePropertyRequest(setPropertyRequestBuilder) + .setUserInfo(getUserIfNotExists(ozoneManager)) .build(); } @@ -109,13 +134,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut boolean acquireVolumeLock = false; OMClientResponse omClientResponse = null; try { - // check Acl - if (ozoneManager.getAclsEnabled()) { - checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, - OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE, volume, - null, null); - } - mergeOmLockDetails(omMetadataManager.getLock().acquireWriteLock( VOLUME_LOCK, volume)); acquireVolumeLock = getOmLockDetails().isLockAcquired(); @@ -184,7 +202,7 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager, || volumeQuotaInBytes == 0) { return false; } - + // if volume quota is for reset, no need further check if (volumeQuotaInBytes == OzoneConsts.QUOTA_RESET) { return true; @@ -205,13 +223,13 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager, break; } } - + if (!isBucketQuotaSet) { throw new OMException("Can not set volume space quota on volume " + "as some of buckets in this volume have no quota set.", OMException.ResultCodes.QUOTA_ERROR); } - + if (volumeQuotaInBytes < totalBucketQuota && volumeQuotaInBytes != OzoneConsts.QUOTA_RESET) { throw new OMException("Total buckets quota in this volume " + diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java index 23b522ad77a6..0af78aa9d73a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.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.hadoop.hdds.utils.db.cache.CacheKey; @@ -28,6 +29,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; @@ -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); + + // ACL check during preExecute + if (ozoneManager.getAclsEnabled()) { + String volume = getVolumeName(); + try { + checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, + OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, + volume, null, null); + } catch (IOException ex) { + // Ensure audit log captures preExecute failures + Map auditMap = new LinkedHashMap<>(); + auditMap.put(OzoneConsts.VOLUME, volume); + 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 trxnLogIndex = context.getIndex(); @@ -71,12 +110,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut boolean lockAcquired = false; Result result; try { - // check Acl - if (ozoneManager.getAclsEnabled()) { - checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, - OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, null, null); - } mergeOmLockDetails(omMetadataManager.getLock().acquireWriteLock( VOLUME_LOCK, volume)); lockAcquired = getOmLockDetails().isLockAcquired(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java index c0e87043ea34..6dfc64547189 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java @@ -58,14 +58,16 @@ public class OMVolumeAddAclRequest extends OMVolumeAclRequest { @Override public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + // Call parent preExecute to perform ACL check + OMRequest omRequest = super.preExecute(ozoneManager); + long modificationTime = Time.now(); OzoneManagerProtocolProtos.AddAclRequest.Builder addAclRequestBuilder = - getOmRequest().getAddAclRequest().toBuilder() + omRequest.getAddAclRequest().toBuilder() .setModificationTime(modificationTime); - return getOmRequest().toBuilder() + return omRequest.toBuilder() .setAddAclRequest(addAclRequestBuilder) - .setUserInfo(getUserInfo()) .build(); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeRemoveAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeRemoveAclRequest.java index 05f338957ee6..ceabf00b5663 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeRemoveAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeRemoveAclRequest.java @@ -58,14 +58,16 @@ public class OMVolumeRemoveAclRequest extends OMVolumeAclRequest { @Override public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + // Call parent preExecute to perform ACL check + OMRequest omRequest = super.preExecute(ozoneManager); + long modificationTime = Time.now(); OzoneManagerProtocolProtos.RemoveAclRequest.Builder removeAclRequestBuilder - = getOmRequest().getRemoveAclRequest().toBuilder() + = omRequest.getRemoveAclRequest().toBuilder() .setModificationTime(modificationTime); - return getOmRequest().toBuilder() + return omRequest.toBuilder() .setRemoveAclRequest(removeAclRequestBuilder) - .setUserInfo(getUserInfo()) .build(); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeSetAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeSetAclRequest.java index 6abffc2197fa..c68f24906d71 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeSetAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeSetAclRequest.java @@ -57,14 +57,16 @@ public class OMVolumeSetAclRequest extends OMVolumeAclRequest { @Override public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + // Call parent preExecute to perform ACL check + OMRequest omRequest = super.preExecute(ozoneManager); + long modificationTime = Time.now(); OzoneManagerProtocolProtos.SetAclRequest.Builder setAclRequestBuilder = - getOmRequest().getSetAclRequest().toBuilder() + omRequest.getSetAclRequest().toBuilder() .setModificationTime(modificationTime); - return getOmRequest().toBuilder() + return omRequest.toBuilder() .setSetAclRequest(setAclRequestBuilder) - .setUserInfo(getUserInfo()) .build(); }