Fix implementation and Unit Tests

Author: lbajsarowicz

app/code/Magento/Customer/Controller/Plugin/Account.php

@@ -35,27 +35,20 @@ class Account
      * @var array
      */
     private $allowedActions = [];
-    /**
-     * @var ActionFlag
-     */
-    private $actionFlag;
 
     /**
      * @param RequestInterface $request
      * @param Session $customerSession
-     * @param ActionFlag $actionFlag
      * @param array $allowedActions List of actions that are allowed for not authorized users
      */
     public function __construct(
         RequestInterface $request,
         Session $customerSession,
-        ActionFlag $actionFlag,
         array $allowedActions = []
     ) {
+        $this->request = $request;
         $this->session = $customerSession;
         $this->allowedActions = $allowedActions;
-        $this->request = $request;
-        $this->actionFlag = $actionFlag;
     }
 
     /**
@@ -68,16 +61,11 @@ public function __construct(
     public function aroundExecute(AccountInterface $controllerAction, Closure $proceed)
     {
         if ($this->isActionAllowed()) {
-            $this->session->setNoReferer(true);
-            $response = $proceed();
-            $this->session->unsNoReferer(false);
-
-            return $response;
+            return $proceed();
         }
 
-        if (!$this->session->authenticate()) {
-            $this->actionFlag->set('', ActionInterface::FLAG_NO_DISPATCH, true);
-        }
+        /** @FIXME Move Authentication and redirect out of Session model */
+        $this->session->authenticate();
     }
 
     /**

app/code/Magento/Customer/Test/Unit/Controller/Plugin/AccountTest.php

@@ -6,6 +6,7 @@
 
 namespace Magento\Customer\Test\Unit\Controller\Plugin;
 
+use Closure;
 use Magento\Customer\Controller\AccountInterface;
 use Magento\Customer\Controller\Plugin\Account;
 use Magento\Customer\Model\Session;
@@ -83,40 +84,38 @@ protected function setUp()
     /**
      * @param string $action
      * @param array $allowedActions
-     * @param boolean $isActionAllowed
+     * @param boolean $isAllowed
      * @param boolean $isAuthenticated
      *
      * @dataProvider beforeExecuteDataProvider
      */
-    public function testBeforeExecute($action, $allowedActions, $isActionAllowed, $isAuthenticated)
+    public function testAroundExecuteInterruptsOriginalCallWhenNotAllowed(string $action, array $allowedActions, bool $isAllowed, bool $isAuthenticated)
     {
+        /** @var callable|MockObject $proceedMock */
+        $proceedMock = $this->getMockBuilder(\stdClass::class)
+            ->setMethods(['__invoke'])
+            ->getMock();
+
+        $closureMock = Closure::fromCallable($proceedMock);
+
         $this->requestMock->expects($this->once())
             ->method('getActionName')
             ->willReturn($action);
 
-        if ($isActionAllowed) {
-            $this->sessionMock->expects($this->once())
-                ->method('setNoReferer')
-                ->with(true)
-                ->willReturnSelf();
+        if ($isAllowed) {
+            $proceedMock->expects($this->once())->method('__invoke')->willReturn($this->resultMock);
         } else {
-            $this->sessionMock->expects($this->once())
-                ->method('authenticate')
-                ->willReturn($isAuthenticated);
-            if (!$isAuthenticated) {
-                $this->actionMock->expects($this->once())
-                    ->method('getActionFlag')
-                    ->willReturn($this->actionFlagMock);
-
-                $this->actionFlagMock->expects($this->once())
-                    ->method('set')
-                    ->with('', ActionInterface::FLAG_NO_DISPATCH, true)
-                    ->willReturnSelf();
-            }
+            $proceedMock->expects($this->never())->method('__invoke');
         }
 
         $plugin = new Account($this->requestMock, $this->sessionMock, $allowedActions);
-        $plugin->beforeExecute($this->actionMock);
+        $result = $plugin->aroundExecute($this->actionMock, $closureMock);
+
+        if ($isAllowed) {
+            $this->assertSame($this->resultMock, $result);
+        } else {
+            $this->assertNull($result);
+        }
     }
 
     /**
@@ -157,24 +156,4 @@ public function beforeExecuteDataProvider()
             ],
         ];
     }
-
-    public function testAfterExecute()
-    {
-        $this->sessionMock->expects($this->once())
-            ->method('unsNoReferer')
-            ->with(false)
-            ->willReturnSelf();
-
-        $plugin = (new ObjectManager($this))->getObject(
-            Account::class,
-            [
-                'session' => $this->sessionMock,
-                'allowedActions' => ['testaction']
-            ]
-        );
-        $this->assertSame(
-            $this->resultMock,
-            $plugin->afterExecute($this->actionMock, $this->resultMock, $this->requestMock)
-        );
-    }
 }