From 7a952ee07978b932eaa3824e1f3295990c27edc7 Mon Sep 17 00:00:00 2001 From: Rishabh-Rathod Date: Sat, 25 May 2024 01:27:12 +0530 Subject: [PATCH 1/2] fix: encode queryParam value and fix parsing logic for queryParams --- .../connections/ApiKeyAuthentication.java | 8 ++- .../helpers/RequestCaptureFilter.java | 60 +++++++------------ .../external/plugins/RestApiPluginTest.java | 2 +- 3 files changed, 30 insertions(+), 40 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/connections/ApiKeyAuthentication.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/connections/ApiKeyAuthentication.java index d52c6252f83..2789634fad3 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/connections/ApiKeyAuthentication.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/connections/ApiKeyAuthentication.java @@ -17,6 +17,8 @@ import reactor.core.publisher.Mono; import java.net.URI; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; @Setter @Getter @@ -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) .fragment(oldUrl.getFragment()) .build() .toUri(); diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RequestCaptureFilter.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RequestCaptureFilter.java index 6e5fecf8cac..6534ab7b935 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RequestCaptureFilter.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RequestCaptureFilter.java @@ -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; @@ -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; @@ -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) { + MultiValueMap 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> entry : queryParams.entrySet()) { + String key = entry.getKey(); + List 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(); } public ActionExecutionRequest populateRequestFields( @@ -105,7 +95,6 @@ public ActionExecutionRequest populateRequestFields( AtomicBoolean isMultipart = new AtomicBoolean(false); String headerToMask = ""; - String URLString = request.url().toString(); AuthenticationDTO authentication = datasourceConfiguration.getAuthentication(); @@ -117,8 +106,7 @@ 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()); } } } @@ -126,9 +114,7 @@ public ActionExecutionRequest populateRequestFields( MultiValueMap 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); diff --git a/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java b/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java index 26d22cdcc4b..558fc6e0f1b 100644 --- a/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java +++ b/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java @@ -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(); From 128696b41b22b1ba94cdf30ea784d381b620f71c Mon Sep 17 00:00:00 2001 From: Rishabh-Rathod Date: Mon, 27 May 2024 12:31:58 +0530 Subject: [PATCH 2/2] use URIComponentBuilder encode method instead of URLEncoder --- .../restApiUtils/connections/ApiKeyAuthentication.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/connections/ApiKeyAuthentication.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/connections/ApiKeyAuthentication.java index 2789634fad3..83cb8a5eb5c 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/connections/ApiKeyAuthentication.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/connections/ApiKeyAuthentication.java @@ -17,8 +17,6 @@ import reactor.core.publisher.Mono; import java.net.URI; -import java.net.URLEncoder; -import java.nio.charset.StandardCharsets; @Setter @Getter @@ -74,17 +72,14 @@ private String getHeaderValue() { } 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, encodedValue) + .queryParam(label, value) + .encode() .fragment(oldUrl.getFragment()) .build() .toUri();