Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix: apiKey security issue (#33528)

Browse Source 
## Description
Fixes #30009

### Summary:
This PR addresses the issue of masking sensitive information in query
parameters or headers based on the authentication type selected by the
user. The changes ensure that sensitive data is properly masked before
sending back as response.

### Changes:
- RequestCaptureFilter.java
Added logic to check the authentication type and mask the appropriate
query parameters or headers.

### Testing:
Verified that the masking functionality works as expected for API_KEY
authentication types.

## Automation

/test datasource

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/9203547387>
> Commit: 9a7fc9cc1942ddc61c54f6ae9451706ad527f49c
> Cypress dashboard url: <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9203547387&attempt=2"
target="_blank">Click here!</a>

<!-- end of auto-generated comment: Cypress test results  -->







## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This commit is contained in:
Rishabh Rathod 2024-05-23 13:45:16 +05:30 committed by GitHub
parent 4e7216f6b9
commit b59838c03a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 159 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RequestCaptureFilter.java vendored
View File

@ -2,6 +2,9 @@ package com.appsmith.external.helpers.restApiUtils.helpers;
import com.appsmith.external.models.ActionConfiguration;
import com.appsmith.external.models.ActionExecutionRequest;
import com.appsmith.external.models.ApiKeyAuth;
import com.appsmith.external.models.AuthenticationDTO;
import com.appsmith.external.models.DatasourceConfiguration;
import com.appsmith.external.models.Property;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.Getter;
@ -18,11 +21,13 @@ import org.springframework.web.reactive.function.client.ExchangeFunction;
import reactor.core.publisher.Mono;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.StringJoiner;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
@ -39,6 +44,7 @@ public class RequestCaptureFilter implements ExchangeFilterFunction {
private ClientRequest request;
private final ObjectMapper objectMapper;
private final BodyReceiver bodyReceiver = new BodyReceiver();
private final String MASKED_VALUE = "****";
public RequestCaptureFilter(ObjectMapper objectMapper) {
this.objectMapper = objectMapper;
@ -50,8 +56,44 @@ public class RequestCaptureFilter implements ExchangeFilterFunction {
return next.exchange(request);
}
private URI createURIWithMaskedQueryParam(URI uriToMask, String queryParamKeyToMask) {
String query = uriToMask.getQuery();
if (query == null) {
return uriToMask;
}
String[] queryParams = query.split("&");
StringJoiner newQuery = new StringJoiner("&");
for (String queryParam : queryParams) {
String[] keyValuePair = queryParam.split("=");
if (queryParamKeyToMask.equals(keyValuePair[0])) {
// fix when value is not present
if (keyValuePair.length > 1) {
keyValuePair[1] = MASKED_VALUE;
}
}
newQuery.add(keyValuePair[0] + "=" + keyValuePair[1]);
}
try {
return new URI(
uriToMask.getScheme(),
uriToMask.getUserInfo(),
uriToMask.getHost(),
uriToMask.getPort(),
uriToMask.getPath(),
newQuery.toString(),
uriToMask.getRawFragment());
} catch (URISyntaxException e) {
return uriToMask;
}
}
public ActionExecutionRequest populateRequestFields(
ActionExecutionRequest existing, boolean isBodySentWithApiRequest) {
ActionExecutionRequest existing,
boolean isBodySentWithApiRequest,
DatasourceConfiguration datasourceConfiguration) {
final ActionExecutionRequest actionExecutionRequest = new ActionExecutionRequest();
if (request == null) {
@ -60,14 +102,34 @@ public class RequestCaptureFilter implements ExchangeFilterFunction {
return actionExecutionRequest;
}
actionExecutionRequest.setUrl(request.url().toString());
actionExecutionRequest.setHttpMethod(request.method());
AtomicBoolean isMultipart = new AtomicBoolean(false);
String headerToMask = "";
String URLString = request.url().toString();
AuthenticationDTO authentication = datasourceConfiguration.getAuthentication();
// if authenticationType is api_key then check the addTo method and mask the api_key set by user accordingly.
if (authentication instanceof ApiKeyAuth auth) {
if (auth.getAddTo() != null) {
if (auth.getAddTo().equals(ApiKeyAuth.Type.HEADER)) {
headerToMask = auth.getLabel();
}
if (auth.getAddTo().equals(ApiKeyAuth.Type.QUERY_PARAMS)) {
URI maskedURI = createURIWithMaskedQueryParam(request.url(), auth.getLabel());
URLString = maskedURI.toString();
}
}
}
String finalHeaderToMask = headerToMask;
MultiValueMap<String, String> headers =
CollectionUtils.toMultiValueMap(new LinkedCaseInsensitiveMap<>(8, Locale.ENGLISH));
AtomicBoolean isMultipart = new AtomicBoolean(false);
request.headers().forEach((header, value) -> {
if (HttpHeaders.AUTHORIZATION.equalsIgnoreCase(header) || "api_key".equalsIgnoreCase(header)) {
headers.add(header, "****");
if (HttpHeaders.AUTHORIZATION.equalsIgnoreCase(header)
|| "api_key".equalsIgnoreCase(header)
|| finalHeaderToMask.equals(header)) {
headers.add(header, MASKED_VALUE);
} else {
headers.addAll(header, value);
}
@ -76,8 +138,10 @@ public class RequestCaptureFilter implements ExchangeFilterFunction {
isMultipart.set(true);
}
});
actionExecutionRequest.setHeaders(objectMapper.valueToTree(headers));
actionExecutionRequest.setUrl(URLString);
actionExecutionRequest.setHeaders(objectMapper.valueToTree(headers));
actionExecutionRequest.setHttpMethod(request.method());
actionExecutionRequest.setRequestParams(existing.getRequestParams());
actionExecutionRequest.setExecutionParameters(existing.getExecutionParameters());
actionExecutionRequest.setProperties(existing.getProperties());
@ -90,7 +154,6 @@ public class RequestCaptureFilter implements ExchangeFilterFunction {
} else {
actionExecutionRequest.setBody(existing.getBody());
}
return actionExecutionRequest;
}
@ -137,7 +200,7 @@ public class RequestCaptureFilter implements ExchangeFilterFunction {
.filter(property -> property.getKey() != null)
.forEach(property -> bodyDataMap.put(property.getKey(), property.getValue()));
actionExecutionRequest.setBody(bodyDataMap);
} else if (MediaType.MULTIPART_FORM_DATA_VALUE.equals(reqContentType.get())) {
} else if (MULTIPART_FORM_DATA_VALUE.equals(reqContentType.get())) {
final List<Property> bodyFormData = actionConfiguration.getBodyFormData();
Map<String, Object> bodyDataMap = new HashMap<>();
bodyFormData

5
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RestAPIActivateUtils.java vendored
View File

@ -71,7 +71,8 @@ public class RestAPIActivateUtils {
ObjectMapper objectMapper,
Set<String> hintMessages,
ActionExecutionResult errorResult,
RequestCaptureFilter requestCaptureFilter) {
RequestCaptureFilter requestCaptureFilter,
DatasourceConfiguration datasourceConfiguration) {
return httpCall(client, httpMethod, uri, requestBody, 0)
.flatMap(clientResponse -> clientResponse.toEntity(byte[].class))
.map(stringResponseEntity -> {
@ -94,7 +95,7 @@ public class RestAPIActivateUtils {
// Set the request fields
boolean isBodySentWithApiRequest = requestBody == null ? false : true;
result.setRequest(requestCaptureFilter.populateRequestFields(
actionExecutionRequest, isBodySentWithApiRequest));
actionExecutionRequest, isBodySentWithApiRequest, datasourceConfiguration));
result.setStatusCode(statusCode.toString());

5
app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/plugins/GraphQLPlugin.java vendored
View File

@ -281,11 +281,12 @@ public class GraphQLPlugin extends BasePlugin {
objectMapper,
hintMessages,
errorResult,
requestCaptureFilter)
requestCaptureFilter,
datasourceConfiguration)
.onErrorResume(error -> {
boolean isBodySentWithApiRequest = requestBodyObj == null ? false : true;
errorResult.setRequest(requestCaptureFilter.populateRequestFields(
actionExecutionRequest, isBodySentWithApiRequest));
actionExecutionRequest, isBodySentWithApiRequest, datasourceConfiguration));
errorResult.setIsExecutionSuccess(false);
if (!(error instanceof AppsmithPluginException)) {
error = new AppsmithPluginException(

5
app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java vendored
View File

@ -202,11 +202,12 @@ public class RestApiPlugin extends BasePlugin {
objectMapper,
hintMessages,
errorResult,
requestCaptureFilter)
requestCaptureFilter,
datasourceConfiguration)
.onErrorResume(error -> {
boolean isBodySentWithApiRequest = requestBodyObj == null ? false : true;
errorResult.setRequest(requestCaptureFilter.populateRequestFields(
actionExecutionRequest, isBodySentWithApiRequest));
actionExecutionRequest, isBodySentWithApiRequest, datasourceConfiguration));
errorResult.setIsExecutionSuccess(false);
log.debug(
"An error has occurred while trying to run the API query for url: {}, path : {}",

78
app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java vendored
View File

@ -2652,4 +2652,82 @@ public class RestApiPluginTest {
})
.verifyComplete();
}
@Test
public void testMaskingOfAPIKeyAddedToQueryParams() {
DatasourceConfiguration dsConfig = new DatasourceConfiguration();
String baseUrl = String.format("http://%s:%s", mockEndpoint.getHostName(), mockEndpoint.getPort());
dsConfig.setUrl(baseUrl);
mockEndpoint.enqueue(new MockResponse().setBody("{}").addHeader("Content-Type", "application/json"));
AuthenticationDTO authenticationDTO =
new ApiKeyAuth(ApiKeyAuth.Type.QUERY_PARAMS, "secret", null, "secret_value");
dsConfig.setAuthentication(authenticationDTO);
ActionConfiguration actionConfig = new ActionConfiguration();
actionConfig.setHeaders(List.of(
new Property("content-type", "application/json"),
new Property(HttpHeaders.AUTHORIZATION, "auth-value")));
actionConfig.setAutoGeneratedHeaders(List.of(new Property("content-type", "application/json")));
actionConfig.setHttpMethod(HttpMethod.POST);
String requestBody = "{\"key\":\"value\"}";
actionConfig.setBody(requestBody);
final APIConnection apiConnection =
pluginExecutor.datasourceCreate(dsConfig).block();
Mono<ActionExecutionResult> resultMono =
pluginExecutor.executeParameterized(apiConnection, new ExecuteActionDTO(), dsConfig, actionConfig);
StepVerifier.create(resultMono)
.assertNext(result -> {
assertTrue(result.getIsExecutionSuccess());
assertNotNull(result.getRequest().getBody());
final String resultURL = result.getRequest().getUrl();
assertEquals(baseUrl + "?secret=****", resultURL);
})
.verifyComplete();
}
@Test
public void testMaskingOfAPIKeyAddedToHeaders() {
DatasourceConfiguration dsConfig = new DatasourceConfiguration();
String baseUrl = String.format("http://%s:%s", mockEndpoint.getHostName(), mockEndpoint.getPort());
dsConfig.setUrl(baseUrl);
mockEndpoint.enqueue(new MockResponse().setBody("{}").addHeader("Content-Type", "application/json"));
AuthenticationDTO authenticationDTO = new ApiKeyAuth(ApiKeyAuth.Type.HEADER, "secret", null, "secret_value");
dsConfig.setAuthentication(authenticationDTO);
ActionConfiguration actionConfig = new ActionConfiguration();
actionConfig.setHeaders(List.of(
new Property("content-type", "application/json"),
new Property(HttpHeaders.AUTHORIZATION, "auth-value")));
actionConfig.setAutoGeneratedHeaders(List.of(new Property("content-type", "application/json")));
actionConfig.setHttpMethod(HttpMethod.POST);
String requestBody = "{\"key\":\"value\"}";
actionConfig.setBody(requestBody);
final APIConnection apiConnection =
pluginExecutor.datasourceCreate(dsConfig).block();
Mono<ActionExecutionResult> resultMono =
pluginExecutor.executeParameterized(apiConnection, new ExecuteActionDTO(), dsConfig, actionConfig);
StepVerifier.create(resultMono)
.assertNext(result -> {
assertTrue(result.getIsExecutionSuccess());
assertNotNull(result.getRequest().getBody());
final Iterator<Map.Entry<String, JsonNode>> fields =
((ObjectNode) result.getRequest().getHeaders()).fields();
fields.forEachRemaining(field -> {
if ("secret".equalsIgnoreCase(field.getKey())) {
assertEquals("****", field.getValue().get(0).asText());
}
});
})
.verifyComplete();
}
}