Add vary: origin header when origin is "*"

[balexand]

I ran into a bug with a caching proxy with the following configuration:

config :my_app, :corsica,
  allow_credentials: true,
  origins: "*"

The response header access-control-allow-origin was being set to the actual origin due to this code:

corsica/lib/corsica.ex

Lines 579 to 586 in 02fd930

	value =
	if origins == "*" and not allow_credentials do
	"*"
	else
	actual_origin
	end
	put_resp_header(conn, "access-control-allow-origin", value)

This meant that the access-control-allow-origin response header varied depending on the origin request header. But the vary: origin response header wasn't being set. This pull-request fixes this.

[whatyouhide]

Hey @balexand, there are a couple of things to consider here.

1. If you don't set access-control-allow-origin to * but instead set it dynamically to the request's host, then you have to use the vary: origin header.

2. If we always send the actual origin instead of * we'll take up more bytes in the response without a strong reason for doing so (the code is not complex now, so I don't feel the need to simplify it).

3. From the CORS spec:

If the resource supports credentials add a single Access-Control-Allow-Origin header, with the value of the Origin header as value, and add a single Access-Control-Allow-Credentials header with the case-sensitive string "true" as value. Otherwise, add a single Access-Control-Allow-Origin header, with either the value of the Origin header or the string "*" as value.

As you can see, it clearly says that if the resource supports credentials, you shouldn't set access-control-allow-origin to *. Hopefully this all makes sense but good catch and thanks for your report and contribution! 💟

[balexand]

Hi @whatyouhide. Thanks for your thoughtful response and I agree with all of your points. Based on your response I'm confused as to why you closed this without merging it.

1. If you don't set access-control-allow-origin to * but instead set it dynamically to the request's host, then you have to use the vary: origin header.

This is true, the CORS spec agrees with this, and MDN also explains that this is important. Yet Corsica is not following this advice and this pull-request fixes that.

Your 2nd and 3rd points are unrelated to the content of this pull-request. This pull-request doesn't change the value sent for the access-control-allow-origin header. This pull-request only affects the vary response header, ensuring that vary: origin is sent as suggested by the spec. I've deleted a comment that I made about an alternative solution that is unrelated to the changes in this pull-request since that may have caused confusion.

If you're not satisfied with the fix that I implemented here then I would be happy to open an issue so at least this bug can be documented.

Thanks for you work on this library! 💛

[whatyouhide]

Oh, I was absolutely confused and also wrong 😄 This all makes sense. Thanks a lot for the contribution and great catch! 💟

[whatyouhide]

Release v1.1.3, thanks!

[balexand]

Thank you so much! I've really appreciated using both Corsica and Redix. 😄