[
https://issues.apache.org/jira/browse/GEODE-3974?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16330733#comment-16330733
]
ASF GitHub Bot commented on GEODE-3974:
---------------------------------------
jinmeiliao commented on a change in pull request #1287: GEODE-3974: function
security improvement
URL: https://github.com/apache/geode/pull/1287#discussion_r162397689
##########
File path:
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecution.java
##########
@@ -31,138 +34,149 @@
import org.apache.geode.internal.ClassPathLoader;
import org.apache.geode.internal.InternalEntity;
import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.security.AuthenticationRequiredException;
/**
* @since GemFire 7.0
*/
public class UserFunctionExecution implements Function<Object[]>,
InternalEntity {
public static final String ID = UserFunctionExecution.class.getName();
+ private static Logger logger = LogService.getLogger();
private static final long serialVersionUID = 1L;
@Override
public void execute(FunctionContext<Object[]> context) {
Cache cache = context.getCache();
DistributedMember member =
cache.getDistributedSystem().getDistributedMember();
- try {
- String[] functionArgs = null;
- Object[] args = context.getArguments();
- if (args == null) {
- context.getResultSender().lastResult(new
CliFunctionResult(member.getId(), false,
- CliStrings.EXECUTE_FUNCTION__MSG__COULD_NOT_RETRIEVE_ARGUMENTS));
- return;
- }
+ String[] functionArgs = null;
+ Object[] args = context.getArguments();
+ if (args == null) {
+ context.getResultSender().lastResult(new
CliFunctionResult(member.getId(), false,
+ CliStrings.EXECUTE_FUNCTION__MSG__COULD_NOT_RETRIEVE_ARGUMENTS));
+ return;
+ }
- String functionId = ((String) args[0]);
- String filterString = ((String) args[1]);
- String resultCollectorName = ((String) args[2]);
- String argumentsString = ((String) args[3]);
- String onRegion = ((String) args[4]);
- Properties credentials = (Properties) args[5];
+ String functionId = ((String) args[0]);
+ String filterString = ((String) args[1]);
+ String resultCollectorName = ((String) args[2]);
+ String argumentsString = ((String) args[3]);
+ String onRegion = ((String) args[4]);
+ Properties credentials = (Properties) args[5];
- SecurityService securityService = ((InternalCache)
context.getCache()).getSecurityService();
+ SecurityService securityService = ((InternalCache)
context.getCache()).getSecurityService();
+ boolean loginNeeded = false;
+ try {
+ // if the function is executed on a server with jmx-manager that user is
already logged into
+ // then we do not need to do login/logout here.
+ Subject subject = securityService.getSubject();
+ loginNeeded = subject == null || !subject.isAuthenticated();
+ } catch (AuthenticationRequiredException e) {
+ loginNeeded = true;
+ }
- try {
+ try {
+ if (loginNeeded) {
securityService.login(credentials);
+ }
- if (argumentsString != null && argumentsString.length() > 0) {
- functionArgs = argumentsString.split(",");
- }
- Set<String> filters = new HashSet<>();
- ResultCollector resultCollectorInstance = null;
- if (resultCollectorName != null && resultCollectorName.length() > 0) {
- resultCollectorInstance = (ResultCollector)
ClassPathLoader.getLatest()
- .forName(resultCollectorName).newInstance();
- }
- if (filterString != null && filterString.length() > 0) {
- filters.add(filterString);
- }
+ if (argumentsString != null && argumentsString.length() > 0) {
+ functionArgs = argumentsString.split(",");
+ }
+ Set<String> filters = new HashSet<>();
+ ResultCollector resultCollectorInstance = null;
+ if (resultCollectorName != null && resultCollectorName.length() > 0) {
+ resultCollectorInstance = (ResultCollector) ClassPathLoader.getLatest()
+ .forName(resultCollectorName).newInstance();
+ }
+ if (filterString != null && filterString.length() > 0) {
+ filters.add(filterString);
+ }
- Function<?> function = FunctionService.getFunction(functionId);
- if (function == null) {
- context.getResultSender()
- .lastResult(new CliFunctionResult(member.getId(), false,
- (CliStrings.format(
-
CliStrings.EXECUTE_FUNCTION__MSG__DOES_NOT_HAVE_FUNCTION_0_REGISTERED,
- functionId))));
- return;
- }
+ Function<?> function = FunctionService.getFunction(functionId);
+ if (function == null) {
+ context.getResultSender()
+ .lastResult(new CliFunctionResult(member.getId(), false,
+ (CliStrings.format(
+
CliStrings.EXECUTE_FUNCTION__MSG__DOES_NOT_HAVE_FUNCTION_0_REGISTERED,
+ functionId))));
+ return;
+ }
- // security check
-
function.getRequiredPermissions(onRegion).forEach(securityService::authorize);
+ // security check
+
function.getRequiredPermissions(onRegion).forEach(securityService::authorize);
- Execution execution = null;
- if (onRegion != null && onRegion.length() > 0) {
- Region region = cache.getRegion(onRegion);
- if (region == null) {
- context.getResultSender().lastResult(
- new CliFunctionResult(member.getId(), false, onRegion + " does
not exist"));
- return;
- }
- execution = FunctionService.onRegion(region);
- } else {
- execution = FunctionService.onMember(member);
- }
-
- if (execution == null) {
- context.getResultSender()
- .lastResult(new CliFunctionResult(member.getId(), false,
- CliStrings.format(
-
CliStrings.EXECUTE_FUNCTION__MSG__ERROR_IN_EXECUTING_0_ON_MEMBER_1_ON_REGION_2_DETAILS_3,
- functionId, member.getId(), onRegion,
-
CliStrings.EXECUTE_FUNCTION__MSG__ERROR_IN_RETRIEVING_EXECUTOR)));
+ Execution execution = null;
+ if (onRegion != null && onRegion.length() > 0) {
+ Region region = cache.getRegion(onRegion);
+ if (region == null) {
+ context.getResultSender().lastResult(
+ new CliFunctionResult(member.getId(), false, onRegion + " does
not exist"));
return;
}
+ execution = FunctionService.onRegion(region);
+ } else {
+ execution = FunctionService.onMember(member);
+ }
- if (resultCollectorInstance != null) {
- execution = execution.withCollector(resultCollectorInstance);
- }
+ if (execution == null) {
+ context.getResultSender()
+ .lastResult(new CliFunctionResult(member.getId(), false,
+ CliStrings.format(
+
CliStrings.EXECUTE_FUNCTION__MSG__ERROR_IN_EXECUTING_0_ON_MEMBER_1_ON_REGION_2_DETAILS_3,
+ functionId, member.getId(), onRegion,
+
CliStrings.EXECUTE_FUNCTION__MSG__ERROR_IN_RETRIEVING_EXECUTOR)));
+ return;
+ }
- if (functionArgs != null && functionArgs.length > 0) {
- execution = execution.setArguments(functionArgs);
- }
- if (filters.size() > 0) {
- execution = execution.withFilter(filters);
- }
+ if (resultCollectorInstance != null) {
+ execution = execution.withCollector(resultCollectorInstance);
+ }
- List<Object> results = (List<Object>)
execution.execute(function.getId()).getResult();
- List<String> resultMessage = new ArrayList<>();
- boolean functionSuccess = true;
-
- if (results != null) {
- for (Object resultObj : results) {
- if (resultObj != null) {
- if (resultObj instanceof Exception) {
- resultMessage.add(((Exception) resultObj).getMessage());
- functionSuccess = false;
- } else {
- resultMessage.add(resultObj.toString());
- }
+ if (functionArgs != null && functionArgs.length > 0) {
+ execution = execution.setArguments(functionArgs);
+ }
+ if (filters.size() > 0) {
+ execution = execution.withFilter(filters);
+ }
+
+ List<Object> results = (List<Object>)
execution.execute(function.getId()).getResult();
+ List<String> resultMessage = new ArrayList<>();
+ boolean functionSuccess = true;
+
+ if (results != null) {
+ for (Object resultObj : results) {
+ if (resultObj != null) {
+ if (resultObj instanceof Exception) {
+ resultMessage.add(((Exception) resultObj).getMessage());
+ functionSuccess = false;
+ } else {
+ resultMessage.add(resultObj.toString());
}
}
}
- context.getResultSender().lastResult(
- new CliFunctionResult(member.getId(), functionSuccess,
resultMessage.toString()));
-
- } catch (ClassNotFoundException | IllegalAccessException |
InstantiationException e) {
- context.getResultSender()
- .lastResult(new CliFunctionResult(member.getId(), false,
- CliStrings.format(
-
CliStrings.EXECUTE_FUNCTION__MSG__RESULT_COLLECTOR_0_NOT_FOUND_ERROR_1,
- resultCollectorName, e.getMessage())));
- } catch (Exception e) {
- context.getResultSender().lastResult(
- new CliFunctionResult(member.getId(), false, "Exception: " +
e.getMessage()));
- } finally {
- securityService.logout();
}
+ context.getResultSender().lastResult(
+ new CliFunctionResult(member.getId(), functionSuccess,
resultMessage.toString()));
- } catch (Exception ex) {
+ } catch (ClassNotFoundException | IllegalAccessException |
InstantiationException e) {
+ context.getResultSender()
+ .lastResult(new CliFunctionResult(member.getId(), false,
+ CliStrings.format(
+
CliStrings.EXECUTE_FUNCTION__MSG__RESULT_COLLECTOR_0_NOT_FOUND_ERROR_1,
+ resultCollectorName, e.getMessage())));
+ } catch (Exception e) {
+ logger.error("error executing function " + functionId, e);
context.getResultSender()
- .lastResult(new CliFunctionResult(member.getId(), false,
ex.getMessage()));
+ .lastResult(new CliFunctionResult(member.getId(), false, "Exception:
" + e.getMessage()));
+ } finally {
+ if (loginNeeded) {
+ securityService.logout();
Review comment:
fixed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> improve permission for Internal functions
> -----------------------------------------
>
> Key: GEODE-3974
> URL: https://issues.apache.org/jira/browse/GEODE-3974
> Project: Geode
> Issue Type: Bug
> Components: docs, management
> Reporter: Jinmei Liao
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.5.0
>
>
> Internal functions needs to be updated to require appropriate permissions
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
