1
0
mirror of https://github.com/flarum/core.git synced 2025-07-22 17:21:27 +02:00

Refactor Route Resolving and Dispatch (#2425)

- Split DispatchRoute. This allows us to run middleware after we figure out which route we're on, but before we actually execute the controller for that route.
- By making the route name explicitly available to middlewares, applications like CSRF and floodgate can set patterns based on route names instead of the path, which is an implementation detail.
- Support using route name match for CSRF extender, deprecate path match
This commit is contained in:
Alexander Skvortsov
2020-11-10 12:52:12 -05:00
committed by GitHub
parent 29157ac2a9
commit 10e356e1b2
12 changed files with 116 additions and 31 deletions

View File

@@ -54,9 +54,10 @@ class AdminServiceProvider extends AbstractServiceProvider
HttpMiddleware\StartSession::class, HttpMiddleware\StartSession::class,
HttpMiddleware\RememberFromCookie::class, HttpMiddleware\RememberFromCookie::class,
HttpMiddleware\AuthenticateWithSession::class, HttpMiddleware\AuthenticateWithSession::class,
HttpMiddleware\CheckCsrfToken::class,
HttpMiddleware\SetLocale::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 () { $this->app->singleton('flarum.admin.handler', function () {
$pipe = new MiddlewarePipe; $pipe = new MiddlewarePipe;
@@ -75,7 +80,7 @@ class AdminServiceProvider extends AbstractServiceProvider
$pipe->pipe($this->app->make($middleware)); $pipe->pipe($this->app->make($middleware));
} }
$pipe->pipe(new HttpMiddleware\DispatchRoute($this->app->make('flarum.admin.routes'))); $pipe->pipe(new HttpMiddleware\ExecuteRoute());
return $pipe; return $pipe;
}); });

View File

@@ -51,8 +51,9 @@ class ApiServiceProvider extends AbstractServiceProvider
HttpMiddleware\RememberFromCookie::class, HttpMiddleware\RememberFromCookie::class,
HttpMiddleware\AuthenticateWithSession::class, HttpMiddleware\AuthenticateWithSession::class,
HttpMiddleware\AuthenticateWithHeader::class, HttpMiddleware\AuthenticateWithHeader::class,
HttpMiddleware\CheckCsrfToken::class,
HttpMiddleware\SetLocale::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 () { $this->app->singleton('flarum.api.handler', function () {
$pipe = new MiddlewarePipe; $pipe = new MiddlewarePipe;
@@ -71,7 +76,7 @@ class ApiServiceProvider extends AbstractServiceProvider
$pipe->pipe($this->app->make($middleware)); $pipe->pipe($this->app->make($middleware));
} }
$pipe->pipe(new HttpMiddleware\DispatchRoute($this->app->make('flarum.api.routes'))); $pipe->pipe(new HttpMiddleware\ExecuteRoute());
return $pipe; return $pipe;
}); });

View File

@@ -14,11 +14,28 @@ use Illuminate\Contracts\Container\Container;
class Csrf implements ExtenderInterface 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) public function exemptPath(string $path)
{ {
$this->csrfExemptPaths[] = $path; $this->csrfExemptRoutes[] = $path;
return $this; return $this;
} }
@@ -26,7 +43,7 @@ class Csrf implements ExtenderInterface
public function extend(Container $container, Extension $extension = null) public function extend(Container $container, Extension $extension = null)
{ {
$container->extend('flarum.http.csrfExemptPaths', function ($existingExemptPaths) { $container->extend('flarum.http.csrfExemptPaths', function ($existingExemptPaths) {
return array_merge($existingExemptPaths, $this->csrfExemptPaths); return array_merge($existingExemptPaths, $this->csrfExemptRoutes);
}); });
} }
} }

View File

