Move floodgate to middleware, add extender + integration tests (#2170)

This commit is contained in:
Alexander Skvortsov 2020-11-29 17:13:22 -05:00 committed by GitHub
parent 5450fcda00
commit ed9131b36c
14 changed files with 400 additions and 2 deletions

View File

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

View File

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

View File

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

View File

@ -0,0 +1,57 @@
<?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\Api\Middleware;
use Flarum\Post\Exception\FloodingException;
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 ThrottleApi implements Middleware
{
protected $throttlers;
public function __construct(array $throttlers)
{
$this->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;
}
}

View File

@ -0,0 +1,74 @@
<?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\Extend;
use Flarum\Extension\Extension;
use Flarum\Foundation\ContainerUtil;
use Illuminate\Contracts\Container\Container;
class ThrottleApi implements ExtenderInterface
{
private $setThrottlers = [];
private $removeThrottlers = [];
/**
* Add a new throttler (or override one with the same name).
*
* @param string $name: The name of the throttler.
* @param string|callable $callback
*
* The callable can be a closure or invokable class, and should accept:
* - $request: The current `\Psr\Http\Message\ServerRequestInterface` request object.
* `$request->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;
});
}
}

View File

@ -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();

View File

@ -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
{
/**

View File

@ -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}
*/

View File

@ -28,6 +28,9 @@ class WithApiKeyTest extends TestCase
$this->adminUser(),
$this->normalUser(),
],
'group_permission' => [
['permission' => 'viewUserList', 'group_id' => 3]
],
'api_keys' => [],
]);
}

View File

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

View File

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

View File

@ -29,6 +29,12 @@ class MailTest extends TestCase
'users' => [
$this->adminUser(),
],
'groups' => [
$this->adminGroup(),
],
'group_user' => [
['user_id' => 1, 'group_id' => 1],
],
]);
}

View File

@ -0,0 +1,85 @@
<?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\Tests\integration\extenders;
use Flarum\Extend;
use Flarum\Tests\integration\RetrievesAuthorizedUsers;
use Flarum\Tests\integration\TestCase;
class ThrottleApiTest extends TestCase
{
use RetrievesAuthorizedUsers;
protected function prepDb(): void
{
$this->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());
}
}

View File

@ -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'],
],