Skip to content

Commit

Permalink
MAGETWO-88431: Add mandatory form key validation to front controller
Browse files Browse the repository at this point in the history
  • Loading branch information
Oleksandr Gorkun committed Jul 19, 2018
1 parent 5a186a3 commit b475068
Show file tree
Hide file tree
Showing 30 changed files with 1,594 additions and 1,105 deletions.
7 changes: 3 additions & 4 deletions app/code/Magento/Backend/App/AbstractAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,6 @@ private function _moveBlockToContainer(\Magento\Framework\View\Element\AbstractB
*/
public function dispatch(\Magento\Framework\App\RequestInterface $request)
{
if (!$this->_processUrlKeys()) {
return parent::dispatch($request);
}

if ($request->isDispatched() && $request->getActionName() !== 'denied' && !$this->_isAllowed()) {
$this->_response->setStatusHeader(403, '1.1', 'Forbidden');
if (!$this->_auth->isLoggedIn()) {
Expand Down Expand Up @@ -252,6 +248,9 @@ protected function _isUrlChecked()
* Check url keys. If non valid - redirect
*
* @return bool
*
* @see \Magento\Backend\App\Request\BackendValidator for default
* request validation.
*/
public function _processUrlKeys()
{
Expand Down
180 changes: 180 additions & 0 deletions app/code/Magento/Backend/App/Request/BackendValidator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

declare(strict_types=1);

namespace Magento\Backend\App\Request;

use Magento\Backend\App\AbstractAction;
use Magento\Framework\App\ActionInterface;
use Magento\Framework\App\CsrfAwareActionInterface;
use Magento\Framework\App\Request\InvalidRequestException;
use Magento\Framework\App\Request\ValidatorInterface;
use Magento\Framework\App\RequestInterface;
use Magento\Backend\Model\Auth;
use Magento\Framework\App\Request\Http as HttpRequest;
use Magento\Framework\Controller\Result\RawFactory;
use Magento\Framework\Controller\Result\Raw as RawResult;
use Magento\Framework\Controller\Result\RedirectFactory;
use Magento\Framework\Data\Form\FormKey\Validator as FormKeyValidator;
use Magento\Backend\Model\UrlInterface as BackendUrl;
use Magento\Framework\Phrase;

/**
* Do backend validations.
*
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
*/
class BackendValidator implements ValidatorInterface
{
/**
* @var Auth
*/
private $auth;

/**
* @var FormKeyValidator
*/
private $formKeyValidator;

/**
* @var BackendUrl
*/
private $backendUrl;

/**
* @var RedirectFactory
*/
private $redirectFactory;

/**
* @var RawFactory
*/
private $rawResultFactory;

/**
* @param Auth $auth
* @param FormKeyValidator $formKeyValidator
* @param BackendUrl $backendUrl
* @param RedirectFactory $redirectFactory
* @param RawFactory $rawResultFactory
*/
public function __construct(
Auth $auth,
FormKeyValidator $formKeyValidator,
BackendUrl $backendUrl,
RedirectFactory $redirectFactory,
RawFactory $rawResultFactory
) {
$this->auth = $auth;
$this->formKeyValidator = $formKeyValidator;
$this->backendUrl = $backendUrl;
$this->redirectFactory = $redirectFactory;
$this->rawResultFactory = $rawResultFactory;
}

/**
* @param RequestInterface $request
* @param ActionInterface $action
*
* @return bool
*/
private function validateRequest(
RequestInterface $request,
ActionInterface $action
): bool {
/** @var bool|null $valid */
$valid = null;

if ($action instanceof CsrfAwareActionInterface) {
$valid = $action->validateForCsrf($request);
}

if ($valid === null) {
$validFormKey = true;
$validSecretKey = true;
if ($request instanceof HttpRequest && $request->isPost()) {
$validFormKey = $this->formKeyValidator->validate($request);
} elseif ($this->auth->isLoggedIn()
&& $this->backendUrl->useSecretKey()
) {
$secretKeyValue = (string)$request->getParam(
BackendUrl::SECRET_KEY_PARAM_NAME,
null
);
$secretKey = $this->backendUrl->getSecretKey();
$validSecretKey = ($secretKeyValue === $secretKey);
}
$valid = $validFormKey && $validSecretKey;
}

return $valid;
}

/**
* @param RequestInterface $request
* @param ActionInterface $action
*
* @return InvalidRequestException
*/
private function createException(
RequestInterface $request,
ActionInterface $action
): InvalidRequestException {
/** @var InvalidRequestException|null $exception */
$exception = null;

if ($action instanceof CsrfAwareActionInterface) {
$exception = $action->createCsrfValidationException($request);
}

if ($exception === null) {
if ($request instanceof HttpRequest && $request->isAjax()) {
//Sending empty response for AJAX request since we don't know
//the expected response format and it's pointless to redirect.
/** @var RawResult $response */
$response = $this->rawResultFactory->create();
$response->setHttpResponseCode(401);
$response->setContents('');
$exception = new InvalidRequestException($response);
} else {
//For regular requests.
$response = $this->redirectFactory->create()
->setUrl($this->backendUrl->getStartupPageUrl());
$exception = new InvalidRequestException(
$response,
[
new Phrase(
'Invalid security or form key. Please refresh the page.'
)
]
);
}
}

return $exception;
}

/**
* @inheritDoc
*/
public function validate(
RequestInterface $request,
ActionInterface $action
): void {
if ($action instanceof AbstractAction) {
//Abstract Action has build-in validation.
if (!$action->_processUrlKeys()) {
throw new InvalidRequestException($action->getResponse());
}
} else {
//Fallback validation.
if (!$this->validateRequest($request, $action)) {
throw $this->createException($request, $action);
}
}
}
}
2 changes: 2 additions & 0 deletions app/code/Magento/Backend/etc/adminhtml/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
<preference for="Magento\Framework\App\DefaultPathInterface" type="Magento\Backend\App\DefaultPath" />
<preference for="Magento\Backend\App\ConfigInterface" type="Magento\Backend\App\Config" />
<preference for="Magento\Framework\App\Response\Http\FileFactory" type="Magento\Backend\App\Response\Http\FileFactory" />
<preference for="Magento\Framework\App\Request\ValidatorInterface"
type="Magento\Backend\App\Request\BackendValidator" />
<type name="Magento\Framework\Stdlib\DateTime\Timezone">
<arguments>
<argument name="scopeType" xsi:type="const">Magento\Framework\App\Config\ScopeConfigInterface::SCOPE_TYPE_DEFAULT</argument>
Expand Down
46 changes: 39 additions & 7 deletions app/code/Magento/Customer/Controller/Account/CreatePost.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,13 @@
use Magento\Framework\App\Action\Context;
use Magento\Customer\Model\Session;
use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Framework\App\CsrfAwareActionInterface;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\App\Request\InvalidRequestException;
use Magento\Framework\App\RequestInterface;
use Magento\Framework\Controller\Result\Redirect;
use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\Phrase;
use Magento\Store\Model\StoreManagerInterface;
use Magento\Customer\Api\AccountManagementInterface;
use Magento\Customer\Helper\Address;
Expand All @@ -29,12 +34,13 @@
use Magento\Framework\Exception\StateException;
use Magento\Framework\Exception\InputException;
use Magento\Framework\Data\Form\FormKey\Validator;
use Magento\Customer\Controller\AbstractAccount;

/**
* @SuppressWarnings(PHPMD.TooManyFields)
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
*/
class CreatePost extends \Magento\Customer\Controller\AbstractAccount
class CreatePost extends AbstractAccount implements CsrfAwareActionInterface
{
/**
* @var \Magento\Customer\Api\AccountManagementInterface
Expand Down Expand Up @@ -273,6 +279,31 @@ protected function extractAddress()
return $addressDataObject;
}

/**
* @inheritDoc
*/
public function createCsrfValidationException(
RequestInterface $request
): ?InvalidRequestException {
/** @var Redirect $resultRedirect */
$resultRedirect = $this->resultRedirectFactory->create();
$url = $this->urlModel->getUrl('*/*/create', ['_secure' => true]);
$resultRedirect->setUrl($this->_redirect->error($url));

return new InvalidRequestException(
$resultRedirect,
[new Phrase('Invalid Form Key. Please refresh the page.')]
);
}

/**
* @inheritDoc
*/
public function validateForCsrf(RequestInterface $request): ?bool
{
return null;
}

/**
* Create customer account action
*
Expand All @@ -282,17 +313,19 @@ protected function extractAddress()
*/
public function execute()
{
/** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */
/** @var Redirect $resultRedirect */
$resultRedirect = $this->resultRedirectFactory->create();
if ($this->session->isLoggedIn() || !$this->registration->isAllowed()) {
$resultRedirect->setPath('*/*/');
return $resultRedirect;
}

if (!$this->getRequest()->isPost() || !$this->formKeyValidator->validate($this->getRequest())) {
if (!$this->getRequest()->isPost()
|| !$this->formKeyValidator->validate($this->getRequest())
) {
$url = $this->urlModel->getUrl('*/*/create', ['_secure' => true]);
$resultRedirect->setUrl($this->_redirect->error($url));
return $resultRedirect;
return $this->resultRedirectFactory->create()
->setUrl($this->_redirect->error($url));
}

$this->session->regenerateId();
Expand Down Expand Up @@ -375,8 +408,7 @@ public function execute()

$this->session->setCustomerFormData($this->getRequest()->getPostValue());
$defaultUrl = $this->urlModel->getUrl('*/*/create', ['_secure' => true]);
$resultRedirect->setUrl($this->_redirect->error($defaultUrl));
return $resultRedirect;
return $resultRedirect->setUrl($this->_redirect->error($defaultUrl));
}

/**
Expand Down
38 changes: 35 additions & 3 deletions app/code/Magento/Customer/Controller/Account/EditPost.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@
use Magento\Customer\Model\AuthenticationInterface;
use Magento\Customer\Model\Customer\Mapper;
use Magento\Customer\Model\EmailNotificationInterface;
use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Framework\App\CsrfAwareActionInterface;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\App\Request\InvalidRequestException;
use Magento\Framework\App\RequestInterface;
use Magento\Framework\Controller\Result\Redirect;
use Magento\Framework\Data\Form\FormKey\Validator;
use Magento\Customer\Api\AccountManagementInterface;
use Magento\Customer\Api\CustomerRepositoryInterface;
Expand All @@ -21,12 +24,14 @@
use Magento\Framework\Exception\InputException;
use Magento\Framework\Exception\InvalidEmailOrPasswordException;
use Magento\Framework\Exception\State\UserLockedException;
use Magento\Customer\Controller\AbstractAccount;
use Magento\Framework\Phrase;

/**
* Class EditPost
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
*/
class EditPost extends \Magento\Customer\Controller\AbstractAccount
class EditPost extends AbstractAccount implements CsrfAwareActionInterface
{
/**
* Form code for data extractor
Expand Down Expand Up @@ -131,6 +136,30 @@ private function getEmailNotification()
}
}

/**
* @inheritDoc
*/
public function createCsrfValidationException(
RequestInterface $request
): ?InvalidRequestException {
/** @var Redirect $resultRedirect */
$resultRedirect = $this->resultRedirectFactory->create();
$resultRedirect->setPath('*/*/edit');

return new InvalidRequestException(
$resultRedirect,
[new Phrase('Invalid Form Key. Please refresh the page.')]
);
}

/**
* @inheritDoc
*/
public function validateForCsrf(RequestInterface $request): ?bool
{
return null;
}

/**
* Change customer email or password action
*
Expand Down Expand Up @@ -190,7 +219,10 @@ public function execute()
$this->session->setCustomerFormData($this->getRequest()->getPostValue());
}

return $resultRedirect->setPath('*/*/edit');
/** @var Redirect $resultRedirect */
$resultRedirect = $this->resultRedirectFactory->create();
$resultRedirect->setPath('*/*/edit');
return $resultRedirect;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion app/code/Magento/Customer/Controller/Account/Login.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
use Magento\Customer\Model\Session;
use Magento\Framework\App\Action\Context;
use Magento\Framework\View\Result\PageFactory;
use Magento\Customer\Controller\AbstractAccount;

class Login extends \Magento\Customer\Controller\AbstractAccount
class Login extends AbstractAccount
{
/**
* @var Session
Expand Down
Loading

0 comments on commit b475068

Please sign in to comment.