Skip to content

Forbid wildcard (*) CORS headers when credentials are true - #400

Open
Vansh0204 wants to merge 1 commit into
expressjs:masterfrom
Vansh0204:fix/credentials-wildcard
Open

Vansh0204 wants to merge 1 commit into
expressjs:masterfrom
Vansh0204:fix/credentials-wildcard

Conversation

@Vansh0204

Copy link
Copy Markdown

Description

This PR addresses issue #333 by ensuring that the * wildcard is not used in Access-Control-Allow-Origin, Access-Control-Allow-Methods, Access-Control-Allow-Headers, or Access-Control-Expose-Headers response headers when Access-Control-Allow-Credentials is set to true, as required by the Fetch Standard.

Changes

  • Origin: If origin: '*' (default) is set and credentials: true, the middleware now reflects the Origin request header and adds Vary: Origin.
  • Methods: If methods: '*' is explicitly set and credentials: true, the Access-Control-Allow-Methods header is omitted from the response.
  • Allowed Headers: If allowedHeaders: '*' is explicitly set and credentials: true, the Access-Control-Allow-Headers header is omitted from the response.
  • Exposed Headers: If exposedHeaders: '*' is explicitly set and credentials: true, the Access-Control-Expose-Headers header is omitted from the response.

Verification

  • Added a new test file test/issue-333.js covering all the above scenarios.
  • All existing tests pass.
  • Code coverage remains at 100%.

@kilisamemarisaaa kilisamemarisaaa left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The new origin test is too weak to prove the proposed behavior. It only asserts that Access-Control-Allow-Origin is not '*'; an implementation that omits the header entirely would pass that assertion even though a credentialed CORS request with Origin: http://example.com should receive the reflected origin (and Vary: Origin).

Please assert the concrete contract, for example:

assert.equal(res.getHeader('Access-Control-Allow-Origin'), req.headers.origin)
assert.equal(res.getHeader('Vary'), 'Origin')

A separate request without an Origin header can cover the intentional no-header case. The methods/headers/exposed-header tests should likewise assert the complete header set rather than only notEqual('*').

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.

2 participants