From 49f27b2cca2356858f73548d74771340c5f19ca2 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Wed, 5 Jun 2024 14:08:00 +0200 Subject: [PATCH 01/28] Initial refactor to session auth. --- config/authorization.yaml | 5 +- config/pimcore/config.yaml | 4 + config/pimcore/firewall.yaml | 14 +++ config/pimcore/security.yaml | 4 + .../InvalidCredentials.php} | 15 +-- .../Controller/AuthorizationController.php | 111 ------------------ .../Controller/LoginController.php | 57 +++++++++ .../Controller/LogoutController.php | 46 ++++++++ .../EventListener/LogoutListener.php | 40 +++++++ .../Schema/{Token.php => LoginSuccess.php} | 23 ++-- src/Authorization/Service/TokenService.php | 111 ------------------ .../Service/TokenServiceInterface.php | 37 ------ src/DependencyInjection/Configuration.php | 10 +- .../PimcoreStudioBackendExtension.php | 16 ++- src/Security/Service/SecurityService.php | 24 ++-- src/Security/Voter/AuthorizationVoter.php | 15 --- 16 files changed, 211 insertions(+), 321 deletions(-) create mode 100644 config/pimcore/firewall.yaml create mode 100644 config/pimcore/security.yaml rename src/Authorization/Attributes/{Request/TokenRequestBody.php => Response/InvalidCredentials.php} (69%) delete mode 100644 src/Authorization/Controller/AuthorizationController.php create mode 100644 src/Authorization/Controller/LoginController.php create mode 100644 src/Authorization/Controller/LogoutController.php create mode 100644 src/Authorization/EventListener/LogoutListener.php rename src/Authorization/Schema/{Token.php => LoginSuccess.php} (64%) delete mode 100644 src/Authorization/Service/TokenService.php delete mode 100644 src/Authorization/Service/TokenServiceInterface.php diff --git a/config/authorization.yaml b/config/authorization.yaml index d346910cf..044847027 100644 --- a/config/authorization.yaml +++ b/config/authorization.yaml @@ -13,5 +13,6 @@ services: tags: [ 'controller.service_arguments' ] - Pimcore\Bundle\StudioBackendBundle\Authorization\Service\TokenServiceInterface: - class: Pimcore\Bundle\StudioBackendBundle\Authorization\Service\TokenService \ No newline at end of file + Pimcore\Bundle\StudioBackendBundle\Authorization\EventListener\LogoutListener: + tags: + - { name: 'kernel.event_subscriber', dispatcher: 'security.event_dispatcher.pimcore_studio' } \ No newline at end of file diff --git a/config/pimcore/config.yaml b/config/pimcore/config.yaml index 59c7e2cd7..95dc438b5 100644 --- a/config/pimcore/config.yaml +++ b/config/pimcore/config.yaml @@ -1,3 +1,7 @@ +imports: + - { resource: security.yaml } + - { resource: firewall.yaml } + pimcore: translations: domains: diff --git a/config/pimcore/firewall.yaml b/config/pimcore/firewall.yaml new file mode 100644 index 000000000..bf04ade1b --- /dev/null +++ b/config/pimcore/firewall.yaml @@ -0,0 +1,14 @@ +pimcore_studio_backend: + security_firewall: + pattern: ^/studio/api(/.*)?$ + user_checker: Pimcore\Security\User\UserChecker + context: pimcore_admin + provider: pimcore_studio_backend + stateless: false + login_throttling: + max_attempts: 3 + interval: '5 minutes' + logout: + path: /studio/api/logout + json_login: + check_path: /studio/api/login \ No newline at end of file diff --git a/config/pimcore/security.yaml b/config/pimcore/security.yaml new file mode 100644 index 000000000..e4ae3f7b1 --- /dev/null +++ b/config/pimcore/security.yaml @@ -0,0 +1,4 @@ +security: + providers: + pimcore_studio_backend: + id: Pimcore\Security\User\UserProvider \ No newline at end of file diff --git a/src/Authorization/Attributes/Request/TokenRequestBody.php b/src/Authorization/Attributes/Response/InvalidCredentials.php similarity index 69% rename from src/Authorization/Attributes/Request/TokenRequestBody.php rename to src/Authorization/Attributes/Response/InvalidCredentials.php index fb6c49f06..88dc3fd7c 100644 --- a/src/Authorization/Attributes/Request/TokenRequestBody.php +++ b/src/Authorization/Attributes/Response/InvalidCredentials.php @@ -14,21 +14,22 @@ * @license http://www.pimcore.org/license GPLv3 and PCL */ -namespace Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Request; +namespace Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Response; use Attribute; -use OpenApi\Attributes\JsonContent; -use OpenApi\Attributes\RequestBody; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\Refresh; +use OpenApi\Attributes\Response; +/** + * @internal + */ #[Attribute(Attribute::TARGET_METHOD)] -final class TokenRequestBody extends RequestBody +final class InvalidCredentials extends Response { public function __construct() { parent::__construct( - required: true, - content: new JsonContent(ref: Refresh::class) + response: 401, + description: 'Invalid credentials Response', ); } } diff --git a/src/Authorization/Controller/AuthorizationController.php b/src/Authorization/Controller/AuthorizationController.php deleted file mode 100644 index 4798e68d1..000000000 --- a/src/Authorization/Controller/AuthorizationController.php +++ /dev/null @@ -1,111 +0,0 @@ -name] - )] - #[CredentialsRequestBody] - #[SuccessResponse( - description: 'Token, lifetime and user identifier', - content: new JsonContent(ref: Token::class) - )] - #[DefaultResponses([ - HttpResponseCodes::UNAUTHORIZED - ])] - public function login(#[MapRequestPayload] Credentials $credentials): JsonResponse - { - /** @var User $user */ - $user = $this->securityService->authenticateUser($credentials); - - $token = $this->tokenService->generateAndSaveToken($user->getUserIdentifier()); - - return $this->jsonResponse(new Token($token, $this->tokenService->getLifetime(), $user->getUserIdentifier())); - } - - /** - * @throws TokenNotFoundException - */ - #[Route('/refresh', name: 'pimcore_studio_api_refresh', methods: ['POST'])] - #[POST( - path: self::API_PATH . '/refresh', - operationId: 'refresh', - summary: 'Login with user credentials and get access token', - tags: ['Authorization'] - )] - #[TokenRequestBody] - #[SuccessResponse( - description: 'Token, lifetime and user identifier', - content: new JsonContent(ref: Token::class) - )] - #[DefaultResponses([ - HttpResponseCodes::UNAUTHORIZED - ])] - public function refresh(#[MapRequestPayload] Refresh $refresh): JsonResponse - { - $tokenInfo = $this->tokenService->refreshToken($refresh->getToken()); - - return $this->jsonResponse( - new Token( - $tokenInfo->getToken(), - $this->tokenService->getLifetime(), - $tokenInfo->getUsername()) - ); - } -} diff --git a/src/Authorization/Controller/LoginController.php b/src/Authorization/Controller/LoginController.php new file mode 100644 index 000000000..7c277d064 --- /dev/null +++ b/src/Authorization/Controller/LoginController.php @@ -0,0 +1,57 @@ +name] + )] + #[CredentialsRequestBody] + #[SuccessResponse( + description: 'Login successful', + content: new JsonContent(ref: LoginSuccess::class) + )] + #[InvalidCredentials] + public function login(#[CurrentUser] User $user): JsonResponse + { + return $this->jsonResponse([ + 'username' => $user->getUserIdentifier(), + 'roles' => $user->getRoles() + ]); + } +} diff --git a/src/Authorization/Controller/LogoutController.php b/src/Authorization/Controller/LogoutController.php new file mode 100644 index 000000000..f922cd84e --- /dev/null +++ b/src/Authorization/Controller/LogoutController.php @@ -0,0 +1,46 @@ +name] + )] + #[SuccessResponse( + description: 'Logout successful', + )] + public function logout(): void + { + throw new Exception('Should not be called. Handled by symfony.'); + } +} diff --git a/src/Authorization/EventListener/LogoutListener.php b/src/Authorization/EventListener/LogoutListener.php new file mode 100644 index 000000000..7ad57b389 --- /dev/null +++ b/src/Authorization/EventListener/LogoutListener.php @@ -0,0 +1,40 @@ + 'onLogout' + ]; + } + + public function onLogout(LogoutEvent $event): void + { + $event->setResponse(new JsonResponse([])); + } +} \ No newline at end of file diff --git a/src/Authorization/Schema/Token.php b/src/Authorization/Schema/LoginSuccess.php similarity index 64% rename from src/Authorization/Schema/Token.php rename to src/Authorization/Schema/LoginSuccess.php index ed6f75a57..0a0e22704 100644 --- a/src/Authorization/Schema/Token.php +++ b/src/Authorization/Schema/LoginSuccess.php @@ -16,6 +16,7 @@ namespace Pimcore\Bundle\StudioBackendBundle\Authorization\Schema; +use OpenApi\Attributes\Items; use OpenApi\Attributes\Property; use OpenApi\Attributes\Schema; @@ -27,28 +28,20 @@ description: 'Token Scheme for API', type: 'object' )] -final readonly class Token +final readonly class LoginSuccess { public function __construct( - #[Property(description: 'Token', type: 'string', example: 'This could be your token')] - private string $token, - #[Property(description: 'Lifetime in seconds', type: 'integer', format: 'int', example: 3600)] - private int $lifetime, - #[Property(description: 'Username', type: 'string', example: 'shaquille.oatmeal')] - private string $username + #[Property(description: 'Username', type: 'string', example: 'admin')] + private string $username, + #[Property(description: 'Roles', type: 'array', items: new Items(type: 'string', example: 'ROLE_PIMCORE_ADMIN'))] + private array $roles, ) { } - public function getToken(): string + public function getRoles(): array { - return $this->token; + return $this->roles; } - - public function getLifetime(): int - { - return $this->lifetime; - } - public function getUsername(): string { return $this->username; diff --git a/src/Authorization/Service/TokenService.php b/src/Authorization/Service/TokenService.php deleted file mode 100644 index 3d046195d..000000000 --- a/src/Authorization/Service/TokenService.php +++ /dev/null @@ -1,111 +0,0 @@ -tokenGenerator->generateToken(); - $entry = $this->tmpStoreResolver->get($token); - } while ($entry !== null); - - $this->saveToken(new Info($token, $userIdentifier)); - - return $token; - } - - /** - * @throws TokenNotFoundException - */ - public function refreshToken(string $token): Info - { - $entry = $this->tmpStoreResolver->get($token); - if($entry === null) { - throw new TokenNotFoundException('Token not found'); - } - - $data = $entry->getData(); - - if(!isset($data['username'])) { - throw new TokenNotFoundException('Token not found'); - } - - $tokenInfo = new Info($token, $data['username']); - $this->saveToken($tokenInfo); - - return $tokenInfo; - } - - public function getLifetime(): int - { - return $this->tokenLifetime; - } - - private function saveToken(Info $tokenInfo): void - { - $this->tmpStoreResolver->set( - $tokenInfo->getToken(), - [ - 'username' => $tokenInfo->getUsername(), - ], - $this->getTmpStoreTag($tokenInfo->getUsername()), - $this->tokenLifetime - ); - } - - public function getCurrentToken(): string - { - $currentRequest = $this->getCurrentRequest($this->requestStack); - - return $this->getAuthToken($currentRequest); - } - - private function getTmpStoreTag(string $userId): string - { - return str_replace( - self::TMP_STORE_TAG_PLACEHOLDER, - $userId, - self::TMP_STORE_TAG - ); - } -} diff --git a/src/Authorization/Service/TokenServiceInterface.php b/src/Authorization/Service/TokenServiceInterface.php deleted file mode 100644 index e987d8d04..000000000 --- a/src/Authorization/Service/TokenServiceInterface.php +++ /dev/null @@ -1,37 +0,0 @@ -addOpenApiScanPathsNode($rootNode); $this->addApiTokenNode($rootNode); $this->addAllowedHostsForCorsNode($rootNode); + $this->addSecurityFirewall($rootNode); return $treeBuilder; } @@ -52,7 +53,6 @@ private function addOpenApiScanPathsNode(ArrayNodeDefinition $node): void $node->children() ->arrayNode('open_api_scan_paths') ->prototype('scalar')->end() - ->validate() ->always( function ($paths) { foreach ($paths as $path) { @@ -111,4 +111,12 @@ private function addAllowedHostsForCorsNode(ArrayNodeDefinition $node): void ->end() ->end(); } + + public function addSecurityFirewall(ArrayNodeDefinition $node): void + { + $node + ->children() + ->variableNode('security_firewall')->end() + ->end(); + } } diff --git a/src/DependencyInjection/PimcoreStudioBackendExtension.php b/src/DependencyInjection/PimcoreStudioBackendExtension.php index 30687e120..c423eebe3 100644 --- a/src/DependencyInjection/PimcoreStudioBackendExtension.php +++ b/src/DependencyInjection/PimcoreStudioBackendExtension.php @@ -17,11 +17,12 @@ namespace Pimcore\Bundle\StudioBackendBundle\DependencyInjection; use Exception; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Service\TokenServiceInterface; +use Pimcore\Bundle\CoreBundle\DependencyInjection\ConfigurationHelper; use Pimcore\Bundle\StudioBackendBundle\EventSubscriber\CorsSubscriber; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Service\OpenApiServiceInterface; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface; use Symfony\Component\DependencyInjection\Loader\YamlFileLoader; use Symfony\Component\HttpKernel\DependencyInjection\Extension; @@ -34,7 +35,7 @@ /** * @internal */ -class PimcoreStudioBackendExtension extends Extension +class PimcoreStudioBackendExtension extends Extension implements PrependExtensionInterface { /** * {@inheritdoc} @@ -71,13 +72,18 @@ public function load(array $configs, ContainerBuilder $container): void $loader->load('updater.yaml'); $loader->load('versions.yaml'); - $definition = $container->getDefinition(TokenServiceInterface::class); - $definition->setArgument('$tokenLifetime', $config['api_token']['lifetime']); - $definition = $container->getDefinition(OpenApiServiceInterface::class); $definition->setArgument('$openApiScanPaths', $config['open_api_scan_paths']); $definition = $container->getDefinition(CorsSubscriber::class); $definition->setArgument('$allowedHosts', $config['allowed_hosts_for_cors']); } + + public function prepend(ContainerBuilder $container): void + { + if (!$container->hasParameter('pimcore_studio_backend.firewall_settings')) { + $containerConfig = ConfigurationHelper::getConfigNodeFromSymfonyTree($container, 'pimcore_studio_backend'); + $container->setParameter('pimcore_studio_backend.firewall_settings', $containerConfig['security_firewall']); + } + } } diff --git a/src/Security/Service/SecurityService.php b/src/Security/Service/SecurityService.php index 5ef9ee5ac..f7a36582b 100644 --- a/src/Security/Service/SecurityService.php +++ b/src/Security/Service/SecurityService.php @@ -17,12 +17,10 @@ namespace Pimcore\Bundle\StudioBackendBundle\Security\Service; use Pimcore\Bundle\GenericDataIndexBundle\Service\Permission\ElementPermissionServiceInterface; +use Pimcore\Bundle\StaticResolverBundle\Lib\Tools\Authentication\AuthenticationResolverInterface; use Pimcore\Bundle\StaticResolverBundle\Models\Tool\TmpStoreResolverInterface; -use Pimcore\Bundle\StaticResolverBundle\Models\User\UserResolverInterface; use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\Credentials; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Service\TokenServiceInterface; use Pimcore\Bundle\StudioBackendBundle\Exception\AccessDeniedException; -use Pimcore\Bundle\StudioBackendBundle\Exception\NotAuthorizedException; use Pimcore\Model\Element\ElementInterface; use Pimcore\Model\User; use Pimcore\Model\UserInterface; @@ -39,10 +37,9 @@ public function __construct( private ElementPermissionServiceInterface $elementPermissionService, private UserProvider $userProvider, - private UserResolverInterface $userResolver, private UserPasswordHasherInterface $passwordHasher, private TmpStoreResolverInterface $tmpStoreResolver, - private TokenServiceInterface $tokenService, + private AuthenticationResolverInterface $authenticationResolver, ) { } @@ -75,23 +72,16 @@ public function checkAuthToken(string $token): bool } /** - * @throws NotAuthorizedException + * @throws UserNotFoundException */ public function getCurrentUser(): UserInterface { - $entry = $this->tmpStoreResolver->get($this->tokenService->getCurrentToken()); - - if($entry === null || !is_array($entry->getData()) || !isset($entry->getData()['username'])) { - throw new NotAuthorizedException(); + $pimcoreUser = $this->authenticationResolver->authenticateSession(); + if (!$pimcoreUser instanceof User) { + throw new UserNotFoundException('No pimcore user found'); } - $user = $this->userResolver->getByName($entry->getData()['username']); - - if(!$user) { - throw new NotAuthorizedException(); - } - - return $user; + return $pimcoreUser; } /** diff --git a/src/Security/Voter/AuthorizationVoter.php b/src/Security/Voter/AuthorizationVoter.php index 9d38c196b..734f0e376 100644 --- a/src/Security/Voter/AuthorizationVoter.php +++ b/src/Security/Voter/AuthorizationVoter.php @@ -16,10 +16,8 @@ namespace Pimcore\Bundle\StudioBackendBundle\Security\Voter; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Service\TokenServiceInterface; use Pimcore\Bundle\StudioBackendBundle\Exception\NoRequestException; use Pimcore\Bundle\StudioBackendBundle\Exception\NotAuthorizedException; -use Pimcore\Bundle\StudioBackendBundle\Security\Service\SecurityServiceInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\Voter\Voter; @@ -30,13 +28,6 @@ final class AuthorizationVoter extends Voter { private const SUPPORTED_ATTRIBUTE = 'STUDIO_API'; - public function __construct( - private readonly TokenServiceInterface $tokenService, - private readonly SecurityServiceInterface $securityService - - ) { - } - /** * @inheritDoc */ @@ -54,12 +45,6 @@ protected function voteOnAttribute(string $attribute, mixed $subject, TokenInter return false; } - $authToken = $this->tokenService->getCurrentToken(); - - if(!$this->securityService->checkAuthToken($authToken)) { - throw new NotAuthorizedException(); - } - return true; } } From ffab95a123064f84da716830946d8bd1a82c079c Mon Sep 17 00:00:00 2001 From: martineiber Date: Wed, 5 Jun 2024 12:14:10 +0000 Subject: [PATCH 02/28] Apply php-cs-fixer changes --- src/Authorization/Controller/LoginController.php | 4 ++-- src/Authorization/Controller/LogoutController.php | 1 - src/Authorization/EventListener/LogoutListener.php | 7 +++---- src/Security/Voter/AuthorizationVoter.php | 2 +- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Authorization/Controller/LoginController.php b/src/Authorization/Controller/LoginController.php index 7c277d064..87701efd1 100644 --- a/src/Authorization/Controller/LoginController.php +++ b/src/Authorization/Controller/LoginController.php @@ -19,6 +19,7 @@ use OpenApi\Attributes\JsonContent; use OpenApi\Attributes\Post; use Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Request\CredentialsRequestBody; +use Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Response\InvalidCredentials; use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\LoginSuccess; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\SuccessResponse; @@ -27,7 +28,6 @@ use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\Routing\Attribute\Route; use Symfony\Component\Security\Http\Attribute\CurrentUser; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Response\InvalidCredentials; /** * @internal @@ -51,7 +51,7 @@ public function login(#[CurrentUser] User $user): JsonResponse { return $this->jsonResponse([ 'username' => $user->getUserIdentifier(), - 'roles' => $user->getRoles() + 'roles' => $user->getRoles(), ]); } } diff --git a/src/Authorization/Controller/LogoutController.php b/src/Authorization/Controller/LogoutController.php index f922cd84e..10c0e185b 100644 --- a/src/Authorization/Controller/LogoutController.php +++ b/src/Authorization/Controller/LogoutController.php @@ -18,7 +18,6 @@ use Exception; use OpenApi\Attributes\Post; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Response\LogoutSuccessful; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\SuccessResponse; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Config\Tags; diff --git a/src/Authorization/EventListener/LogoutListener.php b/src/Authorization/EventListener/LogoutListener.php index 7ad57b389..666592812 100644 --- a/src/Authorization/EventListener/LogoutListener.php +++ b/src/Authorization/EventListener/LogoutListener.php @@ -14,12 +14,11 @@ * @license http://www.pimcore.org/license GPLv3 and PCL */ - namespace Pimcore\Bundle\StudioBackendBundle\Authorization\EventListener; -use Symfony\Component\Security\Http\Event\LogoutEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\JsonResponse; +use Symfony\Component\Security\Http\Event\LogoutEvent; /** * @internal @@ -29,7 +28,7 @@ final class LogoutListener implements EventSubscriberInterface public static function getSubscribedEvents(): array { return [ - LogoutEvent::class => 'onLogout' + LogoutEvent::class => 'onLogout', ]; } @@ -37,4 +36,4 @@ public function onLogout(LogoutEvent $event): void { $event->setResponse(new JsonResponse([])); } -} \ No newline at end of file +} diff --git a/src/Security/Voter/AuthorizationVoter.php b/src/Security/Voter/AuthorizationVoter.php index 734f0e376..f9c434348 100644 --- a/src/Security/Voter/AuthorizationVoter.php +++ b/src/Security/Voter/AuthorizationVoter.php @@ -41,7 +41,7 @@ protected function supports(string $attribute, mixed $subject): bool */ protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool { - if($attribute !== self::SUPPORTED_ATTRIBUTE) { + if ($attribute !== self::SUPPORTED_ATTRIBUTE) { return false; } From 2e2227601b6b8deee54e43696ed1d04c1f16688a Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Wed, 5 Jun 2024 14:42:42 +0200 Subject: [PATCH 03/28] Refactor path checker for the scan path on open api. --- src/DependencyInjection/Configuration.php | 15 --------------- .../PimcoreStudioBackendExtension.php | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 157568332..b337c3380 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -53,21 +53,6 @@ private function addOpenApiScanPathsNode(ArrayNodeDefinition $node): void $node->children() ->arrayNode('open_api_scan_paths') ->prototype('scalar')->end() - ->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(); } diff --git a/src/DependencyInjection/PimcoreStudioBackendExtension.php b/src/DependencyInjection/PimcoreStudioBackendExtension.php index 893bb7161..ee2a7790f 100644 --- a/src/DependencyInjection/PimcoreStudioBackendExtension.php +++ b/src/DependencyInjection/PimcoreStudioBackendExtension.php @@ -19,6 +19,7 @@ use Exception; use Pimcore\Bundle\CoreBundle\DependencyInjection\ConfigurationHelper; use Pimcore\Bundle\StudioBackendBundle\EventSubscriber\CorsSubscriber; +use Pimcore\Bundle\StudioBackendBundle\Exception\InvalidPathException; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Service\OpenApiServiceInterface; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -73,6 +74,7 @@ public function load(array $configs, ContainerBuilder $container): void $loader->load('users.yaml'); $loader->load('versions.yaml'); + $this->checkValidOpenApiScanPaths($config['open_api_scan_paths']); $definition = $container->getDefinition(OpenApiServiceInterface::class); $definition->setArgument('$openApiScanPaths', $config['open_api_scan_paths']); @@ -87,4 +89,18 @@ public function prepend(ContainerBuilder $container): void $container->setParameter('pimcore_studio_backend.firewall_settings', $containerConfig['security_firewall']); } } + + private function checkValidOpenApiScanPaths(array $config): void + { + foreach ($config as $path) { + if (!is_dir($path)) { + throw new InvalidPathException( + sprintf( + 'The path "%s" is not a valid directory.', + $path + ) + ); + } + } + } } From 13dd544e3ed2c09fcaca33fa34107c94d5da998f Mon Sep 17 00:00:00 2001 From: martineiber Date: Wed, 5 Jun 2024 12:43:24 +0000 Subject: [PATCH 04/28] Apply php-cs-fixer changes --- src/DependencyInjection/Configuration.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index b337c3380..cd11bca7d 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -17,7 +17,6 @@ namespace Pimcore\Bundle\StudioBackendBundle\DependencyInjection; use Pimcore\Bundle\StudioBackendBundle\Exception\InvalidHostException; -use Pimcore\Bundle\StudioBackendBundle\Exception\InvalidPathException; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; From b6f30b90779d7f2cb0097dff9d81bc535c90f2d7 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Wed, 5 Jun 2024 15:30:29 +0200 Subject: [PATCH 05/28] Remove Security Schema. --- .../Controller/CustomSettingsController.php | 1 - src/Asset/Controller/Data/TextController.php | 1 - src/Asset/Controller/GetController.php | 1 - src/Asset/Controller/UpdateController.php | 1 - src/Controller/AbstractApiController.php | 2 - .../Controller/CollectionController.php | 1 - src/DataObject/Controller/GetController.php | 1 - .../Element/CollectionController.php | 1 - src/Note/Controller/CollectionController.php | 1 - src/Note/Controller/DeleteController.php | 1 - .../Element/CollectionController.php | 1 - .../Controller/Element/CreateController.php | 1 - src/OpenApi/Config/Security.php | 37 ------------------- .../Controller/CollectionController.php | 1 - src/Property/Controller/CreateController.php | 1 - src/Property/Controller/DeleteController.php | 1 - .../Element/CollectionController.php | 1 - src/Property/Controller/UpdateController.php | 1 - src/Schedule/Controller/DeleteController.php | 1 - .../Element/CollectionController.php | 1 - .../Controller/Element/CreateController.php | 1 - .../Controller/Element/UpdateController.php | 1 - src/Setting/Controller/GetController.php | 1 - .../Controller/TranslationController.php | 1 - src/Version/Controller/DeleteController.php | 1 - .../Controller/Element/CleanupController.php | 1 - .../Element/CollectionController.php | 1 - src/Version/Controller/GetController.php | 1 - src/Version/Controller/PublishController.php | 1 - .../DetailsCollectionController.php | 1 - .../Controller/SubmitActionController.php | 1 - 31 files changed, 68 deletions(-) delete mode 100644 src/OpenApi/Config/Security.php diff --git a/src/Asset/Controller/CustomSettingsController.php b/src/Asset/Controller/CustomSettingsController.php index e75189420..5d06f123d 100644 --- a/src/Asset/Controller/CustomSettingsController.php +++ b/src/Asset/Controller/CustomSettingsController.php @@ -55,7 +55,6 @@ public function __construct( operationId: 'getAssetCustomSettingsById', description: 'Get custom settings of an asset by its id by path parameter', summary: 'Get custom settings of an asset by id', - security: self::SECURITY_SCHEME, tags: [Tags::Assets->name] )] #[IdParameter(type: 'asset')] diff --git a/src/Asset/Controller/Data/TextController.php b/src/Asset/Controller/Data/TextController.php index a3b2d08c0..24218e4e0 100644 --- a/src/Asset/Controller/Data/TextController.php +++ b/src/Asset/Controller/Data/TextController.php @@ -58,7 +58,6 @@ public function __construct( path: self::API_PATH . '/assets/{id}/text', operationId: 'getAssetDataTextById', summary: 'Get asset data in text UTF8 representation by id', - security: self::SECURITY_SCHEME, tags: [Tags::Assets->name] )] #[IdParameter(type: 'asset')] diff --git a/src/Asset/Controller/GetController.php b/src/Asset/Controller/GetController.php index 93d660e59..2cb35d868 100644 --- a/src/Asset/Controller/GetController.php +++ b/src/Asset/Controller/GetController.php @@ -54,7 +54,6 @@ public function __construct( operationId: 'getAssetById', description: 'Get assets by id by path parameter', summary: 'Get assets by id', - security: self::SECURITY_SCHEME, tags: [Tags::Assets->name] )] #[IdParameter(type: 'asset')] diff --git a/src/Asset/Controller/UpdateController.php b/src/Asset/Controller/UpdateController.php index 14c4a41fe..6b05f9b80 100644 --- a/src/Asset/Controller/UpdateController.php +++ b/src/Asset/Controller/UpdateController.php @@ -54,7 +54,6 @@ public function __construct( operationId: 'updateAssetById', description: 'Update assets by id', summary: 'Update asset', - security: self::SECURITY_SCHEME, tags: [Tags::Assets->name] )] #[IdParameter(type: 'asset')] diff --git a/src/Controller/AbstractApiController.php b/src/Controller/AbstractApiController.php index 0fb379e48..19dad7408 100644 --- a/src/Controller/AbstractApiController.php +++ b/src/Controller/AbstractApiController.php @@ -33,8 +33,6 @@ abstract class AbstractApiController extends AbstractController public const API_PATH = '/studio/api'; - public const SECURITY_SCHEME = [['auth_token' => []]]; - public function __construct(protected readonly SerializerInterface $serializer) { diff --git a/src/DataObject/Controller/CollectionController.php b/src/DataObject/Controller/CollectionController.php index 793c738cd..a91f08acd 100644 --- a/src/DataObject/Controller/CollectionController.php +++ b/src/DataObject/Controller/CollectionController.php @@ -66,7 +66,6 @@ public function __construct( operationId: 'getDataObjects', description: 'Get paginated data objects', summary: 'Get all DataObjects', - security: self::SECURITY_SCHEME, tags: [Tags::DataObjects->name], )] #[PageParameter] diff --git a/src/DataObject/Controller/GetController.php b/src/DataObject/Controller/GetController.php index 7871d4adc..1dfb87feb 100644 --- a/src/DataObject/Controller/GetController.php +++ b/src/DataObject/Controller/GetController.php @@ -49,7 +49,6 @@ public function __construct( operationId: 'getDataObjectById', description: 'Get data object by id by path parameter', summary: 'Get data object by id', - security: self::SECURITY_SCHEME, tags: [Tags::DataObjects->name] )] #[IdParameter(type: 'data-object')] diff --git a/src/Dependency/Controller/Element/CollectionController.php b/src/Dependency/Controller/Element/CollectionController.php index fa796980c..a9576c200 100644 --- a/src/Dependency/Controller/Element/CollectionController.php +++ b/src/Dependency/Controller/Element/CollectionController.php @@ -63,7 +63,6 @@ public function __construct( Pass dependency mode to get either all elements that depend on the provided element or all dependencies for the provided element.', summary: 'Get all dependencies for provided element.', - security: self::SECURITY_SCHEME, tags: [Tags::Dependencies->name] )] #[ElementTypeParameter] diff --git a/src/Note/Controller/CollectionController.php b/src/Note/Controller/CollectionController.php index 9ed088eb6..c1b167e3f 100644 --- a/src/Note/Controller/CollectionController.php +++ b/src/Note/Controller/CollectionController.php @@ -66,7 +66,6 @@ public function __construct( path: self::API_PATH . '/notes', operationId: 'getNotes', summary: 'Get notes', - security: self::SECURITY_SCHEME, tags: [Tags::Notes->name] )] #[PageParameter] diff --git a/src/Note/Controller/DeleteController.php b/src/Note/Controller/DeleteController.php index ea3adcafc..d1dd3a817 100644 --- a/src/Note/Controller/DeleteController.php +++ b/src/Note/Controller/DeleteController.php @@ -54,7 +54,6 @@ public function __construct( path: self::API_PATH . '/notes/{id}', operationId: 'deleteNote', summary: 'Deleting note by id', - security: self::SECURITY_SCHEME, tags: [Tags::Notes->name] )] #[IdParameter] diff --git a/src/Note/Controller/Element/CollectionController.php b/src/Note/Controller/Element/CollectionController.php index 1959c2b44..aef290bbe 100644 --- a/src/Note/Controller/Element/CollectionController.php +++ b/src/Note/Controller/Element/CollectionController.php @@ -68,7 +68,6 @@ public function __construct( path: self::API_PATH . '/notes/{elementType}/{id}', operationId: 'getNotesForElementByTypeAndId', summary: 'Get notes for an element', - security: self::SECURITY_SCHEME, tags: [Tags::NotesForElement->name] )] #[ElementTypeParameter] diff --git a/src/Note/Controller/Element/CreateController.php b/src/Note/Controller/Element/CreateController.php index 8ce71d025..3a2cf6429 100644 --- a/src/Note/Controller/Element/CreateController.php +++ b/src/Note/Controller/Element/CreateController.php @@ -58,7 +58,6 @@ public function __construct( path: self::API_PATH . '/notes/{elementType}/{id}', operationId: 'createNoteForElement', summary: 'Creating new note for element', - security: self::SECURITY_SCHEME, tags: [Tags::NotesForElement->name] )] #[ElementTypeParameter] diff --git a/src/OpenApi/Config/Security.php b/src/OpenApi/Config/Security.php deleted file mode 100644 index d25ff43f0..000000000 --- a/src/OpenApi/Config/Security.php +++ /dev/null @@ -1,37 +0,0 @@ -name] )] #[ElementTypeParameter(false, null)] diff --git a/src/Property/Controller/CreateController.php b/src/Property/Controller/CreateController.php index ff65575e9..65d6bc007 100644 --- a/src/Property/Controller/CreateController.php +++ b/src/Property/Controller/CreateController.php @@ -50,7 +50,6 @@ public function __construct( path: self::API_PATH . '/property', operationId: 'createProperty', summary: 'Creating new property with default values', - security: self::SECURITY_SCHEME, tags: [Tags::Properties->name] )] #[SuccessResponse( diff --git a/src/Property/Controller/DeleteController.php b/src/Property/Controller/DeleteController.php index bf2d51b31..ea27cfdb0 100644 --- a/src/Property/Controller/DeleteController.php +++ b/src/Property/Controller/DeleteController.php @@ -52,7 +52,6 @@ public function __construct( path: self::API_PATH . '/properties/{id}', operationId: 'deleteProperty', summary: 'Delete property with given id', - security: self::SECURITY_SCHEME, tags: [Tags::Properties->name] )] #[IdParameter(type: 'property', schema: new Schema(type: 'string', example: 'alpha-numerical'))] diff --git a/src/Property/Controller/Element/CollectionController.php b/src/Property/Controller/Element/CollectionController.php index a69e49872..ae532ec82 100644 --- a/src/Property/Controller/Element/CollectionController.php +++ b/src/Property/Controller/Element/CollectionController.php @@ -53,7 +53,6 @@ public function __construct( path: self::API_PATH . '/properties/{elementType}/{id}', operationId: 'getPropertiesForElementByTypeAndId', summary: 'Get properties for an element based on the element type and the element id', - security: self::SECURITY_SCHEME, tags: [Tags::PropertiesForElement->value] )] #[ElementTypeParameter] diff --git a/src/Property/Controller/UpdateController.php b/src/Property/Controller/UpdateController.php index 92b1f5da2..767d8bc22 100644 --- a/src/Property/Controller/UpdateController.php +++ b/src/Property/Controller/UpdateController.php @@ -55,7 +55,6 @@ public function __construct( path: self::API_PATH . '/properties/{id}', operationId: 'updateProperty', summary: 'Updating a property', - security: self::SECURITY_SCHEME, tags: [Tags::Properties->name] )] #[IdParameter(type: 'property', schema: new Schema(type: 'string', example: 'alpha-numerical'))] diff --git a/src/Schedule/Controller/DeleteController.php b/src/Schedule/Controller/DeleteController.php index ad6108270..905b2cc7c 100644 --- a/src/Schedule/Controller/DeleteController.php +++ b/src/Schedule/Controller/DeleteController.php @@ -53,7 +53,6 @@ public function __construct( path: self::API_PATH . '/schedules/{id}', operationId: 'deleteSchedule', summary: 'Delete schedule with given id', - security: self::SECURITY_SCHEME, tags: [Tags::Schedule->name] )] #[IdParameter(type: 'schedule', schema: new Schema(type: 'integer', example: 123))] diff --git a/src/Schedule/Controller/Element/CollectionController.php b/src/Schedule/Controller/Element/CollectionController.php index eff362184..d59559d7d 100644 --- a/src/Schedule/Controller/Element/CollectionController.php +++ b/src/Schedule/Controller/Element/CollectionController.php @@ -52,7 +52,6 @@ public function __construct( path: self::API_PATH . '/schedules/{elementType}/{id}', operationId: 'getSchedulesForElementByTypeAndId', summary: 'Get schedules for an element', - security: self::SECURITY_SCHEME, tags: [Tags::Schedule->name] )] #[ElementTypeParameter] diff --git a/src/Schedule/Controller/Element/CreateController.php b/src/Schedule/Controller/Element/CreateController.php index bc174a224..31839e56d 100644 --- a/src/Schedule/Controller/Element/CreateController.php +++ b/src/Schedule/Controller/Element/CreateController.php @@ -54,7 +54,6 @@ public function __construct( path: self::API_PATH . '/schedules/{elementType}/{id}', operationId: 'createSchedule', summary: 'Create schedule for element', - security: self::SECURITY_SCHEME, tags: [Tags::Schedule->name] )] #[ElementTypeParameter] diff --git a/src/Schedule/Controller/Element/UpdateController.php b/src/Schedule/Controller/Element/UpdateController.php index 6d682e04c..bb30f2e70 100644 --- a/src/Schedule/Controller/Element/UpdateController.php +++ b/src/Schedule/Controller/Element/UpdateController.php @@ -59,7 +59,6 @@ public function __construct( path: self::API_PATH . '/schedules/{elementType}/{id}', operationId: 'updateSchedulesForElementByTypeAndId', summary: 'Update schedules for an element', - security: self::SECURITY_SCHEME, tags: [Tags::Schedule->name] )] #[ElementTypeParameter] diff --git a/src/Setting/Controller/GetController.php b/src/Setting/Controller/GetController.php index 60c39442f..c6fb757e4 100644 --- a/src/Setting/Controller/GetController.php +++ b/src/Setting/Controller/GetController.php @@ -47,7 +47,6 @@ public function __construct( operationId: 'getSystemSettings', description: 'Get system settings', summary: 'Get system settings', - security: self::SECURITY_SCHEME, tags: [Tags::Settings->name] )] #[SuccessResponse( diff --git a/src/Translation/Controller/TranslationController.php b/src/Translation/Controller/TranslationController.php index 6d0d583d8..0cc5429fd 100644 --- a/src/Translation/Controller/TranslationController.php +++ b/src/Translation/Controller/TranslationController.php @@ -56,7 +56,6 @@ public function __construct( operationId: 'getTranslations', description: 'Get translations for given keys and locale', summary: 'Get translations', - security: self::SECURITY_SCHEME, tags: [Tags::Translation->name] )] #[TranslationRequestBody] diff --git a/src/Version/Controller/DeleteController.php b/src/Version/Controller/DeleteController.php index 33f7a17b2..0f3390159 100644 --- a/src/Version/Controller/DeleteController.php +++ b/src/Version/Controller/DeleteController.php @@ -51,7 +51,6 @@ public function __construct( operationId: 'deleteVersion', description: 'Delete version based on the version ID', summary: 'Delete version', - security: self::SECURITY_SCHEME, tags: [Tags::Versions->name] )] #[IdParameter(type: 'version')] diff --git a/src/Version/Controller/Element/CleanupController.php b/src/Version/Controller/Element/CleanupController.php index c872ccf59..689739fe3 100644 --- a/src/Version/Controller/Element/CleanupController.php +++ b/src/Version/Controller/Element/CleanupController.php @@ -54,7 +54,6 @@ public function __construct( operationId: 'cleanupVersion', description: 'Cleanup versions based on the provided parameters', summary: 'Cleanup versions', - security: self::SECURITY_SCHEME, tags: [Tags::Versions->name] )] #[ElementTypeParameter] diff --git a/src/Version/Controller/Element/CollectionController.php b/src/Version/Controller/Element/CollectionController.php index ccbe659a8..a7f652466 100644 --- a/src/Version/Controller/Element/CollectionController.php +++ b/src/Version/Controller/Element/CollectionController.php @@ -60,7 +60,6 @@ public function __construct( operationId: 'getVersions', description: 'Get paginated versions', summary: 'Get all versions of element', - security: self::SECURITY_SCHEME, tags: [Tags::Versions->name] )] #[ElementTypeParameter] diff --git a/src/Version/Controller/GetController.php b/src/Version/Controller/GetController.php index 44968af12..6558376aa 100644 --- a/src/Version/Controller/GetController.php +++ b/src/Version/Controller/GetController.php @@ -50,7 +50,6 @@ public function __construct( operationId: 'getVersionById', description: 'Get version based on the version ID', summary: 'Get version by ID', - security: self::SECURITY_SCHEME, tags: [Tags::Versions->name] )] #[IdParameter(type: 'version')] diff --git a/src/Version/Controller/PublishController.php b/src/Version/Controller/PublishController.php index de6db93d2..ee578faeb 100644 --- a/src/Version/Controller/PublishController.php +++ b/src/Version/Controller/PublishController.php @@ -50,7 +50,6 @@ public function __construct( operationId: 'publishVersion', description: 'Publish element based on the version ID', summary: 'Publish version', - security: self::SECURITY_SCHEME, tags: [Tags::Versions->name] )] #[IdParameter(type: 'version')] diff --git a/src/Workflow/Controller/DetailsCollectionController.php b/src/Workflow/Controller/DetailsCollectionController.php index 6670dc7e0..ecd53d455 100644 --- a/src/Workflow/Controller/DetailsCollectionController.php +++ b/src/Workflow/Controller/DetailsCollectionController.php @@ -57,7 +57,6 @@ public function __construct( operationId: 'getWorkflowsDetails', description: 'Get details of the element workflows', summary: 'Get all workflow details of an element', - security: self::SECURITY_SCHEME, tags: [Tags::Workflows->name] )] #[IdParameter('ID of the element', 'element')] diff --git a/src/Workflow/Controller/SubmitActionController.php b/src/Workflow/Controller/SubmitActionController.php index fd0a8ec91..2b02395f7 100644 --- a/src/Workflow/Controller/SubmitActionController.php +++ b/src/Workflow/Controller/SubmitActionController.php @@ -56,7 +56,6 @@ public function __construct( operationId: 'submitWorkflowAction', description: 'Submit action based on the workflow name, action name and action type', summary: 'Submit workflow action', - security: self::SECURITY_SCHEME, tags: [Tags::Workflows->name] )] #[WorkflowActionRequestBody] From d5676d87d12a99a8a6e7d0a05e2b30dc5a765690 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Wed, 5 Jun 2024 15:39:01 +0200 Subject: [PATCH 06/28] use RuntimeException. --- src/Authorization/Controller/LogoutController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Authorization/Controller/LogoutController.php b/src/Authorization/Controller/LogoutController.php index 10c0e185b..e945e6978 100644 --- a/src/Authorization/Controller/LogoutController.php +++ b/src/Authorization/Controller/LogoutController.php @@ -16,7 +16,7 @@ namespace Pimcore\Bundle\StudioBackendBundle\Authorization\Controller; -use Exception; +use RuntimeException; use OpenApi\Attributes\Post; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\SuccessResponse; @@ -40,6 +40,6 @@ final class LogoutController extends AbstractApiController )] public function logout(): void { - throw new Exception('Should not be called. Handled by symfony.'); + throw new RuntimeException('Should not be called. Handled by symfony.'); } } From defebbffbd98619906afc4006a263ec902754643 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Wed, 5 Jun 2024 15:39:39 +0200 Subject: [PATCH 07/28] Fix to long line. --- src/Authorization/Schema/LoginSuccess.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Authorization/Schema/LoginSuccess.php b/src/Authorization/Schema/LoginSuccess.php index 0a0e22704..7dbbde31d 100644 --- a/src/Authorization/Schema/LoginSuccess.php +++ b/src/Authorization/Schema/LoginSuccess.php @@ -33,7 +33,11 @@ public function __construct( #[Property(description: 'Username', type: 'string', example: 'admin')] private string $username, - #[Property(description: 'Roles', type: 'array', items: new Items(type: 'string', example: 'ROLE_PIMCORE_ADMIN'))] + #[Property( + description: 'Roles', + type: 'array', + items: new Items(type: 'string', example: 'ROLE_PIMCORE_ADMIN') + )] private array $roles, ) { } From 2f20b1f9c208879db0d7a283b1a300148fd810f7 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Wed, 5 Jun 2024 15:42:07 +0200 Subject: [PATCH 08/28] Remove SECURITY_SCHEME --- src/Asset/Controller/CollectionController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Asset/Controller/CollectionController.php b/src/Asset/Controller/CollectionController.php index 8a6662739..3a781d415 100644 --- a/src/Asset/Controller/CollectionController.php +++ b/src/Asset/Controller/CollectionController.php @@ -73,7 +73,6 @@ public function __construct( operationId: 'getAssets', description: 'Get paginated assets', summary: 'Get all assets', - security: self::SECURITY_SCHEME, tags: [Tags::Assets->name] )] #[PageParameter] From 412d054f6797b52633346d72454d66b4569fc5b6 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 06:52:50 +0200 Subject: [PATCH 09/28] Adapt SecurityServiceTest. --- .../Service/Security/SecurityServiceTest.php | 47 +++++++------------ 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/tests/Unit/Service/Security/SecurityServiceTest.php b/tests/Unit/Service/Security/SecurityServiceTest.php index 79d1109a0..5e0b91e1a 100644 --- a/tests/Unit/Service/Security/SecurityServiceTest.php +++ b/tests/Unit/Service/Security/SecurityServiceTest.php @@ -19,12 +19,11 @@ use Codeception\Test\Unit; use Exception; use Pimcore\Bundle\GenericDataIndexBundle\Service\Permission\ElementPermissionServiceInterface; +use Pimcore\Bundle\StaticResolverBundle\Lib\Tools\Authentication\AuthenticationResolverInterface; use Pimcore\Bundle\StaticResolverBundle\Models\Tool\TmpStoreResolverInterface; -use Pimcore\Bundle\StaticResolverBundle\Models\User\UserResolverInterface; use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\Credentials; use Pimcore\Bundle\StudioBackendBundle\Authorization\Service\TokenServiceInterface; use Pimcore\Bundle\StudioBackendBundle\Exception\AccessDeniedException; -use Pimcore\Bundle\StudioBackendBundle\Exception\NotAuthorizedException; use Pimcore\Bundle\StudioBackendBundle\Security\Service\SecurityService; use Pimcore\Bundle\StudioBackendBundle\Security\Service\SecurityServiceInterface; use Pimcore\Model\Asset; @@ -96,24 +95,25 @@ public function testTokenAllowedFalse(): void /** * @throws Exception */ - public function testGetCurrentUserWithInvalidToken(): void + public function testGetCurrentUserWithOutValidUser(): void { $securityService = $this->mockSecurityService(false, false, false); - $this->expectException(NotAuthorizedException::class); + $this->expectException(UserNotFoundException::class); $securityService->getCurrentUser(); } /** * @throws Exception */ - public function testGetCurrentUserWithValidToken(): void + public function testGetCurrentUserWithValidUser(): void { - $securityService = $this->mockSecurityService(); + $securityService = $this->mockSecurityService(false, true, false); $user = $securityService->getCurrentUser(); $this->assertInstanceOf(PimcoreUser::class, $user); + $this->assertSame('test', $user->getUsername()); } /** @@ -144,15 +144,14 @@ private function mockSecurityService( $validPassword = true, bool $withUser = true, bool $withTmpStore = true, - bool $hasPermission = true + bool $hasPermission = true, ): SecurityServiceInterface { return new SecurityService( $this->mockElementPermissionService($hasPermission), $withUser ? $this->mockUserProviderWithUser() : $this->mockUserProviderWithOutUser(), - $this->mockUserResolverService(), $this->mockPasswordHasher($validPassword), $this->mockTmpStoreResolver($withTmpStore), - $this->mockTokenService(), + $this->mockAuthenticationResolver($withUser) ); } @@ -180,26 +179,6 @@ private function mockUserProviderWithOutUser(): UserProvider ]); } - /** - * @throws Exception - */ - private function mockUserResolverService(): UserResolverInterface - { - return $this->makeEmpty(UserResolverInterface::class, [ - 'getByName' => new PimcoreUser(), - ]); - } - - /** - * @throws Exception - */ - private function mockTokenService(): TokenServiceInterface - { - return $this->makeEmpty(TokenServiceInterface::class, [ - 'getCurrentToken' => 'test', - ]); - } - /** * @throws Exception */ @@ -235,4 +214,14 @@ private function mockElementPermissionService(bool $hasPermission): ElementPermi 'isAllowed' => $hasPermission, ]); } + + private function mockAuthenticationResolver(bool $withUser): AuthenticationResolverInterface + { + $user = new PimcoreUser(); + $user->setUsername('test'); + + return $this->makeEmpty(AuthenticationResolverInterface::class, [ + 'authenticateSession' => $withUser ? $user : null, + ]); + } } From cdce2862c64c8190d557a0f89f3d4d62d3c6e4c7 Mon Sep 17 00:00:00 2001 From: martineiber Date: Thu, 6 Jun 2024 04:53:17 +0000 Subject: [PATCH 10/28] Apply php-cs-fixer changes --- src/Authorization/Controller/LogoutController.php | 2 +- tests/Unit/Service/Security/SecurityServiceTest.php | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Authorization/Controller/LogoutController.php b/src/Authorization/Controller/LogoutController.php index e945e6978..36bc7b377 100644 --- a/src/Authorization/Controller/LogoutController.php +++ b/src/Authorization/Controller/LogoutController.php @@ -16,11 +16,11 @@ namespace Pimcore\Bundle\StudioBackendBundle\Authorization\Controller; -use RuntimeException; use OpenApi\Attributes\Post; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\SuccessResponse; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Config\Tags; +use RuntimeException; use Symfony\Component\Routing\Attribute\Route; /** diff --git a/tests/Unit/Service/Security/SecurityServiceTest.php b/tests/Unit/Service/Security/SecurityServiceTest.php index 5e0b91e1a..1416411c1 100644 --- a/tests/Unit/Service/Security/SecurityServiceTest.php +++ b/tests/Unit/Service/Security/SecurityServiceTest.php @@ -22,7 +22,6 @@ use Pimcore\Bundle\StaticResolverBundle\Lib\Tools\Authentication\AuthenticationResolverInterface; use Pimcore\Bundle\StaticResolverBundle\Models\Tool\TmpStoreResolverInterface; use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\Credentials; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Service\TokenServiceInterface; use Pimcore\Bundle\StudioBackendBundle\Exception\AccessDeniedException; use Pimcore\Bundle\StudioBackendBundle\Security\Service\SecurityService; use Pimcore\Bundle\StudioBackendBundle\Security\Service\SecurityServiceInterface; From c4f0752c44caf207f25abac3dc43b86fad7987de Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 06:58:03 +0200 Subject: [PATCH 11/28] Add installation Guide for firewall to docs. --- doc/01_Installation.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/doc/01_Installation.md b/doc/01_Installation.md index f6ee5a77d..f0f2b15a7 100644 --- a/doc/01_Installation.md +++ b/doc/01_Installation.md @@ -37,4 +37,18 @@ bin/console pimcore:bundle:install PimcoreStudioBackendBundle ## Setting up generic data index Pimcore Studio Backend also requires the installation and setup of the generic data index. The bundle is required by default and also automatically enabled in the bundles. -To install the generic data index refer to [Generic-Data-Index](https://github.com/pimcore/generic-data-index-bundle?tab=readme-ov-file) \ No newline at end of file +To install the generic data index refer to [Generic-Data-Index](https://github.com/pimcore/generic-data-index-bundle?tab=readme-ov-file) + +## Enable Firewall settings + +To enable the firewall settings, add the following configuration to your `config/packages/security.yaml` file: + +```yaml +security: + firewalls: + pimcore_studio: '%pimcore_studio_backend.firewall_settings%' + access_control: + - { path: ^/studio/api/docs$, roles: PUBLIC_ACCESS } + - { path: ^/studio/api/docs.json$, roles: PUBLIC_ACCESS } + - { path: ^/studio, roles: ROLE_PIMCORE_USER } +``` From 005199327b25a3bd26f7ee7e70a0ec8017eb3199 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 07:02:55 +0200 Subject: [PATCH 12/28] Fix Exception Handling. --- .../Controller/LogoutController.php | 4 +-- src/Exception/UnreachableException.php | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 src/Exception/UnreachableException.php diff --git a/src/Authorization/Controller/LogoutController.php b/src/Authorization/Controller/LogoutController.php index 36bc7b377..4fe14f12d 100644 --- a/src/Authorization/Controller/LogoutController.php +++ b/src/Authorization/Controller/LogoutController.php @@ -16,11 +16,11 @@ namespace Pimcore\Bundle\StudioBackendBundle\Authorization\Controller; +use Pimcore\Bundle\StudioBackendBundle\Exception\UnreachableException; use OpenApi\Attributes\Post; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\SuccessResponse; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Config\Tags; -use RuntimeException; use Symfony\Component\Routing\Attribute\Route; /** @@ -40,6 +40,6 @@ final class LogoutController extends AbstractApiController )] public function logout(): void { - throw new RuntimeException('Should not be called. Handled by symfony.'); + throw new UnreachableException('Should not be called. Handled by symfony.'); } } diff --git a/src/Exception/UnreachableException.php b/src/Exception/UnreachableException.php new file mode 100644 index 000000000..15edd6f27 --- /dev/null +++ b/src/Exception/UnreachableException.php @@ -0,0 +1,28 @@ + Date: Thu, 6 Jun 2024 05:05:20 +0000 Subject: [PATCH 13/28] Apply php-cs-fixer changes --- src/Authorization/Controller/LogoutController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Authorization/Controller/LogoutController.php b/src/Authorization/Controller/LogoutController.php index 4fe14f12d..83de04bdf 100644 --- a/src/Authorization/Controller/LogoutController.php +++ b/src/Authorization/Controller/LogoutController.php @@ -16,9 +16,9 @@ namespace Pimcore\Bundle\StudioBackendBundle\Authorization\Controller; -use Pimcore\Bundle\StudioBackendBundle\Exception\UnreachableException; use OpenApi\Attributes\Post; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; +use Pimcore\Bundle\StudioBackendBundle\Exception\UnreachableException; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\SuccessResponse; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Config\Tags; use Symfony\Component\Routing\Attribute\Route; From befb37877febcdc276bec0b3729096ffa72fbf3a Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 08:24:45 +0200 Subject: [PATCH 14/28] Use named Routing. --- config/pimcore/firewall.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/pimcore/firewall.yaml b/config/pimcore/firewall.yaml index bf04ade1b..b117c8644 100644 --- a/config/pimcore/firewall.yaml +++ b/config/pimcore/firewall.yaml @@ -9,6 +9,6 @@ pimcore_studio_backend: max_attempts: 3 interval: '5 minutes' logout: - path: /studio/api/logout + path: pimcore_studio_api_logout json_login: - check_path: /studio/api/login \ No newline at end of file + check_path: pimcore_studio_api_login \ No newline at end of file From 172e4b5ca5f9fbb3c750326e18c464504d32f29f Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 08:26:48 +0200 Subject: [PATCH 15/28] Rename to EventSubscriber --- config/authorization.yaml | 2 +- .../LogoutSubscriber.php} | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename src/Authorization/{EventListener/LogoutListener.php => EventSubscriber/LogoutSubscriber.php} (92%) diff --git a/config/authorization.yaml b/config/authorization.yaml index 044847027..a153afe83 100644 --- a/config/authorization.yaml +++ b/config/authorization.yaml @@ -13,6 +13,6 @@ services: tags: [ 'controller.service_arguments' ] - Pimcore\Bundle\StudioBackendBundle\Authorization\EventListener\LogoutListener: + Pimcore\Bundle\StudioBackendBundle\Authorization\EventSubscriber\LogoutSubscriber: tags: - { name: 'kernel.event_subscriber', dispatcher: 'security.event_dispatcher.pimcore_studio' } \ No newline at end of file diff --git a/src/Authorization/EventListener/LogoutListener.php b/src/Authorization/EventSubscriber/LogoutSubscriber.php similarity index 92% rename from src/Authorization/EventListener/LogoutListener.php rename to src/Authorization/EventSubscriber/LogoutSubscriber.php index 666592812..bb7ea38eb 100644 --- a/src/Authorization/EventListener/LogoutListener.php +++ b/src/Authorization/EventSubscriber/LogoutSubscriber.php @@ -14,7 +14,7 @@ * @license http://www.pimcore.org/license GPLv3 and PCL */ -namespace Pimcore\Bundle\StudioBackendBundle\Authorization\EventListener; +namespace Pimcore\Bundle\StudioBackendBundle\Authorization\EventSubscriber; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\JsonResponse; @@ -23,7 +23,7 @@ /** * @internal */ -final class LogoutListener implements EventSubscriberInterface +final class LogoutSubscriber implements EventSubscriberInterface { public static function getSubscribedEvents(): array { From a0f7a9497c6395398af59e4643e43f4158dd3e23 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 08:57:13 +0200 Subject: [PATCH 16/28] Switch to empty response on logout. --- src/Authorization/EventSubscriber/LogoutSubscriber.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Authorization/EventSubscriber/LogoutSubscriber.php b/src/Authorization/EventSubscriber/LogoutSubscriber.php index bb7ea38eb..31375522e 100644 --- a/src/Authorization/EventSubscriber/LogoutSubscriber.php +++ b/src/Authorization/EventSubscriber/LogoutSubscriber.php @@ -16,8 +16,9 @@ namespace Pimcore\Bundle\StudioBackendBundle\Authorization\EventSubscriber; +use Pimcore\Bundle\StudioBackendBundle\Util\Constants\HttpResponseCodes; use Symfony\Component\EventDispatcher\EventSubscriberInterface; -use Symfony\Component\HttpFoundation\JsonResponse; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Security\Http\Event\LogoutEvent; /** @@ -34,6 +35,6 @@ public static function getSubscribedEvents(): array public function onLogout(LogoutEvent $event): void { - $event->setResponse(new JsonResponse([])); + $event->setResponse(new Response(null, HttpResponseCodes::SUCCESS->value)); } } From 54d323e78d7e1d5e8cb3240be6462f8a98f2384f Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 09:01:27 +0200 Subject: [PATCH 17/28] Add DefaultResponses to login. --- src/Authorization/Controller/LoginController.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Authorization/Controller/LoginController.php b/src/Authorization/Controller/LoginController.php index 87701efd1..b84f2c2f2 100644 --- a/src/Authorization/Controller/LoginController.php +++ b/src/Authorization/Controller/LoginController.php @@ -22,6 +22,7 @@ use Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Response\InvalidCredentials; use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\LoginSuccess; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; +use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\DefaultResponses; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\SuccessResponse; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Config\Tags; use Pimcore\Security\User\User; @@ -47,6 +48,7 @@ final class LoginController extends AbstractApiController content: new JsonContent(ref: LoginSuccess::class) )] #[InvalidCredentials] + #[DefaultResponses] public function login(#[CurrentUser] User $user): JsonResponse { return $this->jsonResponse([ From c4494f96f8adbf43d5372eceda0ba4bc8eec60ad Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 09:02:27 +0200 Subject: [PATCH 18/28] Add throws block. --- src/DependencyInjection/PimcoreStudioBackendExtension.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/DependencyInjection/PimcoreStudioBackendExtension.php b/src/DependencyInjection/PimcoreStudioBackendExtension.php index ee2a7790f..e2a62ef8d 100644 --- a/src/DependencyInjection/PimcoreStudioBackendExtension.php +++ b/src/DependencyInjection/PimcoreStudioBackendExtension.php @@ -90,6 +90,9 @@ public function prepend(ContainerBuilder $container): void } } + /** + * @throws InvalidPathException + */ private function checkValidOpenApiScanPaths(array $config): void { foreach ($config as $path) { From 95420e38bc191ec0c57f10f7998b00646012fab2 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 09:30:17 +0200 Subject: [PATCH 19/28] Update Login Schema. --- ...als.php => InvalidCredentialsResponse.php} | 5 ++- .../Controller/LoginController.php | 4 +- .../Schema/InvalidCredentials.php | 42 +++++++++++++++++++ src/Authorization/Schema/LoginSuccess.php | 4 +- 4 files changed, 50 insertions(+), 5 deletions(-) rename src/Authorization/Attributes/Response/{InvalidCredentials.php => InvalidCredentialsResponse.php} (77%) create mode 100644 src/Authorization/Schema/InvalidCredentials.php diff --git a/src/Authorization/Attributes/Response/InvalidCredentials.php b/src/Authorization/Attributes/Response/InvalidCredentialsResponse.php similarity index 77% rename from src/Authorization/Attributes/Response/InvalidCredentials.php rename to src/Authorization/Attributes/Response/InvalidCredentialsResponse.php index 88dc3fd7c..e725a9e52 100644 --- a/src/Authorization/Attributes/Response/InvalidCredentials.php +++ b/src/Authorization/Attributes/Response/InvalidCredentialsResponse.php @@ -17,19 +17,22 @@ namespace Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Response; use Attribute; +use OpenApi\Attributes\JsonContent; use OpenApi\Attributes\Response; +use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\InvalidCredentials; /** * @internal */ #[Attribute(Attribute::TARGET_METHOD)] -final class InvalidCredentials extends Response +final class InvalidCredentialsResponse extends Response { public function __construct() { parent::__construct( response: 401, description: 'Invalid credentials Response', + content: new JsonContent(ref: InvalidCredentials::class) ); } } diff --git a/src/Authorization/Controller/LoginController.php b/src/Authorization/Controller/LoginController.php index b84f2c2f2..af55e13d5 100644 --- a/src/Authorization/Controller/LoginController.php +++ b/src/Authorization/Controller/LoginController.php @@ -19,7 +19,7 @@ use OpenApi\Attributes\JsonContent; use OpenApi\Attributes\Post; use Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Request\CredentialsRequestBody; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Response\InvalidCredentials; +use Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Response\InvalidCredentialsResponse; use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\LoginSuccess; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\DefaultResponses; @@ -47,7 +47,7 @@ final class LoginController extends AbstractApiController description: 'Login successful', content: new JsonContent(ref: LoginSuccess::class) )] - #[InvalidCredentials] + #[InvalidCredentialsResponse] #[DefaultResponses] public function login(#[CurrentUser] User $user): JsonResponse { diff --git a/src/Authorization/Schema/InvalidCredentials.php b/src/Authorization/Schema/InvalidCredentials.php new file mode 100644 index 000000000..a23fb3525 --- /dev/null +++ b/src/Authorization/Schema/InvalidCredentials.php @@ -0,0 +1,42 @@ +error; + } +} diff --git a/src/Authorization/Schema/LoginSuccess.php b/src/Authorization/Schema/LoginSuccess.php index 7dbbde31d..4da5dabd5 100644 --- a/src/Authorization/Schema/LoginSuccess.php +++ b/src/Authorization/Schema/LoginSuccess.php @@ -24,8 +24,8 @@ * @internal */ #[Schema( - title: 'Token', - description: 'Token Scheme for API', + title: 'Login Success', + description: 'Login Success Response Schema for Pimcore Admin', type: 'object' )] final readonly class LoginSuccess From 93f36e613efaa6936dd06c249cfccc965a3445d8 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 11:02:35 +0200 Subject: [PATCH 20/28] Refactor SecurityService. --- src/Security/Service/SecurityService.php | 31 ----- .../Service/SecurityServiceInterface.php | 13 +- .../Voter/PublicAuthorizationVoter.php | 10 -- .../Service/Security/SecurityServiceTest.php | 125 +----------------- 4 files changed, 4 insertions(+), 175 deletions(-) diff --git a/src/Security/Service/SecurityService.php b/src/Security/Service/SecurityService.php index f7a36582b..1fc29db10 100644 --- a/src/Security/Service/SecurityService.php +++ b/src/Security/Service/SecurityService.php @@ -36,41 +36,10 @@ { public function __construct( private ElementPermissionServiceInterface $elementPermissionService, - private UserProvider $userProvider, - private UserPasswordHasherInterface $passwordHasher, - private TmpStoreResolverInterface $tmpStoreResolver, private AuthenticationResolverInterface $authenticationResolver, ) { } - /** - * @throws AccessDeniedException - */ - public function authenticateUser(Credentials $credentials): PasswordAuthenticatedUserInterface - { - try { - $user = $this->userProvider->loadUserByIdentifier($credentials->getUsername()); - } catch (UserNotFoundException) { - throw new AccessDeniedException(); - } - - if( - !$user instanceof PasswordAuthenticatedUserInterface || - !$this->passwordHasher->isPasswordValid($user, $credentials->getPassword()) - ) { - throw new AccessDeniedException(); - } - - return $user; - } - - public function checkAuthToken(string $token): bool - { - $entry = $this->tmpStoreResolver->get($token); - - return $entry !== null && $entry->getId() === $token; - } - /** * @throws UserNotFoundException */ diff --git a/src/Security/Service/SecurityServiceInterface.php b/src/Security/Service/SecurityServiceInterface.php index 29c4df7ea..14b266a97 100644 --- a/src/Security/Service/SecurityServiceInterface.php +++ b/src/Security/Service/SecurityServiceInterface.php @@ -16,12 +16,10 @@ namespace Pimcore\Bundle\StudioBackendBundle\Security\Service; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\Credentials; use Pimcore\Bundle\StudioBackendBundle\Exception\AccessDeniedException; -use Pimcore\Bundle\StudioBackendBundle\Exception\NotAuthorizedException; use Pimcore\Model\Element\ElementInterface; use Pimcore\Model\UserInterface; -use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; +use Symfony\Component\Security\Core\Exception\UserNotFoundException; /** * @internal @@ -29,14 +27,7 @@ interface SecurityServiceInterface { /** - * @throws AccessDeniedException - */ - public function authenticateUser(Credentials $credentials): PasswordAuthenticatedUserInterface; - - public function checkAuthToken(string $token): bool; - - /** - * @throws NotAuthorizedException + * @throws UserNotFoundException */ public function getCurrentUser(): UserInterface; diff --git a/src/Security/Voter/PublicAuthorizationVoter.php b/src/Security/Voter/PublicAuthorizationVoter.php index b8f9f24c3..456387d22 100644 --- a/src/Security/Voter/PublicAuthorizationVoter.php +++ b/src/Security/Voter/PublicAuthorizationVoter.php @@ -60,16 +60,6 @@ protected function voteOnAttribute(string $attribute, mixed $subject, TokenInter $request = $this->getCurrentRequest($this->requestStack); $subjectName = $this->getSubjectName($subject); - try { - $authToken = $this->getAuthToken($request); - } catch (NotAuthorizedException) { - return $this->voteOnRequest($request, $subjectName); - } - - if ($this->securityService->checkAuthToken($authToken)) { - return true; - } - return $this->voteOnRequest($request, $subjectName); } diff --git a/tests/Unit/Service/Security/SecurityServiceTest.php b/tests/Unit/Service/Security/SecurityServiceTest.php index 1416411c1..4407e03b0 100644 --- a/tests/Unit/Service/Security/SecurityServiceTest.php +++ b/tests/Unit/Service/Security/SecurityServiceTest.php @@ -20,83 +20,22 @@ use Exception; use Pimcore\Bundle\GenericDataIndexBundle\Service\Permission\ElementPermissionServiceInterface; use Pimcore\Bundle\StaticResolverBundle\Lib\Tools\Authentication\AuthenticationResolverInterface; -use Pimcore\Bundle\StaticResolverBundle\Models\Tool\TmpStoreResolverInterface; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\Credentials; use Pimcore\Bundle\StudioBackendBundle\Exception\AccessDeniedException; use Pimcore\Bundle\StudioBackendBundle\Security\Service\SecurityService; use Pimcore\Bundle\StudioBackendBundle\Security\Service\SecurityServiceInterface; use Pimcore\Model\Asset; -use Pimcore\Model\Tool\TmpStore; use Pimcore\Model\User as PimcoreUser; -use Pimcore\Security\User\User; -use Pimcore\Security\User\UserProvider; -use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface; use Symfony\Component\Security\Core\Exception\UserNotFoundException; final class SecurityServiceTest extends Unit { - /** - * @throws Exception - */ - public function testSecurityService(): void - { - $securityService = $this->mockSecurityService(); - $user = $securityService->authenticateUser(new Credentials('test', 'test')); - - $this->assertInstanceOf(User::class, $user); - $this->assertSame('test', $user->getPassword()); - } - - /** - * @throws Exception - */ - public function testInvalidPassword(): void - { - $securityService = $this->mockSecurityService(false); - - $this->expectException(AccessDeniedException::class); - $this->expectExceptionMessage('Bad credentials'); - $securityService->authenticateUser(new Credentials('test', 'test')); - } - - /** - * @throws Exception - */ - public function testUserNotFound(): void - { - $securityService = $this->mockSecurityService(false, false); - - $this->expectException(AccessDeniedException::class); - $this->expectExceptionMessage('Bad credentials'); - $securityService->authenticateUser(new Credentials('test', 'test')); - } - - /** - * @throws Exception - */ - public function testTokenAllowedTrue(): void - { - $securityService = $this->mockSecurityService(false, false); - - $this->assertTrue($securityService->checkAuthToken('test')); - } - - /** - * @throws Exception - */ - public function testTokenAllowedFalse(): void - { - $securityService = $this->mockSecurityService(false, false, false); - - $this->assertFalse($securityService->checkAuthToken('test')); - } /** * @throws Exception */ public function testGetCurrentUserWithOutValidUser(): void { - $securityService = $this->mockSecurityService(false, false, false); + $securityService = $this->mockSecurityService(false, false); $this->expectException(UserNotFoundException::class); $securityService->getCurrentUser(); @@ -107,7 +46,7 @@ public function testGetCurrentUserWithOutValidUser(): void */ public function testGetCurrentUserWithValidUser(): void { - $securityService = $this->mockSecurityService(false, true, false); + $securityService = $this->mockSecurityService(true, false); $user = $securityService->getCurrentUser(); @@ -121,8 +60,6 @@ public function testGetCurrentUserWithValidUser(): void public function testHasElementPermission(): void { $securityService = $this->mockSecurityService( - true, - true, true, false ); @@ -140,73 +77,15 @@ public function testHasElementPermission(): void * @throws Exception */ private function mockSecurityService( - $validPassword = true, bool $withUser = true, - bool $withTmpStore = true, bool $hasPermission = true, ): SecurityServiceInterface { return new SecurityService( $this->mockElementPermissionService($hasPermission), - $withUser ? $this->mockUserProviderWithUser() : $this->mockUserProviderWithOutUser(), - $this->mockPasswordHasher($validPassword), - $this->mockTmpStoreResolver($withTmpStore), $this->mockAuthenticationResolver($withUser) ); } - /** - * @throws Exception - */ - private function mockUserProviderWithUser(): UserProvider - { - return $this->makeEmpty(UserProvider::class, [ - 'loadUserByIdentifier' => function () { - return $this->makeEmpty(User::class, [ - 'getPassword' => 'test', - ]); - }, - ]); - } - - /** - * @throws Exception - */ - private function mockUserProviderWithOutUser(): UserProvider - { - return $this->makeEmpty(UserProvider::class, [ - 'loadUserByIdentifier' => fn () => throw new UserNotFoundException('User not found'), - ]); - } - - /** - * @throws Exception - */ - private function mockPasswordHasher($validPassword = true): UserPasswordHasherInterface - { - return $this->makeEmpty(UserPasswordHasherInterface::class, [ - 'isPasswordValid' => $validPassword, - ]); - } - - /** - * @throws Exception - */ - private function mockTmpStoreResolver($withTmpStore = true): TmpStoreResolverInterface - { - return $this->makeEmpty(TmpStoreResolverInterface::class, [ - 'get' => $withTmpStore ? $this->mockTmpStore() : null, - ]); - } - - private function mockTmpStore(): TmpStore - { - $tmpStore = new TmpStore(); - $tmpStore->setId('test'); - $tmpStore->setData(['username' => 'test']); - - return $tmpStore; - } - private function mockElementPermissionService(bool $hasPermission): ElementPermissionServiceInterface { return $this->makeEmpty(ElementPermissionServiceInterface::class, [ From ac536beb2d0b85464a438938cfac83b011a270f0 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 11:07:26 +0200 Subject: [PATCH 21/28] Add required to schema. Small clean up. --- src/Authorization/Schema/Credentials.php | 1 + .../Schema/InvalidCredentials.php | 1 + src/Authorization/Schema/LoginSuccess.php | 1 + src/Authorization/Schema/Refresh.php | 42 ------------------- tests/Unit/Dto/Token/CreateTest.php | 30 ------------- tests/Unit/Dto/Token/InfoTest.php | 30 ------------- tests/Unit/Dto/Token/RefreshTest.php | 29 ------------- 7 files changed, 3 insertions(+), 131 deletions(-) delete mode 100644 src/Authorization/Schema/Refresh.php delete mode 100644 tests/Unit/Dto/Token/CreateTest.php delete mode 100644 tests/Unit/Dto/Token/InfoTest.php delete mode 100644 tests/Unit/Dto/Token/RefreshTest.php diff --git a/src/Authorization/Schema/Credentials.php b/src/Authorization/Schema/Credentials.php index e97b25ba0..5f76bc075 100644 --- a/src/Authorization/Schema/Credentials.php +++ b/src/Authorization/Schema/Credentials.php @@ -25,6 +25,7 @@ #[Schema( title: 'Credentials', description: 'Credentials for authentication', + required: ['username', 'password'], type: 'object' )] final readonly class Credentials diff --git a/src/Authorization/Schema/InvalidCredentials.php b/src/Authorization/Schema/InvalidCredentials.php index a23fb3525..6835766d0 100644 --- a/src/Authorization/Schema/InvalidCredentials.php +++ b/src/Authorization/Schema/InvalidCredentials.php @@ -26,6 +26,7 @@ #[Schema( title: 'Invalid Credentials', description: 'Invalid credentials after login attempt', + required: ['error'], type: 'object' )] final readonly class InvalidCredentials diff --git a/src/Authorization/Schema/LoginSuccess.php b/src/Authorization/Schema/LoginSuccess.php index 4da5dabd5..09fbf83ed 100644 --- a/src/Authorization/Schema/LoginSuccess.php +++ b/src/Authorization/Schema/LoginSuccess.php @@ -26,6 +26,7 @@ #[Schema( title: 'Login Success', description: 'Login Success Response Schema for Pimcore Admin', + required: ['username', 'roles'], type: 'object' )] final readonly class LoginSuccess diff --git a/src/Authorization/Schema/Refresh.php b/src/Authorization/Schema/Refresh.php deleted file mode 100644 index 88d3bc1e5..000000000 --- a/src/Authorization/Schema/Refresh.php +++ /dev/null @@ -1,42 +0,0 @@ -token; - } -} diff --git a/tests/Unit/Dto/Token/CreateTest.php b/tests/Unit/Dto/Token/CreateTest.php deleted file mode 100644 index 53c0e211c..000000000 --- a/tests/Unit/Dto/Token/CreateTest.php +++ /dev/null @@ -1,30 +0,0 @@ -assertSame('token', $create->getUsername()); - $this->assertSame('test', $create->getPassword()); - } -} diff --git a/tests/Unit/Dto/Token/InfoTest.php b/tests/Unit/Dto/Token/InfoTest.php deleted file mode 100644 index 2988f8897..000000000 --- a/tests/Unit/Dto/Token/InfoTest.php +++ /dev/null @@ -1,30 +0,0 @@ -assertSame('token', $info->getToken()); - $this->assertSame('test', $info->getUsername()); - } -} diff --git a/tests/Unit/Dto/Token/RefreshTest.php b/tests/Unit/Dto/Token/RefreshTest.php deleted file mode 100644 index 96510fbbb..000000000 --- a/tests/Unit/Dto/Token/RefreshTest.php +++ /dev/null @@ -1,29 +0,0 @@ -assertSame('token', $refresh->getToken()); - } -} From d26945712d365ec9ea76ac451d173bb21fe4a3c1 Mon Sep 17 00:00:00 2001 From: martineiber Date: Thu, 6 Jun 2024 09:08:15 +0000 Subject: [PATCH 22/28] Apply php-cs-fixer changes --- tests/Unit/Service/Security/SecurityServiceTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Unit/Service/Security/SecurityServiceTest.php b/tests/Unit/Service/Security/SecurityServiceTest.php index 4407e03b0..d9da250a7 100644 --- a/tests/Unit/Service/Security/SecurityServiceTest.php +++ b/tests/Unit/Service/Security/SecurityServiceTest.php @@ -29,7 +29,6 @@ final class SecurityServiceTest extends Unit { - /** * @throws Exception */ From 0dab2ff3b2ea8c8954ecae0f90a596261f482e69 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 11:09:32 +0200 Subject: [PATCH 23/28] Update Readme. --- doc/01_Installation.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/doc/01_Installation.md b/doc/01_Installation.md index f0f2b15a7..2d84b53ec 100644 --- a/doc/01_Installation.md +++ b/doc/01_Installation.md @@ -48,7 +48,6 @@ security: firewalls: pimcore_studio: '%pimcore_studio_backend.firewall_settings%' access_control: - - { path: ^/studio/api/docs$, roles: PUBLIC_ACCESS } - - { path: ^/studio/api/docs.json$, roles: PUBLIC_ACCESS } - - { path: ^/studio, roles: ROLE_PIMCORE_USER } + - { path: ^/studio/api/(docs|docs.json|translations)$, roles: PUBLIC_ACCESS } + - { path: ^/studio, roles: ROLE_PIMCORE_USER } ``` From 2af49c510230bc4d5c7a2292faaedf00cfa4d741 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 6 Jun 2024 11:12:33 +0200 Subject: [PATCH 24/28] Remove unused Property --- src/Security/Voter/PublicAuthorizationVoter.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Security/Voter/PublicAuthorizationVoter.php b/src/Security/Voter/PublicAuthorizationVoter.php index 456387d22..84b9dddc5 100644 --- a/src/Security/Voter/PublicAuthorizationVoter.php +++ b/src/Security/Voter/PublicAuthorizationVoter.php @@ -42,7 +42,6 @@ final class PublicAuthorizationVoter extends Voter public function __construct( private readonly RequestStack $requestStack, - private readonly SecurityServiceInterface $securityService ) { } From 29147e666d4f3dc048c20fea0a7d159934f9c3ed Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Mon, 10 Jun 2024 10:21:47 +0200 Subject: [PATCH 25/28] Add current user request. --- .../Controller/CurrentUserController.php | 55 +++++++++++++++++++ .../Controller/LoginController.php | 4 +- .../{LoginSuccess.php => UserInformation.php} | 2 +- 3 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 src/Authorization/Controller/CurrentUserController.php rename src/Authorization/Schema/{LoginSuccess.php => UserInformation.php} (97%) diff --git a/src/Authorization/Controller/CurrentUserController.php b/src/Authorization/Controller/CurrentUserController.php new file mode 100644 index 000000000..c6b251ca5 --- /dev/null +++ b/src/Authorization/Controller/CurrentUserController.php @@ -0,0 +1,55 @@ +name] + )] + #[SuccessResponse( + description: 'Current user informations.', + content: new JsonContent(ref: UserInformation::class) + )] + #[DefaultResponses] + public function login(#[CurrentUser] User $user): JsonResponse + { + return $this->jsonResponse([ + 'username' => $user->getUserIdentifier(), + 'roles' => $user->getRoles(), + ]); + } +} diff --git a/src/Authorization/Controller/LoginController.php b/src/Authorization/Controller/LoginController.php index af55e13d5..740fac0a3 100644 --- a/src/Authorization/Controller/LoginController.php +++ b/src/Authorization/Controller/LoginController.php @@ -20,7 +20,7 @@ use OpenApi\Attributes\Post; use Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Request\CredentialsRequestBody; use Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Response\InvalidCredentialsResponse; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\LoginSuccess; +use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\UserInformation; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\DefaultResponses; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\SuccessResponse; @@ -45,7 +45,7 @@ final class LoginController extends AbstractApiController #[CredentialsRequestBody] #[SuccessResponse( description: 'Login successful', - content: new JsonContent(ref: LoginSuccess::class) + content: new JsonContent(ref: UserInformation::class) )] #[InvalidCredentialsResponse] #[DefaultResponses] diff --git a/src/Authorization/Schema/LoginSuccess.php b/src/Authorization/Schema/UserInformation.php similarity index 97% rename from src/Authorization/Schema/LoginSuccess.php rename to src/Authorization/Schema/UserInformation.php index 09fbf83ed..c5a6d3972 100644 --- a/src/Authorization/Schema/LoginSuccess.php +++ b/src/Authorization/Schema/UserInformation.php @@ -29,7 +29,7 @@ required: ['username', 'roles'], type: 'object' )] -final readonly class LoginSuccess +final readonly class UserInformation { public function __construct( #[Property(description: 'Username', type: 'string', example: 'admin')] From 3756a7f0e5b8d7cf0c58b39b3a70bab460119abb Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Mon, 10 Jun 2024 10:44:01 +0200 Subject: [PATCH 26/28] Move current user request to user Section. --- .../Controller/LoginController.php | 2 +- .../Schema/UserInformation.php | 6 +++--- .../Controller/CurrentUserController.php | 17 ++++++++++------- 3 files changed, 14 insertions(+), 11 deletions(-) rename src/{Authorization => OpenApi}/Schema/UserInformation.php (87%) rename src/{Authorization => User}/Controller/CurrentUserController.php (74%) diff --git a/src/Authorization/Controller/LoginController.php b/src/Authorization/Controller/LoginController.php index 740fac0a3..2d31bcff6 100644 --- a/src/Authorization/Controller/LoginController.php +++ b/src/Authorization/Controller/LoginController.php @@ -20,11 +20,11 @@ use OpenApi\Attributes\Post; use Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Request\CredentialsRequestBody; use Pimcore\Bundle\StudioBackendBundle\Authorization\Attributes\Response\InvalidCredentialsResponse; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\UserInformation; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\DefaultResponses; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\SuccessResponse; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Config\Tags; +use Pimcore\Bundle\StudioBackendBundle\OpenApi\Schema\UserInformation; use Pimcore\Security\User\User; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\Routing\Attribute\Route; diff --git a/src/Authorization/Schema/UserInformation.php b/src/OpenApi/Schema/UserInformation.php similarity index 87% rename from src/Authorization/Schema/UserInformation.php rename to src/OpenApi/Schema/UserInformation.php index c5a6d3972..11ebeea05 100644 --- a/src/Authorization/Schema/UserInformation.php +++ b/src/OpenApi/Schema/UserInformation.php @@ -14,7 +14,7 @@ * @license http://www.pimcore.org/license GPLv3 and PCL */ -namespace Pimcore\Bundle\StudioBackendBundle\Authorization\Schema; +namespace Pimcore\Bundle\StudioBackendBundle\OpenApi\Schema; use OpenApi\Attributes\Items; use OpenApi\Attributes\Property; @@ -24,8 +24,8 @@ * @internal */ #[Schema( - title: 'Login Success', - description: 'Login Success Response Schema for Pimcore Admin', + title: 'User Informations', + description: 'Informations about the user with username and roles', required: ['username', 'roles'], type: 'object' )] diff --git a/src/Authorization/Controller/CurrentUserController.php b/src/User/Controller/CurrentUserController.php similarity index 74% rename from src/Authorization/Controller/CurrentUserController.php rename to src/User/Controller/CurrentUserController.php index c6b251ca5..2afe63732 100644 --- a/src/Authorization/Controller/CurrentUserController.php +++ b/src/User/Controller/CurrentUserController.php @@ -14,15 +14,16 @@ * @license http://www.pimcore.org/license GPLv3 and PCL */ -namespace Pimcore\Bundle\StudioBackendBundle\Authorization\Controller; +namespace Pimcore\Bundle\StudioBackendBundle\User\Controller; use OpenApi\Attributes\Get; use OpenApi\Attributes\JsonContent; -use Pimcore\Bundle\StudioBackendBundle\Authorization\Schema\UserInformation; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\DefaultResponses; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attributes\Response\SuccessResponse; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Config\Tags; +use Pimcore\Bundle\StudioBackendBundle\OpenApi\Schema\UserInformation; +use Pimcore\Bundle\StudioBackendBundle\Util\Constants\HttpResponseCodes; use Pimcore\Security\User\User; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\Routing\Attribute\Route; @@ -33,18 +34,20 @@ */ final class CurrentUserController extends AbstractApiController { - #[Route('/current-user', name: 'pimcore_studio_api_current_user', methods: ['GET'])] + #[Route('/user/current-user-information', name: 'pimcore_studio_api_current_user', methods: ['GET'])] #[Get( - path: self::API_PATH . '/current-user', - operationId: 'current-user', + path: self::API_PATH . '/user/current-user-information', + operationId: 'current-user-information', summary: 'Retrieve informations about the current logged in user.', - tags: [Tags::Authorization->name] + tags: [Tags::User->value] )] #[SuccessResponse( description: 'Current user informations.', content: new JsonContent(ref: UserInformation::class) )] - #[DefaultResponses] + #[DefaultResponses([ + HttpResponseCodes::UNAUTHORIZED + ])] public function login(#[CurrentUser] User $user): JsonResponse { return $this->jsonResponse([ From 5caa6c5c0b1ee00b0c2d6133fd45c63a08361383 Mon Sep 17 00:00:00 2001 From: martineiber Date: Mon, 10 Jun 2024 08:44:26 +0000 Subject: [PATCH 27/28] Apply php-cs-fixer changes --- src/User/Controller/CurrentUserController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/User/Controller/CurrentUserController.php b/src/User/Controller/CurrentUserController.php index 2afe63732..88c1cd75f 100644 --- a/src/User/Controller/CurrentUserController.php +++ b/src/User/Controller/CurrentUserController.php @@ -46,7 +46,7 @@ final class CurrentUserController extends AbstractApiController content: new JsonContent(ref: UserInformation::class) )] #[DefaultResponses([ - HttpResponseCodes::UNAUTHORIZED + HttpResponseCodes::UNAUTHORIZED, ])] public function login(#[CurrentUser] User $user): JsonResponse { From 4e0aa802ef29a6fc650ef641f165e1d814bd2901 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Tue, 11 Jun 2024 11:52:24 +0200 Subject: [PATCH 28/28] Remove added SECURITY_SCHEME --- src/Version/Controller/Asset/DownloadController.php | 1 - src/Version/Controller/Asset/ImageStreamController.php | 1 - 2 files changed, 2 deletions(-) diff --git a/src/Version/Controller/Asset/DownloadController.php b/src/Version/Controller/Asset/DownloadController.php index 1766ea89c..76a1250b6 100644 --- a/src/Version/Controller/Asset/DownloadController.php +++ b/src/Version/Controller/Asset/DownloadController.php @@ -58,7 +58,6 @@ public function __construct( operationId: 'downloadAssetVersionById', description: 'Download asset version based on the version ID', summary: 'Download asset version by ID', - security: self::SECURITY_SCHEME, tags: [Tags::Versions->name] )] #[IdParameter(type: 'version')] diff --git a/src/Version/Controller/Asset/ImageStreamController.php b/src/Version/Controller/Asset/ImageStreamController.php index af2b369e7..2299d7fa5 100644 --- a/src/Version/Controller/Asset/ImageStreamController.php +++ b/src/Version/Controller/Asset/ImageStreamController.php @@ -59,7 +59,6 @@ public function __construct( operationId: 'streamImageVersionById', description: 'Get image version stream based on the version ID', summary: 'Get image version stream by ID', - security: self::SECURITY_SCHEME, tags: [Tags::Versions->name] )] #[IdParameter(type: 'version')]