asm89/stack-cors

Cross-origin resource sharing library and stack middleware.

stack-cors's Issues

Settings per path

Hey, following from this discussion: nelmio/NelmioCorsBundle#14 I think I'm gonna switch https://github.com/barryvdh/laravel-cors to using your CorsService (because of tests and why duplicate etc) insated of the NelmioCorsBundle I based my CORS logic on.
The biggest functional difference from your and NelmioCorsBundle, is that yours doesn't define any paths. (As far I can tell with this quick look)
Do you suggest to just allow crossdomain for every path, of should I check for the path myself? Or are you thinking about implementing this in the future?
I'm not sure how many people actually use multiple configurations for their app, but it seems more 'right' to at least define what paths are accessible for the base configuration, or do you think otherwise?
It could be just one op the options, that defaults to * or something.

Release a 1.0.1 with just https://github.com/asm89/stack-cors/commit/6daf5757971173b79ddabbd755d59d333c278f42

Hi.

Thank you for release 1.1.0 of this library.
We, as in Drupal, depend on your library, so we have a small issue regarding your library:
We don't allow minor updates in our patch updates, as this conflicts with semantic versioning.

Given that I'm wondering whether you could release a 1.0.1 version with just the bugfix (potentially more) applied.

Packagist not auto-updating

The v1.3.0 tag has not been picked up, since the packagist auto-update hooks are not setup.

Rename supports_credentials to allow_credentials

I think allowCredentials better reflects the actual purpose (and CORS error messages).

We could just check both versions in normalizeOptions to prevent BC breaks.

Always set `Vary: Origin`

Quoting fruitcake/laravel-cors#20:

In this case I tend to agree with your story and we should probably always add the Vary header.
— @asm89

If stack-cors is changed to set the Vary header even on requests without an Origin, then this laravel-cors will need to be changed to always load the middleware
— @drbild

Yes I know, if this has been changed, and #11 or similar has merged + tagged, ping me back and I'll change that.
— @barryvdh

… but actually #11 did not change this. It is still not adding the Vary: Origin header always.

Over at fruitcake/laravel-cors#20 (comment), the maintainer of this repo (@asm89) agreed that that would be sensible to do.

@asm89, would you be still open to doing that?

Composer says PHP 5.5 supported, but not tested on CI

If support for PHP 5 is going to be dropped, both composer and the CI need to be informed.

Stack middleware causes loss of existing Vary headers

If you have already set the Vary headers, it may be an array.

        if (!$response->headers->has('Vary')) {
            $response->headers->set('Vary', 'Origin');
        } else {
            $response->headers->set('Vary', $response->headers->get('Vary') . ', Origin');
        }

This tries to perform string concatenation on an array, resulting in the Vary header always be , Vary.

i don't see any tests with ports, maybe there should be?

i am successfully doing ajax requests however when uploading a file i am getting that no header is set, same domain but different port and allowing all origins. Is this a bug? if you confirm i would like to send a failing test. 😊 @asm89

False positives when detecting CORs Request

A discussion with the developers of laravel-cors and TrustedProxy lead me to believe that there are cases when stack-cors is miss identifying a request as CORS request.

In CorsService::isCorsRequest() the request is only check for an existing 'Origin' header.

Should this function not all all check if the contents of the 'Origin' match the Scheme and host of the request as well?

Example:

public function isCorsRequest(Request $request)
{
    return $request->headers->has('Origin') &&
           $request->headers->get('Origin') != $request->getSchemeAndHttpHost();
}

Right now, @barryvdh does this check when register his ServiceProvider in laravel-cors. This early cross-origin check seems to be incompatible with other ServiceProviders like @fideloper 's TrustedProxy.

Would it make more sense to do the cross-origin check in your middleware instead? Or is there a case when CORS hander would need to trigger for a request with matching scheme and host.

Nullable header values bug

Hi!

PSR-7 says that header must be string or string[] but this service sets Access-Control-Allow-Origin to null then allowedOrigins is ['*'], supportsCredentials is true and Origin header doesn't set.

I suggest to add additional check here

Tag new release

Could you tag a new release with the Symfony 3 support please?

TypeError: implode(): Argument #2 ($array) must be of type ?array, bool given in implode() (line 199 of /vendor/asm89/stack-cors/src/CorsService.php)

Error Message:

TypeError: implode(): Argument #2 ($array) must be of type ?array, bool given in implode() (line 199 of /vendor/asm89/stack-cors/src/CorsService.php)

• /vendor/asm89/stack-cors/src/CorsService.php(199): implode(', ', true)
• /vendor/asm89/stack-cors/src/CorsService.php(133): Asm89\Stack\CorsService->configureExposedHeaders(Object(Symfony\Component\HttpFoundation\JsonResponse), Object(Symfony\Component\HttpFoundation\Request))
• /vendor/asm89/stack-cors/src/Cors.php(59): Asm89\Stack\CorsService->addActualRequestHeaders(Object(Symfony\Component\HttpFoundation\JsonResponse), Object(Symfony\Component\HttpFoundation\Request))
• /docroot/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Asm89\Stack\Cors->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
• /docroot/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
• /docroot/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(51): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
• /docroot/core/lib/Drupal/Core/DrupalKernel.php(704): Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
• /docroot/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
• /vendor/drush/drush/misc/d8-rs-router.php(48): require('/Applications/M...')
• {main}

Allow manual override of the Access-Control-Allow-Origin header

Hi, and thanks for the excellent middleware.

