This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new 3af3fde623 Fix batch scan audit logging (#5483)
3af3fde623 is described below
commit 3af3fde6232bd11bd1cb05731d667723aaa2984a
Author: Christopher Tubbs <[email protected]>
AuthorDate: Thu Apr 17 22:28:54 2025 -0400
Fix batch scan audit logging (#5483)
* Revert the new log message added to the ZKAuthorizor in
3825d582e7ddecb2d3661f96a707c13120bec609 for #5480 because the
information is already available to the users by another means, and
it's not worth the risk of bad user requests spamming the server logs
* Remove a similar extraneous exception and try-catch block for
getActiveScans method that was removed in #5480
* Improve exception handling of TabletClientHandler.checkPermission by
separating out the nested try-throw-catch of ThriftSecurityException
into separate steps, so that the correct messages can be logged when
the exception occurs due to permission being denied, rather than due
to an unauthenticatable user (previously, the log message would always
say it was from an unauthenticatable user, which is not true);
this also removes the redundant logging at the trace level when the
message was already logged differently
* Use actual AuditedSecurityOperation type instead of base type, to make
it easier to identify which methods on the base class actually need to
be exposed, and limit the methods visibilities to protected or
private, wherever possible
* Remove redundant constant for scan auditing template (the same
template is used for single range and batch scan; the only difference
is how the range is converted to a String) and consolidate
implementation using a supplier to convert either a single or multiple
ranges to a String for the audit message
* Fix parameters to AuditedSecurityOperation.canScan to match paramaters
needed by ThriftScanClientHandler.startMultiScan, and use that method
instead of the base class method which checks permissions without an
auditing message, which fixes the lack of auditing on batch scan
* Remove unused methods from SecurityOperation base class
* Fix typo in SecurityOperation.validateStoredUserCredentials
* Remove redundant security parameter in TservConstraintEnv constructor,
and update its unit test, fixing EasyMock usage by adding verification
---
.../server/client/ClientServiceHandler.java | 4 +-
.../server/security/AuditedSecurityOperation.java | 68 ++++++---------
.../server/security/SecurityOperation.java | 99 +++++++++-------------
.../server/security/handler/ZKAuthorizor.java | 3 -
.../coordinator/CompactionCoordinator.java | 4 +-
.../java/org/apache/accumulo/manager/Manager.java | 6 +-
.../replication/ManagerReplicationCoordinator.java | 4 +-
.../manager/tableOps/create/SetupPermissions.java | 3 +-
.../create/SetupNamespacePermissions.java | 3 +-
.../tableImport/ImportSetupPermissions.java | 3 +-
.../accumulo/tserver/TabletClientHandler.java | 80 ++++++++---------
.../org/apache/accumulo/tserver/TabletServer.java | 7 --
.../accumulo/tserver/ThriftScanClientHandler.java | 25 +++---
.../accumulo/tserver/TservConstraintEnv.java | 12 ++-
.../replication/ReplicationServicerHandler.java | 4 +-
.../accumulo/tserver/TservConstraintEnvTest.java | 17 ++--
16 files changed, 150 insertions(+), 192 deletions(-)
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java
b/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java
index 466bc53257..2edf8a55c2 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java
@@ -65,7 +65,7 @@ import org.apache.accumulo.server.ServerContext;
import org.apache.accumulo.server.conf.store.NamespacePropKey;
import org.apache.accumulo.server.conf.store.SystemPropKey;
import org.apache.accumulo.server.conf.store.TablePropKey;
-import org.apache.accumulo.server.security.SecurityOperation;
+import org.apache.accumulo.server.security.AuditedSecurityOperation;
import org.apache.accumulo.server.util.ServerBulkImportStatus;
import org.apache.accumulo.server.util.TableDiskUsage;
import org.apache.accumulo.server.zookeeper.TransactionWatcher;
@@ -77,7 +77,7 @@ public class ClientServiceHandler implements
ClientService.Iface {
private static final Logger log =
LoggerFactory.getLogger(ClientServiceHandler.class);
protected final TransactionWatcher transactionWatcher;
protected final ServerContext context;
- protected final SecurityOperation security;
+ protected final AuditedSecurityOperation security;
private final ServerBulkImportStatus bulkImportStatus = new
ServerBulkImportStatus();
public ClientServiceHandler(ServerContext context, TransactionWatcher
transactionWatcher) {
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java
b/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java
index e4f61ed9ce..f1068ec6ff 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java
@@ -18,13 +18,17 @@
*/
package org.apache.accumulo.server.security;
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toMap;
+
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Set;
-import java.util.stream.Collectors;
+import java.util.function.Supplier;
import org.apache.accumulo.core.client.TableNotFoundException;
import org.apache.accumulo.core.clientImpl.Credentials;
@@ -36,7 +40,6 @@ import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.core.dataImpl.KeyExtent;
import org.apache.accumulo.core.dataImpl.thrift.IterInfo;
import org.apache.accumulo.core.dataImpl.thrift.TColumn;
-import org.apache.accumulo.core.dataImpl.thrift.TKeyExtent;
import org.apache.accumulo.core.dataImpl.thrift.TRange;
import org.apache.accumulo.core.manager.thrift.FateOperation;
import org.apache.accumulo.core.metadata.MetadataTable;
@@ -138,26 +141,22 @@ public class AuditedSecurityOperation extends
SecurityOperation {
return result;
}
- @Override
- public boolean canScan(TCredentials credentials, TableId tableId,
NamespaceId namespaceId,
- TRange range, List<TColumn> columns, List<IterInfo> ssiList,
+ private boolean canScan(TCredentials credentials, TableId tableId,
NamespaceId namespaceId,
+ Supplier<String> rangeStringSupplier, List<TColumn> columns,
List<IterInfo> ssiList,
Map<String,Map<String,String>> ssio, List<ByteBuffer> authorizations)
throws ThriftSecurityException {
if (shouldAudit(credentials, tableId)) {
- Range convertedRange = new Range(range);
- List<String> convertedColumns =
-
truncate(columns.stream().map(Column::new).collect(Collectors.toList()));
+ String rangeString = rangeStringSupplier.get();
+ List<String> convertedColumns =
truncate(columns.stream().map(Column::new).collect(toList()));
String tableName = getTableName(tableId);
-
try {
boolean canScan = super.canScan(credentials, tableId, namespaceId);
audit(credentials, canScan, CAN_SCAN_AUDIT_TEMPLATE, tableName,
- getAuthString(authorizations), convertedRange, convertedColumns,
ssiList, ssio);
-
+ getAuthString(authorizations), rangeString, convertedColumns,
ssiList, ssio);
return canScan;
} catch (ThriftSecurityException ex) {
audit(credentials, ex, CAN_SCAN_AUDIT_TEMPLATE,
getAuthString(authorizations), tableId,
- convertedRange, convertedColumns, ssiList, ssio);
+ rangeString, convertedColumns, ssiList, ssio);
throw ex;
}
} else {
@@ -165,41 +164,30 @@ public class AuditedSecurityOperation extends
SecurityOperation {
}
}
- public static final String CAN_SCAN_BATCH_AUDIT_TEMPLATE =
- "action: scan; targetTable: %s; authorizations: %s; range: %s; columns:
%s;"
- + " iterators: %s; iteratorOptions: %s;";
-
- @Override
public boolean canScan(TCredentials credentials, TableId tableId,
NamespaceId namespaceId,
- Map<TKeyExtent,List<TRange>> tbatch, List<TColumn> tcolumns,
List<IterInfo> ssiList,
+ TRange range, List<TColumn> columns, List<IterInfo> ssiList,
Map<String,Map<String,String>> ssio, List<ByteBuffer> authorizations)
throws ThriftSecurityException {
- if (shouldAudit(credentials, tableId)) {
+ Supplier<String> rangeToString = () -> new Range(range).toString();
+ return canScan(credentials, tableId, namespaceId, rangeToString, columns,
ssiList, ssio,
+ authorizations);
+ }
+ public boolean canScan(TCredentials credentials, TableId tableId,
NamespaceId namespaceId,
+ Map<KeyExtent,List<TRange>> tbatch, List<TColumn> columns,
List<IterInfo> ssiList,
+ Map<String,Map<String,String>> ssio, List<ByteBuffer> authorizations)
+ throws ThriftSecurityException {
+ Supplier<String> rangeToString = () -> {
// @formatter:off
- Map<KeyExtent, List<String>> truncated =
tbatch.entrySet().stream().collect(Collectors.toMap(
- entry -> KeyExtent.fromThrift(entry.getKey()),
- entry ->
truncate(entry.getValue().stream().map(Range::new).collect(Collectors.toList()))
+ Map<KeyExtent,List<String>> truncated =
tbatch.entrySet().stream().collect(toMap(
+ Entry::getKey,
+ entry ->
truncate(entry.getValue().stream().map(Range::new).collect(toList()))
));
// @formatter:on
- List<Column> convertedColumns =
- tcolumns.stream().map(Column::new).collect(Collectors.toList());
- String tableName = getTableName(tableId);
-
- try {
- boolean canScan = super.canScan(credentials, tableId, namespaceId);
- audit(credentials, canScan, CAN_SCAN_BATCH_AUDIT_TEMPLATE, tableName,
- getAuthString(authorizations), truncated, convertedColumns,
ssiList, ssio);
-
- return canScan;
- } catch (ThriftSecurityException ex) {
- audit(credentials, ex, CAN_SCAN_BATCH_AUDIT_TEMPLATE,
getAuthString(authorizations),
- tableId, truncated, convertedColumns, ssiList, ssio);
- throw ex;
- }
- } else {
- return super.canScan(credentials, tableId, namespaceId);
- }
+ return truncated.toString();
+ };
+ return canScan(credentials, tableId, namespaceId, rangeToString, columns,
ssiList, ssio,
+ authorizations);
}
public static final String CHANGE_AUTHORIZATIONS_AUDIT_TEMPLATE =
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
b/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
index dd59cebd93..45e665c882 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
@@ -22,7 +22,6 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import java.nio.ByteBuffer;
import java.util.List;
-import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;
@@ -38,10 +37,6 @@ import
org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.data.NamespaceId;
import org.apache.accumulo.core.data.TableId;
-import org.apache.accumulo.core.dataImpl.thrift.IterInfo;
-import org.apache.accumulo.core.dataImpl.thrift.TColumn;
-import org.apache.accumulo.core.dataImpl.thrift.TKeyExtent;
-import org.apache.accumulo.core.dataImpl.thrift.TRange;
import org.apache.accumulo.core.fate.zookeeper.ZooCache;
import org.apache.accumulo.core.manager.thrift.FateOperation;
import org.apache.accumulo.core.metadata.MetadataTable;
@@ -344,7 +339,7 @@ public class SecurityOperation {
*
* @return true if a user exists and has permission; false otherwise
*/
- protected boolean hasTablePermission(TCredentials credentials, TableId
tableId,
+ private boolean hasTablePermission(TCredentials credentials, TableId tableId,
NamespaceId namespaceId, TablePermission permission, boolean useCached)
throws ThriftSecurityException {
if (isSystemUser(credentials)) {
@@ -437,20 +432,6 @@ public class SecurityOperation {
return hasTablePermission(credentials, tableId, namespaceId,
TablePermission.READ, true);
}
- public boolean canScan(TCredentials credentials, TableId tableId,
NamespaceId namespaceId,
- TRange range, List<TColumn> columns, List<IterInfo> ssiList,
- Map<String,Map<String,String>> ssio, List<ByteBuffer> authorizations)
- throws ThriftSecurityException {
- return canScan(credentials, tableId, namespaceId);
- }
-
- public boolean canScan(TCredentials credentials, TableId table, NamespaceId
namespaceId,
- Map<TKeyExtent,List<TRange>> tbatch, List<TColumn> tcolumns,
List<IterInfo> ssiList,
- Map<String,Map<String,String>> ssio, List<ByteBuffer> authorizations)
- throws ThriftSecurityException {
- return canScan(credentials, table, namespaceId);
- }
-
public boolean canWrite(TCredentials credentials, TableId tableId,
NamespaceId namespaceId)
throws ThriftSecurityException {
authenticate(credentials);
@@ -466,8 +447,8 @@ public class SecurityOperation {
&& hasTablePermission(credentials, tableID, namespaceId,
TablePermission.READ, true);
}
- public boolean canSplitTablet(TCredentials credentials, TableId tableId,
NamespaceId namespaceId)
- throws ThriftSecurityException {
+ protected boolean canSplitTablet(TCredentials credentials, TableId tableId,
+ NamespaceId namespaceId) throws ThriftSecurityException {
authenticate(credentials);
return hasSystemPermissionWithNamespaceId(credentials,
SystemPermission.ALTER_TABLE,
namespaceId, false)
@@ -481,39 +462,40 @@ public class SecurityOperation {
* This is the check to perform any system action. This includes tserver's
loading of a tablet,
* shutting the system down, or altering system properties.
*/
- public boolean canPerformSystemActions(TCredentials credentials) throws
ThriftSecurityException {
+ protected boolean canPerformSystemActions(TCredentials credentials)
+ throws ThriftSecurityException {
authenticate(credentials);
return hasSystemPermission(credentials, SystemPermission.SYSTEM, false);
}
- public boolean canFlush(TCredentials c, TableId tableId, NamespaceId
namespaceId)
+ protected boolean canFlush(TCredentials c, TableId tableId, NamespaceId
namespaceId)
throws ThriftSecurityException {
authenticate(c);
return hasTablePermission(c, tableId, namespaceId, TablePermission.WRITE,
false)
|| hasTablePermission(c, tableId, namespaceId,
TablePermission.ALTER_TABLE, false);
}
- public boolean canAlterTable(TCredentials c, TableId tableId, NamespaceId
namespaceId)
+ protected boolean canAlterTable(TCredentials c, TableId tableId, NamespaceId
namespaceId)
throws ThriftSecurityException {
authenticate(c);
return hasTablePermission(c, tableId, namespaceId,
TablePermission.ALTER_TABLE, false)
|| hasSystemPermissionWithNamespaceId(c, SystemPermission.ALTER_TABLE,
namespaceId, false);
}
- public boolean canCreateTable(TCredentials c, String tableName, NamespaceId
namespaceId)
+ protected boolean canCreateTable(TCredentials c, String tableName,
NamespaceId namespaceId)
throws ThriftSecurityException {
authenticate(c);
return hasSystemPermissionWithNamespaceId(c,
SystemPermission.CREATE_TABLE, namespaceId, false);
}
- public boolean canRenameTable(TCredentials c, TableId tableId, String
oldTableName,
+ protected boolean canRenameTable(TCredentials c, TableId tableId, String
oldTableName,
String newTableName, NamespaceId namespaceId) throws
ThriftSecurityException {
authenticate(c);
return hasSystemPermissionWithNamespaceId(c, SystemPermission.ALTER_TABLE,
namespaceId, false)
|| hasTablePermission(c, tableId, namespaceId,
TablePermission.ALTER_TABLE, false);
}
- public boolean canCloneTable(TCredentials c, TableId tableId, String
tableName,
+ protected boolean canCloneTable(TCredentials c, TableId tableId, String
tableName,
NamespaceId destinationNamespaceId, NamespaceId srcNamespaceId)
throws ThriftSecurityException {
authenticate(c);
@@ -522,14 +504,14 @@ public class SecurityOperation {
&& hasTablePermission(c, tableId, srcNamespaceId,
TablePermission.READ, false);
}
- public boolean canDeleteTable(TCredentials c, TableId tableId, NamespaceId
namespaceId)
+ protected boolean canDeleteTable(TCredentials c, TableId tableId,
NamespaceId namespaceId)
throws ThriftSecurityException {
authenticate(c);
return hasSystemPermissionWithNamespaceId(c, SystemPermission.DROP_TABLE,
namespaceId, false)
|| hasTablePermission(c, tableId, namespaceId,
TablePermission.DROP_TABLE, false);
}
- public boolean canOnlineOfflineTable(TCredentials c, TableId tableId,
FateOperation op,
+ protected boolean canOnlineOfflineTable(TCredentials c, TableId tableId,
FateOperation op,
NamespaceId namespaceId) throws ThriftSecurityException {
authenticate(c);
return hasSystemPermissionWithNamespaceId(c, SystemPermission.SYSTEM,
namespaceId, false)
@@ -537,7 +519,7 @@ public class SecurityOperation {
|| hasTablePermission(c, tableId, namespaceId,
TablePermission.ALTER_TABLE, false);
}
- public boolean canMerge(TCredentials c, TableId tableId, NamespaceId
namespaceId)
+ protected boolean canMerge(TCredentials c, TableId tableId, NamespaceId
namespaceId)
throws ThriftSecurityException {
authenticate(c);
return hasSystemPermissionWithNamespaceId(c, SystemPermission.SYSTEM,
namespaceId, false)
@@ -545,20 +527,20 @@ public class SecurityOperation {
|| hasTablePermission(c, tableId, namespaceId,
TablePermission.ALTER_TABLE, false);
}
- public boolean canDeleteRange(TCredentials c, TableId tableId, String
tableName, Text startRow,
+ protected boolean canDeleteRange(TCredentials c, TableId tableId, String
tableName, Text startRow,
Text endRow, NamespaceId namespaceId) throws ThriftSecurityException {
authenticate(c);
return hasSystemPermissionWithNamespaceId(c, SystemPermission.SYSTEM,
namespaceId, false)
|| hasTablePermission(c, tableId, namespaceId, TablePermission.WRITE,
false);
}
- public boolean canBulkImport(TCredentials c, TableId tableId, String
tableName, String dir,
+ protected boolean canBulkImport(TCredentials c, TableId tableId, String
tableName, String dir,
String failDir, NamespaceId namespaceId) throws ThriftSecurityException {
authenticate(c);
return hasTablePermission(c, tableId, namespaceId,
TablePermission.BULK_IMPORT, false);
}
- public boolean canCompact(TCredentials c, TableId tableId, NamespaceId
namespaceId)
+ protected boolean canCompact(TCredentials c, TableId tableId, NamespaceId
namespaceId)
throws ThriftSecurityException {
authenticate(c);
return hasSystemPermissionWithNamespaceId(c, SystemPermission.ALTER_TABLE,
namespaceId, false)
@@ -566,24 +548,24 @@ public class SecurityOperation {
|| hasTablePermission(c, tableId, namespaceId, TablePermission.WRITE,
false);
}
- public boolean canChangeAuthorizations(TCredentials c, String user)
+ protected boolean canChangeAuthorizations(TCredentials c, String user)
throws ThriftSecurityException {
authenticate(c);
return hasSystemPermission(c, SystemPermission.ALTER_USER, false);
}
- public boolean canChangePassword(TCredentials c, String user) throws
ThriftSecurityException {
+ protected boolean canChangePassword(TCredentials c, String user) throws
ThriftSecurityException {
authenticate(c);
return c.getPrincipal().equals(user)
|| hasSystemPermission(c, SystemPermission.ALTER_USER, false);
}
- public boolean canCreateUser(TCredentials c, String user) throws
ThriftSecurityException {
+ protected boolean canCreateUser(TCredentials c, String user) throws
ThriftSecurityException {
authenticate(c);
return hasSystemPermission(c, SystemPermission.CREATE_USER, false);
}
- public boolean canDropUser(TCredentials c, String user) throws
ThriftSecurityException {
+ protected boolean canDropUser(TCredentials c, String user) throws
ThriftSecurityException {
authenticate(c);
if (user.equals(getRootUsername())) {
throw new ThriftSecurityException(c.getPrincipal(),
SecurityErrorCode.PERMISSION_DENIED);
@@ -591,20 +573,20 @@ public class SecurityOperation {
return hasSystemPermission(c, SystemPermission.DROP_USER, false);
}
- public boolean canGrantSystem(TCredentials c, String user, SystemPermission
sysPerm)
+ protected boolean canGrantSystem(TCredentials c, String user,
SystemPermission sysPerm)
throws ThriftSecurityException {
authenticate(c);
return hasSystemPermission(c, SystemPermission.GRANT, false);
}
- public boolean canGrantTable(TCredentials c, String user, TableId tableId,
+ protected boolean canGrantTable(TCredentials c, String user, TableId tableId,
NamespaceId namespaceId) throws ThriftSecurityException {
authenticate(c);
return hasSystemPermissionWithNamespaceId(c, SystemPermission.ALTER_TABLE,
namespaceId, false)
|| hasTablePermission(c, tableId, namespaceId, TablePermission.GRANT,
false);
}
- public boolean canGrantNamespace(TCredentials c, NamespaceId namespace)
+ private boolean canGrantNamespace(TCredentials c, NamespaceId namespace)
throws ThriftSecurityException {
return canModifyNamespacePermission(c, namespace);
}
@@ -623,7 +605,7 @@ public class SecurityOperation {
|| hasNamespacePermission(c, c.principal, namespace,
NamespacePermission.GRANT);
}
- public boolean canRevokeSystem(TCredentials c, String user, SystemPermission
sysPerm)
+ protected boolean canRevokeSystem(TCredentials c, String user,
SystemPermission sysPerm)
throws ThriftSecurityException {
authenticate(c);
// can't modify root user
@@ -634,19 +616,19 @@ public class SecurityOperation {
return hasSystemPermission(c, SystemPermission.GRANT, false);
}
- public boolean canRevokeTable(TCredentials c, String user, TableId tableId,
+ protected boolean canRevokeTable(TCredentials c, String user, TableId
tableId,
NamespaceId namespaceId) throws ThriftSecurityException {
authenticate(c);
return hasSystemPermissionWithNamespaceId(c, SystemPermission.ALTER_TABLE,
namespaceId, false)
|| hasTablePermission(c, tableId, namespaceId, TablePermission.GRANT,
false);
}
- public boolean canRevokeNamespace(TCredentials c, NamespaceId namespace)
+ private boolean canRevokeNamespace(TCredentials c, NamespaceId namespace)
throws ThriftSecurityException {
return canModifyNamespacePermission(c, namespace);
}
- public void changeAuthorizations(TCredentials credentials, String user,
+ protected void changeAuthorizations(TCredentials credentials, String user,
Authorizations authorizations) throws ThriftSecurityException {
if (!canChangeAuthorizations(credentials, user)) {
throw new ThriftSecurityException(credentials.getPrincipal(),
@@ -664,7 +646,7 @@ public class SecurityOperation {
}
}
- public void changePassword(TCredentials credentials, Credentials toChange)
+ protected void changePassword(TCredentials credentials, Credentials toChange)
throws ThriftSecurityException {
if (!canChangePassword(credentials, toChange.getPrincipal())) {
throw new ThriftSecurityException(credentials.getPrincipal(),
@@ -680,7 +662,7 @@ public class SecurityOperation {
}
}
- public void createUser(TCredentials credentials, Credentials newUser,
+ protected void createUser(TCredentials credentials, Credentials newUser,
Authorizations authorizations) throws ThriftSecurityException {
if (!canCreateUser(credentials, newUser.getPrincipal())) {
throw new ThriftSecurityException(credentials.getPrincipal(),
@@ -710,7 +692,7 @@ public class SecurityOperation {
}
}
- public void dropUser(TCredentials credentials, String user) throws
ThriftSecurityException {
+ protected void dropUser(TCredentials credentials, String user) throws
ThriftSecurityException {
if (!canDropUser(credentials, user)) {
throw new ThriftSecurityException(credentials.getPrincipal(),
SecurityErrorCode.PERMISSION_DENIED);
@@ -725,7 +707,7 @@ public class SecurityOperation {
}
}
- public void grantSystemPermission(TCredentials credentials, String user,
+ protected void grantSystemPermission(TCredentials credentials, String user,
SystemPermission permissionById) throws ThriftSecurityException {
if (!canGrantSystem(credentials, user, permissionById)) {
throw new ThriftSecurityException(credentials.getPrincipal(),
@@ -743,8 +725,8 @@ public class SecurityOperation {
}
}
- public void grantTablePermission(TCredentials c, String user, TableId
tableId, String tableName,
- TablePermission permission, NamespaceId namespaceId)
+ protected void grantTablePermission(TCredentials c, String user, TableId
tableId,
+ String tableName, TablePermission permission, NamespaceId namespaceId)
throws ThriftSecurityException, TableNotFoundException {
if (!canGrantTable(c, user, tableId, namespaceId)) {
throw new ThriftSecurityException(c.getPrincipal(),
SecurityErrorCode.PERMISSION_DENIED);
@@ -782,7 +764,7 @@ public class SecurityOperation {
}
}
- public void revokeSystemPermission(TCredentials credentials, String user,
+ protected void revokeSystemPermission(TCredentials credentials, String user,
SystemPermission permission) throws ThriftSecurityException {
if (!canRevokeSystem(credentials, user, permission)) {
throw new ThriftSecurityException(credentials.getPrincipal(),
@@ -801,7 +783,7 @@ public class SecurityOperation {
}
}
- public void revokeTablePermission(TCredentials c, String user, TableId
tableId,
+ protected void revokeTablePermission(TCredentials c, String user, TableId
tableId,
TablePermission permission, NamespaceId namespaceId) throws
ThriftSecurityException {
if (!canRevokeTable(c, user, tableId, namespaceId)) {
throw new ThriftSecurityException(c.getPrincipal(),
SecurityErrorCode.PERMISSION_DENIED);
@@ -841,7 +823,7 @@ public class SecurityOperation {
}
}
- public boolean hasSystemPermission(TCredentials credentials, String user,
+ protected boolean hasSystemPermission(TCredentials credentials, String user,
SystemPermission permissionById) throws ThriftSecurityException {
if (!canAskAboutOtherUsers(credentials, user)) {
throw new ThriftSecurityException(credentials.getPrincipal(),
@@ -907,13 +889,13 @@ public class SecurityOperation {
}
}
- public boolean canExport(TCredentials credentials, TableId tableId, String
tableName,
+ protected boolean canExport(TCredentials credentials, TableId tableId,
String tableName,
String exportDir, NamespaceId namespaceId) throws
ThriftSecurityException {
authenticate(credentials);
return hasTablePermission(credentials, tableId, namespaceId,
TablePermission.READ, false);
}
- public boolean canImport(TCredentials credentials, String tableName,
Set<String> importDir,
+ protected boolean canImport(TCredentials credentials, String tableName,
Set<String> importDir,
NamespaceId namespaceId) throws ThriftSecurityException {
authenticate(credentials);
return hasSystemPermissionWithNamespaceId(credentials,
SystemPermission.CREATE_TABLE,
@@ -946,7 +928,8 @@ public class SecurityOperation {
namespaceId, false);
}
- public boolean canObtainDelegationToken(TCredentials credentials) throws
ThriftSecurityException {
+ protected boolean canObtainDelegationToken(TCredentials credentials)
+ throws ThriftSecurityException {
authenticate(credentials);
return hasSystemPermission(credentials,
SystemPermission.OBTAIN_DELEGATION_TOKEN, false);
}
@@ -958,7 +941,7 @@ public class SecurityOperation {
false);
}
- public boolean validateStoredUserCreditentials() {
+ public boolean validateStoredUserCredentials() {
if (authenticator instanceof ZKAuthenticator) {
return !((ZKAuthenticator) authenticator).hasOutdatedHashes();
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKAuthorizor.java
b/server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKAuthorizor.java
index 2877b096ef..e23b13e25e 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKAuthorizor.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKAuthorizor.java
@@ -152,9 +152,6 @@ public class ZKAuthorizor implements Authorizor {
for (ByteBuffer auth : auths) {
if (!userauths.contains(ByteBufferUtil.toBytes(auth))) {
- log.info(
- "User {} attempted to use Authorization {} which they do not have
assigned to them.",
- user, new String(auth.array(), UTF_8));
return false;
}
}
diff --git
a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
index 4b49e88415..2079bfe0da 100644
---
a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
+++
b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
@@ -88,7 +88,7 @@ import
org.apache.accumulo.server.manager.LiveTServerSet.TServerConnection;
import org.apache.accumulo.server.rpc.ServerAddress;
import org.apache.accumulo.server.rpc.TServerUtils;
import org.apache.accumulo.server.rpc.ThriftProcessorTypes;
-import org.apache.accumulo.server.security.SecurityOperation;
+import org.apache.accumulo.server.security.AuditedSecurityOperation;
import org.apache.thrift.TException;
import org.apache.thrift.transport.TTransport;
import org.apache.thrift.transport.TTransportException;
@@ -131,7 +131,7 @@ public class CompactionCoordinator extends AbstractServer
implements
private static final Map<String,Long> TIME_COMPACTOR_LAST_CHECKED = new
ConcurrentHashMap<>();
private final GarbageCollectionLogger gcLogger = new
GarbageCollectionLogger();
- protected SecurityOperation security;
+ protected AuditedSecurityOperation security;
protected final AccumuloConfiguration aconf;
protected CompactionFinalizer compactionFinalizer;
protected LiveTServerSet tserverSet;
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
index 8cb51d160b..ee9314d958 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
@@ -133,7 +133,7 @@ import
org.apache.accumulo.server.rpc.HighlyAvailableServiceWrapper;
import org.apache.accumulo.server.rpc.ServerAddress;
import org.apache.accumulo.server.rpc.TServerUtils;
import org.apache.accumulo.server.rpc.ThriftProcessorTypes;
-import org.apache.accumulo.server.security.SecurityOperation;
+import org.apache.accumulo.server.security.AuditedSecurityOperation;
import
org.apache.accumulo.server.security.delegation.AuthenticationTokenKeyManager;
import
org.apache.accumulo.server.security.delegation.ZooAuthenticationKeyDistributor;
import org.apache.accumulo.server.tables.TableManager;
@@ -182,7 +182,7 @@ public class Manager extends AbstractServer implements
LiveTServerSet.Listener,
private final Object balancedNotifier = new Object();
final LiveTServerSet tserverSet;
private final List<TabletGroupWatcher> watchers = new ArrayList<>();
- final SecurityOperation security;
+ final AuditedSecurityOperation security;
final Map<TServerInstance,AtomicInteger> badServers =
Collections.synchronizedMap(new HashMap<>());
final Set<TServerInstance> serversToShutdown =
Collections.synchronizedSet(new HashSet<>());
@@ -1423,7 +1423,7 @@ public class Manager extends AbstractServer implements
LiveTServerSet.Listener,
ThreadPools.watchNonCriticalScheduledTask(future);
// checking stored user hashes if any of them uses an outdated algorithm
- security.validateStoredUserCreditentials();
+ security.validateStoredUserCredentials();
// The manager is fully initialized. Clients are allowed to connect now.
managerInitialized.set(true);
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/replication/ManagerReplicationCoordinator.java
b/server/manager/src/main/java/org/apache/accumulo/manager/replication/ManagerReplicationCoordinator.java
index 9063ad7af2..48aebf42db 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/replication/ManagerReplicationCoordinator.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/replication/ManagerReplicationCoordinator.java
@@ -34,7 +34,7 @@ import
org.apache.accumulo.core.replication.thrift.ReplicationCoordinatorErrorCo
import
org.apache.accumulo.core.replication.thrift.ReplicationCoordinatorException;
import org.apache.accumulo.core.securityImpl.thrift.TCredentials;
import org.apache.accumulo.manager.Manager;
-import org.apache.accumulo.server.security.SecurityOperation;
+import org.apache.accumulo.server.security.AuditedSecurityOperation;
import org.apache.thrift.TException;
import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
@@ -50,7 +50,7 @@ public class ManagerReplicationCoordinator implements
ReplicationCoordinator.Ifa
private final Manager manager;
private final ZooReader reader;
- private final SecurityOperation security;
+ private final AuditedSecurityOperation security;
public ManagerReplicationCoordinator(Manager manager) {
this(manager, manager.getContext().getZooReader());
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/SetupPermissions.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/SetupPermissions.java
index dea8dc77ff..49ff9f2914 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/SetupPermissions.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/SetupPermissions.java
@@ -24,7 +24,6 @@ import org.apache.accumulo.core.security.TablePermission;
import org.apache.accumulo.manager.Manager;
import org.apache.accumulo.manager.tableOps.ManagerRepo;
import org.apache.accumulo.manager.tableOps.TableInfo;
-import org.apache.accumulo.server.security.SecurityOperation;
import org.slf4j.LoggerFactory;
class SetupPermissions extends ManagerRepo {
@@ -40,7 +39,7 @@ class SetupPermissions extends ManagerRepo {
@Override
public Repo<Manager> call(long tid, Manager env) throws Exception {
// give all table permissions to the creator
- SecurityOperation security = env.getContext().getSecurityOperation();
+ var security = env.getContext().getSecurityOperation();
if
(!tableInfo.getUser().equals(env.getContext().getCredentials().getPrincipal()))
{
for (TablePermission permission : TablePermission.values()) {
try {
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/namespace/create/SetupNamespacePermissions.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/namespace/create/SetupNamespacePermissions.java
index ec856bd5a3..9eac28dbfb 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/namespace/create/SetupNamespacePermissions.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/namespace/create/SetupNamespacePermissions.java
@@ -23,7 +23,6 @@ import org.apache.accumulo.core.fate.Repo;
import org.apache.accumulo.core.security.NamespacePermission;
import org.apache.accumulo.manager.Manager;
import org.apache.accumulo.manager.tableOps.ManagerRepo;
-import org.apache.accumulo.server.security.SecurityOperation;
import org.slf4j.LoggerFactory;
class SetupNamespacePermissions extends ManagerRepo {
@@ -39,7 +38,7 @@ class SetupNamespacePermissions extends ManagerRepo {
@Override
public Repo<Manager> call(long tid, Manager env) throws Exception {
// give all namespace permissions to the creator
- SecurityOperation security = env.getContext().getSecurityOperation();
+ var security = env.getContext().getSecurityOperation();
for (var permission : NamespacePermission.values()) {
try {
security.grantNamespacePermission(env.getContext().rpcCreds(),
namespaceInfo.user,
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportSetupPermissions.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportSetupPermissions.java
index 61757ac11a..4f622a78e9 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportSetupPermissions.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportSetupPermissions.java
@@ -23,7 +23,6 @@ import org.apache.accumulo.core.fate.Repo;
import org.apache.accumulo.core.security.TablePermission;
import org.apache.accumulo.manager.Manager;
import org.apache.accumulo.manager.tableOps.ManagerRepo;
-import org.apache.accumulo.server.security.SecurityOperation;
import org.slf4j.LoggerFactory;
class ImportSetupPermissions extends ManagerRepo {
@@ -44,7 +43,7 @@ class ImportSetupPermissions extends ManagerRepo {
@Override
public Repo<Manager> call(long tid, Manager env) throws Exception {
// give all table permissions to the creator
- SecurityOperation security = env.getContext().getSecurityOperation();
+ var security = env.getContext().getSecurityOperation();
for (TablePermission permission : TablePermission.values()) {
try {
security.grantTablePermission(env.getContext().rpcCreds(),
tableInfo.user,
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java
index d1ccbda59b..1188bbdf1d 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java
@@ -110,7 +110,7 @@ import org.apache.accumulo.server.data.ServerMutation;
import org.apache.accumulo.server.fs.TooManyFilesException;
import org.apache.accumulo.server.fs.VolumeManager;
import org.apache.accumulo.server.rpc.TServerUtils;
-import org.apache.accumulo.server.security.SecurityOperation;
+import org.apache.accumulo.server.security.AuditedSecurityOperation;
import org.apache.accumulo.server.zookeeper.TransactionWatcher;
import org.apache.accumulo.tserver.ConditionCheckerContext.ConditionChecker;
import org.apache.accumulo.tserver.RowLocks.RowLock;
@@ -144,7 +144,7 @@ public class TabletClientHandler implements
TabletClientService.Iface {
private final TabletServer server;
protected final TransactionWatcher watcher;
protected final ServerContext context;
- protected final SecurityOperation security;
+ protected final AuditedSecurityOperation security;
private final WriteTracker writeTracker;
private final RowLocks rowLocks = new RowLocks();
@@ -263,23 +263,22 @@ public class TabletClientHandler implements
TabletClientService.Iface {
security.authenticateUser(credentials, credentials);
server.updateMetrics.addPermissionErrors(0);
- UpdateSession us =
- new UpdateSession(new TservConstraintEnv(server.getContext(),
security, credentials),
- credentials, durability) {
- @Override
- public boolean cleanup() {
- // This is called when a client abandons a session. When this
happens need to decrement
- // any queued mutations.
- if (queuedMutationSize > 0) {
- log.trace(
- "cleaning up abandoned update session, decrementing
totalQueuedMutationSize by {}",
- queuedMutationSize);
- server.updateTotalQueuedMutationSize(-queuedMutationSize);
- queuedMutationSize = 0;
- }
- return true;
- }
- };
+ UpdateSession us = new UpdateSession(new
TservConstraintEnv(server.getContext(), credentials),
+ credentials, durability) {
+ @Override
+ public boolean cleanup() {
+ // This is called when a client abandons a session. When this happens
need to decrement
+ // any queued mutations.
+ if (queuedMutationSize > 0) {
+ log.trace(
+ "cleaning up abandoned update session, decrementing
totalQueuedMutationSize by {}",
+ queuedMutationSize);
+ server.updateTotalQueuedMutationSize(-queuedMutationSize);
+ queuedMutationSize = 0;
+ }
+ return true;
+ }
+ };
return server.sessionManager.createSession(us, false);
}
@@ -688,7 +687,7 @@ public class TabletClientHandler implements
TabletClientService.Iface {
Span span = TraceUtil.startSpan(this.getClass(), "update::prep");
try (Scope scope = span.makeCurrent()) {
prepared = tablet.prepareMutationsForCommit(
- new TservConstraintEnv(server.getContext(), security,
credentials), mutations);
+ new TservConstraintEnv(server.getContext(), credentials),
mutations);
} catch (Exception e) {
TraceUtil.setException(span, e, true);
throw e;
@@ -820,7 +819,7 @@ public class TabletClientHandler implements
TabletClientService.Iface {
if (!mutations.isEmpty()) {
PreparedMutations prepared = tablet.prepareMutationsForCommit(
- new TservConstraintEnv(server.getContext(), security,
sess.credentials), mutations);
+ new TservConstraintEnv(server.getContext(), sess.credentials),
mutations);
if (prepared.tabletClosed()) {
addMutationsAsTCMResults(results, mutations, TCMStatus.IGNORED);
@@ -1126,16 +1125,11 @@ public class TabletClientHandler implements
TabletClientService.Iface {
return result;
}
- static void checkPermission(SecurityOperation security, ServerContext
context,
- TabletHostingServer server, TCredentials credentials, String lock, final
String request)
- throws ThriftSecurityException {
+ static void checkPermission(ServerContext context, TabletHostingServer
server,
+ TCredentials credentials, String lock, final String request) throws
ThriftSecurityException {
+ boolean canPerformSystemActions = false;
try {
- log.trace("Got {} message from user: {}", request,
credentials.getPrincipal());
- if (!security.canPerformSystemActions(credentials)) {
- log.warn("Got {} message from user: {}", request,
credentials.getPrincipal());
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
+ canPerformSystemActions =
context.getSecurityOperation().canPerformSystemActions(credentials);
} catch (ThriftSecurityException e) {
log.warn("Got {} message from unauthenticatable user: {}", request,
e.getUser());
if (context.getCredentials().getToken().getClass().getName()
@@ -1146,11 +1140,19 @@ public class TabletClientHandler implements
TabletClientService.Iface {
throw e;
}
+ if (!canPerformSystemActions) {
+ log.warn("Denied {} request from user: {}", request,
credentials.getPrincipal());
+ throw new ThriftSecurityException(credentials.getPrincipal(),
+ SecurityErrorCode.PERMISSION_DENIED);
+ }
+
if (server.getLock() == null || !server.getLock().wasLockAcquired()) {
- log.debug("Got {} message before my lock was acquired, ignoring...",
request);
+ log.debug("Got {} message from user {} before my lock was acquired,
ignoring...", request,
+ credentials.getPrincipal());
throw new RuntimeException("Lock not acquired");
}
+ log.trace("Got {} message from user: {}", request,
credentials.getPrincipal());
if (server.getLock() != null && server.getLock().wasLockAcquired()
&& !server.getLock().isLocked()) {
Halt.halt(1, () -> {
@@ -1186,7 +1188,7 @@ public class TabletClientHandler implements
TabletClientService.Iface {
final TKeyExtent textent) {
try {
- checkPermission(security, context, server, credentials, lock,
"loadTablet");
+ checkPermission(context, server, credentials, lock, "loadTablet");
} catch (ThriftSecurityException e) {
log.error("Caller doesn't have permission to load a tablet", e);
throw new RuntimeException(e);
@@ -1269,7 +1271,7 @@ public class TabletClientHandler implements
TabletClientService.Iface {
public void unloadTablet(TInfo tinfo, TCredentials credentials, String lock,
TKeyExtent textent,
TUnloadTabletGoal goal, long requestTime) {
try {
- checkPermission(security, context, server, credentials, lock,
"unloadTablet");
+ checkPermission(context, server, credentials, lock, "unloadTablet");
} catch (ThriftSecurityException e) {
log.error("Caller doesn't have permission to unload a tablet", e);
throw new RuntimeException(e);
@@ -1285,7 +1287,7 @@ public class TabletClientHandler implements
TabletClientService.Iface {
public void flush(TInfo tinfo, TCredentials credentials, String lock, String
tableId,
ByteBuffer startRow, ByteBuffer endRow) {
try {
- checkPermission(security, context, server, credentials, lock, "flush");
+ checkPermission(context, server, credentials, lock, "flush");
} catch (ThriftSecurityException e) {
log.error("Caller doesn't have permission to flush a table", e);
throw new RuntimeException(e);
@@ -1318,7 +1320,7 @@ public class TabletClientHandler implements
TabletClientService.Iface {
@Override
public void flushTablet(TInfo tinfo, TCredentials credentials, String lock,
TKeyExtent textent) {
try {
- checkPermission(security, context, server, credentials, lock,
"flushTablet");
+ checkPermission(context, server, credentials, lock, "flushTablet");
} catch (ThriftSecurityException e) {
log.error("Caller doesn't have permission to flush a tablet", e);
throw new RuntimeException(e);
@@ -1340,7 +1342,7 @@ public class TabletClientHandler implements
TabletClientService.Iface {
public void halt(TInfo tinfo, TCredentials credentials, String lock)
throws ThriftSecurityException {
- checkPermission(security, context, server, credentials, lock, "halt");
+ checkPermission(context, server, credentials, lock, "halt");
Halt.halt(0, () -> {
log.info("Manager requested tablet server halt");
@@ -1371,7 +1373,7 @@ public class TabletClientHandler implements
TabletClientService.Iface {
@Override
public void chop(TInfo tinfo, TCredentials credentials, String lock,
TKeyExtent textent) {
try {
- checkPermission(security, context, server, credentials, lock, "chop");
+ checkPermission(context, server, credentials, lock, "chop");
} catch (ThriftSecurityException e) {
log.error("Caller doesn't have permission to chop extent", e);
throw new RuntimeException(e);
@@ -1389,7 +1391,7 @@ public class TabletClientHandler implements
TabletClientService.Iface {
public void compact(TInfo tinfo, TCredentials credentials, String lock,
String tableId,
ByteBuffer startRow, ByteBuffer endRow) {
try {
- checkPermission(security, context, server, credentials, lock, "compact");
+ checkPermission(context, server, credentials, lock, "compact");
} catch (ThriftSecurityException e) {
log.error("Caller doesn't have permission to compact a table", e);
throw new RuntimeException(e);
@@ -1421,7 +1423,7 @@ public class TabletClientHandler implements
TabletClientService.Iface {
public List<ActiveCompaction> getActiveCompactions(TInfo tinfo, TCredentials
credentials)
throws ThriftSecurityException, TException {
try {
- checkPermission(security, context, server, credentials, null,
"getActiveCompactions");
+ checkPermission(context, server, credentials, null,
"getActiveCompactions");
} catch (ThriftSecurityException e) {
log.error("Caller doesn't have permission to get active compactions", e);
throw e;
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
index 2d4749b90b..c70f5f48e4 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
@@ -128,7 +128,6 @@ import
org.apache.accumulo.server.manager.recovery.RecoveryPath;
import org.apache.accumulo.server.rpc.ServerAddress;
import org.apache.accumulo.server.rpc.TServerUtils;
import org.apache.accumulo.server.rpc.ThriftProcessorTypes;
-import org.apache.accumulo.server.security.SecurityOperation;
import org.apache.accumulo.server.security.SecurityUtil;
import
org.apache.accumulo.server.security.delegation.ZooAuthenticationKeyWatcher;
import org.apache.accumulo.server.util.FileSystemMonitor;
@@ -219,7 +218,6 @@ public class TabletServer extends AbstractServer
final Map<KeyExtent,Long> recentlyUnloadedCache =
Collections.synchronizedMap(new LRUMap<>(1000));
final TabletServerResourceManager resourceManager;
- private final SecurityOperation security;
private final BlockingDeque<ManagerMessage> managerMessages = new
LinkedBlockingDeque<>();
@@ -356,7 +354,6 @@ public class TabletServer extends AbstractServer
logger = new TabletServerLogger(this, walMaxSize, syncCounter,
flushCounter,
walCreationRetryFactory, walWritingRetryFactory, walMaxAge);
this.resourceManager = new TabletServerResourceManager(context, this);
- this.security = context.getSecurityOperation();
watchCriticalScheduledTask(context.getScheduledExecutor().scheduleWithFixedDelay(
TabletLocator::clearLocators, jitter(), jitter(),
TimeUnit.MILLISECONDS));
@@ -1342,10 +1339,6 @@ public class TabletServer extends AbstractServer
return resourceManager.holdTime();
}
- public SecurityOperation getSecurityOperation() {
- return security;
- }
-
// avoid unnecessary redundant markings to meta
final ConcurrentHashMap<DfsLogger,EnumSet<TabletLevel>> metadataTableLogs =
new ConcurrentHashMap<>();
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/ThriftScanClientHandler.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/ThriftScanClientHandler.java
index 6331cc9c45..b254fd3e40 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/ThriftScanClientHandler.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/ThriftScanClientHandler.java
@@ -18,6 +18,8 @@
*/
package org.apache.accumulo.tserver;
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toMap;
import static
org.apache.accumulo.core.util.UtilWaitThread.sleepUninterruptibly;
import java.io.IOException;
@@ -27,12 +29,12 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
-import java.util.stream.Collectors;
import org.apache.accumulo.core.client.SampleNotPresentException;
import org.apache.accumulo.core.client.TableNotFoundException;
@@ -70,7 +72,7 @@ import org.apache.accumulo.core.trace.thrift.TInfo;
import org.apache.accumulo.server.ServerContext;
import org.apache.accumulo.server.fs.TooManyFilesException;
import org.apache.accumulo.server.rpc.TServerUtils;
-import org.apache.accumulo.server.security.SecurityOperation;
+import org.apache.accumulo.server.security.AuditedSecurityOperation;
import org.apache.accumulo.tserver.scan.LookupTask;
import org.apache.accumulo.tserver.scan.NextBatchTask;
import org.apache.accumulo.tserver.scan.ScanParameters;
@@ -94,7 +96,7 @@ public class ThriftScanClientHandler implements
TabletScanClientService.Iface {
private final TabletHostingServer server;
protected final ServerContext context;
- protected final SecurityOperation security;
+ protected final AuditedSecurityOperation security;
private final WriteTracker writeTracker;
private final long MAX_TIME_TO_WAIT_FOR_SCAN_RESULT_MILLIS;
@@ -395,7 +397,8 @@ public class ThriftScanClientHandler implements
TabletScanClientService.Iface {
// check if user has permission to the tables
for (TableId tableId : tables) {
NamespaceId namespaceId = getNamespaceId(credentials, tableId);
- if (!security.canScan(credentials, tableId, namespaceId)) {
+ if (!security.canScan(credentials, tableId, namespaceId, tbatch,
tcolumns, ssiList, ssio,
+ authorizations)) {
throw new ThriftSecurityException(credentials.getPrincipal(),
SecurityErrorCode.PERMISSION_DENIED);
}
@@ -407,9 +410,9 @@ public class ThriftScanClientHandler implements
TabletScanClientService.Iface {
}
// @formatter:off
- Map<KeyExtent, List<Range>> batch =
tbatch.entrySet().stream().collect(Collectors.toMap(
- entry -> entry.getKey(),
- entry ->
entry.getValue().stream().map(Range::new).collect(Collectors.toList())
+ Map<KeyExtent,List<Range>> batch =
tbatch.entrySet().stream().collect(toMap(
+ Entry::getKey,
+ entry -> entry.getValue().stream().map(Range::new).collect(toList())
));
// @formatter:on
@@ -538,13 +541,7 @@ public class ThriftScanClientHandler implements
TabletScanClientService.Iface {
@Override
public List<ActiveScan> getActiveScans(TInfo tinfo, TCredentials credentials)
throws ThriftSecurityException, TException {
- try {
- TabletClientHandler.checkPermission(security, context, server,
credentials, null, "getScans");
- } catch (ThriftSecurityException e) {
- log.error("Caller doesn't have permission to get active scans", e);
- throw e;
- }
-
+ TabletClientHandler.checkPermission(context, server, credentials, null,
"getScans");
return server.getSessionManager().getActiveScans();
}
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TservConstraintEnv.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TservConstraintEnv.java
index d307f725b0..f50f9264a5 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TservConstraintEnv.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TservConstraintEnv.java
@@ -18,8 +18,9 @@
*/
package org.apache.accumulo.tserver;
+import static java.util.Collections.singletonList;
+
import java.nio.ByteBuffer;
-import java.util.Collections;
import org.apache.accumulo.core.data.TabletId;
import org.apache.accumulo.core.dataImpl.KeyExtent;
@@ -28,7 +29,6 @@ import
org.apache.accumulo.core.security.AuthorizationContainer;
import org.apache.accumulo.core.securityImpl.thrift.TCredentials;
import org.apache.accumulo.server.ServerContext;
import org.apache.accumulo.server.constraints.SystemEnvironment;
-import org.apache.accumulo.server.security.SecurityOperation;
@SuppressWarnings("deprecation")
public class TservConstraintEnv
@@ -36,12 +36,10 @@ public class TservConstraintEnv
private final ServerContext context;
private final TCredentials credentials;
- private final SecurityOperation security;
private KeyExtent ke;
- TservConstraintEnv(ServerContext context, SecurityOperation secOp,
TCredentials credentials) {
+ TservConstraintEnv(ServerContext context, TCredentials credentials) {
this.context = context;
- this.security = secOp;
this.credentials = credentials;
}
@@ -66,8 +64,8 @@ public class TservConstraintEnv
@Override
public AuthorizationContainer getAuthorizationsContainer() {
- return auth -> security.authenticatedUserHasAuthorizations(credentials,
Collections
- .singletonList(ByteBuffer.wrap(auth.getBackingArray(), auth.offset(),
auth.length())));
+ return auth ->
context.getSecurityOperation().authenticatedUserHasAuthorizations(credentials,
+ singletonList(ByteBuffer.wrap(auth.getBackingArray(), auth.offset(),
auth.length())));
}
@Override
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/replication/ReplicationServicerHandler.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/replication/ReplicationServicerHandler.java
index ab8400dc5e..56f3431e5c 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/replication/ReplicationServicerHandler.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/replication/ReplicationServicerHandler.java
@@ -53,8 +53,8 @@ public class ReplicationServicerHandler implements Iface {
throws TException {
TableId tableId = TableId.of(tableIdStr);
log.debug("Got replication request to tableID {} with {} edits", tableId,
data.getEditsSize());
-
tabletServer.getSecurityOperation().authenticateUser(tabletServer.getContext().rpcCreds(),
- tcreds);
+ tabletServer.getContext().getSecurityOperation()
+ .authenticateUser(tabletServer.getContext().rpcCreds(), tcreds);
String tableName;
diff --git
a/server/tserver/src/test/java/org/apache/accumulo/tserver/TservConstraintEnvTest.java
b/server/tserver/src/test/java/org/apache/accumulo/tserver/TservConstraintEnvTest.java
index 0c6b1038c2..7056b3483c 100644
---
a/server/tserver/src/test/java/org/apache/accumulo/tserver/TservConstraintEnvTest.java
+++
b/server/tserver/src/test/java/org/apache/accumulo/tserver/TservConstraintEnvTest.java
@@ -22,6 +22,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import static org.easymock.EasyMock.createMock;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -32,14 +33,16 @@ import java.util.List;
import org.apache.accumulo.core.data.ArrayByteSequence;
import org.apache.accumulo.core.data.ByteSequence;
import org.apache.accumulo.core.securityImpl.thrift.TCredentials;
-import org.apache.accumulo.server.security.SecurityOperation;
+import org.apache.accumulo.server.ServerContext;
+import org.apache.accumulo.server.security.AuditedSecurityOperation;
import org.junit.jupiter.api.Test;
public class TservConstraintEnvTest {
@Test
public void testGetAuthorizationsContainer() {
- SecurityOperation security = createMock(SecurityOperation.class);
+ ServerContext context = createMock(ServerContext.class);
+ AuditedSecurityOperation security =
createMock(AuditedSecurityOperation.class);
TCredentials goodCred = createMock(TCredentials.class);
TCredentials badCred = createMock(TCredentials.class);
@@ -49,11 +52,11 @@ public class TservConstraintEnvTest {
expect(security.authenticatedUserHasAuthorizations(goodCred,
bbList)).andReturn(true);
expect(security.authenticatedUserHasAuthorizations(badCred,
bbList)).andReturn(false);
- replay(security);
+ expect(context.getSecurityOperation()).andReturn(security).atLeastOnce();
+ replay(context, security, goodCred, badCred);
- assertTrue(
- new TservConstraintEnv(null, security,
goodCred).getAuthorizationsContainer().contains(bs));
- assertFalse(
- new TservConstraintEnv(null, security,
badCred).getAuthorizationsContainer().contains(bs));
+ assertTrue(new TservConstraintEnv(context,
goodCred).getAuthorizationsContainer().contains(bs));
+ assertFalse(new TservConstraintEnv(context,
badCred).getAuthorizationsContainer().contains(bs));
+ verify(context, security, goodCred, badCred);
}
}
