Adding after middleware to ControllerCollection does not add preflight headers

[bburnichon]

For one of my applications, I do need CORS handling but I can not use it for all routes.

I then added the after middleware to some routes.

The OPTIONS routes are properly added but then, the after $app['cors'] is not called on these generated routes. The consequence is that although the OPTIONS route exists, I does not return Access-Control-Allow-Origin header on the preflight calls.

Are there any ways to add the OPTIONS routes to ControllerCollections instead of after all routes are added?

[jdesrosiers]

I can't believe I never realized this wasn't working. The fix isn't going to be straight forward. I'm still working toward a good solution, but here is the work around I can give you for now.

<?php

require __DIR__ . "/vendor/autoload.php";

function generateRouteName($methods, $path)
{
    $methods = implode('_', $methods).'_';

    $routeName = $methods.$path;
    $routeName = str_replace(array('/', ':', '|', '-'), '_', $routeName);
    $routeName = preg_replace('/[^a-z0-9A-Z_.]+/', '', $routeName);

    // Collapse consecutive underscores down into a single underscore.
    $routeName = preg_replace('/_+/', '_', $routeName);

    return $routeName;
}

$app = new Silex\Application();
$app["debug"] = true;

$app->register(new JDesrosiers\Silex\Provider\CorsServiceProvider());

// CORS enabled controllers
$fooControllers = $app["controllers_factory"];
$fooControllers->post("/", function () use ($app) {
    return $app->json("foo");
});
$fooControllers->before(function (Symfony\Component\HttpFoundation\Request $request) use ($app) {
    // Add a line like this for each route
    $app["routes"]->get(generateRouteName(["OPTIONS"], "/foo/"))->after($app["cors"]);
}); 
$fooControllers->after($app["cors"]);
$app->mount("/foo", $fooControllers);

// Not CORS enabled controllers
$app->post("/bar/", function () use ($app) {
     return $app->json("bar");
}); 

$app->run();

In this example only POST /foo/ would have CORS enabled. POST /bar/ would not. In the before function you will need to add a line for each OPTIONS route you need to support.

I'll keep working on a better solution. But, I won't have a lot of time to invest in it for a while.

[tarlepp]

Any progress with this? It seems that I can't get that example working with my application :(

[jdesrosiers]

Sorry, no progress. It's a complicated problem and I haven't had the free time to invest in finding a solution.

[tarlepp]

damn, any ideas / examples how to get that working on structure like this: https://github.com/tarlepp/silex-backend ?

[jdesrosiers]

I took a look at your project. I'm not seeing anywhere in your application where you add the "cors" after-middleware (re-read the Usage section of the README again if you don't know what I'm talking about). When you register the CorsServiceProvider, it automatically sets up OPTIONS routes that are needed for CORS, but you have to explicitly add the "cors" after-middleware to the routes you want CORS functionality to apply to. Usually it is all routes and it is added to the whole application. I think that is what you want. This issue relates to applying the "cors" after-middleware to only a subset of the routes in an application. It doesn't look like that is what you are trying to do.

[tarlepp]

Register of provider is founded: https://github.com/tarlepp/silex-backend/blob/master/src%2FApp%2FApplication.php#L154

And main application after: https://github.com/tarlepp/silex-backend/blob/master/src%2FApp%2FApplication.php#L86

And when I'm using mounts like this: https://github.com/tarlepp/silex-backend/blob/master/src%2FApp%2FControllerProvider.php#L100-L102

Example solution above doesn't work.

[jdesrosiers]

Sorry, I missed that in my scan of the code and the Github search didn't show it either. Thanks for pointing it out.

However, it still doesn't look like the problem you are having is related to this issue. There is no known problem when setting the "cors" after-middleware on the whole application. You are using the provider in a straightforward way that is known to work.

If I have time later, I'll run try to run your project and see what happens.

[jdesrosiers]

I just had another thought. I noticed that you are using the SecurityServiceProvider. The issue you are having could be related to that. Checkout issue https://github.com/jdesrosiers/silex-cors-provider/issues/18 for more information and a possible solution.

[alcalyn]

Hi,

Still getting the issue, I found another workaround.

I think the logic is more to listen for preflight request before the application, then send the response to the OPTIONS HTTP request directly, depending on the cors configuration.

I implemented the solution here: https://github.com/eole-io/sandstone-fullstack/blob/f8cd60c541d6e5dfd2a23626c9cf0bfb3249b0d1/src/App/Application.php#L52-L75

I add it here again:

// Register CorsServiceProvider as usual
$this->register(new \JDesrosiers\Silex\Provider\CorsServiceProvider(), $this['environment']['cors']);
$this->after($this['cors']);

// Adding the preflight checker
$this->on(KernelEvents::REQUEST, function (GetResponseEvent $event) {
    $request = $event->getRequest();

    $isPreflightRequest =
        $request->getMethod() === 'OPTIONS' &&
        $request->headers->has("Access-Control-Request-Method")
    ;

    if ($isPreflightRequest) {
        $response = new Response();
        $response->setStatusCode(Response::HTTP_OK);
        $response->headers->add([
            'Access-Control-Allow-Headers' => $request->headers->get('Access-Control-Request-Headers'),
            'Access-Control-Allow-Methods' => !is_null($this['cors.allowMethods']) ? $this['cors.allowMethods'] : $request->headers->get('Access-Control-Request-Method'),
            'Access-Control-Max-Age' => $this['cors.maxAge'],
            'Access-Control-Allow-Origin' => $this['cors.allowOrigin'],
            'Access-Control-Allow-Credentials' => true === $this['cors.allowCredentials'] ? 'true' : null,
        ]);
        $event->setResponse($response);
    }
    return null;
}, 128);

I use $app->on instead of $app->before to get the event instance, and a higher priority over the router listener.

If you're ok with this strategy, I could take time to submit a PR. This part of code could replace the one in Cors class.

[jdesrosiers]

Thanks. I'm looking into it now.

[jdesrosiers]

This doesn't appear to address the bug this issue describes. The issue is with using $this["cors"] on a ControllerCollection instead of on an Application. Maybe you are encountering another bug I'm not aware of?

[alcalyn]

Ah ok didn't seen it was related to a specific ControllerCollection. I'll try to reproduce the bug and identify where it occurs exactly, because I had a case with, even using $app->after($app["cors"]);, the preflight request were not well handled.

[alcalyn]

So different to this issue, I opened a new one: https://github.com/jdesrosiers/silex-cors-provider/issues/33

[jdesrosiers]

I just pushed a fix for this. I added a new service cors-enabled that can be used instead of cors to enable CORS for controller collections and controllers.