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

[💡FEATURE REQUEST]: Enhance CORS support #909

Closed
tntrex opened this issue Nov 3, 2021 · 3 comments
Closed

[💡FEATURE REQUEST]: Enhance CORS support #909

tntrex opened this issue Nov 3, 2021 · 3 comments
Assignees
Labels
help-needed-easy Call for participation: Experience needed to fix: Easy / not much P-HTTP Plugin: HTTP Y-Medium Priority: Medium
Milestone

Comments

@tntrex
Copy link

tntrex commented Nov 3, 2021

Current CORS headers settings doesn't covers my use cases.

First of all I want a support of multiple allowed origins (or even any origin) with credentials enabled. Currently http.headers.cors.allowed_origin = * and http.headers.cors.allow_credentials = true is not working together.

Would be nice if allowed_origin can be configured as single origin or array of origins, and option to reflect Origin from request header (maybe combination of allowed_origin = * and allow_credentials = true or some reserved allowed_origin value). CORS middleware example in Go, that can acts like described: https://github.com/rs/cors

Second little problem is deprecation message from browser like that, when allowed_headers = *:

"Authorization" will not be covered by the wildcard symbol (*)in CORS "Access-Control-Allow-Headers" handling.

I found out that it can be reflected from Access-Control-Request-Headers request header if it specified by client.

@rustatian rustatian transferred this issue from roadrunner-server/roadrunner Nov 3, 2021
@rustatian
Copy link
Member

Hey @hustlahusky , thanks for the FR. I need more details about the described problems:

Currently http.headers.cors.allowed_origin = * and http.headers.cors.allow_credentials = true is not working together.

What do you mean by not working here? It should not work according to the standard. https://fetch.spec.whatwg.org/#cors-protocol-and-credentials

Would be nice if allowed_origin can be configured as single origin or array of origins

Access-Control-Allow-Origin header can contain only 1 origin. Here is the trick to have an array with the origins and check every incoming origin with the origins in the array and then reject the request or set the origin from the request if it's in the original allowed origins array. So yes, I guess we can use the sliced version of the allowed_origin here, that won't break the existing config.

and option to reflect Origin from request header

It should be reflected automatically if the Origin is in the allowed origins list.

"Authorization" will not be covered by the wildcard symbol (*)in CORS "Access-Control-Allow-Headers" handling.

Could you please share the http.headers configuration, so, I'd be able to reproduce that?

@tntrex
Copy link
Author

tntrex commented Nov 25, 2021

What do you mean by not working here? It should not work according to the standard. https://fetch.spec.whatwg.org/#cors-protocol-and-credentials

I mean that allowed_origin = * and allow_credentials = true generates next Access-Control-Allow-Origin: * and Access-Control-Allow-Credentials: true headers, that will never allow browsers to use credentials. I expect that RoadRunner can handle this situation for me and allow any specified in request header origin to use credentials.

It should be reflected automatically if the Origin is in the allowed origins list.

I mean RoadRunner can allow any origin reflected from request header when using asterisk in config.

Could you please share the http.headers configuration, so, I'd be able to reproduce that?

http:
  headers:
    cors:
      allowed_origin: '*'
      allowed_headers: '*'
      allowed_methods: '*'
      allow_credentials: true

@rustatian rustatian changed the title Dec 25, 2021
@rustatian rustatian transferred this issue from roadrunner-server/roadrunner-plugins Jan 16, 2022
@rustatian rustatian added P-HTTP Plugin: HTTP Y-Medium Priority: Medium labels Jan 16, 2022
@rustatian rustatian moved this to Backlog in General Jan 16, 2022
@rustatian rustatian self-assigned this May 22, 2023
@rustatian rustatian added the help-needed-easy Call for participation: Experience needed to fix: Easy / not much label May 22, 2023
@rustatian
Copy link
Member

Multiply allowed_origins now supported. Fixed by the: roadrunner-server/headers#48

RoadRunner can allow any origin reflected from request header

This is not an expected behavior (and non-standard), origins should be set explicitly. Not planned to implement.

@github-project-automation github-project-automation bot moved this from Backlog to Unreleased in General Jun 19, 2023
@rustatian rustatian added this to the v2023.2.0 milestone Jun 19, 2023
@rustatian rustatian moved this from Unreleased to Done in General Jul 6, 2023
@rustatian rustatian mentioned this issue Jul 6, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help-needed-easy Call for participation: Experience needed to fix: Easy / not much P-HTTP Plugin: HTTP Y-Medium Priority: Medium
Projects
No open projects
Status: Done
Development

No branches or pull requests

2 participants