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

Allow * for Access-Control-Expose-Headers #252

Closed
roryhewitt opened this issue Mar 18, 2016 · 18 comments · Fixed by #298
Closed

Allow * for Access-Control-Expose-Headers #252

roryhewitt opened this issue Mar 18, 2016 · 18 comments · Fixed by #298
Labels
addition/proposal New features or enhancements

Comments

@roryhewitt
Copy link

This has almost certainly been discussed before, but would it be possible to allow * (allow-all) as a separate value for the Access-Control-Expose-Headers CORS response header?

This would allow all non-simple headers passed in the request to be 'exposable' to client-side code.

When the original CORS spec was written, there was an assumption (I assume!) that the preflight OPTIONS response would be served by the same application that would serve the subsequent CORS GET/POST/whatever request. Therefore, that application would have full knowledge of what headers the API might eventually respond with, and which of them should be exposed or suppressed.

However, with the advent of CDN's and load balancers, it may be that the code that responds to the preflight OPTIONS request is not the actual application that will process the subsequent GET/POST/whatever request. Indeed, website owners might want the OPTIONS response to be served from non-application code, because that code may be quicker to respond to the client.

(Full disclosure: I work for Akamai, a large CDN. Whilst my views don't represent any official Akamai position, we would obviously like this, since it would make our (and out customer's) lives easier. I have also implemented CORS solutions using an F5 LTM load balancer, and run into this problem).

So the spec would become the following:

Access-Control-Expose-Headers = "Access-Control-Expose-Headers" ":" #field-name | "*"

If Access-Control-Expose-Headers: * is returned, browser would allow all headers to be returned using e.g. XMLHttpRequest getResponseHeader().

@annevk
Copy link
Member

annevk commented Mar 18, 2016

Implementers have traditionally been wary about doing this, since it could be a footgun. Servers might have forgotten about certain debug headers exposing information that was meant to be kept secret, etc.

@sicking @tyoshino?

@sicking
Copy link

sicking commented Mar 18, 2016

I'd have to defer to the mozilla security team. I'm personally not super worried about this one though.

@annevk
Copy link
Member

annevk commented Mar 18, 2016

@dveditz @bifurcation @freddyb?

@annevk
Copy link
Member

annevk commented Mar 25, 2016

@rlbmoz? Or maybe you know who to ask?

@annevk annevk changed the title Update Access-Control-Expose-Headers CORS response header to allow * (allow-all) Allow * for Access-Control-Expose-Headers Mar 25, 2016
@annevk
Copy link
Member

annevk commented Mar 25, 2016

See #253 (comment) for a comment from @sicking which suggests this should be reasonably safe to add, even for credentialed requests.

@tyoshino
Copy link
Member

Hmm, From Jonas's comments, #253, I don't quite understand where the difference in handling (consideration) of credentials between wildcarded ACEH and the others (wildcarded ACAH, etc.) comes from. Maybe I'm missing something?

Jonas's #253 (comment) is assuming that the server doesn't alter the values of the non-simple headers based on the received cookie value?

@sicking
Copy link

sicking commented Mar 28, 2016

I don't really have a strong reason for feeling more ok with * in ACEH than for the other AC* headers.

My thinking was mainly that it's less likely that headers contain personal data in general.

This isn't an argument for allowing * for ACAH since the concern there isn't about leaking private data, but rather about triggering dangerous side effects on the server.

I'm totally fine with allowing * in ACEH only for credential-less requests. It certainly makes things more consistent.

@annevk annevk added the addition/proposal New features or enhancements label May 4, 2016
@dveditz
Copy link
Member

dveditz commented May 5, 2016

I could support '*' for UN-credentialed requests (for consistency with ACAO) and it's unlikely to harm anything. I doubt that helps with the original use-case all that much though.

If your system has data for the application layer to use why can't it be included in the data/body itself? Why must it be stuffed into the metadata for the HTTP transport layer?

@dveditz
Copy link
Member

dveditz commented May 5, 2016

It would help to see examples of real-world applications using ACEH to evaluate the need and safety of allowing '*' for credentialed requests. I'd rather respond to a demonstrated need than offer it just because we think it might be useful. It's a footgun to be sure.

@annevk
Copy link
Member

annevk commented May 5, 2016

Thank you @dveditz. We'll keep it restricted and consistent for now.

annevk added a commit that referenced this issue May 5, 2016
annevk added a commit that referenced this issue May 5, 2016
annevk added a commit that referenced this issue May 24, 2016
annevk added a commit that referenced this issue May 24, 2016
Enable Access-Control-Expose-Headers, Access-Control-Allow-Methods, and Access-Control-Allow-Headers to use a wildcard, with the same restriction as placed upon wildcards in Access-Control-Allow-Origin. Namely, it can only be used for requests where the credentials mode is "omit".

The Authorization header still needs to be explicitly listed by Access-Control-Allow-Headers even with the wildcard.

This also makes the CORS cache wildcard-aware and updates some of the terminology around CORS caches to share more concepts.

Fixes #251 and fixes #252.
@ghost
Copy link

ghost commented May 28, 2016

#251

@Mouvedia
Copy link

Mouvedia commented Dec 25, 2016

…it can only be used for requests where the credentials mode is "omit".

@annevk since XHR only supports 'include' and 'same-origin'. Does that mean that there are no scenarios where an XHR request would yield an Access-Control-Expose-Headers with a * field value?

It contradicts this sentence from the specification:

Similarly, Access-Control-Expose-Headers, Access-Control-Allow-Methods, and Access-Control-Allow-Headers response headers can only use * as value when request’s credentials mode is not "include".

Can you have a mode "cors" credentials "same-origin" request that yield an Access-Control-Expose-Headers with a * field value?

@annevk
Copy link
Member

annevk commented Dec 27, 2016

You can. The commit message was not entirely accurate.

@wuliang142857
Copy link

my solution:

app.use(function(request, response, next) {
    response.header("Access-Control-Allow-Origin", request.headers.origin);
    response.header("Access-Control-Allow-Headers", "Content-Type, Access-Control-Allow-Headers, Authorization, X-Requested-With");
    response.header("Access-Control-Allow-Methods", "GET,HEAD,POST,PUT,DELETE,OPTIONS");
    response.header("Access-Control-Allow-Credentials", "true");
    next();
});

annevk added a commit to web-platform-tests/wpt that referenced this issue Mar 6, 2017
annevk added a commit to web-platform-tests/wpt that referenced this issue Mar 7, 2017
@conorgdaly-st
Copy link

When the original CORS spec was written, there was an assumption (I assume!) that the preflight OPTIONS response would be served by the same application that would serve the subsequent CORS GET/POST/whatever request.
Therefore, that application would have full knowledge of what headers the API might eventually respond with, and which of them should be exposed or suppressed.

'Access-Control-Expose-Headers' is a response header that needs to be on the actual response, not the (pre-flight) OPTIONS response.

I don't quite get the problem description behind the original post?

@annevk
Copy link
Member

annevk commented Apr 6, 2017

@conorgdaly-st well spotted, that justification is indeed wrong. Nevertheless, not having to exhaustively list the actual headers on a response is nice to have.

@roryhewitt
Copy link
Author

@conorgdaly-st you're quite right - I was writing one issue to cover multiple things at once (one for ACEH, one for ACAH/ACAM and one to add a new Access-Control-Suppress-Headers response header), and when I split them into separate issues, things got mixed up...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements
Development

Successfully merging a pull request may close this issue.

8 participants