Browse Source

Fix SecurityComponent using deprecated properties.

This has required a minor breaking change in `generateToken()`. Because
of how this method was setup it was impossible to make a backwards
compatible change. I'm relying on the hope that not many people directly
invoke this method and that the break will not disrupt people too
greatly.
Mark Story 8 years ago
parent
commit
823870ce53

+ 16 - 14
src/Controller/Component/SecurityComponent.php

@@ -101,14 +101,15 @@ class SecurityComponent extends Component
     {
         /* @var \Cake\Controller\Controller $controller */
         $controller = $event->getSubject();
-        $this->session = $controller->request->getSession();
-        $this->_action = $controller->request->getParam('action');
-        $hasData = ($controller->request->getData() || $controller->request->is(['put', 'post', 'delete', 'patch']));
+        $request = $controller->request;
+        $this->session = $request->getSession();
+        $this->_action = $request->getParam('action');
+        $hasData = ($request->getData() || $request->is(['put', 'post', 'delete', 'patch']));
         try {
             $this->_secureRequired($controller);
             $this->_authRequired($controller);
 
-            $isNotRequestAction = !$controller->request->getParam('requested');
+            $isNotRequestAction = !$request->getParam('requested');
 
             if ($this->_action === $this->_config['blackHoleCallback']) {
                 throw new AuthSecurityException(sprintf('Action %s is defined as the blackhole callback.', $this->_action));
@@ -117,17 +118,19 @@ class SecurityComponent extends Component
             if (!in_array($this->_action, (array)$this->_config['unlockedActions']) &&
                 $hasData &&
                 $isNotRequestAction &&
-                $this->_config['validatePost']) {
+                $this->_config['validatePost']
+            ) {
                     $this->_validatePost($controller);
             }
         } catch (SecurityException $se) {
             $this->blackHole($controller, $se->getType(), $se);
         }
 
-        $this->generateToken($controller->request);
+        $request = $this->generateToken($request);
         if ($hasData && is_array($controller->request->getData())) {
-            unset($controller->request->data['_Token']);
+            $request = $request->withoutData('_Token');
         }
+        $controller->request = $request;
     }
 
     /**
@@ -556,16 +559,16 @@ class SecurityComponent extends Component
      * request object.
      *
      * @param \Cake\Http\ServerRequest $request The request object to add into.
-     * @return bool
+     * @return \Cake\Http\ServerRequest The modified request.
      */
     public function generateToken(ServerRequest $request)
     {
         if ($request->is('requested')) {
             if ($this->session->check('_Token')) {
-                $request->params['_Token'] = $this->session->read('_Token');
+                $request = $request->withParam('_Token', $this->session->read('_Token'));
             }
 
-            return false;
+            return $request;
         }
         $token = [
             'allowedControllers' => $this->_config['allowedControllers'],
@@ -574,11 +577,10 @@ class SecurityComponent extends Component
         ];
 
         $this->session->write('_Token', $token);
-        $request->params['_Token'] = [
-            'unlockedFields' => $token['unlockedFields']
-        ];
 
-        return true;
+        return $request->withParam('_Token', [
+            'unlockedFields' => $token['unlockedFields']
+        ]);
     }
 
     /**

+ 85 - 85
tests/TestCase/Controller/Component/SecurityComponentTest.php

@@ -446,10 +446,10 @@ class SecurityComponentTest extends TestCase
         $unlocked = '';
         $debug = '';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => ['username' => 'nate', 'password' => 'foo', 'valid' => '0'],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
         $this->assertTrue($this->validatePost());
     }
 
@@ -506,10 +506,10 @@ class SecurityComponentTest extends TestCase
 
         $fields = 'a5475372b40f6e3ccbf9f8af191f20e1642fd877%3AModel.valid';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => ['username' => 'nate', 'password' => 'foo', 'valid' => '0'],
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
         $this->assertFalse($this->validatePost('AuthSecurityException', 'Unexpected field \'Model.password\' in POST data, Unexpected field \'Model.username\' in POST data'));
     }
 
@@ -529,10 +529,10 @@ class SecurityComponentTest extends TestCase
 
         $fields = 'a5475372b40f6e3ccbf9f8af191f20e1642fd877%3AModel.valid';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => ['username' => 'nate', 'password' => 'foo', 'valid' => '0'],
             '_Token' => compact('fields')
-        ];
+        ]);
         $this->assertFalse($this->validatePost('AuthSecurityException', '\'_Token.unlocked\' was not found in request data.'));
     }
 
@@ -550,10 +550,10 @@ class SecurityComponentTest extends TestCase
         $this->Security->startup($event);
         $unlocked = '';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => ['username' => 'nate', 'password' => 'foo', 'valid' => '0'],
             '_Token' => compact('unlocked')
-        ];
+        ]);
         $result = $this->validatePost('AuthSecurityException', '\'_Token.fields\' was not found in request data.');
         $this->assertFalse($result, 'validatePost passed when fields were missing. %s');
     }
@@ -602,10 +602,10 @@ class SecurityComponentTest extends TestCase
         $attack = 'O:3:"App":1:{s:5:"__map";a:1:{s:3:"foo";s:7:"Hacked!";s:1:"fail"}}';
         $fields .= urlencode(':' . str_rot13($attack));
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => ['username' => 'mark', 'password' => 'foo', 'valid' => '0'],
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
         $result = $this->validatePost('SecurityException', 'Bad Request');
         $this->assertFalse($result, 'validatePost passed when key was missing. %s');
     }
@@ -627,11 +627,11 @@ class SecurityComponentTest extends TestCase
         $unlocked = '';
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             '_csrfToken' => 'abc123',
             'Model' => ['multi_field' => ['1', '3']],
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
         $this->assertTrue($this->validatePost());
     }
 
@@ -656,16 +656,16 @@ class SecurityComponentTest extends TestCase
             []
         ]));
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => ['multi_field' => ['1', '3']],
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
         $this->assertTrue($this->validatePost());
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => ['multi_field' => [12 => '1', 20 => '3']],
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
         $this->assertTrue($this->validatePost());
     }
 
@@ -689,10 +689,10 @@ class SecurityComponentTest extends TestCase
             []
         ]));
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             1 => 'value,',
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
         $this->assertTrue($this->validatePost());
     }
 
@@ -711,10 +711,10 @@ class SecurityComponentTest extends TestCase
         $unlocked = '';
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'anything' => 'some_data',
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
 
         $result = $this->validatePost();
         $this->assertTrue($result);
@@ -735,10 +735,10 @@ class SecurityComponentTest extends TestCase
         $unlocked = '';
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => ['username' => '', 'password' => ''],
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
 
         $result = $this->validatePost();
         $this->assertTrue($result);
@@ -761,7 +761,7 @@ class SecurityComponentTest extends TestCase
         $unlocked = '';
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Addresses' => [
                 '0' => [
                     'id' => '123456', 'title' => '', 'first_name' => '', 'last_name' => '',
@@ -773,7 +773,7 @@ class SecurityComponentTest extends TestCase
                 ]
             ],
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
         $result = $this->validatePost();
         $this->assertTrue($result);
     }
@@ -795,33 +795,33 @@ class SecurityComponentTest extends TestCase
         $unlocked = '';
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Tag' => ['Tag' => [1, 2]],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
         $result = $this->validatePost();
         $this->assertTrue($result);
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Tag' => ['Tag' => [1, 2, 3]],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
         $result = $this->validatePost();
         $this->assertTrue($result);
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Tag' => ['Tag' => [1, 2, 3, 4]],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
         $result = $this->validatePost();
         $this->assertTrue($result);
 
         $fields = '1e4c9269b64756e9b141d364497c5f037b428a37%3A';
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'User.password' => 'bar', 'User.name' => 'foo', 'User.is_valid' => '1',
             'Tag' => ['Tag' => [1]],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
         $result = $this->validatePost();
         $this->assertTrue($result);
     }
@@ -843,31 +843,31 @@ class SecurityComponentTest extends TestCase
         $unlocked = '';
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => ['username' => '', 'password' => '', 'valid' => '0'],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
 
         $result = $this->validatePost();
         $this->assertTrue($result);
 
         $fields = '3f368401f9a8610bcace7746039651066cdcdc38%3A';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => ['username' => '', 'password' => '', 'valid' => '0'],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
 
         $result = $this->validatePost();
         $this->assertTrue($result);
 
-        $this->Controller->request->data = [];
+        $this->Controller->request = $this->Controller->request->withParsedBody([]);
         $this->Security->startup($event);
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => ['username' => '', 'password' => '', 'valid' => '0'],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
 
         $result = $this->validatePost();
         $this->assertTrue($result);
@@ -887,13 +887,13 @@ class SecurityComponentTest extends TestCase
         $unlocked = '';
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => [
                 'username' => '', 'password' => '', 'hidden' => '0',
                 'other_hidden' => 'some hidden value'
             ],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
         $result = $this->validatePost();
         $this->assertTrue($result);
     }
@@ -913,12 +913,12 @@ class SecurityComponentTest extends TestCase
         $unlocked = '';
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => [
                 'username' => '', 'password' => '', 'hidden' => '0'
             ],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
 
         $result = $this->validatePost();
         $this->assertTrue($result);
@@ -943,14 +943,14 @@ class SecurityComponentTest extends TestCase
         );
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => [
                 'username' => 'mark',
                 'password' => 'sekret',
                 'hidden' => '0'
             ],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
 
         $result = $this->validatePost();
         $this->assertTrue($result);
@@ -971,14 +971,14 @@ class SecurityComponentTest extends TestCase
         $fields = ['Model.hidden', 'Model.password', 'Model.username'];
         $fields = urlencode(Security::hash(serialize($fields) . Security::getSalt()));
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => [
                 'username' => 'mark',
                 'password' => 'sekret',
                 'hidden' => '0'
             ],
             '_Token' => compact('fields')
-        ];
+        ]);
 
         $result = $this->validatePost('SecurityException', '\'_Token.unlocked\' was not found in request data.');
         $this->assertFalse($result);
@@ -1000,14 +1000,14 @@ class SecurityComponentTest extends TestCase
         $fields = urlencode(Security::hash(serialize($fields) . Security::getSalt()));
         $unlocked = '';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => [
                 'username' => 'mark',
                 'password' => 'sekret',
                 'hidden' => '0'
             ],
             '_Token' => compact('fields', 'unlocked')
-        ];
+        ]);
 
         $result = $this->validatePost('SecurityException', '\'_Token.debug\' was not found in request data.');
         $this->assertFalse($result);
@@ -1029,14 +1029,14 @@ class SecurityComponentTest extends TestCase
         $fields = urlencode(Security::hash(serialize($fields) . Security::getSalt()));
         $unlocked = '';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => [
                 'username' => 'mark',
                 'password' => 'sekret',
                 'hidden' => '0'
             ],
             '_Token' => compact('fields', 'unlocked')
-        ];
+        ]);
         Configure::write('debug', false);
         $result = $this->validatePost('SecurityException', 'The request has been black-holed');
     }
@@ -1065,14 +1065,14 @@ class SecurityComponentTest extends TestCase
         // Tamper the values.
         $unlocked = 'Model.username|Model.password';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => [
                 'username' => 'mark',
                 'password' => 'sekret',
                 'hidden' => '0'
             ],
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
 
         $result = $this->validatePost('SecurityException', 'Missing field \'Model.password\' in POST data, Unexpected unlocked field \'Model.password\' in POST data');
         $this->assertFalse($result);
@@ -1092,12 +1092,12 @@ class SecurityComponentTest extends TestCase
         $unlocked = '';
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => ['username' => '', 'password' => '', 'valid' => '0'],
             'Model2' => ['valid' => '0'],
             'Model3' => ['valid' => '0'],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
         $result = $this->validatePost();
         $this->assertTrue($result);
     }
@@ -1117,7 +1117,7 @@ class SecurityComponentTest extends TestCase
         $unlocked = '';
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => [
                 [
                     'username' => 'username', 'password' => 'password',
@@ -1129,7 +1129,7 @@ class SecurityComponentTest extends TestCase
                 ]
             ],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
 
         $result = $this->validatePost();
         $this->assertTrue($result);
@@ -1150,7 +1150,7 @@ class SecurityComponentTest extends TestCase
         $unlocked = '';
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Address' => [
                 0 => [
                     'id' => '123',
@@ -1174,7 +1174,7 @@ class SecurityComponentTest extends TestCase
                 ]
             ],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
 
         $result = $this->validatePost();
         $this->assertTrue($result);
@@ -1199,13 +1199,13 @@ class SecurityComponentTest extends TestCase
         );
         $debug = 'not used';
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'TaxonomyData' => [
                 1 => [[2]],
                 2 => [[3]]
             ],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
         $result = $this->validatePost();
         $this->assertTrue($result);
     }
@@ -1248,7 +1248,7 @@ class SecurityComponentTest extends TestCase
             []
         ]));
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Address' => [
                 0 => [
                     'id' => '123',
@@ -1272,7 +1272,7 @@ class SecurityComponentTest extends TestCase
                 ]
             ],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
         $result = $this->validatePost('SecurityException', 'Bad Request');
         $this->assertFalse($result);
     }
@@ -1296,20 +1296,20 @@ class SecurityComponentTest extends TestCase
             []
         ]));
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'MyModel' => ['name' => 'some data'],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
         $result = $this->validatePost('SecurityException', 'Unexpected field \'MyModel.name\' in POST data');
         $this->assertFalse($result);
 
         $this->Security->startup($event);
         $this->Security->setConfig('disabledFields', ['MyModel.name']);
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'MyModel' => ['name' => 'some data'],
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
 
         $result = $this->validatePost();
         $this->assertTrue($result);
@@ -1335,30 +1335,30 @@ class SecurityComponentTest extends TestCase
             []
         ]));
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             '_Token' => compact('fields', 'unlocked', 'debug'),
-        ];
+        ]);
         $result = $this->validatePost('SecurityException', 'Bad Request');
         $this->assertFalse($result);
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             '_Token' => compact('fields', 'unlocked', 'debug'),
             'Test' => ['test' => '']
-        ];
+        ]);
         $result = $this->validatePost();
         $this->assertTrue($result);
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             '_Token' => compact('fields', 'unlocked', 'debug'),
             'Test' => ['test' => '1']
-        ];
+        ]);
         $result = $this->validatePost();
         $this->assertTrue($result);
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             '_Token' => compact('fields', 'unlocked', 'debug'),
             'Test' => ['test' => '2']
-        ];
+        ]);
         $result = $this->validatePost();
         $this->assertTrue($result);
     }
@@ -1432,10 +1432,10 @@ class SecurityComponentTest extends TestCase
     public function testGenerateToken()
     {
         $request = $this->Controller->request;
-        $this->Security->generateToken($request);
+        $request = $this->Security->generateToken($request);
 
-        $this->assertNotEmpty($request->params['_Token']);
-        $this->assertTrue(isset($request->params['_Token']['unlockedFields']));
+        $this->assertNotEmpty($request->getParam('_Token'));
+        $this->assertSame([], $request->getParam('_Token.unlockedFields'));
     }
 
     /**
@@ -1450,7 +1450,7 @@ class SecurityComponentTest extends TestCase
     {
         $_SERVER['REQUEST_METHOD'] = 'POST';
         $event = new Event('Controller.startup', $this->Controller);
-        $this->Controller->request->data = ['data'];
+        $this->Controller->request = $this->Controller->request->withParsedBody(['data']);
         $this->Security->unlockedActions = 'index';
         $this->Security->blackHoleCallback = null;
         $result = $this->Controller->Security->startup($event);
@@ -1479,14 +1479,14 @@ class SecurityComponentTest extends TestCase
             ['not expected']
         ]));
 
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => [
                 'username' => 'mark',
                 'password' => 'sekret',
                 'hidden' => '0'
             ],
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
 
         $result = $this->validatePost('SecurityException', 'Invalid security debug token.');
         $this->assertFalse($result);
@@ -1555,13 +1555,13 @@ class SecurityComponentTest extends TestCase
         ]));
         $fields = urlencode(Security::hash(serialize($fields) . $unlocked . Security::getSalt()));
         $fields .= urlencode(':Model.hidden|Model.id');
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => [
                 'hidden' => 'tampered',
                 'id' => '1',
             ],
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
 
         $result = $this->validatePost('SecurityException', 'Tampered field \'Model.hidden\' in POST data (expected value \'value\' but found \'tampered\')');
         $this->assertFalse($result);
@@ -1621,13 +1621,13 @@ class SecurityComponentTest extends TestCase
         ]));
         $fields = urlencode(Security::hash(serialize($fields) . $unlocked . Security::getSalt()));
         $fields .= urlencode(':Model.hidden|Model.id');
-        $this->Controller->request->data = [
+        $this->Controller->request = $this->Controller->request->withParsedBody([
             'Model' => [
                 'hidden' => ['some-key' => 'some-value'],
                 'id' => '1',
             ],
             '_Token' => compact('fields', 'unlocked', 'debug')
-        ];
+        ]);
         Configure::write('debug', false);
         $result = $this->validatePost('SecurityException', 'Unexpected \'_Token.debug\' found in request data');
         $this->assertFalse($result);