You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/10/08 17:07:58 UTC
[GitHub] [accumulo] cshannon opened a new pull request, #3012: Update ConfigCommand to check site and system permissions
cshannon opened a new pull request, #3012:
URL: https://github.com/apache/accumulo/pull/3012
The ConfigCommand has been updated to handle security exceptions for users that don't have access to site/system configs and will instead just print the default config and the override of either namespace or table property.
This is part 2 of #3010
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on pull request #3012: Update ConfigCommand to check site and system permissions
Posted by GitBox <gi...@apache.org>.
cshannon commented on PR #3012:
URL: https://github.com/apache/accumulo/pull/3012#issuecomment-1272361510
This is a draft/prototype of just using default configs for determining the level of overrides for properties for users that don't have access to site/system configs after the changes in #2994 .
It probably still needs a bit of work (hence draft status) but I wanted to get some feedback on what people think and if this is what we want to do to solve the problem or if we want to do something else. I probably won't have a lot of time to work on this again until Friday and next weekend so if someone wants to take this over and polish it up feel free as it's a blocker for 2.1.0, assuming this is the direction we want to go in.
If this is not the strategy we want then as I mentioned previously in #3010, the other 2 possibilities would be to move the logic for determining the overrides for the config to the server (which would require more extensive API and RPC changes which it's a bit late for with 2.1.0) or to just revert back the permissions checks entirely and then make the changes for 3.0 (either client side like this or handle it server side)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on pull request #3012: Update ConfigCommand to check site and system permissions
Posted by GitBox <gi...@apache.org>.
cshannon commented on PR #3012:
URL: https://github.com/apache/accumulo/pull/3012#issuecomment-1276417168
Closing this as it looks like we will be going with #3015
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #3012: Update ConfigCommand to check site and system permissions
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #3012:
URL: https://github.com/apache/accumulo/pull/3012#discussion_r992749223
##########
shell/src/main/java/org/apache/accumulo/shell/commands/ConfigCommand.java:
##########
@@ -155,30 +156,56 @@ public int execute(final String fullCommand, final CommandLine cl, final Shell s
} else {
// display properties
final TreeMap<String,String> systemConfig = new TreeMap<>();
- systemConfig
- .putAll(shellState.getAccumuloClient().instanceOperations().getSystemConfiguration());
+ final TreeMap<String,String> siteConfig = new TreeMap<>();
+ boolean canReadSystemSiteConfig = false;
+
+ try {
+ // The user may not have permissions for system/site configs so handle security exception by
+ // wrapping
+ // in a try/catch block and setting a flag
+ systemConfig
+ .putAll(shellState.getAccumuloClient().instanceOperations().getSystemConfiguration());
+ siteConfig
+ .putAll(shellState.getAccumuloClient().instanceOperations().getSiteConfiguration());
+ canReadSystemSiteConfig = true;
+ } catch (AccumuloException | AccumuloSecurityException e) {
+ if (isSecurityException(e) && (tableName != null || namespace != null)) {
+ Shell.log.debug(
+ "User does not have permission to read system and/or site configuration: {}",
+ e.getMessage());
Review Comment:
I would put this as a warning rather than debug. This could also be separate messages, with information about what shell command to use to grant the appropriate permission.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on pull request #3012: Update ConfigCommand to check site and system permissions
Posted by GitBox <gi...@apache.org>.
cshannon commented on PR #3012:
URL: https://github.com/apache/accumulo/pull/3012#issuecomment-1275251184
Right, it definitely opened up a can of worms with the permissions stuff when locked down so that's why one of the options was to just revert this entirely. @dlmarion has a PR with #3015 which is a lot less changes that could be another option
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon closed pull request #3012: Update ConfigCommand to check site and system permissions
Posted by GitBox <gi...@apache.org>.
cshannon closed pull request #3012: Update ConfigCommand to check site and system permissions
URL: https://github.com/apache/accumulo/pull/3012
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org