This is an automated email from the ASF dual-hosted git repository.
Apache9 pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2 by this push:
new 66e750fe044 HBASE-30337 Miscellaneous improvements on rpc checks
(#8560) (#8589)
66e750fe044 is described below
commit 66e750fe044526ceaaf0eef49226660e486b92e2
Author: Duo Zhang <[email protected]>
AuthorDate: Thu Sep 3 22:15:56 2026 +0800
HBASE-30337 Miscellaneous improvements on rpc checks (#8560) (#8589)
Signed-off-by: Viraj Jasani <[email protected]>
Signed-off-by: Xiao Liu <[email protected]>
Reviewed-by: Aman Poonia <[email protected]>
(cherry picked from commit fb4286d3046ff525e8ffca8cb7fec1d2fb45b9f0)
---
.../hbase/security/access/AccessControlUtil.java | 16 ++
.../hadoop/hbase/security/access/Permission.java | 5 +-
.../security/access/ShadedAccessControlUtil.java | 16 ++
.../hadoop/hbase/coprocessor/MasterObserver.java | 52 ++++-
.../hadoop/hbase/master/MasterCoprocessorHost.java | 32 ++-
.../hadoop/hbase/master/MasterRpcServices.java | 18 +-
.../hadoop/hbase/regionserver/RSRpcServices.java | 6 +-
.../hbase/security/access/AccessController.java | 38 +++-
.../security/access/TestAccessController.java | 62 ++++++
.../TestAccessControllerObserverCoverage.java | 233 +++++++++++++++++++++
.../security/access/TestNamespaceCommands.java | 1 +
11 files changed, 457 insertions(+), 22 deletions(-)
diff --git
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java
index 125a7f5e897..4d498aca817 100644
---
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java
+++
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java
@@ -856,4 +856,20 @@ public class AccessControlUtil {
.setUser(ByteString.copyFromUtf8(username)).setPermission(ret))
.build();
}
+
+ public static Permission.Scope
toPermissionScope(AccessControlProtos.Permission.Type protoType) {
+ if (protoType == null) {
+ return null;
+ }
+ switch (protoType) {
+ case Global:
+ return Permission.Scope.GLOBAL;
+ case Namespace:
+ return Permission.Scope.NAMESPACE;
+ case Table:
+ return Permission.Scope.TABLE;
+ default:
+ return null;
+ }
+ }
}
diff --git
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java
index ace6f302a19..78a53f725ec 100644
---
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java
+++
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java
@@ -26,6 +26,7 @@ import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import org.apache.hadoop.hbase.HBaseInterfaceAudience;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.io.VersionedWritable;
@@ -62,8 +63,8 @@ public class Permission extends VersionedWritable {
}
}
- @InterfaceAudience.Private
- protected enum Scope {
+ @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.COPROC)
+ public enum Scope {
GLOBAL('G'),
NAMESPACE('N'),
TABLE('T'),
diff --git
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java
index cf7c797c98a..740c80dbceb 100644
---
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java
+++
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java
@@ -328,4 +328,20 @@ public class ShadedAccessControlUtil {
}
return builder.build();
}
+
+ public static Permission.Scope
toPermissionScope(AccessControlProtos.Permission.Type protoType) {
+ if (protoType == null) {
+ return null;
+ }
+ switch (protoType) {
+ case Global:
+ return Permission.Scope.GLOBAL;
+ case Namespace:
+ return Permission.Scope.NAMESPACE;
+ case Table:
+ return Permission.Scope.TABLE;
+ default:
+ return null;
+ }
+ }
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java
index 85ab42b9a55..248446ccfaa 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java
@@ -673,7 +673,7 @@ public interface MasterObserver {
*/
@SuppressWarnings("unused")
default void preTruncateRegionAction(final
ObserverContext<MasterCoprocessorEnvironment> c,
- final RegionInfo regionInfo) {
+ final RegionInfo regionInfo) throws IOException {
}
/**
@@ -683,7 +683,7 @@ public interface MasterObserver {
*/
@SuppressWarnings("unused")
default void preTruncateRegion(final
ObserverContext<MasterCoprocessorEnvironment> c,
- RegionInfo regionInfo) {
+ RegionInfo regionInfo) throws IOException {
}
/**
@@ -693,7 +693,7 @@ public interface MasterObserver {
*/
@SuppressWarnings("unused")
default void postTruncateRegion(final
ObserverContext<MasterCoprocessorEnvironment> c,
- RegionInfo regionInfo) {
+ RegionInfo regionInfo) throws IOException {
}
/**
@@ -703,7 +703,7 @@ public interface MasterObserver {
*/
@SuppressWarnings("unused")
default void postTruncateRegionAction(final
ObserverContext<MasterCoprocessorEnvironment> c,
- final RegionInfo regionInfo) {
+ final RegionInfo regionInfo) throws IOException {
}
/**
@@ -1837,12 +1837,34 @@ public interface MasterObserver {
* @param family the table column family, null if don't get table family
permission
* @param qualifier the table column qualifier, null if don't get table
qualifier permission
* @throws IOException if something went wrong
+ * @deprecated Since 2.5.17, 2.6.8, 2.7.0, 3.0.1 and 3.1.0, will be removed
in 4.0.0. Use
+ * {@link #preGetUserPermissions(ObserverContext, String,
String, TableName, byte[], byte[], Permission.Scope)}
+ * instead.
*/
+ @Deprecated
default void
preGetUserPermissions(ObserverContext<MasterCoprocessorEnvironment> ctx,
String userName, String namespace, TableName tableName, byte[] family,
byte[] qualifier)
throws IOException {
}
+ /**
+ * Called before getting user permissions.
+ * @param ctx the coprocessor instance's environment
+ * @param userName the user name, null if get all user permissions
+ * @param namespace the namespace, null if don't get namespace
permission
+ * @param tableName the table name, null if don't get table permission
+ * @param family the table column family, null if don't get table
family permission
+ * @param qualifier the table column qualifier, null if don't get
table qualifier permission
+ * @param permissionScope the scope of permission being requested (GLOBAL,
NAMESPACE, or TABLE),
+ * may be null for backward compatibility
+ * @throws IOException if something went wrong
+ */
+ default void
preGetUserPermissions(ObserverContext<MasterCoprocessorEnvironment> ctx,
+ String userName, String namespace, TableName tableName, byte[] family,
byte[] qualifier,
+ Permission.Scope permissionScope) throws IOException {
+ preGetUserPermissions(ctx, userName, namespace, tableName, family,
qualifier);
+ }
+
/**
* Called after getting user permissions.
* @param ctx the coprocessor instance's environment
@@ -1852,12 +1874,34 @@ public interface MasterObserver {
* @param family the table column family, null if don't get table family
permission
* @param qualifier the table column qualifier, null if don't get table
qualifier permission
* @throws IOException if something went wrong
+ * @deprecated Since 2.5.17, 2.6.8, 2.7.0, 3.0.1 and 3.1.0, will be removed
in 4.0.0. Use
+ * {@link #postGetUserPermissions(ObserverContext, String,
String, TableName, byte[], byte[], Permission.Scope)}
+ * instead.
*/
+ @Deprecated
default void
postGetUserPermissions(ObserverContext<MasterCoprocessorEnvironment> ctx,
String userName, String namespace, TableName tableName, byte[] family,
byte[] qualifier)
throws IOException {
}
+ /**
+ * Called after getting user permissions.
+ * @param ctx the coprocessor instance's environment
+ * @param userName the user name, null if get all user permissions
+ * @param namespace the namespace, null if don't get namespace
permission
+ * @param tableName the table name, null if don't get table permission
+ * @param family the table column family, null if don't get table
family permission
+ * @param qualifier the table column qualifier, null if don't get
table qualifier permission
+ * @param permissionScope the scope of permission being requested (GLOBAL,
NAMESPACE, or TABLE),
+ * may be null for backward compatibility
+ * @throws IOException if something went wrong
+ */
+ default void
postGetUserPermissions(ObserverContext<MasterCoprocessorEnvironment> ctx,
+ String userName, String namespace, TableName tableName, byte[] family,
byte[] qualifier,
+ Permission.Scope permissionScope) throws IOException {
+ postGetUserPermissions(ctx, userName, namespace, tableName, family,
qualifier);
+ }
+
/*
* Called before checking if user has permissions.
* @param ctx the coprocessor instance's environment
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
index 026eee7c69d..6a84d365357 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
@@ -869,7 +869,7 @@ public class MasterCoprocessorHost
public void preTruncateRegion(RegionInfo regionInfo) throws IOException {
execOperation(coprocEnvironments.isEmpty() ? null : new
MasterObserverOperation() {
@Override
- public void call(MasterObserver observer) {
+ public void call(MasterObserver observer) throws IOException {
observer.preTruncateRegion(this, regionInfo);
}
});
@@ -882,7 +882,7 @@ public class MasterCoprocessorHost
public void postTruncateRegion(RegionInfo regionInfo) throws IOException {
execOperation(coprocEnvironments.isEmpty() ? null : new
MasterObserverOperation() {
@Override
- public void call(MasterObserver observer) {
+ public void call(MasterObserver observer) throws IOException {
observer.postTruncateRegion(this, regionInfo);
}
});
@@ -2017,22 +2017,46 @@ public class MasterCoprocessorHost
});
}
+ /**
+ * @deprecated Since 2.5.17, 2.6.8, 2.7.0, 3.0.1 and 3.1.0, will be removed
in 4.0.0. Use
+ * {@link #preGetUserPermissions(String, String, TableName,
byte[], byte[], Permission.Scope)}
+ * instead.
+ */
+ @Deprecated
public void preGetUserPermissions(String userName, String namespace,
TableName tableName,
byte[] family, byte[] qualifier) throws IOException {
+ preGetUserPermissions(userName, namespace, tableName, family, qualifier,
null);
+ }
+
+ public void preGetUserPermissions(String userName, String namespace,
TableName tableName,
+ byte[] family, byte[] qualifier, Permission.Scope permissionScope) throws
IOException {
execOperation(coprocEnvironments.isEmpty() ? null : new
MasterObserverOperation() {
@Override
public void call(MasterObserver observer) throws IOException {
- observer.preGetUserPermissions(this, userName, namespace, tableName,
family, qualifier);
+ observer.preGetUserPermissions(this, userName, namespace, tableName,
family, qualifier,
+ permissionScope);
}
});
}
+ /**
+ * @deprecated Since 2.5.17, 2.6.8, 2.7.0, 3.0.1 and 3.1.0, will be removed
in 4.0.0. Use
+ * {@link #postGetUserPermissions(String, String, TableName,
byte[], byte[], Permission.Scope)}
+ * instead.
+ */
+ @Deprecated
public void postGetUserPermissions(String userName, String namespace,
TableName tableName,
byte[] family, byte[] qualifier) throws IOException {
+ postGetUserPermissions(userName, namespace, tableName, family, qualifier,
null);
+ }
+
+ public void postGetUserPermissions(String userName, String namespace,
TableName tableName,
+ byte[] family, byte[] qualifier, Permission.Scope permissionScope) throws
IOException {
execOperation(coprocEnvironments.isEmpty() ? null : new
MasterObserverOperation() {
@Override
public void call(MasterObserver observer) throws IOException {
- observer.postGetUserPermissions(this, userName, namespace, tableName,
family, qualifier);
+ observer.postGetUserPermissions(this, userName, namespace, tableName,
family, qualifier,
+ permissionScope);
}
});
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
index c2618b9641a..9b5be9d4e4b 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
@@ -2743,6 +2743,7 @@ public class MasterRpcServices extends RSRpcServices
@Override
public MasterProtos.AssignsResponse assigns(RpcController controller,
MasterProtos.AssignsRequest request) throws ServiceException {
+ rpcPreCheck("assigns");
checkMasterProcedureExecutor();
final ProcedureExecutor<MasterProcedureEnv> pe =
master.getMasterProcedureExecutor();
final AssignmentManager am = master.getAssignmentManager();
@@ -2770,6 +2771,7 @@ public class MasterRpcServices extends RSRpcServices
@Override
public MasterProtos.UnassignsResponse unassigns(RpcController controller,
MasterProtos.UnassignsRequest request) throws ServiceException {
+ rpcPreCheck("unassigns");
checkMasterProcedureExecutor();
final ProcedureExecutor<MasterProcedureEnv> pe =
master.getMasterProcedureExecutor();
final AssignmentManager am = master.getAssignmentManager();
@@ -2800,6 +2802,7 @@ public class MasterRpcServices extends RSRpcServices
@Override
public MasterProtos.BypassProcedureResponse bypassProcedure(RpcController
controller,
MasterProtos.BypassProcedureRequest request) throws ServiceException {
+ rpcPreCheck("bypassProcedure");
try {
LOG.info("{} bypass procedures={}, waitTime={}, override={},
recursive={}",
master.getClientIdAuditPrefix(), request.getProcIdList(),
request.getWaitTime(),
@@ -2817,6 +2820,7 @@ public class MasterRpcServices extends RSRpcServices
public MasterProtos.ScheduleServerCrashProcedureResponse
scheduleServerCrashProcedure(
RpcController controller, MasterProtos.ScheduleServerCrashProcedureRequest
request)
throws ServiceException {
+ rpcPreCheck("scheduleServerCrashProcedure");
List<Long> pids = new ArrayList<>();
for (HBaseProtos.ServerName sn : request.getServerNameList()) {
ServerName serverName = ProtobufUtil.toServerName(sn);
@@ -2835,7 +2839,7 @@ public class MasterRpcServices extends RSRpcServices
public MasterProtos.ScheduleSCPsForUnknownServersResponse
scheduleSCPsForUnknownServers(
RpcController controller,
MasterProtos.ScheduleSCPsForUnknownServersRequest request)
throws ServiceException {
-
+ rpcPreCheck("scheduleSCPsForUnknownServers");
List<Long> pids = new ArrayList<>();
final Set<ServerName> serverNames =
master.getAssignmentManager().getRegionStates()
.getRegionStates().stream().map(RegionState::getServerName).collect(Collectors.toSet());
@@ -2980,7 +2984,10 @@ public class MasterRpcServices extends RSRpcServices
byte[] cq =
request.hasColumnQualifier() ?
request.getColumnQualifier().toByteArray() : null;
Type permissionType = request.hasType() ? request.getType() : null;
- master.getMasterCoprocessorHost().preGetUserPermissions(userName,
namespace, table, cf, cq);
+ Permission.Scope permissionScope =
+ ShadedAccessControlUtil.toPermissionScope(permissionType);
+ master.getMasterCoprocessorHost().preGetUserPermissions(userName,
namespace, table, cf, cq,
+ permissionScope);
List<UserPermission> perms = null;
if (permissionType == Type.Table) {
@@ -3005,8 +3012,8 @@ public class MasterRpcServices extends RSRpcServices
}
}
- master.getMasterCoprocessorHost().postGetUserPermissions(userName,
namespace, table, cf,
- cq);
+ master.getMasterCoprocessorHost().postGetUserPermissions(userName,
namespace, table, cf, cq,
+ permissionScope);
AccessControlProtos.GetUserPermissionsResponse response =
ShadedAccessControlUtil.buildGetUserPermissionsResponse(perms);
return response;
@@ -3100,6 +3107,7 @@ public class MasterRpcServices extends RSRpcServices
public HBaseProtos.LogEntry getLogEntries(RpcController controller,
HBaseProtos.LogRequest request) throws ServiceException {
try {
+ requirePermission("getLogEntries", Permission.Action.ADMIN);
final String logClassName = request.getLogClassName();
Class<?> logClass =
Class.forName(logClassName).asSubclass(Message.class);
Method method = logClass.getMethod("parseFrom", ByteString.class);
@@ -3121,7 +3129,7 @@ public class MasterRpcServices extends RSRpcServices
.setLogMessage(balancerRejectionsResponse.toByteString()).build();
}
} catch (ClassNotFoundException | NoSuchMethodException |
IllegalAccessException
- | InvocationTargetException e) {
+ | InvocationTargetException | IOException e) {
LOG.error("Error while retrieving log entries.", e);
throw new ServiceException(e);
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
index 70605579bb4..cfc0f113f8d 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
@@ -4122,16 +4122,18 @@ public class RSRpcServices implements
HBaseRPCErrorHandler, AdminService.Blockin
}
@Override
+ @QosPriority(priority = HConstants.ADMIN_QOS)
public HBaseProtos.LogEntry getLogEntries(RpcController controller,
HBaseProtos.LogRequest request) throws ServiceException {
try {
+ requirePermission("getLogEntries", Permission.Action.ADMIN);
final String logClassName = request.getLogClassName();
Class<?> logClass =
Class.forName(logClassName).asSubclass(Message.class);
Method method = logClass.getMethod("parseFrom", ByteString.class);
if (logClassName.contains("SlowLogResponseRequest")) {
SlowLogResponseRequest slowLogResponseRequest =
(SlowLogResponseRequest) method.invoke(null,
request.getLogMessage());
- final NamedQueueRecorder namedQueueRecorder =
this.regionServer.getNamedQueueRecorder();
+ final NamedQueueRecorder namedQueueRecorder =
regionServer.getNamedQueueRecorder();
final List<SlowLogPayload> slowLogPayloads =
getSlowLogPayloads(slowLogResponseRequest, namedQueueRecorder);
SlowLogResponses slowLogResponses =
@@ -4141,7 +4143,7 @@ public class RSRpcServices implements
HBaseRPCErrorHandler, AdminService.Blockin
.setLogMessage(slowLogResponses.toByteString()).build();
}
} catch (ClassNotFoundException | NoSuchMethodException |
IllegalAccessException
- | InvocationTargetException e) {
+ | InvocationTargetException | IOException e) {
LOG.error("Error while retrieving log entries.", e);
throw new ServiceException(e);
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
index 44e18f65654..a63da714f7e 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
@@ -874,6 +874,13 @@ public class AccessController implements
MasterCoprocessor, RegionCoprocessor,
});
}
+ @Override
+ public void preTruncateRegion(final
ObserverContext<MasterCoprocessorEnvironment> ctx,
+ final RegionInfo regionInfo) throws IOException {
+ requirePermission(ctx, "truncateRegion", regionInfo.getTable(), null,
null, Action.ADMIN,
+ Action.CREATE);
+ }
+
@Override
public TableDescriptor
preModifyTable(ObserverContext<MasterCoprocessorEnvironment> c,
TableName tableName, TableDescriptor currentDesc, TableDescriptor newDesc)
throws IOException {
@@ -1145,10 +1152,11 @@ public class AccessController implements
MasterCoprocessor, RegionCoprocessor,
@Override
public void preModifyNamespace(ObserverContext<MasterCoprocessorEnvironment>
ctx,
- NamespaceDescriptor ns) throws IOException {
+ NamespaceDescriptor currentNsDescriptor, NamespaceDescriptor
newNsDescriptor)
+ throws IOException {
// We require only global permission so that
// a user with NS admin cannot altering namespace configurations. i.e.
namespace quota
- requireGlobalPermission(ctx, "modifyNamespace", Action.ADMIN,
ns.getName());
+ requireGlobalPermission(ctx, "modifyNamespace", Action.ADMIN,
newNsDescriptor.getName());
}
@Override
@@ -1986,7 +1994,10 @@ public class AccessController implements
MasterCoprocessor, RegionCoprocessor,
request.hasColumnFamily() ? request.getColumnFamily().toByteArray()
: null;
final byte[] cq =
request.hasColumnQualifier() ?
request.getColumnQualifier().toByteArray() : null;
- preGetUserPermissions(caller, userName, namespace, table, cf, cq);
+ AccessControlProtos.Permission.Type protoType =
+ request.hasType() ? request.getType() : null;
+ Permission.Scope permissionScope =
AccessControlUtil.toPermissionScope(protoType);
+ preGetUserPermissions(caller, userName, namespace, table, cf, cq,
permissionScope);
GetUserPermissionsRequest getUserPermissionsRequest = null;
if (request.getType() == AccessControlProtos.Permission.Type.Table) {
getUserPermissionsRequest =
GetUserPermissionsRequest.newBuilder(table).withFamily(cf)
@@ -2408,9 +2419,26 @@ public class AccessController implements
MasterCoprocessor, RegionCoprocessor,
@Override
public void
preGetUserPermissions(ObserverContext<MasterCoprocessorEnvironment> ctx,
- String userName, String namespace, TableName tableName, byte[] family,
byte[] qualifier)
+ String userName, String namespace, TableName tableName, byte[] family,
byte[] qualifier,
+ Permission.Scope permissionScope) throws IOException {
+ preGetUserPermissions(getActiveUser(ctx), userName, namespace, tableName,
family, qualifier,
+ permissionScope);
+ }
+
+ private void preGetUserPermissions(User caller, String userName, String
namespace,
+ TableName tableName, byte[] family, byte[] qualifier, Permission.Scope
permissionScope)
throws IOException {
- preGetUserPermissions(getActiveUser(ctx), userName, namespace, tableName,
family, qualifier);
+ if (permissionScope == Permission.Scope.TABLE) {
+ accessChecker.requirePermission(caller, "getUserPermissions", tableName,
family, qualifier,
+ userName, Action.ADMIN);
+ } else if (permissionScope == Permission.Scope.NAMESPACE) {
+ accessChecker.requireNamespacePermission(caller, "getUserPermissions",
namespace, userName,
+ Action.ADMIN);
+ } else if (permissionScope == Permission.Scope.GLOBAL) {
+ accessChecker.requirePermission(caller, "getUserPermissions", userName,
Action.ADMIN);
+ } else {
+ preGetUserPermissions(caller, userName, namespace, tableName, family,
qualifier);
+ }
}
private void preGetUserPermissions(User caller, String userName, String
namespace,
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
index f471b64bffa..d784f8d486a 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
@@ -3405,6 +3405,68 @@ public class TestAccessController extends SecureTestUtil
{
}
}
+ @Test
+ public void testGetUserPermissionsScopeBasedAuthorization() throws Throwable
{
+ // Regression test: authorization must be based on permissionScope
(derived from
+ // the type field), not on the tableName field. A user with only
table-level ADMIN
+ // should not be able to request global permissions by passing tableName
with
+ // permissionScope=GLOBAL.
+ AccessTestAction tablePermWithGlobalScope = new AccessTestAction() {
+ @Override
+ public Object run() throws Exception {
+
ACCESS_CONTROLLER.preGetUserPermissions(ObserverContextImpl.createAndPrepare(CP_ENV),
null,
+ null, TEST_TABLE, null, null, Permission.Scope.GLOBAL);
+ return null;
+ }
+ };
+
+ // USER_ADMIN has global ADMIN, should be allowed
+ verifyAllowed(tablePermWithGlobalScope, SUPERUSER, USER_ADMIN);
+ // Users with only table-level ADMIN should be denied
+ verifyDenied(tablePermWithGlobalScope, USER_OWNER, USER_ADMIN_CF,
USER_CREATE, USER_RW, USER_RO,
+ USER_NONE);
+
+ AccessTestAction tablePermWithNamespaceScope = new AccessTestAction() {
+ @Override
+ public Object run() throws Exception {
+
ACCESS_CONTROLLER.preGetUserPermissions(ObserverContextImpl.createAndPrepare(CP_ENV),
null,
+ TEST_TABLE.getNamespaceAsString(), TEST_TABLE, null, null,
Permission.Scope.NAMESPACE);
+ return null;
+ }
+ };
+
+ // Users without namespace ADMIN should be denied
+ verifyDenied(tablePermWithNamespaceScope, USER_OWNER, USER_ADMIN_CF,
USER_CREATE, USER_RW,
+ USER_RO, USER_NONE);
+
+ AccessTestAction tablePermWithTableScope = new AccessTestAction() {
+ @Override
+ public Object run() throws Exception {
+
ACCESS_CONTROLLER.preGetUserPermissions(ObserverContextImpl.createAndPrepare(CP_ENV),
null,
+ null, TEST_TABLE, null, null, Permission.Scope.TABLE);
+ return null;
+ }
+ };
+
+ // Users with table ADMIN should be allowed
+ verifyAllowed(tablePermWithTableScope, SUPERUSER, USER_ADMIN, USER_OWNER);
+ verifyDenied(tablePermWithTableScope, USER_ADMIN_CF, USER_CREATE, USER_RW,
USER_RO, USER_NONE);
+
+ AccessTestAction globalPermWithNullScope = new AccessTestAction() {
+ @Override
+ public Object run() throws Exception {
+
ACCESS_CONTROLLER.preGetUserPermissions(ObserverContextImpl.createAndPrepare(CP_ENV),
null,
+ null, null, null, null, null);
+ return null;
+ }
+ };
+
+ // null scope falls back to old behavior (no tableName -> require global
ADMIN)
+ verifyAllowed(globalPermWithNullScope, SUPERUSER, USER_ADMIN);
+ verifyDenied(globalPermWithNullScope, USER_OWNER, USER_ADMIN_CF,
USER_CREATE, USER_RW, USER_RO,
+ USER_NONE);
+ }
+
@Test
public void testHasPermission() throws Throwable {
Connection conn = null;
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControllerObserverCoverage.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControllerObserverCoverage.java
new file mode 100644
index 00000000000..4dfd24d6eee
--- /dev/null
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControllerObserverCoverage.java
@@ -0,0 +1,233 @@
+/*
+ * 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.hbase.security.access;
+
+import static org.junit.jupiter.api.Assertions.fail;
+
+import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.apache.hadoop.hbase.coprocessor.BulkLoadObserver;
+import org.apache.hadoop.hbase.coprocessor.EndpointObserver;
+import org.apache.hadoop.hbase.coprocessor.MasterObserver;
+import org.apache.hadoop.hbase.coprocessor.RegionObserver;
+import org.apache.hadoop.hbase.coprocessor.RegionServerObserver;
+import org.apache.hadoop.hbase.testclassification.SecurityTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Verifies that AccessController implements every security-relevant method
declared in the five
+ * observer interfaces it claims to implement: MasterObserver, RegionObserver,
RegionServerObserver,
+ * EndpointObserver, BulkLoadObserver.
+ * <p>
+ * If a new hook is added to any of these interfaces and AccessController does
not override it, the
+ * default no-op implementation will silently skip the permission check — a
potential privilege
+ * escalation. This test catches that at build time.
+ * <p>
+ * Skipped methods are determined by two mechanisms:
+ * <ul>
+ * <li><b>Pattern rules</b> — methods matching these patterns are always safe
to skip:
+ * <ul>
+ * <li>{@code post*} — post-operation notifications; the operation has already
been authorized and
+ * executed.</li>
+ * <li>{@code pre*Action} — procedure-level action hooks; authorization
happens at the RPC layer in
+ * the corresponding {@code pre*} hook.</li>
+ * </ul>
+ * </li>
+ * <li><b>Explicit whitelist</b> — methods that don't match the above rules
but are still safe to
+ * skip (internal lifecycle hooks, deprecated overloads with default
delegation, read-only queries).
+ * Each entry has a justification comment.</li>
+ * </ul>
+ */
+@Tag(SecurityTests.TAG)
+@Tag(SmallTests.TAG)
+public class TestAccessControllerObserverCoverage {
+
+ /**
+ * Explicit whitelist for methods that don't match the pattern rules but are
intentionally not
+ * overridden in AccessController.
+ * <p>
+ * Use simple method name (covers all overloads) or full signature key
+ * "methodName(ParamType1,ParamType2,..." using simple class names.
+ */
+ private static final Set<String> WHITELIST = new HashSet<>(Arrays.asList(
+
+ // --- Internal lifecycle hooks (not client-facing RPCs) ---
+ // Store file / WAL internal hooks
+ "preStoreFileReaderOpen", "preStoreScannerOpen", "preCommitStoreFile",
"preReplayWALs",
+ "preWALRestore",
+ // Master internal
+ "preMasterStoreFlush",
+ // Lifecycle markers (not triggered by client RPC)
+ "preMasterInitialization", "preCreateTableRegionsInfos",
+ // --- Read-only query hooks (no mutation, no authorization needed) ---
+ "preGetClusterMetrics", "preGetTableNames", "preListNamespaceDescriptors",
"preListNamespaces",
+ // RSGroup related methods, access control is implemented in
RSGroupAdminEndpoint
+ "preMoveServers", "preMoveServersAndTables", "preMoveTables",
"preRemoveServers",
+
+ // --- Deprecated overloads: interface default delegates to non-deprecated
---
+ // prePut(3-arg) delegates to prePut(4-arg Durability)
+
"prePut(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.Put,org.apache.hadoop.hbase.wal.WALEdit)",
+ // preDelete(3-arg) delegates to preDelete(4-arg Durability)
+
"preDelete(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.Delete,org.apache.hadoop.hbase.wal.WALEdit)",
+ // preAppend(3-arg) delegates to preAppend(2-arg deprecated)
+
"preAppend(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.Append,org.apache.hadoop.hbase.wal.WALEdit)",
+ // preAppendAfterRowLock(2-arg) delegates to preAppendAfterRowLock(1-arg
deprecated)
+
"preAppendAfterRowLock(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.Append)",
+ // preIncrement(3-arg) delegates to preIncrement(2-arg deprecated)
+
"preIncrement(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.Increment,org.apache.hadoop.hbase.wal.WALEdit)",
+ // preIncrementAfterRowLock(2-arg) delegates to
preIncrementAfterRowLock(1-arg deprecated)
+
"preIncrementAfterRowLock(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.Increment)",
+ // preCheckAndMutate delegates to preCheckAndPut/preCheckAndDelete
+
"preCheckAndMutate(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.CheckAndMutate,org.apache.hadoop.hbase.client.CheckAndMutateResult)",
+ // preCheckAndMutateAfterRowLock delegates to
+ // preCheckAndPutAfterRowLock/preCheckAndDeleteAfterRowLock
+
"preCheckAndMutateAfterRowLock(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.CheckAndMutate,org.apache.hadoop.hbase.client.CheckAndMutateResult)",
+ // Filter-based CheckAnd* are deprecated; framework uses byte[] overloads
+ // Note: Class.getName() returns "[B" for byte[], not "byte[]"
+
"preCheckAndPut(org.apache.hadoop.hbase.coprocessor.ObserverContext,[B,org.apache.hadoop.hbase.filter.Filter,org.apache.hadoop.hbase.client.Put,boolean)",
+
"preCheckAndPutAfterRowLock(org.apache.hadoop.hbase.coprocessor.ObserverContext,[B,org.apache.hadoop.hbase.filter.Filter,org.apache.hadoop.hbase.client.Put,boolean)",
+
"preCheckAndDelete(org.apache.hadoop.hbase.coprocessor.ObserverContext,[B,org.apache.hadoop.hbase.filter.Filter,org.apache.hadoop.hbase.client.Delete,boolean)",
+
"preCheckAndDeleteAfterRowLock(org.apache.hadoop.hbase.coprocessor.ObserverContext,[B,org.apache.hadoop.hbase.filter.Filter,org.apache.hadoop.hbase.client.Delete,boolean)",
+ // Deprecated preGetUserPermissions(6-arg) delegates to
preGetUserPermissions(7-arg with Scope)
+
"preGetUserPermissions(ObserverContext,String,String,TableName,byte[],byte[])",
+ // Deprecated WAL-append / timestamp hooks
+ "prePrepareTimeStampForDeleteVersion", "preWALAppend",
+ // Deprecated preModifyXXX where we do not pass the old descriptor
+
"preModifyNamespace(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.NamespaceDescriptor)",
+
"preModifyTable(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.TableName,org.apache.hadoop.hbase.client.TableDescriptor)",
+ // Deprecated dangerous force unassign
+
"preUnassign(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.RegionInfo,boolean)",
+ // --- Replication sink is a trusted internal cluster-to-cluster operation
---
+ "preReplicationSinkBatchMutate"));
+
+ private static final Class<?>[] OBSERVER_INTERFACES =
+ { MasterObserver.class, RegionObserver.class, RegionServerObserver.class,
+ EndpointObserver.class, BulkLoadObserver.class };
+
+ /**
+ * Returns true if the method matches a pattern rule that makes it safe to
skip without an
+ * explicit whitelist entry.
+ */
+ private static boolean matchesSkipPattern(Class<?> iface, Method m) {
+ String name = m.getName();
+ // All post* methods are post-operation notifications.
+ // Permission checks must happen before the operation, not after.
+ if (name.startsWith("post")) {
+ return true;
+ }
+ // *Action suffix on pre* hooks are procedure-level callbacks.
+ // Authorization is done at the RPC layer in the corresponding pre* hook.
+ if (name.endsWith("Action")) {
+ return true;
+ }
+ // RSGroup related access control is implemented separated in
RSGroupAdminEndpoint
+ if (name.contains("RSGroup")) {
+ return true;
+ }
+ // RegionObserver internal storage hooks: flush, compaction, in-memory
+ // compaction. These are sub-step callbacks within a region storage
+ // operation. The region-level entry hook (preFlush, preCompact) already
+ // handles authorization in AccessController.
+ if (
+ iface == RegionObserver.class && (name.startsWith("preFlush") ||
name.startsWith("preCompact")
+ || name.startsWith("preMemStore"))
+ ) {
+ return true;
+ }
+ return false;
+ }
+
+ private static String methodSignatureKey(Method m) {
+ String paramTypes =
Arrays.stream(m.getParameterTypes()).map(Class::getSimpleName)
+ .collect(Collectors.joining(","));
+ return m.getName() + "(" + paramTypes + ")";
+ }
+
+ private static String fullMethodSignatureKey(Method m) {
+ String paramTypes =
+
Arrays.stream(m.getParameterTypes()).map(Class::getName).collect(Collectors.joining(","));
+ return m.getName() + "(" + paramTypes + ")";
+ }
+
+ private static boolean isMethodImplemented(Class<?> implClass, Method
ifaceMethod) {
+ Class<?> clazz = implClass;
+ while (clazz != null) {
+ for (Method m : clazz.getDeclaredMethods()) {
+ if (
+ m.getName().equals(ifaceMethod.getName())
+ && Arrays.equals(m.getParameterTypes(),
ifaceMethod.getParameterTypes())
+ ) {
+ return true;
+ }
+ }
+ clazz = clazz.getSuperclass();
+ }
+ return false;
+ }
+
+ @Test
+ public void testAllObserverMethodsAreImplemented() {
+ Set<String> missing = new TreeSet<>();
+
+ for (Class<?> iface : OBSERVER_INTERFACES) {
+ for (Method m : iface.getMethods()) {
+ if (m.getDeclaringClass() == Object.class) {
+ continue;
+ }
+ if (!m.getDeclaringClass().equals(iface)) {
+ continue;
+ }
+
+ if (matchesSkipPattern(iface, m)) {
+ continue;
+ }
+
+ String simpleName = m.getName();
+ String simpleKey = methodSignatureKey(m);
+ String fullKey = fullMethodSignatureKey(m);
+
+ if (
+ WHITELIST.contains(simpleName) || WHITELIST.contains(simpleKey)
+ || WHITELIST.contains(fullKey)
+ ) {
+ continue;
+ }
+
+ if (!isMethodImplemented(AccessController.class, m)) {
+ missing.add(" " + iface.getSimpleName() + "." + simpleKey);
+ }
+ }
+ }
+
+ if (!missing.isEmpty()) {
+ StringBuilder sb = new StringBuilder();
+ sb.append("AccessController does not implement the following observer
methods.\n");
+ sb.append("Either override them in AccessController (with permission
checks),\n");
+ sb.append("or add them to the WHITELIST with a justification
comment.\n\n");
+ sb.append("Missing methods:\n");
+ missing.forEach(m -> sb.append(m).append("\n"));
+ fail(sb.toString());
+ }
+ }
+}
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
index cda9c0fb0a2..f154a23eb31 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
@@ -244,6 +244,7 @@ public class TestNamespaceCommands extends SecureTestUtil {
@Override
public Object run() throws Exception {
ACCESS_CONTROLLER.preModifyNamespace(ObserverContextImpl.createAndPrepare(CP_ENV),
+ NamespaceDescriptor.create(TEST_NAMESPACE).build(),
NamespaceDescriptor.create(TEST_NAMESPACE).addConfiguration("abc",
"156").build());
return null;
}
