From fa19431a184c5e06a7327c2a8940a220fadb9a8b Mon Sep 17 00:00:00 2001 From: Sammi Chen Date: Tue, 14 Jul 2026 11:52:32 +0800 Subject: [PATCH 1/3] Revert "HDDS-15467. Do not fall back to the OM starter user in OMClientRequest (#10469)" This reverts commit 6f0b5a3b3909239fa672883317fec3bf84c202c8. --- .../ozone/om/request/OMClientRequest.java | 39 +++++-- .../request/key/OMAllocateBlockRequest.java | 4 +- .../om/request/key/OMKeyDeleteRequest.java | 2 +- .../om/request/key/OMKeyRenameRequest.java | 2 +- .../TestOMClientRequestUserInfoFallback.java | 106 ------------------ 5 files changed, 35 insertions(+), 118 deletions(-) delete mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMClientRequestUserInfoFallback.java diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java index 9ffdef1784b7..884c5fa310bc 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java @@ -115,7 +115,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) .setVersion(ozoneManager.getVersionManager().getMetadataLayoutVersion()) .build(); omRequest = getOmRequest().toBuilder() - .setUserInfo(getUserInfo()) + .setUserInfo(getUserIfNotExists(ozoneManager)) .setLayoutVersion(layoutVersion).build(); return omRequest; } @@ -193,18 +193,41 @@ public OzoneManagerProtocolProtos.UserInfo getUserInfo() throws IOException { && grpcContextClientIpAddress != null) { userInfo.setHostName(grpcContextClientHostname); userInfo.setRemoteAddress(grpcContextClientIpAddress); - } else if (omRequest.hasUserInfo() - && omRequest.getUserInfo().hasRemoteAddress()) { - // For non-RPC internal service requests (e.g. the Trash emptier) that - // populate their own UserInfo, preserve the supplied host/address since - // there is no RPC or gRPC client context to derive it from. - userInfo.setHostName(omRequest.getUserInfo().getHostName()); - userInfo.setRemoteAddress(omRequest.getUserInfo().getRemoteAddress()); } return userInfo.build(); } + /** + * For non-rpc internal calls Server.getRemoteUser() + * and Server.getRemoteIp() will be null. + * Passing getCurrentUser() and Ip of the Om node that started it. + * @return User Info. + */ + public OzoneManagerProtocolProtos.UserInfo getUserIfNotExists( + OzoneManager ozoneManager) throws IOException { + OzoneManagerProtocolProtos.UserInfo userInfo = getUserInfo(); + if (!userInfo.hasRemoteAddress() || !userInfo.hasUserName()) { + OzoneManagerProtocolProtos.UserInfo.Builder newuserInfo = + OzoneManagerProtocolProtos.UserInfo.newBuilder(); + UserGroupInformation user; + InetAddress remoteAddress; + try { + user = UserGroupInformation.getCurrentUser(); + remoteAddress = ozoneManager.getOmRpcServerAddr() + .getAddress(); + } catch (Exception e) { + LOG.debug("Couldn't get om Rpc server address", e); + return getUserInfo(); + } + newuserInfo.setUserName(user.getUserName()); + newuserInfo.setHostName(remoteAddress.getHostName()); + newuserInfo.setRemoteAddress(remoteAddress.getHostAddress()); + return newuserInfo.build(); + } + return getUserInfo(); + } + /** * Check Acls of ozone object. * @param ozoneManager diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java index f85cc8b777e3..0e11f1d76773 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java @@ -102,7 +102,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { // BlockOutputStreamEntryPool, so we are fine for now. But if one some // one uses direct omclient we might be in trouble. - UserInfo userInfo = getOmRequest().getUserInfo(); + UserInfo userInfo = getUserIfNotExists(ozoneManager); ReplicationConfig repConfig = ReplicationConfig.fromProto(keyArgs.getType(), keyArgs.getFactor(), keyArgs.getEcReplicationConfig()); // To allocate atleast one block passing requested size and scmBlockSize @@ -133,7 +133,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { newAllocatedBlockRequest.setKeyLocation( omKeyLocationInfoList.get(0).getProtobuf(getOmRequest().getVersion())); - return getOmRequest().toBuilder() + return getOmRequest().toBuilder().setUserInfo(userInfo) .setAllocateBlockRequest(newAllocatedBlockRequest).build(); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java index 820c4d171979..26287ca66d26 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java @@ -92,7 +92,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { return getOmRequest().toBuilder() .setDeleteKeyRequest(deleteKeyRequest.toBuilder() .setKeyArgs(resolvedArgs)) - .build(); + .setUserInfo(getUserIfNotExists(ozoneManager)).build(); } protected KeyArgs resolveBucketAndCheckAcls(OzoneManager ozoneManager, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java index 2c2e54d3a7be..850f111a913f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java @@ -95,7 +95,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { return getOmRequest().toBuilder() .setRenameKeyRequest(renameKeyRequest.toBuilder().setToKeyName(dstKey) .setKeyArgs(resolvedArgs)) - .build(); + .setUserInfo(getUserIfNotExists(ozoneManager)).build(); } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMClientRequestUserInfoFallback.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMClientRequestUserInfoFallback.java deleted file mode 100644 index 202ada1b016c..000000000000 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMClientRequestUserInfoFallback.java +++ /dev/null @@ -1,106 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.hadoop.ozone.om.request; - -import static org.apache.hadoop.ozone.om.request.OMRequestTestUtils.newBucketInfoBuilder; -import static org.apache.hadoop.ozone.om.request.OMRequestTestUtils.newCreateBucketRequest; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.mockStatic; - -import java.util.UUID; -import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StorageTypeProto; -import org.apache.hadoop.ipc_.Server; -import org.apache.hadoop.ozone.om.request.bucket.OMBucketCreateRequest; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.BucketInfo; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.UserInfo; -import org.apache.hadoop.security.authentication.client.AuthenticationException; -import org.junit.jupiter.api.Test; -import org.mockito.MockedStatic; - -/** - * Tests that {@link OMClientRequest} does not silently fall back to the OM - * starter/login user when a request carries no user information (HDDS-15467). - */ -public class TestOMClientRequestUserInfoFallback { - - private OMRequest newBucketRequest(UserInfo userInfo) { - BucketInfo.Builder bucketInfo = newBucketInfoBuilder( - UUID.randomUUID().toString(), UUID.randomUUID().toString()) - .setIsVersionEnabled(true) - .setStorageType(StorageTypeProto.DISK); - OMRequest.Builder builder = newCreateBucketRequest(bucketInfo); - if (userInfo != null) { - builder.setUserInfo(userInfo); - } - return builder.build(); - } - - /** - * With no RPC/gRPC context and no UserInfo on the request, getUserInfo() must - * not manufacture an identity from the OM starter user; createUGI() then - * fails closed instead of silently escalating. - */ - @Test - public void noFallbackToServerUserWhenUserInfoMissing() throws Exception { - try (MockedStatic mockedRpcServer = mockStatic(Server.class)) { - mockedRpcServer.when(Server::getRemoteUser).thenReturn(null); - mockedRpcServer.when(Server::getRemoteIp).thenReturn(null); - - OMRequest omRequest = newBucketRequest(null); - OMClientRequest request = new OMBucketCreateRequest(omRequest); - - UserInfo userInfo = request.getUserInfo(); - assertFalse(userInfo.hasUserName()); - assertFalse(userInfo.hasRemoteAddress()); - - OMClientRequest withUserInfo = new OMBucketCreateRequest( - omRequest.toBuilder().setUserInfo(userInfo).build()); - assertThrows(AuthenticationException.class, withUserInfo::createUGI); - } - } - - /** - * An internal service (e.g. the Trash emptier) populates its own UserInfo. - * With no RPC/gRPC context, getUserInfo() must preserve that identity rather - * than replacing it with the OM starter user. - */ - @Test - public void internalServiceUserInfoIsPreserved() throws Exception { - try (MockedStatic mockedRpcServer = mockStatic(Server.class)) { - mockedRpcServer.when(Server::getRemoteUser).thenReturn(null); - mockedRpcServer.when(Server::getRemoteIp).thenReturn(null); - - UserInfo serviceUserInfo = UserInfo.newBuilder() - .setUserName("trash-service-user") - .setHostName("om-host") - .setRemoteAddress("10.0.0.9") - .build(); - - OMClientRequest request = - new OMBucketCreateRequest(newBucketRequest(serviceUserInfo)); - - UserInfo result = request.getUserInfo(); - assertEquals("trash-service-user", result.getUserName()); - assertEquals("10.0.0.9", result.getRemoteAddress()); - assertEquals("om-host", result.getHostName()); - } - } -} From 613e1d2c1e8b5fb12ca90cd536bd498e82bcc98a Mon Sep 17 00:00:00 2001 From: Priyesh Karatha Date: Tue, 14 Jul 2026 11:51:18 +0530 Subject: [PATCH 2/3] HDDS-15853. Fix Ranger ACL validation for lifecycle requests --- ...OMLifecycleConfigurationDeleteRequest.java | 21 ++++++++++++++++--- .../OMLifecycleConfigurationSetRequest.java | 21 ++++++++++++++++--- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationDeleteRequest.java index 56cbf6d96039..66ea614a0b07 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationDeleteRequest.java @@ -49,6 +49,7 @@ import org.apache.hadoop.ozone.request.validation.RequestProcessingPhase; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.OzoneObj; +import org.apache.hadoop.security.UserGroupInformation; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -77,9 +78,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { Pair.of(volumeName, bucketName), this); if (ozoneManager.getAclsEnabled()) { - checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, OzoneObj.StoreType.OZONE, - IAccessAuthorizer.ACLType.ALL, resolvedBucket.realVolume(), - resolvedBucket.realBucket(), null); + checkAclPermission(ozoneManager, resolvedBucket.realVolume(), resolvedBucket.realBucket()); } // Update the request with resolved volume and bucket names @@ -175,6 +174,22 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut } } + private void checkAclPermission(OzoneManager ozoneManager, String volumeName, String bucketName) + throws IOException { + if (ozoneManager.getAccessAuthorizer().isNative()) { + UserGroupInformation ugi = createUGIForApi(); + String bucketOwner = ozoneManager.getBucketOwner(volumeName, bucketName, + IAccessAuthorizer.ACLType.READ, OzoneObj.ResourceType.BUCKET); + if (!ozoneManager.isAdmin(ugi) && !ozoneManager.isOwner(ugi, bucketOwner)) { + throw new OMException("Lifecycle configuration can only be deleted by bucket Admin or Owner", + OMException.ResultCodes.PERMISSION_DENIED); + } + } else { + checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, OzoneObj.StoreType.OZONE, + IAccessAuthorizer.ACLType.WRITE, volumeName, bucketName, null); + } + } + @RequestFeatureValidator( conditions = ValidationCondition.CLUSTER_NEEDS_FINALIZATION, processingPhase = RequestProcessingPhase.PRE_PROCESS, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationSetRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationSetRequest.java index 86edbaad1801..ec3abe4e522a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationSetRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationSetRequest.java @@ -54,6 +54,7 @@ import org.apache.hadoop.ozone.request.validation.RequestProcessingPhase; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.OzoneObj; +import org.apache.hadoop.security.UserGroupInformation; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -86,9 +87,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { Pair.of(volumeName, bucketName), this); if (ozoneManager.getAclsEnabled()) { - checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, OzoneObj.StoreType.OZONE, - IAccessAuthorizer.ACLType.ALL, resolvedBucket.realVolume(), - resolvedBucket.realBucket(), null); + checkAclPermission(ozoneManager, resolvedBucket.realVolume(), resolvedBucket.realBucket()); } if (resolvedBucket.bucketLayout().toProto() != request.getLifecycleConfiguration().getBucketLayout()) { @@ -204,6 +203,22 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut } } + private void checkAclPermission(OzoneManager ozoneManager, String volumeName, String bucketName) + throws IOException { + if (ozoneManager.getAccessAuthorizer().isNative()) { + UserGroupInformation ugi = createUGIForApi(); + String bucketOwner = ozoneManager.getBucketOwner(volumeName, bucketName, + IAccessAuthorizer.ACLType.READ, OzoneObj.ResourceType.BUCKET); + if (!ozoneManager.isAdmin(ugi) && !ozoneManager.isOwner(ugi, bucketOwner)) { + throw new OMException("Lifecycle configuration can only be set by bucket Admin or Owner", + OMException.ResultCodes.PERMISSION_DENIED); + } + } else { + checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, OzoneObj.StoreType.OZONE, + IAccessAuthorizer.ACLType.WRITE, volumeName, bucketName, null); + } + } + @RequestFeatureValidator( conditions = ValidationCondition.CLUSTER_NEEDS_FINALIZATION, processingPhase = RequestProcessingPhase.PRE_PROCESS, From f3edff3d40e597ecaff6f07b534d8b89d9b0b693 Mon Sep 17 00:00:00 2001 From: Priyesh Karatha Date: Tue, 14 Jul 2026 21:41:54 +0530 Subject: [PATCH 3/3] addressing review commets --- ...OMLifecycleConfigurationDeleteRequest.java | 2 +- .../OMLifecycleConfigurationSetRequest.java | 2 +- .../OMLifecycleSetServiceStatusRequest.java | 67 ++++++----- ...OMLifecycleConfigurationDeleteRequest.java | 106 ++++++++++++++++++ ...estOMLifecycleConfigurationSetRequest.java | 106 ++++++++++++++++++ 5 files changed, 247 insertions(+), 36 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationDeleteRequest.java index 66ea614a0b07..3d4100e06fc9 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationDeleteRequest.java @@ -181,7 +181,7 @@ private void checkAclPermission(OzoneManager ozoneManager, String volumeName, St String bucketOwner = ozoneManager.getBucketOwner(volumeName, bucketName, IAccessAuthorizer.ACLType.READ, OzoneObj.ResourceType.BUCKET); if (!ozoneManager.isAdmin(ugi) && !ozoneManager.isOwner(ugi, bucketOwner)) { - throw new OMException("Lifecycle configuration can only be deleted by bucket Admin or Owner", + throw new OMException("Lifecycle configuration can only be deleted by cluster Admin or bucket Owner", OMException.ResultCodes.PERMISSION_DENIED); } } else { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationSetRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationSetRequest.java index ec3abe4e522a..533809437888 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationSetRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationSetRequest.java @@ -210,7 +210,7 @@ private void checkAclPermission(OzoneManager ozoneManager, String volumeName, St String bucketOwner = ozoneManager.getBucketOwner(volumeName, bucketName, IAccessAuthorizer.ACLType.READ, OzoneObj.ResourceType.BUCKET); if (!ozoneManager.isAdmin(ugi) && !ozoneManager.isOwner(ugi, bucketOwner)) { - throw new OMException("Lifecycle configuration can only be set by bucket Admin or Owner", + throw new OMException("Lifecycle configuration can only be set by cluster Admin or bucket Owner", OMException.ResultCodes.PERMISSION_DENIED); } } else { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleSetServiceStatusRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleSetServiceStatusRequest.java index 99ab7813d5da..f07e7798b5db 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleSetServiceStatusRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleSetServiceStatusRequest.java @@ -55,54 +55,53 @@ public OMLifecycleSetServiceStatusRequest(OMRequest omRequest) { super(omRequest); } + @Override + public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + OMRequest request = super.preExecute(ozoneManager); + + if (ozoneManager.getAclsEnabled()) { + boolean suspend = request.getSetLifecycleServiceStatusRequest().getSuspend(); + UserGroupInformation ugi = createUGIForApi(); + if (!ozoneManager.isAdmin(ugi)) { + throw new OMException("Access denied for user " + ugi + ". " + + "Superuser privilege is required to " + (suspend ? "suspend" : "resume") + " Lifecycle Service.", + OMException.ResultCodes.ACCESS_DENIED); + } + } + + return request; + } + @Override public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) { OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(getOmRequest()); AuditLogger auditLogger = ozoneManager.getAuditLogger(); UserInfo userInfo = getOmRequest().getUserInfo(); HashMap auditMap = new HashMap<>(); - IOException exception = null; - OMClientResponse omClientResponse; boolean suspend = getOmRequest().getSetLifecycleServiceStatusRequest().getSuspend(); auditMap.put("suspend", String.valueOf(suspend)); - try { - if (ozoneManager.getAclsEnabled()) { - UserGroupInformation ugi = createUGIForApi(); - if (!ozoneManager.isAdmin(ugi)) { - throw new OMException("Access denied for user " + ugi + ". " - + "Superuser privilege is required to " + (suspend ? "suspend" : "resume") + " Lifecycle Service.", - OMException.ResultCodes.ACCESS_DENIED); - } - } - - KeyLifecycleService keyLifecycleService = ozoneManager.getKeyManager().getKeyLifecycleService(); - if (keyLifecycleService != null) { - if (suspend) { - keyLifecycleService.suspend(); - LOG.info("KeyLifecycleService has been suspended by user: {}", - userInfo != null ? userInfo.getUserName() : "unknown"); - } else { - keyLifecycleService.resume(); - LOG.info("KeyLifecycleService resume called by user: {}", - userInfo != null ? userInfo.getUserName() : "unknown"); - } + KeyLifecycleService keyLifecycleService = ozoneManager.getKeyManager().getKeyLifecycleService(); + if (keyLifecycleService != null) { + if (suspend) { + keyLifecycleService.suspend(); + LOG.info("KeyLifecycleService has been suspended by user: {}", + userInfo != null ? userInfo.getUserName() : "unknown"); } else { - LOG.warn("KeyLifecycleService is not available"); + keyLifecycleService.resume(); + LOG.info("KeyLifecycleService resume called by user: {}", + userInfo != null ? userInfo.getUserName() : "unknown"); } - - omResponse.setSetLifecycleServiceStatusResponse( - SetLifecycleServiceStatusResponse.newBuilder().build()); - omClientResponse = new OMLifecycleSetServiceStatusResponse(omResponse.build()); - } catch (IOException ex) { - exception = ex; - LOG.error("Failed to " + (suspend ? "suspend" : "resume") + " KeyLifecycleService", ex); - omClientResponse = new OMLifecycleSetServiceStatusResponse( - createErrorOMResponse(omResponse, ex)); + } else { + LOG.warn("KeyLifecycleService is not available"); } + omResponse.setSetLifecycleServiceStatusResponse( + SetLifecycleServiceStatusResponse.newBuilder().build()); + OMClientResponse omClientResponse = new OMLifecycleSetServiceStatusResponse(omResponse.build()); + markForAudit(auditLogger, buildAuditMessage(OMAction.SET_LIFECYCLE_SERVICE_STATUS, - auditMap, exception, userInfo)); + auditMap, null, userInfo)); return omClientResponse; } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/lifecycle/TestOMLifecycleConfigurationDeleteRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/lifecycle/TestOMLifecycleConfigurationDeleteRequest.java index 858bba1fc8ef..ed7a9e146a01 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/lifecycle/TestOMLifecycleConfigurationDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/lifecycle/TestOMLifecycleConfigurationDeleteRequest.java @@ -24,7 +24,13 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.isNull; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.io.IOException; @@ -41,7 +47,10 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; +import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.apache.hadoop.ozone.upgrade.LayoutVersionManager; +import org.apache.hadoop.security.UserGroupInformation; import org.junit.jupiter.api.Test; /** @@ -175,6 +184,103 @@ public void testDisallowDeleteLifecycleConfigurationBeforeFinalization() throws ex.getResult()); } + @Test + public void testPreExecuteNonNativeAuthorizerChecksWriteAcl() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + + when(ozoneManager.getAclsEnabled()).thenReturn(true); + IAccessAuthorizer authorizer = mock(IAccessAuthorizer.class); + when(authorizer.isNative()).thenReturn(false); + when(ozoneManager.getAccessAuthorizer()).thenReturn(authorizer); + + OMRequest omRequest = + createDeleteLifecycleConfigurationRequest(volumeName, bucketName); + OMLifecycleConfigurationDeleteRequest request = + spy(new OMLifecycleConfigurationDeleteRequest(omRequest)); + // Stub the ACL check so the branch can be asserted without a real authorizer. + doNothing().when(request).checkAcls(eq(ozoneManager), eq(OzoneObj.ResourceType.BUCKET), + eq(OzoneObj.StoreType.OZONE), eq(IAccessAuthorizer.ACLType.WRITE), + eq(volumeName), eq(bucketName), isNull()); + + request.preExecute(ozoneManager); + + verify(request).checkAcls(eq(ozoneManager), eq(OzoneObj.ResourceType.BUCKET), + eq(OzoneObj.StoreType.OZONE), eq(IAccessAuthorizer.ACLType.WRITE), + eq(volumeName), eq(bucketName), isNull()); + } + + @Test + public void testPreExecuteNativeAuthorizerDeniesNonAdminNonOwner() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + + when(ozoneManager.getAclsEnabled()).thenReturn(true); + IAccessAuthorizer authorizer = mock(IAccessAuthorizer.class); + when(authorizer.isNative()).thenReturn(true); + when(ozoneManager.getAccessAuthorizer()).thenReturn(authorizer); + when(ozoneManager.getBucketOwner(eq(volumeName), eq(bucketName), + any(IAccessAuthorizer.ACLType.class), any(OzoneObj.ResourceType.class))) + .thenReturn("bucketOwner"); + when(ozoneManager.isAdmin(any(UserGroupInformation.class))).thenReturn(false); + when(ozoneManager.isOwner(any(UserGroupInformation.class), anyString())).thenReturn(false); + + OMRequest omRequest = + createDeleteLifecycleConfigurationRequest(volumeName, bucketName); + OMLifecycleConfigurationDeleteRequest request = + new OMLifecycleConfigurationDeleteRequest(omRequest); + request.setUGI(UserGroupInformation.createRemoteUser("regularUser")); + + OMException ex = assertThrows(OMException.class, + () -> request.preExecute(ozoneManager)); + assertEquals(OMException.ResultCodes.PERMISSION_DENIED, ex.getResult()); + } + + @Test + public void testPreExecuteNativeAuthorizerAllowsAdmin() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + + when(ozoneManager.getAclsEnabled()).thenReturn(true); + IAccessAuthorizer authorizer = mock(IAccessAuthorizer.class); + when(authorizer.isNative()).thenReturn(true); + when(ozoneManager.getAccessAuthorizer()).thenReturn(authorizer); + when(ozoneManager.isAdmin(any(UserGroupInformation.class))).thenReturn(true); + + OMRequest omRequest = + createDeleteLifecycleConfigurationRequest(volumeName, bucketName); + OMLifecycleConfigurationDeleteRequest request = + new OMLifecycleConfigurationDeleteRequest(omRequest); + request.setUGI(UserGroupInformation.createRemoteUser("adminUser")); + + assertNotNull(request.preExecute(ozoneManager)); + } + + @Test + public void testPreExecuteNativeAuthorizerAllowsOwner() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + + when(ozoneManager.getAclsEnabled()).thenReturn(true); + IAccessAuthorizer authorizer = mock(IAccessAuthorizer.class); + when(authorizer.isNative()).thenReturn(true); + when(ozoneManager.getAccessAuthorizer()).thenReturn(authorizer); + when(ozoneManager.getBucketOwner(eq(volumeName), eq(bucketName), + any(IAccessAuthorizer.ACLType.class), any(OzoneObj.ResourceType.class))) + .thenReturn("bucketOwner"); + when(ozoneManager.isAdmin(any(UserGroupInformation.class))).thenReturn(false); + when(ozoneManager.isOwner(any(UserGroupInformation.class), eq("bucketOwner"))) + .thenReturn(true); + + OMRequest omRequest = + createDeleteLifecycleConfigurationRequest(volumeName, bucketName); + OMLifecycleConfigurationDeleteRequest request = + new OMLifecycleConfigurationDeleteRequest(omRequest); + request.setUGI(UserGroupInformation.createRemoteUser("ownerUser")); + + assertNotNull(request.preExecute(ozoneManager)); + } + @Test public void testAllowDeleteLifecycleConfigurationAfterFinalization() throws Exception { String volumeName = UUID.randomUUID().toString(); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/lifecycle/TestOMLifecycleConfigurationSetRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/lifecycle/TestOMLifecycleConfigurationSetRequest.java index e79f7b5716ec..721a012f5a72 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/lifecycle/TestOMLifecycleConfigurationSetRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/lifecycle/TestOMLifecycleConfigurationSetRequest.java @@ -24,7 +24,13 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.isNull; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.UUID; @@ -44,7 +50,10 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; +import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; +import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.apache.hadoop.ozone.upgrade.LayoutVersionManager; +import org.apache.hadoop.security.UserGroupInformation; import org.junit.jupiter.api.Test; /** @@ -246,6 +255,103 @@ private void verifyRequest(OMRequest modifiedRequest, assertEquals(original.getRulesList(), updated.getRulesList()); } + @Test + public void testPreExecuteNonNativeAuthorizerChecksWriteAcl() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + + when(ozoneManager.getAclsEnabled()).thenReturn(true); + IAccessAuthorizer authorizer = mock(IAccessAuthorizer.class); + when(authorizer.isNative()).thenReturn(false); + when(ozoneManager.getAccessAuthorizer()).thenReturn(authorizer); + + OMRequest omRequest = + setLifecycleConfigurationRequest(volumeName, bucketName, "ownerName"); + OMLifecycleConfigurationSetRequest request = + spy(new OMLifecycleConfigurationSetRequest(omRequest)); + // Stub the ACL check so the branch can be asserted without a real authorizer. + doNothing().when(request).checkAcls(eq(ozoneManager), eq(OzoneObj.ResourceType.BUCKET), + eq(OzoneObj.StoreType.OZONE), eq(IAccessAuthorizer.ACLType.WRITE), + eq(volumeName), eq(bucketName), isNull()); + + request.preExecute(ozoneManager); + + verify(request).checkAcls(eq(ozoneManager), eq(OzoneObj.ResourceType.BUCKET), + eq(OzoneObj.StoreType.OZONE), eq(IAccessAuthorizer.ACLType.WRITE), + eq(volumeName), eq(bucketName), isNull()); + } + + @Test + public void testPreExecuteNativeAuthorizerDeniesNonAdminNonOwner() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + + when(ozoneManager.getAclsEnabled()).thenReturn(true); + IAccessAuthorizer authorizer = mock(IAccessAuthorizer.class); + when(authorizer.isNative()).thenReturn(true); + when(ozoneManager.getAccessAuthorizer()).thenReturn(authorizer); + when(ozoneManager.getBucketOwner(eq(volumeName), eq(bucketName), + any(IAccessAuthorizer.ACLType.class), any(OzoneObj.ResourceType.class))) + .thenReturn("bucketOwner"); + when(ozoneManager.isAdmin(any(UserGroupInformation.class))).thenReturn(false); + when(ozoneManager.isOwner(any(UserGroupInformation.class), anyString())).thenReturn(false); + + OMRequest omRequest = + setLifecycleConfigurationRequest(volumeName, bucketName, "ownerName"); + OMLifecycleConfigurationSetRequest request = + new OMLifecycleConfigurationSetRequest(omRequest); + request.setUGI(UserGroupInformation.createRemoteUser("regularUser")); + + OMException ex = assertThrows(OMException.class, + () -> request.preExecute(ozoneManager)); + assertEquals(OMException.ResultCodes.PERMISSION_DENIED, ex.getResult()); + } + + @Test + public void testPreExecuteNativeAuthorizerAllowsAdmin() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + + when(ozoneManager.getAclsEnabled()).thenReturn(true); + IAccessAuthorizer authorizer = mock(IAccessAuthorizer.class); + when(authorizer.isNative()).thenReturn(true); + when(ozoneManager.getAccessAuthorizer()).thenReturn(authorizer); + when(ozoneManager.isAdmin(any(UserGroupInformation.class))).thenReturn(true); + + OMRequest omRequest = + setLifecycleConfigurationRequest(volumeName, bucketName, "ownerName"); + OMLifecycleConfigurationSetRequest request = + new OMLifecycleConfigurationSetRequest(omRequest); + request.setUGI(UserGroupInformation.createRemoteUser("adminUser")); + + assertNotNull(request.preExecute(ozoneManager)); + } + + @Test + public void testPreExecuteNativeAuthorizerAllowsOwner() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + + when(ozoneManager.getAclsEnabled()).thenReturn(true); + IAccessAuthorizer authorizer = mock(IAccessAuthorizer.class); + when(authorizer.isNative()).thenReturn(true); + when(ozoneManager.getAccessAuthorizer()).thenReturn(authorizer); + when(ozoneManager.getBucketOwner(eq(volumeName), eq(bucketName), + any(IAccessAuthorizer.ACLType.class), any(OzoneObj.ResourceType.class))) + .thenReturn("bucketOwner"); + when(ozoneManager.isAdmin(any(UserGroupInformation.class))).thenReturn(false); + when(ozoneManager.isOwner(any(UserGroupInformation.class), eq("bucketOwner"))) + .thenReturn(true); + + OMRequest omRequest = + setLifecycleConfigurationRequest(volumeName, bucketName, "ownerName"); + OMLifecycleConfigurationSetRequest request = + new OMLifecycleConfigurationSetRequest(omRequest); + request.setUGI(UserGroupInformation.createRemoteUser("ownerUser")); + + assertNotNull(request.preExecute(ozoneManager)); + } + @Test public void testDisallowSetLifecycleConfigurationBeforeFinalization() throws Exception { String volumeName = UUID.randomUUID().toString();