spatie / laravel-cors

Send CORS headers in a Laravel application
https://spatie.be/en/opensource/laravel
MIT License
603 stars 59 forks source link

Preflight OPTIONS requests are not processed via the middleware #28

Closed mohamed-abdul-fattah closed 5 years ago

mohamed-abdul-fattah commented 6 years ago

I allow all origins by configuring allow_origins to * in cors.php config file

// cors.php

'default_profile' => [

        'allow_origins' => [
            '*',
        ],

        'allow_methods' => [
            'POST',
            'GET',
            'OPTIONS',
            'PUT',
            'PATCH',
            'DELETE',
        ],

        'allow_headers' => [
            'Content-Type',
            'X-Auth-Token',
            'Origin',
            'Authorization',
        ],

        'expose_headers' => [
            'Cache-Control',
            'Content-Language',
            'Content-Type',
            'Expires',
            'Last-Modified',
            'Pragma',
        ],

        'forbidden_response' => [
            'message' => 'Forbidden (cors).',
            'status' => 403,
        ],

        /*
         * Preflight request will respond with value for the max age header.
         */
        'max_age' => 60 * 60 * 24,
   ],

But when I send a GET request with a Content-Type: application/json header, the browser sends a preflight request with OPTIONS method, then the Access-Control-Allow-Origin header doesn't return in response headers.

[Requested Headers]
Access-Control-Request-Headers: content-type
Access-Control-Request-Method: GET
Origin: null

[Response Headers]
Allow: GET,HEAD
Cache-Control: no-cache, private
Connection: keep-alive
Content-Encoding: gzip
Content-Type: text/html; charset=UTF-8

While any simple request passes through the middleware and returns the proper response headers. This is a request with GET method and no extra headers

[Request Headers]
Origin: null

[Response Headers]
Access-Control-Allow-Origin: *
Access-Control-Expose-Headers: Cache-Control, Content-Language, Content-Type, Expires, Last-Modified, Pragma
Cache-Control: no-cache, private
Connection: keep-alive
Content-Type: application/json

This behaviour seems to be caused by the Laravel getRouteForMethods Route method which returns early with response before going through CORS middleware!

I had to write this piece of code to handle the preflight requests at the top of my api.php routes file

// CORS implementation for preflight requests
Route::options('{any}', function () {
    return response('OK', \Illuminate\Http\Response::HTTP_NO_CONTENT)
          ->header('Access-Control-Allow-Origin', implode(',', config('cors.default_profile.allow_origins')))
          ->header('Access-Control-Allow-Methods', implode(',', config('cors.default_profile.allow_methods')))
          ->header('Access-Control-Allow-Headers', implode(',', config('cors.default_profile.allow_headers')));
});
sweebee commented 6 years ago

I've got the same issue. OPTIONS doesn't return any allow-origin headers.

freekmurze commented 6 years ago

Currently a bit shy for time to fix this. If you need this fixed quick, feel free to submit a fully tested and documented PR.

sweebee commented 6 years ago

I already found my issue. I didn't want to enable it globally so i only enabled it in specific routes. That didn't work.

But extending the middleware this way works:

namespace App\Http\Middleware;

use Closure;

class CorsMiddleware extends \Spatie\Cors\Cors
{

    /**
     * The URIs that should be excluded from CORS verification.
     *
     * @var array
     */
    protected $except = [
        'api/unprotected/*'
    ];

    /**
     * Handle an incoming request.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  \Closure  $next
     * @return mixed
     */
    public function handle($request, Closure $next)
    {
        if($this->inExceptArray($request)){
            header('Access-Control-Allow-Origin: *');
            return $next($request);
        }
        return parent::handle($request, $next);
    }

    /**
     * Determine if the request has a URI that should pass through CORS verification.
     *
     * @param  \Illuminate\Http\Request  $request
     * @return bool
     */
    protected function inExceptArray($request)
    {
        foreach ($this->except as $except) {
            if ($except !== '/') {
                $except = trim($except, '/');
            }

            if ($request->fullUrlIs($except) || $request->is($except)) {
                return true;
            }
        }

        return false;
    }
}
spatie-bot commented 5 years ago

Dear contributor,

because this issue seems to be inactive for quite some time now, I've automatically closed it. If you feel this issue deserves some attention from my human colleagues feel free to reopen it.

mfeldheim commented 5 years ago

You can simply define a catch-all options route in your API routes. Adding the middleware globally will answer every OPTIONs request with cors headers

api.php

Route::group([
      'namespace' => 'Api',
      'middleware' => ['auth:api', 'cors']
   ], function() {
        // add OPTIONS route to fire cors middleware for preflight
        Route::options('{any}');
   });
lazychaser commented 4 years ago

I ran into same issue and figured that Laravel creates alternative route for OPTIONS request without middleware, and catch-all strategy will not work, only global middleware.

protected function getRouteForMethods($request, array $methods)
    {
        if ($request->method() === 'OPTIONS') {
            return (new Route('OPTIONS', $request->path(), function () use ($methods) {
                return new Response('', 200, ['Allow' => implode(',', $methods)]);
            }))->bind($request);
        }

        $this->methodNotAllowed($methods, $request->method());
    }