Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CORS headers to rewrite proxy responses #132

Open
iki opened this issue Sep 13, 2019 · 6 comments
Open

Add CORS headers to rewrite proxy responses #132

iki opened this issue Sep 13, 2019 · 6 comments

Comments

@iki
Copy link

iki commented Sep 13, 2019

Currently the preflight request is served with correct CORS headers for given origin, but the proxied request response is passed without them.

Would it be possible to use the CORS middleware on proxied requests too?

{
  'server.request': {
    requestId: 1,
    method: 'OPTIONS',
    url: '/api/x',
    headers: {
      host: 'localhost:8000',
      connection: 'keep-alive',
      'access-control-request-method': 'GET',
      'sec-fetch-mode': 'cors',
      'access-control-request-headers': 'authentication-token',
      origin: 'http://localhost:3000',
      'sec-fetch-site': 'same-site',
      referer: 'http://localhost:3000/settings',
    }
  }
}
{
  'server.response': {
    requestId: 1,
    statusCode: 204,
    headers: [Object: null prototype] {
      vary: 'Origin',
      'access-control-allow-origin': 'http://localhost:3000',
      'access-control-allow-credentials': 'true',
      'access-control-allow-methods': 'GET,HEAD,PUT,POST,DELETE,PATCH',
      'access-control-allow-headers': 'authentication-token'
    }
  }
}
{
  'server.request': {
    requestId: 2,
    method: 'GET',
    url: '/api/x',
    headers: {
      host: 'localhost:8000',
      connection: 'keep-alive',
      'sec-ch-ua': 'Chromium 78',
      'sec-fetch-mode': 'cors',
      'authentication-token': 'VbegawtfAM4PzAYcoz2KQLaz7aNfYZkC',
      'sec-fetch-dest': 'empty',
      accept: '*/*',
      origin: 'http://localhost:3000',
      'sec-fetch-site': 'same-site',
      referer: 'http://localhost:3000/settings',
      'accept-encoding': 'gzip, deflate, br',
    }
  }
}
{
  'middleware.rewrite.remote.request': {
    rewrite: {
      id: 1,
      from: '/api/x',
      to: 'http://localhost:7000/api/x'
    },
    method: 'GET',
    headers: {
      host: 'localhost:7000',
      'sec-ch-ua': 'Chromium 78',
      'sec-fetch-mode': 'cors',
      'authentication-token': 'VbegawtfAM4PzAYcoz2KQLaz7aNfYZkC',
      'sec-fetch-dest': 'empty',
      accept: '*/*',
      origin: 'http://localhost:3000',
      'sec-fetch-site': 'same-site',
      referer: 'http://localhost:3000/settings',
      'accept-encoding': 'gzip, deflate, br',
      via: '1.1 lws-rewrite'
    }
  }
}
{
  'middleware.rewrite.remote.response': {
    rewrite: {
      id: 1,
      from: '/api/x',
      to: 'http://localhost:7000/api/x'
    },
    status: 200,
    headers: {
      connection: 'close',
      date: 'Fri, 13 Sep 2019 09:30:23 GMT',
      'content-type': 'application/json; charset=utf-8',
      'transfer-encoding': 'chunked',
      via: '1.1 lws-rewrite'
    }
  }
}
{
  'server.response': {
    requestId: 2,
    statusCode: 200,
    headers: [Object: null prototype] {
      date: 'Fri, 13 Sep 2019 09:30:23 GMT',
      'content-type': 'application/json; charset=utf-8',
      via: '1.1 lws-rewrite'
    }
  }
}
@75lb
Copy link
Member

75lb commented Sep 13, 2019

Hi, thanks for the report. Yes, CORS headers should be added in this case - I will look into it.

@75lb
Copy link
Member

75lb commented Sep 14, 2019

I'm not 100% clear what the issue is here.

Your browser made a request to /api/x - as far as your browser knows this is a regular, local request just like any other. Your browser does not know that the request was proxied to a remote target behind the scenes, therefore your browser would not expect to see CORS headers (e.g. Access-Control-Allow-Origin) in the response. If a CORS header appeared in the response to a local request, this would be unexpected.

What exactly do you expect to see in the reponse to /api/x? How does this issue break your app?

@iki
Copy link
Author

iki commented Sep 15, 2019

@75b I was trying if ws can add CORS headers to the proxied api server inside a locel docker:

ws -p 8000 --rewrite "/(.*) -> http://localhost:7000/$1" --cors.credentials -v

Providing CORS headers for proxied requests would be a useful feature. Especially because you get the CORS implementation right on contrary to most CORS proxies on docker hub that use an origin wildcard *, which can't be used with credentials. Nginx can be used, but it's more complex.

@75lb
Copy link
Member

75lb commented Sep 20, 2019

Sorry, I haven't had time to action this yet so I will give you instructions on how to test a solution yourself.

  1. Fork and checkout this repo: https://github.com/lwsjs/rewrite

  2. Look for this line in your local lws-rewrite fork. This is where the remote request headers are set. Fix this line so the headers you require are set.

  3. Launch ws with your local middleware plugin in the stack. In this example, I have provided the path to my local fork. I have also included stack and index for static files and directory listings. You can use any plugin from the ws --default-stack list in your stack (the lws- prefix is optional).

    $ ws --stack /Users/lloyd/Documents/lwsjs/rewrite static index
    

When you have fixed the remote request headers, please let me know how you fixed it. Speak soon!

@75lb
Copy link
Member

75lb commented Nov 24, 2019

@iki did you resolve this in the end?

@iki
Copy link
Author

iki commented Nov 25, 2019

@75lb I used the nginx solution in meantime, but I can test your solution later this week and send a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants