[BUG] The 'x-api-key' header is cut off by the proxy

aleksanderson commented 2 years ago

Summary

NOTE: This may not necessarily be a bug, but it can cause issues when integrating with some client libraries.

The current configuration of the built-in proxy explicitly filters out a set of specific headers. "x-api-key" is among those headers.

At the same time, there are client libraries/SDKs which use the same header to pass an authorization key to their backend. This happens especially when those services use AWS API Gateway where this naming convention is originally coming from. Because the header is cut off via the proxy it causes issues for those types of integration(s).

Based on that it needs to be validated if there is a particular reason for having that header cut off

Steps To Reproduce

Expected result

1. The official documentation needs to be updated to indicate the specifics of some of the headers being cut off. All headers being cut off need to be explicitly listed.. 2. Potentially allow the "x-api-key" header to be proxied as there might be a lot of SDKs using the same name due to its adoption as a convention and fixing it on the SDK/API level of a particular service is not always feasible.

Actual result

1. The current version of the documentation does not indicate that specifics: https://developer.salesforce.com/docs/commerce/pwa-kit-managed-runtime/guide/proxying-requests.html. _2. "x-api-key" header is not proxied

System Information (as applicable)

n/a

Additional information

n/a

drewzboto commented 2 years ago

Thanks @aleksanderson. Can you share which API client libraries you are looking at that rely on the x-api-key header?

We stripped out this common API-gateway header for historical reasons, but we believe this is no longer needed. We would be looking at two changes:

PWA Kit code change to remove the logic stripping this out
Managed Runtime needs the corresponding change to allow this header to pass through for proxies

johnboxall commented 2 years ago

A few notes for others looking at this issue while we work through a fix, to confirm the bug, I an internal testing environment: https://adn-production-us-west-1.mobify-storefront.com/

First, observe that for requests routed to the App Server, many headers are added/overwritten:

curl 'https://adn-production-us-west-1.mobify-storefront.com/' \
  -sH 'x-api-key: my-api-key' \
  | jq '.headers'

{
  "accept": "*/*",
  "cloudfront-forwarded-proto": "https",
  "cloudfront-is-desktop-viewer": "true",
  "cloudfront-is-mobile-viewer": "false",
  "cloudfront-is-tablet-viewer": "false",
  "connection": "close",
  "host": "playground-adn-production-us-west-1-sandbox-ssrserver.mobify-storefront.com",
  "if-none-match": "W/\"730-4p1eTDHfiyagW+PT6+aC2elAGgQ\"",
  "user-agent": "Amazon CloudFront",
  "via": "1.1 5c4026f5b2425caec6a876ce1acb5bd4.cloudfront.net (CloudFront)",
  "x-amz-cf-id": "g9AYEIOHg3soPU3sCdVOo30M5jJ7mo3MmFD05PoJnMKHEnrG65AisA==",
  "x-amzn-trace-id": "Root=1-629fd0b9-0a06e6b427f26d6f3dafe8ad",
  "x-api-key": "*****",
  "x-apigateway-context": "*****",
  "x-apigateway-event": "*****",
  "x-forwarded-for": "13.110.54.39, 64.252.73.200",
  "x-forwarded-port": "443",
  "x-forwarded-proto": "https"
}

A number of cloudfront- headers are added.
The Host header is updated.
The User-Agent header is updated.
via is added.
Tracing headers are added.
API Gateway headers are added including x-api-key
A number of forwarding headers are added.

If any of these headers existed before, they would be replaced!

Second, we look at the behaviour of proxies:

curl 'https://adn-production-us-west-1.mobify-storefront.com/mobify/proxy/httpbin/anything' \
  -sH 'x-api-key: my-api-key' \
  -sH 'x-my-custom-header: my-custom-header' \
  | jq '.headers'

{
  "Accept": "*/*",
  "Cloudfront-Forwarded-Proto": "https",
  "Cloudfront-Is-Desktop-Viewer": "true",
  "Cloudfront-Is-Mobile-Viewer": "false",
  "Cloudfront-Is-Smarttv-Viewer": "false",
  "Cloudfront-Is-Tablet-Viewer": "false",
  "Cloudfront-Viewer-Country": "US",
  "Host": "httpbin.org",
  "User-Agent": "Amazon CloudFront",
  "X-Amz-Cf-Id": "uXg__EEmI0iUKmKWcJw4lo0kRxiSMeujJyB8Rw21p3YfCthxbJWmnw==",
  "X-Amzn-Trace-Id": "Root=1-629fd0d3-7d9cb5a418b1992a33ae84c9",
  "X-Mobify": "true",
  "X-Mobify-Proxypath": "httpbin",
  "X-My-Custom-Header": "my-custom-header"
}

Again we forward a number of CloudFront headers
We add x-mobify headers
We actively strip x-api-key and the other headers mentioned 😢

aleksanderson commented 2 years ago

@drewzboto and @johnboxall, Thank you for confirming.

Can you share which API client libraries you are looking at that rely on the x-api-key header?

I have been recently playing around with integrating PWAKit with Uniform and, their API clients did use x-api-key header

mix3d commented 2 years ago

Watching

mix3d commented 2 years ago

Any update on this, @johnboxall?

We're currently manually editing our local ssr-proxying.js inside the node module folder in order to have our dev envs working.

johnboxall commented 2 years ago

Hey @mix3d, when deployed to Managed Runtime, we use this header internally. We remove it to prevent conflicts with where we use it.

If your use case is:

a) making proxy requests from a browser with the header or b) making proxy requests from the app server with the header

Then removing it from the list is a workaround.

If your use case is forwarding a specific value for it to the app server (eg. the express.js code) then it won't work.

Other work arounds include:

Not using the proxy for these requests
Routing the requests through a middleware server that changes the name of the headers to something else

We're unlikely to revisit this issue in the near term, but as we get closer to working on Secrets and Config values in Managed Runtime, we'll touch this area and see if we can address this issue.

git2gus[bot] commented 10 months ago

This issue has been linked to a new work item: W-14515732

SalesforceCommerceCloud / pwa-kit