This is an automated email from the ASF dual-hosted git repository.
dlmarion 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 8a53e8dab5 Print warning in shell when user does not have permission
to get config (#3015)
8a53e8dab5 is described below
commit 8a53e8dab51aca58a04b513116763bbe4817f0a2
Author: Dave Marion <dlmar...@apache.org>
AuthorDate: Fri Oct 14 07:16:38 2022 -0400
Print warning in shell when user does not have permission to get config
(#3015)
Modified ConfigCommand to only show the user properties that they have
permission to see. When a user is unable to see the properties at a
specific level warnings are printed in the shell letting them know what
permission they are missing and that the properties that they are seeing
for the table or namespace may actually be set at a higher level that they
can't see. Override information is still shown, but only for levels that
the user
has access to.
---
.../core/clientImpl/NamespaceOperationsImpl.java | 6 +-
.../core/clientImpl/TableOperationsHelper.java | 4 +-
.../core/clientImpl/TableOperationsImpl.java | 4 +-
.../server/client/ClientServiceHandler.java | 14 ++-
.../org/apache/accumulo/server/util/Admin.java | 3 +-
shell/pom.xml | 4 +
.../accumulo/shell/commands/ConfigCommand.java | 113 ++++++++++++++++++---
.../accumulo/test/NewTableConfigurationIT.java | 3 +-
8 files changed, 126 insertions(+), 25 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java
b/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java
index 88b1e116ee..cd0dd2b580 100644
---
a/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java
+++
b/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java
@@ -285,12 +285,14 @@ public class NamespaceOperationsImpl extends
NamespaceOperationsHelper {
@Override
public Map<String,String> getConfiguration(final String namespace)
- throws AccumuloException, NamespaceNotFoundException {
+ throws AccumuloException, AccumuloSecurityException,
NamespaceNotFoundException {
EXISTING_NAMESPACE_NAME.validate(namespace);
try {
return ThriftClientTypes.CLIENT.execute(context, client -> client
.getNamespaceConfiguration(TraceUtil.traceInfo(),
context.rpcCreds(), namespace));
+ } catch (AccumuloSecurityException e) {
+ throw e;
} catch (AccumuloException e) {
Throwable t = e.getCause();
if (t instanceof ThriftTableOperationException) {
@@ -395,7 +397,7 @@ public class NamespaceOperationsImpl extends
NamespaceOperationsHelper {
}
private void checkLocalityGroups(String namespace, String propChanged)
- throws AccumuloException, NamespaceNotFoundException {
+ throws AccumuloSecurityException, AccumuloException,
NamespaceNotFoundException {
EXISTING_NAMESPACE_NAME.validate(namespace);
if (LocalityGroupUtil.isLocalityGroupProperty(propChanged)) {
diff --git
a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java
b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java
index 27f7a9f4a5..1748592792 100644
---
a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java
+++
b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java
@@ -81,7 +81,7 @@ public abstract class TableOperationsHelper implements
TableOperations {
@Override
public IteratorSetting getIteratorSetting(String tableName, String name,
IteratorScope scope)
- throws AccumuloException, TableNotFoundException {
+ throws AccumuloSecurityException, AccumuloException,
TableNotFoundException {
EXISTING_TABLE_NAME.validate(tableName);
checkArgument(name != null, "name is null");
checkArgument(scope != null, "scope is null");
@@ -113,7 +113,7 @@ public abstract class TableOperationsHelper implements
TableOperations {
@Override
public Map<String,EnumSet<IteratorScope>> listIterators(String tableName)
- throws AccumuloException, TableNotFoundException {
+ throws AccumuloSecurityException, AccumuloException,
TableNotFoundException {
EXISTING_TABLE_NAME.validate(tableName);
Map<String,EnumSet<IteratorScope>> result = new TreeMap<>();
diff --git
a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
index 65222bf5f9..1f66da5630 100644
---
a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
+++
b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
@@ -1098,7 +1098,7 @@ public class TableOperationsImpl extends
TableOperationsHelper {
}
void checkLocalityGroups(String tableName, String propChanged)
- throws AccumuloException, TableNotFoundException {
+ throws AccumuloException, AccumuloSecurityException,
TableNotFoundException {
if (LocalityGroupUtil.isLocalityGroupProperty(propChanged)) {
Map<String,String> allProps = getConfiguration(tableName);
try {
@@ -1840,7 +1840,7 @@ public class TableOperationsImpl extends
TableOperationsHelper {
@Override
public SamplerConfiguration getSamplerConfiguration(String tableName)
- throws TableNotFoundException, AccumuloException {
+ throws TableNotFoundException, AccumuloSecurityException,
AccumuloException {
EXISTING_TABLE_NAME.validate(tableName);
AccumuloConfiguration conf = new
ConfigurationCopy(this.getProperties(tableName));
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 6d943e81f6..24d345b87f 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
@@ -320,8 +320,11 @@ public class ClientServiceHandler implements
ClientService.Iface {
private void checkTablePermission(TCredentials credentials, TableId tableId,
TablePermission tablePermission) throws ThriftSecurityException {
- if (!(checkSystemUserAndAuthenticate(credentials) ||
security.hasTablePermission(credentials,
- credentials.getPrincipal(), tableId, tablePermission))) {
+ if (!(checkSystemUserAndAuthenticate(credentials)
+ || security.hasSystemPermission(credentials,
credentials.getPrincipal(),
+ SystemPermission.SYSTEM)
+ || security.hasTablePermission(credentials,
credentials.getPrincipal(), tableId,
+ tablePermission))) {
throw new ThriftSecurityException(credentials.getPrincipal(),
SecurityErrorCode.PERMISSION_DENIED);
}
@@ -329,8 +332,11 @@ public class ClientServiceHandler implements
ClientService.Iface {
private void checkNamespacePermission(TCredentials credentials, NamespaceId
namespaceId,
NamespacePermission namespacePermission) throws ThriftSecurityException {
- if (!(checkSystemUserAndAuthenticate(credentials) ||
security.hasNamespacePermission(
- credentials, credentials.getPrincipal(), namespaceId,
namespacePermission))) {
+ if (!(checkSystemUserAndAuthenticate(credentials)
+ || security.hasSystemPermission(credentials,
credentials.getPrincipal(),
+ SystemPermission.SYSTEM)
+ || security.hasNamespacePermission(credentials,
credentials.getPrincipal(), namespaceId,
+ namespacePermission))) {
throw new ThriftSecurityException(credentials.getPrincipal(),
SecurityErrorCode.PERMISSION_DENIED);
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/util/Admin.java
b/server/base/src/main/java/org/apache/accumulo/server/util/Admin.java
index 2f5010846e..a4cc0c11c6 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/util/Admin.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/util/Admin.java
@@ -743,7 +743,8 @@ public class Admin implements KeywordExecutable {
@SuppressFBWarnings(value = "PATH_TRAVERSAL_IN",
justification = "code runs in same security context as user who provided
input")
private void printTableConfiguration(AccumuloClient accumuloClient, String
tableName,
- File outputDirectory) throws AccumuloException, TableNotFoundException,
IOException {
+ File outputDirectory)
+ throws AccumuloSecurityException, AccumuloException,
TableNotFoundException, IOException {
File tableBackup = new File(outputDirectory, tableName + ".cfg");
try (BufferedWriter writer = new BufferedWriter(new
FileWriter(tableBackup, UTF_8))) {
writer.write(createTableFormat.format(new String[] {tableName}));
diff --git a/shell/pom.xml b/shell/pom.xml
index 773ae756d7..d5694e05bf 100644
--- a/shell/pom.xml
+++ b/shell/pom.xml
@@ -71,6 +71,10 @@
<groupId>org.apache.commons</groupId>
<artifactId>commons-collections4</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-lang3</artifactId>
+ </dependency>
<dependency>
<groupId>org.apache.hadoop</groupId>
<artifactId>hadoop-client-api</artifactId>
diff --git
a/shell/src/main/java/org/apache/accumulo/shell/commands/ConfigCommand.java
b/shell/src/main/java/org/apache/accumulo/shell/commands/ConfigCommand.java
index d17f3a8c74..3774cfa780 100644
--- a/shell/src/main/java/org/apache/accumulo/shell/commands/ConfigCommand.java
+++ b/shell/src/main/java/org/apache/accumulo/shell/commands/ConfigCommand.java
@@ -18,6 +18,8 @@
*/
package org.apache.accumulo.shell.commands;
+import static
org.apache.accumulo.core.client.security.SecurityErrorCode.PERMISSION_DENIED;
+
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
@@ -44,6 +46,7 @@ import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.Option;
import org.apache.commons.cli.OptionGroup;
import org.apache.commons.cli.Options;
+import org.apache.commons.lang3.StringUtils;
import org.jline.reader.LineReader;
import com.google.common.collect.ImmutableSortedMap;
@@ -153,16 +156,38 @@ public class ConfigCommand extends Command {
Shell.log.debug("Successfully set system configuration option.");
}
} else {
+ boolean warned = false;
// display properties
final TreeMap<String,String> systemConfig = new TreeMap<>();
- systemConfig
-
.putAll(shellState.getAccumuloClient().instanceOperations().getSystemConfiguration());
+ try {
+ systemConfig
+
.putAll(shellState.getAccumuloClient().instanceOperations().getSystemConfiguration());
+ } catch (AccumuloSecurityException e) {
+ if (e.getSecurityErrorCode() == PERMISSION_DENIED) {
+ Shell.log.warn(
+ "User unable to retrieve system configuration (requires
System.SYSTEM permission)");
+ warned = true;
+ } else {
+ throw e;
+ }
+ }
final String outputFile = cl.getOptionValue(outputFileOpt.getOpt());
final PrintFile printFile = outputFile == null ? null : new
PrintFile(outputFile);
final TreeMap<String,String> siteConfig = new TreeMap<>();
-
siteConfig.putAll(shellState.getAccumuloClient().instanceOperations().getSiteConfiguration());
+ try {
+ siteConfig
+
.putAll(shellState.getAccumuloClient().instanceOperations().getSiteConfiguration());
+ } catch (AccumuloSecurityException e) {
+ if (e.getSecurityErrorCode() == PERMISSION_DENIED) {
+ Shell.log.warn(
+ "User unable to retrieve site configuration (requires
System.SYSTEM permission)");
+ warned = true;
+ } else {
+ throw e;
+ }
+ }
final TreeMap<String,String> defaults = new TreeMap<>();
for (Entry<String,String> defaultEntry :
DefaultConfiguration.getInstance()) {
@@ -173,16 +198,58 @@ public class ConfigCommand extends Command {
if (tableName != null) {
String n = Namespaces.getNamespaceName(shellState.getContext(),
shellState.getContext().getNamespaceId(shellState.getContext().getTableId(tableName)));
-
shellState.getAccumuloClient().namespaceOperations().getConfiguration(n)
- .forEach(namespaceConfig::put);
+ try {
+
shellState.getAccumuloClient().namespaceOperations().getConfiguration(n)
+ .forEach(namespaceConfig::put);
+ } catch (AccumuloSecurityException e) {
+ if (e.getSecurityErrorCode() == PERMISSION_DENIED) {
+ Shell.log.warn(
+ "User unable to retrieve {} namespace configuration (requires
Namespace.ALTER_NAMESPACE permission)",
+ StringUtils.isEmpty(n) ? "default" : n);
+ warned = true;
+ } else {
+ throw e;
+ }
+ }
+ }
+
+ Map<String,String> acuconf = systemConfig;
+ if (acuconf.isEmpty()) {
+ acuconf = defaults;
}
- Map<String,String> acuconf =
-
shellState.getAccumuloClient().instanceOperations().getSystemConfiguration();
if (tableName != null) {
- acuconf =
shellState.getAccumuloClient().tableOperations().getConfiguration(tableName);
+ if (warned) {
+ Shell.log.warn(
+ "User does not have permission to see entire configuration
heirarchy. Property values shown below may be set above the table level.");
+ }
+ try {
+ acuconf =
shellState.getAccumuloClient().tableOperations().getConfiguration(tableName);
+ } catch (AccumuloException e) {
+ if (e.getCause() != null && e.getCause() instanceof
AccumuloSecurityException) {
+ AccumuloSecurityException ase = (AccumuloSecurityException)
e.getCause();
+ if (ase.getSecurityErrorCode() == PERMISSION_DENIED) {
+ Shell.log.error(
+ "User unable to retrieve {} table configuration (requires
Table.ALTER_TABLE permission)",
+ tableName);
+ }
+ }
+ throw e;
+ }
} else if (namespace != null) {
- acuconf =
shellState.getAccumuloClient().namespaceOperations().getConfiguration(namespace);
+ if (warned) {
+ Shell.log.warn(
+ "User does not have permission to see entire configuration
heirarchy. Property values shown below may be set above the namespace level.");
+ }
+ try {
+ acuconf =
+
shellState.getAccumuloClient().namespaceOperations().getConfiguration(namespace);
+ } catch (AccumuloSecurityException e) {
+ Shell.log.error(
+ "User unable to retrieve {} namespace configuration (requires
Namespace.ALTER_NAMESPACE permission)",
+ StringUtils.isEmpty(namespace) ? "default" : namespace);
+ throw e;
+ }
}
final Map<String,String> sortedConf = ImmutableSortedMap.copyOf(acuconf);
@@ -223,10 +290,10 @@ public class ConfigCommand extends Command {
String nspVal = namespaceConfig.get(key);
boolean printed = false;
- if (dfault != null && key.toLowerCase().contains("password")) {
- siteVal = sysVal = dfault = curVal = curVal.replaceAll(".", "*");
- }
if (sysVal != null) {
+ if (dfault != null && key.toLowerCase().contains("password")) {
+ siteVal = sysVal = dfault = curVal = curVal.replaceAll(".", "*");
+ }
if (defaults.containsKey(key) &&
!Property.getPropertyByKey(key).isExperimental()) {
printConfLine(output, "default", key, dfault);
printed = true;
@@ -240,9 +307,15 @@ public class ConfigCommand extends Command {
printConfLine(output, "system", printed ? " @override" : key,
sysVal);
printed = true;
}
-
}
if (nspVal != null) {
+ // If the user can't see the system configuration, then print the
default
+ // configuration value if the current namespace value is different
from it.
+ if (sysVal == null && dfault != null && !dfault.equals(nspVal)
+ && !Property.getPropertyByKey(key).isExperimental()) {
+ printConfLine(output, "default", key, dfault);
+ printed = true;
+ }
if (!systemConfig.containsKey(key) || !sysVal.equals(nspVal)) {
printConfLine(output, "namespace", printed ? " @override" : key,
nspVal);
printed = true;
@@ -251,8 +324,22 @@ public class ConfigCommand extends Command {
// show per-table value only if it is different (overridden)
if (tableName != null && !curVal.equals(nspVal)) {
+ // If the user can't see the system configuration, then print the
default
+ // configuration value if the current table value is different from
it.
+ if (nspVal == null && dfault != null && !dfault.equals(curVal)
+ && !Property.getPropertyByKey(key).isExperimental()) {
+ printConfLine(output, "default", key, dfault);
+ printed = true;
+ }
printConfLine(output, "table", printed ? " @override" : key,
curVal);
} else if (namespace != null && !curVal.equals(sysVal)) {
+ // If the user can't see the system configuration, then print the
default
+ // configuration value if the current namespace value is different
from it.
+ if (sysVal == null && dfault != null && !dfault.equals(curVal)
+ && !Property.getPropertyByKey(key).isExperimental()) {
+ printConfLine(output, "default", key, dfault);
+ printed = true;
+ }
printConfLine(output, "namespace", printed ? " @override" : key,
curVal);
}
}
diff --git
a/test/src/main/java/org/apache/accumulo/test/NewTableConfigurationIT.java
b/test/src/main/java/org/apache/accumulo/test/NewTableConfigurationIT.java
index 95af781007..ae4e1a5685 100644
--- a/test/src/main/java/org/apache/accumulo/test/NewTableConfigurationIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/NewTableConfigurationIT.java
@@ -521,7 +521,8 @@ public class NewTableConfigurationIT extends
SharedMiniClusterBase {
* Verify the expected iterator properties exist.
*/
private void verifyIterators(AccumuloClient client, String tablename,
String[] values,
- boolean withDefaultIts) throws AccumuloException, TableNotFoundException
{
+ boolean withDefaultIts)
+ throws AccumuloSecurityException, AccumuloException,
TableNotFoundException {
Map<String,String> expected = new TreeMap<>();
if (withDefaultIts) {
expected.put("table.iterator.scan.vers",
