diff --git a/.gitignore b/.gitignore index 579d291..cd975cc 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ /target/ -/dependency-reduced-pom.xml \ No newline at end of file +/dependency-reduced-pom.xml +/CLAUDE.md +.claude/ diff --git a/pom.xml b/pom.xml index 29cb440..8954f58 100644 --- a/pom.xml +++ b/pom.xml @@ -52,9 +52,9 @@ 3.2.5 2.43.0 - 2.6.0 + 2.6.4 2.5.0 - 2.8.1 + 2.8.8 @@ -106,6 +106,12 @@ hbase-testing-util ${hbase.version} test + + + org.apache.directory.jdbm + apacheds-jdbm1 + + org.slf4j diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index 2e02c31..f45569f 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -1,17 +1,24 @@ package tech.stackable.hbase; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.MapMaker; import com.google.protobuf.Message; import com.google.protobuf.RpcCallback; import com.google.protobuf.RpcController; import com.google.protobuf.Service; import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.NavigableSet; import java.util.Optional; +import java.util.TreeMap; +import java.util.stream.Collectors; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.Cell; +import org.apache.hadoop.hbase.CellUtil; import org.apache.hadoop.hbase.CompareOperator; import org.apache.hadoop.hbase.CoprocessorEnvironment; import org.apache.hadoop.hbase.NamespaceDescriptor; @@ -20,6 +27,8 @@ import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Append; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.CheckAndMutate; +import org.apache.hadoop.hbase.client.CheckAndMutateResult; import org.apache.hadoop.hbase.client.Delete; import org.apache.hadoop.hbase.client.Durability; import org.apache.hadoop.hbase.client.Get; @@ -29,6 +38,7 @@ import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.Result; +import org.apache.hadoop.hbase.client.RowMutations; import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.client.SnapshotDescription; import org.apache.hadoop.hbase.client.TableDescriptor; @@ -45,6 +55,7 @@ import org.apache.hadoop.hbase.coprocessor.RegionServerCoprocessorEnvironment; import org.apache.hadoop.hbase.coprocessor.RegionServerObserver; import org.apache.hadoop.hbase.filter.ByteArrayComparable; +import org.apache.hadoop.hbase.filter.Filter; import org.apache.hadoop.hbase.ipc.RpcServer; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.quotas.GlobalQuotaSettings; @@ -57,7 +68,6 @@ import org.apache.hadoop.hbase.regionserver.Store; import org.apache.hadoop.hbase.regionserver.compactions.CompactionLifeCycleTracker; import org.apache.hadoop.hbase.regionserver.compactions.CompactionRequest; -import org.apache.hadoop.hbase.replication.ReplicationEndpoint; import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.security.AccessDeniedException; import org.apache.hadoop.hbase.security.User; @@ -66,11 +76,13 @@ import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.Permission.Action; import org.apache.hadoop.hbase.security.access.UserPermission; +import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.wal.WALEdit; import org.apache.hadoop.security.AccessControlException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import tech.stackable.hbase.opa.OpType; import tech.stackable.hbase.opa.OpaAclChecker; public class OpenPolicyAgentAccessController @@ -149,55 +161,65 @@ private User getActiveUser(ObserverContext ctx) throws IOException { @Override public void preCreateNamespace( - ObserverContext c, NamespaceDescriptor ns) throws IOException { - User user = getActiveUser(c); + ObserverContext ctx, NamespaceDescriptor ns) + throws IOException { + User user = getActiveUser(ctx); LOG.debug("preCreateNamespace: user [{}]", user); - opaAclChecker.checkPermissionInfo(user, ns.getName(), Action.ADMIN); + opaAclChecker.checkPermissionInfo( + user, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, Action.ADMIN); } @Override - public void preDeleteNamespace(ObserverContext c, String namespace) - throws IOException { - User user = getActiveUser(c); + public void preDeleteNamespace( + ObserverContext ctx, String namespace) throws IOException { + User user = getActiveUser(ctx); LOG.debug("preDeleteNamespace: user [{}]", user); - opaAclChecker.checkPermissionInfo(user, namespace, Action.ADMIN); + opaAclChecker.checkPermissionInfo( + user, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, Action.ADMIN); } @Override public void preModifyNamespace( - ObserverContext c, NamespaceDescriptor ns) throws IOException { - User user = getActiveUser(c); + ObserverContext ctx, NamespaceDescriptor ns) + throws IOException { + User user = getActiveUser(ctx); LOG.debug("preModifyNamespace: user [{}]", user); - opaAclChecker.checkPermissionInfo(user, ns.getName(), Action.ADMIN); + opaAclChecker.checkPermissionInfo( + user, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, Action.ADMIN); } @Override public void preGetNamespaceDescriptor( - ObserverContext c, String namespace) throws IOException { - User user = getActiveUser(c); + ObserverContext ctx, String namespace) throws IOException { + User user = getActiveUser(ctx); LOG.debug("preGetNamespaceDescriptor: user [{}]", user); opaAclChecker.checkPermissionInfo(user, namespace, Action.ADMIN); } @Override public void postListNamespaces( - ObserverContext c, List namespaces) throws IOException { + ObserverContext ctx, List namespaces) + throws IOException { /* always allow namespace listing */ } @Override public void preCreateTable( - ObserverContext c, TableDescriptor desc, RegionInfo[] regions) + ObserverContext ctx, TableDescriptor desc, RegionInfo[] regions) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); LOG.debug("preCreateTable: user [{}]", user); - - opaAclChecker.checkPermissionInfo(user, desc.getTableName(), Action.CREATE); + requirePermission( + ctx, + desc.getTableName().getNamespaceAsString(), + "createTable", + Action.ADMIN, + Action.CREATE); } @Override public void postCompletedCreateTableAction( - final ObserverContext c, + final ObserverContext ctx, final TableDescriptor desc, final RegionInfo[] regions) { /* @@ -207,18 +229,16 @@ public void postCompletedCreateTableAction( } @Override - public void preDeleteTable(ObserverContext c, TableName tableName) + public void preDeleteTable(ObserverContext ctx, TableName tableName) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); LOG.debug("preDeleteTable: user [{}]", user); - - // the default access controller treats create/delete as requiring the same permissions. - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission(ctx, "deleteTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override public void postDeleteTable( - ObserverContext c, final TableName tableName) { + ObserverContext ctx, final TableName tableName) { /* The default AccessController uses this method to remove the permissions for the deleted table in the internal ACL table. We do not need this as we are managing permissions in OPA. @@ -226,71 +246,75 @@ public void postDeleteTable( } @Override - public void preEnableTable(ObserverContext c, TableName tableName) + public void preEnableTable(ObserverContext ctx, TableName tableName) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); LOG.debug("preEnableTable: user [{}]", user); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission(ctx, "enableTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override - public void preDisableTable(ObserverContext c, TableName tableName) - throws IOException { - User user = getActiveUser(c); + public void preDisableTable( + ObserverContext ctx, TableName tableName) throws IOException { + User user = getActiveUser(ctx); LOG.debug("preDisableTable: user [{}]", user); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission(ctx, "disableTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override public void preGetOp( - final ObserverContext c, final Get get, final List result) + final ObserverContext ctx, + final Get get, + final List result) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace("preGetOp: user [{}] on table [{}] with get [{}]", user, tableName, get); // All users need read access to hbase:meta table. if (TableName.META_TABLE_NAME.equals(tableName)) { return; } - opaAclChecker.checkPermissionInfo(user, tableName, Action.READ); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.READ, OpType.GET, familiesFromQualifiers(get.getFamilyMap())); } @Override public boolean preExists( - final ObserverContext c, final Get get, final boolean exists) + final ObserverContext ctx, final Get get, final boolean exists) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); // All users need read access to hbase:meta table. if (TableName.META_TABLE_NAME.equals(tableName)) { return exists; } LOG.trace("preExists: user [{}] on table [{}] with get [{}]", user, tableName, get); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.READ); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.READ, OpType.EXISTS, familiesFromQualifiers(get.getFamilyMap())); return exists; } @Override - public void preScannerOpen(final ObserverContext c, final Scan scan) - throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + public void preScannerOpen( + final ObserverContext ctx, final Scan scan) throws IOException { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); // All users need read access to hbase:meta table. if (TableName.META_TABLE_NAME.equals(tableName)) { return; } LOG.trace("preScannerOpen: user [{}] on table [{}] with scan [{}]", user, tableName, scan); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.READ); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.READ, OpType.SCAN, familiesFromQualifiers(scan.getFamilyMap())); } @Override public RegionScanner postScannerOpen( - final ObserverContext c, final Scan scan, final RegionScanner s) + final ObserverContext ctx, + final Scan scan, + final RegionScanner s) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); if (user != null && user.getShortName() != null) { // TODO this uses the shortName. Is it possible for the same scanner to be used by // different users across principals who nevertheless have the same shortName? This @@ -303,14 +327,14 @@ public RegionScanner postScannerOpen( @Override public boolean preScannerNext( - final ObserverContext c, + final ObserverContext ctx, final InternalScanner s, final List result, final int limit, final boolean hasNext) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preScannerNext: user [{}] on table [{}] with scan [{}]", user, tableName, s); requireScannerOwner(s); @@ -319,10 +343,10 @@ public boolean preScannerNext( @Override public void preScannerClose( - final ObserverContext c, final InternalScanner s) + final ObserverContext ctx, final InternalScanner s) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preScannerClose: user [{}] on table [{}] with scan [{}]", user, tableName, s); requireScannerOwner(s); @@ -330,10 +354,10 @@ public void preScannerClose( @Override public void postScannerClose( - final ObserverContext c, final InternalScanner s) + final ObserverContext ctx, final InternalScanner s) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("postScannerClose: user [{}] on table [{}] with scan [{}]", user, tableName, s); scannerOwners.remove(s); @@ -353,37 +377,35 @@ private void requireScannerOwner(InternalScanner s) throws AccessDeniedException @Override public void prePut( - final ObserverContext c, + final ObserverContext ctx, final Put put, final WALEdit edit, final Durability durability) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("prePut: user [{}] on table [{}] with put [{}]", user, tableName, put); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.WRITE, OpType.PUT, familiesFromCells(put.getFamilyCellMap())); } @Override public void preDelete( - final ObserverContext c, + final ObserverContext ctx, final Delete delete, final WALEdit edit, final Durability durability) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preDelete: user [{}] on table [{}] with delete [{}]", user, tableName, delete); - - // the default access controller uses a second enum - OpType - to distinguish between - // different types of write action (e.g. write, delete) - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.WRITE, OpType.DELETE, familiesFromCells(delete.getFamilyCellMap())); } @Override public void postDelete( - final ObserverContext c, + final ObserverContext ctx, final Delete delete, final WALEdit edit, final Durability durability) { @@ -391,13 +413,13 @@ public void postDelete( } @Override - public Result preAppend(ObserverContext c, Append append) + public Result preAppend(ObserverContext ctx, Append append) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preAppend: user [{}] on table [{}] with append [{}]", user, tableName, append); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.WRITE, OpType.APPEND, familiesFromCells(append.getFamilyCellMap())); // as per default access controller return null; @@ -405,11 +427,11 @@ public Result preAppend(ObserverContext c, Append @Override public void preBatchMutate( - ObserverContext c, + ObserverContext ctx, MiniBatchOperationInProgress miniBatchOp) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preBatchMutate: user [{}] on table [{}] with miniBatchOp [{}]", user, @@ -420,9 +442,9 @@ public void preBatchMutate( } @Override - public void preOpen(ObserverContext c) throws IOException { - User user = getActiveUser(c); - final Region region = c.getEnvironment().getRegion(); + public void preOpen(ObserverContext ctx) throws IOException { + User user = getActiveUser(ctx); + final Region region = ctx.getEnvironment().getRegion(); if (region == null) { LOG.error("NULL region from RegionCoprocessorEnvironment in preOpen()"); } else { @@ -433,7 +455,7 @@ public void preOpen(ObserverContext c) throws IOEx } @Override - public void postOpen(ObserverContext c) { + public void postOpen(ObserverContext ctx) { // not needed as the ACL table is not used } @@ -442,37 +464,30 @@ public void preTableFlush( final ObserverContext ctx, final TableName tableName) throws IOException { User user = getActiveUser(ctx); - LOG.trace("preTableFlush: user [{}] on table [{}]", user, tableName); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + LOG.debug("preTableFlush: user [{}] on table [{}]", user, tableName); + requirePermission(ctx, "flushTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override public void preFlush( - ObserverContext c, FlushLifeCycleTracker tracker) + ObserverContext ctx, FlushLifeCycleTracker tracker) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); - LOG.trace("preFlush: user [{}] on table [{}]", user, tableName); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + // Internal storage engine flush — not a user-initiated operation, no authorization needed. } @Override public InternalScanner preCompact( - ObserverContext c, + ObserverContext ctx, Store store, InternalScanner scanner, ScanType scanType, CompactionLifeCycleTracker tracker, CompactionRequest request) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preCompact: user [{}] on table [{}] for scanner [{}]", user, tableName, scanner); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); - + requirePermission(ctx, "compact", tableName, null, null, Action.ADMIN, Action.CREATE); return scanner; } @@ -487,15 +502,13 @@ public void preGetTableDescriptors( // We are delegating the authorization check to postGetTableDescriptors as we don't have // any concrete set of table names when a regex is present or the full list is requested. if (regex == null && tableNamesList != null && !tableNamesList.isEmpty()) { - User user = getActiveUser(ctx); - TableName[] sns = null; try (Admin admin = ctx.getEnvironment().getConnection().getAdmin()) { - sns = admin.listTableNames(); - if (sns == null) return; + if (admin.listTableNames() == null) return; for (TableName tableName : tableNamesList) { // Skip checks for a table that does not exist if (!admin.tableExists(tableName)) continue; - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission( + ctx, "getTableDescriptors", tableName, null, null, Action.ADMIN, Action.CREATE); } } } @@ -512,14 +525,20 @@ public void postGetTableDescriptors( if (regex == null && tableNamesList != null && !tableNamesList.isEmpty()) { return; } - User user = getActiveUser(ctx); // Retains only those which passes authorization checks, as the checks weren't done as part // of preGetTableDescriptors. Iterator itr = descriptors.iterator(); while (itr.hasNext()) { TableDescriptor htd = itr.next(); try { - opaAclChecker.checkPermissionInfo(user, htd.getTableName(), Action.CREATE); + requirePermission( + ctx, + "getTableDescriptors", + htd.getTableName(), + null, + null, + Action.ADMIN, + Action.CREATE); } catch (AccessControlException e) { itr.remove(); } @@ -532,13 +551,12 @@ public void postGetTableNames( List descriptors, String regex) throws IOException { - // Retains only those which passes authorization checks. - User user = getActiveUser(ctx); + // Retains only those on which the user has any permission (matches reference AC). Iterator itr = descriptors.iterator(); while (itr.hasNext()) { TableDescriptor htd = itr.next(); try { - opaAclChecker.checkPermissionInfo(user, htd.getTableName(), Action.CREATE); + requirePermission(ctx, "getTableNames", htd.getTableName(), null, null, Action.values()); } catch (AccessControlException e) { itr.remove(); } @@ -547,7 +565,7 @@ public void postGetTableNames( @Override public boolean preCheckAndPut( - final ObserverContext c, + final ObserverContext ctx, final byte[] row, final byte[] family, final byte[] qualifier, @@ -556,17 +574,17 @@ public boolean preCheckAndPut( final Put put, final boolean result) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preCheckAndPut: user [{}] on table [{}] for put [{}]", user, tableName, put); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + requirePermission( + ctx, "checkAndPut", tableName, null, null, OpType.CHECK_AND_PUT, Action.READ, Action.WRITE); return result; } @Override public boolean preCheckAndPutAfterRowLock( - final ObserverContext c, + final ObserverContext ctx, final byte[] row, final byte[] family, final byte[] qualifier, @@ -575,18 +593,18 @@ public boolean preCheckAndPutAfterRowLock( final Put put, final boolean result) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndPutAfterRowLock: user [{}] on table [{}] for put [{}]", user, tableName, put); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + requirePermission( + ctx, "checkAndPut", tableName, null, null, OpType.CHECK_AND_PUT, Action.READ, Action.WRITE); return result; } @Override public boolean preCheckAndDelete( - final ObserverContext c, + final ObserverContext ctx, final byte[] row, final byte[] family, final byte[] qualifier, @@ -595,18 +613,25 @@ public boolean preCheckAndDelete( final Delete delete, final boolean result) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndDelete: user [{}] on table [{}] for delete [{}]", user, tableName, delete); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + requirePermission( + ctx, + "checkAndDelete", + tableName, + null, + null, + OpType.CHECK_AND_DELETE, + Action.READ, + Action.WRITE); return result; } @Override public boolean preCheckAndDeleteAfterRowLock( - final ObserverContext c, + final ObserverContext ctx, final byte[] row, final byte[] family, final byte[] qualifier, @@ -615,18 +640,142 @@ public boolean preCheckAndDeleteAfterRowLock( final Delete delete, final boolean result) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndDeleteAfterRowLock: user [{}] on table [{}] for delete [{}]", user, tableName, delete); + requirePermission( + ctx, + "checkAndDelete", + tableName, + null, + null, + OpType.CHECK_AND_DELETE, + Action.READ, + Action.WRITE); + return result; + } - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + @Override + public boolean preCheckAndPut( + final ObserverContext ctx, + final byte[] row, + final Filter filter, + final Put put, + final boolean result) + throws IOException { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace("preCheckAndPut: user [{}] on table [{}] for put [{}]", user, tableName, put); + requirePermission( + ctx, "checkAndPut", tableName, null, null, OpType.CHECK_AND_PUT, Action.READ, Action.WRITE); return result; } + @Override + public boolean preCheckAndPutAfterRowLock( + final ObserverContext ctx, + final byte[] row, + final Filter filter, + final Put put, + final boolean result) + throws IOException { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace( + "preCheckAndPutAfterRowLock: user [{}] on table [{}] for put [{}]", user, tableName, put); + requirePermission( + ctx, "checkAndPut", tableName, null, null, OpType.CHECK_AND_PUT, Action.READ, Action.WRITE); + return result; + } + + @Override + public boolean preCheckAndDelete( + final ObserverContext ctx, + final byte[] row, + final Filter filter, + final Delete delete, + final boolean result) + throws IOException { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace( + "preCheckAndDelete: user [{}] on table [{}] for delete [{}]", user, tableName, delete); + requirePermission( + ctx, + "checkAndDelete", + tableName, + null, + null, + OpType.CHECK_AND_DELETE, + Action.READ, + Action.WRITE); + return result; + } + + @Override + public boolean preCheckAndDeleteAfterRowLock( + final ObserverContext ctx, + final byte[] row, + final Filter filter, + final Delete delete, + final boolean result) + throws IOException { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace( + "preCheckAndDeleteAfterRowLock: user [{}] on table [{}] for delete [{}]", + user, + tableName, + delete); + requirePermission( + ctx, + "checkAndDelete", + tableName, + null, + null, + OpType.CHECK_AND_DELETE, + Action.READ, + Action.WRITE); + return result; + } + + @Override + public CheckAndMutateResult preCheckAndMutate( + ObserverContext ctx, + CheckAndMutate checkAndMutate, + CheckAndMutateResult result) + throws IOException { + if (checkAndMutate.getAction() instanceof RowMutations) { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace("preCheckAndMutate (RowMutations): user [{}] on table [{}]", user, tableName); + opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.ROW_MUTATIONS); + return result; + } + return RegionObserver.super.preCheckAndMutate(ctx, checkAndMutate, result); + } + + @Override + public CheckAndMutateResult preCheckAndMutateAfterRowLock( + ObserverContext ctx, + CheckAndMutate checkAndMutate, + CheckAndMutateResult result) + throws IOException { + if (checkAndMutate.getAction() instanceof RowMutations) { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace( + "preCheckAndMutateAfterRowLock (RowMutations): user [{}] on table [{}]", user, tableName); + opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.ROW_MUTATIONS); + return result; + } + return RegionObserver.super.preCheckAndMutateAfterRowLock(ctx, checkAndMutate, result); + } + @Override public void postListNamespaceDescriptors( ObserverContext ctx, List descriptors) { @@ -635,12 +784,11 @@ public void postListNamespaceDescriptors( @Override public void preTruncateTable( - ObserverContext c, final TableName tableName) + ObserverContext ctx, final TableName tableName) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); LOG.debug("preTruncateTable: user [{}] on table [{}]", user, tableName); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission(ctx, "truncateTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override @@ -653,37 +801,40 @@ public void postTruncateTable( @Override public TableDescriptor preModifyTable( - ObserverContext c, + ObserverContext ctx, TableName tableName, TableDescriptor currentDesc, TableDescriptor newDesc) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); LOG.debug("preModifyTable: user [{}] on table [{}]", user, tableName); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission(ctx, "modifyTable", tableName, null, null, Action.ADMIN, Action.CREATE); return currentDesc; } @Override public void postModifyTable( - ObserverContext c, + ObserverContext ctx, TableName tableName, final TableDescriptor htd) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); LOG.trace("postModifyTable: user [{}] on table [{}]", user, tableName); } @Override public Result preIncrement( - final ObserverContext c, final Increment increment) + final ObserverContext ctx, final Increment increment) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preIncrement: user [{}] on table [{}]", user, tableName); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfoWithOp( + user, + tableName, + Action.WRITE, + OpType.INCREMENT, + familiesFromCells(increment.getFamilyCellMap())); // as per default controller return null; } @@ -771,24 +922,24 @@ public Optional getRegionServerObserver() { return Optional.of(this); } - /*********************************** Not implemented (yet) ***********************************/ - @Override public String preModifyTableStoreFileTracker( - ObserverContext c, TableName tableName, String dstSFT) { + ObserverContext ctx, TableName tableName, String dstSFT) + throws IOException { requirePermission( - c, "modifyTableStoreFileTracker", tableName, null, null, Action.ADMIN, Action.CREATE); + ctx, "modifyTableStoreFileTracker", tableName, null, null, Action.ADMIN, Action.CREATE); return dstSFT; } @Override public String preModifyColumnFamilyStoreFileTracker( - ObserverContext c, + ObserverContext ctx, TableName tableName, byte[] family, - String dstSFT) { + String dstSFT) + throws IOException { requirePermission( - c, + ctx, "modifyColumnFamilyStoreFileTracker", tableName, family, @@ -800,34 +951,38 @@ public String preModifyColumnFamilyStoreFileTracker( @Override public void preMove( - ObserverContext c, + ObserverContext ctx, RegionInfo region, ServerName srcServer, - ServerName destServer) { - requirePermission(c, "move", region.getTable(), null, null, Action.ADMIN); + ServerName destServer) + throws IOException { + requirePermission(ctx, "move", region.getTable(), null, null, Action.ADMIN); } @Override - public void preAssign(ObserverContext c, RegionInfo regionInfo) { - requirePermission(c, "assign", regionInfo.getTable(), null, null, Action.ADMIN); + public void preAssign(ObserverContext ctx, RegionInfo regionInfo) + throws IOException { + requirePermission(ctx, "assign", regionInfo.getTable(), null, null, Action.ADMIN); } @Override - public void preUnassign(ObserverContext c, RegionInfo regionInfo) { - requirePermission(c, "unassign", regionInfo.getTable(), null, null, Action.ADMIN); + public void preUnassign(ObserverContext ctx, RegionInfo regionInfo) + throws IOException { + requirePermission(ctx, "unassign", regionInfo.getTable(), null, null, Action.ADMIN); } @Override public void preRegionOffline( - ObserverContext c, RegionInfo regionInfo) { - requirePermission(c, "regionOffline", regionInfo.getTable(), null, null, Action.ADMIN); + ObserverContext ctx, RegionInfo regionInfo) throws IOException { + requirePermission(ctx, "regionOffline", regionInfo.getTable(), null, null, Action.ADMIN); } @Override public void preSnapshot( final ObserverContext ctx, final SnapshotDescription snapshot, - final TableDescriptor hTableDescriptor) { + final TableDescriptor hTableDescriptor) + throws IOException { requirePermission( ctx, "snapshot " + snapshot.getName(), @@ -839,90 +994,91 @@ public void preSnapshot( @Override public void preListSnapshot( - ObserverContext ctx, final SnapshotDescription snapshot) { - LOG.debug("preListSnapshot not yet implemented! Snapshot: {}", snapshot); + ObserverContext ctx, final SnapshotDescription snapshot) + throws IOException { + User user = getActiveUser(ctx); + LOG.debug("preListSnapshot: user [{}] snapshot[{}]", user, snapshot); + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "listSnapshot", Action.ADMIN); } @Override public void preCloneSnapshot( final ObserverContext ctx, final SnapshotDescription snapshot, - final TableDescriptor hTableDescriptor) { - LOG.debug("preCloneSnapshot not yet implemented! Snapshot: {}", snapshot); + final TableDescriptor hTableDescriptor) + throws IOException { + User user = getActiveUser(ctx); + TableName tableName = hTableDescriptor.getTableName(); + LOG.debug("preCloneSnapshot: user [{}] snapshot[{}] table [{}]", user, snapshot, tableName); + requirePermission(ctx, tableName.getNamespaceAsString(), "cloneSnapshot", Action.ADMIN); } @Override public void preRestoreSnapshot( final ObserverContext ctx, final SnapshotDescription snapshot, - final TableDescriptor hTableDescriptor) { - LOG.debug("preRestoreSnapshot not yet implemented! Snapshot: {}", snapshot); + final TableDescriptor hTableDescriptor) + throws IOException { + User user = getActiveUser(ctx); + LOG.debug("preRestoreSnapshot: user [{}] snapshot[{}]", user, snapshot); + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "restoreSnapshot", Action.ADMIN); } @Override public void preDeleteSnapshot( - final ObserverContext ctx, final SnapshotDescription snapshot) { - LOG.debug("preDeleteSnapshot not yet implemented! Snapshot: {}", snapshot); + final ObserverContext ctx, final SnapshotDescription snapshot) + throws IOException { + User user = getActiveUser(ctx); + LOG.debug("preDeleteSnapshot: user [{}] snapshot[{}]", user, snapshot); + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "deleteSnapshot", Action.ADMIN); } @Override public void preSplitRegion( final ObserverContext ctx, final TableName tableName, - final byte[] splitRow) { + final byte[] splitRow) + throws IOException { requirePermission(ctx, "split", tableName, null, null, Action.ADMIN); } @Override public void preBulkLoadHFile( - ObserverContext ctx, List> familyPaths) { - LOG.debug("preBulkLoadHFile not implemented!"); - } - - @Override - public void prePrepareBulkLoad(ObserverContext ctx) { - LOG.debug("prePrepareBulkLoad not implemented!"); - } - - @Override - public void preCleanupBulkLoad(ObserverContext ctx) { - LOG.debug("preCleanupBulkLoad not implemented!"); - } - - @Override - public Message preEndpointInvocation( - ObserverContext ctx, - Service service, - String methodName, - Message request) { - LOG.debug("preEndpointInvocation not implemented! {}/{}", methodName, request); - return request; - } - - @Override - public void postEndpointInvocation( - ObserverContext ctx, - Service service, - String methodName, - Message request, - Message.Builder responseBuilder) { - LOG.debug("postEndpointInvocation not implemented! {}/{}", methodName, request); + ObserverContext ctx, List> familyPaths) + throws IOException { + User user = getActiveUser(ctx); + var tableName = ctx.getEnvironment().getRegion().getTableDescriptor().getTableName(); + LOG.debug("preBulkLoadHFile: user [{}] on table [{}]", user, tableName); + requirePermission(ctx, "preBulkLoadHFile", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override - public void preRequestLock( - ObserverContext ctx, - String namespace, - TableName tableName, - RegionInfo[] regionInfos, - String description) { - LOG.debug("preRequestLock not implemented! {}/{}", tableName, regionInfos); + public void prePrepareBulkLoad(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, + "prePrepareBulkLoad", + ctx.getEnvironment().getRegion().getTableDescriptor().getTableName(), + null, + null, + Action.ADMIN, + Action.CREATE); } @Override - public void preLockHeartbeat( - ObserverContext ctx, TableName tableName, String description) { - LOG.debug("preLockHeartbeat not implemented! {}/{}", tableName, description); + public void preCleanupBulkLoad(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, + "preCleanupBulkLoad", + ctx.getEnvironment().getRegion().getTableDescriptor().getTableName(), + null, + null, + Action.ADMIN, + Action.CREATE); } @Override @@ -930,7 +1086,8 @@ public void preSetUserQuota( final ObserverContext ctx, final String userName, final TableName tableName, - final GlobalQuotaSettings quotas) { + final GlobalQuotaSettings quotas) + throws IOException { requirePermission(ctx, "setUserTableQuota", tableName, null, null, Action.ADMIN); } @@ -939,15 +1096,18 @@ public void preSetUserQuota( final ObserverContext ctx, final String userName, final String namespace, - final GlobalQuotaSettings quotas) { - requirePermission(ctx, "setUserNamespaceQuota", Action.ADMIN); + final GlobalQuotaSettings quotas) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "setUserNamespaceQuota", Action.ADMIN); } @Override public void preSetTableQuota( final ObserverContext ctx, final TableName tableName, - final GlobalQuotaSettings quotas) { + final GlobalQuotaSettings quotas) + throws IOException { requirePermission(ctx, "setTableQuota", tableName, null, null, Action.ADMIN); } @@ -955,13 +1115,16 @@ public void preSetTableQuota( public void preSetNamespaceQuota( final ObserverContext ctx, final String namespace, - final GlobalQuotaSettings quotas) { - requirePermission(ctx, "setNamespaceQuota", Action.ADMIN); + final GlobalQuotaSettings quotas) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "setNamespaceQuota", Action.ADMIN); } @Override public void preMergeRegions( - final ObserverContext ctx, final RegionInfo[] regionsToMerge) { + final ObserverContext ctx, final RegionInfo[] regionsToMerge) + throws IOException { requirePermission(ctx, "mergeRegions", regionsToMerge[0].getTable(), null, null, Action.ADMIN); } @@ -972,12 +1135,87 @@ public void preGetUserPermissions( String namespace, TableName tableName, byte[] family, - byte[] qualifier) { - LOG.debug("preGetUserPermissions not implemented! {}/{}", userName, tableName); + byte[] qualifier) + throws IOException { + User user = getActiveUser(ctx); + if (tableName != null) { + LOG.debug("preGetUserPermissions: user [{}] on table [{}]", user, tableName); + requirePermission(ctx, "getUserPermissions", tableName, family, qualifier, Action.ADMIN); + } else if (namespace != null) { + LOG.debug("preGetUserPermissions: user [{}] on namespace [{}]", user, namespace); + requirePermission(ctx, namespace, "getUserPermissions", Action.ADMIN); + } else { + LOG.debug("preGetUserPermissions: user [{}] global", user); + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "getUserPermissions", Action.ADMIN); + } + } + + /** Converts a mutation's family→cell map to a family→qualifier-names map for OPA. */ + private static Map> familiesFromCells( + Map> familyCellMap) { + Map> result = new TreeMap<>(); + for (Map.Entry> entry : familyCellMap.entrySet()) { + String family = Bytes.toString(entry.getKey()); + List qualifiers = + entry.getValue().stream() + .map(cell -> Bytes.toString(CellUtil.cloneQualifier(cell))) + .distinct() + .collect(Collectors.toList()); + result.put(family, qualifiers); + } + return result; + } + + /** Converts a Get/Scan family→qualifier map to a family→qualifier-names map for OPA. */ + private static Map> familiesFromQualifiers( + Map> familyQualMap) { + Map> result = new TreeMap<>(); + for (Map.Entry> entry : familyQualMap.entrySet()) { + String family = Bytes.toString(entry.getKey()); + List qualifiers = + entry.getValue() == null + ? Collections.emptyList() + : entry.getValue().stream().map(Bytes::toString).collect(Collectors.toList()); + result.put(family, qualifiers); + } + return result; } - private void requirePermission(ObserverContext ctx, String request, Action perm) { - LOG.debug("requirePermission not implemented! {}/{}", request, perm); + /** Builds a single-entry family map for OPA from explicit family/qualifier byte arrays. */ + private static ImmutableMap> familyMap(byte[] family, byte[] qualifier) { + if (family == null) return ImmutableMap.of(); + return ImmutableMap.of( + Bytes.toString(family), + qualifier != null + ? Collections.singletonList(Bytes.toString(qualifier)) + : Collections.emptyList()); + } + + private void requirePermission( + final ObserverContext ctx, final String namespace, String request, Action... permissions) + throws IOException { + User user = getActiveUser(ctx); + AccessControlException[] last = {null}; + boolean allowed = + Arrays.stream(permissions) + .anyMatch( + perm -> { + LOG.trace( + "requirePermission: user [{}] namespace[{}] request [{}] permission [{}]", + user, + namespace, + request, + perm); + try { + opaAclChecker.checkPermissionInfo(user, namespace, perm); + return true; + } catch (AccessControlException e) { + last[0] = e; + return false; + } + }); + if (!allowed) throw last[0]; } private void requirePermission( @@ -986,206 +1224,355 @@ private void requirePermission( TableName tableName, byte[] family, byte[] qualifier, - Action... permissions) { - LOG.debug("requirePermission for table not implemented! {}/{}", tableName, permissions); + Action... permissions) + throws IOException { + requirePermission(ctx, request, tableName, family, qualifier, OpType.NONE, permissions); } - /*********** Not implemented (admin tasks coming from the Master or RegionServer) *************************/ + private void requirePermission( + ObserverContext ctx, + String request, + TableName tableName, + byte[] family, + byte[] qualifier, + OpType opType, + Action... permissions) + throws IOException { + User user = getActiveUser(ctx); + AccessControlException[] last = {null}; + boolean allowed = + Arrays.stream(permissions) + .anyMatch( + perm -> { + LOG.trace( + "requirePermission: user [{}] tableName[{}] request [{}] permission [{}]", + user, + tableName, + request, + perm); + try { + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, perm, opType, familyMap(family, qualifier)); + return true; + } catch (AccessControlException e) { + last[0] = e; + return false; + } + }); + if (!allowed) throw last[0]; + } + + @Override + public void preBalance(ObserverContext ctx, BalanceRequest request) + throws IOException { + requirePermission(ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "balance", Action.ADMIN); + } @Override - public void preAbortProcedure( - ObserverContext ctx, final long procId) { - LOG.debug("preAbortProcedure not implemented!"); + public void preBalanceSwitch(ObserverContext ctx, boolean newValue) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "balanceSwitch", Action.ADMIN); } @Override - public void postAbortProcedure(ObserverContext ctx) { - // There is nothing to do at this time after the procedure abort request was sent. + public void preShutdown(ObserverContext ctx) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "shutdown", Action.ADMIN); } @Override - public void preGetProcedures(ObserverContext ctx) { - LOG.debug("preGetProcedures not implemented!"); + public void preStopMaster(ObserverContext ctx) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "stopMaster", Action.ADMIN); } @Override - public void preGetLocks(ObserverContext ctx) { - LOG.trace("preGetLocks not implemented!"); + public void preClearDeadServers(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "clearDeadServers", Action.ADMIN); } @Override - public void preSetSplitOrMergeEnabled( - final ObserverContext ctx, - final boolean newValue, - final MasterSwitchType switchType) { - LOG.debug("preSetSplitOrMergeEnabled not implemented!"); + public void preDecommissionRegionServers( + ObserverContext ctx, List servers, boolean offload) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "decommissionRegionServers", + Action.ADMIN); } @Override - public void preBalance(ObserverContext c, BalanceRequest request) { - LOG.debug("preBalance not implemented!"); + public void preListDecommissionedRegionServers(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "listDecommissionedRegionServers", + Action.READ); } @Override - public void preBalanceSwitch(ObserverContext c, boolean newValue) { - LOG.debug("preBalanceSwitch not implemented!"); + public void preRecommissionRegionServer( + ObserverContext ctx, + ServerName server, + List encodedRegionNames) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "recommissionRegionServers", + Action.ADMIN); } @Override - public void preShutdown(ObserverContext c) { - LOG.debug("preShutdown not implemented!"); + public void preStopRegionServer(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "preStopRegionServer", Action.ADMIN); } @Override - public void preStopMaster(ObserverContext c) { - LOG.debug("preStopMaster not implemented!"); + public Message preEndpointInvocation( + ObserverContext ctx, + Service service, + String methodName, + Message request) + throws IOException { + // Skip EXEC check for calls to the AccessControlService itself to avoid recursive checks. + if (!(service instanceof AccessControlProtos.AccessControlService)) { + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + LOG.debug( + "preEndpointInvocation: user [{}] on table [{}] method [{}]", + user, + tableName, + methodName); + requirePermission( + ctx, + "invoke(" + service.getDescriptorForType().getName() + "." + methodName + ")", + tableName, + null, + null, + Action.EXEC); + } + return request; } @Override - public void postStartMaster(ObserverContext ctx) { - LOG.debug("postStartMaster not implemented!"); + public void postEndpointInvocation( + ObserverContext ctx, + Service service, + String methodName, + Message request, + Message.Builder responseBuilder) { + // as per reference AccessController } @Override - public void preClearDeadServers(ObserverContext ctx) { - LOG.debug("preClearDeadServers not implemented!"); + public void preRequestLock( + ObserverContext ctx, + String namespace, + TableName tableName, + RegionInfo[] regionInfos, + String description) + throws IOException { + User user = getActiveUser(ctx); + LOG.debug("preRequestLock: user [{}] namespace [{}] table [{}]", user, namespace, tableName); + if (namespace != null && !namespace.isEmpty()) { + requirePermission(ctx, namespace, "requestLock", Action.ADMIN, Action.CREATE); + } else { + TableName tn = tableName != null ? tableName : regionInfos[0].getTable(); + requirePermission(ctx, "requestLock", tn, null, null, Action.ADMIN, Action.CREATE); + } } @Override - public void preDecommissionRegionServers( - ObserverContext ctx, - List servers, - boolean offload) { - LOG.debug("preDecommissionRegionServers not implemented!"); + public void preLockHeartbeat( + ObserverContext ctx, TableName tableName, String description) + throws IOException { + User user = getActiveUser(ctx); + LOG.debug("preLockHeartbeat: user [{}] table [{}]", user, tableName); + requirePermission(ctx, "lockHeartbeat", tableName, null, null, Action.ADMIN, Action.CREATE); } + /*********************************** Global admin operations ***********************************/ + @Override - public void preListDecommissionedRegionServers( - ObserverContext ctx) { - LOG.debug("preListDecommissionedRegionServers not implemented!"); + public void preAbortProcedure( + ObserverContext ctx, final long procId) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "abortProcedure", Action.ADMIN); } @Override - public void preRecommissionRegionServer( - ObserverContext ctx, - ServerName server, - List encodedRegionNames) { - LOG.debug("preRecommissionRegionServer not implemented!"); + public void preGetProcedures(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "getProcedures", Action.ADMIN); } @Override - public void preStopRegionServer(ObserverContext ctx) { - LOG.debug("preStopRegionServer not implemented!"); + public void preGetLocks(ObserverContext ctx) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "getLocks", Action.ADMIN); } @Override - public void preRollWALWriterRequest(ObserverContext ctx) { - LOG.debug("preRollWALWriterRequest not implemented!"); + public void preSetSplitOrMergeEnabled( + final ObserverContext ctx, + final boolean newValue, + final MasterSwitchType switchType) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "setSplitOrMergeEnabled", + Action.ADMIN); } @Override - public void postRollWALWriterRequest(ObserverContext ctx) { - // as per default access controller + public void postStartMaster(ObserverContext ctx) { + // This would be used to create an ACL table if it does not already exist. + // We do not use an ACL table as all checks are routed to OPA. + } + + @Override + public void preRollWALWriterRequest(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "rollWALWriterRequest", Action.ADMIN); } @Override public void preSetUserQuota( final ObserverContext ctx, final String userName, - final GlobalQuotaSettings quotas) { - LOG.debug("preSetUserQuota not implemented!"); + final GlobalQuotaSettings quotas) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "setUserQuota", Action.ADMIN); } @Override public void preSetRegionServerQuota( ObserverContext ctx, final String regionServer, - GlobalQuotaSettings quotas) { - LOG.debug("preSetRegionServerQuota not implemented!"); - } - - @Override - public ReplicationEndpoint postCreateReplicationEndPoint( - ObserverContext ctx, ReplicationEndpoint endpoint) { - return endpoint; + GlobalQuotaSettings quotas) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "setRegionServerQuota", Action.ADMIN); } @Override - public void preReplicateLogEntries(ObserverContext ctx) { - LOG.debug("preReplicateLogEntries not implemented!"); + public void preReplicateLogEntries(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "replicateLogEntries", Action.WRITE); } @Override - public void preClearCompactionQueues(ObserverContext ctx) { - LOG.debug("preClearCompactionQueues not implemented!"); + public void preClearCompactionQueues(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "clearCompactionQueues", Action.ADMIN); } @Override public void preAddReplicationPeer( final ObserverContext ctx, String peerId, - ReplicationPeerConfig peerConfig) { - LOG.debug("preAddReplicationPeer not implemented!"); + ReplicationPeerConfig peerConfig) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "addReplicationPeer", Action.ADMIN); } @Override public void preRemoveReplicationPeer( - final ObserverContext ctx, String peerId) { - LOG.debug("preRemoveReplicationPeer not implemented!"); + final ObserverContext ctx, String peerId) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "removeReplicationPeer", Action.ADMIN); } @Override public void preEnableReplicationPeer( - final ObserverContext ctx, String peerId) { - LOG.debug("preEnableReplicationPeer not implemented!"); + final ObserverContext ctx, String peerId) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "enableReplicationPeer", Action.ADMIN); } @Override public void preDisableReplicationPeer( - final ObserverContext ctx, String peerId) { - LOG.debug("preDisableReplicationPeer not implemented!"); + final ObserverContext ctx, String peerId) throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "disableReplicationPeer", + Action.ADMIN); } @Override public void preGetReplicationPeerConfig( - final ObserverContext ctx, String peerId) { - LOG.debug("preGetReplicationPeerConfig not implemented!"); + final ObserverContext ctx, String peerId) throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "getReplicationPeerConfig", + Action.ADMIN); } @Override public void preUpdateReplicationPeerConfig( final ObserverContext ctx, String peerId, - ReplicationPeerConfig peerConfig) { - LOG.debug("preUpdateReplicationPeerConfig not implemented!"); + ReplicationPeerConfig peerConfig) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "updateReplicationPeerConfig", + Action.ADMIN); } @Override public void preListReplicationPeers( - final ObserverContext ctx, String regex) { - LOG.debug("preListReplicationPeers not implemented!"); + final ObserverContext ctx, String regex) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "listReplicationPeers", Action.ADMIN); } @Override public void preExecuteProcedures(ObserverContext ctx) { - LOG.debug("preExecuteProcedures not implemented!"); + // Not implemented: reference AC uses checkSystemOrSuperUser, a superuser mechanism + // not applicable to OPA-based authorization. } @Override public void preSwitchRpcThrottle( - ObserverContext ctx, boolean enable) { - LOG.debug("preSwitchRpcThrottle not implemented!"); + ObserverContext ctx, boolean enable) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "switchRpcThrottle", Action.ADMIN); } @Override - public void preIsRpcThrottleEnabled(ObserverContext ctx) { - LOG.debug("preIsRpcThrottleEnabled not implemented!"); + public void preIsRpcThrottleEnabled(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "isRpcThrottleEnabled", Action.ADMIN); } @Override public void preSwitchExceedThrottleQuota( - ObserverContext ctx, boolean enable) { - LOG.debug("preSwitchExceedThrottleQuota not implemented!"); + ObserverContext ctx, boolean enable) throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "switchExceedThrottleQuota", + Action.ADMIN); } @Override @@ -1193,13 +1580,13 @@ public void preGrant( ObserverContext ctx, UserPermission userPermission, boolean mergeExistingPermissions) { - LOG.debug("preGrant not implemented!"); + // Not implemented: permissions are managed in OPA, not via HBase ACL table operations. } @Override public void preRevoke( ObserverContext ctx, UserPermission userPermission) { - LOG.debug("preRevoke not implemented!"); + // Not implemented: permissions are managed in OPA, not via HBase ACL table operations. } @Override @@ -1207,23 +1594,35 @@ public void preHasUserPermissions( ObserverContext ctx, String userName, List permissions) { - LOG.debug("preHasUserPermissions not implemented!"); + // Not implemented: permission checks are routed to OPA directly. } @Override - public void preClearRegionBlockCache(ObserverContext ctx) { - LOG.debug("preClearRegionBlockCache not implemented!"); + public void preClearRegionBlockCache(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "clearRegionBlockCache", Action.ADMIN); } @Override public void preUpdateRegionServerConfiguration( - ObserverContext ctx, Configuration preReloadConf) { - LOG.debug("preUpdateRegionServerConfiguration not implemented!"); + ObserverContext ctx, Configuration preReloadConf) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "updateRegionServerConfiguration", + Action.ADMIN); } @Override public void preUpdateMasterConfiguration( - ObserverContext ctx, Configuration preReloadConf) { - LOG.debug("preUpdateMasterConfiguration not implemented!"); + ObserverContext ctx, Configuration preReloadConf) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "updateMasterConfiguration", + Action.ADMIN); } } diff --git a/src/main/java/tech/stackable/hbase/opa/OpType.java b/src/main/java/tech/stackable/hbase/opa/OpType.java new file mode 100644 index 0000000..125cd98 --- /dev/null +++ b/src/main/java/tech/stackable/hbase/opa/OpType.java @@ -0,0 +1,30 @@ +package tech.stackable.hbase.opa; + +/** + * Region-level operation types, mirroring the OpType enum in the HBase reference AccessController. + * {@code NONE} and {@code ROW_MUTATIONS} are extensions not present in the reference. + */ +public enum OpType { + NONE("none"), + GET("get"), + EXISTS("exists"), + SCAN("scan"), + PUT("put"), + DELETE("delete"), + CHECK_AND_PUT("checkAndPut"), + CHECK_AND_DELETE("checkAndDelete"), + APPEND("append"), + INCREMENT("increment"), + ROW_MUTATIONS("rowMutations"); + + private final String value; + + OpType(String value) { + this.value = value; + } + + @Override + public String toString() { + return value; + } +} diff --git a/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java b/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java index 9dac7f2..ca7ac07 100644 --- a/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java +++ b/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java @@ -12,6 +12,9 @@ import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; +import java.util.Collections; +import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.concurrent.TimeUnit; @@ -80,13 +83,38 @@ public OpaAclChecker( } } - public void checkPermissionInfo(User user, TableName table, Permission.Action action) + private void checkPermissionInfo(User user, TableName table, Permission.Action action) + throws AccessControlException { + checkPermissionInfoWithOp(user, table, action, OpType.NONE); + } + + public void checkPermissionInfoWithOp( + User user, TableName table, Permission.Action action, OpType operation) + throws AccessControlException { + checkPermissionInfoWithOp(user, table, action, operation, Collections.emptyMap()); + } + + public void checkPermissionInfoWithOp( + User user, + TableName table, + Permission.Action action, + OpType operation, + Map> families) throws AccessControlException { OpaAllowQuery query = - new OpaAllowQuery(new OpaAllowQuery.OpaAllowQueryInput(user.getUGI(), table, action)); + new OpaAllowQuery( + new OpaAllowQuery.OpaAllowQueryInput( + user.getUGI(), table, action, operation, families)); this.checkPermissionInfo(query); } + public void checkPermissionInfo(User user, TableName table, Permission.Action... actions) + throws AccessControlException { + for (Permission.Action action : actions) { + checkPermissionInfo(user, table, action); + } + } + public void checkPermissionInfo(User user, String namespace, Permission.Action action) throws AccessControlException { OpaAllowQuery query = diff --git a/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java b/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java index 1ca0abd..b6abca6 100644 --- a/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java +++ b/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java @@ -1,5 +1,8 @@ package tech.stackable.hbase.opa; +import java.util.Collections; +import java.util.List; +import java.util.Map; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.security.UserGroupInformation; @@ -16,22 +19,45 @@ public static class OpaAllowQueryInput { public final TableName table; public final String namespace; public final Permission.Action action; + public final OpType operation; + + /** + * Column families and their qualifiers being accessed. An empty qualifier list means CF-level + * access; a non-empty list means KV-level access to specific qualifiers within that family. + */ + public final Map> families; public OpaAllowQueryInput(UserGroupInformation ugi, TableName table, Permission.Action action) { - this.callerUgi = new OpaQueryUgi(ugi); + this(ugi, table, action, null); + } + public OpaAllowQueryInput( + UserGroupInformation ugi, TableName table, Permission.Action action, OpType operation) { + this(ugi, table, action, operation, Collections.emptyMap()); + } + + public OpaAllowQueryInput( + UserGroupInformation ugi, + TableName table, + Permission.Action action, + OpType operation, + Map> families) { + this.callerUgi = new OpaQueryUgi(ugi); this.table = table; this.action = action; this.namespace = table.getNamespaceAsString(); + this.operation = operation; + this.families = families; } public OpaAllowQueryInput( UserGroupInformation ugi, String namespace, Permission.Action action) { this.callerUgi = new OpaQueryUgi(ugi); - this.table = null; this.action = action; this.namespace = namespace; + this.operation = OpType.NONE; + this.families = Collections.emptyMap(); } } } diff --git a/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java b/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java new file mode 100644 index 0000000..c00959f --- /dev/null +++ b/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java @@ -0,0 +1,195 @@ +package tech.stackable.hbase; + +import static org.junit.Assert.assertTrue; + +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +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.junit.Test; + +/** + * Verifies that every method in the coprocessor observer interfaces is either explicitly overridden + * in OpenPolicyAgentAccessController or listed in the known exclusion set below. + * + *

Because all HBase observer interface methods have default implementations, adding a new hook + * upstream would not cause a compile error. This test catches that: any new method that appears in + * an observer interface and is not in our class or in EXCLUDED will cause a test failure. + * + *

When upgrading HBase, if this test fails, review the new method(s) and either: + * + *

    + *
  • Implement them in OpenPolicyAgentAccessController, or + *
  • Add them to the appropriate section of EXCLUDED with a justification comment. + *
+ * + *

Note: all {@code post*} methods are excluded automatically — they fire after the operation has + * already been permitted and cannot block it, so OPA enforcement is never applicable. + */ +public class TestCoprocessorInterfaceCoverage { + + private static final Class[] OBSERVER_INTERFACES = { + MasterObserver.class, + RegionObserver.class, + RegionServerObserver.class, + EndpointObserver.class, + BulkLoadObserver.class, + }; + + /** + * Pre-hooks that are deliberately not overridden. Organised by reason. When a new HBase version + * adds a method that belongs here, add it with a comment explaining why. + */ + private static final Set EXCLUDED = + new HashSet<>( + Arrays.asList( + + // --- deprecated overloads superseded by variants we do override --- + // The WALEdit-carrying variants of data-write hooks were deprecated in HBase 2.x; + // the 2-arg versions (which we do override) are the current API. + "preAppend(ObserverContext, Append, WALEdit)", + "preDelete(ObserverContext, Delete, WALEdit)", + "preIncrement(ObserverContext, Increment, WALEdit)", + "prePut(ObserverContext, Put, WALEdit)", + // Old single-descriptor overload; we override the 4-arg (old + new) variant. + "preModifyTable(ObserverContext, TableName, TableDescriptor)", + // Old 2-descriptor namespace overload; we override the 2-arg (new descriptor only) + // variant. + "preModifyNamespace(ObserverContext, NamespaceDescriptor, NamespaceDescriptor)", + // Old 3-arg unassign with boolean; we override the 2-arg variant. + "preUnassign(ObserverContext, RegionInfo, boolean)", + + // --- after-row-lock variants where we check at the pre-lock level --- + // HBase calls the pre-lock hook before acquiring the row lock and the after-lock hook + // after. + // We enforce permissions at the pre-lock level (preAppend, preIncrement), so the + // after-lock + // variants are redundant for OPA authorization. + "preAppendAfterRowLock(ObserverContext, Append)", + "preIncrementAfterRowLock(ObserverContext, Increment)", + + // --- internal HBase multi-step DDL action hooks --- + // These are called internally by the HBase master during multi-step DDL procedures. + // They are not triggered by direct client calls; the public pre* hooks (e.g. + // preCreateTable) + // are already checked before these fire. + "preCreateTableAction(ObserverContext, TableDescriptor, RegionInfo[])", + "preCreateTableRegionsInfos(ObserverContext, TableDescriptor)", + "preDeleteTableAction(ObserverContext, TableName)", + "preEnableTableAction(ObserverContext, TableName)", + "preDisableTableAction(ObserverContext, TableName)", + "preTruncateTableAction(ObserverContext, TableName)", + "preTruncateRegion(ObserverContext, RegionInfo)", + "preTruncateRegionAction(ObserverContext, RegionInfo)", + "preModifyTableAction(ObserverContext, TableName, TableDescriptor)", + "preModifyTableAction(ObserverContext, TableName, TableDescriptor, TableDescriptor)", + "preMergeRegionsAction(ObserverContext, RegionInfo[])", + "preMergeRegionsCommitAction(ObserverContext, RegionInfo[], List)", + "preSplitRegionAction(ObserverContext, TableName, byte[])", + "preSplitRegionBeforeMETAAction(ObserverContext, byte[], List)", + "preSplitRegionAfterMETAAction(ObserverContext)", + + // --- internal storage, compaction, and scan hooks --- + // These are called by HBase's internal storage engine for compaction, flush, and scan + // operations. They are not user-initiated and carry no meaningful authorization + // context. + "preClose(ObserverContext, boolean)", + "preCommitStoreFile(ObserverContext, byte[], List)", + "preCompactScannerOpen(ObserverContext, Store, ScanType, ScanOptions, CompactionLifeCycleTracker, CompactionRequest)", + "preCompactSelection(ObserverContext, Store, List, CompactionLifeCycleTracker)", + "preFlush(ObserverContext, Store, InternalScanner, FlushLifeCycleTracker)", + "preFlushScannerOpen(ObserverContext, Store, ScanOptions, FlushLifeCycleTracker)", + "preMemStoreCompaction(ObserverContext, Store)", + "preMemStoreCompactionCompact(ObserverContext, Store, InternalScanner)", + "preMemStoreCompactionCompactScannerOpen(ObserverContext, Store, ScanOptions)", + "prePrepareTimeStampForDeleteVersion(ObserverContext, Mutation, Cell, byte[], Get)", + "preStoreFileReaderOpen(ObserverContext, FileSystem, Path, FSDataInputStreamWrapper, long, CacheConfig, Reference, StoreFileReader)", + "preStoreScannerOpen(ObserverContext, Store, ScanOptions)", + + // --- WAL, replication, and master lifecycle hooks --- + // Triggered by HBase internals (WAL writers, replication pipeline, master startup), + // not by user requests. + "preMasterInitialization(ObserverContext)", + "preMasterStoreFlush(ObserverContext)", + "preReplayWALs(ObserverContext, RegionInfo, Path)", + "preWALAppend(ObserverContext, WALKey, WALEdit)", + "preWALRestore(ObserverContext, RegionInfo, WALKey, WALEdit)", + "preReplicationSinkBatchMutate(ObserverContext, WALEntry, Mutation)", + + // --- RSGroup management --- + // RSGroups are an optional HBase feature for grouping RegionServers. Not yet + // implemented. + // All RSGroup operations should require ADMIN when implemented. + "preAddRSGroup(ObserverContext, String)", + "preRemoveRSGroup(ObserverContext, String)", + "preBalanceRSGroup(ObserverContext, String, BalanceRequest)", + "preGetRSGroupInfo(ObserverContext, String)", + "preGetRSGroupInfoOfServer(ObserverContext, Address)", + "preGetRSGroupInfoOfTable(ObserverContext, TableName)", + "preListRSGroups(ObserverContext)", + "preMoveServers(ObserverContext, Set, String)", + "preMoveServersAndTables(ObserverContext, Set, Set, String)", + "preMoveTables(ObserverContext, Set, String)", + "preRemoveServers(ObserverContext, Set)", + "preRenameRSGroup(ObserverContext, String, String)", + "preUpdateRSGroupConfig(ObserverContext, String, Map)", + + // --- TODO: genuine gaps that need OPA implementation --- + // These pre-hooks are user-facing and should enforce OPA permissions, but are not yet + // implemented. They are excluded here to keep this test focused on detecting new + // upstream methods; the implementation gaps are tracked separately in plan.md. + // + // Metadata listing hooks: getTableNames is covered post-hoc by postGetTableNames + // filtering; listNamespace* hooks are not currently enforced. + "preGetTableNames(ObserverContext, List, String)", + "preListNamespaceDescriptors(ObserverContext, List)", + "preListNamespaces(ObserverContext, List)", + // Cluster metrics: currently unenforced; reference AC requires ADMIN. + "preGetClusterMetrics(ObserverContext)")); + + @Test + public void testAllObserverMethodsAreExplicitlyOverridden() { + Set interfaceMethodSigs = + Arrays.stream(OBSERVER_INTERFACES) + .flatMap(iface -> Arrays.stream(iface.getDeclaredMethods())) + .filter(m -> !m.isSynthetic() && !Modifier.isStatic(m.getModifiers())) + .map(TestCoprocessorInterfaceCoverage::signature) + .collect(Collectors.toSet()); + + Set ourMethodSigs = + Arrays.stream(OpenPolicyAgentAccessController.class.getDeclaredMethods()) + .filter(m -> !m.isSynthetic()) + .map(TestCoprocessorInterfaceCoverage::signature) + .collect(Collectors.toSet()); + + List unhandled = + interfaceMethodSigs.stream() + .filter(sig -> !sig.startsWith("post")) // post hooks can never block operations + .filter(sig -> !EXCLUDED.contains(sig)) + .filter(sig -> !ourMethodSigs.contains(sig)) + .sorted() + .collect(Collectors.toList()); + + assertTrue( + "Observer interface pre-hooks found that are neither overridden nor in the exclusion list" + + " — review each and either implement it or add it to EXCLUDED with a justification:\n" + + String.join("\n", unhandled), + unhandled.isEmpty()); + } + + private static String signature(Method m) { + String params = + Arrays.stream(m.getParameterTypes()) + .map(Class::getSimpleName) + .collect(Collectors.joining(", ")); + return m.getName() + "(" + params + ")"; + } +} diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java index 7b69db0..bfabe2c 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java @@ -6,44 +6,81 @@ import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static org.apache.hadoop.hbase.security.access.SecureTestUtil.createTable; import static org.apache.hadoop.hbase.security.access.SecureTestUtil.deleteTable; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.junit.WireMockRule; import java.util.ArrayList; import java.util.List; -import java.util.Optional; -import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.NamespaceDescriptor; +import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.MasterSwitchType; import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.client.RegionInfoBuilder; +import org.apache.hadoop.hbase.client.SnapshotDescription; import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment; +import org.apache.hadoop.hbase.coprocessor.ObserverContext; import org.apache.hadoop.hbase.coprocessor.ObserverContextImpl; import org.apache.hadoop.hbase.master.MasterCoprocessorHost; +import org.apache.hadoop.hbase.quotas.GlobalQuotaSettings; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.SecureTestUtil; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.security.AccessControlException; -import org.junit.Rule; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; import org.junit.Test; public class TestOpenPolicyAgentAccessController extends TestUtils { public static final String OPA_URL = "http://localhost:8089"; - @Rule public WireMockRule wireMockRule = new WireMockRule(8089); + @ClassRule public static WireMockRule wireMockRule = new WireMockRule(8089); + + @BeforeClass + public static void setUpClass() throws Exception { + stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); + setup(OpenPolicyAgentAccessController.class, false, OPA_URL); + } + + @Before + public void resetStubs() { + WireMock.reset(); + stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); + } + + @AfterClass + public static void tearDownClass() throws Exception { + tearDown(); + } + + // --- helpers --- + + private ObserverContext ctx() { + return ObserverContextImpl.createAndPrepare(CP_ENV); + } + + private OpenPolicyAgentAccessController getOpaController() { + MasterCoprocessorHost masterCpHost = + TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterCoprocessorHost(); + return masterCpHost.findCoprocessor(OpenPolicyAgentAccessController.class); + } + + // --- original tests (non-standard allow/deny patterns) --- @Test public void testCreateAndPut() throws Exception { LOG.info("testCreateAndPut - start"); - stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); - setup(OpenPolicyAgentAccessController.class, false, OPA_URL); - HTableDescriptor htd = getHTableDescriptor(); - createTable(TEST_UTIL, TEST_UTIL.getAdmin(), htd, new byte[][] {Bytes.toBytes("s")}); - // put some test data List puts = new ArrayList<>(100); for (int i = 0; i < 100; i++) { Put p = new Put(Bytes.toBytes(i)); @@ -54,21 +91,13 @@ public void testCreateAndPut() throws Exception { table.put(puts); deleteTable(TEST_UTIL, TEST_TABLE); - - tearDown(); LOG.info("testCreateAndPut - complete"); } @Test public void testDeniedCreate() throws Exception { LOG.info("testDeniedCreate - start"); - - // let all set-up calls succeed - stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); - setup(OpenPolicyAgentAccessController.class, false, OPA_URL); - try { - // re-stub so that any subsequent calls will fail stubFor(post("/").willReturn(ok().withBody("{\"result\": \"false\"}"))); HTableDescriptor htd = getHTableDescriptor(); createTable(TEST_UTIL, TEST_UTIL.getAdmin(), htd, new byte[][] {Bytes.toBytes("s")}); @@ -76,157 +105,574 @@ public void testDeniedCreate() throws Exception { } catch (AccessControlException e) { logOk(e); } - - tearDown(); LOG.info("testDeniedCreate - complete"); } @Test public void testDeniedCreateByUser() throws Exception { - stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); - setup(OpenPolicyAgentAccessController.class, false, OPA_URL); - User userDenied = User.createUserForTesting(conf, "cannotCreateTables", new String[0]); - SecureTestUtil.AccessTestAction createTable = () -> { - HTableDescriptor htd = getHTableDescriptor(); - getOpaController() - .preCreateTable(ObserverContextImpl.createAndPrepare(CP_ENV), htd, null); + getOpaController().preCreateTable(ctx(), getHTableDescriptor(), null); return null; }; - - // re-stub so that the call fails for the given user stubFor( post("/") .withRequestBody( matchingJsonPath("$.input.callerUgi[?(@.userName == 'cannotCreateTables')]")) .willReturn(ok().withBody("{\"result\": \"false\"}"))); - try { userDenied.runAs(createTable); fail("AccessControlException should have been thrown"); } catch (AccessControlException e) { logOk(e); } - - tearDown(); } @Test - public void testDryRun() throws Exception { - stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); - setup(OpenPolicyAgentAccessController.class, false, OPA_URL, true, false); - - User userDenied = User.createUserForTesting(conf, "cannotCreateTables", new String[0]); - - SecureTestUtil.AccessTestAction createTable = + public void testCreateNamespace() throws Exception { + User userCreater = User.createUserForTesting(conf, "nsCreator", new String[0]); + User userDenied = User.createUserForTesting(conf, "nsNonCreator", new String[0]); + SecureTestUtil.AccessTestAction createNamespace = () -> { - HTableDescriptor htd = getHTableDescriptor(); - getOpaController() - .preCreateTable(ObserverContextImpl.createAndPrepare(CP_ENV), htd, null); + NamespaceDescriptor nsd = NamespaceDescriptor.create("new_ns").build(); + getOpaController().preCreateNamespace(ctx(), nsd); return null; }; - - // re-stub so that the call would fail for the given user in *non*-dryRun mode + try { + userCreater.runAs(createNamespace); + } catch (AccessControlException e) { + throw new AssertionError("AccessControlException should not have been thrown", e); + } stubFor( post("/") - .withRequestBody( - matchingJsonPath("$.input.callerUgi[?(@.userName == 'cannotCreateTables')]")) + .withRequestBody(matchingJsonPath("$.input.callerUgi[?(@.userName == 'nsNonCreator')]")) .willReturn(ok().withBody("{\"result\": \"false\"}"))); - try { - userDenied.runAs(createTable); - LOG.info("Action runs as expected due to being in dryRun mode"); + userDenied.runAs(createNamespace); + fail("AccessControlException should have been thrown"); } catch (AccessControlException e) { - throw new AssertionError("AccessControlException should not have been thrown", e); + logOk(e); } + } - tearDown(); + // --- namespace hooks --- + + @Test + public void testPreDeleteNamespace() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preDeleteNamespace(ctx(), "default"); + return null; + }); } @Test - public void testUseCache() throws Exception { - stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); - setup(OpenPolicyAgentAccessController.class, false, OPA_URL, false, true); + public void testPreModifyNamespace() throws Exception { + NamespaceDescriptor nsd = NamespaceDescriptor.create("default").build(); + assertAllowedThenDenied( + () -> { + getOpaController().preModifyNamespace(ctx(), nsd); + return null; + }); + } - User userDenied = User.createUserForTesting(conf, "useCacheUser", new String[0]); + @Test + public void testPreGetNamespaceDescriptor() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preGetNamespaceDescriptor(ctx(), "default"); + return null; + }); + } - // create a table explicitly using the cache from the cp-processor on the master... - SecureTestUtil.AccessTestAction createTable = + // --- table DDL hooks --- + + @Test + public void testPreDeleteTable() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preDeleteTable(ctx(), TEST_TABLE); + return null; + }); + } + + @Test + public void testPreEnableTable() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preEnableTable(ctx(), TEST_TABLE); + return null; + }); + } + + @Test + public void testPreDisableTable() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preDisableTable(ctx(), TEST_TABLE); + return null; + }); + } + + @Test + public void testPreTruncateTable() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preTruncateTable(ctx(), TEST_TABLE); + return null; + }); + } + + @Test + public void testPreModifyTable() throws Exception { + TableDescriptor td = getHTableDescriptor(); + assertAllowedThenDenied( + () -> { + getOpaController().preModifyTable(ctx(), TEST_TABLE, td, td); + return null; + }); + } + + @Test + public void testPreModifyColumnFamilyStoreFileTracker() throws Exception { + assertAllowedThenDenied( () -> { - HTableDescriptor htd = getHTableDescriptor(); getOpaController() - .preCreateTable(ObserverContextImpl.createAndPrepare(CP_ENV), htd, null); + .preModifyColumnFamilyStoreFileTracker(ctx(), TEST_TABLE, TEST_FAMILY, "FILE"); return null; - }; + }); + } - try { - userDenied.runAs(createTable); - } catch (AccessControlException e) { - throw new AssertionError("AccessControlException should not have been thrown", e); - } + // --- flush / quota hooks --- - // we should have only a single entry for this user as subsequent calls will hit the cache - assertEquals(Optional.of(1L), getOpaController().getAclCacheSize()); + @Test + public void testPreTableFlush() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preTableFlush(ctx(), TEST_TABLE); + return null; + }); + } - tearDown(); + @Test + public void testPreSetUserQuotaTableScope() throws Exception { + GlobalQuotaSettings quotas = null; + assertAllowedThenDenied( + () -> { + getOpaController().preSetUserQuota(ctx(), "u", TEST_TABLE, quotas); + return null; + }); } @Test - public void testCreateNamespace() throws Exception { - stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); - setup(OpenPolicyAgentAccessController.class, false, OPA_URL, false, false); + public void testPreSetUserQuotaNamespaceScope() throws Exception { + GlobalQuotaSettings quotas = null; + assertAllowedThenDenied( + () -> { + getOpaController() + .preSetUserQuota(ctx(), "u", NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, quotas); + return null; + }); + } - User userCreater = User.createUserForTesting(conf, "nsCreator", new String[0]); - User userDenied = User.createUserForTesting(conf, "nsNonCreator", new String[0]); + // --- region assignment / snapshot / quota hooks (existing) --- - SecureTestUtil.AccessTestAction createNamespace = + @Test + public void testPreMove() throws Exception { + RegionInfo regionInfo = RegionInfoBuilder.newBuilder(TEST_TABLE).build(); + assertAllowedThenDenied( () -> { - NamespaceDescriptor nsd = NamespaceDescriptor.create("new_ns").build(); - getOpaController().preCreateNamespace(ObserverContextImpl.createAndPrepare(CP_ENV), nsd); + getOpaController().preMove(ctx(), regionInfo, null, null); return null; - }; + }); + } - try { - userCreater.runAs(createNamespace); - } catch (AccessControlException e) { - throw new AssertionError("AccessControlException should not have been thrown", e); - } + @Test + public void testPreAssign() throws Exception { + RegionInfo regionInfo = RegionInfoBuilder.newBuilder(TEST_TABLE).build(); + assertAllowedThenDenied( + () -> { + getOpaController().preAssign(ctx(), regionInfo); + return null; + }); + } - // re-stub so that the call would fail for the given user in *non*-dryRun mode - stubFor( - post("/") - .withRequestBody(matchingJsonPath("$.input.callerUgi[?(@.userName == 'nsNonCreator')]")) - .willReturn(ok().withBody("{\"result\": \"false\"}"))); + @Test + public void testPreUnassign() throws Exception { + RegionInfo regionInfo = RegionInfoBuilder.newBuilder(TEST_TABLE).build(); + assertAllowedThenDenied( + () -> { + getOpaController().preUnassign(ctx(), regionInfo); + return null; + }); + } - try { - userDenied.runAs(createNamespace); - fail("AccessControlException should have been thrown"); - } catch (AccessControlException e) { - logOk(e); - } + @Test + public void testPreRegionOffline() throws Exception { + RegionInfo regionInfo = RegionInfoBuilder.newBuilder(TEST_TABLE).build(); + assertAllowedThenDenied( + () -> { + getOpaController().preRegionOffline(ctx(), regionInfo); + return null; + }); + } - tearDown(); + @Test + public void testPreSplitRegion() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preSplitRegion(ctx(), TEST_TABLE, null); + return null; + }); + } + + @Test + public void testPreMergeRegions() throws Exception { + RegionInfo ri = RegionInfoBuilder.newBuilder(TEST_TABLE).build(); + assertAllowedThenDenied( + () -> { + getOpaController().preMergeRegions(ctx(), new RegionInfo[] {ri, ri}); + return null; + }); + } + + @Test + public void testPreModifyTableStoreFileTracker() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preModifyTableStoreFileTracker(ctx(), TEST_TABLE, "FILE"); + return null; + }); } - private static void logOk(AccessControlException e) { - LOG.info("AccessControlException as expected: [{}]", e.getMessage()); + @Test + public void testPreSnapshot() throws Exception { + SnapshotDescription snap = new SnapshotDescription("snap", TEST_TABLE); + TableDescriptor td = getHTableDescriptor(); + assertAllowedThenDenied( + () -> { + getOpaController().preSnapshot(ctx(), snap, td); + return null; + }); } - private static HTableDescriptor getHTableDescriptor() { - HTableDescriptor htd = new HTableDescriptor(TEST_TABLE); - HColumnDescriptor hcd = new HColumnDescriptor(TEST_FAMILY); - hcd.setMaxVersions(100); - htd.addFamily(hcd); - htd.setOwner(USER_OWNER); + @Test + public void testPreListSnapshot() throws Exception { + SnapshotDescription snap = new SnapshotDescription("snap", TEST_TABLE); + assertAllowedThenDenied( + () -> { + getOpaController().preListSnapshot(ctx(), snap); + return null; + }); + } + + @Test + public void testPreCloneSnapshot() throws Exception { + SnapshotDescription snap = new SnapshotDescription("snap", TEST_TABLE); + TableDescriptor td = getHTableDescriptor(); + assertAllowedThenDenied( + () -> { + getOpaController().preCloneSnapshot(ctx(), snap, td); + return null; + }); + } - return htd; + @Test + public void testPreRestoreSnapshot() throws Exception { + SnapshotDescription snap = new SnapshotDescription("snap", TEST_TABLE); + TableDescriptor td = getHTableDescriptor(); + assertAllowedThenDenied( + () -> { + getOpaController().preRestoreSnapshot(ctx(), snap, td); + return null; + }); } - private OpenPolicyAgentAccessController getOpaController() { - MasterCoprocessorHost masterCpHost = - TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterCoprocessorHost(); - return masterCpHost.findCoprocessor(OpenPolicyAgentAccessController.class); + @Test + public void testPreDeleteSnapshot() throws Exception { + SnapshotDescription snap = new SnapshotDescription("snap", TEST_TABLE); + assertAllowedThenDenied( + () -> { + getOpaController().preDeleteSnapshot(ctx(), snap); + return null; + }); + } + + @Test + public void testPreSetTableQuota() throws Exception { + GlobalQuotaSettings quotas = null; + assertAllowedThenDenied( + () -> { + getOpaController().preSetTableQuota(ctx(), TEST_TABLE, quotas); + return null; + }); + } + + @Test + public void testPreSetNamespaceQuota() throws Exception { + GlobalQuotaSettings quotas = null; + assertAllowedThenDenied( + () -> { + getOpaController() + .preSetNamespaceQuota(ctx(), NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, quotas); + return null; + }); + } + + @Test + public void testPreGetUserPermissionsTableScope() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preGetUserPermissions(ctx(), "u", null, TEST_TABLE, null, null); + return null; + }); + } + + @Test + public void testPreGetUserPermissionsNamespaceScope() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController() + .preGetUserPermissions( + ctx(), "u", NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, null, null, null); + return null; + }); + } + + @Test + public void testPreBalance() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preBalance(ctx(), BalanceRequest.defaultInstance()); + return null; + }); + } + + @Test + public void testPreBalanceSwitch() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preBalanceSwitch(ctx(), true); + return null; + }); + } + + @Test + public void testPreShutdown() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preShutdown(ctx()); + return null; + }); + } + + @Test + public void testPreStopMaster() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preStopMaster(ctx()); + return null; + }); + } + + @Test + public void testPreClearDeadServers() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preClearDeadServers(ctx()); + return null; + }); + } + + @Test + public void testPreDecommissionRegionServers() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preDecommissionRegionServers(ctx(), List.of(), false); + return null; + }); + } + + @Test + public void testPreListDecommissionedRegionServers() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preListDecommissionedRegionServers(ctx()); + return null; + }); + } + + @Test + public void testPreRecommissionRegionServer() throws Exception { + ServerName serverName = ServerName.valueOf("localhost", 16010, 12345L); + assertAllowedThenDenied( + () -> { + getOpaController().preRecommissionRegionServer(ctx(), serverName, List.of()); + return null; + }); + } + + // --- procedure / lock hooks --- + + @Test + public void testPreAbortProcedure() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preAbortProcedure(ctx(), 1L); + return null; + }); + } + + @Test + public void testPreGetProcedures() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preGetProcedures(ctx()); + return null; + }); + } + + @Test + public void testPreGetLocks() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preGetLocks(ctx()); + return null; + }); + } + + @Test + public void testPreSetSplitOrMergeEnabled() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preSetSplitOrMergeEnabled(ctx(), true, MasterSwitchType.SPLIT); + return null; + }); + } + + // --- quota hooks (global scope) --- + + @Test + public void testPreSetUserQuotaGlobalScope() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preSetUserQuota(ctx(), "u", (GlobalQuotaSettings) null); + return null; + }); + } + + @Test + public void testPreSetRegionServerQuota() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preSetRegionServerQuota(ctx(), "rs1", null); + return null; + }); + } + + // --- replication peer hooks --- + + @Test + public void testPreAddReplicationPeer() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preAddReplicationPeer(ctx(), "peer1", null); + return null; + }); + } + + @Test + public void testPreRemoveReplicationPeer() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preRemoveReplicationPeer(ctx(), "peer1"); + return null; + }); + } + + @Test + public void testPreEnableReplicationPeer() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preEnableReplicationPeer(ctx(), "peer1"); + return null; + }); + } + + @Test + public void testPreDisableReplicationPeer() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preDisableReplicationPeer(ctx(), "peer1"); + return null; + }); + } + + @Test + public void testPreGetReplicationPeerConfig() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preGetReplicationPeerConfig(ctx(), "peer1"); + return null; + }); + } + + @Test + public void testPreUpdateReplicationPeerConfig() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preUpdateReplicationPeerConfig(ctx(), "peer1", null); + return null; + }); + } + + @Test + public void testPreListReplicationPeers() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preListReplicationPeers(ctx(), ".*"); + return null; + }); + } + + // --- throttle hooks --- + + @Test + public void testPreSwitchRpcThrottle() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preSwitchRpcThrottle(ctx(), true); + return null; + }); + } + + @Test + public void testPreIsRpcThrottleEnabled() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preIsRpcThrottleEnabled(ctx()); + return null; + }); + } + + @Test + public void testPreSwitchExceedThrottleQuota() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preSwitchExceedThrottleQuota(ctx(), true); + return null; + }); + } + + // --- configuration hooks --- + + @Test + public void testPreUpdateMasterConfiguration() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preUpdateMasterConfiguration(ctx(), conf); + return null; + }); } } diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java new file mode 100644 index 0000000..cccadbe --- /dev/null +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java @@ -0,0 +1,408 @@ +package tech.stackable.hbase; + +import static com.github.tomakehurst.wiremock.client.WireMock.ok; +import static com.github.tomakehurst.wiremock.client.WireMock.post; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static org.apache.hadoop.hbase.security.access.SecureTestUtil.createTable; +import static org.apache.hadoop.hbase.security.access.SecureTestUtil.deleteTable; + +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import org.apache.hadoop.hbase.CompareOperator; +import org.apache.hadoop.hbase.Coprocessor; +import org.apache.hadoop.hbase.client.Append; +import org.apache.hadoop.hbase.client.CheckAndMutate; +import org.apache.hadoop.hbase.client.CheckAndMutateResult; +import org.apache.hadoop.hbase.client.Delete; +import org.apache.hadoop.hbase.client.Durability; +import org.apache.hadoop.hbase.client.Get; +import org.apache.hadoop.hbase.client.Increment; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.RowMutations; +import org.apache.hadoop.hbase.client.Scan; +import org.apache.hadoop.hbase.coprocessor.ObserverContext; +import org.apache.hadoop.hbase.coprocessor.ObserverContextImpl; +import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment; +import org.apache.hadoop.hbase.coprocessor.RegionServerCoprocessorEnvironment; +import org.apache.hadoop.hbase.filter.Filter; +import org.apache.hadoop.hbase.regionserver.HRegion; +import org.apache.hadoop.hbase.regionserver.HRegionServer; +import org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost; +import org.apache.hadoop.hbase.regionserver.RegionServerCoprocessorHost; +import org.apache.hadoop.hbase.regionserver.ScanType; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; + +public class TestOpenPolicyAgentAccessControllerRegion extends TestUtils { + public static final String OPA_URL = "http://localhost:8089"; + + private static final byte[] TEST_ROW = Bytes.toBytes("testRow"); + private static RegionCoprocessorEnvironment REGION_CP_ENV; + private static RegionServerCoprocessorEnvironment RS_CP_ENV; + + @ClassRule public static WireMockRule wireMockRule = new WireMockRule(8089); + + @BeforeClass + public static void setUpClass() throws Exception { + stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); + setup(OpenPolicyAgentAccessController.class, false, OPA_URL); + + createTable( + TEST_UTIL, TEST_UTIL.getAdmin(), getHTableDescriptor(), new byte[][] {Bytes.toBytes("s")}); + + HRegion region = TEST_UTIL.getHBaseCluster().getRegions(TEST_TABLE).get(0); + RegionCoprocessorHost rcpHost = region.getCoprocessorHost(); + OpenPolicyAgentAccessController regionController = + rcpHost.findCoprocessor(OpenPolicyAgentAccessController.class); + REGION_CP_ENV = + (RegionCoprocessorEnvironment) + rcpHost.createEnvironment(regionController, Coprocessor.PRIORITY_HIGHEST, 1, conf); + + HRegionServer rs = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0); + RegionServerCoprocessorHost rsCpHost = rs.getRegionServerCoprocessorHost(); + OpenPolicyAgentAccessController rsController = + rsCpHost.findCoprocessor(OpenPolicyAgentAccessController.class); + RS_CP_ENV = + (RegionServerCoprocessorEnvironment) + rsCpHost.createEnvironment(rsController, Coprocessor.PRIORITY_HIGHEST, 1, conf); + } + + @Before + public void resetStubs() { + WireMock.reset(); + stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); + } + + @AfterClass + public static void tearDownClass() throws Exception { + deleteTable(TEST_UTIL, TEST_TABLE); + tearDown(); + } + + // --- helpers --- + + private ObserverContext regionCtx() { + return ObserverContextImpl.createAndPrepare(REGION_CP_ENV); + } + + private ObserverContext rsCtx() { + return ObserverContextImpl.createAndPrepare(RS_CP_ENV); + } + + private OpenPolicyAgentAccessController getRegionController() { + HRegion region = TEST_UTIL.getHBaseCluster().getRegions(TEST_TABLE).get(0); + return region.getCoprocessorHost().findCoprocessor(OpenPolicyAgentAccessController.class); + } + + // --- read hooks --- + + @Test + public void testPreGetOp() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preGetOp(regionCtx(), new Get(TEST_ROW), null); + return null; + }); + } + + @Test + public void testPreExists() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preExists(regionCtx(), new Get(TEST_ROW), false); + return null; + }); + } + + @Test + public void testPreScannerOpen() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preScannerOpen(regionCtx(), new Scan()); + return null; + }); + } + + // --- write hooks --- + + @Test + public void testPrePut() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController() + .prePut(regionCtx(), new Put(TEST_ROW), null, Durability.USE_DEFAULT); + return null; + }); + } + + @Test + public void testPreDelete() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController() + .preDelete(regionCtx(), new Delete(TEST_ROW), null, Durability.USE_DEFAULT); + return null; + }); + } + + @Test + public void testPreBatchMutate() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preBatchMutate(regionCtx(), null); + return null; + }); + } + + @Test + public void testPreFlush() throws Exception { + // preFlush is an internal storage engine hook; no authorization check is applied. + getRegionController().preFlush(regionCtx(), null); + } + + @Test + public void testPreCompact() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preCompact(regionCtx(), null, null, ScanType.USER_SCAN, null, null); + return null; + }); + } + + // --- read+write hooks (require WRITE or READ) --- + + @Test + public void testPreAppend() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preAppend(regionCtx(), new Append(TEST_ROW)); + return null; + }); + } + + @Test + public void testPreIncrement() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preIncrement(regionCtx(), new Increment(TEST_ROW)); + return null; + }); + } + + @Test + public void testPreCheckAndPut() throws Exception { + Put put = new Put(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndPut( + regionCtx(), + TEST_ROW, + TEST_FAMILY, + TEST_QUALIFIER, + CompareOperator.EQUAL, + null, + put, + false); + return null; + }); + } + + @Test + public void testPreCheckAndPutAfterRowLock() throws Exception { + Put put = new Put(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndPutAfterRowLock( + regionCtx(), + TEST_ROW, + TEST_FAMILY, + TEST_QUALIFIER, + CompareOperator.EQUAL, + null, + put, + false); + return null; + }); + } + + @Test + public void testPreCheckAndDelete() throws Exception { + Delete delete = new Delete(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndDelete( + regionCtx(), + TEST_ROW, + TEST_FAMILY, + TEST_QUALIFIER, + CompareOperator.EQUAL, + null, + delete, + false); + return null; + }); + } + + @Test + public void testPreCheckAndDeleteAfterRowLock() throws Exception { + Delete delete = new Delete(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndDeleteAfterRowLock( + regionCtx(), + TEST_ROW, + TEST_FAMILY, + TEST_QUALIFIER, + CompareOperator.EQUAL, + null, + delete, + false); + return null; + }); + } + + @Test + public void testPreCheckAndPutWithFilter() throws Exception { + Put put = new Put(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController().preCheckAndPut(regionCtx(), TEST_ROW, (Filter) null, put, false); + return null; + }); + } + + @Test + public void testPreCheckAndPutAfterRowLockWithFilter() throws Exception { + Put put = new Put(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndPutAfterRowLock(regionCtx(), TEST_ROW, (Filter) null, put, false); + return null; + }); + } + + @Test + public void testPreCheckAndDeleteWithFilter() throws Exception { + Delete delete = new Delete(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndDelete(regionCtx(), TEST_ROW, (Filter) null, delete, false); + return null; + }); + } + + @Test + public void testPreCheckAndDeleteAfterRowLockWithFilter() throws Exception { + Delete delete = new Delete(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndDeleteAfterRowLock(regionCtx(), TEST_ROW, (Filter) null, delete, false); + return null; + }); + } + + @Test + public void testPreCheckAndMutateWithRowMutations() throws Exception { + RowMutations rowMutations = new RowMutations(TEST_ROW); + rowMutations.add(new Put(TEST_ROW)); + CheckAndMutate checkAndMutate = + CheckAndMutate.newBuilder(TEST_ROW) + .ifNotExists(TEST_FAMILY, TEST_QUALIFIER) + .build(rowMutations); + CheckAndMutateResult result = new CheckAndMutateResult(true, null); + assertAllowedThenDenied( + () -> { + getRegionController().preCheckAndMutate(regionCtx(), checkAndMutate, result); + return null; + }); + } + + @Test + public void testPreCheckAndMutateAfterRowLockWithRowMutations() throws Exception { + RowMutations rowMutations = new RowMutations(TEST_ROW); + rowMutations.add(new Put(TEST_ROW)); + CheckAndMutate checkAndMutate = + CheckAndMutate.newBuilder(TEST_ROW) + .ifNotExists(TEST_FAMILY, TEST_QUALIFIER) + .build(rowMutations); + CheckAndMutateResult result = new CheckAndMutateResult(true, null); + assertAllowedThenDenied( + () -> { + getRegionController().preCheckAndMutateAfterRowLock(regionCtx(), checkAndMutate, result); + return null; + }); + } + + // --- RegionServer hooks --- + + private OpenPolicyAgentAccessController getRsController() { + HRegionServer rs = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0); + return rs.getRegionServerCoprocessorHost() + .findCoprocessor(OpenPolicyAgentAccessController.class); + } + + @Test + public void testPreRollWALWriterRequest() throws Exception { + assertAllowedThenDenied( + () -> { + getRsController().preRollWALWriterRequest(rsCtx()); + return null; + }); + } + + @Test + public void testPreReplicateLogEntries() throws Exception { + assertAllowedThenDenied( + () -> { + getRsController().preReplicateLogEntries(rsCtx()); + return null; + }); + } + + @Test + public void testPreClearCompactionQueues() throws Exception { + assertAllowedThenDenied( + () -> { + getRsController().preClearCompactionQueues(rsCtx()); + return null; + }); + } + + @Test + public void testPreClearRegionBlockCache() throws Exception { + assertAllowedThenDenied( + () -> { + getRsController().preClearRegionBlockCache(rsCtx()); + return null; + }); + } + + @Test + public void testPreUpdateRegionServerConfiguration() throws Exception { + assertAllowedThenDenied( + () -> { + getRsController().preUpdateRegionServerConfiguration(rsCtx(), conf); + return null; + }); + } + + @Test + public void testPreStopRegionServer() throws Exception { + assertAllowedThenDenied( + () -> { + getRsController().preStopRegionServer(rsCtx()); + return null; + }); + } +} diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java new file mode 100644 index 0000000..6d19bde --- /dev/null +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java @@ -0,0 +1,91 @@ +package tech.stackable.hbase; + +import static com.github.tomakehurst.wiremock.client.WireMock.matchingJsonPath; +import static com.github.tomakehurst.wiremock.client.WireMock.ok; +import static com.github.tomakehurst.wiremock.client.WireMock.post; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static org.junit.Assert.assertEquals; + +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import java.util.Optional; +import org.apache.hadoop.hbase.HTableDescriptor; +import org.apache.hadoop.hbase.coprocessor.ObserverContextImpl; +import org.apache.hadoop.hbase.master.MasterCoprocessorHost; +import org.apache.hadoop.hbase.security.User; +import org.apache.hadoop.hbase.security.access.SecureTestUtil; +import org.apache.hadoop.security.AccessControlException; +import org.junit.Rule; +import org.junit.Test; + +/** + * Tests for non-default coprocessor configurations (dryRun, cache). Each test manages its own + * mini-cluster lifecycle since the coprocessor config differs per test. + */ +public class TestOpenPolicyAgentAccessControllerVariants extends TestUtils { + public static final String OPA_URL = "http://localhost:8089"; + + @Rule public WireMockRule wireMockRule = new WireMockRule(8089); + + @Test + public void testDryRun() throws Exception { + stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); + setup(OpenPolicyAgentAccessController.class, false, OPA_URL, true, false); + + User userDenied = User.createUserForTesting(conf, "cannotCreateTables", new String[0]); + + SecureTestUtil.AccessTestAction createTable = + () -> { + HTableDescriptor htd = getHTableDescriptor(); + getOpaController() + .preCreateTable(ObserverContextImpl.createAndPrepare(CP_ENV), htd, null); + return null; + }; + + stubFor( + post("/") + .withRequestBody( + matchingJsonPath("$.input.callerUgi[?(@.userName == 'cannotCreateTables')]")) + .willReturn(ok().withBody("{\"result\": \"false\"}"))); + + try { + userDenied.runAs(createTable); + LOG.info("Action runs as expected due to being in dryRun mode"); + } catch (AccessControlException e) { + throw new AssertionError("AccessControlException should not have been thrown", e); + } + + tearDown(); + } + + @Test + public void testUseCache() throws Exception { + stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); + setup(OpenPolicyAgentAccessController.class, false, OPA_URL, false, true); + + User userDenied = User.createUserForTesting(conf, "useCacheUser", new String[0]); + + SecureTestUtil.AccessTestAction createTable = + () -> { + HTableDescriptor htd = getHTableDescriptor(); + getOpaController() + .preCreateTable(ObserverContextImpl.createAndPrepare(CP_ENV), htd, null); + return null; + }; + + try { + userDenied.runAs(createTable); + } catch (AccessControlException e) { + throw new AssertionError("AccessControlException should not have been thrown", e); + } + + assertEquals(Optional.of(1L), getOpaController().getAclCacheSize()); + + tearDown(); + } + + private OpenPolicyAgentAccessController getOpaController() { + MasterCoprocessorHost masterCpHost = + TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterCoprocessorHost(); + return masterCpHost.findCoprocessor(OpenPolicyAgentAccessController.class); + } +} diff --git a/src/test/java/tech/stackable/hbase/TestUtils.java b/src/test/java/tech/stackable/hbase/TestUtils.java index 54e476d..2cf25a1 100644 --- a/src/test/java/tech/stackable/hbase/TestUtils.java +++ b/src/test/java/tech/stackable/hbase/TestUtils.java @@ -1,5 +1,9 @@ package tech.stackable.hbase; +import static com.github.tomakehurst.wiremock.client.WireMock.matchingJsonPath; +import static com.github.tomakehurst.wiremock.client.WireMock.ok; +import static com.github.tomakehurst.wiremock.client.WireMock.post; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static org.apache.hadoop.hbase.AuthUtil.toGroupEntry; import static org.apache.hadoop.hbase.security.access.SecureTestUtil.*; import static org.junit.Assert.*; @@ -26,6 +30,7 @@ import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.*; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.UserGroupInformation; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -59,6 +64,9 @@ public class TestUtils { protected static final String GROUP_READ = "group_read"; protected static final String GROUP_WRITE = "group_write"; + protected static User ALLOWED_USER; + protected static User DENIED_USER; + protected static User USER_GROUP_ADMIN; protected static User USER_GROUP_CREATE; protected static User USER_GROUP_READ; @@ -158,6 +166,9 @@ protected static void setup( User.createUserForTesting(conf, "user_group_write", new String[] {GROUP_WRITE}); systemUserConnection = TEST_UTIL.getConnection(); + + ALLOWED_USER = User.createUserForTesting(conf, "allowedUser", new String[0]); + DENIED_USER = User.createUserForTesting(conf, "deniedUser", new String[0]); } protected static void setUpTables() throws Exception { @@ -232,6 +243,24 @@ protected static void setUpTables() throws Exception { assertEquals(5, size); } + protected static void logOk(AccessControlException e) { + LOG.info("AccessControlException as expected: [{}]", e.getMessage()); + } + + protected void assertAllowedThenDenied(SecureTestUtil.AccessTestAction action) throws Exception { + ALLOWED_USER.runAs(action); + stubFor( + post("/") + .withRequestBody(matchingJsonPath("$.input.callerUgi[?(@.userName == 'deniedUser')]")) + .willReturn(ok().withBody("{\"result\": \"false\"}"))); + try { + DENIED_USER.runAs(action); + fail("AccessControlException should have been thrown"); + } catch (AccessControlException e) { + logOk(e); + } + } + protected static void tearDown() throws Exception { TEST_UTIL.shutdownMiniCluster(); } @@ -537,4 +566,13 @@ protected void createTestTable(TableName tname, byte[] cf) throws Exception { htd.setOwner(USER_OWNER); createTable(TEST_UTIL, TEST_UTIL.getAdmin(), htd, new byte[][] {Bytes.toBytes("s")}); } + + protected static HTableDescriptor getHTableDescriptor() { + HTableDescriptor htd = new HTableDescriptor(TEST_TABLE); + HColumnDescriptor hcd = new HColumnDescriptor(TEST_FAMILY); + hcd.setMaxVersions(100); + htd.addFamily(hcd); + htd.setOwner(USER_OWNER); + return htd; + } }