nelmio / NelmioCorsBundle

Adds CORS (Cross-Origin Resource Sharing) headers support in your Symfony application
https://symfony.com/bundles/NelmioCorsBundle/
MIT License
1.89k stars 108 forks source link

In 1.5.6, `forced_allow_origin_value` does not behave as documented and ignores allowed domains #138

Open chrif opened 4 years ago

chrif commented 4 years ago

The response listener for forced_allow_origin_value is added before the call to checkOrigin. This results in the Access-Control-Allow-Origin header being added regardless of the allow_origin config, which is the opposite of the documented behavior:

Be aware that even if you set forced_allow_origin_value to *, if you also set allow_origin to http://example.com, only this specific domain will be allowed to access your resources.

I need the latest version of Nelmio supporting Symfony 3. I tried to use dev-master as it seems fixed there, but it requires Symfony 4. Would a patch for Symfony 3 be possible?

In the meantime I'll just extend the listener and check origin there:

final class CorsListener extends \Nelmio\CorsBundle\EventListener\CorsListener {

    public function forceAccessControlAllowOriginHeader(FilterResponseEvent $event) {
        if (!$options = $this->configurationResolver->getOptions($request = $event->getRequest())) {
            return;
        }

        if (!$this->checkOrigin($request, $options)) {
            return;
        }

        $event->getResponse()->headers->set('Access-Control-Allow-Origin', $options['forced_allow_origin_value']);
    }

}

Thanks!

Seldaek commented 4 years ago

Just tagged 2.0 out of master, so yeah there is no way back there, but could do a 1.5.7 with the fix if you can figure out what the problem is. There weren't that many changes in between https://github.com/nelmio/NelmioCorsBundle/compare/1.5.6...master Sorry right now don't have time to investigate further.

rvanlaak commented 4 years ago

Would it be possible to enhance Symfony's Http Cache so it could work together with the allow_origin policy, so we do not need to set forced_allow_origin_value ?

gndk commented 1 year ago

This is still broken, as the fix only applied to preflight requests.

Seldaek commented 1 year ago

I'm not sure here if this should be fixed or not https://github.com/nelmio/NelmioCorsBundle/pull/72 is the reason it is always set I believe - but maybe it isn't needed anymore now that we set Vary:Origin on response headers