This is an automated email from the ASF dual-hosted git repository.
brfrn169 pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2 by this push:
new 4815740 HBASE-25160 Refactor AccessController and
VisibilityController (#2506)
4815740 is described below
commit 481574072c1f794dad0c4455aa98a3f3c5725481
Author: Toshihiro Suzuki <brfrn...@gmail.com>
AuthorDate: Thu Oct 8 17:04:48 2020 +0900
HBASE-25160 Refactor AccessController and VisibilityController (#2506)
Signed-off-by: stack <st...@apache.org>
---
.../hbase/security/access/AccessController.java | 66 ++++------------------
.../security/visibility/VisibilityController.java | 66 +---------------------
2 files changed, 13 insertions(+), 119 deletions(-)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
index 6cac67a..1f53e27 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
@@ -426,7 +426,6 @@ public class AccessController implements MasterCoprocessor,
RegionCoprocessor,
DELETE("delete"),
CHECK_AND_PUT("checkAndPut"),
CHECK_AND_DELETE("checkAndDelete"),
- INCREMENT_COLUMN_VALUE("incrementColumnValue"),
APPEND("append"),
INCREMENT("increment");
@@ -1503,18 +1502,27 @@ public class AccessController implements
MasterCoprocessor, RegionCoprocessor,
// We have a failure with table, cf and q perm checks and now giving
a chance for cell
// perm check
OpType opType;
+ long timestamp;
if (m instanceof Put) {
checkForReservedTagPresence(user, m);
opType = OpType.PUT;
+ timestamp = m.getTimestamp();
} else if (m instanceof Delete) {
opType = OpType.DELETE;
+ timestamp = m.getTimestamp();
+ } else if (m instanceof Increment) {
+ opType = OpType.INCREMENT;
+ timestamp = ((Increment) m).getTimeRange().getMax();
+ } else if (m instanceof Append) {
+ opType = OpType.APPEND;
+ timestamp = ((Append) m).getTimeRange().getMax();
} else {
- // If the operation type is not Put or Delete, do nothing
+ // If the operation type is not Put/Delete/Increment/Append, do
nothing
continue;
}
AuthResult authResult = null;
if (checkCoveringPermission(user, opType, c.getEnvironment(),
m.getRow(),
- m.getFamilyCellMap(), m.getTimestamp(), Action.WRITE)) {
+ m.getFamilyCellMap(), timestamp, Action.WRITE)) {
authResult = AuthResult.allow(opType.toString(), "Covering cell
set",
user, Action.WRITE, table, m.getFamilyCellMap());
} else {
@@ -1696,32 +1704,6 @@ public class AccessController implements
MasterCoprocessor, RegionCoprocessor,
}
@Override
- public Result preAppendAfterRowLock(final
ObserverContext<RegionCoprocessorEnvironment> c,
- final Append append) throws IOException {
- if (append.getAttribute(CHECK_COVERING_PERM) != null) {
- // We had failure with table, cf and q perm checks and now giving a
chance for cell
- // perm check
- TableName table =
c.getEnvironment().getRegion().getRegionInfo().getTable();
- AuthResult authResult = null;
- User user = getActiveUser(c);
- if (checkCoveringPermission(user, OpType.APPEND, c.getEnvironment(),
append.getRow(),
- append.getFamilyCellMap(), append.getTimeRange().getMax(),
Action.WRITE)) {
- authResult = AuthResult.allow(OpType.APPEND.toString(),
- "Covering cell set", user, Action.WRITE, table,
append.getFamilyCellMap());
- } else {
- authResult = AuthResult.deny(OpType.APPEND.toString(),
- "Covering cell set", user, Action.WRITE, table,
append.getFamilyCellMap());
- }
- AccessChecker.logResult(authResult);
- if (authorizationEnabled && !authResult.isAllowed()) {
- throw new AccessDeniedException("Insufficient permissions " +
- authResult.toContextString());
- }
- }
- return null;
- }
-
- @Override
public Result preIncrement(final
ObserverContext<RegionCoprocessorEnvironment> c,
final Increment increment)
throws IOException {
@@ -1757,32 +1739,6 @@ public class AccessController implements
MasterCoprocessor, RegionCoprocessor,
}
@Override
- public Result preIncrementAfterRowLock(final
ObserverContext<RegionCoprocessorEnvironment> c,
- final Increment increment) throws IOException {
- if (increment.getAttribute(CHECK_COVERING_PERM) != null) {
- // We had failure with table, cf and q perm checks and now giving a
chance for cell
- // perm check
- TableName table =
c.getEnvironment().getRegion().getRegionInfo().getTable();
- AuthResult authResult = null;
- User user = getActiveUser(c);
- if (checkCoveringPermission(user, OpType.INCREMENT, c.getEnvironment(),
increment.getRow(),
- increment.getFamilyCellMap(), increment.getTimeRange().getMax(),
Action.WRITE)) {
- authResult = AuthResult.allow(OpType.INCREMENT.toString(), "Covering
cell set",
- user, Action.WRITE, table, increment.getFamilyCellMap());
- } else {
- authResult = AuthResult.deny(OpType.INCREMENT.toString(), "Covering
cell set",
- user, Action.WRITE, table, increment.getFamilyCellMap());
- }
- AccessChecker.logResult(authResult);
- if (authorizationEnabled && !authResult.isAllowed()) {
- throw new AccessDeniedException("Insufficient permissions " +
- authResult.toContextString());
- }
- }
- return null;
- }
-
- @Override
public List<Pair<Cell, Cell>> postIncrementBeforeWAL(
ObserverContext<RegionCoprocessorEnvironment> ctx, Mutation mutation,
List<Pair<Cell, Cell>> cellPairs) throws IOException {
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
index 01f3634..305c6d1 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
@@ -49,11 +49,9 @@ import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.Tag;
import org.apache.hadoop.hbase.TagType;
import org.apache.hadoop.hbase.client.Admin;
-import org.apache.hadoop.hbase.client.Append;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
import org.apache.hadoop.hbase.client.Delete;
import org.apache.hadoop.hbase.client.Get;
-import org.apache.hadoop.hbase.client.Increment;
import org.apache.hadoop.hbase.client.MasterSwitchType;
import org.apache.hadoop.hbase.client.Mutation;
import org.apache.hadoop.hbase.client.Put;
@@ -73,7 +71,6 @@ import org.apache.hadoop.hbase.coprocessor.RegionCoprocessor;
import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment;
import org.apache.hadoop.hbase.coprocessor.RegionObserver;
import org.apache.hadoop.hbase.exceptions.DeserializationException;
-import org.apache.hadoop.hbase.exceptions.FailedSanityCheckException;
import org.apache.hadoop.hbase.filter.Filter;
import org.apache.hadoop.hbase.filter.FilterBase;
import org.apache.hadoop.hbase.filter.FilterList;
@@ -322,7 +319,7 @@ public class VisibilityController implements
MasterCoprocessor, RegionCoprocesso
}
}
}
- if (!sanityFailure) {
+ if (!sanityFailure && (m instanceof Put || m instanceof Delete)) {
if (cellVisibility != null) {
String labelsExp = cellVisibility.getExpression();
List<Tag> visibilityTags = labelCache.get(labelsExp);
@@ -359,7 +356,7 @@ public class VisibilityController implements
MasterCoprocessor, RegionCoprocesso
if (m instanceof Put) {
Put p = (Put) m;
p.add(cell);
- } else if (m instanceof Delete) {
+ } else {
Delete d = (Delete) m;
d.add(cell);
}
@@ -469,35 +466,6 @@ public class VisibilityController implements
MasterCoprocessor, RegionCoprocesso
return pair;
}
- /**
- * Checks whether cell contains any tag with type as VISIBILITY_TAG_TYPE.
This
- * tag type is reserved and should not be explicitly set by user. There are
- * two versions of this method one that accepts pair and other without pair.
- * In case of preAppend and preIncrement the additional operations are not
- * needed like checking for STRING_VIS_TAG_TYPE and hence the API without
pair
- * could be used.
- *
- * @param cell
- * @throws IOException
- */
- private boolean checkForReservedVisibilityTagPresence(Cell cell) throws
IOException {
- // Bypass this check when the operation is done by a system/super user.
- // This is done because, while Replication, the Cells coming to the peer
- // cluster with reserved
- // typed tags and this is fine and should get added to the peer cluster
- // table
- if (isSystemOrSuperUser()) {
- return true;
- }
- Iterator<Tag> tagsItr = PrivateCellUtil.tagsIterator(cell);
- while (tagsItr.hasNext()) {
- if (RESERVED_VIS_TAG_TYPES.contains(tagsItr.next().getType())) {
- return false;
- }
- }
- return true;
- }
-
private void removeReplicationVisibilityTag(List<Tag> tags) throws
IOException {
Iterator<Tag> iterator = tags.iterator();
while (iterator.hasNext()) {
@@ -657,36 +625,6 @@ public class VisibilityController implements
MasterCoprocessor, RegionCoprocesso
}
@Override
- public Result preAppend(ObserverContext<RegionCoprocessorEnvironment> e,
Append append)
- throws IOException {
- // If authorization is not enabled, we don't care about reserved tags
- if (!authorizationEnabled) {
- return null;
- }
- for (CellScanner cellScanner = append.cellScanner();
cellScanner.advance();) {
- if (!checkForReservedVisibilityTagPresence(cellScanner.current())) {
- throw new FailedSanityCheckException("Append contains cell with
reserved type tag");
- }
- }
- return null;
- }
-
- @Override
- public Result preIncrement(ObserverContext<RegionCoprocessorEnvironment> e,
Increment increment)
- throws IOException {
- // If authorization is not enabled, we don't care about reserved tags
- if (!authorizationEnabled) {
- return null;
- }
- for (CellScanner cellScanner = increment.cellScanner();
cellScanner.advance();) {
- if (!checkForReservedVisibilityTagPresence(cellScanner.current())) {
- throw new FailedSanityCheckException("Increment contains cell with
reserved type tag");
- }
- }
- return null;
- }
-
- @Override
public List<Pair<Cell, Cell>> postIncrementBeforeWAL(
ObserverContext<RegionCoprocessorEnvironment> ctx, Mutation mutation,
List<Pair<Cell, Cell>> cellPairs) throws IOException {
