emissary-ingress / emissary

open source Kubernetes-native API gateway for microservices built on the Envoy Proxy
https://www.getambassador.io
Apache License 2.0
4.36k stars 685 forks source link

CORS response headers not applied when request origin not in explicit origins list #3052

Open jcwilson opened 3 years ago

jcwilson commented 3 years ago

Describe the bug

Note: this was reproduced using an external auth service configuration (configured as an API gateway plugin). It's unclear at this time whether it affects the default authservice functionality that ships with AES

When one specifies an explicit list of origin values in the ambassador Module's cors spec, and the preflight request's Origin is not in this whitelist, the preflight request is sent to the auth service first, then forwarded to the API service where it will likely fail as the API service expects Ambassador to handle CORS concerns.

I don't believe it's intended that preflight requests should ever be routed to an API service, as the edge stack should handle that for us. When the request's origin is found in the whitelist, the preflight request is only sent to the auth service (as expected).

To Reproduce

  1. Deploy an external auth service to your cluster alongside ambassador.
    1. Ensure it can respond appropriately to CORS preflight OPTIONS requests (credentials will not be present)
  2. Configure ambassador to disable the default auth service.
  3. Configure ambassador to use the AuthService plugin instead and point it at your external auth service.
  4. Provide a basic cors config with an explicit origins list in the ambassador Module
    cors:
        origins: "https://www.google.com"  # for example
        methods: GET, POST, PUT, PATCH, OPTIONS, DELETE
        headers: "*"
        exposed_headers: "*"
        max_age: "86400"
  5. Deploy a back end API service and an associated mapping
  6. Make a "non-simple" CORS request to your back end service
    1. Ensure that the request origin is not in the whitelist (i used chrome dev tools while on wikipedia's home page)
    2. This is the simplest example to reproduce this behavior. You can achieve the same results even if you provide credentials on the request or modify the cors: spec to allow credentialed requests. See the attached document for a more complete listing.
      fetch(
      'https://*******.com/edge/echo/ping',
      {
      mode: 'cors',
      method: 'POST',
      body: "{}",
      headers: {
      'Content-Type': 'application/json',
      }
      }
      )
  7. Observe:
    1. the preflight request is successfully handled by the auth service (returns 200)
      1. it's likely that having the auth service return a 4XX here if it determines origin is not allowed will prevent the preflight request from being forwarded to the API service. However, it still seems like a bug to send preflight requests to the API service regardless of the auth result.
    2. the preflight request is forwarded to the API service (this is the unexpected part)
    3. the API service returns a 401
      1. the 401 behavior is somewhat particular to our setup, but I'm including it here for completeness and noting that this may also play a part in the preflight response not having any CORS response headers

Expected behavior

  1. [Most important] Ambassador should not forward the preflight request to the API service under any circumstances.
  2. Ambassador should apply the remaining appropriate CORS response headers to the preflight request, even if the origin is not whitelisted, possibly informed by the auth service. In this case, it perhaps it should be that the Access-Control-Allow-Origin header is not present or is the empty string.
  3. Ambassador should respond with a 200 status code to the preflight OPTIONS request.
    1. There seems to be some wiggle room here in the CORS spec, so perhaps a 4XX is also acceptable. I believe browsers rely solely on the presence of the Access-Control-Allow-* headers to determine whether to deliver the response to the app or not and the status code is ignored.

Versions (please complete the following information):

Additional context I don't know if this is an Ambassador problem or an Envoy-proxy problem.

See the attached document for more details (and another type of CORS issue I have encountered) ambassador-cors-experiment.pdf

This seems possibly related to these issues:

2030

2455

jcwilson commented 3 years ago
  1. Observe:
    1. the preflight request is successfully handled by the auth service (returns 200) a. it's likely that having the auth service return a 4XX here if it determines origin is not allowed will prevent the preflight request from being forwarded to the api service. However, it still seems like a bug to send preflight requests to the API service regardless of the auth result.

I've confirmed that having the auth service reply to the preflight OPTIONS request with a 400 will prevent it from being forwarded to the API service. The response to the client has the 400 status code and whatever body the auth service generated but no CORS response headers.

At the end of the day, since the 400 response does not contain the Access-Control-Allow-Origin header, the browser should fail to follow up with the actual API call. However, it will be difficult for a developer to debug the reason for the CORS failure without the remaining Access-Control-Allow-* headers present to compare the Access-Control-Request-* headers against. My workaround for now is to give a hint in the response body.

jcwilson commented 3 years ago

The workaround noted on #3051 can apply here, too.

I'll copy the content here for completeness:


Casey Kurosawa offered a suggestion in the #ambassador slack channel, which was to add the desired CORS headers to the AuthService allowed_authorization_headers list.

Though the docs only mention that these headers will be added to the request as it's sent upstream, it appears that they are also applied to the response as it's sent back downstream, regardless of whether the request was allowed by the auth service or not.

While this will allow us to work around the behavior described here, I don't think it's ideal. Now it means that we have to maintain CORS configuration in two places - once in the AuthService CRD configuration and a second time in the auth service's implementation. Ideally, we would only need to specify it in the CRD and leave the auth service implementation happily CORS-unaware.

Another downside of this work-around is that the behavior is not consistent between authenticated calls and non-authenticated calls. Authenticated calls use the CRD configuration as the source-of-truth. Whereas calls that fail authentication will receive the response header values provided by the auth service. If one is careful to keep the configurations in sync, this isn't too troublesome, but it's still confusing.

ayratn commented 3 years ago

We are experiencing the same issue in our product: we use external service to acknowledge API access and in case of 4xx errors client's browser fails with CORS error in the console.

As a workaround we also keep CORS in two places now: CRD and auth-service code with CORS taken from application configuration.

We deploy to Openshift so at least we have single variable with the same CORS parameters which are injected both to CRD and auth-service, which seems at least minimizes the drawback of the workaround mentioned by @jcwilson.

Hoping to get it fixed soon anyway. Thank you.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

jcwilson commented 3 years ago

keeping this one alive for now

ajaykumar-gbi commented 1 year ago

+1