Merge remote-tracking branch 'composer/develop' into version/2.x-dev

Reviewed-on: #1

Classes/Annotations/ReadOnly.php

@@ -1,10 +1,12 @@
 <?php
 
+declare(strict_types=1);
+
 namespace DigiComp\FlowSessionLock\Annotations;
 
 /**
  * @Annotation
- * @Target("METHOD")
+ * @Target({"METHOD"})
  */
 final class ReadOnly
 {

Classes/Aspects/ReadOnlyAspect.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace DigiComp\FlowSessionLock\Aspects;
 
 use DigiComp\FlowSessionLock\Http\SessionLockRequestComponent;
@@ -36,37 +38,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);
     }
 }

Classes/Aspects/ReadOnlyFilter.php

@@ -1,12 +1,17 @@
 <?php
 
+declare(strict_types=1);
+
 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 +20,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 +53,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 +95,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 . '"'
             );
         }

Classes/Http/SessionLockRequestComponent.php

@@ -1,26 +1,23 @@
 <?php
 
+declare(strict_types=1);
+
 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\Exception\LockAcquiringException;
 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)
+     * @Flow\Inject
      * @var LoggerInterface
      */
     protected $logger;
@@ -31,6 +28,18 @@ 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
@@ -38,36 +47,41 @@ class SessionLockRequestComponent implements ComponentInterface
     protected bool $autoRelease;
 
     /**
-     * @Flow\InjectConfiguration(package="DigiComp.FlowSessionLock", path="timeToLive")
+     * @Flow\InjectConfiguration(package="DigiComp.FlowSessionLock", path="secondsToWait")
      * @var int
      */
-    protected int $timeToLive;
+    protected int $secondsToWait;
 
     /**
      * @inheritDoc
      */
-    public function handle(ComponentContext $componentContext)
+    public function handle(ComponentContext $componentContext): void
     {
         $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);
-        $this->logger->debug('SessionLock: Acquired ' . $key);
+        $this->logger->debug('SessionLock: Try to get "' . $key . '"');
+        $timedOut = \time() + $this->secondsToWait;
+        while (!$lock->acquire()) {
+            if (\time() >= $timedOut) {
+                throw new LockAcquiringException(
+                    'Could not acquire the lock for "' . $key . '" in ' . $this->secondsToWait . ' seconds.',
+                    1652687960
+                );
+            }
+            \usleep(100000);
+        }
+        $this->logger->debug('SessionLock: Acquired "' . $key . '"');
     }
 }

Classes/Package.php

@@ -1,31 +0,0 @@
-<?php
-
-namespace DigiComp\FlowSessionLock;
-
-use Neos\Flow\Configuration\ConfigurationManager;
-use Neos\Flow\Core\Bootstrap;
-use Neos\Flow\Package\Package as BasePackage;
-use Neos\Utility\Files;
-
-class Package extends BasePackage
-{
-    public function boot(Bootstrap $bootstrap)
-    {
-        parent::boot($bootstrap);
-        $dispatcher = $bootstrap->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);
-                }
-            }
-        );
-    }
-}

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/"

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"

Configuration/Settings.yaml

@@ -1,3 +1,11 @@
+DigiComp:
+  FlowSessionLock:
+    lockStoreConnection: "flock://%FLOW_PATH_DATA%Temporary/Production/SessionLocks/"
+    timeToLive: 300.0
+    autoRelease: true
+    secondsToWait: 30
+    readOnlyExpressions: {}
+
 Neos:
   Flow:
     http:
@@ -5,12 +13,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"

Configuration/Testing/Settings.yaml

