Stop using pass by ref and use smaller methods.

Pass by reference makes code hard to follow and enables multiple return
values which is counter intuitive.

src/Http/Middleware/CsrfProtectionMiddleware.php

@@ -39,11 +39,11 @@ class CsrfProtectionMiddleware
     /**
      * Default config for the CSRF handling.
      *
-     *  - cookieName = The name of the cookie to send.
-     *  - expiry = How long the CSRF token should last. Defaults to browser session.
-     *  - secure = Whether or not the cookie will be set with the Secure flag. Defaults to false.
-     *  - httpOnly = Whether or not the cookie will be set with the HttpOnly flag. Defaults to false.
-     *  - field = The form field to check. Changing this will also require configuring
+     *  - `cookieName` = The name of the cookie to send.
+     *  - `expiry` = How long the CSRF token should last. Defaults to browser session.
+     *  - `secure` = Whether or not the cookie will be set with the Secure flag. Defaults to false.
+     *  - `httpOnly` = Whether or not the cookie will be set with the HttpOnly flag. Defaults to false.
+     *  - `field` = The form field to check. Changing this will also require configuring
      *    FormHelper.
      *
      * @var array
@@ -66,7 +66,7 @@ class CsrfProtectionMiddleware
     /**
      * Constructor
      *
-     * @param array $config Config options
+     * @param array $config Config options. See $_defaultConfig for valid keys.
      */
     public function __construct(array $config = [])
     {
@@ -97,11 +97,12 @@ class CsrfProtectionMiddleware
 
         $method = $request->getMethod();
         if ($method === 'GET' && $cookieData === null) {
-            list($request, $response) = $this->_setToken($request, $response);
+            $token = $this->_createToken();
+            $request = $this->_addTokenToRequest($token, $request);
+            $response = $this->_addTokenCookie($token, $request, $response);
 
             return $next($request, $response);
         }
-
         $request = $this->_validateAndUnsetTokenField($request);
 
         return $next($request, $response);
@@ -128,26 +129,44 @@ class CsrfProtectionMiddleware
     }
 
     /**
-     * Set the token in the response.
-     *
-     * Also sets the request->params['_csrfToken'] so the newly minted
-     * token is available in the request data.
+     * Create a new token to be used for CSRF protection
      *
-     * @param \Psr\Http\Message\ServerRequestInterface $request The request object.
-     * @param \Psr\Http\Message\ResponseInterface $response The response object.
-     * @return array
+     * @return string
      */
-    protected function _setToken(ServerRequestInterface $request, ResponseInterface $response)
+    protected function _createToken()
     {
-        $expiry = new Time($this->_config['expiry']);
-        $value = hash('sha512', Security::randomBytes(16), false);
+        return hash('sha512', Security::randomBytes(16), false);
+    }
 
+    /**
+     * Add a CSRF token to the request parameters.
+     *
+     * @param string $token The token to add.
+     * @param \Psr\Http\Message\ServerRequestInterface $request The request to augment
+     * @return \Psr\Http\Message\ServerRequestInterface Modified request
+     */
+    protected function _addTokenToRequest($token, ServerRequestInterface $request)
+    {
         $params = $request->getAttribute('params');
-        $params['_csrfToken'] = $value;
-        $request = $request->withAttribute('params', $params);
+        $params['_csrfToken'] = $token;
+
+        return $request->withAttribute('params', $params);
+    }
+
+    /**
+     * Add a CSRF token to the response cookies.
+     *
+     * @param string $token The token to add.
+     * @param \Psr\Http\Message\ServerRequestInterface $request The to get cookie path data from
+     * @param \Cake\Http\Response $response The response to augment
+     * @return \Cake\Http\Response Modified response
+     */
+    protected function _addTokenCookie($token, $request, $response)
+    {
+        $expiry = new Time($this->_config['expiry']);
 
-        $response = $response->withCookie($this->_config['cookieName'], [
-            'value' => $value,
+        return $response->withCookie($this->_config['cookieName'], [
+            'value' => $token,
             'expire' => $expiry->format('U'),
             'path' => $request->getAttribute('webroot'),
             'secure' => $this->_config['secure'],

tests/TestCase/Http/Middleware/CsrfProtectionMiddlewareTest.php

@@ -110,7 +110,8 @@ class CsrfProtectionMiddlewareTest extends TestCase
 
         // No exception means the test is valid
         $middleware = new CsrfProtectionMiddleware();
-        $middleware($request, $response, $this->_getNextClosure());
+        $response = $middleware($request, $response, $this->_getNextClosure());
+        $this->assertInstanceOf(Response::class, $response);
     }
 
     /**
@@ -133,7 +134,8 @@ class CsrfProtectionMiddlewareTest extends TestCase
 
         // No exception means the test is valid
         $middleware = new CsrfProtectionMiddleware();
-        $middleware($request, $response, $this->_getNextClosure());
+        $response = $middleware($request, $response, $this->_getNextClosure());
+        $this->assertInstanceOf(Response::class, $response);
     }
 
     /**
@@ -304,6 +306,7 @@ class CsrfProtectionMiddlewareTest extends TestCase
             'field' => 'token',
             'expiry' => 90,
         ]);
-        $middleware($request, $response, $this->_getNextClosure());
+        $response = $middleware($request, $response, $this->_getNextClosure());
+        $this->assertInstanceOf(Response::class, $response);
     }
 }