github-mirror/framework
2
0
Fork 0
mirror of https://github.com/flarum/framework.git synced 2024-11-26 10:14:16 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request from GHSA-3wjh-93gr-chh6

Browse Source 
* Integration tests: Memoize request handler as well

This is useful to send HTTP requests (or their PSR-7 equivalents)
through the entire application's middleware stack (instead of
talking to specific controllers, which should be considered
implementation detail).

* Add tests for CSRF token check

* Integration tests: Configure vendor path

Now that this is possible, make the easy change...

* Implement middleware for CSRF token verification

This fixes a rather large oversight in Flarum's codebase, which was that
we had no explicit CSRF protection using the traditional token approach.

The JS frontend was actually sending these tokens, but the backend did
not require them.

* Accept CSRF token in request body as well

* Refactor tests to shorten HTTP requests

Multiple tests now provide JSON request bodies, and others copy cookies
from previous responses, so let's provide convenient helpers for these.

* Fixed issue with tmp/storage/views not existing, this caused tmpname to notice.
Fixed csrf test that assumed an access token allows application access, which is actually api token.
Improved return type hinting in the StartSession middleware

* Using a different setting key now, so that it won't break tests whenever you re-run them once smtp is set.
Fixed, badly, the test to create users etc caused by the prepareDatabase flushing all settings by default.

* added custom view, now needs translation
This commit is contained in:
Franz Liedke 2019-06-24 09:14:39 +02:00 committed by Daniël Klabbers
parent b669490d33
commit 8e86d38804
12 changed files with 376 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/Admin/AdminServiceProvider.php
View File

@ -61,6 +61,7 @@ class AdminServiceProvider extends AbstractServiceProvider
$pipe->pipe($app->make(HttpMiddleware\StartSession::class));
$pipe->pipe($app->make(HttpMiddleware\RememberFromCookie::class));
$pipe->pipe($app->make(HttpMiddleware\AuthenticateWithSession::class));
$pipe->pipe($app->make(HttpMiddleware\CheckCsrfToken::class));
$pipe->pipe($app->make(HttpMiddleware\SetLocale::class));
$pipe->pipe($app->make(Middleware\RequireAdministrateAbility::class));

1
src/Api/ApiServiceProvider.php
View File

@ -57,6 +57,7 @@ class ApiServiceProvider extends AbstractServiceProvider
$pipe->pipe($app->make(HttpMiddleware\RememberFromCookie::class));
$pipe->pipe($app->make(HttpMiddleware\AuthenticateWithSession::class));
$pipe->pipe($app->make(HttpMiddleware\AuthenticateWithHeader::class));
$pipe->pipe($app->make(HttpMiddleware\CheckCsrfToken::class));
$pipe->pipe($app->make(HttpMiddleware\SetLocale::class));
event(new ConfigureMiddleware($pipe, 'api'));

1
src/Forum/ForumServiceProvider.php
View File

@ -68,6 +68,7 @@ class ForumServiceProvider extends AbstractServiceProvider
$pipe->pipe($app->make(HttpMiddleware\StartSession::class));
$pipe->pipe($app->make(HttpMiddleware\RememberFromCookie::class));
$pipe->pipe($app->make(HttpMiddleware\AuthenticateWithSession::class));
$pipe->pipe($app->make(HttpMiddleware\CheckCsrfToken::class));
$pipe->pipe($app->make(HttpMiddleware\SetLocale::class));
$pipe->pipe($app->make(HttpMiddleware\ShareErrorsFromSession::class));

4
src/Http/Exception/TokenMismatchException.php
View File

@ -15,4 +15,8 @@ use Exception;
class TokenMismatchException extends Exception
{
public function __construct($message = null, $code = 419, Exception $previous = null)
{
parent::__construct($message, $code, $previous);
}
}

1
src/Http/Middleware/AuthenticateWithHeader.php
View File

