diff --git a/framework/core/src/Api/ApiServiceProvider.php b/framework/core/src/Api/ApiServiceProvider.php index af74b2e4c..4f3cfe37b 100644 --- a/framework/core/src/Api/ApiServiceProvider.php +++ b/framework/core/src/Api/ApiServiceProvider.php @@ -42,6 +42,20 @@ class ApiServiceProvider extends AbstractServiceProvider return $routes; }); + $this->app->singleton('flarum.api.throttlers', function () { + return [ + 'bypassThrottlingAttribute' => function ($request) { + if ($request->getAttribute('bypassThrottling')) { + return false; + } + } + ]; + }); + + $this->app->bind(Middleware\ThrottleApi::class, function ($app) { + return new Middleware\ThrottleApi($app->make('flarum.api.throttlers')); + }); + $this->app->singleton('flarum.api.middleware', function () { return [ 'flarum.api.error_handler', @@ -53,7 +67,8 @@ class ApiServiceProvider extends AbstractServiceProvider HttpMiddleware\AuthenticateWithHeader::class, HttpMiddleware\SetLocale::class, 'flarum.api.route_resolver', - HttpMiddleware\CheckCsrfToken::class + HttpMiddleware\CheckCsrfToken::class, + Middleware\ThrottleApi::class ]; }); diff --git a/framework/core/src/Api/Controller/CreateDiscussionController.php b/framework/core/src/Api/Controller/CreateDiscussionController.php index 0a325ee29..e4c8b7717 100644 --- a/framework/core/src/Api/Controller/CreateDiscussionController.php +++ b/framework/core/src/Api/Controller/CreateDiscussionController.php @@ -64,6 +64,9 @@ class CreateDiscussionController extends AbstractCreateController $actor = $request->getAttribute('actor'); $ipAddress = Arr::get($request->getServerParams(), 'REMOTE_ADDR', '127.0.0.1'); + /** + * @deprecated, remove in beta 15. + */ if (! $request->getAttribute('bypassFloodgate')) { $this->floodgate->assertNotFlooding($actor); } diff --git a/framework/core/src/Api/Controller/CreatePostController.php b/framework/core/src/Api/Controller/CreatePostController.php index fd46de865..8209daede 100644 --- a/framework/core/src/Api/Controller/CreatePostController.php +++ b/framework/core/src/Api/Controller/CreatePostController.php @@ -65,6 +65,9 @@ class CreatePostController extends AbstractCreateController $discussionId = Arr::get($data, 'relationships.discussion.data.id'); $ipAddress = Arr::get($request->getServerParams(), 'REMOTE_ADDR', '127.0.0.1'); + /** + * @deprecated, remove in beta 15. + */ if (! $request->getAttribute('bypassFloodgate')) { $this->floodgate->assertNotFlooding($actor); } diff --git a/framework/core/src/Api/Middleware/ThrottleApi.php b/framework/core/src/Api/Middleware/ThrottleApi.php new file mode 100644 index 000000000..f85b345ab --- /dev/null +++ b/framework/core/src/Api/Middleware/ThrottleApi.php @@ -0,0 +1,57 @@ +throttlers = $throttlers; + } + + public function process(Request $request, Handler $handler): Response + { + if ($this->throttle($request)) { + throw new FloodingException; + } + + return $handler->handle($request); + } + + /** + * @return bool + */ + public function throttle(Request $request): bool + { + $throttle = false; + foreach ($this->throttlers as $throttler) { + $result = $throttler($request); + + // Explicitly returning false overrides all throttling. + // Explicitly returning true marks the request to be throttled. + // Anything else is ignored. + if ($result === false) { + return false; + } elseif ($result === true) { + $throttle = true; + } + } + + return $throttle; + } +} diff --git a/framework/core/src/Extend/ThrottleApi.php b/framework/core/src/Extend/ThrottleApi.php new file mode 100644 index 000000000..14ac744cd --- /dev/null +++ b/framework/core/src/Extend/ThrottleApi.php @@ -0,0 +1,74 @@ +getAttribute('actor')` can be used to get the current user. + * `$request->getAttribute('routeName')` can be used to get the current route. + * Please note that every throttler runs by default on every route. + * If you only want to throttle certain routes, you'll need to check for that inside your logic. + * + * The callable should return one of: + * - `false`: This marks the request as NOT to be throttled. It overrides all other throttlers + * - `true`: This marks the request as to be throttled. + * All other outputs will be ignored. + * + * @return self + */ + public function set(string $name, $callback) + { + $this->setThrottlers[$name] = $callback; + + return $this; + } + + /** + * Remove a throttler registered with this name. + * + * @param string $name: The name of the throttler to remove. + * + * @return self + */ + public function remove(string $name) + { + $this->removeThrottlers[] = $name; + + return $this; + } + + public function extend(Container $container, Extension $extension = null) + { + $container->extend('flarum.api.throttlers', function ($throttlers) use ($container) { + $throttlers = array_diff_key($throttlers, array_flip($this->removeThrottlers)); + + foreach ($this->setThrottlers as $name => $throttler) { + $throttlers[$name] = ContainerUtil::wrapCallback($throttler, $container); + } + + return $throttlers; + }); + } +} diff --git a/framework/core/src/Http/Middleware/AuthenticateWithHeader.php b/framework/core/src/Http/Middleware/AuthenticateWithHeader.php index ae74cd769..90e8bee40 100644 --- a/framework/core/src/Http/Middleware/AuthenticateWithHeader.php +++ b/framework/core/src/Http/Middleware/AuthenticateWithHeader.php @@ -38,7 +38,7 @@ class AuthenticateWithHeader implements Middleware $actor = $key->user ?? $this->getUser($userId); $request = $request->withAttribute('apiKey', $key); - $request = $request->withAttribute('bypassFloodgate', true); + $request = $request->withAttribute('bypassThrottling', true); } elseif ($token = AccessToken::find($id)) { $token->touch(); diff --git a/framework/core/src/Post/Floodgate.php b/framework/core/src/Post/Floodgate.php index d9e3e9f41..55374964f 100644 --- a/framework/core/src/Post/Floodgate.php +++ b/framework/core/src/Post/Floodgate.php @@ -15,6 +15,9 @@ use Flarum\Post\Exception\FloodingException; use Flarum\User\User; use Illuminate\Contracts\Events\Dispatcher; +/** + * @deprecated beta 14, removed beta 15 in favor of Floodgate middleware + */ class Floodgate { /** diff --git a/framework/core/src/Post/PostServiceProvider.php b/framework/core/src/Post/PostServiceProvider.php index be1c9d7c7..37308d558 100644 --- a/framework/core/src/Post/PostServiceProvider.php +++ b/framework/core/src/Post/PostServiceProvider.php @@ -9,11 +9,38 @@ namespace Flarum\Post; +use DateTime; use Flarum\Event\ConfigurePostTypes; use Flarum\Foundation\AbstractServiceProvider; class PostServiceProvider extends AbstractServiceProvider { + /** + * {@inheritdoc} + */ + public function register() + { + $this->app->extend('flarum.api.throttlers', function ($throttlers) { + $throttlers['postTimeout'] = function ($request) { + if (! in_array($request->getAttribute('routeName'), ['discussions.create', 'posts.create'])) { + return; + } + + $actor = $request->getAttribute('actor'); + + if ($actor->can('postWithoutThrottle')) { + return false; + } + + if (Post::where('user_id', $actor->id)->where('created_at', '>=', new DateTime('-10 seconds'))->exists()) { + return true; + } + }; + + return $throttlers; + }); + } + /** * {@inheritdoc} */ diff --git a/framework/core/tests/integration/api/authentication/WithApiKeyTest.php b/framework/core/tests/integration/api/authentication/WithApiKeyTest.php index 4a07c5e42..37e9bf3ca 100644 --- a/framework/core/tests/integration/api/authentication/WithApiKeyTest.php +++ b/framework/core/tests/integration/api/authentication/WithApiKeyTest.php @@ -28,6 +28,9 @@ class WithApiKeyTest extends TestCase $this->adminUser(), $this->normalUser(), ], + 'group_permission' => [ + ['permission' => 'viewUserList', 'group_id' => 3] + ], 'api_keys' => [], ]); } diff --git a/framework/core/tests/integration/api/discussions/CreateTest.php b/framework/core/tests/integration/api/discussions/CreateTest.php index 19d289e5e..471bf1a2c 100644 --- a/framework/core/tests/integration/api/discussions/CreateTest.php +++ b/framework/core/tests/integration/api/discussions/CreateTest.php @@ -27,13 +27,20 @@ class CreateTest extends TestCase 'posts' => [], 'users' => [ $this->adminUser(), + $this->normalUser(), ], 'groups' => [ $this->adminGroup(), + $this->memberGroup(), ], 'group_user' => [ ['user_id' => 1, 'group_id' => 1], + ['user_id' => 2, 'group_id' => 3], ], + 'group_permission' => [ + ['permission' => 'viewDiscussions', 'group_id' => 3], + ['permission' => 'startDiscussion', 'group_id' => 3], + ] ]); } @@ -137,4 +144,76 @@ class CreateTest extends TestCase $this->assertEquals('test - too-obscure', $discussion->title); $this->assertEquals('test - too-obscure', Arr::get($data, 'data.attributes.title')); } + + /** + * @test + */ + public function discussion_creation_limited_by_throttler() + { + $this->send( + $this->request('POST', '/api/discussions', [ + 'authenticatedAs' => 2, + 'json' => [ + 'data' => [ + 'attributes' => [ + 'title' => 'test - too-obscure', + 'content' => 'predetermined content for automated testing - too-obscure', + ], + ] + ], + ]) + ); + + $response = $this->send( + $this->request('POST', '/api/discussions', [ + 'authenticatedAs' => 2, + 'json' => [ + 'data' => [ + 'attributes' => [ + 'title' => 'test - too-obscure', + 'content' => 'Second predetermined content for automated testing - too-obscure', + ], + ] + ], + ]) + ); + + $this->assertEquals(429, $response->getStatusCode()); + } + + /** + * @test + */ + public function throttler_doesnt_apply_to_admin() + { + $this->send( + $this->request('POST', '/api/discussions', [ + 'authenticatedAs' => 1, + 'json' => [ + 'data' => [ + 'attributes' => [ + 'title' => 'test - too-obscure', + 'content' => 'predetermined content for automated testing - too-obscure', + ], + ] + ], + ]) + ); + + $response = $this->send( + $this->request('POST', '/api/discussions', [ + 'authenticatedAs' => 1, + 'json' => [ + 'data' => [ + 'attributes' => [ + 'title' => 'test - too-obscure', + 'content' => 'Second predetermined content for automated testing - too-obscure', + ], + ] + ], + ]) + ); + + $this->assertEquals(201, $response->getStatusCode()); + } } diff --git a/framework/core/tests/integration/api/posts/CreateTest.php b/framework/core/tests/integration/api/posts/CreateTest.php index bad67ba42..613810078 100644 --- a/framework/core/tests/integration/api/posts/CreateTest.php +++ b/framework/core/tests/integration/api/posts/CreateTest.php @@ -64,4 +64,44 @@ class CreateTest extends TestCase $this->assertEquals(201, $response->getStatusCode()); } + + /** + * @test + */ + public function limited_by_throttler() + { + $this->send( + $this->request('POST', '/api/posts', [ + 'authenticatedAs' => 2, + 'json' => [ + 'data' => [ + 'attributes' => [ + 'content' => 'reply with predetermined content for automated testing - too-obscure', + ], + 'relationships' => [ + 'discussion' => ['data' => ['id' => 1]], + ], + ], + ], + ]) + ); + + $response = $this->send( + $this->request('POST', '/api/posts', [ + 'authenticatedAs' => 2, + 'json' => [ + 'data' => [ + 'attributes' => [ + 'content' => 'Second reply with predetermined content for automated testing - too-obscure', + ], + 'relationships' => [ + 'discussion' => ['data' => ['id' => 1]], + ], + ], + ], + ]) + ); + + $this->assertEquals(429, $response->getStatusCode()); + } } diff --git a/framework/core/tests/integration/extenders/MailTest.php b/framework/core/tests/integration/extenders/MailTest.php index 42e843853..7a8a1dc52 100644 --- a/framework/core/tests/integration/extenders/MailTest.php +++ b/framework/core/tests/integration/extenders/MailTest.php @@ -29,6 +29,12 @@ class MailTest extends TestCase 'users' => [ $this->adminUser(), ], + 'groups' => [ + $this->adminGroup(), + ], + 'group_user' => [ + ['user_id' => 1, 'group_id' => 1], + ], ]); } diff --git a/framework/core/tests/integration/extenders/ThrottleApiTest.php b/framework/core/tests/integration/extenders/ThrottleApiTest.php new file mode 100644 index 000000000..a384eeaf7 --- /dev/null +++ b/framework/core/tests/integration/extenders/ThrottleApiTest.php @@ -0,0 +1,85 @@ +prepareDatabase([ + 'users' => [ + $this->normalUser(), + ], + 'groups' => [ + $this->memberGroup(), + ], + 'group_user' => [ + ['user_id' => 2, 'group_id' => 3], + ], + 'group_permission' => [ + ['permission' => 'viewDiscussions', 'group_id' => 3], + ] + ]); + } + + /** + * @test + */ + public function list_discussions_not_restricted_by_default() + { + $this->prepDb(); + + $response = $this->send($this->request('GET', '/api/discussions', ['authenticatedAs' => 2])); + + $this->assertEquals(200, $response->getStatusCode()); + } + + /** + * @test + */ + public function list_discussions_can_be_restricted() + { + $this->extend((new Extend\ThrottleApi)->set('blockListDiscussions', function ($request) { + if ($request->getAttribute('routeName') === 'discussions.index') { + return true; + } + })); + $response = $this->send($this->request('GET', '/api/discussions', ['authenticatedAs' => 2])); + + $this->assertEquals(429, $response->getStatusCode()); + } + + /** + * @test + */ + public function false_overrides_true_for_evaluating_throttlers() + { + $this->extend((new Extend\ThrottleApi)->set('blockListDiscussions', function ($request) { + if ($request->getAttribute('routeName') === 'discussions.index') { + return true; + } + })); + $this->extend((new Extend\ThrottleApi)->set('blockListDiscussionsOverride', function ($request) { + if ($request->getAttribute('routeName') === 'discussions.index') { + return false; + } + })); + + $response = $this->send($this->request('GET', '/api/discussions', ['authenticatedAs' => 2])); + + $this->assertEquals(200, $response->getStatusCode()); + } +} diff --git a/framework/core/tests/integration/extenders/UserTest.php b/framework/core/tests/integration/extenders/UserTest.php index 7e00b7d26..bb11412b4 100644 --- a/framework/core/tests/integration/extenders/UserTest.php +++ b/framework/core/tests/integration/extenders/UserTest.php @@ -26,6 +26,9 @@ class UserTest extends TestCase $this->adminUser(), $this->normalUser(), ], + 'group_permission' => [ + ['permission' => 'viewUserList', 'group_id' => 3] + ], 'settings' => [ ['key' => 'display_name_driver', 'value' => 'custom'], ],