@@ -0,0 +1,5 @@
+DigiComp:
+  FlowSessionLock:
+    lockStoreConnection: "flock://%FLOW_PATH_DATA%Temporary/Testing/SessionLocks/"
+    readOnlyExpressions:
+      TestUnprotected: "method(DigiComp\\FlowSessionLock\\Tests\\Functional\\Fixtures\\Controller\\ExampleController->unprotectedByConfigurationAction())"

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())"
+```

Tests/Functional/Fixtures/Controller/ExampleController.php

@@ -0,0 +1,43 @@
+<?php
+
+namespace DigiComp\FlowSessionLock\Tests\Functional\Fixtures\Controller;
+
+use DigiComp\FlowSessionLock\Annotations as FlowSessionLock;
+use Neos\Flow\Annotations as Flow;
+use Neos\Flow\Mvc\Controller\ActionController;
+
+class ExampleController extends ActionController
+{
+    public const CONTROLLER_TIME = 200;
+
+    /**
+     * @Flow\Session(autoStart=true);
+     * @return string
+     */
+    public function protectedAction()
+    {
+        \usleep(static::CONTROLLER_TIME * 1000);
+        return 'Hello World!';
+    }
+
+    /**
+     * @Flow\Session(autoStart=true);
+     * @FlowSessionLock\ReadOnly
+     * @return string
+     */
+    public function unprotectedByAnnotationAction()
+    {
+        \usleep(static::CONTROLLER_TIME * 1000);
+        return 'Hello World!';
+    }
+
+    /**
+     * @Flow\Session(autoStart=true);
+     * @return string
+     */
+    public function unprotectedByConfigurationAction()
+    {
+        \usleep(static::CONTROLLER_TIME * 1000);
+        return 'Hello World!';
+    }
+}

Tests/Functional/SessionLockRequestComponentTest.php

@@ -0,0 +1,100 @@
+<?php
+
+namespace DigiComp\FlowSessionLock\Tests\Functional;
+
+use DigiComp\FlowSessionLock\Tests\Functional\Fixtures\Controller\ExampleController;
+use GuzzleHttp\Psr7\Uri;
+use Neos\Flow\Http\Cookie;
+use Neos\Flow\Mvc\Routing\Route;
+use Neos\Flow\Tests\FunctionalTestCase;
+use Psr\Http\Message\ServerRequestFactoryInterface;
+
+class SessionLockRequestComponentTest extends FunctionalTestCase
+{
+    protected ServerRequestFactoryInterface $serverRequestFactory;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->serverRequestFactory = $this->objectManager->get(ServerRequestFactoryInterface::class);
+        $route = new Route();
+        $route->setName('Functional Test - SessionRequestComponent::Restricted');
+        $route->setUriPattern('test/sessionlock/{@action}');
+        $route->setDefaults([
+            '@package' => 'DigiComp.FlowSessionLock',
+            '@subpackage' => 'Tests\Functional\Fixtures',
+            '@controller' => 'Example',
+            '@action' => 'protected',
+            '@format' => 'html',
+        ]);
+        $route->setAppendExceedingArguments(true);
+        $this->router->addRoute($route);
+    }
+
+    public function expectedDuration(): array
+    {
+        $parallelChecker = function ($allRequests, $oneRequest) {
+            self::assertGreaterThan(ExampleController::CONTROLLER_TIME, $oneRequest * 1000);
+            self::assertLessThan(ExampleController::CONTROLLER_TIME * 4, $allRequests * 1000);
+        };
+        return [
+            [
+                'http://localhost/test/sessionlock/protected',
+                function ($allRequests, $oneRequest) {
+                    self::assertGreaterThan(ExampleController::CONTROLLER_TIME, $oneRequest * 1000);
+                    self::assertGreaterThan(ExampleController::CONTROLLER_TIME * 4, $allRequests * 1000);
+                }
+            ],
+            [
+                'http://localhost/test/sessionlock/unprotectedbyannotation',
+                $parallelChecker
+            ],
+            [
+                'http://localhost/test/sessionlock/unprotectedbyconfiguration',
+                $parallelChecker
+            ]
+        ];
+    }
+
+    /**
+     * @dataProvider expectedDuration
+     * @test
+     */
+    public function itDoesNotAllowToEnterMoreThanOneWithTheSameSession(string $url, \Closure $checker): void
+    {
+        $request = $this->serverRequestFactory
+            ->createServerRequest('GET', new Uri($url));
+        $start = microtime(true);
+        $response = $this->browser->sendRequest($request);
+        $neededForOne = microtime(true) - $start;
+
+        $sessionCookies = array_map(static function ($cookie) {
+            return Cookie::createFromRawSetCookieHeader($cookie);
+        }, $response->getHeader('Set-Cookie'));
+        self::assertNotEmpty($sessionCookies);
+
+        $cookies = array_reduce($sessionCookies, static function ($out, $cookie) {
+            $out[$cookie->getName()] = $cookie->getValue();
+            return $out;
+        }, []);
+        $nextRequest = $this->serverRequestFactory
+            ->createServerRequest('GET', new Uri($url))
+            ->withCookieParams($cookies);
+        $childs = [];
+        $start = microtime(true);
+        for ($i = 0; $i < 4; $i++) {
+            $child = \pcntl_fork();
+            if ($child === 0) {
+                $this->browser->sendRequest($nextRequest);
+                exit();
+            }
+            $childs[] = $child;
+        }
+        foreach ($childs as $child) {
+            \pcntl_waitpid($child, $status);
+        }
+        $neededForAll = microtime(true) - $start;
+
+        $checker($neededForAll, $neededForOne);
+    }
+}

composer.json

@@ -1,25 +1,14 @@
 {
     "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.3.0",
+        "php": ">=7.4",
+        "symfony/lock": "^5.2.0"
+    },
+    "require-dev": {
+        "ext-pcntl": "*"
     },
     "autoload": {
         "psr-4": {
@@ -34,5 +23,19 @@
             "dev-develop": "3.0.x-dev",
             "dev-version/2.x-dev": "2.1.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"
+    ]
 }