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.

Sign up for GitHub

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

Jump to bottom

Add CORS middleware #532

Closed
SBrandeis opened this issue Aug 26, 2022 · 3 comments · Fixed by #538
Closed

Add CORS middleware #532

SBrandeis opened this issue Aug 26, 2022 · 3 comments · Fixed by #538
Assignees
@severo
Labels
security

Comments

@SBrandeis
Copy link

We will need it to request the dataset server from the browser (eg in AutoTrain)
@severo
Copy link
Collaborator

severo commented Aug 26, 2022

Would you want to propose a PR, as you have a clear idea of the configuration you will need? I understand it should be quick (add https://www.starlette.io/middleware/#corsmiddleware to https://github.com/huggingface/datasets-server/blob/main/services/api/src/api/app.py#L44?)

@SBrandeis SBrandeis self-assigned this Aug 26, 2022
@severo
Copy link
Collaborator

severo commented Sep 5, 2022

Mentioned here: https://github.com/huggingface/autotrain-ui/pull/312

Read https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS.

In particular, all the parts that define the expected server behavior, for example, never returning * in an Access-Control-Allow-... when responding to a credentialed request.

Also, it must respond to the OPTIONS verb.

@severo severo assigned severo and unassigned SBrandeis Sep 5, 2022
severo added a commit that referenced this issue Sep 6, 2022
@severo
feat: 🎸 support OPTIONS requests (CORS pre-flight requests)
120ddb9 
Fixes #532
@severo severo mentioned this issue Sep 6, 2022
Merged
@severo
Copy link
Collaborator

severo commented Sep 6, 2022

See #538.

severo added a commit that referenced this issue Sep 6, 2022
@severo
feat: 🎸 support OPTIONS requests (CORS pre-flight requests) (#538)
53e165e 
* feat: 🎸 support OPTIONS requests (CORS pre-flight requests)

Fixes #532

* feat: 🎸 upgrade docker images

* fix: 🐛 don't set the Access-Control-Allow-Origin in nginx

The services are responsible to set it if required (note that when
credentials are passed in the request, the Access-Control-Allow-Origin
header must not return *, but the origin)

* fix: 🐛 remove obsolete configuration

nginx no longer caches the proxied responses
mattstern31 added a commit to mattstern31/datasets-server-storage-admin that referenced this issue Nov 11, 2023
@mattstern31
feat: 🎸 support OPTIONS requests (CORS pre-flight requests) (#538)
9e5ec63 
* feat: 🎸 support OPTIONS requests (CORS pre-flight requests)

Fixes huggingface/dataset-viewer#532

* feat: 🎸 upgrade docker images

* fix: 🐛 don't set the Access-Control-Allow-Origin in nginx

The services are responsible to set it if required (note that when
credentials are passed in the request, the Access-Control-Allow-Origin
header must not return *, but the origin)

* fix: 🐛 remove obsolete configuration

nginx no longer caches the proxied responses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@severo severo

Labels
security
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: 🎸 support OPTIONS requests (CORS pre-flight requests) huggingface/dataset-viewer
2 participants
@severo @SBrandeis