@@ -64,8 +64,9 @@ class ForumServiceProvider extends AbstractServiceProvider
HttpMiddleware\StartSession::class, HttpMiddleware\StartSession::class,
HttpMiddleware\RememberFromCookie::class, HttpMiddleware\RememberFromCookie::class,
HttpMiddleware\AuthenticateWithSession::class, HttpMiddleware\AuthenticateWithSession::class,
HttpMiddleware\CheckCsrfToken::class,
HttpMiddleware\SetLocale::class, HttpMiddleware\SetLocale::class,
'flarum.forum.route_resolver',
HttpMiddleware\CheckCsrfToken::class,
HttpMiddleware\ShareErrorsFromSession::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 () { $this->app->singleton('flarum.forum.handler', function () {
$pipe = new MiddlewarePipe; $pipe = new MiddlewarePipe;
@@ -85,7 +90,7 @@ class ForumServiceProvider extends AbstractServiceProvider
$pipe->pipe($this->app->make($middleware)); $pipe->pipe($this->app->make($middleware));
} }
$pipe->pipe(new HttpMiddleware\DispatchRoute($this->app->make('flarum.forum.routes'))); $pipe->pipe(new HttpMiddleware\ExecuteRoute());
return $pipe; return $pipe;
}); });
@@ -198,8 +203,8 @@ class ForumServiceProvider extends AbstractServiceProvider
$factory = $this->app->make(RouteHandlerFactory::class); $factory = $this->app->make(RouteHandlerFactory::class);
$defaultRoute = $this->app->make('flarum.settings')->get('default_route'); $defaultRoute = $this->app->make('flarum.settings')->get('default_route');
if (isset($routes->getRouteData()[0]['GET'][$defaultRoute])) { if (isset($routes->getRouteData()[0]['GET'][$defaultRoute]['handler'])) {
$toDefaultController = $routes->getRouteData()[0]['GET'][$defaultRoute]; $toDefaultController = $routes->getRouteData()[0]['GET'][$defaultRoute]['handler'];
} else { } else {
$toDefaultController = $factory->toForum(Content\Index::class); $toDefaultController = $factory->toForum(Content\Index::class);
} }

View File

@@ -9,7 +9,7 @@
namespace Flarum\Foundation; namespace Flarum\Foundation;
use Flarum\Http\Middleware\DispatchRoute; use Flarum\Http\Middleware as HttpMiddleware;
use Flarum\Settings\SettingsRepositoryInterface; use Flarum\Settings\SettingsRepositoryInterface;
use Illuminate\Console\Command; use Illuminate\Console\Command;
use Illuminate\Contracts\Container\Container; use Illuminate\Contracts\Container\Container;
@@ -85,8 +85,9 @@ class InstalledApp implements AppInterface
$pipe = new MiddlewarePipe; $pipe = new MiddlewarePipe;
$pipe->pipe(new BasePath($this->basePath())); $pipe->pipe(new BasePath($this->basePath()));
$pipe->pipe( $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; return $pipe;
} }

View File