I'm thinking about adding a check in the addActualRequestHeaders() function, to see if an Access-Control-Allow-Origin header has already been set on the response, and set it in this function only in case it hasn't been set.

Does anyone see any issue (security or other) with this?

I will be happy to provide a pull request if this makes sense to all.

Access control is unnecessary

Problem
This library attempts to control access to ensure that the request meets the allowable criteria under the config. However, this isn't necessary because the browser already does this. The most restrictive thing you can do, is doing nothing. CORS is about allowing things, not restricting them.

The preflight cannot even be cached because it has to varied by Access-Control-Request-Headers. However this isn't necessary, the config is the same regardless of what is requested.

Proposed Solution
Remove access checking which is unnecessary and breaks caching by forcing the request to be varied by the Origin.

Problem with asm89/stack-cors 1.3.0, installing barryvdh/laravel-cors

Hi! I am starting with CORS and I am following a tutorial to configure it.
I`m working with Laravel Framework 7.7.1 and I have a problem to run "composer require barryvdh / laravel-cors", I would appreciate if you could give me a clue to solve it

When I run "composer require barryvdh / laravel-cors" I get the following error:
$ composer require barryvdh/laravel-corsUsing version ^2.0 for barryvdh/laravel-cors
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.

Problem 1
- Conclusion: don't install barryvdh/laravel-cors v2.0.0
- Conclusion: don't install barryvdh/laravel-cors v2.0.0-beta3
- Conclusion: don't install barryvdh/laravel-cors v2.0.0-beta2
- Conclusion: don't install barryvdh/laravel-cors v2.0.0-beta1
- Conclusion: remove asm89/stack-cors 1.3.0
- Installation request for barryvdh/laravel-cors ^2.0 -> satisfiable by barryvdh/laravel-cors[2.0.x-dev, v2.0.0, v2.0.0-beta1, v2.0.0-beta2, v2.0.0-beta3].
- Conclusion: don't install asm89/stack-cors 1.3.0
- barryvdh/laravel-cors 2.0.x-dev requires asm89/stack-cors ^2 -> satisfiable by asm89/stack-cors[2.0.0, 2.0.0-beta1, 2.0.0-beta2, 2.0.0-beta3, 2.0.x-dev].
- Can only install one of: asm89/stack-cors[2.0.0, 1.3.0].
- Can only install one of: asm89/stack-cors[2.0.0-beta1, 1.3.0].
- Can only install one of: asm89/stack-cors[2.0.0-beta2, 1.3.0].
- Can only install one of: asm89/stack-cors[2.0.0-beta3, 1.3.0].
- Can only install one of: asm89/stack-cors[2.0.x-dev, 1.3.0].
- Installation request for asm89/stack-cors (locked at 1.3.0) -> satisfiable by asm89/stack-cors[1.3.0].

Installation failed, reverting ./composer.json to its original content.

Delete pointless develop branch

This should be deleted IMO.

Browser receiving Access-Control-Allow-Origin:* instead of actual values

I have to admit I don't fully understand how this is supposed to work but I have been having trouble with setting Access-Control-Allow-Origin on my Drupal site. This is my configuration:

  cors.config:
    enabled: true
    allowedHeaders: ['x-csrf-token','authorization','content-type','allow','origin','x-requested-with']
    allowedMethods: ['*']
    allowedOrigins: ['http://localhost:3000']
    exposedHeaders: false
    maxAge: false
    supportsCredentials: true

The response I get shows:
Access-Control-Allow-Origin: *

At first I thought it was using * and not http://localhost:3000. I tried changing the allowed origin to a fake host, just to test, and it didn't work so I know that it is actually using http://localhost:3000 instead of *. The problem for me is when I try to use 'include' for credentials, I get the following response:

Response to preflight request doesn't pass access control check: The value of the 'Access-Control-Allow-Origin' header in the response must not be the wildcard '*' when the request's credentials mode is 'include'. Origin 'http://localhost:3000' is therefore not allowed access.

Upgrade squizlabs/php_codesniffer

The version of squizlabs/php_codesniffer being used throws deprecation warning in PHP 7+ to prevent these warnings, upgrade the library.

Add regex support in allowedOrigins

I need to support review apps with dynamic domains.

This could be solved by allowedOrigins supporting regexp, or by second option allowedOriginRegEx only for regex.

It PR with this feature welcomed?

Only Vary the request when necessary

Problem
If I set allowedOrigins to * and leave all of the other options to the default, I would expect to get a rsponse like this:

Access-Control-Allow-Origin: *

instead I get this:

Access-Control-Allow-Origin: https://example.com
Vary: Origin

I realize that some of the CORS headers need the origin to be set, like Access-Control-Allow-Credentials, but if this is the only one that is being set, it means the cache is being broken unnecssarily.

Proposed Solution
To preserve caching, if possible, the middleware should seek to set the same value on every response, and not add/modify the Vary header. It should only the Vary the response if the config mandates it.

New tag

Hi Alexander,

Could you please create a new tag? I'd like to avoid requiring dev-master :)

Thanks!

William

More helpful error response

Currently when the policy is violated, only a simple "Not allowed." response is sent. I'd appreciate it if the message was a little more specific, for instance "Access denied by CORS policy". Alternatively, the ability set a custom message or even supply a custom response would be great!

How to integrate into Laravel 4 filters?

As this works with Symphony, it'll work in Laravel. How do I integrate it into the filters? Any ideas?

New tag for Symfony 3 support

I'd appreciate a new tag for Symfony 3.x support.

Thanks.