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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

fix: encode queryParam value and fix parsing logic for queryParams #33720

Merged
merged 3 commits into from
May 27, 2024
Merged

fix: encode queryParam value and fix parsing logic for queryParams #33720

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7a952ee
fix: encode queryParam value and fix parsing logic for queryParams
rishabhrathod01 May 24, 2024
128696b
use URIComponentBuilder encode method instead of URLEncoder
rishabhrathod01 May 27, 2024
602b0f5
Merge branch 'release' into fix/ApiKeySecretInParamFlaw
rishabhrathod01 May 27, 2024
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
8 changes: 6 additions & 2 deletions ...ain/java/com/appsmith/external/helpers/restApiUtils/connections/ApiKeyAuthentication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import reactor.core.publisher.Mono;

import java.net.URI;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;

@Setter
@Getter
Expand Down Expand Up @@ -68,19 +70,21 @@ private String getHeaderValue() {
headerValue = this.headerPrefix.trim() + " ";
}
headerValue += this.value;

return headerValue.trim();
}

private URI appendApiKeyParamToUrl(URI oldUrl) {

// encode to value to avoid error for value having special chars like "secret&another=123" here without encoding
// it will result it creating 2 queryParam.
String encodedValue = URLEncoder.encode(value, StandardCharsets.UTF_8);
return UriComponentsBuilder.newInstance()
.scheme(oldUrl.getScheme())
.host(oldUrl.getHost())
.port(oldUrl.getPort())
.path(oldUrl.getPath())
.query(oldUrl.getQuery())
.queryParam(label, value)
.queryParam(label, encodedValue)
rishabhrathod01 marked this conversation as resolved.
Show resolved Hide resolved
.fragment(oldUrl.getFragment())
.build()
.toUri();
Expand Down
60 changes: 23 additions & 37 deletions ...rc/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RequestCaptureFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,15 @@
import org.springframework.web.reactive.function.client.ClientResponse;
import org.springframework.web.reactive.function.client.ExchangeFilterFunction;
import org.springframework.web.reactive.function.client.ExchangeFunction;
import org.springframework.web.util.UriComponentsBuilder;
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;

Expand All @@ -44,7 +43,7 @@ public class RequestCaptureFilter implements ExchangeFilterFunction {
private ClientRequest request;
private final ObjectMapper objectMapper;
private final BodyReceiver bodyReceiver = new BodyReceiver();
private final String MASKED_VALUE = "****";
private static final String MASKED_VALUE = "****";

public RequestCaptureFilter(ObjectMapper objectMapper) {
this.objectMapper = objectMapper;
Expand All @@ -56,38 +55,29 @@ public RequestCaptureFilter(ObjectMapper objectMapper) {
return next.exchange(request);
}

private URI createURIWithMaskedQueryParam(URI uriToMask, String queryParamKeyToMask) {
private static String maskQueryParamInURL(URI uriToMask, String queryParamKeyToMask) {
rishabhrathod01 marked this conversation as resolved.
Show resolved Hide resolved
MultiValueMap<String, String> queryParams =
UriComponentsBuilder.fromUri(uriToMask).build().getQueryParams();
UriComponentsBuilder uriBuilder = UriComponentsBuilder.fromUri(uriToMask);
// Clear the query parameters from the builder
uriBuilder.replaceQuery(null);
// Loop through the query parameters
for (Map.Entry<String, List<String>> entry : queryParams.entrySet()) {
String key = entry.getKey();
List<String> values = entry.getValue();

// If the key matches the queryParamKeyToMask, mask its values
if (key.equals(queryParamKeyToMask)) {
values = values.stream().map(value -> MASKED_VALUE).toList();
}

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;
}
// Add the (possibly masked) parameters back to the builder
for (String value : values) {
uriBuilder.queryParam(key, 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;
}
return uriBuilder.build().toUriString();
rishabhrathod01 marked this conversation as resolved.
Show resolved Hide resolved
}

public ActionExecutionRequest populateRequestFields(
Expand All @@ -105,7 +95,6 @@ public ActionExecutionRequest populateRequestFields(
AtomicBoolean isMultipart = new AtomicBoolean(false);

String headerToMask = "";

String URLString = request.url().toString();

AuthenticationDTO authentication = datasourceConfiguration.getAuthentication();
Expand All @@ -117,18 +106,15 @@ public ActionExecutionRequest populateRequestFields(
headerToMask = auth.getLabel();
}
if (auth.getAddTo().equals(ApiKeyAuth.Type.QUERY_PARAMS)) {
URI maskedURI = createURIWithMaskedQueryParam(request.url(), auth.getLabel());
URLString = maskedURI.toString();
URLString = maskQueryParamInURL(request.url(), auth.getLabel());
}
}
}
String finalHeaderToMask = headerToMask;
MultiValueMap<String, String> headers =
CollectionUtils.toMultiValueMap(new LinkedCaseInsensitiveMap<>(8, Locale.ENGLISH));
request.headers().forEach((header, value) -> {
if (HttpHeaders.AUTHORIZATION.equalsIgnoreCase(header)
|| "api_key".equalsIgnoreCase(header)
|| finalHeaderToMask.equals(header)) {
if (HttpHeaders.AUTHORIZATION.equalsIgnoreCase(header) || finalHeaderToMask.equals(header)) {
headers.add(header, MASKED_VALUE);
} else {
headers.addAll(header, value);
Expand Down
2 changes: 1 addition & 1 deletion .../appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2662,7 +2662,7 @@ public void testMaskingOfAPIKeyAddedToQueryParams() {
mockEndpoint.enqueue(new MockResponse().setBody("{}").addHeader("Content-Type", "application/json"));

AuthenticationDTO authenticationDTO =
new ApiKeyAuth(ApiKeyAuth.Type.QUERY_PARAMS, "secret", null, "secret_value");
new ApiKeyAuth(ApiKeyAuth.Type.QUERY_PARAMS, "secret", null, "secret&another=value");
dsConfig.setAuthentication(authenticationDTO);

ActionConfiguration actionConfig = new ActionConfiguration();
Expand Down
Loading