@@ -19,7 +19,7 @@ class HttpServiceProvider extends AbstractServiceProvider
public function register() public function register()
{ {
$this->app->singleton('flarum.http.csrfExemptPaths', function () { $this->app->singleton('flarum.http.csrfExemptPaths', function () {
return ['/api/token']; return ['token'];
}); });
$this->app->bind(Middleware\CheckCsrfToken::class, function ($app) { $this->app->bind(Middleware\CheckCsrfToken::class, function ($app) {

View File

@@ -28,7 +28,10 @@ class CheckCsrfToken implements Middleware
{ {
$path = $request->getAttribute('originalUri')->getPath(); $path = $request->getAttribute('originalUri')->getPath();
foreach ($this->exemptRoutes as $exemptRoute) { 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); return $handler->handle($request);
} }
} }

View File

@@ -0,0 +1,29 @@
<?php
/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\Http\Middleware;
use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface as Request;
use Psr\Http\Server\MiddlewareInterface as Middleware;
use Psr\Http\Server\RequestHandlerInterface as Handler;
class ExecuteRoute implements Middleware
{
/**
* Executes the route handler resolved in ResolveRoute.
*/
public function process(Request $request, Handler $handler): Response
{
$handler = $request->getAttribute('routeHandler');
$parameters = $request->getAttribute('routeParameters');
return $handler($request, $parameters);
}
}

View File

@@ -18,7 +18,7 @@ use Psr\Http\Message\ServerRequestInterface as Request;
use Psr\Http\Server\MiddlewareInterface as Middleware; use Psr\Http\Server\MiddlewareInterface as Middleware;
use Psr\Http\Server\RequestHandlerInterface as Handler; use Psr\Http\Server\RequestHandlerInterface as Handler;
class DispatchRoute implements Middleware class ResolveRoute implements Middleware
{ {
/** /**
* @var RouteCollection * @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 MethodNotAllowedException
* @throws RouteNotFoundException * @throws RouteNotFoundException
@@ -59,10 +59,12 @@ class DispatchRoute implements Middleware
case Dispatcher::METHOD_NOT_ALLOWED: case Dispatcher::METHOD_NOT_ALLOWED:
throw new MethodNotAllowedException($method); throw new MethodNotAllowedException($method);
case Dispatcher::FOUND: case Dispatcher::FOUND:
$handler = $routeInfo[1]; $request = $request
$parameters = $routeInfo[2]; ->withAttribute('routeName', $routeInfo[1]['name'])
->withAttribute('routeHandler', $routeInfo[1]['handler'])
->withAttribute('routeParameters', $routeInfo[2]);
return $handler($request, $parameters); return $handler->handle($request);
} }
} }

View File

@@ -66,7 +66,7 @@ class RouteCollection
$routeDatas = $this->routeParser->parse($path); $routeDatas = $this->routeParser->parse($path);
foreach ($routeDatas as $routeData) { foreach ($routeDatas as $routeData) {
$this->dataGenerator->addRoute($method, $routeData, $handler); $this->dataGenerator->addRoute($method, $routeData, ['name' => $name, 'handler' => $handler]);
} }
$this->reverse[$name] = $routeDatas; $this->reverse[$name] = $routeDatas;

View File

@@ -13,9 +13,7 @@ use Flarum\Foundation\AppInterface;
use Flarum\Foundation\ErrorHandling\Registry; use Flarum\Foundation\ErrorHandling\Registry;
use Flarum\Foundation\ErrorHandling\Reporter; use Flarum\Foundation\ErrorHandling\Reporter;
use Flarum\Foundation\ErrorHandling\WhoopsFormatter; use Flarum\Foundation\ErrorHandling\WhoopsFormatter;
use Flarum\Http\Middleware\DispatchRoute; use Flarum\Http\Middleware as HttpMiddleware;
use Flarum\Http\Middleware\HandleErrors;
use Flarum\Http\Middleware\StartSession;
use Flarum\Install\Console\InstallCommand; use Flarum\Install\Console\InstallCommand;
use Illuminate\Contracts\Container\Container; use Illuminate\Contracts\Container\Container;
use Laminas\Stratigility\MiddlewarePipe; use Laminas\Stratigility\MiddlewarePipe;
@@ -38,15 +36,16 @@ class Installer implements AppInterface
public function getRequestHandler() public function getRequestHandler()
{ {
$pipe = new MiddlewarePipe; $pipe = new MiddlewarePipe;
$pipe->pipe(new HandleErrors( $pipe->pipe(new HttpMiddleware\HandleErrors(
$this->container->make(Registry::class), $this->container->make(Registry::class),
$this->container->make(WhoopsFormatter::class), $this->container->make(WhoopsFormatter::class),
$this->container->tagged(Reporter::class) $this->container->tagged(Reporter::class)
)); ));
$pipe->pipe($this->container->make(StartSession::class)); $pipe->pipe($this->container->make(HttpMiddleware\StartSession::class));
$pipe->pipe( $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; return $pipe;
} }

View File

@@ -50,6 +50,7 @@ class CsrfTest extends TestCase
/** /**
* @test * @test
* @deprecated
*/ */
public function create_user_post_doesnt_need_csrf_token_if_whitelisted() public function create_user_post_doesnt_need_csrf_token_if_whitelisted()
{ {
@@ -82,19 +83,37 @@ class CsrfTest extends TestCase
/** /**
* @test * @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(); $this->prepDb();
$response = $this->send( $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 * @test
* @deprecated
*/ */
public function csrf_matches_wildcards_properly() public function csrf_matches_wildcards_properly()
{ {