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 Origin to Vary header on credentialed CORS response #1

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jcwilson
Copy link
Owner

@jcwilson jcwilson commented Nov 26, 2020

According to the MDN CORS docs, the Origin item should be added to the Vary header list when
the Access-Control-Allow-Origin is set to an explicit origin value when it could change due to
something like allow_origins being the * wildcard, a multi-item whitelist or allow_origin_regex
being in use.

If the server specifies a single origin (that may dynamically change based on the requesting origin
as part of a white-list) rather than the "*" wildcard, then the server should also include Origin in
the Vary response header — to indicate to clients that server responses will differ based on the
value of the Origin request header.

The existing code fails to update the Vary list when the server is configured to allow all
origins (*) and the request has a Cookie header (ie. credentialed). In that situation, the
Access-Control-Allow-Origin header will be set to the request's Origin value. It should
be noted that the code does currently update the Vary list when the allowed origins are
defined by an explicit whitelist (even if it only has one value and doesn't actually vary) or
a regex pattern.

It appears this may have just been a simple oversight in the original implementation. This updates
the code to add Origin to the Vary header under these circumstances. If it was intentionally
omitted, I'd be delighted to learn why.

Josh Wilson added 2 commits November 26, 2020 02:09
According to the [MDN CORS docs]
(https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#Access-Control-Allow-Origin), the `Origin`
item should be added to the `Vary` header when the `Access-Control-Allow-Origin` is set to an
explicit origin value, as opposed to the `*` wildcard.

>If the server specifies a single origin (that may dynamically change based on the requesting origin
as part of a white-list) rather than the "*" wildcard, then the server should also include Origin in
the Vary response header — to indicate to clients that server responses will differ based on the
value of the Origin request header.

The existing code fails to update the `Vary` list when the server is configured to allow all
origins (`*`) and the request has a `Cookie` header (ie. credentialed). In that situation, the
`Access-Control-Allow-Origin` header will be set to the request's `Origin` value.

It appears this may have just been a simple oversight in the original implementation. This updates
the code to add `Origin` to the `Vary` header under these circumstancesIf it was intentionally
omitted, I'd be delighted to learn why.
@jcwilson jcwilson force-pushed the vary-origin-credentialed-cors branch 3 times, most recently from 5df76ea to 1673983 Compare December 17, 2020 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant