From a105aea811ebcf7f70f1af0630e580d3ae8ce024 Mon Sep 17 00:00:00 2001 From: Alwin Joseph Date: Wed, 19 Aug 2026 12:58:09 +0530 Subject: [PATCH 1/3] SITES-49845: Adds configurable forwarding of client IP and other request headers to Commerce --- .../client/ForwardedHeadersConfig.java | 76 +++++ .../client/ForwardedHeadersConfigService.java | 103 +++++++ .../client/MagentoGraphqlClientImpl.java | 103 +++++++ .../client/MagentoGraphqlClientImplTest.java | 268 ++++++++++++++++++ 4 files changed, 550 insertions(+) create mode 100644 bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfig.java create mode 100644 bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfigService.java diff --git a/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfig.java b/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfig.java new file mode 100644 index 000000000..39d24d360 --- /dev/null +++ b/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfig.java @@ -0,0 +1,76 @@ +/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ~ Copyright 2026 Adobe + ~ + ~ Licensed under the Apache License, Version 2.0 (the "License"); + ~ you may not use this file except in compliance with the License. + ~ You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/ +package com.adobe.cq.commerce.core.components.internal.client; + +import org.osgi.service.metatype.annotations.AttributeDefinition; +import org.osgi.service.metatype.annotations.ObjectClassDefinition; + +/** + * Single configuration covering all forwarding of incoming request headers to the outbound Commerce GraphQL + * request: a master switch, an arbitrary list of headers forwarded as-is, and a dedicated client IP section with + * its own switch and fields (source header/pattern/outbound name), since the client IP needs more than a plain + * name to be forwarded correctly. Every header forwarded through this configuration, generic or client IP, is + * excluded from the GraphQL response cache key, since it carries per-request metadata that does not influence the + * response. Headers on the internal denylist (Authorization, Cookie, Host, Content-Length, etc.) are never + * forwarded, even if configured here. + */ +@ObjectClassDefinition(name = "CIF Forwarded Request Headers Configuration") +public @interface ForwardedHeadersConfig { + + @AttributeDefinition( + name = "Enabled", + description = "Master switch for all header forwarding configured below. Disable to turn everything off without " + + "clearing the individual fields.") + boolean enabled() default false; + + @AttributeDefinition( + name = "Forwarded header names", + description = "Names of incoming request headers whose current value should be forwarded as-is, under the same name, on " + + "the outbound GraphQL request to Commerce (e.g. a tracing/correlation id header).") + String[] forwardedHeaderNames() default {}; + + @AttributeDefinition( + name = "Enable client IP forwarding", + description = "Forwards the end-user IP using the dedicated fields below, in addition to any generic headers above. Only " + + "enable this once the CDN/dispatcher in front of AEM is confirmed to set the source header from the actual client " + + "connection, not from an unvalidated client-supplied value.") + boolean clientIpEnabled() default false; + + @AttributeDefinition( + name = "Client IP header name", + description = "Incoming request header set by the CDN/edge/dispatcher that carries the original client IP. Defaults to " + + "the standard 'X-Forwarded-For', already populated by the AEMaaCS managed CDN and by common on-premise " + + "dispatcher/reverse-proxy setups. Different CDNs may use a dedicated header instead, e.g. 'CF-Connecting-IP' " + + "(Cloudflare), 'True-Client-IP' (Akamai), 'Fastly-Client-IP' (Fastly). The reserved value 'REMOTE_ADDR' reads the " + + "direct TCP connection IP instead of a header: only correct when AEM is reached with no proxy/CDN/dispatcher in " + + "between (e.g. local development), since behind any proxy this would instead resolve to that proxy's own IP.") + String clientIpHeaderName() default "X-Forwarded-For"; + + @AttributeDefinition( + name = "Client IP outbound header name", + description = "Header name used to forward the client IP on the outbound Commerce request, so it never collides with a " + + "header name already used for another purpose between AEM and Commerce.") + String clientIpOutboundHeaderName() default "X-Adobe-Client-IP"; + + @AttributeDefinition( + name = "Client IP value extraction pattern", + description = "Regex with a single capturing group used to extract the client IP from the header value above. The " + + "default pattern takes the leftmost token, which works for a plain single-IP header (e.g. 'CF-Connecting-IP') as " + + "well as a multi-hop 'X-Forwarded-For' chain ('client, proxy1, proxy2'). Use 'for=\"?\\[?([0-9a-fA-F:.]+)' for the " + + "standards-based 'Forwarded' header (RFC 7239), or '^([0-9a-fA-F:.]+):\\d+$' to strip a trailing port, e.g. " + + "CloudFront's 'CloudFront-Viewer-Address'.") + String clientIpHeaderValuePattern() default "^\\s*([0-9a-fA-F:.]+)"; +} diff --git a/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfigService.java b/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfigService.java new file mode 100644 index 000000000..158af2131 --- /dev/null +++ b/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfigService.java @@ -0,0 +1,103 @@ +/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ~ Copyright 2026 Adobe + ~ + ~ Licensed under the Apache License, Version 2.0 (the "License"); + ~ you may not use this file except in compliance with the License. + ~ You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/ +package com.adobe.cq.commerce.core.components.internal.client; + +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.Set; +import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; + +import org.apache.commons.lang3.StringUtils; +import org.osgi.service.component.annotations.Activate; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.metatype.annotations.Designate; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Holds the {@link ForwardedHeadersConfig} in a form ready to use by {@link MagentoGraphqlClientImpl}: the client + * IP pattern is pre-compiled once here rather than on every request, and an invalid pattern disables client IP + * forwarding instead of failing GraphQL requests at runtime. + */ +@Component(service = ForwardedHeadersConfigService.class) +@Designate(ocd = ForwardedHeadersConfig.class) +public class ForwardedHeadersConfigService { + + /** + * Reserved {@link ForwardedHeadersConfig#clientIpHeaderName()} value: read the direct TCP connection IP + * instead of a header. Only correct with no proxy/CDN/dispatcher in front of AEM (e.g. local development). + */ + static final String REMOTE_ADDR = "REMOTE_ADDR"; + + private static final Logger LOGGER = LoggerFactory.getLogger(ForwardedHeadersConfigService.class); + + private boolean enabled; + private Set forwardedHeaderNames = Collections.emptySet(); + private boolean clientIpEnabled; + private String clientIpHeaderName; + private String clientIpOutboundHeaderName; + private Pattern clientIpHeaderValuePattern; + + @Activate + protected void activate(ForwardedHeadersConfig config) { + this.enabled = config.enabled(); + + String[] configuredNames = config.forwardedHeaderNames(); + this.forwardedHeaderNames = configuredNames != null && configuredNames.length > 0 + ? new LinkedHashSet<>(Arrays.asList(configuredNames)) + : Collections.emptySet(); + + this.clientIpEnabled = config.clientIpEnabled(); + this.clientIpHeaderName = config.clientIpHeaderName(); + this.clientIpOutboundHeaderName = StringUtils.isNotBlank(config.clientIpOutboundHeaderName()) + ? config.clientIpOutboundHeaderName() + : config.clientIpHeaderName(); + + try { + this.clientIpHeaderValuePattern = Pattern.compile(config.clientIpHeaderValuePattern()); + } catch (PatternSyntaxException e) { + LOGGER.error("Invalid client IP header value pattern '{}', client IP forwarding is disabled", + config.clientIpHeaderValuePattern(), e); + this.clientIpEnabled = false; + } + } + + public boolean isEnabled() { + return enabled; + } + + public Set getForwardedHeaderNames() { + return forwardedHeaderNames; + } + + public boolean isClientIpEnabled() { + return clientIpEnabled; + } + + public String getClientIpHeaderName() { + return clientIpHeaderName; + } + + public String getClientIpOutboundHeaderName() { + return clientIpOutboundHeaderName; + } + + public Pattern getClientIpHeaderValuePattern() { + return clientIpHeaderValuePattern; + } +} diff --git a/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImpl.java b/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImpl.java index 859749421..29b59cdb6 100644 --- a/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImpl.java +++ b/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImpl.java @@ -21,6 +21,7 @@ import java.util.Calendar; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -28,6 +29,8 @@ import java.util.Set; import java.util.TimeZone; import java.util.concurrent.atomic.AtomicLong; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import javax.annotation.PostConstruct; @@ -41,6 +44,7 @@ import org.apache.sling.api.resource.ResourceResolver; import org.apache.sling.models.annotations.Model; import org.apache.sling.models.annotations.injectorspecific.InjectionStrategy; +import org.apache.sling.models.annotations.injectorspecific.OSGiService; import org.apache.sling.models.annotations.injectorspecific.ScriptVariable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -92,6 +96,11 @@ public class MagentoGraphqlClientImpl implements MagentoGraphqlClient { private Resource resource; @ScriptVariable(injectionStrategy = InjectionStrategy.OPTIONAL) private Page currentPage; + // Admin-configured forwarding of incoming request headers (generic headers, plus a dedicated client IP + // section) to the outbound Commerce request. Which header/source and how to parse the client IP differs per + // CDN/dispatcher (AEMaaCS vs. on-premise, and across CDN vendors), hence configuration, not hardcoded here. + @OSGiService(injectionStrategy = InjectionStrategy.OPTIONAL) + private ForwardedHeadersConfigService forwardedHeadersConfigService; private GraphqlClient graphqlClient; private RequestOptions requestOptions; @@ -208,6 +217,20 @@ private void initModel(Resource resource, Page page, SlingHttpServletRequest req httpMethod = HttpMethod.POST; } + // Headers carrying per-request metadata (client IP, forwarded tracing/correlation ids, ...) must not + // influence the GraphQL response cache key, since they do not affect the response itself. + Set nonCacheKeyHeaderNames = new HashSet<>(); + + if (request != null && forwardedHeadersConfigService != null && forwardedHeadersConfigService.isEnabled()) { + // Checked separately from the generic forwarded headers below, since the client IP needs its own + // source pattern and outbound name rather than a plain same-name pass-through. + if (forwardedHeadersConfigService.isClientIpEnabled()) { + applyClientIpForwarding(request, forwardedHeadersConfigService, headers, nonCacheKeyHeaderNames); + } + + applyGenericHeaderForwarding(request, forwardedHeadersConfigService, headers, nonCacheKeyHeaderNames); + } + this.httpHeaders = headers; // In certain situations resource.getResourceType() returns an enforced resource type. // We prefer the resource type of the component proxy for the cache name. @@ -218,6 +241,7 @@ private void initModel(Resource resource, Page page, SlingHttpServletRequest req .withCacheName(cacheName) .withDataFetchingPolicy(DataFetchingPolicy.CACHE_FIRST)) .withHeaders(headers.size() > 0 ? headers : null) + .withNonCacheKeyHeaderNames(nonCacheKeyHeaderNames) .withHttpMethod(httpMethod); if (request != null) { @@ -361,6 +385,85 @@ private static List
getCustomHttpHeaders(ComponentsConfiguration configu return headers; } + /** + * Forwards the client IP using the dedicated source header/pattern/outbound name from + * {@link ForwardedHeadersConfig}, since the client IP needs more than a plain same-name pass-through: a source + * that may be {@code REMOTE_ADDR}, a value extraction pattern, and typically a different outbound name. + */ + private static void applyClientIpForwarding(SlingHttpServletRequest request, ForwardedHeadersConfigService config, + List
headers, Set nonCacheKeyHeaderNames) { + String outboundHeaderName = config.getClientIpOutboundHeaderName(); + String value = readRequestValue(request, config.getClientIpHeaderName(), config.getClientIpHeaderValuePattern()); + addForwardedHeader(outboundHeaderName, value, headers, nonCacheKeyHeaderNames); + } + + /** + * Forwards each configured generic header (e.g. a tracing/correlation id) as-is, under the same name, with no + * value extraction pattern. Kept separate from client IP forwarding above so each stays simple to read. + */ + private static void applyGenericHeaderForwarding(SlingHttpServletRequest request, ForwardedHeadersConfigService config, + List
headers, Set nonCacheKeyHeaderNames) { + for (String headerName : config.getForwardedHeaderNames()) { + String value = readRequestValue(request, headerName, null); + addForwardedHeader(headerName, value, headers, nonCacheKeyHeaderNames); + } + } + + /** + * Adds {@code value} to {@code headers} under {@code outboundHeaderName} and marks it as excluded from the + * response cache key, unless the value is missing, the name is denylisted, or a header with that name is + * already present (a statically configured header always takes precedence). + */ + private static void addForwardedHeader(String outboundHeaderName, String value, List
headers, + Set nonCacheKeyHeaderNames) { + if (value == null) { + return; + } + if (DENIED_HEADERS.contains(outboundHeaderName.toLowerCase(Locale.ROOT))) { + LOGGER.warn("Ignoring denylisted outbound header '{}' configured for forwarding", outboundHeaderName); + return; + } + if (headers.stream().noneMatch(header -> header.getName().equalsIgnoreCase(outboundHeaderName))) { + headers.add(new BasicHeader(outboundHeaderName, value)); + nonCacheKeyHeaderNames.add(outboundHeaderName); + } + } + + /** + * Reads the value to forward from the configured source: either the direct TCP connection IP + * ({@code REMOTE_ADDR}), or the named incoming header, optionally parsed with a pattern. A {@code null} + * pattern means the header's raw value is forwarded as-is. Which source to read, and how to parse it, is + * configuration ({@link ForwardedHeadersConfig}) rather than hardcoded here, since different CDNs/dispatchers + * in front of AEM (AEMaaCS vs. on-premise, and across CDN vendors) expose values like the client IP + * differently. + */ + private static String readRequestValue(SlingHttpServletRequest request, String headerName, Pattern headerValuePattern) { + if (StringUtils.isBlank(headerName)) { + return null; + } + + if (ForwardedHeadersConfigService.REMOTE_ADDR.equalsIgnoreCase(headerName)) { + return StringUtils.trimToNull(request.getRemoteAddr()); + } + + String headerValue = StringUtils.trimToNull(request.getHeader(headerName)); + if (headerValue == null) { + return null; + } + + if (headerValuePattern == null) { + return headerValue; + } + + Matcher matcher = headerValuePattern.matcher(headerValue); + if (matcher.find()) { + return matcher.group(1); + } + + LOGGER.warn("Could not extract a value from header '{}' using the configured pattern", headerName); + return null; + } + private static Long getTimeWarpEpoch(SlingHttpServletRequest request) { String timeWarp = request.getParameter("timewarp"); if (timeWarp == null) { diff --git a/bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImplTest.java b/bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImplTest.java index e635f4ae1..2c19b39f5 100644 --- a/bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImplTest.java +++ b/bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImplTest.java @@ -403,6 +403,274 @@ private void testPreviewVersionHeaderWithTimewarp(Long expectedTimeInMillis) { verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); } + private void registerClientIpForwarding(String headerName, String headerValuePattern) { + context.registerInjectActivateService(new ForwardedHeadersConfigService(), ImmutableMap.of( + "enabled", true, + "clientIpEnabled", true, + "clientIpHeaderName", headerName, + "clientIpHeaderValuePattern", headerValuePattern)); + } + + private void registerClientIpForwarding(String headerName, String outboundHeaderName, String headerValuePattern) { + context.registerInjectActivateService(new ForwardedHeadersConfigService(), ImmutableMap.of( + "enabled", true, + "clientIpEnabled", true, + "clientIpHeaderName", headerName, + "clientIpOutboundHeaderName", outboundHeaderName, + "clientIpHeaderValuePattern", headerValuePattern)); + } + + private void registerGenericHeaderForwarding(String... headerNames) { + context.registerInjectActivateService(new ForwardedHeadersConfigService(), ImmutableMap.of( + "enabled", true, + "forwardedHeaderNames", headerNames)); + } + + private void registerComponentsConfigurationForPageA(ComponentsConfiguration configuration) { + context.registerAdapter(Resource.class, ComponentsConfiguration.class, + (Function) resource -> resource + .getPath() + .startsWith(PAGE_A) ? configuration : ComponentsConfiguration.EMPTY); + context.currentResource(PAGE_A); + } + + @Test + public void testClientIpForwardedFromConfiguredHeader() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + registerClientIpForwarding("X-Forwarded-For", "^\\s*([0-9a-fA-F:.]+)"); + context.request().addHeader("X-Forwarded-For", "203.0.113.25"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = new ArrayList<>(); + headers.add(new BasicHeader("Store", "my-store")); + headers.add(new BasicHeader("X-Adobe-Client-IP", "203.0.113.25")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testLeftmostEntryTakenFromMultiHopXForwardedForHeader() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + registerClientIpForwarding("X-Forwarded-For", "^\\s*([0-9a-fA-F:.]+)"); + // First entry is the original client; subsequent entries were appended by trusted hops (CDN, dispatcher) + context.request().addHeader("X-Forwarded-For", "203.0.113.25, 198.51.100.10, 192.0.2.5"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = new ArrayList<>(); + headers.add(new BasicHeader("Store", "my-store")); + headers.add(new BasicHeader("X-Adobe-Client-IP", "203.0.113.25")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testClientIpExtractedFromDedicatedCdnHeader() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + // e.g. Cloudflare's dedicated single-value header instead of X-Forwarded-For + registerClientIpForwarding("CF-Connecting-IP", "^\\s*([0-9a-fA-F:.]+)"); + context.request().addHeader("CF-Connecting-IP", "203.0.113.25"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = new ArrayList<>(); + headers.add(new BasicHeader("Store", "my-store")); + headers.add(new BasicHeader("X-Adobe-Client-IP", "203.0.113.25")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testOutboundHeaderNameDefaultsToXAdobeClientIp() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + // No explicit outbound name or pattern: relies purely on the ForwardedHeadersConfig annotation's defaults + context.registerInjectActivateService(new ForwardedHeadersConfigService(), ImmutableMap.of( + "enabled", true, + "clientIpEnabled", true, + "clientIpHeaderName", "X-Forwarded-For")); + context.request().addHeader("X-Forwarded-For", "203.0.113.25"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = new ArrayList<>(); + headers.add(new BasicHeader("Store", "my-store")); + headers.add(new BasicHeader("X-Adobe-Client-IP", "203.0.113.25")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testOutboundHeaderNameCanDifferFromIncomingHeaderName() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + // Read from the CDN's dedicated header, but forward to Commerce under a fixed, unrelated name + registerClientIpForwarding("CF-Connecting-IP", "X-Custom-Client-IP", "^\\s*([0-9a-fA-F:.]+)"); + context.request().addHeader("CF-Connecting-IP", "203.0.113.25"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = new ArrayList<>(); + headers.add(new BasicHeader("Store", "my-store")); + headers.add(new BasicHeader("X-Custom-Client-IP", "203.0.113.25")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testClientIpReadFromRemoteAddrForLocalTesting() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + // REMOTE_ADDR is only correct with no proxy/CDN in front of AEM, e.g. local development + registerClientIpForwarding("REMOTE_ADDR", "^\\s*([0-9a-fA-F:.]+)"); + context.request().setRemoteAddr("127.0.0.1"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = new ArrayList<>(); + headers.add(new BasicHeader("Store", "my-store")); + headers.add(new BasicHeader("X-Adobe-Client-IP", "127.0.0.1")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testMalformedClientIpHeaderIsIgnored() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + registerClientIpForwarding("X-Forwarded-For", "^\\s*([0-9a-fA-F:.]+)"); + context.request().addHeader("X-Forwarded-For", ""); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = Collections.singletonList(new BasicHeader("Store", "my-store")); + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testClientIpForwardingDisabledByDefault() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + // No ForwardedHeadersConfigService registered/enabled: incoming header must not be forwarded + context.request().addHeader("X-Forwarded-For", "203.0.113.25"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = Collections.singletonList(new BasicHeader("Store", "my-store")); + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testMasterSwitchDisablesForwardingEvenWhenClientIpAndGenericHeadersConfigured() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + // Master "enabled" off: neither the client IP section nor the generic header list should apply + context.registerInjectActivateService(new ForwardedHeadersConfigService(), ImmutableMap.of( + "enabled", false, + "clientIpEnabled", true, + "clientIpHeaderName", "X-Forwarded-For", + "forwardedHeaderNames", new String[] { "X-Request-Id" })); + context.request().addHeader("X-Forwarded-For", "203.0.113.25"); + context.request().addHeader("X-Request-Id", "abc-123"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = Collections.singletonList(new BasicHeader("Store", "my-store")); + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testGenericHeaderForwardedAsIs() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + registerGenericHeaderForwarding("X-Request-Id"); + context.request().addHeader("X-Request-Id", "abc-123"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = new ArrayList<>(); + headers.add(new BasicHeader("Store", "my-store")); + headers.add(new BasicHeader("X-Request-Id", "abc-123")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testDenylistedGenericHeaderIsIgnoredEvenIfConfigured() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + registerGenericHeaderForwarding("Authorization", "X-Request-Id"); + context.request().addHeader("Authorization", "Bearer secret"); + context.request().addHeader("X-Request-Id", "abc-123"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = new ArrayList<>(); + headers.add(new BasicHeader("Store", "my-store")); + headers.add(new BasicHeader("X-Request-Id", "abc-123")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testClientIpAndGenericHeadersForwardedTogether() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + context.registerInjectActivateService(new ForwardedHeadersConfigService(), ImmutableMap.of( + "enabled", true, + "clientIpEnabled", true, + "clientIpHeaderName", "X-Forwarded-For", + "forwardedHeaderNames", new String[] { "X-Request-Id" })); + context.request().addHeader("X-Forwarded-For", "203.0.113.25"); + context.request().addHeader("X-Request-Id", "abc-123"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = new ArrayList<>(); + headers.add(new BasicHeader("Store", "my-store")); + headers.add(new BasicHeader("X-Adobe-Client-IP", "203.0.113.25")); + headers.add(new BasicHeader("X-Request-Id", "abc-123")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testConfiguredHttpHeaderTakesPrecedenceOverClientIpHeader() { + ValueMap configWithClientIpHeader = new ValueMapDecorator(ImmutableMap.of("cq:graphqlClient", "default", "magentoStore", + "my-store", "httpHeaders", new String[] { "X-Adobe-Client-IP=configured-value" })); + ComponentsConfiguration configObject = new ComponentsConfiguration(configWithClientIpHeader); + + registerComponentsConfigurationForPageA(configObject); + registerClientIpForwarding("X-Forwarded-For", "^\\s*([0-9a-fA-F:.]+)"); + context.request().addHeader("X-Forwarded-For", "203.0.113.25"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = new ArrayList<>(); + headers.add(new BasicHeader("Store", "my-store")); + headers.add(new BasicHeader("X-Adobe-Client-IP", "configured-value")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + @Test public void testErrorResponses() { Page page = spy(context.pageManager().getPage(PAGE_A)); From e8c5963510c0886b775eb7a111271ed3085677b7 Mon Sep 17 00:00:00 2001 From: Alwin Joseph Date: Wed, 26 Aug 2026 12:07:08 +0530 Subject: [PATCH 2/3] SITES-49845: Simplify client IP/header forwarding to read config from GraphqlClientConfiguration --- .../client/ForwardedHeadersConfig.java | 76 ------- .../client/ForwardedHeadersConfigService.java | 103 --------- .../client/MagentoGraphqlClientImpl.java | 113 ++-------- .../client/MagentoGraphqlClientImplTest.java | 208 +++--------------- 4 files changed, 53 insertions(+), 447 deletions(-) delete mode 100644 bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfig.java delete mode 100644 bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfigService.java diff --git a/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfig.java b/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfig.java deleted file mode 100644 index 39d24d360..000000000 --- a/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfig.java +++ /dev/null @@ -1,76 +0,0 @@ -/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - ~ Copyright 2026 Adobe - ~ - ~ Licensed under the Apache License, Version 2.0 (the "License"); - ~ you may not use this file except in compliance with the License. - ~ You may obtain a copy of the License at - ~ - ~ http://www.apache.org/licenses/LICENSE-2.0 - ~ - ~ Unless required by applicable law or agreed to in writing, software - ~ distributed under the License is distributed on an "AS IS" BASIS, - ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - ~ See the License for the specific language governing permissions and - ~ limitations under the License. - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/ -package com.adobe.cq.commerce.core.components.internal.client; - -import org.osgi.service.metatype.annotations.AttributeDefinition; -import org.osgi.service.metatype.annotations.ObjectClassDefinition; - -/** - * Single configuration covering all forwarding of incoming request headers to the outbound Commerce GraphQL - * request: a master switch, an arbitrary list of headers forwarded as-is, and a dedicated client IP section with - * its own switch and fields (source header/pattern/outbound name), since the client IP needs more than a plain - * name to be forwarded correctly. Every header forwarded through this configuration, generic or client IP, is - * excluded from the GraphQL response cache key, since it carries per-request metadata that does not influence the - * response. Headers on the internal denylist (Authorization, Cookie, Host, Content-Length, etc.) are never - * forwarded, even if configured here. - */ -@ObjectClassDefinition(name = "CIF Forwarded Request Headers Configuration") -public @interface ForwardedHeadersConfig { - - @AttributeDefinition( - name = "Enabled", - description = "Master switch for all header forwarding configured below. Disable to turn everything off without " - + "clearing the individual fields.") - boolean enabled() default false; - - @AttributeDefinition( - name = "Forwarded header names", - description = "Names of incoming request headers whose current value should be forwarded as-is, under the same name, on " - + "the outbound GraphQL request to Commerce (e.g. a tracing/correlation id header).") - String[] forwardedHeaderNames() default {}; - - @AttributeDefinition( - name = "Enable client IP forwarding", - description = "Forwards the end-user IP using the dedicated fields below, in addition to any generic headers above. Only " - + "enable this once the CDN/dispatcher in front of AEM is confirmed to set the source header from the actual client " - + "connection, not from an unvalidated client-supplied value.") - boolean clientIpEnabled() default false; - - @AttributeDefinition( - name = "Client IP header name", - description = "Incoming request header set by the CDN/edge/dispatcher that carries the original client IP. Defaults to " - + "the standard 'X-Forwarded-For', already populated by the AEMaaCS managed CDN and by common on-premise " - + "dispatcher/reverse-proxy setups. Different CDNs may use a dedicated header instead, e.g. 'CF-Connecting-IP' " - + "(Cloudflare), 'True-Client-IP' (Akamai), 'Fastly-Client-IP' (Fastly). The reserved value 'REMOTE_ADDR' reads the " - + "direct TCP connection IP instead of a header: only correct when AEM is reached with no proxy/CDN/dispatcher in " - + "between (e.g. local development), since behind any proxy this would instead resolve to that proxy's own IP.") - String clientIpHeaderName() default "X-Forwarded-For"; - - @AttributeDefinition( - name = "Client IP outbound header name", - description = "Header name used to forward the client IP on the outbound Commerce request, so it never collides with a " - + "header name already used for another purpose between AEM and Commerce.") - String clientIpOutboundHeaderName() default "X-Adobe-Client-IP"; - - @AttributeDefinition( - name = "Client IP value extraction pattern", - description = "Regex with a single capturing group used to extract the client IP from the header value above. The " - + "default pattern takes the leftmost token, which works for a plain single-IP header (e.g. 'CF-Connecting-IP') as " - + "well as a multi-hop 'X-Forwarded-For' chain ('client, proxy1, proxy2'). Use 'for=\"?\\[?([0-9a-fA-F:.]+)' for the " - + "standards-based 'Forwarded' header (RFC 7239), or '^([0-9a-fA-F:.]+):\\d+$' to strip a trailing port, e.g. " - + "CloudFront's 'CloudFront-Viewer-Address'.") - String clientIpHeaderValuePattern() default "^\\s*([0-9a-fA-F:.]+)"; -} diff --git a/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfigService.java b/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfigService.java deleted file mode 100644 index 158af2131..000000000 --- a/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/ForwardedHeadersConfigService.java +++ /dev/null @@ -1,103 +0,0 @@ -/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - ~ Copyright 2026 Adobe - ~ - ~ Licensed under the Apache License, Version 2.0 (the "License"); - ~ you may not use this file except in compliance with the License. - ~ You may obtain a copy of the License at - ~ - ~ http://www.apache.org/licenses/LICENSE-2.0 - ~ - ~ Unless required by applicable law or agreed to in writing, software - ~ distributed under the License is distributed on an "AS IS" BASIS, - ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - ~ See the License for the specific language governing permissions and - ~ limitations under the License. - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/ -package com.adobe.cq.commerce.core.components.internal.client; - -import java.util.Arrays; -import java.util.Collections; -import java.util.LinkedHashSet; -import java.util.Set; -import java.util.regex.Pattern; -import java.util.regex.PatternSyntaxException; - -import org.apache.commons.lang3.StringUtils; -import org.osgi.service.component.annotations.Activate; -import org.osgi.service.component.annotations.Component; -import org.osgi.service.metatype.annotations.Designate; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Holds the {@link ForwardedHeadersConfig} in a form ready to use by {@link MagentoGraphqlClientImpl}: the client - * IP pattern is pre-compiled once here rather than on every request, and an invalid pattern disables client IP - * forwarding instead of failing GraphQL requests at runtime. - */ -@Component(service = ForwardedHeadersConfigService.class) -@Designate(ocd = ForwardedHeadersConfig.class) -public class ForwardedHeadersConfigService { - - /** - * Reserved {@link ForwardedHeadersConfig#clientIpHeaderName()} value: read the direct TCP connection IP - * instead of a header. Only correct with no proxy/CDN/dispatcher in front of AEM (e.g. local development). - */ - static final String REMOTE_ADDR = "REMOTE_ADDR"; - - private static final Logger LOGGER = LoggerFactory.getLogger(ForwardedHeadersConfigService.class); - - private boolean enabled; - private Set forwardedHeaderNames = Collections.emptySet(); - private boolean clientIpEnabled; - private String clientIpHeaderName; - private String clientIpOutboundHeaderName; - private Pattern clientIpHeaderValuePattern; - - @Activate - protected void activate(ForwardedHeadersConfig config) { - this.enabled = config.enabled(); - - String[] configuredNames = config.forwardedHeaderNames(); - this.forwardedHeaderNames = configuredNames != null && configuredNames.length > 0 - ? new LinkedHashSet<>(Arrays.asList(configuredNames)) - : Collections.emptySet(); - - this.clientIpEnabled = config.clientIpEnabled(); - this.clientIpHeaderName = config.clientIpHeaderName(); - this.clientIpOutboundHeaderName = StringUtils.isNotBlank(config.clientIpOutboundHeaderName()) - ? config.clientIpOutboundHeaderName() - : config.clientIpHeaderName(); - - try { - this.clientIpHeaderValuePattern = Pattern.compile(config.clientIpHeaderValuePattern()); - } catch (PatternSyntaxException e) { - LOGGER.error("Invalid client IP header value pattern '{}', client IP forwarding is disabled", - config.clientIpHeaderValuePattern(), e); - this.clientIpEnabled = false; - } - } - - public boolean isEnabled() { - return enabled; - } - - public Set getForwardedHeaderNames() { - return forwardedHeaderNames; - } - - public boolean isClientIpEnabled() { - return clientIpEnabled; - } - - public String getClientIpHeaderName() { - return clientIpHeaderName; - } - - public String getClientIpOutboundHeaderName() { - return clientIpOutboundHeaderName; - } - - public Pattern getClientIpHeaderValuePattern() { - return clientIpHeaderValuePattern; - } -} diff --git a/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImpl.java b/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImpl.java index 29b59cdb6..d775195f6 100644 --- a/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImpl.java +++ b/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImpl.java @@ -21,7 +21,6 @@ import java.util.Calendar; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -29,8 +28,6 @@ import java.util.Set; import java.util.TimeZone; import java.util.concurrent.atomic.AtomicLong; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Collectors; import javax.annotation.PostConstruct; @@ -44,7 +41,6 @@ import org.apache.sling.api.resource.ResourceResolver; import org.apache.sling.models.annotations.Model; import org.apache.sling.models.annotations.injectorspecific.InjectionStrategy; -import org.apache.sling.models.annotations.injectorspecific.OSGiService; import org.apache.sling.models.annotations.injectorspecific.ScriptVariable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -96,11 +92,6 @@ public class MagentoGraphqlClientImpl implements MagentoGraphqlClient { private Resource resource; @ScriptVariable(injectionStrategy = InjectionStrategy.OPTIONAL) private Page currentPage; - // Admin-configured forwarding of incoming request headers (generic headers, plus a dedicated client IP - // section) to the outbound Commerce request. Which header/source and how to parse the client IP differs per - // CDN/dispatcher (AEMaaCS vs. on-premise, and across CDN vendors), hence configuration, not hardcoded here. - @OSGiService(injectionStrategy = InjectionStrategy.OPTIONAL) - private ForwardedHeadersConfigService forwardedHeadersConfigService; private GraphqlClient graphqlClient; private RequestOptions requestOptions; @@ -217,18 +208,8 @@ private void initModel(Resource resource, Page page, SlingHttpServletRequest req httpMethod = HttpMethod.POST; } - // Headers carrying per-request metadata (client IP, forwarded tracing/correlation ids, ...) must not - // influence the GraphQL response cache key, since they do not affect the response itself. - Set nonCacheKeyHeaderNames = new HashSet<>(); - - if (request != null && forwardedHeadersConfigService != null && forwardedHeadersConfigService.isEnabled()) { - // Checked separately from the generic forwarded headers below, since the client IP needs its own - // source pattern and outbound name rather than a plain same-name pass-through. - if (forwardedHeadersConfigService.isClientIpEnabled()) { - applyClientIpForwarding(request, forwardedHeadersConfigService, headers, nonCacheKeyHeaderNames); - } - - applyGenericHeaderForwarding(request, forwardedHeadersConfigService, headers, nonCacheKeyHeaderNames); + if (request != null) { + forwardCacheKeyExcludedHeaders(request, headers); } this.httpHeaders = headers; @@ -241,7 +222,6 @@ private void initModel(Resource resource, Page page, SlingHttpServletRequest req .withCacheName(cacheName) .withDataFetchingPolicy(DataFetchingPolicy.CACHE_FIRST)) .withHeaders(headers.size() > 0 ? headers : null) - .withNonCacheKeyHeaderNames(nonCacheKeyHeaderNames) .withHttpMethod(httpMethod); if (request != null) { @@ -386,82 +366,37 @@ private static List
getCustomHttpHeaders(ComponentsConfiguration configu } /** - * Forwards the client IP using the dedicated source header/pattern/outbound name from - * {@link ForwardedHeadersConfig}, since the client IP needs more than a plain same-name pass-through: a source - * that may be {@code REMOTE_ADDR}, a value extraction pattern, and typically a different outbound name. - */ - private static void applyClientIpForwarding(SlingHttpServletRequest request, ForwardedHeadersConfigService config, - List
headers, Set nonCacheKeyHeaderNames) { - String outboundHeaderName = config.getClientIpOutboundHeaderName(); - String value = readRequestValue(request, config.getClientIpHeaderName(), config.getClientIpHeaderValuePattern()); - addForwardedHeader(outboundHeaderName, value, headers, nonCacheKeyHeaderNames); - } - - /** - * Forwards each configured generic header (e.g. a tracing/correlation id) as-is, under the same name, with no - * value extraction pattern. Kept separate from client IP forwarding above so each stays simple to read. + * Forwards, as-is under the same name, any incoming request header named in this instance's {@code GraphqlClient} + * connection's {@code cacheKeyExcludedHeaders()} (e.g. a client IP header set by the CDN/dispatcher in front of + * AEM) - so the caller doesn't need a second, separate configuration to know which headers to forward. */ - private static void applyGenericHeaderForwarding(SlingHttpServletRequest request, ForwardedHeadersConfigService config, - List
headers, Set nonCacheKeyHeaderNames) { - for (String headerName : config.getForwardedHeaderNames()) { - String value = readRequestValue(request, headerName, null); - addForwardedHeader(headerName, value, headers, nonCacheKeyHeaderNames); - } - } - - /** - * Adds {@code value} to {@code headers} under {@code outboundHeaderName} and marks it as excluded from the - * response cache key, unless the value is missing, the name is denylisted, or a header with that name is - * already present (a statically configured header always takes precedence). - */ - private static void addForwardedHeader(String outboundHeaderName, String value, List
headers, - Set nonCacheKeyHeaderNames) { - if (value == null) { + private void forwardCacheKeyExcludedHeaders(SlingHttpServletRequest request, List
headers) { + if (graphqlClient == null) { return; } - if (DENIED_HEADERS.contains(outboundHeaderName.toLowerCase(Locale.ROOT))) { - LOGGER.warn("Ignoring denylisted outbound header '{}' configured for forwarding", outboundHeaderName); + GraphqlClientConfiguration configuration = graphqlClient.getConfiguration(); + if (configuration == null) { return; } - if (headers.stream().noneMatch(header -> header.getName().equalsIgnoreCase(outboundHeaderName))) { - headers.add(new BasicHeader(outboundHeaderName, value)); - nonCacheKeyHeaderNames.add(outboundHeaderName); - } - } - /** - * Reads the value to forward from the configured source: either the direct TCP connection IP - * ({@code REMOTE_ADDR}), or the named incoming header, optionally parsed with a pattern. A {@code null} - * pattern means the header's raw value is forwarded as-is. Which source to read, and how to parse it, is - * configuration ({@link ForwardedHeadersConfig}) rather than hardcoded here, since different CDNs/dispatchers - * in front of AEM (AEMaaCS vs. on-premise, and across CDN vendors) expose values like the client IP - * differently. - */ - private static String readRequestValue(SlingHttpServletRequest request, String headerName, Pattern headerValuePattern) { - if (StringUtils.isBlank(headerName)) { - return null; - } - - if (ForwardedHeadersConfigService.REMOTE_ADDR.equalsIgnoreCase(headerName)) { - return StringUtils.trimToNull(request.getRemoteAddr()); - } - - String headerValue = StringUtils.trimToNull(request.getHeader(headerName)); - if (headerValue == null) { - return null; - } - - if (headerValuePattern == null) { - return headerValue; + String[] headerNames = configuration.cacheKeyExcludedHeaders(); + if (headerNames == null) { + return; } - Matcher matcher = headerValuePattern.matcher(headerValue); - if (matcher.find()) { - return matcher.group(1); + for (String headerName : headerNames) { + String value = StringUtils.trimToNull(request.getHeader(headerName)); + if (value == null) { + continue; + } + if (DENIED_HEADERS.contains(headerName.toLowerCase(Locale.ROOT))) { + LOGGER.warn("Ignoring denylisted header '{}' configured for forwarding", headerName); + continue; + } + if (headers.stream().noneMatch(header -> header.getName().equalsIgnoreCase(headerName))) { + headers.add(new BasicHeader(headerName, value)); + } } - - LOGGER.warn("Could not extract a value from header '{}' using the configured pattern", headerName); - return null; } private static Long getTimeWarpEpoch(SlingHttpServletRequest request) { diff --git a/bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImplTest.java b/bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImplTest.java index 2c19b39f5..284086983 100644 --- a/bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImplTest.java +++ b/bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImplTest.java @@ -52,6 +52,7 @@ import com.adobe.cq.commerce.graphql.client.CachingStrategy; import com.adobe.cq.commerce.graphql.client.CachingStrategy.DataFetchingPolicy; import com.adobe.cq.commerce.graphql.client.GraphqlClient; +import com.adobe.cq.commerce.graphql.client.GraphqlClientConfiguration; import com.adobe.cq.commerce.graphql.client.GraphqlRequestException; import com.adobe.cq.commerce.graphql.client.GraphqlResponse; import com.adobe.cq.commerce.graphql.client.HttpMethod; @@ -403,27 +404,10 @@ private void testPreviewVersionHeaderWithTimewarp(Long expectedTimeInMillis) { verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); } - private void registerClientIpForwarding(String headerName, String headerValuePattern) { - context.registerInjectActivateService(new ForwardedHeadersConfigService(), ImmutableMap.of( - "enabled", true, - "clientIpEnabled", true, - "clientIpHeaderName", headerName, - "clientIpHeaderValuePattern", headerValuePattern)); - } - - private void registerClientIpForwarding(String headerName, String outboundHeaderName, String headerValuePattern) { - context.registerInjectActivateService(new ForwardedHeadersConfigService(), ImmutableMap.of( - "enabled", true, - "clientIpEnabled", true, - "clientIpHeaderName", headerName, - "clientIpOutboundHeaderName", outboundHeaderName, - "clientIpHeaderValuePattern", headerValuePattern)); - } - - private void registerGenericHeaderForwarding(String... headerNames) { - context.registerInjectActivateService(new ForwardedHeadersConfigService(), ImmutableMap.of( - "enabled", true, - "forwardedHeaderNames", headerNames)); + private void registerCacheKeyExcludedHeaders(String... headerNames) { + GraphqlClientConfiguration configuration = Mockito.mock(GraphqlClientConfiguration.class); + when(configuration.cacheKeyExcludedHeaders()).thenReturn(headerNames); + when(graphqlClient.getConfiguration()).thenReturn(configuration); } private void registerComponentsConfigurationForPageA(ComponentsConfiguration configuration) { @@ -435,185 +419,77 @@ private void registerComponentsConfigurationForPageA(ComponentsConfiguration con } @Test - public void testClientIpForwardedFromConfiguredHeader() { + public void testHeaderNotForwardedWhenNotConfigured() { registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - registerClientIpForwarding("X-Forwarded-For", "^\\s*([0-9a-fA-F:.]+)"); + // No cacheKeyExcludedHeaders configured: the incoming header must not be forwarded context.request().addHeader("X-Forwarded-For", "203.0.113.25"); MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); client.execute("{dummy}"); - List
headers = new ArrayList<>(); - headers.add(new BasicHeader("Store", "my-store")); - headers.add(new BasicHeader("X-Adobe-Client-IP", "203.0.113.25")); - - RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); - verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); - } - - @Test - public void testLeftmostEntryTakenFromMultiHopXForwardedForHeader() { - registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - registerClientIpForwarding("X-Forwarded-For", "^\\s*([0-9a-fA-F:.]+)"); - // First entry is the original client; subsequent entries were appended by trusted hops (CDN, dispatcher) - context.request().addHeader("X-Forwarded-For", "203.0.113.25, 198.51.100.10, 192.0.2.5"); - - MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); - client.execute("{dummy}"); - - List
headers = new ArrayList<>(); - headers.add(new BasicHeader("Store", "my-store")); - headers.add(new BasicHeader("X-Adobe-Client-IP", "203.0.113.25")); - + List
headers = Collections.singletonList(new BasicHeader("Store", "my-store")); RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); } @Test - public void testClientIpExtractedFromDedicatedCdnHeader() { + public void testConfiguredHeaderNotForwardedWhenAbsentFromIncomingRequest() { registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - // e.g. Cloudflare's dedicated single-value header instead of X-Forwarded-For - registerClientIpForwarding("CF-Connecting-IP", "^\\s*([0-9a-fA-F:.]+)"); - context.request().addHeader("CF-Connecting-IP", "203.0.113.25"); + registerCacheKeyExcludedHeaders("X-Forwarded-For"); + // Header configured for forwarding, but not present on the incoming request MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); client.execute("{dummy}"); - List
headers = new ArrayList<>(); - headers.add(new BasicHeader("Store", "my-store")); - headers.add(new BasicHeader("X-Adobe-Client-IP", "203.0.113.25")); - + List
headers = Collections.singletonList(new BasicHeader("Store", "my-store")); RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); } @Test - public void testOutboundHeaderNameDefaultsToXAdobeClientIp() { + public void testMultipleCacheKeyExcludedHeadersForwardedTogether() { registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - // No explicit outbound name or pattern: relies purely on the ForwardedHeadersConfig annotation's defaults - context.registerInjectActivateService(new ForwardedHeadersConfigService(), ImmutableMap.of( - "enabled", true, - "clientIpEnabled", true, - "clientIpHeaderName", "X-Forwarded-For")); + registerCacheKeyExcludedHeaders("X-Forwarded-For", "X-Request-Id"); context.request().addHeader("X-Forwarded-For", "203.0.113.25"); + context.request().addHeader("X-Request-Id", "abc-123"); MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); client.execute("{dummy}"); List
headers = new ArrayList<>(); headers.add(new BasicHeader("Store", "my-store")); - headers.add(new BasicHeader("X-Adobe-Client-IP", "203.0.113.25")); - - RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); - verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); - } - - @Test - public void testOutboundHeaderNameCanDifferFromIncomingHeaderName() { - registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - // Read from the CDN's dedicated header, but forward to Commerce under a fixed, unrelated name - registerClientIpForwarding("CF-Connecting-IP", "X-Custom-Client-IP", "^\\s*([0-9a-fA-F:.]+)"); - context.request().addHeader("CF-Connecting-IP", "203.0.113.25"); - - MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); - client.execute("{dummy}"); - - List
headers = new ArrayList<>(); - headers.add(new BasicHeader("Store", "my-store")); - headers.add(new BasicHeader("X-Custom-Client-IP", "203.0.113.25")); - - RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); - verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); - } - - @Test - public void testClientIpReadFromRemoteAddrForLocalTesting() { - registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - // REMOTE_ADDR is only correct with no proxy/CDN in front of AEM, e.g. local development - registerClientIpForwarding("REMOTE_ADDR", "^\\s*([0-9a-fA-F:.]+)"); - context.request().setRemoteAddr("127.0.0.1"); - - MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); - client.execute("{dummy}"); - - List
headers = new ArrayList<>(); - headers.add(new BasicHeader("Store", "my-store")); - headers.add(new BasicHeader("X-Adobe-Client-IP", "127.0.0.1")); - - RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); - verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); - } - - @Test - public void testMalformedClientIpHeaderIsIgnored() { - registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - registerClientIpForwarding("X-Forwarded-For", "^\\s*([0-9a-fA-F:.]+)"); - context.request().addHeader("X-Forwarded-For", ""); - - MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); - client.execute("{dummy}"); + headers.add(new BasicHeader("X-Forwarded-For", "203.0.113.25")); + headers.add(new BasicHeader("X-Request-Id", "abc-123")); - List
headers = Collections.singletonList(new BasicHeader("Store", "my-store")); RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); } @Test - public void testClientIpForwardingDisabledByDefault() { - registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - // No ForwardedHeadersConfigService registered/enabled: incoming header must not be forwarded - context.request().addHeader("X-Forwarded-For", "203.0.113.25"); - - MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); - client.execute("{dummy}"); + public void testConfiguredHttpHeaderTakesPrecedenceOverForwardedHeader() { + ValueMap configWithForwardedHeader = new ValueMapDecorator(ImmutableMap.of("cq:graphqlClient", "default", "magentoStore", + "my-store", "httpHeaders", new String[] { "X-Forwarded-For=configured-value" })); + ComponentsConfiguration configObject = new ComponentsConfiguration(configWithForwardedHeader); - List
headers = Collections.singletonList(new BasicHeader("Store", "my-store")); - RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); - verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); - } - - @Test - public void testMasterSwitchDisablesForwardingEvenWhenClientIpAndGenericHeadersConfigured() { - registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - // Master "enabled" off: neither the client IP section nor the generic header list should apply - context.registerInjectActivateService(new ForwardedHeadersConfigService(), ImmutableMap.of( - "enabled", false, - "clientIpEnabled", true, - "clientIpHeaderName", "X-Forwarded-For", - "forwardedHeaderNames", new String[] { "X-Request-Id" })); + registerComponentsConfigurationForPageA(configObject); + registerCacheKeyExcludedHeaders("X-Forwarded-For"); context.request().addHeader("X-Forwarded-For", "203.0.113.25"); - context.request().addHeader("X-Request-Id", "abc-123"); - - MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); - client.execute("{dummy}"); - - List
headers = Collections.singletonList(new BasicHeader("Store", "my-store")); - RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); - verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); - } - - @Test - public void testGenericHeaderForwardedAsIs() { - registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - registerGenericHeaderForwarding("X-Request-Id"); - context.request().addHeader("X-Request-Id", "abc-123"); MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); client.execute("{dummy}"); List
headers = new ArrayList<>(); headers.add(new BasicHeader("Store", "my-store")); - headers.add(new BasicHeader("X-Request-Id", "abc-123")); + headers.add(new BasicHeader("X-Forwarded-For", "configured-value")); RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); } @Test - public void testDenylistedGenericHeaderIsIgnoredEvenIfConfigured() { + public void testCacheKeyExcludedHeaderForwardedAsIs() { registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - registerGenericHeaderForwarding("Authorization", "X-Request-Id"); - context.request().addHeader("Authorization", "Bearer secret"); + registerCacheKeyExcludedHeaders("X-Request-Id"); context.request().addHeader("X-Request-Id", "abc-123"); MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); @@ -628,14 +504,10 @@ public void testDenylistedGenericHeaderIsIgnoredEvenIfConfigured() { } @Test - public void testClientIpAndGenericHeadersForwardedTogether() { + public void testDenylistedCacheKeyExcludedHeaderIsIgnoredEvenIfConfigured() { registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - context.registerInjectActivateService(new ForwardedHeadersConfigService(), ImmutableMap.of( - "enabled", true, - "clientIpEnabled", true, - "clientIpHeaderName", "X-Forwarded-For", - "forwardedHeaderNames", new String[] { "X-Request-Id" })); - context.request().addHeader("X-Forwarded-For", "203.0.113.25"); + registerCacheKeyExcludedHeaders("Authorization", "X-Request-Id"); + context.request().addHeader("Authorization", "Bearer secret"); context.request().addHeader("X-Request-Id", "abc-123"); MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); @@ -643,34 +515,12 @@ public void testClientIpAndGenericHeadersForwardedTogether() { List
headers = new ArrayList<>(); headers.add(new BasicHeader("Store", "my-store")); - headers.add(new BasicHeader("X-Adobe-Client-IP", "203.0.113.25")); headers.add(new BasicHeader("X-Request-Id", "abc-123")); RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); } - @Test - public void testConfiguredHttpHeaderTakesPrecedenceOverClientIpHeader() { - ValueMap configWithClientIpHeader = new ValueMapDecorator(ImmutableMap.of("cq:graphqlClient", "default", "magentoStore", - "my-store", "httpHeaders", new String[] { "X-Adobe-Client-IP=configured-value" })); - ComponentsConfiguration configObject = new ComponentsConfiguration(configWithClientIpHeader); - - registerComponentsConfigurationForPageA(configObject); - registerClientIpForwarding("X-Forwarded-For", "^\\s*([0-9a-fA-F:.]+)"); - context.request().addHeader("X-Forwarded-For", "203.0.113.25"); - - MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); - client.execute("{dummy}"); - - List
headers = new ArrayList<>(); - headers.add(new BasicHeader("Store", "my-store")); - headers.add(new BasicHeader("X-Adobe-Client-IP", "configured-value")); - - RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); - verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); - } - @Test public void testErrorResponses() { Page page = spy(context.pageManager().getPage(PAGE_A)); From 49c365b833818e5156e17502bd9de667db79063b Mon Sep 17 00:00:00 2001 From: Alwin Joseph Date: Fri, 28 Aug 2026 13:21:04 +0530 Subject: [PATCH 3/3] SITES-49845: Keep passthrough headers off the store-config export and align with passthroughHeaders --- .../client/MagentoGraphqlClientImpl.java | 39 ++++++--- .../client/MagentoGraphqlClientImplTest.java | 81 ++++++++++++++++--- 2 files changed, 100 insertions(+), 20 deletions(-) diff --git a/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImpl.java b/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImpl.java index d775195f6..d63ff3db1 100644 --- a/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImpl.java +++ b/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImpl.java @@ -208,11 +208,18 @@ private void initModel(Resource resource, Page page, SlingHttpServletRequest req httpMethod = HttpMethod.POST; } + this.httpHeaders = headers; + + // Build the outbound header set: the custom/advertised headers above, plus any per-request passthrough + // headers forwarded from the incoming request (e.g. a client IP). Passthrough headers are intentionally + // kept out of this.httpHeaders, which is exported into the cacheable store-config tag - forwarding + // a per-user value (e.g. the end-user IP) there would leak it across consumers of a cached page. + List
outboundHeaders = headers; if (request != null) { - forwardCacheKeyExcludedHeaders(request, headers); + outboundHeaders = new ArrayList<>(headers); + forwardPassthroughHeaders(request, outboundHeaders); } - this.httpHeaders = headers; // In certain situations resource.getResourceType() returns an enforced resource type. // We prefer the resource type of the component proxy for the cache name. String cacheName = resource.getValueMap().get(ResourceResolver.PROPERTY_RESOURCE_TYPE, resource.getResourceType()); @@ -221,7 +228,7 @@ private void initModel(Resource resource, Page page, SlingHttpServletRequest req .withCachingStrategy(new CachingStrategy() .withCacheName(cacheName) .withDataFetchingPolicy(DataFetchingPolicy.CACHE_FIRST)) - .withHeaders(headers.size() > 0 ? headers : null) + .withHeaders(outboundHeaders.size() > 0 ? outboundHeaders : null) .withHttpMethod(httpMethod); if (request != null) { @@ -366,11 +373,15 @@ private static List
getCustomHttpHeaders(ComponentsConfiguration configu } /** - * Forwards, as-is under the same name, any incoming request header named in this instance's {@code GraphqlClient} - * connection's {@code cacheKeyExcludedHeaders()} (e.g. a client IP header set by the CDN/dispatcher in front of - * AEM) - so the caller doesn't need a second, separate configuration to know which headers to forward. + * Adds to {@code outboundHeaders}, as-is under the same name, any incoming request header named in this instance's + * {@code GraphqlClient} connection's {@code passthroughHeaders()} (e.g. a client IP header set by the + * CDN/dispatcher in front of AEM) - so the caller doesn't need a second, separate configuration to know which + * headers to forward. The same list makes the client exclude these headers from its response cache key, so a + * per-request value does not fragment the cache. These are added only to the outbound request, never to + * {@link #httpHeaders} (the store-config export surface), so a per-user value is not embedded in cacheable page + * HTML. */ - private void forwardCacheKeyExcludedHeaders(SlingHttpServletRequest request, List
headers) { + private void forwardPassthroughHeaders(SlingHttpServletRequest request, List
outboundHeaders) { if (graphqlClient == null) { return; } @@ -379,12 +390,18 @@ private void forwardCacheKeyExcludedHeaders(SlingHttpServletRequest request, Lis return; } - String[] headerNames = configuration.cacheKeyExcludedHeaders(); + String[] headerNames = configuration.passthroughHeaders(); if (headerNames == null) { return; } - for (String headerName : headerNames) { + for (String configuredName : headerNames) { + // The OSGi config editor can produce empty entries in a String[]; ignore them, and tolerate + // incidental whitespace around a configured name so " X-Forwarded-For" still matches. + String headerName = StringUtils.trimToNull(configuredName); + if (headerName == null) { + continue; + } String value = StringUtils.trimToNull(request.getHeader(headerName)); if (value == null) { continue; @@ -393,8 +410,8 @@ private void forwardCacheKeyExcludedHeaders(SlingHttpServletRequest request, Lis LOGGER.warn("Ignoring denylisted header '{}' configured for forwarding", headerName); continue; } - if (headers.stream().noneMatch(header -> header.getName().equalsIgnoreCase(headerName))) { - headers.add(new BasicHeader(headerName, value)); + if (outboundHeaders.stream().noneMatch(header -> header.getName().equalsIgnoreCase(headerName))) { + outboundHeaders.add(new BasicHeader(headerName, value)); } } } diff --git a/bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImplTest.java b/bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImplTest.java index 284086983..430ce9b24 100644 --- a/bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImplTest.java +++ b/bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/client/MagentoGraphqlClientImplTest.java @@ -67,7 +67,9 @@ import io.wcm.testing.mock.aem.junit.AemContext; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -404,9 +406,9 @@ private void testPreviewVersionHeaderWithTimewarp(Long expectedTimeInMillis) { verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); } - private void registerCacheKeyExcludedHeaders(String... headerNames) { + private void registerPassthroughHeaders(String... headerNames) { GraphqlClientConfiguration configuration = Mockito.mock(GraphqlClientConfiguration.class); - when(configuration.cacheKeyExcludedHeaders()).thenReturn(headerNames); + when(configuration.passthroughHeaders()).thenReturn(headerNames); when(graphqlClient.getConfiguration()).thenReturn(configuration); } @@ -421,7 +423,7 @@ private void registerComponentsConfigurationForPageA(ComponentsConfiguration con @Test public void testHeaderNotForwardedWhenNotConfigured() { registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - // No cacheKeyExcludedHeaders configured: the incoming header must not be forwarded + // No passthrough headers configured: the incoming header must not be forwarded context.request().addHeader("X-Forwarded-For", "203.0.113.25"); MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); @@ -435,7 +437,7 @@ public void testHeaderNotForwardedWhenNotConfigured() { @Test public void testConfiguredHeaderNotForwardedWhenAbsentFromIncomingRequest() { registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - registerCacheKeyExcludedHeaders("X-Forwarded-For"); + registerPassthroughHeaders("X-Forwarded-For"); // Header configured for forwarding, but not present on the incoming request MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); @@ -447,9 +449,9 @@ public void testConfiguredHeaderNotForwardedWhenAbsentFromIncomingRequest() { } @Test - public void testMultipleCacheKeyExcludedHeadersForwardedTogether() { + public void testMultiplePassthroughHeadersForwardedTogether() { registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - registerCacheKeyExcludedHeaders("X-Forwarded-For", "X-Request-Id"); + registerPassthroughHeaders("X-Forwarded-For", "X-Request-Id"); context.request().addHeader("X-Forwarded-For", "203.0.113.25"); context.request().addHeader("X-Request-Id", "abc-123"); @@ -472,7 +474,7 @@ public void testConfiguredHttpHeaderTakesPrecedenceOverForwardedHeader() { ComponentsConfiguration configObject = new ComponentsConfiguration(configWithForwardedHeader); registerComponentsConfigurationForPageA(configObject); - registerCacheKeyExcludedHeaders("X-Forwarded-For"); + registerPassthroughHeaders("X-Forwarded-For"); context.request().addHeader("X-Forwarded-For", "203.0.113.25"); MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); @@ -486,10 +488,71 @@ public void testConfiguredHttpHeaderTakesPrecedenceOverForwardedHeader() { verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); } + @Test + public void testStaticHeaderTakesPrecedenceOverForwardedHeaderCaseInsensitively() { + // A statically configured header must win over a forwarded one even when their names differ only in case, + // exercising the case-insensitive de-duplication in forwardPassthroughHeaders(). + ValueMap configWithStaticHeader = new ValueMapDecorator(ImmutableMap.of("cq:graphqlClient", "default", "magentoStore", + "my-store", "httpHeaders", new String[] { "X-Forwarded-For=static-value" })); + registerComponentsConfigurationForPageA(new ComponentsConfiguration(configWithStaticHeader)); + registerPassthroughHeaders("X-FORWARDED-FOR"); + context.request().addHeader("X-FORWARDED-FOR", "203.0.113.25"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = new ArrayList<>(); + headers.add(new BasicHeader("Store", "my-store")); + headers.add(new BasicHeader("X-Forwarded-For", "static-value")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + + @Test + public void testForwardedPassthroughHeaderIsSentToBackendButNotExportedInStoreConfig() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + registerPassthroughHeaders("X-Forwarded-For"); + context.request().addHeader("X-Forwarded-For", "203.0.113.25"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + // Forwarded on the outbound request to Commerce... + List
outbound = new ArrayList<>(); + outbound.add(new BasicHeader("Store", "my-store")); + outbound.add(new BasicHeader("X-Forwarded-For", "203.0.113.25")); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.argThat(new RequestOptionsMatcher(outbound, null))); + + // ...but NOT advertised via the header maps, which are serialized into the cacheable store-config . + assertThat(client.getHttpHeaderMap().keySet(), hasItem("Store")); + assertThat(client.getHttpHeaderMap().keySet(), not(hasItem("X-Forwarded-For"))); + assertThat(client.getHttpHeaders().keySet(), not(hasItem("X-Forwarded-For"))); + } + + @Test + public void testPassthroughHeaderNameIsTrimmed() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + // Incidental whitespace around a configured name must not prevent the match. + registerPassthroughHeaders(" X-Forwarded-For "); + context.request().addHeader("X-Forwarded-For", "203.0.113.25"); + + MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); + client.execute("{dummy}"); + + List
headers = new ArrayList<>(); + headers.add(new BasicHeader("Store", "my-store")); + headers.add(new BasicHeader("X-Forwarded-For", "203.0.113.25")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); + } + @Test public void testCacheKeyExcludedHeaderForwardedAsIs() { registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - registerCacheKeyExcludedHeaders("X-Request-Id"); + registerPassthroughHeaders("X-Request-Id"); context.request().addHeader("X-Request-Id", "abc-123"); MagentoGraphqlClient client = context.request().adaptTo(MagentoGraphqlClient.class); @@ -506,7 +569,7 @@ public void testCacheKeyExcludedHeaderForwardedAsIs() { @Test public void testDenylistedCacheKeyExcludedHeaderIsIgnoredEvenIfConfigured() { registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); - registerCacheKeyExcludedHeaders("Authorization", "X-Request-Id"); + registerPassthroughHeaders("Authorization", "X-Request-Id"); context.request().addHeader("Authorization", "Bearer secret"); context.request().addHeader("X-Request-Id", "abc-123");