This is an automated email from the ASF dual-hosted git repository.
cshannon 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 f758cd9b5c Update permission validation methods to throw exceptions
(#3009)
f758cd9b5c is described below
commit f758cd9b5c1757fba399d308d59a9183f72ba006
Author: Christopher L. Shannon <christopher.l.shan...@gmail.com>
AuthorDate: Fri Oct 7 19:02:29 2022 -0400
Update permission validation methods to throw exceptions (#3009)
This cleans up the code a bit by having the validation methods
just throw the permission exception instead of returning a boolean
This is a follow on to #2994
---
.../server/client/ClientServiceHandler.java | 118 +++++++++------------
1 file changed, 50 insertions(+), 68 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 4eaef274d2..6d943e81f6 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
@@ -310,14 +310,36 @@ public class ClientServiceHandler implements
ClientService.Iface {
&& security.authenticateUser(credentials, credentials);
}
- @Override
- public Map<String,String> getConfiguration(TInfo tinfo, TCredentials
credentials,
- ConfigurationType type) throws TException {
+ private void checkSystemPermission(TCredentials credentials) throws
ThriftSecurityException {
if (!(checkSystemUserAndAuthenticate(credentials) ||
security.hasSystemPermission(credentials,
credentials.getPrincipal(), SystemPermission.SYSTEM))) {
throw new ThriftSecurityException(credentials.getPrincipal(),
SecurityErrorCode.PERMISSION_DENIED);
}
+ }
+
+ private void checkTablePermission(TCredentials credentials, TableId tableId,
+ TablePermission tablePermission) throws ThriftSecurityException {
+ if (!(checkSystemUserAndAuthenticate(credentials) ||
security.hasTablePermission(credentials,
+ credentials.getPrincipal(), tableId, tablePermission))) {
+ throw new ThriftSecurityException(credentials.getPrincipal(),
+ SecurityErrorCode.PERMISSION_DENIED);
+ }
+ }
+
+ private void checkNamespacePermission(TCredentials credentials, NamespaceId
namespaceId,
+ NamespacePermission namespacePermission) throws ThriftSecurityException {
+ if (!(checkSystemUserAndAuthenticate(credentials) ||
security.hasNamespacePermission(
+ credentials, credentials.getPrincipal(), namespaceId,
namespacePermission))) {
+ throw new ThriftSecurityException(credentials.getPrincipal(),
+ SecurityErrorCode.PERMISSION_DENIED);
+ }
+ }
+
+ @Override
+ public Map<String,String> getConfiguration(TInfo tinfo, TCredentials
credentials,
+ ConfigurationType type) throws TException {
+ checkSystemPermission(credentials);
switch (type) {
case CURRENT:
context.getPropStore().getCache().remove(SystemPropKey.of(context));
@@ -333,68 +355,43 @@ public class ClientServiceHandler implements
ClientService.Iface {
@Override
public Map<String,String> getSystemProperties(TInfo tinfo, TCredentials
credentials)
throws ThriftSecurityException {
- if (checkSystemUserAndAuthenticate(credentials) ||
security.hasSystemPermission(credentials,
- credentials.getPrincipal(), SystemPermission.SYSTEM)) {
- return context.getPropStore().get(SystemPropKey.of(context)).asMap();
- } else {
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
+ checkSystemPermission(credentials);
+ return context.getPropStore().get(SystemPropKey.of(context)).asMap();
}
@Override
public TVersionedProperties getVersionedSystemProperties(TInfo tinfo,
TCredentials credentials)
throws ThriftSecurityException {
- if (checkSystemUserAndAuthenticate(credentials) ||
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);
- }
+ checkSystemPermission(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 (checkSystemUserAndAuthenticate(credentials) ||
security.hasTablePermission(credentials,
- credentials.getPrincipal(), tableId, TablePermission.ALTER_TABLE)) {
- 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);
- }
+ checkTablePermission(credentials, tableId, TablePermission.ALTER_TABLE);
+ 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 (checkSystemUserAndAuthenticate(credentials) ||
security.hasTablePermission(credentials,
- credentials.getPrincipal(), tableId, TablePermission.ALTER_TABLE)) {
- return context.getPropStore().get(TablePropKey.of(context,
tableId)).asMap();
- } else {
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
+ checkTablePermission(credentials, tableId, TablePermission.ALTER_TABLE);
+ 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 (checkSystemUserAndAuthenticate(credentials) ||
security.hasTablePermission(credentials,
- credentials.getPrincipal(), tableId, TablePermission.ALTER_TABLE)) {
- 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);
- }
+ checkTablePermission(credentials, tableId, TablePermission.ALTER_TABLE);
+ return Optional.of(context.getPropStore().get(TablePropKey.of(context,
tableId)))
+ .map(vProps -> new TVersionedProperties(vProps.getDataVersion(),
vProps.asMap())).get();
}
@Override
@@ -532,15 +529,11 @@ public class ClientServiceHandler implements
ClientService.Iface {
throw new ThriftTableOperationException(null, ns, null,
TableOperationExceptionType.NAMESPACE_NOTFOUND, why);
}
- if (checkSystemUserAndAuthenticate(credentials) ||
security.hasNamespacePermission(credentials,
- credentials.getPrincipal(), namespaceId,
NamespacePermission.ALTER_NAMESPACE)) {
- 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);
- }
+ checkNamespacePermission(credentials, namespaceId,
NamespacePermission.ALTER_NAMESPACE);
+ context.getPropStore().getCache().remove(NamespacePropKey.of(context,
namespaceId));
+ AccumuloConfiguration config =
context.getNamespaceConfiguration(namespaceId);
+ return conf(credentials, config);
+
}
@Override
@@ -549,14 +542,9 @@ public class ClientServiceHandler implements
ClientService.Iface {
NamespaceId namespaceId;
try {
namespaceId = Namespaces.getNamespaceId(context, ns);
- if (checkSystemUserAndAuthenticate(credentials)
- || security.hasNamespacePermission(credentials,
credentials.getPrincipal(), namespaceId,
- NamespacePermission.ALTER_NAMESPACE)) {
- return context.getPropStore().get(NamespacePropKey.of(context,
namespaceId)).asMap();
- } else {
- throw new ThriftSecurityException(credentials.getPrincipal(),
- SecurityErrorCode.PERMISSION_DENIED);
- }
+ checkNamespacePermission(credentials, namespaceId,
NamespacePermission.ALTER_NAMESPACE);
+ 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,
@@ -570,15 +558,9 @@ public class ClientServiceHandler implements
ClientService.Iface {
NamespaceId namespaceId;
try {
namespaceId = Namespaces.getNamespaceId(context, ns);
- if (checkSystemUserAndAuthenticate(credentials)
- || security.hasNamespacePermission(credentials,
credentials.getPrincipal(), namespaceId,
- NamespacePermission.ALTER_NAMESPACE)) {
- 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);
- }
+ checkNamespacePermission(credentials, namespaceId,
NamespacePermission.ALTER_NAMESPACE);
+ 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,