@ -40,6 +40,7 @@ class AuthenticateWithHeader implements Middleware
$request = $request->withAttribute('apiKey', $key);
$request = $request->withAttribute('bypassFloodgate', true);
$request = $request->withAttribute('bypassCsrfToken', true);
} elseif ($token = AccessToken::find($id)) {
$token->touch();

48
src/Http/Middleware/CheckCsrfToken.php Normal file
View File

@ -0,0 +1,48 @@
<?php
/*
* This file is part of Flarum.
*
* (c) Toby Zerner <toby.zerner@gmail.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Flarum\Http\Middleware;
use Flarum\Http\Exception\TokenMismatchException;
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 CheckCsrfToken implements Middleware
{
public function process(Request $request, Handler $handler): Response
{
if (in_array($request->getMethod(), ['GET', 'HEAD', 'OPTIONS'])) {
return $handler->handle($request);
}
if ($request->getAttribute('bypassCsrfToken', false)) {
return $handler->handle($request);
}
if ($this->tokensMatch($request)) {
return $handler->handle($request);
}
throw new TokenMismatchException('CSRF token did not match');
}
private function tokensMatch(Request $request): bool
{
$expected = (string) $request->getAttribute('session')->token();
$provided = $request->getParsedBody()['csrfToken'] ??
$request->getHeaderLine('X-CSRF-Token');
return hash_equals($expected, $provided);
}
}

8
src/Http/Middleware/StartSession.php
View File

@ -67,7 +67,7 @@ class StartSession implements Middleware
return $this->withSessionCookie($response, $session);
}
private function makeSession(Request $request)
private function makeSession(Request $request): Store
{
return new Store(
$this->config['cookie'],
@ -76,12 +76,12 @@ class StartSession implements Middleware
);
}
private function withCsrfTokenHeader(Response $response, Session $session)
private function withCsrfTokenHeader(Response $response, Session $session): Response
{
return $response->withHeader('X-CSRF-Token', $session->token());
}
private function withSessionCookie(Response $response, Session $session)
private function withSessionCookie(Response $response, Session $session): Response
{
return FigResponseCookies::set(
$response,
@ -89,7 +89,7 @@ class StartSession implements Middleware
);
}
private function getSessionLifetimeInSeconds()
private function getSessionLifetimeInSeconds(): int
{
return $this->config['lifetime'] * 60;
}

122
tests/integration/TestCase.php
View File

@ -11,8 +11,13 @@
namespace Flarum\Tests\integration;
use Dflydev\FigCookies\SetCookie;
use Flarum\Foundation\InstalledSite;
use Illuminate\Database\ConnectionInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Zend\Diactoros\CallbackStream;
use Zend\Diactoros\ServerRequest;
abstract class TestCase extends \PHPUnit\Framework\TestCase
{
@ -24,27 +29,37 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase
$this->app();
}
/**
* @var \Flarum\Foundation\InstalledApp
*/
protected $app;
/**
* @var \Psr\Http\Server\RequestHandlerInterface
*/
protected $server;
/**
* @return \Flarum\Foundation\InstalledApp
*/
protected function app()
{
if (! is_null($this->app)) {
return $this->app;
if (is_null($this->app)) {
$site = new InstalledSite(
[
'base' => __DIR__.'/tmp',
'vendor' => __DIR__.'/../../vendor',
'public' => __DIR__.'/tmp/public',
'storage' => __DIR__.'/tmp/storage',
],
include __DIR__.'/tmp/config.php'
);
$this->app = $site->bootApp();
$this->server = $this->app->getRequestHandler();
}
$site = new InstalledSite(
[
'base' => __DIR__.'/tmp',
'public' => __DIR__.'/tmp/public',
'storage' => __DIR__.'/tmp/storage',
],
include __DIR__.'/tmp/config.php'
);
return $this->app = $site->bootApp();
return $this->app;
}
protected $database;
@ -67,15 +82,94 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase
// First, truncate all referenced tables so that they are empty.
foreach (array_keys($tableData) as $table) {
$this->database()->table($table)->truncate();
if ($table !== 'settings') {
$this->database()->table($table)->truncate();
}
}
// Then, insert all rows required for this test case.
foreach ($tableData as $table => $rows) {
$this->database()->table($table)->insert($rows);
foreach ($rows as $row) {
if ($table === 'settings') {
$this->database()->table($table)->updateOrInsert(
['key' => $row['key']],
$row
);
} else {
$this->database()->table($table)->updateOrInsert(
isset($row['id']) ? ['id' => $row['id']] : $row,
$row
);
}
}
}
// And finally, turn on foreign key checks again.
$this->database()->getSchemaBuilder()->enableForeignKeyConstraints();
}
/**
* Send a full HTTP request through Flarum's middleware stack.
*/
protected function send(ServerRequestInterface $request): ResponseInterface
{
return $this->server->handle($request);
}
/**
* Build a HTTP request that can be passed through middleware.
*
* This method simplifies building HTTP request for use in our HTTP-level
* integration tests. It provides options for all features repeatedly being
* used in those tests.
*
* @param string $method
* @param string $path
* @param array $options
* An array of optional request properties.
* Currently supported:
* - "json" should point to a JSON-serializable object that will be
* serialized and used as request body. The corresponding Content-Type
* header will be set automatically.
* - "cookiesFrom" should hold a response object from a previous HTTP
* interaction. All cookies returned from the server in that response
* (via the "Set-Cookie" header) will be copied to the cookie params of
* the new request.
* @return ServerRequestInterface
*/
protected function request(string $method, string $path, array $options = []): ServerRequestInterface
{
$request = new ServerRequest([], [], $path, $method);
// Do we want a JSON request body?
if (isset($options['json'])) {
$request = $request
->withHeader('Content-Type', 'application/json')
->withBody(
new CallbackStream(function () use ($options) {
return json_encode($options['json']);
})
);
}
// Let's copy the cookies from a previous response
if (isset($options['cookiesFrom'])) {
/** @var ResponseInterface $previousResponse */
$previousResponse = $options['cookiesFrom'];
$cookies = array_reduce(
$previousResponse->getHeader('Set-Cookie'),
function ($memo, $setCookieString) {
$setCookie = SetCookie::fromSetCookieString($setCookieString);
$memo[$setCookie->getName()] = $setCookie->getValue();
return $memo;
},
[]
);
$request = $request->withCookieParams($cookies);
}
return $request;
}
}

