From 7c144f9858d82f08bd2bfce9566563176e9b5a90 Mon Sep 17 00:00:00 2001 From: Matthias Schuhmayer <38959016+mattamon@users.noreply.github.com> Date: Mon, 22 Apr 2024 15:38:17 +0200 Subject: [PATCH] [Research] CORS (#24) * Add cors subscriber and config * Apply php-cs-fixer changes * Update event_subscriber.yaml * Fix static analysis * Apply php-cs-fixer changes * Introduce trait * Apply php-cs-fixer changes * Adding error responses * Apply php-cs-fixer changes * Disable voter --------- Co-authored-by: mattamon --- config/event_subscribers.yaml | 6 +- .../Response/Error/BadRequestResponse.php | 9 +- .../Error/MethodNotAllowedResponse.php | 11 ++- .../Response/Error/UnauthorizedResponse.php | 9 +- .../Error/UnprocessableContentResponse.php | 32 ++++++ .../Error/UnsupportedMediaTypeResponse.php | 40 ++++++++ .../Api/Assets/CollectionController.php | 4 + src/Controller/Api/Assets/GetController.php | 4 + .../Authorization/AuthorizationController.php | 4 + .../Api/DataObjects/CollectionController.php | 4 + src/Controller/Api/TranslationController.php | 4 + src/DependencyInjection/Configuration.php | 78 ++++++++++++--- .../PimcoreStudioApiExtension.php | 4 + .../ApiExceptionSubscriber.php | 43 +++++--- src/EventSubscriber/CorsSubscriber.php | 98 +++++++++++++++++++ src/EventSubscriber/StudioApiPathTrait.php | 30 ++++++ src/Exception/InvalidHostException.php | 26 +++++ src/Response/Schema/DevError.php | 41 ++++++++ src/Response/Schema/Error.php | 3 +- src/Response/Schemas.php | 7 ++ 20 files changed, 420 insertions(+), 37 deletions(-) create mode 100644 src/Attributes/Response/Error/UnprocessableContentResponse.php create mode 100644 src/Attributes/Response/Error/UnsupportedMediaTypeResponse.php create mode 100644 src/EventSubscriber/CorsSubscriber.php create mode 100644 src/EventSubscriber/StudioApiPathTrait.php create mode 100644 src/Exception/InvalidHostException.php create mode 100644 src/Response/Schema/DevError.php diff --git a/config/event_subscribers.yaml b/config/event_subscribers.yaml index b284d97fe..76beeb7f8 100644 --- a/config/event_subscribers.yaml +++ b/config/event_subscribers.yaml @@ -5,5 +5,9 @@ services: public: false #Subscriber + Pimcore\Bundle\StudioApiBundle\EventSubscriber\CorsSubscriber: + tags: [ 'kernel.event_subscriber' ] + Pimcore\Bundle\StudioApiBundle\EventSubscriber\ApiExceptionSubscriber: - tags: [ 'kernel.event_subscriber' ] \ No newline at end of file + tags: [ 'kernel.event_subscriber' ] + arguments: ["%kernel.environment%"] \ No newline at end of file diff --git a/src/Attributes/Response/Error/BadRequestResponse.php b/src/Attributes/Response/Error/BadRequestResponse.php index bc11089aa..b87344006 100644 --- a/src/Attributes/Response/Error/BadRequestResponse.php +++ b/src/Attributes/Response/Error/BadRequestResponse.php @@ -19,7 +19,8 @@ use Attribute; use OpenApi\Attributes\JsonContent; use OpenApi\Attributes\Response; -use Pimcore\Bundle\StudioApiBundle\Response\Schema\Error; +use OpenApi\Attributes\Schema; +use Pimcore\Bundle\StudioApiBundle\Response\Schemas; #[Attribute(Attribute::TARGET_CLASS | Attribute::TARGET_METHOD | Attribute::IS_REPEATABLE)] final class BadRequestResponse extends Response @@ -29,7 +30,11 @@ public function __construct() parent::__construct( response: 400, description: 'Bad Request', - content: new JsonContent(ref: Error::class, example: ['message' => 'Something bad you did']) + content: new JsonContent( + oneOf: array_map(static function ($class) { + return new Schema(ref: $class); + }, Schemas::Errors), + ) ); } } diff --git a/src/Attributes/Response/Error/MethodNotAllowedResponse.php b/src/Attributes/Response/Error/MethodNotAllowedResponse.php index bc35c1543..4c7bd9c50 100644 --- a/src/Attributes/Response/Error/MethodNotAllowedResponse.php +++ b/src/Attributes/Response/Error/MethodNotAllowedResponse.php @@ -19,7 +19,8 @@ use Attribute; use OpenApi\Attributes\JsonContent; use OpenApi\Attributes\Response; -use Pimcore\Bundle\StudioApiBundle\Response\Schema\Error; +use OpenApi\Attributes\Schema; +use Pimcore\Bundle\StudioApiBundle\Response\Schemas; #[Attribute(Attribute::TARGET_CLASS | Attribute::TARGET_METHOD | Attribute::IS_REPEATABLE)] final class MethodNotAllowedResponse extends Response @@ -28,8 +29,12 @@ public function __construct() { parent::__construct( response: 405, - description: 'Bad Request', - content: new JsonContent(ref: Error::class, example: ['message' => 'Using the wrong method you are']) + description: 'Method Not Allowed', + content: new JsonContent( + oneOf: array_map(static function ($class) { + return new Schema(ref: $class); + }, Schemas::Errors), + ) ); } } diff --git a/src/Attributes/Response/Error/UnauthorizedResponse.php b/src/Attributes/Response/Error/UnauthorizedResponse.php index 277f35ecc..8d85c42b7 100644 --- a/src/Attributes/Response/Error/UnauthorizedResponse.php +++ b/src/Attributes/Response/Error/UnauthorizedResponse.php @@ -19,7 +19,8 @@ use Attribute; use OpenApi\Attributes\JsonContent; use OpenApi\Attributes\Response; -use Pimcore\Bundle\StudioApiBundle\Response\Schema\Error; +use OpenApi\Attributes\Schema; +use Pimcore\Bundle\StudioApiBundle\Response\Schemas; #[Attribute(Attribute::TARGET_CLASS | Attribute::TARGET_METHOD | Attribute::IS_REPEATABLE)] final class UnauthorizedResponse extends Response @@ -29,7 +30,11 @@ public function __construct() parent::__construct( response: 401, description: 'Unauthorized', - content: new JsonContent(ref: Error::class, example: ['message' => 'Computer says no']) + content: new JsonContent( + oneOf: array_map(static function ($class) { + return new Schema(ref: $class); + }, Schemas::Errors), + ) ); } } diff --git a/src/Attributes/Response/Error/UnprocessableContentResponse.php b/src/Attributes/Response/Error/UnprocessableContentResponse.php new file mode 100644 index 000000000..26d052d96 --- /dev/null +++ b/src/Attributes/Response/Error/UnprocessableContentResponse.php @@ -0,0 +1,32 @@ +filterService->applyFilters( diff --git a/src/Controller/Api/Assets/GetController.php b/src/Controller/Api/Assets/GetController.php index 6ecdc679b..7c1c9dd15 100644 --- a/src/Controller/Api/Assets/GetController.php +++ b/src/Controller/Api/Assets/GetController.php @@ -21,6 +21,8 @@ use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Content\OneOfAssetJson; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\MethodNotAllowedResponse; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\UnauthorizedResponse; +use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\UnprocessableContentResponse; +use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\UnsupportedMediaTypeResponse; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\SuccessResponse; use Pimcore\Bundle\StudioApiBundle\Config\Tags; use Pimcore\Bundle\StudioApiBundle\Controller\Api\AbstractApiController; @@ -58,6 +60,8 @@ public function __construct( )] #[UnauthorizedResponse] #[MethodNotAllowedResponse] + #[UnsupportedMediaTypeResponse] + #[UnprocessableContentResponse] public function getAssetById(int $id): JsonResponse { return $this->jsonResponse($this->assetSearchService->getAssetById($id)); diff --git a/src/Controller/Api/Authorization/AuthorizationController.php b/src/Controller/Api/Authorization/AuthorizationController.php index 753884e4a..8f7f095fa 100644 --- a/src/Controller/Api/Authorization/AuthorizationController.php +++ b/src/Controller/Api/Authorization/AuthorizationController.php @@ -22,6 +22,8 @@ use Pimcore\Bundle\StudioApiBundle\Attributes\Request\TokenRequestBody; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\MethodNotAllowedResponse; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\UnauthorizedResponse; +use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\UnprocessableContentResponse; +use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\UnsupportedMediaTypeResponse; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\SuccessResponse; use Pimcore\Bundle\StudioApiBundle\Config\Tags; use Pimcore\Bundle\StudioApiBundle\Controller\Api\AbstractApiController; @@ -87,6 +89,8 @@ public function login(#[MapRequestPayload] Credentials $credentials): JsonRespon )] #[UnauthorizedResponse] #[MethodNotAllowedResponse] + #[UnsupportedMediaTypeResponse] + #[UnprocessableContentResponse] public function refresh(#[MapRequestPayload] Refresh $refresh): JsonResponse { $tokenInfo = $this->tokenService->refreshToken($refresh->getToken()); diff --git a/src/Controller/Api/DataObjects/CollectionController.php b/src/Controller/Api/DataObjects/CollectionController.php index d3ccb650f..fdcc40b2b 100644 --- a/src/Controller/Api/DataObjects/CollectionController.php +++ b/src/Controller/Api/DataObjects/CollectionController.php @@ -30,6 +30,8 @@ use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\BadRequestResponse; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\MethodNotAllowedResponse; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\UnauthorizedResponse; +use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\UnprocessableContentResponse; +use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\UnsupportedMediaTypeResponse; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Property\DataObjectCollection; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\SuccessResponse; use Pimcore\Bundle\StudioApiBundle\Config\Tags; @@ -85,6 +87,8 @@ public function __construct( #[BadRequestResponse] #[UnauthorizedResponse] #[MethodNotAllowedResponse] + #[UnsupportedMediaTypeResponse] + #[UnprocessableContentResponse] public function getDataObjects(#[MapQueryString] DataObjectParameters $parameters): JsonResponse { diff --git a/src/Controller/Api/TranslationController.php b/src/Controller/Api/TranslationController.php index 0cde64818..920702ae1 100644 --- a/src/Controller/Api/TranslationController.php +++ b/src/Controller/Api/TranslationController.php @@ -21,6 +21,8 @@ use Pimcore\Bundle\StudioApiBundle\Attributes\Request\TranslationRequestBody; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\MethodNotAllowedResponse; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\UnauthorizedResponse; +use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\UnprocessableContentResponse; +use Pimcore\Bundle\StudioApiBundle\Attributes\Response\Error\UnsupportedMediaTypeResponse; use Pimcore\Bundle\StudioApiBundle\Attributes\Response\SuccessResponse; use Pimcore\Bundle\StudioApiBundle\Config\Tags; use Pimcore\Bundle\StudioApiBundle\Request\Query\Translation; @@ -61,6 +63,8 @@ public function __construct( )] #[UnauthorizedResponse] #[MethodNotAllowedResponse] + #[UnsupportedMediaTypeResponse] + #[UnprocessableContentResponse] public function getTranslations( #[MapRequestPayload] Translation $translation, ): JsonResponse { diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index bd3801409..ad4cf7c1c 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -16,7 +16,9 @@ namespace Pimcore\Bundle\StudioApiBundle\DependencyInjection; +use Pimcore\Bundle\StudioApiBundle\Exception\InvalidHostException; use Pimcore\Bundle\StudioApiBundle\Exception\InvalidPathException; +use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; @@ -31,6 +33,9 @@ class Configuration implements ConfigurationInterface /** * {@inheritdoc} + * + * @throws InvalidPathException + * @throws InvalidHostException */ public function getConfigTreeBuilder(): TreeBuilder { @@ -38,21 +43,41 @@ public function getConfigTreeBuilder(): TreeBuilder $rootNode = $treeBuilder->getRootNode(); $rootNode->addDefaultsIfNotSet(); - $rootNode->children() + $this->addOpenApiScanPathsNode($rootNode); + $this->addApiTokenNode($rootNode); + $this->addAllowedHostsForCorsNode($rootNode); + + return $treeBuilder; + } + + private function addOpenApiScanPathsNode(ArrayNodeDefinition $node): void + { + $node->children() ->arrayNode('openApiScanPaths') - ->prototype('scalar')->end() - ->validate() - ->always(function ($paths) { - foreach ($paths as $path) { - if (!is_dir($path)) { - throw new InvalidPathException(sprintf('The path "%s" is not a valid directory.', $path)); - } - } + ->prototype('scalar')->end() + ->validate() + ->always( + function ($paths) { + foreach ($paths as $path) { + if (!is_dir($path)) { + throw new InvalidPathException( + sprintf( + 'The path "%s" is not a valid directory.', + $path + ) + ); + } + } - return $paths; - }) - ->end() - ->end() + return $paths; + }) + ->end() + ->end(); + } + + private function addApiTokenNode(ArrayNodeDefinition $node): void + { + $node->children() ->arrayNode('api_token') ->addDefaultsIfNotSet() ->children() @@ -61,7 +86,32 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() ->end(); + } - return $treeBuilder; + private function addAllowedHostsForCorsNode(ArrayNodeDefinition $node): void + { + $node->children() + ->arrayNode('allowedHostsForCors') + ->prototype('scalar')->end() + ->validate() + ->always( + /** + * @throws InvalidHostException + */ function ($hosts) { + foreach ($hosts as $host) { + if (!filter_var($host)) { + throw new InvalidHostException( + sprintf( + 'The host "%s" is not a valid url.', + $host + ) + ); + } + } + + return $hosts; + }) + ->end() + ->end(); } } diff --git a/src/DependencyInjection/PimcoreStudioApiExtension.php b/src/DependencyInjection/PimcoreStudioApiExtension.php index cdcbf8e99..e5f96ea15 100644 --- a/src/DependencyInjection/PimcoreStudioApiExtension.php +++ b/src/DependencyInjection/PimcoreStudioApiExtension.php @@ -17,6 +17,7 @@ namespace Pimcore\Bundle\StudioApiBundle\DependencyInjection; use Exception; +use Pimcore\Bundle\StudioApiBundle\EventSubscriber\CorsSubscriber; use Pimcore\Bundle\StudioApiBundle\Service\OpenApiServiceInterface; use Pimcore\Bundle\StudioApiBundle\Service\TokenServiceInterface; use Symfony\Component\Config\FileLocator; @@ -56,5 +57,8 @@ public function load(array $configs, ContainerBuilder $container): void $definition = $container->getDefinition(OpenApiServiceInterface::class); $definition->setArgument('$openApiScanPaths', $config['openApiScanPaths']); + + $definition = $container->getDefinition(CorsSubscriber::class); + $definition->setArgument('$allowedHosts', $config['allowedHostsForCors']); } } diff --git a/src/EventSubscriber/ApiExceptionSubscriber.php b/src/EventSubscriber/ApiExceptionSubscriber.php index 01b4e31a8..b11bed4f1 100644 --- a/src/EventSubscriber/ApiExceptionSubscriber.php +++ b/src/EventSubscriber/ApiExceptionSubscriber.php @@ -16,17 +16,23 @@ namespace Pimcore\Bundle\StudioApiBundle\EventSubscriber; -use Pimcore\Bundle\StudioApiBundle\Controller\Api\AbstractApiController; -use Pimcore\Bundle\StudioApiBundle\Exception\ApiExceptionInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\JsonResponse; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\ExceptionEvent; +use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface; /** * @internal */ final class ApiExceptionSubscriber implements EventSubscriberInterface { + use StudioApiPathTrait; + + public function __construct(private readonly string $environment) + { + } + public static function getSubscribedEvents(): array { return [ @@ -39,22 +45,33 @@ public function onKernelException(ExceptionEvent $event): void $exception = $event->getThrowable(); $request = $event->getRequest(); - if(!$exception instanceof ApiExceptionInterface && !$this->isStudioApiCall($request->getPathInfo())) { + if(!$this->isStudioApiPath($request->getPathInfo())) { + return; + } + + if(!$exception instanceof HttpExceptionInterface) { return; } - /** @var ApiExceptionInterface $exception */ - $response = new JsonResponse( - [ - 'message' => $exception->getMessage(), - ], - $exception->getStatusCode() - ); - $event->setResponse($response); + $event->setResponse($this->createResponse($exception)); } - private function isStudioApiCall(string $path): bool + private function createResponse(HttpExceptionInterface $exception): Response { - return str_starts_with($path, AbstractApiController::API_PATH); + if(!$exception->getMessage()) { + return new Response(null, $exception->getStatusCode()); + } + $responseData = [ + 'message' => $exception->getMessage(), + ]; + + if ($this->environment === 'dev') { + $responseData['detail'] = $exception->getTraceAsString(); + } + + return new JsonResponse( + $responseData, + $exception->getStatusCode(), + ); } } diff --git a/src/EventSubscriber/CorsSubscriber.php b/src/EventSubscriber/CorsSubscriber.php new file mode 100644 index 000000000..bc27b5338 --- /dev/null +++ b/src/EventSubscriber/CorsSubscriber.php @@ -0,0 +1,98 @@ +router->getRouteCollection()->getIterator() as $route) { + if($this->isStudioApiPath($route->getPath())) { + $this->routeMethods[$route->getPath()] = implode(', ', $route->getMethods()); + } + } + } + + public static function getSubscribedEvents(): array + { + return [ + KernelEvents::REQUEST => ['onKernelRequest', 9999], + KernelEvents::RESPONSE => ['onKernelResponse', 9999], + ]; + } + + public function onKernelRequest(RequestEvent $event): void + { + // Only check main requests + if (!$event->isMainRequest()) { + return; + } + + $request = $event->getRequest(); + + if(!$this->isStudioApiPath($request->getPathInfo())) { + return; + } + + // perform preflight checks + if ('OPTIONS' === $request->getMethod()) { + $response = new Response(); + + $response->headers->set('Access-Control-Allow-Credentials', 'true'); + $response->headers->set('Access-Control-Allow-Methods', $this->routeMethods[$request->getPathInfo()]); + $response->headers->set('Access-Control-Allow-Headers', 'Origin, Content-Type, Accept, Authorization'); + $response->headers->set('Access-Control-Max-Age', '3600'); + + $event->setResponse($response); + } + } + + public function onKernelResponse(ResponseEvent $event): void + { + $request = $event->getRequest(); + + if (!$this->isStudioApiPath($request->getPathInfo())) { + return; + } + // Run CORS check in here to ensure domain is in the system + $corsOrigin = $request->headers->get('origin'); + + if (in_array($corsOrigin, $this->allowedHosts, true)) { + $response = $event->getResponse(); + + $response->headers->set('Access-Control-Allow-Credentials', 'true'); + $response->headers->set('Access-Control-Allow-Headers', 'Origin, Content-Type, Accept, Authorization'); + $response->headers->set('Access-Control-Allow-Origin', $corsOrigin); + $response->headers->set('Access-Control-Allow-Methods', 'POST, GET, PUT, DELETE, PATCH, OPTIONS'); + $response->headers->set('Vary', 'Origin'); + + } + } +} diff --git a/src/EventSubscriber/StudioApiPathTrait.php b/src/EventSubscriber/StudioApiPathTrait.php new file mode 100644 index 000000000..4c15cfc78 --- /dev/null +++ b/src/EventSubscriber/StudioApiPathTrait.php @@ -0,0 +1,30 @@ +