Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hides user and credential field from search response #680

Merged
merged 4 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
### Bug Fixes
- Reset workflow state to initial state after successful deprovision ([#635](https://github.com/opensearch-project/flow-framework/pull/635))
- Silently ignore content on APIs that don't require it ([#639](https://github.com/opensearch-project/flow-framework/pull/639))

- Hide user and credential field from search response ([#680](https://github.com/opensearch-project/flow-framework/pull/680))
owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved
### Infrastructure
### Documentation
### Maintenance
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ private CommonValue() {}
public static final String CREATE_TIME = "create_time";
/** The template field name for the user who created the workflow **/
public static final String USER_FIELD = "user";
/** Path to credential field **/
public static final String PATH_TO_CREDENTIAL_FIELD = "workflows.provision.nodes.user_inputs.credential";
owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved
/** The created time field */
public static final String CREATED_TIME = "created_time";
/** The last updated time field */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,13 @@
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.action.RestResponseListener;
import org.opensearch.script.Script;
import org.opensearch.script.ScriptType;
import org.opensearch.search.builder.SearchSourceBuilder;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import static org.opensearch.core.xcontent.ToXContent.EMPTY_PARAMS;
import static org.opensearch.flowframework.common.CommonValue.GLOBAL_CONTEXT_INDEX;
import static org.opensearch.flowframework.common.FlowFrameworkSettings.FLOW_FRAMEWORK_ENABLED;
import static org.opensearch.flowframework.util.RestHandlerUtils.getSourceContext;

Expand Down Expand Up @@ -93,19 +89,6 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
searchSourceBuilder.seqNoAndPrimaryTerm(true).version(true);
searchSourceBuilder.timeout(flowFrameworkSettings.getRequestTimeout());

// Apply credential filter when searching templates
if (index.equals(GLOBAL_CONTEXT_INDEX)) {
searchSourceBuilder.scriptField(
"filter",
new Script(
ScriptType.INLINE,
"painless",
"def filteredSource = new HashMap(params._source); def workflows = filteredSource.get(\"workflows\"); if (workflows != null) { def provision = workflows.get(\"provision\"); if (provision != null) { def nodes = provision.get(\"nodes\"); if (nodes != null) { for (node in nodes) { def userInputs = node.get(\"user_inputs\"); if (userInputs != null) { userInputs.remove(\"credential\"); } } } } } return filteredSource;",
Collections.emptyMap()
)
);
}

SearchRequest searchRequest = new SearchRequest().source(searchSourceBuilder).indices(index);
return channel -> client.execute(actionType, searchRequest, search(channel));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener<GetW
listener.onFailure(new FlowFrameworkException(errorMessage, RestStatus.NOT_FOUND));
} else {
// Remove any credential from response
owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved
Template template = encryptorUtils.redactTemplateCredentials(Template.parse(response.getSourceAsString()));
Template template = encryptorUtils.redactTemplateSecuredFields(Template.parse(response.getSourceAsString()));
listener.onResponse(new GetWorkflowResponse(template));
}
}, exception -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@
/**
* Applies the given cipher function on template credentials
* @param template the template to process
* @param cipher the encryption/decryption function to apply on credential values
* @param cipherFunction the encryption/decryption function to apply on credential values
* @return template with encrypted credentials
*/
private Template processTemplateCredentials(Template template, Function<String, String> cipherFunction) {
Expand Down Expand Up @@ -204,9 +204,29 @@
* @param template the template
* @return the redacted template
*/
public Template redactTemplateCredentials(Template template) {
public Template redactTemplateSecuredFields(Template template) {
Template updatedTemplate = null;

if (template.getUser() != null) {
updatedTemplate = new Template.Builder(template).name(template.name())
.description(template.description())
.useCase(template.useCase())
.templateVersion(template.templateVersion())
.user(null)
.uiMetadata(template.getUiMetadata())
.compatibilityVersion(template.compatibilityVersion())
.workflows(template.workflows())
.createdTime(template.createdTime())
.lastUpdatedTime(template.lastUpdatedTime())
.lastProvisionedTime(template.lastProvisionedTime())
.build();
owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved
} else {
updatedTemplate = template;

Check warning on line 224 in src/main/java/org/opensearch/flowframework/util/EncryptorUtils.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/flowframework/util/EncryptorUtils.java#L224

Added line #L224 was not covered by tests
}

Map<String, Workflow> processedWorkflows = new HashMap<>();
for (Map.Entry<String, Workflow> entry : template.workflows().entrySet()) {

for (Map.Entry<String, Workflow> entry : updatedTemplate.workflows().entrySet()) {
owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved

List<WorkflowNode> processedNodes = new ArrayList<>();
for (WorkflowNode node : entry.getValue().nodes()) {
Expand All @@ -227,7 +247,7 @@
processedWorkflows.put(entry.getKey(), new Workflow(entry.getValue().userParams(), processedNodes, entry.getValue().edges()));
}

return new Template.Builder(template).workflows(processedWorkflows).build();
return new Template.Builder(updatedTemplate).workflows(processedWorkflows).build();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package org.opensearch.flowframework.util;

import org.apache.commons.lang3.ArrayUtils;
import org.opensearch.core.common.Strings;
import org.opensearch.flowframework.common.CommonValue;
import org.opensearch.rest.RestRequest;
import org.opensearch.search.builder.SearchSourceBuilder;
Expand All @@ -20,7 +21,12 @@
public class RestHandlerUtils {

/** Fields that need to be excluded from the Search Response*/
public static final String[] USER_EXCLUDE = new String[] { CommonValue.USER_FIELD, CommonValue.UI_METADATA_FIELD };
public static final String[] DASHBOARD_EXCLUDES = new String[] {

Check warning on line 24 in src/main/java/org/opensearch/flowframework/util/RestHandlerUtils.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/flowframework/util/RestHandlerUtils.java#L24

Added line #L24 was not covered by tests
CommonValue.USER_FIELD,
CommonValue.UI_METADATA_FIELD,
CommonValue.PATH_TO_CREDENTIAL_FIELD };

public static final String[] EXCLUDES = new String[] { CommonValue.USER_FIELD, CommonValue.PATH_TO_CREDENTIAL_FIELD };

Check warning on line 29 in src/main/java/org/opensearch/flowframework/util/RestHandlerUtils.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/flowframework/util/RestHandlerUtils.java#L29

Added line #L29 was not covered by tests

private RestHandlerUtils() {}

Expand All @@ -35,10 +41,11 @@
// TODO
// 1. check if the request came from dashboard and exclude UI_METADATA
if (searchSourceBuilder.fetchSource() != null) {
String[] newArray = (String[]) ArrayUtils.addAll(searchSourceBuilder.fetchSource().excludes(), USER_EXCLUDE);
String[] newArray = (String[]) ArrayUtils.addAll(searchSourceBuilder.fetchSource().excludes(), DASHBOARD_EXCLUDES);

Check warning on line 44 in src/main/java/org/opensearch/flowframework/util/RestHandlerUtils.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/flowframework/util/RestHandlerUtils.java#L44

Added line #L44 was not covered by tests
amitgalitz marked this conversation as resolved.
Show resolved Hide resolved
return new FetchSourceContext(true, searchSourceBuilder.fetchSource().includes(), newArray);
} else {
return null;
// When user does not set the _source field in search api request, searchSourceBuilder.fetchSource becomes null
amitgalitz marked this conversation as resolved.
Show resolved Hide resolved
return new FetchSourceContext(true, Strings.EMPTY_ARRAY, EXCLUDES);

Check warning on line 48 in src/main/java/org/opensearch/flowframework/util/RestHandlerUtils.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/flowframework/util/RestHandlerUtils.java#L48

Added line #L48 was not covered by tests
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,21 @@ public void testRedactTemplateCredential() {
assertNotNull(node.userInputs().get(CREDENTIAL_FIELD));

// Redact template with credential field
Template redactedTemplate = encryptorUtils.redactTemplateCredentials(testTemplate);
Template redactedTemplate = encryptorUtils.redactTemplateSecuredFields(testTemplate);

// Validate the credential field has been removed
WorkflowNode redactedNode = redactedTemplate.workflows().get("provision").nodes().get(0);
assertNull(redactedNode.userInputs().get(CREDENTIAL_FIELD));
}

public void testRedactTemplateUserField() {
// Confirm user is present in the non-redacted template
assertNotNull(testTemplate.getUser());

// Redact template with user field
Template redactedTemplate = encryptorUtils.redactTemplateSecuredFields(testTemplate);

// Validate the user field has been removed
assertNull(redactedTemplate.getUser());
}
}
Loading