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 #1111

Merged
merged 4 commits into from Apr 6, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 8 additions & 3 deletions starlette/middleware/cors.py
Expand Up @@ -157,11 +157,16 @@ async def send(
# If request includes any cookie headers, then we must respond
# with the specific origin instead of '*'.
if self.allow_all_origins and has_cookie:
headers["Access-Control-Allow-Origin"] = origin
self.allow_explicit_origin(headers, origin)

# If we only allow specific origins, then we have to mirror back
# the Origin header in the response.
elif not self.allow_all_origins and self.is_allowed_origin(origin=origin):
headers["Access-Control-Allow-Origin"] = origin
headers.add_vary_header("Origin")
self.allow_explicit_origin(headers, origin)

await send(message)

@staticmethod
def allow_explicit_origin(headers: MutableHeaders, origin: str) -> None:
headers["Access-Control-Allow-Origin"] = origin
headers.add_vary_header("Origin")
27 changes: 23 additions & 4 deletions tests/middleware/test_cors.py
Expand Up @@ -245,12 +245,30 @@ def homepage(request):
assert response.headers["vary"] == "Origin"


def test_cors_vary_header_is_properly_set():
def test_cors_vary_header_is_properly_set_for_credentialed_request():
Copy link
Member

@euri10 euri10 Dec 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add another test_cors_vary_header_is_properly_set_for_not_credentialed_request ? and have the same setup as here without the Cookie sent ?

app = Starlette()

app.add_middleware(CORSMiddleware, allow_origins=["https://example.org"])
app.add_middleware(CORSMiddleware, allow_origins=["*"])

headers = {"Origin": "https://example.org"}
@app.route("/")
def homepage(request):
return PlainTextResponse(
"Homepage", status_code=200, headers={"Vary": "Accept-Encoding"}
)

client = TestClient(app)

response = client.get(
"/", headers={"Cookie": "foo=bar", "Origin": "https://someplace.org"}
)
assert response.status_code == 200
assert response.headers["vary"] == "Accept-Encoding, Origin"


def test_cors_vary_header_is_properly_set_when_allow_origins_is_not_wildcard():
app = Starlette()

app.add_middleware(CORSMiddleware, allow_origins=["https://example.org"])

@app.route("/")
def homepage(request):
Expand All @@ -260,13 +278,14 @@ def homepage(request):

client = TestClient(app)

response = client.get("/", headers=headers)
response = client.get("/", headers={"Origin": "https://example.org"})
assert response.status_code == 200
assert response.headers["vary"] == "Accept-Encoding, Origin"


def test_cors_allowed_origin_does_not_leak_between_credentialed_requests():
app = Starlette()

app.add_middleware(
CORSMiddleware, allow_origins=["*"], allow_headers=["*"], allow_methods=["*"]
)
Expand Down