diff --git a/framework/core/src/Admin/AdminServiceProvider.php b/framework/core/src/Admin/AdminServiceProvider.php index 8e8e598f7..443a2766e 100644 --- a/framework/core/src/Admin/AdminServiceProvider.php +++ b/framework/core/src/Admin/AdminServiceProvider.php @@ -54,9 +54,10 @@ class AdminServiceProvider extends AbstractServiceProvider HttpMiddleware\StartSession::class, HttpMiddleware\RememberFromCookie::class, HttpMiddleware\AuthenticateWithSession::class, - HttpMiddleware\CheckCsrfToken::class, HttpMiddleware\SetLocale::class, - Middleware\RequireAdministrateAbility::class, + 'flarum.admin.route_resolver', + HttpMiddleware\CheckCsrfToken::class, + Middleware\RequireAdministrateAbility::class ]; }); @@ -68,6 +69,10 @@ class AdminServiceProvider extends AbstractServiceProvider ); }); + $this->app->bind('flarum.admin.route_resolver', function () { + return new HttpMiddleware\ResolveRoute($this->app->make('flarum.admin.routes')); + }); + $this->app->singleton('flarum.admin.handler', function () { $pipe = new MiddlewarePipe; @@ -75,7 +80,7 @@ class AdminServiceProvider extends AbstractServiceProvider $pipe->pipe($this->app->make($middleware)); } - $pipe->pipe(new HttpMiddleware\DispatchRoute($this->app->make('flarum.admin.routes'))); + $pipe->pipe(new HttpMiddleware\ExecuteRoute()); return $pipe; }); diff --git a/framework/core/src/Api/ApiServiceProvider.php b/framework/core/src/Api/ApiServiceProvider.php index aa2e53eae..af74b2e4c 100644 --- a/framework/core/src/Api/ApiServiceProvider.php +++ b/framework/core/src/Api/ApiServiceProvider.php @@ -51,8 +51,9 @@ class ApiServiceProvider extends AbstractServiceProvider HttpMiddleware\RememberFromCookie::class, HttpMiddleware\AuthenticateWithSession::class, HttpMiddleware\AuthenticateWithHeader::class, - HttpMiddleware\CheckCsrfToken::class, HttpMiddleware\SetLocale::class, + 'flarum.api.route_resolver', + HttpMiddleware\CheckCsrfToken::class ]; }); @@ -64,6 +65,10 @@ class ApiServiceProvider extends AbstractServiceProvider ); }); + $this->app->bind('flarum.api.route_resolver', function () { + return new HttpMiddleware\ResolveRoute($this->app->make('flarum.api.routes')); + }); + $this->app->singleton('flarum.api.handler', function () { $pipe = new MiddlewarePipe; @@ -71,7 +76,7 @@ class ApiServiceProvider extends AbstractServiceProvider $pipe->pipe($this->app->make($middleware)); } - $pipe->pipe(new HttpMiddleware\DispatchRoute($this->app->make('flarum.api.routes'))); + $pipe->pipe(new HttpMiddleware\ExecuteRoute()); return $pipe; }); diff --git a/framework/core/src/Extend/Csrf.php b/framework/core/src/Extend/Csrf.php index 7b8bd54b1..bc8cffdcf 100644 --- a/framework/core/src/Extend/Csrf.php +++ b/framework/core/src/Extend/Csrf.php @@ -14,11 +14,28 @@ use Illuminate\Contracts\Container\Container; class Csrf implements ExtenderInterface { - protected $csrfExemptPaths = []; + protected $csrfExemptRoutes = []; + /** + * Exempt a named route from CSRF checks. + * + * @param string $routeName + */ + public function exemptRoute(string $routeName) + { + $this->csrfExemptRoutes[] = $routeName; + + return $this; + } + + /** + * Exempt a path from csrf checks. Wildcards are supported. + * + * @deprecated beta 15, remove beta 16. Exempt routes should be used instead. + */ public function exemptPath(string $path) { - $this->csrfExemptPaths[] = $path; + $this->csrfExemptRoutes[] = $path; return $this; } @@ -26,7 +43,7 @@ class Csrf implements ExtenderInterface public function extend(Container $container, Extension $extension = null) { $container->extend('flarum.http.csrfExemptPaths', function ($existingExemptPaths) { - return array_merge($existingExemptPaths, $this->csrfExemptPaths); + return array_merge($existingExemptPaths, $this->csrfExemptRoutes); }); } } diff --git a/framework/core/src/Forum/ForumServiceProvider.php b/framework/core/src/Forum/ForumServiceProvider.php index bbe609fa5..454408c73 100644 --- a/framework/core/src/Forum/ForumServiceProvider.php +++ b/framework/core/src/Forum/ForumServiceProvider.php @@ -64,8 +64,9 @@ class ForumServiceProvider extends AbstractServiceProvider HttpMiddleware\StartSession::class, HttpMiddleware\RememberFromCookie::class, HttpMiddleware\AuthenticateWithSession::class, - HttpMiddleware\CheckCsrfToken::class, HttpMiddleware\SetLocale::class, + 'flarum.forum.route_resolver', + HttpMiddleware\CheckCsrfToken::class, HttpMiddleware\ShareErrorsFromSession::class ]; }); @@ -78,6 +79,10 @@ class ForumServiceProvider extends AbstractServiceProvider ); }); + $this->app->bind('flarum.forum.route_resolver', function () { + return new HttpMiddleware\ResolveRoute($this->app->make('flarum.forum.routes')); + }); + $this->app->singleton('flarum.forum.handler', function () { $pipe = new MiddlewarePipe; @@ -85,7 +90,7 @@ class ForumServiceProvider extends AbstractServiceProvider $pipe->pipe($this->app->make($middleware)); } - $pipe->pipe(new HttpMiddleware\DispatchRoute($this->app->make('flarum.forum.routes'))); + $pipe->pipe(new HttpMiddleware\ExecuteRoute()); return $pipe; }); @@ -198,8 +203,8 @@ class ForumServiceProvider extends AbstractServiceProvider $factory = $this->app->make(RouteHandlerFactory::class); $defaultRoute = $this->app->make('flarum.settings')->get('default_route'); - if (isset($routes->getRouteData()[0]['GET'][$defaultRoute])) { - $toDefaultController = $routes->getRouteData()[0]['GET'][$defaultRoute]; + if (isset($routes->getRouteData()[0]['GET'][$defaultRoute]['handler'])) { + $toDefaultController = $routes->getRouteData()[0]['GET'][$defaultRoute]['handler']; } else { $toDefaultController = $factory->toForum(Content\Index::class); } diff --git a/framework/core/src/Foundation/InstalledApp.php b/framework/core/src/Foundation/InstalledApp.php index 705156292..5fbccf656 100644 --- a/framework/core/src/Foundation/InstalledApp.php +++ b/framework/core/src/Foundation/InstalledApp.php @@ -9,7 +9,7 @@ namespace Flarum\Foundation; -use Flarum\Http\Middleware\DispatchRoute; +use Flarum\Http\Middleware as HttpMiddleware; use Flarum\Settings\SettingsRepositoryInterface; use Illuminate\Console\Command; use Illuminate\Contracts\Container\Container; @@ -85,8 +85,9 @@ class InstalledApp implements AppInterface $pipe = new MiddlewarePipe; $pipe->pipe(new BasePath($this->basePath())); $pipe->pipe( - new DispatchRoute($this->container->make('flarum.update.routes')) + new HttpMiddleware\ResolveRoute($this->container->make('flarum.update.routes')) ); + $pipe->pipe(new HttpMiddleware\ExecuteRoute()); return $pipe; } diff --git a/framework/core/src/Http/HttpServiceProvider.php b/framework/core/src/Http/HttpServiceProvider.php index aac4b3196..4a7d71b24 100644 --- a/framework/core/src/Http/HttpServiceProvider.php +++ b/framework/core/src/Http/HttpServiceProvider.php @@ -19,7 +19,7 @@ class HttpServiceProvider extends AbstractServiceProvider public function register() { $this->app->singleton('flarum.http.csrfExemptPaths', function () { - return ['/api/token']; + return ['token']; }); $this->app->bind(Middleware\CheckCsrfToken::class, function ($app) { diff --git a/framework/core/src/Http/Middleware/CheckCsrfToken.php b/framework/core/src/Http/Middleware/CheckCsrfToken.php index e0a389f4e..c9f6f9494 100644 --- a/framework/core/src/Http/Middleware/CheckCsrfToken.php +++ b/framework/core/src/Http/Middleware/CheckCsrfToken.php @@ -28,7 +28,10 @@ class CheckCsrfToken implements Middleware { $path = $request->getAttribute('originalUri')->getPath(); foreach ($this->exemptRoutes as $exemptRoute) { - if (fnmatch($exemptRoute, $path)) { + /** + * @deprecated path match should be removed in beta 16, only route name match should be supported. + */ + if ($exemptRoute === $request->getAttribute('routeName') || fnmatch($exemptRoute, $path)) { return $handler->handle($request); } } diff --git a/framework/core/src/Http/Middleware/ExecuteRoute.php b/framework/core/src/Http/Middleware/ExecuteRoute.php new file mode 100644 index 000000000..fb5c4a1a5 --- /dev/null +++ b/framework/core/src/Http/Middleware/ExecuteRoute.php @@ -0,0 +1,29 @@ +getAttribute('routeHandler'); + $parameters = $request->getAttribute('routeParameters'); + + return $handler($request, $parameters); + } +} diff --git a/framework/core/src/Http/Middleware/DispatchRoute.php b/framework/core/src/Http/Middleware/ResolveRoute.php similarity index 81% rename from framework/core/src/Http/Middleware/DispatchRoute.php rename to framework/core/src/Http/Middleware/ResolveRoute.php index dd5090051..cc203a0b7 100644 --- a/framework/core/src/Http/Middleware/DispatchRoute.php +++ b/framework/core/src/Http/Middleware/ResolveRoute.php @@ -18,7 +18,7 @@ use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Http\Server\MiddlewareInterface as Middleware; use Psr\Http\Server\RequestHandlerInterface as Handler; -class DispatchRoute implements Middleware +class ResolveRoute implements Middleware { /** * @var RouteCollection @@ -41,7 +41,7 @@ class DispatchRoute implements Middleware } /** - * Dispatch the given request to our route collection. + * Resolve the given request from our route collection. * * @throws MethodNotAllowedException * @throws RouteNotFoundException @@ -59,10 +59,12 @@ class DispatchRoute implements Middleware case Dispatcher::METHOD_NOT_ALLOWED: throw new MethodNotAllowedException($method); case Dispatcher::FOUND: - $handler = $routeInfo[1]; - $parameters = $routeInfo[2]; + $request = $request + ->withAttribute('routeName', $routeInfo[1]['name']) + ->withAttribute('routeHandler', $routeInfo[1]['handler']) + ->withAttribute('routeParameters', $routeInfo[2]); - return $handler($request, $parameters); + return $handler->handle($request); } } diff --git a/framework/core/src/Http/RouteCollection.php b/framework/core/src/Http/RouteCollection.php index 87fc44d35..c59fef4c2 100644 --- a/framework/core/src/Http/RouteCollection.php +++ b/framework/core/src/Http/RouteCollection.php @@ -66,7 +66,7 @@ class RouteCollection $routeDatas = $this->routeParser->parse($path); foreach ($routeDatas as $routeData) { - $this->dataGenerator->addRoute($method, $routeData, $handler); + $this->dataGenerator->addRoute($method, $routeData, ['name' => $name, 'handler' => $handler]); } $this->reverse[$name] = $routeDatas; diff --git a/framework/core/src/Install/Installer.php b/framework/core/src/Install/Installer.php index 6ca95121e..2765a1232 100644 --- a/framework/core/src/Install/Installer.php +++ b/framework/core/src/Install/Installer.php @@ -13,9 +13,7 @@ use Flarum\Foundation\AppInterface; use Flarum\Foundation\ErrorHandling\Registry; use Flarum\Foundation\ErrorHandling\Reporter; use Flarum\Foundation\ErrorHandling\WhoopsFormatter; -use Flarum\Http\Middleware\DispatchRoute; -use Flarum\Http\Middleware\HandleErrors; -use Flarum\Http\Middleware\StartSession; +use Flarum\Http\Middleware as HttpMiddleware; use Flarum\Install\Console\InstallCommand; use Illuminate\Contracts\Container\Container; use Laminas\Stratigility\MiddlewarePipe; @@ -38,15 +36,16 @@ class Installer implements AppInterface public function getRequestHandler() { $pipe = new MiddlewarePipe; - $pipe->pipe(new HandleErrors( + $pipe->pipe(new HttpMiddleware\HandleErrors( $this->container->make(Registry::class), $this->container->make(WhoopsFormatter::class), $this->container->tagged(Reporter::class) )); - $pipe->pipe($this->container->make(StartSession::class)); + $pipe->pipe($this->container->make(HttpMiddleware\StartSession::class)); $pipe->pipe( - new DispatchRoute($this->container->make('flarum.install.routes')) + new HttpMiddleware\ResolveRoute($this->container->make('flarum.install.routes')) ); + $pipe->pipe(new HttpMiddleware\ExecuteRoute()); return $pipe; } diff --git a/framework/core/tests/integration/extenders/CsrfTest.php b/framework/core/tests/integration/extenders/CsrfTest.php index 8684b5b03..3219ab48e 100644 --- a/framework/core/tests/integration/extenders/CsrfTest.php +++ b/framework/core/tests/integration/extenders/CsrfTest.php @@ -50,6 +50,7 @@ class CsrfTest extends TestCase /** * @test + * @deprecated */ public function create_user_post_doesnt_need_csrf_token_if_whitelisted() { @@ -82,19 +83,37 @@ class CsrfTest extends TestCase /** * @test */ - public function post_to_unknown_route_will_cause_400_error_without_csrf_override() + public function create_user_post_doesnt_need_csrf_token_if_whitelisted_via_routename() { + $this->extend( + (new Extend\Csrf) + ->exemptRoute('users.create') + ); + $this->prepDb(); $response = $this->send( - $this->request('POST', '/api/fake/route/i/made/up') + $this->request('POST', '/api/users', [ + 'json' => [ + 'data' => [ + 'attributes' => $this->testUser + ] + ], + ]) ); - $this->assertEquals(400, $response->getStatusCode()); + $this->assertEquals(201, $response->getStatusCode()); + + $user = User::where('username', $this->testUser['username'])->firstOrFail(); + + $this->assertEquals(0, $user->is_email_confirmed); + $this->assertEquals($this->testUser['username'], $user->username); + $this->assertEquals($this->testUser['email'], $user->email); } /** * @test + * @deprecated */ public function csrf_matches_wildcards_properly() {