diff --git a/framework/core/src/Api/ApiServiceProvider.php b/framework/core/src/Api/ApiServiceProvider.php index f092924a8..e95766cfb 100644 --- a/framework/core/src/Api/ApiServiceProvider.php +++ b/framework/core/src/Api/ApiServiceProvider.php @@ -102,6 +102,34 @@ class ApiServiceProvider extends AbstractServiceProvider 'discussionRenamed' => BasicDiscussionSerializer::class ]; }); + + $this->container->singleton('flarum.api_client.exclude_middleware', function () { + return [ + HttpMiddleware\InjectActorReference::class, + HttpMiddleware\ParseJsonBody::class, + Middleware\FakeHttpMethods::class, + HttpMiddleware\StartSession::class, + HttpMiddleware\AuthenticateWithSession::class, + HttpMiddleware\AuthenticateWithHeader::class, + HttpMiddleware\CheckCsrfToken::class + ]; + }); + + $this->container->singleton(Client::class, function ($container) { + $pipe = new MiddlewarePipe; + + $middlewareStack = array_filter($container->make('flarum.api.middleware'), function ($middlewareClass) use ($container) { + return ! in_array($middlewareClass, $container->make('flarum.api_client.exclude_middleware')); + }); + + foreach ($middlewareStack as $middleware) { + $pipe->pipe($container->make($middleware)); + } + + $pipe->pipe(new HttpMiddleware\ExecuteRoute()); + + return new Client($pipe); + }); } /** diff --git a/framework/core/src/Api/Client.php b/framework/core/src/Api/Client.php index 4d35b20d8..237c03b91 100644 --- a/framework/core/src/Api/Client.php +++ b/framework/core/src/Api/Client.php @@ -9,76 +9,138 @@ namespace Flarum\Api; -use Exception; -use Flarum\Foundation\ErrorHandling\JsonApiFormatter; -use Flarum\Foundation\ErrorHandling\Registry; use Flarum\Http\RequestUtil; use Flarum\User\User; use Illuminate\Contracts\Container\Container; -use InvalidArgumentException; use Laminas\Diactoros\ServerRequestFactory; +use Laminas\Diactoros\Uri; +use Laminas\Stratigility\MiddlewarePipeInterface; use Psr\Http\Message\ResponseInterface; -use Psr\Http\Server\RequestHandlerInterface; -use Throwable; +use Psr\Http\Message\ServerRequestInterface; class Client { /** - * @var Container + * @var MiddlewarePipeInterface */ - protected $container; + protected $pipe; /** - * @var Registry + * @var User */ - protected $registry; + protected $actor; + + /** + * @var ServerRequestInterface + */ + protected $parent; + + /** + * @var array + */ + protected $queryParams = []; + + /** + * @var array + */ + protected $body = []; /** * @param Container $container - * @param Registry $registry */ - public function __construct(Container $container, Registry $registry) + public function __construct(MiddlewarePipeInterface $pipe) { - $this->container = $container; - $this->registry = $registry; + $this->pipe = $pipe; + } + + /** + * Set the request actor. + * This is not needed if a parent request is provided. + * It can, however, override the parent request's actor. + */ + public function withActor(User $actor): Client + { + $new = clone $this; + $new->actor = $actor; + + return $new; + } + + public function withParentRequest(ServerRequestInterface $parent): Client + { + $new = clone $this; + $new->parent = $parent; + + return $new; + } + + public function withQueryParams(array $queryParams): Client + { + $new = clone $this; + $new->queryParams = $queryParams; + + return $new; + } + + public function withBody(array $body): Client + { + $new = clone $this; + $new->body = $body; + + return $new; + } + + public function get(string $path): ResponseInterface + { + return $this->send('GET', $path); + } + + public function post(string $path): ResponseInterface + { + return $this->send('POST', $path); + } + + public function put(string $path): ResponseInterface + { + return $this->send('PUT', $path); + } + + public function patch(string $path): ResponseInterface + { + return $this->send('PATCH', $path); + } + + public function delete(string $path): ResponseInterface + { + return $this->send('DELETE', $path); } /** * Execute the given API action class, pass the input and return its response. * - * @param string|RequestHandlerInterface $controller - * @param User|null $actor - * @param array $queryParams - * @param array $body + * @param string $method + * @param string $path * @return ResponseInterface - * @throws Exception + * + * @internal */ - public function send($controller, User $actor = null, array $queryParams = [], array $body = []): ResponseInterface + public function send(string $method, string $path): ResponseInterface { - $request = ServerRequestFactory::fromGlobals(null, $queryParams, $body); + $request = ServerRequestFactory::fromGlobals(null, $this->queryParams, $this->body) + ->withMethod($method) + ->withUri(new Uri($path)); - $request = RequestUtil::withActor($request, $actor); - - if (is_string($controller)) { - $controller = $this->container->make($controller); + if ($this->parent) { + $request = $request + ->withAttribute('session', $this->parent->getAttribute('session')); + $request = RequestUtil::withActor($request, RequestUtil::getActor($this->parent)); } - if (! ($controller instanceof RequestHandlerInterface)) { - throw new InvalidArgumentException( - 'Endpoint must be an instance of '.RequestHandlerInterface::class - ); + // This should override the actor from the parent request, if one exists. + if ($this->actor) { + $request = RequestUtil::withActor($request, $this->actor); } - try { - return $controller->handle($request); - } catch (Throwable $e) { - $error = $this->registry->handle($e); - - if ($error->shouldBeReported()) { - throw $e; - } - - return (new JsonApiFormatter)->format($error, $request); - } + return $this->pipe->handle($request); } } diff --git a/framework/core/src/Forum/Content/Discussion.php b/framework/core/src/Forum/Content/Discussion.php index 2f5c0c447..21935f10f 100644 --- a/framework/core/src/Forum/Content/Discussion.php +++ b/framework/core/src/Forum/Content/Discussion.php @@ -12,9 +12,7 @@ namespace Flarum\Forum\Content; use Flarum\Api\Client; use Flarum\Frontend\Document; use Flarum\Http\Exception\RouteNotFoundException; -use Flarum\Http\RequestUtil; use Flarum\Http\UrlGenerator; -use Flarum\User\User; use Illuminate\Contracts\View\Factory; use Illuminate\Support\Arr; use Psr\Http\Message\ServerRequestInterface as Request; @@ -51,10 +49,11 @@ class Discussion public function __invoke(Document $document, Request $request) { $queryParams = $request->getQueryParams(); + $id = Arr::get($queryParams, 'id'); $page = max(1, intval(Arr::get($queryParams, 'page'))); $params = [ - 'id' => (int) Arr::get($queryParams, 'id'), + 'id' => $id, 'page' => [ 'near' => Arr::get($queryParams, 'near'), 'offset' => ($page - 1) * 20, @@ -62,7 +61,7 @@ class Discussion ] ]; - $apiDocument = $this->getApiDocument(RequestUtil::getActor($request), $params); + $apiDocument = $this->getApiDocument($request, $id, $params); $getResource = function ($link) use ($apiDocument) { return Arr::first($apiDocument->included, function ($value) use ($link) { @@ -98,15 +97,15 @@ class Discussion /** * Get the result of an API request to show a discussion. * - * @param User $actor - * @param array $params - * @return object * @throws RouteNotFoundException */ - protected function getApiDocument(User $actor, array $params) + protected function getApiDocument(Request $request, string $id, array $params) { $params['bySlug'] = true; - $response = $this->api->send('Flarum\Api\Controller\ShowDiscussionController', $actor, $params); + $response = $this->api + ->withParentRequest($request) + ->withQueryParams($params) + ->get("/discussions/$id"); $statusCode = $response->getStatusCode(); if ($statusCode === 404) { diff --git a/framework/core/src/Forum/Content/Index.php b/framework/core/src/Forum/Content/Index.php index 223869c8e..a290bcecf 100644 --- a/framework/core/src/Forum/Content/Index.php +++ b/framework/core/src/Forum/Content/Index.php @@ -10,12 +10,9 @@ namespace Flarum\Forum\Content; use Flarum\Api\Client; -use Flarum\Api\Controller\ListDiscussionsController; use Flarum\Frontend\Document; -use Flarum\Http\RequestUtil; use Flarum\Http\UrlGenerator; use Flarum\Settings\SettingsRepositoryInterface; -use Flarum\User\User; use Illuminate\Contracts\View\Factory; use Illuminate\Support\Arr; use Psr\Http\Message\ServerRequestInterface as Request; @@ -84,7 +81,7 @@ class Index $params['filter']['q'] = $q; } - $apiDocument = $this->getApiDocument(RequestUtil::getActor($request), $params); + $apiDocument = $this->getApiDocument($request, $params); $defaultRoute = $this->settings->get('default_route'); $document->title = $this->translator->trans('core.forum.index.meta_title_text'); @@ -113,12 +110,12 @@ class Index /** * Get the result of an API request to list discussions. * - * @param User $actor + * @param Request $request * @param array $params * @return object */ - private function getApiDocument(User $actor, array $params) + protected function getApiDocument(Request $request, array $params) { - return json_decode($this->api->send(ListDiscussionsController::class, $actor, $params)->getBody()); + return json_decode($this->api->withParentRequest($request)->withQueryParams($params)->get('/discussions')->getBody()); } } diff --git a/framework/core/src/Forum/Content/User.php b/framework/core/src/Forum/Content/User.php index 53cd26ea2..50a67f761 100644 --- a/framework/core/src/Forum/Content/User.php +++ b/framework/core/src/Forum/Content/User.php @@ -10,11 +10,8 @@ namespace Flarum\Forum\Content; use Flarum\Api\Client; -use Flarum\Api\Controller\ShowUserController; use Flarum\Frontend\Document; -use Flarum\Http\RequestUtil; use Flarum\Http\UrlGenerator; -use Flarum\User\User as FlarumUser; use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Support\Arr; use Psr\Http\Message\ServerRequestInterface as Request; @@ -44,14 +41,9 @@ class User public function __invoke(Document $document, Request $request) { $queryParams = $request->getQueryParams(); - $actor = RequestUtil::getActor($request); - $userId = Arr::get($queryParams, 'username'); + $username = Arr::get($queryParams, 'username'); - $params = [ - 'id' => $userId, - ]; - - $apiDocument = $this->getApiDocument($actor, $params); + $apiDocument = $this->getApiDocument($request, $username); $user = $apiDocument->data->attributes; $document->title = $user->displayName; @@ -64,15 +56,11 @@ class User /** * Get the result of an API request to show a user. * - * @param FlarumUser $actor - * @param array $params - * @return object * @throws ModelNotFoundException */ - protected function getApiDocument(FlarumUser $actor, array $params) + protected function getApiDocument(Request $request, string $username) { - $params['bySlug'] = true; - $response = $this->api->send(ShowUserController::class, $actor, $params); + $response = $this->api->withParentRequest($request)->withQueryParams(['bySlug' => true])->get("/users/$username"); $statusCode = $response->getStatusCode(); if ($statusCode === 404) { diff --git a/framework/core/src/Forum/Controller/LogInController.php b/framework/core/src/Forum/Controller/LogInController.php index efb58c24b..30212d3b8 100644 --- a/framework/core/src/Forum/Controller/LogInController.php +++ b/framework/core/src/Forum/Controller/LogInController.php @@ -10,11 +10,9 @@ namespace Flarum\Forum\Controller; use Flarum\Api\Client; -use Flarum\Api\Controller\CreateTokenController; use Flarum\Http\AccessToken; use Flarum\Http\RememberAccessToken; use Flarum\Http\Rememberer; -use Flarum\Http\RequestUtil; use Flarum\Http\SessionAuthenticator; use Flarum\User\Event\LoggedIn; use Flarum\User\UserRepository; @@ -71,11 +69,10 @@ class LogInController implements RequestHandlerInterface */ public function handle(Request $request): ResponseInterface { - $actor = RequestUtil::getActor($request); $body = $request->getParsedBody(); $params = Arr::only($body, ['identification', 'password', 'remember']); - $response = $this->apiClient->send(CreateTokenController::class, $actor, [], $params); + $response = $this->apiClient->withParentRequest($request)->withBody($params)->post('/token'); if ($response->getStatusCode() === 200) { $data = json_decode($response->getBody()); diff --git a/framework/core/src/Forum/Controller/RegisterController.php b/framework/core/src/Forum/Controller/RegisterController.php index a6e6e2135..e732101e9 100644 --- a/framework/core/src/Forum/Controller/RegisterController.php +++ b/framework/core/src/Forum/Controller/RegisterController.php @@ -10,10 +10,8 @@ namespace Flarum\Forum\Controller; use Flarum\Api\Client; -use Flarum\Api\Controller\CreateUserController; use Flarum\Http\RememberAccessToken; use Flarum\Http\Rememberer; -use Flarum\Http\RequestUtil; use Flarum\Http\SessionAuthenticator; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface as Request; @@ -53,11 +51,9 @@ class RegisterController implements RequestHandlerInterface */ public function handle(Request $request): ResponseInterface { - $controller = CreateUserController::class; - $actor = RequestUtil::getActor($request); - $body = ['data' => ['attributes' => $request->getParsedBody()]]; + $params = ['data' => ['attributes' => $request->getParsedBody()]]; - $response = $this->api->send($controller, $actor, [], $body); + $response = $this->api->withParentRequest($request)->withBody($params)->post('/users'); $body = json_decode($response->getBody()); diff --git a/framework/core/src/Frontend/Content/CorePayload.php b/framework/core/src/Frontend/Content/CorePayload.php index 2d635377d..cbe625d2d 100644 --- a/framework/core/src/Frontend/Content/CorePayload.php +++ b/framework/core/src/Frontend/Content/CorePayload.php @@ -10,7 +10,6 @@ namespace Flarum\Frontend\Content; use Flarum\Api\Client; -use Flarum\Api\Controller\ShowUserController; use Flarum\Frontend\Document; use Flarum\Http\RequestUtil; use Flarum\Locale\LocaleManager; @@ -55,7 +54,7 @@ class CorePayload $actor = RequestUtil::getActor($request); if ($actor->exists) { - $user = $this->getUserApiDocument($actor); + $user = $this->getUserApiDocument($request, $actor); $data = array_merge($data, $this->getDataFromApiDocument($user)); } @@ -81,13 +80,12 @@ class CorePayload return $data; } - private function getUserApiDocument(User $user): array + private function getUserApiDocument(Request $request, User $actor): array { - // TODO: to avoid an extra query, something like - // $controller = new ShowUserController(new PreloadedUserRepository($user)); + $id = $actor->id; return $this->getResponseBody( - $this->api->send(ShowUserController::class, $user, ['id' => $user->id]) + $this->api->withParentRequest($request)->get("/users/$id") ); } diff --git a/framework/core/src/Frontend/Frontend.php b/framework/core/src/Frontend/Frontend.php index 103ba083d..05a4e5405 100644 --- a/framework/core/src/Frontend/Frontend.php +++ b/framework/core/src/Frontend/Frontend.php @@ -10,8 +10,6 @@ namespace Flarum\Frontend; use Flarum\Api\Client; -use Flarum\Api\Controller\ShowForumController; -use Flarum\Http\RequestUtil; use Illuminate\Contracts\View\Factory; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; @@ -67,10 +65,8 @@ class Frontend private function getForumDocument(Request $request): array { - $actor = RequestUtil::getActor($request); - return $this->getResponseBody( - $this->api->send(ShowForumController::class, $actor) + $this->api->withParentRequest($request)->get('/') ); } diff --git a/framework/core/tests/integration/extenders/ThrottleApiTest.php b/framework/core/tests/integration/extenders/ThrottleApiTest.php index c61299da5..e827e5167 100644 --- a/framework/core/tests/integration/extenders/ThrottleApiTest.php +++ b/framework/core/tests/integration/extenders/ThrottleApiTest.php @@ -79,4 +79,20 @@ class ThrottleApiTest extends TestCase $this->assertEquals(200, $response->getStatusCode()); } + + /** + * @test + */ + public function throttling_applies_to_api_client() + { + $this->extend((new Extend\ThrottleApi)->set('blockRegistration', function ($request) { + if ($request->getAttribute('routeName') === 'users.create') { + return true; + } + })); + + $response = $this->send($this->request('POST', '/register')->withAttribute('bypassCsrfToken', true)); + + $this->assertEquals(429, $response->getStatusCode()); + } } diff --git a/framework/core/tests/integration/forum/IndexTest.php b/framework/core/tests/integration/forum/IndexTest.php new file mode 100644 index 000000000..e92898596 --- /dev/null +++ b/framework/core/tests/integration/forum/IndexTest.php @@ -0,0 +1,72 @@ +extend( + (new Extend\Csrf)->exemptRoute('login') + ); + + $this->prepareDatabase([ + 'users' => [ + $this->normalUser() + ] + ]); + } + + /** + * @test + */ + public function guest_not_serialized_by_current_user_serializer() + { + $response = $this->send( + $this->request('GET', '/') + ); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringNotContainsString('preferences', $response->getBody()->getContents()); + } + + /** + * @test + */ + public function user_serialized_by_current_user_serializer() + { + $login = $this->send( + $this->request('POST', '/login', [ + 'json' => [ + 'identification' => 'normal', + 'password' => 'too-obscure' + ] + ]) + ); + + $response = $this->send( + $this->request('GET', '/', [ + 'cookiesFrom' => $login + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringContainsString('preferences', $response->getBody()->getContents()); + } +} diff --git a/framework/core/tests/integration/forum/LoginTest.php b/framework/core/tests/integration/forum/LoginTest.php new file mode 100644 index 000000000..826cda945 --- /dev/null +++ b/framework/core/tests/integration/forum/LoginTest.php @@ -0,0 +1,95 @@ +extend( + (new Extend\Csrf)->exemptRoute('login') + ); + + $this->prepareDatabase([ + 'users' => [ + $this->normalUser() + ] + ]); + } + + /** + * @test + */ + public function cant_login_without_data() + { + $response = $this->send( + $this->request('POST', '/login', [ + 'json' => [] + ]) + ); + + $this->assertEquals(401, $response->getStatusCode()); + } + + /** + * @test + */ + public function cant_login_with_wrong_password() + { + $response = $this->send( + $this->request('POST', '/login', [ + 'json' => [ + 'identification' => 'normal', + 'password' => 'incorrect' + ] + ]) + ); + + $this->assertEquals(401, $response->getStatusCode()); + } + + /** + * @test + */ + public function can_login_with_data() + { + $response = $this->send( + $this->request('POST', '/login', [ + 'json' => [ + 'identification' => 'normal', + 'password' => 'too-obscure' + ] + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + + // The response body should contain the user ID... + $body = (string) $response->getBody(); + $this->assertJson($body); + + $data = json_decode($body, true); + $this->assertEquals(2, $data['userId']); + + // ...and an access token belonging to this user. + $token = $data['token']; + $this->assertEquals(2, AccessToken::whereToken($token)->firstOrFail()->user_id); + } +} diff --git a/framework/core/tests/integration/forum/RegisterTest.php b/framework/core/tests/integration/forum/RegisterTest.php new file mode 100644 index 000000000..9668e31b9 --- /dev/null +++ b/framework/core/tests/integration/forum/RegisterTest.php @@ -0,0 +1,90 @@ +extend( + (new Extend\Csrf)->exemptRoute('register') + ); + } + + /** + * @test + */ + public function cant_register_without_data() + { + $response = $this->send( + $this->request('POST', '/register') + ); + + $this->assertEquals(422, $response->getStatusCode()); + + // The response body should contain details about the failed validation + $body = (string) $response->getBody(); + $this->assertJson($body); + $this->assertEquals([ + 'errors' => [ + [ + 'status' => '422', + 'code' => 'validation_error', + 'detail' => 'The username field is required.', + 'source' => ['pointer' => '/data/attributes/username'], + ], + [ + 'status' => '422', + 'code' => 'validation_error', + 'detail' => 'The email field is required.', + 'source' => ['pointer' => '/data/attributes/email'], + ], + [ + 'status' => '422', + 'code' => 'validation_error', + 'detail' => 'The password field is required.', + 'source' => ['pointer' => '/data/attributes/password'], + ], + ], + ], json_decode($body, true)); + } + + /** + * @test + */ + public function can_register_with_data() + { + $response = $this->send( + $this->request('POST', '/register', [ + 'json' => [ + 'username' => 'test', + 'password' => 'too-obscure', + 'email' => 'test@machine.local', + ] + ]) + ); + + $this->assertEquals(201, $response->getStatusCode()); + + /** @var User $user */ + $user = User::where('username', 'test')->firstOrFail(); + + $this->assertEquals(0, $user->is_email_confirmed); + $this->assertEquals('test', $user->username); + $this->assertEquals('test@machine.local', $user->email); + } +}