Skip to content

Commit

Permalink
Mask password fields of inputs returned by the REST API. (#5432)
Browse files Browse the repository at this point in the history
* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(Cherry-Pick from a562a33)
  • Loading branch information
dennisoelkers authored and mpfz0r committed Jan 8, 2019
1 parent 81cc4c7 commit e0e05c1
Show file tree
Hide file tree
Showing 3 changed files with 374 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,23 @@
package org.graylog2.rest.resources.system.inputs;

import com.codahale.metrics.annotation.Timed;
import com.google.common.annotations.VisibleForTesting;
import io.swagger.annotations.Api;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiParam;
import io.swagger.annotations.ApiResponse;
import io.swagger.annotations.ApiResponses;
import org.apache.shiro.authz.annotation.RequiresAuthentication;
import org.apache.shiro.authz.annotation.RequiresPermissions;
import org.elasticsearch.common.Strings;
import org.graylog2.audit.AuditEventTypes;
import org.graylog2.audit.jersey.AuditEvent;
import org.graylog2.inputs.Input;
import org.graylog2.inputs.InputService;
import org.graylog2.plugin.configuration.ConfigurationException;
import org.graylog2.plugin.configuration.ConfigurationRequest;
import org.graylog2.plugin.configuration.fields.ConfigurationField;
import org.graylog2.plugin.configuration.fields.TextField;
import org.graylog2.plugin.database.ValidationException;
import org.graylog2.plugin.inputs.MessageInput;
import org.graylog2.rest.models.system.inputs.requests.InputCreateRequest;
Expand Down Expand Up @@ -59,6 +64,8 @@
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import java.net.URI;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -91,7 +98,7 @@ public InputsResource(InputService inputService, MessageInputFactory messageInpu
@ApiResponse(code = 404, message = "No such input.")
})
public InputSummary get(@ApiParam(name = "inputId", required = true)
@PathParam("inputId") String inputId) throws org.graylog2.database.NotFoundException {
@PathParam("inputId") String inputId) throws org.graylog2.database.NotFoundException {
checkPermission(RestPermissions.INPUTS_READ, inputId);

final Input input = inputService.find(inputId);
Expand Down Expand Up @@ -200,6 +207,9 @@ public Response update(@ApiParam(name = "JSON body", required = true) @Valid @No
private InputSummary getInputSummary(Input input) {
final InputDescription inputDescription = this.availableInputs.get(input.getType());
final String name = inputDescription != null ? inputDescription.getName() : "Unknown Input (" + input.getType() + ")";
final ConfigurationRequest configurationRequest = inputDescription != null ? inputDescription.getConfigurationRequest() : null;
final Map<String, Object> configuration = isPermitted(RestPermissions.INPUTS_EDIT, input.getId()) ?
input.getConfiguration() : maskPasswordsInConfiguration(input.getConfiguration(), configurationRequest);
return InputSummary.create(input.getTitle(),
input.isGlobal(),
name,
Expand All @@ -208,9 +218,34 @@ private InputSummary getInputSummary(Input input) {
input.getCreatedAt(),
input.getType(),
input.getCreatorUserId(),
input.getConfiguration(),
configuration,
input.getStaticFields(),
input.getNodeId()
);
}

@VisibleForTesting
Map<String, Object> maskPasswordsInConfiguration(Map<String, Object> configuration, ConfigurationRequest configurationRequest) {
if (configuration == null || configurationRequest == null) {
return configuration;
}
return configuration.entrySet()
.stream()
.collect(
HashMap::new,
(map, entry) -> {
final ConfigurationField field = configurationRequest.getField(entry.getKey());
if (field instanceof TextField) {
final TextField textField = (TextField) field;
if (textField.getAttributes().contains(TextField.Attribute.IS_PASSWORD.toString().toLowerCase(Locale.ENGLISH))
&& !Strings.isNullOrEmpty((String) entry.getValue())) {
map.put(entry.getKey(), "<password set>");
return;
}
}
map.put(entry.getKey(), entry.getValue());
},
HashMap::putAll
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.graylog2.shared.inputs;

import org.graylog2.plugin.configuration.ConfigurationRequest;
import org.graylog2.plugin.inputs.MessageInput;

import java.util.Map;
Expand Down Expand Up @@ -44,4 +45,8 @@ public String getLinkToDocs() {
public Map<String, Map<String, Object>> getRequestedConfiguration() {
return config.combinedRequestedConfiguration().asList();
}

public ConfigurationRequest getConfigurationRequest() {
return config.combinedRequestedConfiguration();
}
}
Loading

0 comments on commit e0e05c1

Please sign in to comment.