Skip to content

[v3-2-test] Make CORS allow_credentials configurable (#66503)#67181

Closed
github-actions[bot] wants to merge 1 commit into
v3-2-testfrom
backport-2723f66-v3-2-test
Closed

[v3-2-test] Make CORS allow_credentials configurable (#66503)#67181
github-actions[bot] wants to merge 1 commit into
v3-2-testfrom
backport-2723f66-v3-2-test

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

  • Make CORS allow_credentials configurable

The FastAPI CORS middleware was initialized with allow_credentials=True
hardcoded. Add a new [api] access_control_allow_credentials boolean
option (default True to preserve existing behavior) so deployments that
configure access_control_allow_origins but do not want credentialed
cross-origin requests can opt out.

  • Address review nits: bump version_added to 3.2.2, hoist imports
  • config.yml: version_added 3.2.0 -> 3.2.2 to match the backport target
  • test_app.py: move FastAPI, CORSMiddleware, init_config, and conf_vars
    imports to the top of the file instead of inline inside the test method.

(cherry picked from commit 2723f66)

Co-authored-by: Jarek Potiuk jarek@potiuk.com
Co-authored-by: vatsrahul1001 rah.sharma11@gmail.com

* Make CORS allow_credentials configurable

The FastAPI CORS middleware was initialized with allow_credentials=True
hardcoded. Add a new [api] access_control_allow_credentials boolean
option (default True to preserve existing behavior) so deployments that
configure access_control_allow_origins but do not want credentialed
cross-origin requests can opt out.

* Address review nits: bump version_added to 3.2.2, hoist imports

- config.yml: version_added 3.2.0 -> 3.2.2 to match the backport target
- test_app.py: move FastAPI, CORSMiddleware, init_config, and conf_vars
  imports to the top of the file instead of inline inside the test method.

---------
(cherry picked from commit 2723f66)

Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Co-authored-by: vatsrahul1001 <rah.sharma11@gmail.com>
Copy link
Copy Markdown
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

Lets resolve the conversion on the original PR before backporting this.

Blocking to not merge by mistake.

@pierrejeambrun
Copy link
Copy Markdown
Member

Closing, Jarek is reverting on main. That's not needed.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants