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

CORS requests with credentials should forbid * #333

Open
ehmicky opened this issue Oct 19, 2024 · 2 comments
Open

CORS requests with credentials should forbid * #333

ehmicky opened this issue Oct 19, 2024 · 2 comments

Comments

@ehmicky
Copy link

ehmicky commented Oct 19, 2024

The standard forbids using * in the Access-Control-Allow-Origin, Access-Control-Expose-Headers, Access-Control-Allow-Methods, or Access-Control-Allow-Headers response header, if the Access-Control-Allow-Credentials request header is set to true.

https://fetch.spec.whatwg.org/#cors-protocol-and-credentials

https://fetch.spec.whatwg.org/#http-new-header-syntax

Right now, this module allows it. In fact, it does it by default if the credentials option is set to true.

Instead, it could either:

  • Throw an error
  • Not set CORS response headers, i.e. rejecting the CORS request
  • Use the Origin request header, if specified. The Vary: Origin response header would need to be set too then.
@shawnleetw
Copy link

Hi, I have a few years of web dev experiences including Express.

But I'm new to open-source contribution and would like to find a chance to start. Anything I could help here?

@ctcpip
Copy link
Member

ctcpip commented Jan 21, 2025

That's not entirely true... it's only not allowed if the request's credentials mode is "include". The credentials option in this library determines whether to set the response header.

In any case, where this gets tricky is that we cannot know in all cases what the credentials mode/intent is. For example, if cookies are sent, those cookies are not necessarily credentials. We would not want to break users just based on the presence of arbitrary cookies alone.

It's also probably not prudent for the framework itself to be checking client certificates in order to determine header content.

It might be safe to check authorization headers. e.g. if auth header(s) are present, then do something different. (FWIW, I haven't actually looked into what the current behavior is in this scenario.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants