Skip to content

Commit

Permalink
Fix mismatched URL comparisions in SecurityComponent.
Browse files Browse the repository at this point in the history
When SecurityComponent was updated to use new request methods, I forgot
to take into account base URL paths. In order to get those applied, we
can use Router::url(). This ensures we get the same urls that FormHelper
creates. I've also fixed an issue where if the session hadn't already
been started secured forms would fail as session_id() would return ''
instead of the actual session id.

Refs #11932
  • Loading branch information
markstory committed Apr 17, 2018
1 parent 887dc4d commit e2ed51f
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 5 deletions.
16 changes: 12 additions & 4 deletions src/Controller/Component/SecurityComponent.php
Expand Up @@ -22,6 +22,7 @@
use Cake\Event\Event;
use Cake\Http\Exception\BadRequestException;
use Cake\Http\ServerRequest;
use Cake\Routing\Router;
use Cake\Utility\Hash;
use Cake\Utility\Security;

Expand Down Expand Up @@ -378,14 +379,21 @@ protected function _validToken(Controller $controller)
*/
protected function _hashParts(Controller $controller)
{
$fieldList = $this->_fieldsList($controller->request->getData());
$unlocked = $this->_sortedUnlocked($controller->request->getData());
$request = $controller->getRequest();

// Start the session to ensure we get the correct session id.
$session = $request->getSession();
$session->start();

$data = $request->getData();
$fieldList = $this->_fieldsList($data);
$unlocked = $this->_sortedUnlocked($data);

return [
$controller->request->getRequestTarget(),
Router::url($request->getRequestTarget()),
serialize($fieldList),
$unlocked,
session_id(),
$session->id()
];
}

Expand Down
34 changes: 33 additions & 1 deletion tests/TestCase/Controller/Component/SecurityComponentTest.php
Expand Up @@ -21,6 +21,7 @@
use Cake\Event\Event;
use Cake\Http\ServerRequest;
use Cake\Http\Session;
use Cake\Routing\Router;
use Cake\TestSuite\TestCase;
use Cake\Utility\Security;

Expand Down Expand Up @@ -189,7 +190,7 @@ public function tearDown()
unset($this->Controller);
}

public function validatePost($expectedException = null, $expectedExceptionMessage = null)
public function validatePost($expectedException = 'SecurityException', $expectedExceptionMessage = null)
{
try {
return $this->Controller->Security->validatePost($this->Controller);
Expand Down Expand Up @@ -744,6 +745,37 @@ public function testValidatePostSimple()
$this->assertTrue($result);
}

/**
* test validatePost uses full URL
*
* @return void
* @triggers Controller.startup $this->Controller
*/
public function testValidatePostSubdirectory()
{
// set the base path.
$this->Controller->request = $this->Controller->request
->withAttribute('base', 'subdir')
->withAttributE('webroot', 'subdir/');
Router::pushRequest($this->Controller->request);

$event = new Event('Controller.startup', $this->Controller);
$this->Security->startup($event);

// Differs from testValidatePostSimple because of base url
$fields = 'cc9b6af3f33147235ae8f8037b0a71399a2425f2%3A';
$unlocked = '';
$debug = '';

$this->Controller->request = $this->Controller->request->withParsedBody([
'Model' => ['username' => '', 'password' => ''],
'_Token' => compact('fields', 'unlocked', 'debug')
]);

$result = $this->validatePost();
$this->assertTrue($result);
}

/**
* testValidatePostComplex method
*
Expand Down

0 comments on commit e2ed51f

Please sign in to comment.