Skip to content

Commit fe2ed1b

Browse files
cparticacpartica
authored
Merge pull request #2878 from magento-qwerty/MAGETWO-88431
[Qwerty] Request validation improvements
2 parents de27216 + d3b3b72 commit fe2ed1b

File tree

30 files changed

+1594
-1105
lines changed

30 files changed

+1594
-1105
lines changed

‎app/code/Magento/Backend/App/AbstractAction.php

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,6 @@ private function _moveBlockToContainer(\Magento\Framework\View\Element\AbstractB
205205
*/
206206
public function dispatch(\Magento\Framework\App\RequestInterface $request)
207207
{
208-
if (!$this->_processUrlKeys()) {
209-
return parent::dispatch($request);
210-
}
211-
212208
if ($request->isDispatched() && $request->getActionName() !== 'denied' && !$this->_isAllowed()) {
213209
$this->_response->setStatusHeader(403, '1.1', 'Forbidden');
214210
if (!$this->_auth->isLoggedIn()) {
@@ -252,6 +248,9 @@ protected function _isUrlChecked()
252248
* Check url keys. If non valid - redirect
253249
*
254250
* @return bool
251+
*
252+
* @see \Magento\Backend\App\Request\BackendValidator for default
253+
* request validation.
255254
*/
256255
public function _processUrlKeys()
257256
{

‎app/code/Magento/Backend/App/Request/BackendValidator.php

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
namespace Magento\Backend\App\Request;
10+
11+
use Magento\Backend\App\AbstractAction;
12+
use Magento\Framework\App\ActionInterface;
13+
use Magento\Framework\App\CsrfAwareActionInterface;
14+
use Magento\Framework\App\Request\InvalidRequestException;
15+
use Magento\Framework\App\Request\ValidatorInterface;
16+
use Magento\Framework\App\RequestInterface;
17+
use Magento\Backend\Model\Auth;
18+
use Magento\Framework\App\Request\Http as HttpRequest;
19+
use Magento\Framework\Controller\Result\RawFactory;
20+
use Magento\Framework\Controller\Result\Raw as RawResult;
21+
use Magento\Framework\Controller\Result\RedirectFactory;
22+
use Magento\Framework\Data\Form\FormKey\Validator as FormKeyValidator;
23+
use Magento\Backend\Model\UrlInterface as BackendUrl;
24+
use Magento\Framework\Phrase;
25+
26+
/**
27+
* Do backend validations.
28+
*
29+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
30+
*/
31+
class BackendValidator implements ValidatorInterface
32+
{
33+
/**
34+
* @var Auth
35+
*/
36+
private $auth;
37+
38+
/**
39+
* @var FormKeyValidator
40+
*/
41+
private $formKeyValidator;
42+
43+
/**
44+
* @var BackendUrl
45+
*/
46+
private $backendUrl;
47+
48+
/**
49+
* @var RedirectFactory
50+
*/
51+
private $redirectFactory;
52+
53+
/**
54+
* @var RawFactory
55+
*/
56+
private $rawResultFactory;
57+
58+
/**
59+
* @param Auth $auth
60+
* @param FormKeyValidator $formKeyValidator
61+
* @param BackendUrl $backendUrl
62+
* @param RedirectFactory $redirectFactory
63+
* @param RawFactory $rawResultFactory
64+
*/
65+
public function __construct(
66+
Auth $auth,
67+
FormKeyValidator $formKeyValidator,
68+
BackendUrl $backendUrl,
69+
RedirectFactory $redirectFactory,
70+
RawFactory $rawResultFactory
71+
) {
72+
$this->auth = $auth;
73+
$this->formKeyValidator = $formKeyValidator;
74+
$this->backendUrl = $backendUrl;
75+
$this->redirectFactory = $redirectFactory;
76+
$this->rawResultFactory = $rawResultFactory;
77+
}
78+
79+
/**
80+
* @param RequestInterface $request
81+
* @param ActionInterface $action
82+
*
83+
* @return bool
84+
*/
85+
private function validateRequest(
86+
RequestInterface $request,
87+
ActionInterface $action
88+
): bool {
89+
/** @var bool|null $valid */
90+
$valid = null;
91+
92+
if ($action instanceof CsrfAwareActionInterface) {
93+
$valid = $action->validateForCsrf($request);
94+
}
95+
96+
if ($valid === null) {
97+
$validFormKey = true;
98+
$validSecretKey = true;
99+
if ($request instanceof HttpRequest && $request->isPost()) {
100+
$validFormKey = $this->formKeyValidator->validate($request);
101+
} elseif ($this->auth->isLoggedIn()
102+
&& $this->backendUrl->useSecretKey()
103+
) {
104+
$secretKeyValue = (string)$request->getParam(
105+
BackendUrl::SECRET_KEY_PARAM_NAME,
106+
null
107+
);
108+
$secretKey = $this->backendUrl->getSecretKey();
109+
$validSecretKey = ($secretKeyValue === $secretKey);
110+
}
111+
$valid = $validFormKey && $validSecretKey;
112+
}
113+
114+
return $valid;
115+
}
116+
117+
/**
118+
* @param RequestInterface $request
119+
* @param ActionInterface $action
120+
*
121+
* @return InvalidRequestException
122+
*/
123+
private function createException(
124+
RequestInterface $request,
125+
ActionInterface $action
126+
): InvalidRequestException {
127+
/** @var InvalidRequestException|null $exception */
128+
$exception = null;
129+
130+
if ($action instanceof CsrfAwareActionInterface) {
131+
$exception = $action->createCsrfValidationException($request);
132+
}
133+
134+
if ($exception === null) {
135+
if ($request instanceof HttpRequest && $request->isAjax()) {
136+
//Sending empty response for AJAX request since we don't know
137+
//the expected response format and it's pointless to redirect.
138+
/** @var RawResult $response */
139+
$response = $this->rawResultFactory->create();
140+
$response->setHttpResponseCode(401);
141+
$response->setContents('');
142+
$exception = new InvalidRequestException($response);
143+
} else {
144+
//For regular requests.
145+
$response = $this->redirectFactory->create()
146+
->setUrl($this->backendUrl->getStartupPageUrl());
147+
$exception = new InvalidRequestException(
148+
$response,
149+
[
150+
new Phrase(
151+
'Invalid security or form key. Please refresh the page.'
152+
)
153+
]
154+
);
155+
}
156+
}
157+
158+
return $exception;
159+
}
160+
161+
/**
162+
* @inheritDoc
163+
*/
164+
public function validate(
165+
RequestInterface $request,
166+
ActionInterface $action
167+
): void {
168+
if ($action instanceof AbstractAction) {
169+
//Abstract Action has build-in validation.
170+
if (!$action->_processUrlKeys()) {
171+
throw new InvalidRequestException($action->getResponse());
172+
}
173+
} else {
174+
//Fallback validation.
175+
if (!$this->validateRequest($request, $action)) {
176+
throw $this->createException($request, $action);
177+
}
178+
}
179+
}
180+
}

‎app/code/Magento/Backend/etc/adminhtml/di.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
<preference for="Magento\Framework\App\DefaultPathInterface" type="Magento\Backend\App\DefaultPath" />
1515
<preference for="Magento\Backend\App\ConfigInterface" type="Magento\Backend\App\Config" />
1616
<preference for="Magento\Framework\App\Response\Http\FileFactory" type="Magento\Backend\App\Response\Http\FileFactory" />
17+
<preference for="Magento\Framework\App\Request\ValidatorInterface"
18+
type="Magento\Backend\App\Request\BackendValidator" />
1719
<type name="Magento\Framework\Stdlib\DateTime\Timezone">
1820
<arguments>
1921
<argument name="scopeType" xsi:type="const">Magento\Framework\App\Config\ScopeConfigInterface::SCOPE_TYPE_DEFAULT</argument>

‎app/code/Magento/Customer/Controller/Account/CreatePost.php

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,13 @@
1111
use Magento\Framework\App\Action\Context;
1212
use Magento\Customer\Model\Session;
1313
use Magento\Framework\App\Config\ScopeConfigInterface;
14+
use Magento\Framework\App\CsrfAwareActionInterface;
1415
use Magento\Framework\App\ObjectManager;
16+
use Magento\Framework\App\Request\InvalidRequestException;
17+
use Magento\Framework\App\RequestInterface;
18+
use Magento\Framework\Controller\Result\Redirect;
1519
use Magento\Framework\Exception\LocalizedException;
20+
use Magento\Framework\Phrase;
1621
use Magento\Store\Model\StoreManagerInterface;
1722
use Magento\Customer\Api\AccountManagementInterface;
1823
use Magento\Customer\Helper\Address;
@@ -29,12 +34,13 @@
2934
use Magento\Framework\Exception\StateException;
3035
use Magento\Framework\Exception\InputException;
3136
use Magento\Framework\Data\Form\FormKey\Validator;
37+
use Magento\Customer\Controller\AbstractAccount;
3238

3339
/**
3440
* @SuppressWarnings(PHPMD.TooManyFields)
3541
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
3642
*/
37-
class CreatePost extends \Magento\Customer\Controller\AbstractAccount
43+
class CreatePost extends AbstractAccount implements CsrfAwareActionInterface
3844
{
3945
/**
4046
* @var \Magento\Customer\Api\AccountManagementInterface
@@ -273,6 +279,31 @@ protected function extractAddress()
273279
return $addressDataObject;
274280
}
275281

282+
/**
283+
* @inheritDoc
284+
*/
285+
public function createCsrfValidationException(
286+
RequestInterface $request
287+
): ?InvalidRequestException {
288+
/** @var Redirect $resultRedirect */
289+
$resultRedirect = $this->resultRedirectFactory->create();
290+
$url = $this->urlModel->getUrl('*/*/create', ['_secure' => true]);
291+
$resultRedirect->setUrl($this->_redirect->error($url));
292+
293+
return new InvalidRequestException(
294+
$resultRedirect,
295+
[new Phrase('Invalid Form Key. Please refresh the page.')]
296+
);
297+
}
298+
299+
/**
300+
* @inheritDoc
301+
*/
302+
public function validateForCsrf(RequestInterface $request): ?bool
303+
{
304+
return null;
305+
}
306+
276307
/**
277308
* Create customer account action
278309
*
@@ -282,17 +313,19 @@ protected function extractAddress()
282313
*/
283314
public function execute()
284315
{
285-
/** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */
316+
/** @var Redirect $resultRedirect */
286317
$resultRedirect = $this->resultRedirectFactory->create();
287318
if ($this->session->isLoggedIn() || !$this->registration->isAllowed()) {
288319
$resultRedirect->setPath('*/*/');
289320
return $resultRedirect;
290321
}
291322

292-
if (!$this->getRequest()->isPost() || !$this->formKeyValidator->validate($this->getRequest())) {
323+
if (!$this->getRequest()->isPost()
324+
|| !$this->formKeyValidator->validate($this->getRequest())
325+
) {
293326
$url = $this->urlModel->getUrl('*/*/create', ['_secure' => true]);
294-
$resultRedirect->setUrl($this->_redirect->error($url));
295-
return $resultRedirect;
327+
return $this->resultRedirectFactory->create()
328+
->setUrl($this->_redirect->error($url));
296329
}
297330

298331
$this->session->regenerateId();
@@ -375,8 +408,7 @@ public function execute()
375408

376409
$this->session->setCustomerFormData($this->getRequest()->getPostValue());
377410
$defaultUrl = $this->urlModel->getUrl('*/*/create', ['_secure' => true]);
378-
$resultRedirect->setUrl($this->_redirect->error($defaultUrl));
379-
return $resultRedirect;
411+
return $resultRedirect->setUrl($this->_redirect->error($defaultUrl));
380412
}
381413

382414
/**

‎app/code/Magento/Customer/Controller/Account/EditPost.php

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,11 @@
1010
use Magento\Customer\Model\AuthenticationInterface;
1111
use Magento\Customer\Model\Customer\Mapper;
1212
use Magento\Customer\Model\EmailNotificationInterface;
13-
use Magento\Framework\App\Config\ScopeConfigInterface;
13+
use Magento\Framework\App\CsrfAwareActionInterface;
1414
use Magento\Framework\App\ObjectManager;
15+
use Magento\Framework\App\Request\InvalidRequestException;
16+
use Magento\Framework\App\RequestInterface;
17+
use Magento\Framework\Controller\Result\Redirect;
1518
use Magento\Framework\Data\Form\FormKey\Validator;
1619
use Magento\Customer\Api\AccountManagementInterface;
1720
use Magento\Customer\Api\CustomerRepositoryInterface;
@@ -21,12 +24,14 @@
2124
use Magento\Framework\Exception\InputException;
2225
use Magento\Framework\Exception\InvalidEmailOrPasswordException;
2326
use Magento\Framework\Exception\State\UserLockedException;
27+
use Magento\Customer\Controller\AbstractAccount;
28+
use Magento\Framework\Phrase;
2429

2530
/**
2631
* Class EditPost
2732
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2833
*/
29-
class EditPost extends \Magento\Customer\Controller\AbstractAccount
34+
class EditPost extends AbstractAccount implements CsrfAwareActionInterface
3035
{
3136
/**
3237
* Form code for data extractor
@@ -131,6 +136,30 @@ private function getEmailNotification()
131136
}
132137
}
133138

139+
/**
140+
* @inheritDoc
141+
*/
142+
public function createCsrfValidationException(
143+
RequestInterface $request
144+
): ?InvalidRequestException {
145+
/** @var Redirect $resultRedirect */
146+
$resultRedirect = $this->resultRedirectFactory->create();
147+
$resultRedirect->setPath('*/*/edit');
148+
149+
return new InvalidRequestException(
150+
$resultRedirect,
151+
[new Phrase('Invalid Form Key. Please refresh the page.')]
152+
);
153+
}
154+
155+
/**
156+
* @inheritDoc
157+
*/
158+
public function validateForCsrf(RequestInterface $request): ?bool
159+
{
160+
return null;
161+
}
162+
134163
/**
135164
* Change customer email or password action
136165
*
@@ -190,7 +219,10 @@ public function execute()
190219
$this->session->setCustomerFormData($this->getRequest()->getPostValue());
191220
}
192221

193-
return $resultRedirect->setPath('*/*/edit');
222+
/** @var Redirect $resultRedirect */
223+
$resultRedirect = $this->resultRedirectFactory->create();
224+
$resultRedirect->setPath('*/*/edit');
225+
return $resultRedirect;
194226
}
195227

196228
/**

‎app/code/Magento/Customer/Controller/Account/Login.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@
99
use Magento\Customer\Model\Session;
1010
use Magento\Framework\App\Action\Context;
1111
use Magento\Framework\View\Result\PageFactory;
12+
use Magento\Customer\Controller\AbstractAccount;
1213

13-
class Login extends \Magento\Customer\Controller\AbstractAccount
14+
class Login extends AbstractAccount
1415
{
1516
/**
1617
* @var Session

0 commit comments

Comments
 (0)