You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by dl...@apache.org on 2022/10/14 11:16:44 UTC
[accumulo] branch main updated: Print warning in shell when user does not have permission to get config (#3015)
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 <dl...@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",