From 3b3566bb736e2e191986c1d8c9812b63da840766 Mon Sep 17 00:00:00 2001 From: Robin Krahnen Date: Tue, 15 Mar 2022 09:32:42 +0100 Subject: [PATCH 1/7] revised code --- Classes/Annotations/ReadOnly.php | 2 +- Classes/Aspects/ReadOnlyAspect.php | 30 ++++--- Classes/Aspects/ReadOnlyFilter.php | 91 +++++++++++++------- Classes/Http/SessionLockRequestComponent.php | 40 ++++----- Classes/Package.php | 31 ------- Configuration/Development/Settings.yaml | 2 +- Configuration/Objects.yaml | 16 ++-- Configuration/Settings.yaml | 18 ++-- Configuration/Testing/Settings.yaml | 3 + composer.json | 38 ++++---- 10 files changed, 135 insertions(+), 136 deletions(-) delete mode 100644 Classes/Package.php create mode 100644 Configuration/Testing/Settings.yaml diff --git a/Classes/Annotations/ReadOnly.php b/Classes/Annotations/ReadOnly.php index ce2e966..cac72be 100644 --- a/Classes/Annotations/ReadOnly.php +++ b/Classes/Annotations/ReadOnly.php @@ -4,7 +4,7 @@ namespace DigiComp\FlowSessionLock\Annotations; /** * @Annotation - * @Target("METHOD") + * @Target({"METHOD"}) */ final class ReadOnly { diff --git a/Classes/Aspects/ReadOnlyAspect.php b/Classes/Aspects/ReadOnlyAspect.php index f45ab95..355ddf4 100644 --- a/Classes/Aspects/ReadOnlyAspect.php +++ b/Classes/Aspects/ReadOnlyAspect.php @@ -36,37 +36,43 @@ class ReadOnlyAspect /** * @Flow\Around("methodAnnotatedWith(DigiComp\FlowSessionLock\Annotations\ReadOnly) || filter(DigiComp\FlowSessionLock\Aspects\ReadOnlyFilter)") * @param JoinPointInterface $joinPoint - * - * @return void + * @return mixed */ public function demoteLockToReadOnly(JoinPointInterface $joinPoint) { - $handler = $this->bootstrap->getActiveRequestHandler(); - if (! $handler instanceof HttpRequestHandlerInterface) { - $this->logger->debug(\get_class($handler)); + $activeRequestHandler = $this->bootstrap->getActiveRequestHandler(); + if (!$activeRequestHandler instanceof HttpRequestHandlerInterface) { + $this->logger->debug('SessionLock: ' . \get_class($activeRequestHandler)); + return $joinPoint->getAdviceChain()->proceed($joinPoint); } - $componentContext = $handler->getComponentContext(); - /** @var Lock $lock */ - $lock = $componentContext->getParameter(SessionLockRequestComponent::class, 'sessionLock'); + $this->readOnly = true; - if ($lock) { - $this->logger->debug('SessionLock: Release, as this is marked read only'); + + /** @var Lock|null $lock */ + $lock = $activeRequestHandler->getComponentContext()->getParameter( + SessionLockRequestComponent::class, + SessionLockRequestComponent::PARAMETER_NAME + ); + if ($lock !== null) { + $this->logger->debug('SessionLock: Release, as this is marked read only.'); $lock->release(); } + return $joinPoint->getAdviceChain()->proceed($joinPoint); } /** * @Flow\Around("method(Neos\Flow\Session\Session->shutdownObject())") - * * @param JoinPointInterface $joinPoint + * @return mixed|void */ public function doNotSaveSession(JoinPointInterface $joinPoint) { if ($this->readOnly) { return; } - $joinPoint->getAdviceChain()->proceed($joinPoint); + + return $joinPoint->getAdviceChain()->proceed($joinPoint); } } diff --git a/Classes/Aspects/ReadOnlyFilter.php b/Classes/Aspects/ReadOnlyFilter.php index f7d7d04..4925d4c 100644 --- a/Classes/Aspects/ReadOnlyFilter.php +++ b/Classes/Aspects/ReadOnlyFilter.php @@ -4,9 +4,12 @@ namespace DigiComp\FlowSessionLock\Aspects; use Neos\Flow\Annotations as Flow; use Neos\Flow\Aop\Builder\ClassNameIndex; +use Neos\Flow\Aop\Exception as NeosFlowAopException; +use Neos\Flow\Aop\Exception\InvalidPointcutExpressionException; use Neos\Flow\Aop\Pointcut\PointcutFilterComposite; use Neos\Flow\Aop\Pointcut\PointcutFilterInterface; use Neos\Flow\Configuration\ConfigurationManager; +use Neos\Flow\Configuration\Exception\InvalidConfigurationTypeException; use Neos\Flow\Security\Authorization\Privilege\Method\MethodTargetExpressionParser; /** @@ -15,20 +18,32 @@ use Neos\Flow\Security\Authorization\Privilege\Method\MethodTargetExpressionPars */ class ReadOnlyFilter implements PointcutFilterInterface { + /** + * @var ConfigurationManager + */ protected ConfigurationManager $configurationManager; + /** + * @var MethodTargetExpressionParser + */ protected MethodTargetExpressionParser $methodTargetExpressionParser; /** * @var PointcutFilterComposite[] */ - protected ?array $filters = null; + protected ?array $pointcutFilterComposites = null; + /** + * @param ConfigurationManager $configurationManager + */ public function injectConfigurationManager(ConfigurationManager $configurationManager): void { $this->configurationManager = $configurationManager; } + /** + * @param MethodTargetExpressionParser $methodTargetExpressionParser + */ public function injectMethodTargetExpressionParser(MethodTargetExpressionParser $methodTargetExpressionParser): void { $this->methodTargetExpressionParser = $methodTargetExpressionParser; @@ -36,30 +51,28 @@ class ReadOnlyFilter implements PointcutFilterInterface /** * @inheritDoc + * @throws InvalidConfigurationTypeException + * @throws InvalidPointcutExpressionException + * @throws NeosFlowAopException */ public function matches($className, $methodName, $methodDeclaringClassName, $pointcutQueryIdentifier): bool { - if ($this->filters === null) { - $this->buildPointcutFilters(); - } + $this->buildPointcutFilters(); - $matchingFilters = \array_filter( - $this->filters, - function (PointcutFilterInterface $filter) use ( - $className, - $methodName, - $methodDeclaringClassName, - $pointcutQueryIdentifier - ): bool { - return $filter->matches($className, $methodName, $methodDeclaringClassName, $pointcutQueryIdentifier); + foreach ($this->pointcutFilterComposites as $pointcutFilterComposite) { + if ( + $pointcutFilterComposite->matches( + $className, + $methodName, + $methodDeclaringClassName, + $pointcutQueryIdentifier + ) + ) { + return true; } - ); - - if ($matchingFilters === []) { - return false; } - return true; + return false; } /** @@ -80,33 +93,47 @@ class ReadOnlyFilter implements PointcutFilterInterface /** * @inheritDoc + * @throws InvalidConfigurationTypeException + * @throws InvalidPointcutExpressionException + * @throws NeosFlowAopException */ public function reduceTargetClassNames(ClassNameIndex $classNameIndex): ClassNameIndex { - if ($this->filters === null) { - $this->buildPointcutFilters(); - } + $this->buildPointcutFilters(); $result = new ClassNameIndex(); - foreach ($this->filters as $filter) { - $result->applyUnion($filter->reduceTargetClassNames($classNameIndex)); + + foreach ($this->pointcutFilterComposites as $pointcutFilterComposite) { + $result->applyUnion($pointcutFilterComposite->reduceTargetClassNames($classNameIndex)); } + return $result; } + /** + * @throws InvalidConfigurationTypeException + * @throws InvalidPointcutExpressionException + * @throws NeosFlowAopException + */ protected function buildPointcutFilters(): void { - $this->filters = []; - $readOnlyExpressions = $this->configurationManager->getConfiguration( - ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, - 'DigiComp.FlowSessionLock.readOnlyExpressions' - ) ?? []; - foreach ($readOnlyExpressions as $key => $pointcut) { - if ($pointcut === null) { + if ($this->pointcutFilterComposites !== null) { + return; + } + + $this->pointcutFilterComposites = []; + foreach ( + $this->configurationManager->getConfiguration( + ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, + 'DigiComp.FlowSessionLock.readOnlyExpressions' + ) as $key => $pointcutExpression + ) { + if ($pointcutExpression === null) { continue; } - $this->filters[] = $this->methodTargetExpressionParser->parse( - $pointcut, + + $this->pointcutFilterComposites[] = $this->methodTargetExpressionParser->parse( + $pointcutExpression, 'Settings.yaml at "DigiComp.FlowSessionLock.readOnlyExpressions", key: "' . $key . '"' ); } diff --git a/Classes/Http/SessionLockRequestComponent.php b/Classes/Http/SessionLockRequestComponent.php index 964e9e2..9677b46 100644 --- a/Classes/Http/SessionLockRequestComponent.php +++ b/Classes/Http/SessionLockRequestComponent.php @@ -5,25 +5,19 @@ namespace DigiComp\FlowSessionLock\Http; use Neos\Flow\Annotations as Flow; use Neos\Flow\Http\Component\ComponentContext; use Neos\Flow\Http\Component\ComponentInterface; -use Neos\Flow\Utility\Environment; -use Neos\Utility\Files; use Psr\Log\LoggerInterface; use Symfony\Component\Lock\Key; use Symfony\Component\Lock\LockFactory; class SessionLockRequestComponent implements ComponentInterface { - /** - * @Flow\InjectConfiguration(package="Neos.Flow", path="session") - * @var array - */ - protected $sessionSettings; + public const PARAMETER_NAME = 'sessionLock'; /** * @Flow\Inject(lazy=false) * @var LoggerInterface */ - protected $logger; + protected LoggerInterface $logger; /** * @Flow\Inject(name="DigiComp.FlowSessionLock:LockFactory") @@ -31,40 +25,42 @@ class SessionLockRequestComponent implements ComponentInterface */ protected $lockFactory; + /** + * @Flow\InjectConfiguration(package="Neos.Flow", path="session") + * @var array + */ + protected array $sessionSettings; + + /** + * @Flow\InjectConfiguration(package="DigiComp.FlowSessionLock", path="timeToLive") + * @var float + */ + protected float $timeToLive; + /** * @Flow\InjectConfiguration(package="DigiComp.FlowSessionLock", path="autoRelease") * @var bool */ protected bool $autoRelease; - /** - * @Flow\InjectConfiguration(package="DigiComp.FlowSessionLock", path="timeToLive") - * @var int - */ - protected int $timeToLive; - /** * @inheritDoc */ public function handle(ComponentContext $componentContext) { $sessionCookieName = $this->sessionSettings['name']; - $request = $componentContext->getHttpRequest(); - $cookies = $request->getCookieParams(); + $cookies = $componentContext->getHttpRequest()->getCookieParams(); if (!isset($cookies[$sessionCookieName])) { return; } - $sessionIdentifier = $cookies[$sessionCookieName]; - - $key = new Key( - 'session-' . $sessionIdentifier - ); //TODO: sessionIdentifier might be wrong, probably it should probably be storage identifier + // TODO: sessionIdentifier might be wrong, probably it should probably be storage identifier + $key = new Key('session-' . $cookies[$sessionCookieName]); $lock = $this->lockFactory->createLockFromKey($key, $this->timeToLive, $this->autoRelease); - $componentContext->setParameter(SessionLockRequestComponent::class, 'sessionLock', $lock); + $componentContext->setParameter(SessionLockRequestComponent::class, static::PARAMETER_NAME, $lock); $this->logger->debug('SessionLock: Get ' . $key); $lock->acquire(true); diff --git a/Classes/Package.php b/Classes/Package.php deleted file mode 100644 index 66e97be..0000000 --- a/Classes/Package.php +++ /dev/null @@ -1,31 +0,0 @@ -getSignalSlotDispatcher(); - - $dispatcher->connect( - ConfigurationManager::class, - 'configurationManagerReady', - function (ConfigurationManager $configurationManager) { - $lockStoreDir = $configurationManager->getConfiguration( - ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, - 'DigiComp.FlowSessionLock.lockStoreDir' - ); - if (is_string($lockStoreDir)) { - Files::createDirectoryRecursively($lockStoreDir); - } - } - ); - } -} diff --git a/Configuration/Development/Settings.yaml b/Configuration/Development/Settings.yaml index 0760c4d..23b7e23 100644 --- a/Configuration/Development/Settings.yaml +++ b/Configuration/Development/Settings.yaml @@ -1,3 +1,3 @@ DigiComp: FlowSessionLock: - lockStoreDir: '%FLOW_PATH_DATA%Temporary/Development/SessionLocks' + lockStoreConnection: "flock://%FLOW_PATH_DATA%Temporary/Development/SessionLocks/" diff --git a/Configuration/Objects.yaml b/Configuration/Objects.yaml index 6523a12..95f5480 100644 --- a/Configuration/Objects.yaml +++ b/Configuration/Objects.yaml @@ -1,12 +1,10 @@ DigiComp.FlowSessionLock:LockFactory: - className: 'Symfony\Component\Lock\LockFactory' + className: "Symfony\\Component\\Lock\\LockFactory" arguments: 1: - object: 'DigiComp.FlowSessionLock:LockStore' - -DigiComp.FlowSessionLock:LockStore: - className: 'Symfony\Component\Lock\Store\FlockStore' - scope: 'singleton' - arguments: - 1: - setting: 'DigiComp.FlowSessionLock.lockStoreDir' + object: + factoryObjectName: "Symfony\\Component\\Lock\\Store\\StoreFactory" + factoryMethodName: "createStore" + arguments: + 1: + setting: "DigiComp.FlowSessionLock.lockStoreConnection" diff --git a/Configuration/Settings.yaml b/Configuration/Settings.yaml index e49711d..f5b77b2 100644 --- a/Configuration/Settings.yaml +++ b/Configuration/Settings.yaml @@ -1,3 +1,10 @@ +DigiComp: + FlowSessionLock: + lockStoreConnection: "flock://%FLOW_PATH_DATA%Temporary/Production/SessionLocks/" + timeToLive: 300.0 + autoRelease: true + readOnlyExpressions: {} + Neos: Flow: http: @@ -5,12 +12,5 @@ Neos: preprocess: chain: lockSession: - position: 'before getSessionCookieFromRequest' - component: 'DigiComp\FlowSessionLock\Http\SessionLockRequestComponent' - -DigiComp: - FlowSessionLock: - lockStoreDir: '%FLOW_PATH_DATA%Temporary/Production/SessionLocks' - readOnlyExpressions: [] - autoRelease: true - timeToLive: 300 + position: "before getSessionCookieFromRequest" + component: "DigiComp\\FlowSessionLock\\Http\\SessionLockRequestComponent" diff --git a/Configuration/Testing/Settings.yaml b/Configuration/Testing/Settings.yaml new file mode 100644 index 0000000..07b1b05 --- /dev/null +++ b/Configuration/Testing/Settings.yaml @@ -0,0 +1,3 @@ +DigiComp: + FlowSessionLock: + lockStoreConnection: "flock://%FLOW_PATH_DATA%Temporary/Testing/SessionLocks/" diff --git a/composer.json b/composer.json index 811e3c2..e774de9 100644 --- a/composer.json +++ b/composer.json @@ -1,25 +1,11 @@ { "name": "digicomp/flowsessionlock", + "description": "Session locking for Neos Flow - it secures the session becoming corrupted by concurrent access to the same session by different requests", "type": "neos-package", - "description": "Sesion locking for Neos Flow - it secures the session becoming corrupted by concurrent access to the same session by different requests", - "keywords": [ - "flow", - "neos" - ], - "authors": [ - { - "name": "Ferdinand Kuhl", - "email": "f.kuhl@digital-competence.de", - "homepage": "http://www.digital-competence.de", - "role": "Developer" - } - ], - "license": "MIT", - "homepage": "https://github.com/digicomp/DigiComp.FlowSessionLock", "require": { - "neos/flow": "^6.2", - "php": "^7.4", - "symfony/lock": "^5.2" + "neos/flow": "^6.2.0", + "php": ">=7.4", + "symfony/lock": "^5.2.0" }, "autoload": { "psr-4": { @@ -33,5 +19,19 @@ "branch-alias": { "dev-develop": "1.0.x-dev" } - } + }, + "authors": [ + { + "name": "Ferdinand Kuhl", + "email": "f.kuhl@digital-competence.de", + "homepage": "https://www.digital-competence.de", + "role": "Developer" + } + ], + "license": "MIT", + "homepage": "https://github.com/digital-competence/FlowSessionLock", + "keywords": [ + "Neos", + "Flow" + ] } From 7420419663d1729212b43a21a24f95a0288cfe86 Mon Sep 17 00:00:00 2001 From: Robin Krahnen Date: Mon, 4 Apr 2022 22:41:28 +0200 Subject: [PATCH 2/7] change dependency to neos/flow --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index e774de9..98ad265 100644 --- a/composer.json +++ b/composer.json @@ -3,7 +3,7 @@ "description": "Session locking for Neos Flow - it secures the session becoming corrupted by concurrent access to the same session by different requests", "type": "neos-package", "require": { - "neos/flow": "^6.2.0", + "neos/flow": "^6.3.0", "php": ">=7.4", "symfony/lock": "^5.2.0" }, From 737047d856f58f8613831de3fe5cd2566c5592d7 Mon Sep 17 00:00:00 2001 From: Robin Krahnen Date: Tue, 5 Apr 2022 12:31:33 +0200 Subject: [PATCH 3/7] do load lazy if possible --- Classes/Http/SessionLockRequestComponent.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Classes/Http/SessionLockRequestComponent.php b/Classes/Http/SessionLockRequestComponent.php index 9677b46..5b59d42 100644 --- a/Classes/Http/SessionLockRequestComponent.php +++ b/Classes/Http/SessionLockRequestComponent.php @@ -14,10 +14,10 @@ class SessionLockRequestComponent implements ComponentInterface public const PARAMETER_NAME = 'sessionLock'; /** - * @Flow\Inject(lazy=false) + * @Flow\Inject * @var LoggerInterface */ - protected LoggerInterface $logger; + protected $logger; /** * @Flow\Inject(name="DigiComp.FlowSessionLock:LockFactory") From e7ca78f855a4ac70eb5b2d43615181ab34557109 Mon Sep 17 00:00:00 2001 From: Robin Krahnen Date: Tue, 5 Apr 2022 15:19:26 +0200 Subject: [PATCH 4/7] code style --- Classes/Http/SessionLockRequestComponent.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/Http/SessionLockRequestComponent.php b/Classes/Http/SessionLockRequestComponent.php index 5b59d42..691c95b 100644 --- a/Classes/Http/SessionLockRequestComponent.php +++ b/Classes/Http/SessionLockRequestComponent.php @@ -46,7 +46,7 @@ class SessionLockRequestComponent implements ComponentInterface /** * @inheritDoc */ - public function handle(ComponentContext $componentContext) + public function handle(ComponentContext $componentContext): void { $sessionCookieName = $this->sessionSettings['name']; From f784fb3d3f3a97fdc45748367a85d1091860936d Mon Sep 17 00:00:00 2001 From: Robin Krahnen Date: Wed, 20 Apr 2022 17:36:30 +0200 Subject: [PATCH 5/7] revised code --- README.md | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 781bc71..0717b21 100644 --- a/README.md +++ b/README.md @@ -1,13 +1,16 @@ DigiComp.FlowSessionLock ------------------------ -By default the session established by Flow is not "protected" in any way. -This package restricts every request to load the session only, if there are no other requests having it in access currently. -It allows to set custom pointcut which will set the session in "ReadOnly" mode, which allows concurrent requests to read, but disallows the current request to write the session. +By default, the session established by Flow is not "protected" in any way. This package restricts every request to load +the session only, if there are no other requests having it in access currently. It allows to set custom pointcut which +will set the session in "ReadOnly" mode, which allows concurrent requests to read, but disallows the current request to +write the session. -If you want to allow concurrent access somewhere, you can add your trigger pointcut in Settings.yaml like such: - - DigiComp: - FlowSessionLock: - readOnlyExpressions: - 'AcmeLock': 'method(Acme/SuperPackage/Controller/ConcurrentController->concurrentAction())' +If you want to allow concurrent access somewhere, you can add your trigger pointcut in `Settings.yaml` like such: + +```yaml +DigiComp: + FlowSessionLock: + readOnlyExpressions: + MyLock: "method(My\\Package\\Controller\\MyController->myAction())" +``` From f93dfd0df6fcb72be3b5b95402772cf55a318320 Mon Sep 17 00:00:00 2001 From: Robin Krahnen Date: Mon, 2 May 2022 09:56:10 +0200 Subject: [PATCH 6/7] add "declare(strict_types=1);" --- Classes/Annotations/ReadOnly.php | 2 ++ Classes/Aspects/ReadOnlyAspect.php | 2 ++ Classes/Aspects/ReadOnlyFilter.php | 2 ++ Classes/Http/SessionLockRequestComponent.php | 2 ++ 4 files changed, 8 insertions(+) diff --git a/Classes/Annotations/ReadOnly.php b/Classes/Annotations/ReadOnly.php index cac72be..24cc7db 100644 --- a/Classes/Annotations/ReadOnly.php +++ b/Classes/Annotations/ReadOnly.php @@ -1,5 +1,7 @@ Date: Wed, 4 May 2022 19:01:15 +0200 Subject: [PATCH 7/7] optimized versions in composer.json --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 98ad265..567cb4a 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,7 @@ "package-key": "DigiComp.FlowSessionLock" }, "branch-alias": { - "dev-develop": "1.0.x-dev" + "dev-develop": "2.0.x-dev" } }, "authors": [