This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new ac5a1227a7 Revert "Added missing permission checks to
ClientServiceHandler Thrift API methods (#2988)"
ac5a1227a7 is described below
commit ac5a1227a743dd0fb50d90cda9f93d2b6c2d6edc
Author: Christopher Tubbs <ctubb...@apache.org>
AuthorDate: Mon Oct 3 01:46:48 2022 -0400
Revert "Added missing permission checks to ClientServiceHandler Thrift API
methods (#2988)"
This reverts commit a5c9b3724ddfb5c24968272c0891da9723675472.
This patch is not ready, and causes a lot of test failures. It should be
re-applied after further consideration of desired behavior and after
ensuring tests are not failing as a result of the change.
---
.../server/client/ClientServiceHandler.java | 91 +++++-----------------
1 file changed, 18 insertions(+), 73 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 79f47b46c8..57ced4a3c8 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
@@ -293,6 +293,7 @@ public class ClientServiceHandler implements
ClientService.Iface {
private Map<String,String> conf(TCredentials credentials,
AccumuloConfiguration conf)
throws TException {
+ security.authenticateUser(credentials, credentials);
conf.invalidateCache();
Map<String,String> result = new HashMap<>();
@@ -307,11 +308,6 @@ public class ClientServiceHandler implements
ClientService.Iface {
@Override
public Map<String,String> getConfiguration(TInfo tinfo, TCredentials
credentials,
ConfigurationType type) throws TException {
- if (!security.hasSystemPermission(credentials, credentials.getPrincipal(),
- SystemPermission.SYSTEM)) {
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
switch (type) {
case CURRENT:
context.getPropStore().getCache().remove(SystemPropKey.of(context));
@@ -325,70 +321,38 @@ public class ClientServiceHandler implements
ClientService.Iface {
}
@Override
- public Map<String,String> getSystemProperties(TInfo tinfo, TCredentials
credentials)
- throws ThriftSecurityException {
- if (security.hasSystemPermission(credentials, credentials.getPrincipal(),
- SystemPermission.SYSTEM)) {
- return context.getPropStore().get(SystemPropKey.of(context)).asMap();
- } else {
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
+ public Map<String,String> getSystemProperties(TInfo tinfo, TCredentials
credentials) {
+ return context.getPropStore().get(SystemPropKey.of(context)).asMap();
}
@Override
- public TVersionedProperties getVersionedSystemProperties(TInfo tinfo,
TCredentials credentials)
- throws ThriftSecurityException {
- if (security.hasSystemPermission(credentials, credentials.getPrincipal(),
- SystemPermission.SYSTEM)) {
- return Optional.of(context.getPropStore().get(SystemPropKey.of(context)))
- .map(vProps -> new TVersionedProperties(vProps.getDataVersion(),
vProps.asMap())).get();
- } else {
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
+ public TVersionedProperties getVersionedSystemProperties(TInfo tinfo,
TCredentials credentials) {
+ return Optional.of(context.getPropStore().get(SystemPropKey.of(context)))
+ .map(vProps -> new TVersionedProperties(vProps.getDataVersion(),
vProps.asMap())).get();
}
@Override
public Map<String,String> getTableConfiguration(TInfo tinfo, TCredentials
credentials,
String tableName) throws TException, ThriftTableOperationException {
TableId tableId = checkTableId(context, tableName, null);
- if (security.hasTablePermission(credentials, credentials.getPrincipal(),
tableId,
- TablePermission.READ)) {
- context.getPropStore().getCache().remove(TablePropKey.of(context,
tableId));
- AccumuloConfiguration config = context.getTableConfiguration(tableId);
- return conf(credentials, config);
- } else {
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
+ context.getPropStore().getCache().remove(TablePropKey.of(context,
tableId));
+ AccumuloConfiguration config = context.getTableConfiguration(tableId);
+ return conf(credentials, config);
}
@Override
public Map<String,String> getTableProperties(TInfo tinfo, TCredentials
credentials,
String tableName) throws TException {
final TableId tableId = checkTableId(context, tableName, null);
- if (security.hasTablePermission(credentials, credentials.getPrincipal(),
tableId,
- TablePermission.READ)) {
- return context.getPropStore().get(TablePropKey.of(context,
tableId)).asMap();
- } else {
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
+ return context.getPropStore().get(TablePropKey.of(context,
tableId)).asMap();
}
@Override
public TVersionedProperties getVersionedTableProperties(TInfo tinfo,
TCredentials credentials,
String tableName) throws TException {
final TableId tableId = checkTableId(context, tableName, null);
- if (security.hasTablePermission(credentials, credentials.getPrincipal(),
tableId,
- TablePermission.READ)) {
- return Optional.of(context.getPropStore().get(TablePropKey.of(context,
tableId)))
- .map(vProps -> new TVersionedProperties(vProps.getDataVersion(),
vProps.asMap())).get();
- } else {
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
+ return Optional.of(context.getPropStore().get(TablePropKey.of(context,
tableId)))
+ .map(vProps -> new TVersionedProperties(vProps.getDataVersion(),
vProps.asMap())).get();
}
@Override
@@ -526,15 +490,9 @@ public class ClientServiceHandler implements
ClientService.Iface {
throw new ThriftTableOperationException(null, ns, null,
TableOperationExceptionType.NAMESPACE_NOTFOUND, why);
}
- if (security.hasNamespacePermission(credentials,
credentials.getPrincipal(), namespaceId,
- NamespacePermission.READ)) {
- context.getPropStore().getCache().remove(NamespacePropKey.of(context,
namespaceId));
- AccumuloConfiguration config =
context.getNamespaceConfiguration(namespaceId);
- return conf(credentials, config);
- } else {
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
+ context.getPropStore().getCache().remove(NamespacePropKey.of(context,
namespaceId));
+ AccumuloConfiguration config =
context.getNamespaceConfiguration(namespaceId);
+ return conf(credentials, config);
}
@Override
@@ -543,13 +501,7 @@ public class ClientServiceHandler implements
ClientService.Iface {
NamespaceId namespaceId;
try {
namespaceId = Namespaces.getNamespaceId(context, ns);
- if (security.hasNamespacePermission(credentials,
credentials.getPrincipal(), namespaceId,
- NamespacePermission.READ)) {
- return context.getPropStore().get(NamespacePropKey.of(context,
namespaceId)).asMap();
- } else {
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
+ return context.getPropStore().get(NamespacePropKey.of(context,
namespaceId)).asMap();
} catch (NamespaceNotFoundException e) {
String why = "Could not find namespace while getting configuration.";
throw new ThriftTableOperationException(null, ns, null,
@@ -563,14 +515,8 @@ public class ClientServiceHandler implements
ClientService.Iface {
NamespaceId namespaceId;
try {
namespaceId = Namespaces.getNamespaceId(context, ns);
- if (security.hasNamespacePermission(credentials,
credentials.getPrincipal(), namespaceId,
- NamespacePermission.READ)) {
- return
Optional.of(context.getPropStore().get(NamespacePropKey.of(context,
namespaceId)))
- .map(vProps -> new TVersionedProperties(vProps.getDataVersion(),
vProps.asMap())).get();
- } else {
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
+ return
Optional.of(context.getPropStore().get(NamespacePropKey.of(context,
namespaceId)))
+ .map(vProps -> new TVersionedProperties(vProps.getDataVersion(),
vProps.asMap())).get();
} catch (NamespaceNotFoundException e) {
String why = "Could not find namespace while getting configuration.";
throw new ThriftTableOperationException(null, ns, null,
@@ -581,5 +527,4 @@ public class ClientServiceHandler implements
ClientService.Iface {
public List<BulkImportStatus> getBulkLoadStatus() {
return bulkImportStatus.getBulkLoadStatus();
}
-
}
