whatwg / xhr

XMLHttpRequest Standard
https://xhr.spec.whatwg.org/
Other
314 stars 131 forks source link

Non-simple CORS requests on redirect #255

Closed priyankn closed 4 years ago

priyankn commented 5 years ago

I have run into this specific scenario where if a XHR initiates a CORS request with special headers ( Eg- Access-Control-Allow-Headers: authorization, content-type), and response to it is a redirect (say 303) to a URI which is still cross-origin, XHR will initiate a new pre-flight request to this new URI. The issue is that if this third party origin happily reflects all the ‘Access-Control-Allow-Headers’ values, and has ‘Access-Control-Allow-Origin’ as a wildcard, the final request will also sent with our custom authorization and content-type headers to this third party origin.

In short,

mainapp.com -> (Need auth header for API) api.app.com -> (Redirects, but also sends Auth header, thus leaking to CDN) -> CDN (anotherorigin.com)

This behavior is that this is similar to a configuring a wildcard Access-Control-Allow-Origin, while allowing credentials. There is a good reason this is disallowed. But in this case, instead of the credentials being passed as Cookies, we can pass an auth header?

The original CORS spec (https://www.w3.org/TR/cors/) disallowed this specific scenario (sec 7.1.5) :

This is the actual request. Apply the make a request steps and observe the request rules below while making the request.
If the response has an HTTP status code of 301, 302, 303, 307, or 308
Apply the cache and network error steps.

I understand that we can use fetch() with controlling redirects for a “fix”, but it would probably make the code deployment-specific. Open for discussion.

P.S. - This was observed on the latest versions of both FF and Chrome, will consider opening an issue with them as well for discussion.

annevk commented 5 years ago

Allowing redirects there was always a goal. Your API endpoint could also take the token and send it separately to the CDN unbeknownst to you. You have to be able to trust them regardless.

priyankn commented 5 years ago

The Issue is not with crossing trust boundaries with a CDN, that is just one example of this case which can have unintended consequences.

The thing is with redirects for non-simple requests. I think these are related?

https://stackoverflow.com/questions/34949492/cors-request-with-preflight-and-redirect-disallowed-workarounds/39728229#39728229

https://github.com/whatwg/fetch/commit/0d9a4db8bc02251cc9e391543bb3c1322fb882f2

annevk commented 5 years ago

Yes, that's when we changed this.

annevk commented 4 years ago

I filed https://github.com/whatwg/fetch/issues/944 to investigate this a bit more. It seems there's interest in maybe doing something special for Authorization.

priyankn commented 4 years ago

@annevk Looks like httpwg/http-core#424 made some changes which might address the issue? How does this work now, is it upto the browser vendors to interpret it and change this behaviour?

annevk commented 4 years ago

Yeah, it's optional. I'm going to close this as this is really a change that needs to be made in Fetch.

priyankn commented 4 years ago

Ok great. Should I open a ticket with fetch as well, who maintains it?

annevk commented 4 years ago

https://github.com/whatwg/fetch/issues/944 is the relevant issue there. I mostly maintain it, but there's no active implementer interest in making a change there. Perhaps if someone did some work on specification text and tests that could get some commitments though.