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..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 @@ -209,6 +209,17 @@ private void initModel(Resource resource, Page page, SlingHttpServletRequest req } 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) { + outboundHeaders = new ArrayList<>(headers); + forwardPassthroughHeaders(request, outboundHeaders); + } + // 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()); @@ -217,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) { @@ -361,6 +372,50 @@ private static List
getCustomHttpHeaders(ComponentsConfiguration configu return headers; } + /** + * 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 forwardPassthroughHeaders(SlingHttpServletRequest request, List
outboundHeaders) { + if (graphqlClient == null) { + return; + } + GraphqlClientConfiguration configuration = graphqlClient.getConfiguration(); + if (configuration == null) { + return; + } + + String[] headerNames = configuration.passthroughHeaders(); + if (headerNames == null) { + return; + } + + 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; + } + if (DENIED_HEADERS.contains(headerName.toLowerCase(Locale.ROOT))) { + LOGGER.warn("Ignoring denylisted header '{}' configured for forwarding", headerName); + continue; + } + if (outboundHeaders.stream().noneMatch(header -> header.getName().equalsIgnoreCase(headerName))) { + outboundHeaders.add(new BasicHeader(headerName, value)); + } + } + } + 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..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 @@ -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; @@ -66,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; @@ -403,6 +406,184 @@ private void testPreviewVersionHeaderWithTimewarp(Long expectedTimeInMillis) { verify(graphqlClient).execute(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.argThat(matcher)); } + private void registerPassthroughHeaders(String... headerNames) { + GraphqlClientConfiguration configuration = Mockito.mock(GraphqlClientConfiguration.class); + when(configuration.passthroughHeaders()).thenReturn(headerNames); + when(graphqlClient.getConfiguration()).thenReturn(configuration); + } + + 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 testHeaderNotForwardedWhenNotConfigured() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + // 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); + 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 testConfiguredHeaderNotForwardedWhenAbsentFromIncomingRequest() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + registerPassthroughHeaders("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 = 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 testMultiplePassthroughHeadersForwardedTogether() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + 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"); + + 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")); + 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 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); + + registerComponentsConfigurationForPageA(configObject); + 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", "configured-value")); + + RequestOptionsMatcher matcher = new RequestOptionsMatcher(headers, null); + 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); + registerPassthroughHeaders("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 testDenylistedCacheKeyExcludedHeaderIsIgnoredEvenIfConfigured() { + registerComponentsConfigurationForPageA(MOCK_CONFIGURATION_OBJECT); + registerPassthroughHeaders("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 testErrorResponses() { Page page = spy(context.pageManager().getPage(PAGE_A));