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

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;
         });

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;
         });

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);
         });
     }
 }

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);
         }

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;
     }

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) {

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);
             }
         }

src/Http/Middleware/ExecuteRoute.php

@@ -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);
+    }
+}

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);
         }
     }
 

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;

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;
     }

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()
     {