3
tests/integration/api/Controller/CreateUserControllerTest.php
View File

@ -40,6 +40,9 @@ class CreateUserControllerTest extends ApiControllerTestCase
'group_user' => [
['user_id' => 1, 'group_id' => 1],
],
'settings' => [
['key' => 'mail_driver', 'value' => 'log']
]
]);
}

193
tests/integration/api/csrf_protection/RequireCsrfTokenTest.php Normal file
View File

@ -0,0 +1,193 @@
<?php
/*
* This file is part of Flarum.
*
* (c) Toby Zerner <toby.zerner@gmail.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Flarum\Tests\integration\api\csrf_protection;
use Flarum\Tests\integration\RetrievesAuthorizedUsers;
use Flarum\Tests\integration\TestCase;
class RequireCsrfTokenTest extends TestCase
{
use RetrievesAuthorizedUsers;
public function setUp()
{
parent::setUp();
$this->prepareDatabase([
'users' => [
$this->adminUser(),
],
'groups' => [
$this->adminGroup(),
],
'group_user' => [
['user_id' => 1, 'group_id' => 1],
],
'group_permission' => [
['permission' => 'viewUserList', 'group_id' => 3],
],
'api_keys' => [
['user_id' => 1, 'key' => 'superadmin'],
],
'settings' => [
['key' => 'csrf_test', 'value' => 1],
],
]);
}
/**
* @test
*/
public function error_when_doing_cookie_auth_without_csrf_token()
{
$auth = $this->send(
$this->request(
'POST', '/login',
[
'json' => ['identification' => 'admin', 'password' => 'password'],
]
)
);
$response = $this->send(
$this->request(
'POST', '/api/settings',
[
'cookiesFrom' => $auth,
'json' => ['csrf_test' => 2],
]
)
);
// Response should be "HTTP 400 Bad Request"
$this->assertEquals(400, $response->getStatusCode());
// The response body should contain proper error details
$body = (string) $response->getBody();
$this->assertJson($body);
$this->assertEquals([
'errors' => [
['status' => '400', 'code' => 'csrf_token_mismatch'],
],
], json_decode($body, true));
}
/**
* @test
*/
public function cookie_auth_succeeds_with_csrf_token_in_header()
{
$initial = $this->send(
$this->request('GET', '/')
);
$token = $initial->getHeaderLine('X-CSRF-Token');
$auth = $this->send(
$this->request(
'POST', '/login',
[
'cookiesFrom' => $initial,
'json' => ['identification' => 'admin', 'password' => 'password'],
]
)->withHeader('X-CSRF-Token', $token)
);
$token = $auth->getHeaderLine('X-CSRF-Token');
$response = $this->send(
$this->request(
'POST', '/api/settings',
[
'cookiesFrom' => $auth,
'json' => ['csrf_test' => 2],
]
)->withHeader('X-CSRF-Token', $token)
);
// Successful response?
$this->assertEquals(204, $response->getStatusCode());
// Was the setting actually changed in the database?
$this->assertEquals(
2,
$this->database()->table('settings')->where('key', 'csrf_test')->first()->value
);
}
/**
* @test
*/
public function cookie_auth_succeeds_with_csrf_token_in_body()
{
$initial = $this->send(
$this->request('GET', '/')
);
$token = $initial->getHeaderLine('X-CSRF-Token');
$auth = $this->send(
$this->request(
'POST', '/login',
[
'cookiesFrom' => $initial,
'json' => ['identification' => 'admin', 'password' => 'password', 'csrfToken' => $token],
]
)
);
$token = $auth->getHeaderLine('X-CSRF-Token');
$response = $this->send(
$this->request(
'POST', '/api/settings',
[
'cookiesFrom' => $auth,
'json' => ['csrf_test' => 2, 'csrfToken' => $token],
]
)
);
// Successful response?
$this->assertEquals(204, $response->getStatusCode());
// Was the setting actually changed in the database?
$this->assertEquals(
2,
$this->database()->table('settings')->where('key', 'csrf_test')->first()->value
);
}
/**
* @test
*/
public function master_api_token_does_not_need_csrf_token()
{
$response = $this->send(
$this->request(
'POST', '/api/settings',
[
'json' => ['csrf_test' => 2],
]
)->withHeader('Authorization', 'Token superadmin')
);
// Successful response?
$this->assertEquals(204, $response->getStatusCode());
// Was the setting actually changed in the database?
$this->assertEquals(
2,
$this->database()->table('settings')->where('key', 'csrf_test')->first()->value
);
}
}

0
tests/integration/tmp/storage/.gitkeep
View File

12
views/error/419.blade.php Normal file
View File

@ -0,0 +1,12 @@
@extends('flarum.forum::layouts.basic')
@section('content')
<p>
{{ $message }}
</p>
<p>
<a href="{{ $app->url() }}">
{{ $translator->trans('core.views.error.419_return_link', ['{forum}' => $settings->get('forum_title')]) }}
</a>
</p>
@endsection