refactor and improve unit tests

src/Controller/Component/SecurityComponent.php

@@ -104,39 +104,31 @@ class SecurityComponent extends Component
         $controller = $event->subject();
         $this->session = $this->request->session();
         $this->_action = $this->request->params['action'];
+        $hasData = !empty($this->request->data);
         try {
             $this->_secureRequired($controller);
-        } catch (SecurityException $se) {
-            $this->blackHole($controller, $se->getType(), $se);
-        }
-        try {
             $this->_authRequired($controller);
-        } catch (AuthSecurityException $ase) {
-            $this->blackHole($controller, $ase->getType(), $ase);
-        }
 
+            $isNotRequestAction = (
+                !isset($controller->request->params['requested']) ||
+                $controller->request->params['requested'] != 1
+            );
 
-        $hasData = !empty($this->request->data);
-        $isNotRequestAction = (
-            !isset($controller->request->params['requested']) ||
-            $controller->request->params['requested'] != 1
-        );
-
-        if ($this->_action === $this->_config['blackHoleCallback']) {
-            return $this->blackHole($controller, 'auth');
-        }
+            if ($this->_action === $this->_config['blackHoleCallback']) {
+                throw new AuthSecurityException(sprintf('Action %s is defined as the blackhole callback.', $this->_action));
+            }
 
-        if (!in_array($this->_action, (array)$this->_config['unlockedActions']) &&
-            $hasData && $isNotRequestAction
-        ) {
-            if ($this->_config['validatePost']) {
-                try {
+            if (!in_array($this->_action, (array)$this->_config['unlockedActions']) &&
+                $hasData && $isNotRequestAction
+            ) {
+                if ($this->_config['validatePost']) {
                     $this->_validatePost($controller);
-                } catch (SecurityException $se) {
-                    return $this->blackHole($controller, $se->getType(), $se);
                 }
             }
+        } catch (SecurityException $se) {
+            $this->blackHole($controller, $se->getType(), $se);
         }
+
         $this->generateToken($controller->request);
         if ($hasData && is_array($controller->request->data)) {
             unset($controller->request->data['_Token']);
@@ -188,7 +180,8 @@ class SecurityComponent extends Component
      *
      * @param \Cake\Controller\Controller $controller Instantiating controller
      * @param string $error Error method
-     * @param \Cake\Controller\Exception\SecurityException $exception Additional debug info describing the cause
+     * @param \Cake\Controller\Exception\SecurityException $exception Additional debug info describing the cause,
+     * debug mode only
      * @return mixed If specified, controller blackHoleCallback's response, or no return otherwise
      * @see SecurityComponent::$blackHoleCallback
      * @link http://book.cakephp.org/3.0/en/controllers/components/security.html#handling-blackhole-callbacks
@@ -301,11 +294,7 @@ class SecurityComponent extends Component
      * Validate submitted form
      *
      * @param \Cake\Controller\Controller $controller Instantiating controller
-<<<<<<< HEAD
      * @throws \Cake\Controller\Exception\SecurityException
-=======
-     * @throws SecurityException
->>>>>>> e73f064c716f82b627e39b5ad3730282202f6670
      * @return bool true if submitted form is valid
      */
     protected function _validatePost(Controller $controller)

tests/TestCase/Controller/Component/SecurityComponentTest.php

@@ -1482,8 +1482,6 @@ class SecurityComponentTest extends TestCase
         $this->Controller->Security->config('requireAuth', ['protected']);
         $this->Controller->request->params['action'] = 'protected';
         $this->Controller->request->data = 'notEmpty';
-        $event = new Event('Controller.startup', $this->Controller);
-        $this->Controller->Security->startup($event);
         $this->Controller->Security->authRequired($this->Controller);
     }
 
@@ -1500,8 +1498,6 @@ class SecurityComponentTest extends TestCase
         $this->Controller->Security->config('requireAuth', ['protected']);
         $this->Controller->request->params['action'] = 'protected';
         $this->Controller->request->data = ['_Token' => 'not empty'];
-        $event = new Event('Controller.startup', $this->Controller);
-        $this->Controller->Security->startup($event);
         $this->Controller->Security->authRequired($this->Controller);
     }
 
@@ -1522,8 +1518,6 @@ class SecurityComponentTest extends TestCase
         $this->Controller->request->session()->write('_Token', [
             'allowedControllers' => ['Allowed', 'AnotherAllowed']
         ]);
-        $event = new Event('Controller.startup', $this->Controller);
-        $this->Controller->Security->startup($event);
         $this->Controller->Security->authRequired($this->Controller);
     }
 
@@ -1544,8 +1538,6 @@ class SecurityComponentTest extends TestCase
         $this->Controller->request->session()->write('_Token', [
             'allowedActions' => ['index', 'view']
         ]);
-        $event = new Event('Controller.startup', $this->Controller);
-        $this->Controller->Security->startup($event);
         $this->Controller->Security->authRequired($this->Controller);
     }
 
@@ -1565,8 +1557,6 @@ class SecurityComponentTest extends TestCase
             'allowedActions' => ['protected'],
             'allowedControllers' => ['Allowed'],
         ]);
-        $event = new Event('Controller.startup', $this->Controller);
-        $this->Controller->Security->startup($event);
         $this->assertTrue($this->Controller->Security->authRequired($this->Controller));
     }
 }