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..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 @@ -68,12 +68,10 @@ private String getHeaderValue() { headerValue = this.headerPrefix.trim() + " "; } headerValue += this.value; - return headerValue.trim(); } private URI appendApiKeyParamToUrl(URI oldUrl) { - return UriComponentsBuilder.newInstance() .scheme(oldUrl.getScheme()) .host(oldUrl.getHost()) @@ -81,6 +79,7 @@ private URI appendApiKeyParamToUrl(URI oldUrl) { .path(oldUrl.getPath()) .query(oldUrl.getQuery()) .queryParam(label, value) + .encode() .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();